Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SegmentsView re-renders unnecessarily often #7895

Open
philippotto opened this issue Jun 25, 2024 · 0 comments · May be fixed by #7958
Open

SegmentsView re-renders unnecessarily often #7895

philippotto opened this issue Jun 25, 2024 · 0 comments · May be fixed by #7958

Comments

@philippotto
Copy link
Member

When the segments tab is visible, it rerenders unnecessarily often when moving through the dataset (other when the store is updated in any other way). I think, I tracked it down to the multiSelectMenu prop which is passed to <SegmentListItem />. That prop is a new instance on each render execution, meaning that it will also be rerendered by react.
The prop is responsible for the context menu of the segment items. However, the context menu only becomes relevant when the user opens that menu. Ideally, we would have a dummy menu (that is a constant) and then swap that menu as soon as the menu is really opened. However, I'm not sure how exactly this could be built.

philippotto added a commit that referenced this issue Jun 25, 2024
@philippotto philippotto self-assigned this Jul 29, 2024
@philippotto philippotto linked a pull request Jul 30, 2024 that will close this issue
7 tasks
philippotto added a commit that referenced this issue Jul 31, 2024
…ntend (#7654)

* add datastore route to list all agglomerate ids

* remove unused file access

* types

* Apply hdf5 mappings in frontend [WIP]

* Fix segment and agglomerate id mixup in proofreading saga

* add route to tracingstore

* edit mapping after merge and set it in redux store

* Fix mapping initialization

* WIP: implement split in frontend

* Only load subset of mapping for segments visible 5s after page load

* small fix

* cleanup, time measurements, throttle mapping requests to 500ms -> currently too laggy

* Update redux-saga to use throttle with effectChannels

* Only refresh mapping if bucket data changed. Fix partial mapping updates after master merge.

* Fix how proofreading actions update the mapping in the frontend

* disable most ci checks

* misc improvements

* refine shouldUseDataStore logic

* fix type error

* fix some type errors

* fix more type errors

* use NumberLike type for number and bigint at various places

* more NumberLike usages

* fix lots of typescript errors (related to bigint and to redux)

* fix more type errors

* fix the last type errors

* restore proper NumberLike definition and fix remaining type errors

* remove unused imports

* fix race condition which could cause mapping to not be properly initialized upon reload

* only pass isCentered instead of centeredSegmentId to SegmentListItems

* fix react warning when closing context menu for the first time

* add debug code for rare scenario where segment id is not an integer

* fix TS error

* fix that hovered unmapped segment id would not update sometimes

* avoid two map look ups

* add dryUpdate step before saving proofreading update actions

* undo provoking the error

* make selective segment visibility in proofreading an option

* fix NaN value after mapping unknown segment key

* avoid parallel executions of updateHdf5Mapping and also cancel updateHdf5Mapping if wk enters a busy state to avoid using outdated values

* always compare known segment ids to newest mapping instead of 'previous' one that isn't updated when doing proofreading operations

* don't crash completely when segment mapping is not known yet and user initiates proofreading action

* remove artificial delays

* improve logging

* fix ts error

* remove debugging code for NaN mapped id

* tweak hovering

* rename MIN_CUT_AGGLOMERATE actions

* adapt cutFromNeighbors to magic mapping approach

* avoid roundtrip for mesh reloading by using newest mapping

* tweak hovering (II)

* extract code into gatherInfoForOperation

* fix context menu for skeletons in 3d viewport

* make agglomerate-skeleton-proofreading compatible with magic mappings

* don't run rendering code for context menu when its not open

* remove unused previousMappingObject

* dynamically switch between local and remotely applied mappings when switching to/from proofreading tool etc. (unfortunately, buggy)

* auto-reload page if dev-proxy fails

* fix missing reload when disabling/re-enabling mapping (now too many reloads are performed)

* extract finishMappingInitialization action

* extract ensureMappingsAreLoadedAndRequestedMappingExists

* rename mappingIsEditable to hasEditableMapping

* introduce BucketRetrievalSource to better cancel/restart bucket watchers and reload the layer when necessary

* introduce clearMappingAction

* make sure that getBucketRetrievalSource doesn't create new instances all the time when multiple volume layers exist

* move cuckoo modules into libs/cuckoo

* use cuckoo hashing for gpu-based mapping instead of binary search (proper uint64 support needs to be tested)

* add missing module

* remove logging

* make use of protobuf shortcut getHasEditableMapping

* Update frontend/javascripts/oxalis/model/sagas/mapping_saga.ts

Co-authored-by: MichaelBuessemeyer <39529669+MichaelBuessemeyer@users.noreply.github.com>

* Update frontend/javascripts/oxalis/model/sagas/proofread_saga.ts

Co-authored-by: MichaelBuessemeyer <39529669+MichaelBuessemeyer@users.noreply.github.com>

* mappingIsEditable -> hasEditableMapping to fix compilation error

* remove unnecessary braces

* incorporate some pr feedback

* more pr feedback

* add comment to cuckoo table 64 bit

* also implement cuckoo table for uint32 keys and values

* fix invalid initialization of mapping texture when mapping is disabled

* fix that mapping was applied twice

* fix uint32 cuckoo implementation (still hardcoded to always use 32 bit)

* use 64 bit look up when necessary in shader

* remove unused mappingSize

* avoid toolbar rerendering

* only write necessary changes to cuckoo table instead of rewriting it from scratch every time

* ensure diffing of previous and new mapping is fast be providing cached diff operation for which the cache is manually set by the mapping saga

* eagerly compute value set when reasonable to avoid clustering of that computation and to improve FPS

* add missing worker

* implement maintenance of value set for entire data cube to avoid recomputation

* Revert maintenance of value set because it didn't help with performance

This reverts commit c7bb508.

* remove todo because it was tried in c7bb508 and reverted afterwards

* remove some time logging

* optimize/combine some set operations (2x as fast now)

* move and rename diffSets function

* further optimization of set operations (in total 4x faster compared to the initial version)

* remove unused code

* remove superfluous parameter

* add renaming todo

* delete create_set_from_array webworker as it didn't show an improved FPS rate (therefore, the overhead of passing the data doesn't seem reasonable)

* delete debugging code from cuckoo modules

* move attemptMappingLookUp glsl code

* add some comments

* reactivate CI checks

* remove unused code

* remove obsolete imports

* simplify editableMapping check

* fix ts problems

* fix cyclic dependencies

* fix 64 bit mapping rendering

* remove UI warnings that the merger mode doesn't support 64 bit, because now it does

* fix linting

* fix proto related tests

* fix more specs

* fix pullqueue spec

* refactor setNumberLike in cuckoo tables and update some todo comments

* remove unused route, change tracingstore mapping route to proto ListOfLong

* toSet

* remove unused getAgglomerateIdForSegmentId

* fix rendering bug outside of viewport on some GPUs

* adapt agglomeratesForSegments route to new protobuf interface

* link 64-bit issue (#6921) in todo comments

* introduce mappingIsPartial uniform

* mention #7895 in todo comment

* remove commented code

* avoid expensive console.log for large mappings

* disable most ci checks

* fix incorrect hash table size and use hashCombine twice to fix poor capacity utilization due to suboptimal hash distribution

* fix cuckootable rehash (did redundant work) and adapt max iterations parameter

* if many inserts are done for the mapping, flush the table at the end

* fix weak hashing for power-of-two table sizes

* refactor diminished hash capacity tweak; clean up and extend tests so that maxing out capacity is tested, too

* clean up reloadData related code in mapping_saga

* fix serializing bigint to protobuf long

* fix toggling of json mappings

* group consecutive actions in action logger middleware; add debug logging for dispatched actions

* fix that forceful disabled tool wasn't re-enabled when possible (e.g., after toggling segmentation opacity)

* mention 64 bit support issue in comment

* fix broken mapping of ids by sorting the input ids for the server

* add comment about sorting

* don't map ids dynamically in segment list view (instead segment items are created with the mapped id if a mapping exists); see #7919 as a follow-up

* test reaching critical capacity and remove todo comment

* remove some todo comments regarding mapId code that might return unmapped ids if the mapping is partial (impact should be low, add comments for it)

* remove more todos and fix toggling of hdf mappings when no volume tracing exists

* cast to number when sorting bigint

* use bigint in proofreading_saga where sensible and cast to number as late as possible (e.g., in action creators, in REST API etc)

* disable more ci stuff

* try to handle most ids as Number and cast to Number only when dealing with mapping object and buckets

* show zoom warning for agglomerate files only when the mapping is applied remotely

* fix that supervoxel highlighting of mesh stays active after leaving proofreading tool (fixes #7867)

* fix that after changing the color of a mesh via the segments tab the mesh is always highlighted after initial hover (fixes #7845)

* remove unused imports

* fix proper type of values returned from getAgglomeratesForSegmentsFrom*

* fix incorrect bigint check and refactor to avoid the same mistake in the future

* fix unnecessary type adaption that failed on null

* fix another sorting bug

* integrate pr feedback

* rename cuckoo table to CuckooTableVec3

* unify 64 bit todo comments

* forceful -> forcefully

* refactor eager value set computation

* use 0s to initialize mapping uniforms

* refactor/fix mapId logic for unmapped ids

* fix selective visibility for alpha != 0.2

* only emit soft errors when a data value could not be mapped

* fix mapping message hiding too early/never; fix disabled message in mapping UI

* misc console stuff

* also skip texture updates for cuckoo table when lots of unsets need to be done

* show short user notification when segmentation layer is reloaded

* highlight whole segment mesh on hover even when geometry is not merged (i.e., super-voxels are highlightable) if not in proofreading tool

* use current mag when reading segment ids in proofreading (unless agglomerate skeletons are used)

* remove last todo comments

* update changelog

* remove console.log

* re-enable ci checks

* fix linting

---------

Co-authored-by: Daniel Werner <daniel.werner@scalableminds.com>
Co-authored-by: Charlie Meister <charlie.meister@student.hpi.de>
Co-authored-by: Philipp Otto <philipp.4096@gmail.com>
Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
Co-authored-by: MichaelBuessemeyer <39529669+MichaelBuessemeyer@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant