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

Connectome Tab #5894

Merged
merged 108 commits into from
Feb 28, 2022
Merged

Connectome Tab #5894

merged 108 commits into from
Feb 28, 2022

Conversation

daniel-wer
Copy link
Member

@daniel-wer daniel-wer commented Dec 7, 2021

This PR implements the internal work package D.1 Connectome Viewing in webKnossos (see internal communication). Refer to the work package for the full feature set and a mockup.

This PR adds a new "Connectome Tab" and I tried to encapsulate the new functionality fairly well so that most of it is contained in a couple of files. Still, I had to adapt the code in a couple of spots to support some new use cases which I'll describe in the following:

  • Allow to set the enableRenderOnDemand prop for layout tabs. Setting the prop to false will disable the lazy-loading of the tab and instead always render it. For the connectome tab, this is important to allow hotlinking to agglomerates/synapses without the tab being visible when loading the annotation. Otherwise, the code encapsulation would not work as well. I don't think this is a problem, though, since the tab doesn't do much if it's not actively being used.
  • Allow to add multiple Skeletons (skeleton.js) to the scene. This is used to separate the connectome skeleton data from the "normal" skeleton data so the two don't interfere with each other and the connectome skeleton data is effectively read-only. Also allow to remove a Skeleton from the scene. This is done during cleanup or when the segmentation layer is switched.
  • Introduce the lastVisibleSegmentationLayerName to allow to continue to use the connectome tab even if the segmentation was temporarily disabled. This might be useful in other spots as well.

ToDo:

  • Separate connectome_view.js into separate logical units
  • Adapt docs/sharing.md#sharing-link-format to new link format
  • Check all check boxes that represent the same segment id (not only the clicked one). No idea what would be a good way to do that yet
  • Show the segment color next to the entry in the tree (if the agglomerate skeleton is loaded) Wait for feedback, maybe followup
  • The connectome tab should work in view mode
  • Do we need a layout migration or is it sufficient to increase the layout version number?
  • Hovering over a Segment entry in the tab could highlight the segment in the data viewport. The same feature exists for the segment tab, so it might be easy to copy.
  • The context menu in the data viewport could have an entry for loading a segment into the connectome tab
  • If an agglomerate could not be loaded, because the node limit is exceeded, a nicer error message could be rendered. Currently, one has to expand the details of the error message to find out that this is not a bug.

URL of deployed dev instance (used for testing):

Steps to test:

  • Open a dataset without a connectome. The connectome tab should state there is no connectome.
  • Use the l4dense_motta_et_al_dev dataset that has a linked connectome file on the deployed dev instance
  • The connectome tab should work in view mode as well as in annotations
  • Right-Click on a segment in the viewport and select "Import Agglomerate and Synapses". The segment should be shown in the connectome tab, its agglomerate skeleton loaded and all synapses shown as single node trees as well.
    • The same should happen if you enter the segment ID by hand
  • Entering two segment IDs (comma-separated) should show both skeletons and only the synapses between the two agglomerates
  • It should be possible to toggle the segmentation layer while a segment is shown in the connectome tab without the info in the connectome tab to disappear
  • For datasets with multiple segmentation layers, if the segmentation layer is switched the tab should be reset
  • The filters by synapse direction or synapse type should work and hide the filtered synapses/agglomerates from the tree list as well as the viewports
  • Right-click on a 2nd-level segment in the tree list and select "Show All Synapses of This Segment" to see the synapses of that segment, instead
  • The "Reset" buttons to reset the filters as well as the connectome tab should work
  • It should be possible to link to a specific agglomerate and its synapses (tested in the screenshot tests), for example, this link

Issues:


@daniel-wer daniel-wer self-assigned this Dec 7, 2021
daniel-wer and others added 19 commits December 7, 2021 14:59
…ynapse types and display as Tag next to synapse
…ynamically diff connectome data and add/delete synapse trees accordingly, add reset button
…ve agglomerate skeletons if they are no longer part of the connectome data
…state level to remove filtered synapse trees as well
@daniel-wer
Copy link
Member Author

What does the frontend do if a wkconnect dataset is opened, where the new routes do not exist? Should we add the list connectomes route there (always returning empty array)?

Yes, good call, that's the only route that should be needed 👍

@fm3
Copy link
Member

fm3 commented Feb 23, 2022

I created the PR scalableminds/webknossos-connect#127 to restore compatibility. That one should be probably merged first.
During testing I noticed that this branch is quite out of date with the master, you may want to reserve some time to check if everything still works after master merge :)

@daniel-wer
Copy link
Member Author

During testing I noticed that this branch is quite out of date with the master, you may want to reserve some time to check if everything still works after master merge :)

Do you mean this connectome-viewing branch? If so, I merged the current master, yesterday, so it shouldn't be that out of date 🤔 Still, we'll definitely test the new functionality before merging 👍

@fm3
Copy link
Member

fm3 commented Feb 23, 2022

You are of course right, I just did not realize I did not pull before testing.

@philippotto
Copy link
Member

  • Currently the connectome tab is always visible. Since it is kind of a niche functionality, should it maybe only be visible for specific organizations (that want to test this functionality and/or we know to be in the connectomics field)? And if so, does that have any implications for the viewport configurations (if the tab is sometimes there and sometimes not)?

Yeah, I think, it would currently be best if the tab could be enabled/disabled via a feature flag so that we can control it per organization. Otherwise, the tab might cause a lot of irritation, as no one will know how to use it. I think, it should "just work" ™️ to adapt default_layout_configs.js so that it uses the features. Hopefully, this will work. The layout version needs to be bumped then, too, I assume..

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks great :) Didn't test, yet, though, since there are some empty checkboxes still.

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@daniel-wer
Copy link
Member Author

Yeah, I think, it would currently be best if the tab could be enabled/disabled via a feature flag so that we can control it per organization. Otherwise, the tab might cause a lot of irritation, as no one will know how to use it. I think, it should "just work" tm to adapt default_layout_configs.js so that it uses the features. Hopefully, this will work. The layout version needs to be bumped then, too, I assume.

Hmm, yeah, I think this method would work, but I really dislike that each time another organization is added, (one needs to remember to increase the layout version and) all layout configs become invalidated :/
What I could try to write would be a kind of evolution that runs after loading the persisted layout (or loading the default one) and adds or removes the Connectome Tab, depending on the feature flag 🤔 This would also allow us to add new tabs in the future without having to reset all layout configs for all users.

@philippotto
Copy link
Member

Yeah, I think, it would currently be best if the tab could be enabled/disabled via a feature flag so that we can control it per organization. Otherwise, the tab might cause a lot of irritation, as no one will know how to use it. I think, it should "just work" tm to adapt default_layout_configs.js so that it uses the features. Hopefully, this will work. The layout version needs to be bumped then, too, I assume.

Hmm, yeah, I think this method would work, but I really dislike that each time another organization is added, (one needs to remember to increase the layout version and) all layout configs become invalidated :/ What I could try to write would be a kind of evolution that runs after loading the persisted layout (or loading the default one) and adds or removes the Connectome Tab, depending on the feature flag thinking This would also allow us to add new tabs in the future without having to reset all layout configs for all users.

You are totally right. Dynamically adding/removing the tab would be best probably.

…st to the currentlyRenderedZoomStepAtPosition by waiting until buckets are loaded before checking whether the zoomStep is renderable for a specific position. This is needed to wait for loaded data after an agglomerate file mapping was activated, for example, when activating a mapping through the context menu before loading a precomputed mesh or before loading synapses for an agglomerate
@daniel-wer
Copy link
Member Author

Right-Click on a segment in the viewport and select "Import Agglomerate and Synapses".

I confirmed that this works now, even if a mapping is activated on-the-fly, with my latest commit :)

conf/application.conf Outdated Show resolved Hide resolved
@philippotto philippotto enabled auto-merge (squash) February 28, 2022 13:49
@philippotto philippotto merged commit a552064 into master Feb 28, 2022
@philippotto philippotto deleted the connectome-viewing branch February 28, 2022 14:15
hotzenklotz added a commit that referenced this pull request Mar 2, 2022
* 'docs' of github.com:scalableminds/webknossos:
  Connectome Tab (#5894)
  Fix duplicating annotations via toggling show archived annotations button and fix downsample modal rendering errors (#6058)
hotzenklotz added a commit that referenced this pull request Mar 29, 2022
* docs:
  mpre pr feedback
  Updates to wK Docs (#5995)
  restored some sections in sharing docs
  added context-menu hint to shortcuts
  more pr feedback
  Connectome Tab (#5894)
  Fix duplicating annotations via toggling show archived annotations button and fix downsample modal rendering errors (#6058)
hotzenklotz added a commit that referenced this pull request Mar 29, 2022
* master:
  fix docs formatting
  Remove experimental auto brush feature (#6107)
  Fix markdown issues in docs (#6105)
  Unify long-running RPC timeouts (#6103)
  Disallow deactivating users with active tasks (#6099)
  Add remote origin headers also in error case (#6098)
  Linking layers during upload no longer restricted to public datasets (#6097)
  Fix layer headers layout (#6087)
  Fix more docs links (#6095)
  Update datasets.md (#6094)
  More PR feedback for Docs (#6091)
  Updates to wK Docs (#5995)
  Connectome Tab (#5894)
  Fix duplicating annotations via toggling show archived annotations button and fix downsample modal rendering errors (#6058)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connectome Viewing in webKnossos
3 participants