Isosurface List

[grittaweisheit]

This PR adds a list of isosurfaces in the meshes tab while viewing a dataset.

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• view a dataset
• enable isosurfaces
• open meshes tab
• create isosufaces by pressing shift and clicking on a cell
• the list shoul diplay evey isosurface
• delete isosurfaces by pressing ctrl and clicking on the isosurface
• deleted isosurfaces should not be in the list
• download an isosurface by clicking on its download icon in the list

Issues:

• Improve isosurface loading for rendering a single cell #4835

---

• Updated (unreleased) changelog
• Updated (unreleased) migration guide if applicable
• Updated documentation if applicable
• Adapted wk-connect if datastore API changes
• Needs datastore update after deployment
• Ready for review

[philippotto]

Awesome! This is really, really cool and will make the isosurface feature so much more usable. Also, great job for showing the segment colors in the list as circles 👍

I think, we can iterate on this PR a bit to fully leverage the new UI for the isosurfaces. The first things which come to my mind are the following:

• clicking on "Segment X" could move the current position in webKnossos to that isosurface (where exactly is not too important for now. Maybe store the seed position from where the calculation was started?)
• a reload-button would be great (functionality would probably be very similar to the existing reload-button in the 3D viewport). however, that reload-button should only turn into a spinner if the segment which belongs to the button is loading. in another iteration, we could even distinguish between "completely reload the isosurface" and "continue computation".
• a delete-button would be cool (the current way of deleting via ctrl-click is very hidden and could also be removed if it makes things easier)

You probably need to move more state into the global Store to enable this. So, instead of having isosurfaces as a simple list of ids, you probably need a dict-like structure like this:

isosurfaces: {
  3: {
    segmentId: 3, // this is redundant to the dict-key, but this makes passing around a dict-value simpler
    seedPosition: [1, 2, 3],
    isLoading: false
  }
}

Note that the trees property in the store also uses a dict-like structure, but that is actually a DiffableMap. I think, for your usecase that would be a bit overkill. Instead, I would use a plain JS Object. In the reducers, you can make use of the deep-update-helpers to update an isosurface element (something like updateKey3(state, "isosurfaces", segmentId, "isLoading", true);

Let me know in case you have any questions!

[grittaweisheit]

Thanks for the input! The things you suggested are now implemented. I'm open for further suggestions :).

[philippotto]

Very cool! I just tested it and the new features feel very good 👍

I noticed a few small things which we could improve:

• When the list is empty, nothing is shown. I think, we should use the space to explain the feature a bit. Similar to the empty tree tab:
  
• It's not clear that the list items are clickable. At least, change the cursor via css to pointer. Also, the active segment should be highlighted somehow (e.g., some background color as it's done in the tree tab)
• The layout is not quite right if the segment ids differ in their lengths. The buttons should be right-aligned properly. Alternatively, only show the buttons on hover (then, you don't need to align them). Also, I think, that you can remove the vertical bars between the buttons, as the buttons itself are pretty well distinguishable as different click targets.
  
• Currently, the feature is enabled/disabled via a setting in the Dataset tab (called "Render Isosurfaces (Beta)"). I think, we can remove it from that spot and instead show the setting in the "Meshes" tab. I'm thinking of a switch button; maybe with a label (e.g., see the "Text & icon" label in the antd doc). At that point, we might as well remove the "(Beta)" part :)

Only fyi:

• If one uses the download feature, deletes the isosurface and then re-imports it via the "import" button, the isosurface is not shown in the list. However, this is probably a bit more complicated to fix. I built the feature back then, and as far as I remember, the import merely attaches the STL file to the scene and sets the correct color. The proper isosurface loading logic is completely decoupled from this. I'll create a separate issue for this :)

[grittaweisheit]

I really liked your mock up ideas! The only deviation I made was putting the buttons for loading isosurfaces directly beneath the isosurface heading. I thinks that's a little bit more intuitive.

I now highlight a list item, when the segment is at the current position.

There are only a few things I would like to mention, that I don't find ideal but don't know how to fix and want to check if it would be worth the effort to figure it out. I would appreciate your input to them!

• When loading the isosurface to the active cell there is no position I can anchor the isosurface to, which makes it impossible to navigate to the cell when clicking on the corresponding list entry. I'm not sure where I could get a suitable position to fix that.
• The same goes for imported isosurfaces.
• When uploading an isosurface I can include it in the list, but it can't be removed like normally loaded isosurfaces and instead only vanished from the list, while the isosurface itself still exists in the 3D Viewport.

Additionally I have two questions:

• Should the renderIsosurface setting be removed entirely?
• The shift + click shortcut behaves differently in different hybrid tracings. There it only loads the isosurface for the active cell and not the one you click on. Should it be removed, fixed or kept?

I would appreciate your input :)

[philippotto]

Great stuff!!

I really liked your mock up ideas! The only deviation I made was putting the buttons for loading isosurfaces directly beneath the isosurface heading. I thinks that's a little bit more intuitive.

Yes, that's good 👍 I'd add some whitespace between the two buttons. However, maybe the load-button for the active cell should be removed, anyway, since it causes problems due to the missing seed position.

• When loading the isosurface to the active cell there is no position I can anchor the isosurface to, which makes it impossible to navigate to the cell when clicking on the corresponding list entry. I'm not sure where I could get a suitable position to fix that.

Very good point. Then, I'd remove the button for now 😄

• The same goes for imported isosurfaces.

Hm, maybe we should store the seed position in the STL file. Right now, we already embed the segment id in the output file (doesn't belong to the official file format). However, it's fine for now if this does not work yet in my opinion.

• When uploading an isosurface I can include it in the list, but it can't be removed like normally loaded isosurfaces and instead only vanished from the list, while the isosurface itself still exists in the 3D Viewport.

Do you know why this does not work? From what I see the SceneController adds a threejs group to this.isosurfacesGroupsPerSegmentationId from which later deletion should be possible. We can also call about this if that's easier :)

• Should the renderIsosurface setting be removed entirely?

Very good question. I thought about this for a bit and came to the following conclusion. until now, the setting had two uses: 1) initiate the isosurface rendering 2) update the isosurface. for 1) we now have the button (and later the context menu). and for 2) I think that the isosurface should always be updated for a flycam action (see yield _takeEvery(FlycamActions, ensureSuitableIsosurface); in the isosurface saga) if there is an activated isosurface entry in the list. So, if the user has not used the isosurfaces, nothing is loaded (equal to an disabled isosurface setting). As soon as the user has loaded an isosurface, that isosurface is updated until it's deleted or another isosurface is activated.
Does this make sense? Then, the setting could be removed.

• The shift + click shortcut behaves differently in different hybrid tracings. There it only loads the isosurface for the active cell and not the one you click on. Should it be removed, fixed or kept?

I think, we should remove the (isosurface related) shortcut (shift+click to activate a cell should still work in a volume tracing). An explicit button works better for now and later on we can re-add it to the context menu which will land soon.

---

Other than that, I have some minor suggestions:

• the blue background for the active list item should ideally only go as far as the segment text (similar to the active tree). Then, the icons which appear on hover are also on a white background automatically which increases the contrast.
• the meshes-headline should have some margin-top
• the meshes-section should also have an empty-placeholder (e.g., "There are no meshes. You can import an STL file to change this.")

[grittaweisheit]

Thanks again for your input!

I removed the "load active..." button, the shortcut for isosurface-loading, the renderIsosurface setting and adjusted the setting of the active isosurface a little bit so that the last used isosurface is the active one.
The deletion of imported isosurfaces works again. I'm not sure why it didn't work but works now again.
Furthermore I made the lists of isosurfaces prettier according to your suggestions.

[philippotto]

Awesome stuff! Thanks for going through all the iterations. This will be a huge improvement and boost for the isosurfaces 🎉 🎉 🎉

I took the brazen liberty to push some finishing touches which caught my eye when I tested the last version. I think, this was easier than writing the stuff down again and having you go through it :) I hope this was alright!

It now looks like this:

I'm still not 100% satisfied with the prominent "load-button", but I think it's better to have it next to the other buttons instead of in a separate line 🤔

From my point of view, this PR should be ready for merging :) Go ahead if you agree!

[philippotto]

We forgot to add a changelog entry 🙈 I added that in #5102, though.