isosurfaces for volume tracings #4567

youri-k · 2020-04-24T08:07:04Z

This PR makes it possible to fetch isosurfaces from the backend in volume tracings. An isosurface can be loaded by using ctrl + leftMouse. This PR adds a button to the 3d viewport that makes it possible to reload the fetched isosurfaces whose annotation changed.

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Steps to test:

Open a volume tracing and do some volume annotation.
Load the isosurface of your annotation by usning ctrl + leftMouse in one of the plane viewports. The isosurface should be loaded. Then change the annotation of that cell and use the reload button in the top right corner of the 3d viewport. The isosurface should be erased and the reloaded to the newest version.

Issues:

fixes Isosurface for volume annotations #3681

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

…surface-for-volume-tracing

...nossos-datastore/app/com/scalableminds/webknossos/datastore/services/IsosurfaceService.scala

MichaelBuessemeyer · 2020-07-29T16:30:31Z

Finally, I to the time to work on this issue. 😌

I added a button to the 3d viewport that is only visible if the isosurface feature is enabled. The isosurface saga now also tracks the cell ids of cells that got annotated between reloads. A reload will only reload the cells that got annotated.

Still existing problems:

Currently, the reload button can be always triggered. I'd like to disable the button when no cell with a loaded isosurface got modified. @philippotto Do you have an idea how to do this with little effort? -> thats fine
The loading of an isosurface stops at the border of a cube and so will the reloading. If the user loaded all isosurface segments of all cubes, the reloading will only reload the isosurface data of a single cube (as the request stops at the border of the cube)
-> @philippotto What do you think about modifying the isosurface loading functionality to load isosurfaces across cubes?

MichaelBuessemeyer · 2020-07-30T09:27:42Z

The loading of an isosurface stops at the border of a cube and so will the reloading. If the user loaded all isosurface segments of all cubes, the reloading will only reload the isosurface data of a single cube (as the request stops at the border of the cube)
-> @philippotto What do you think about modifying the isosurface loading functionality to load isosurfaces across cubes?

@philippotto just fixed it :D

MichaelBuessemeyer · 2020-07-30T09:39:40Z

@philippotto The frontend is ready to receive your awesome feedback 🎉

…surface-for-volume-tracing

philippotto · 2020-07-31T09:31:04Z

Very cool! What's the state in the back-end? Maybe you could update the PR description a bit, so that there is some explanation about the future and that current state is clear :)

MichaelBuessemeyer · 2020-07-31T11:04:26Z

Very cool! What's the state in the back-end?

I think the backend is working but I am not sure whether the code is reviewed and cleaned up. @youri-k Do you know more about this?

youri-k · 2020-08-09T14:19:15Z

I think the backend is good for review 🙂 @fm3 could you have a look?

philippotto · 2020-08-10T09:18:29Z

Unfortunately, the CI doesn't pass, as the smoke test repeatedly fails. Is this related to the back-end changes?

youri-k · 2020-08-11T08:28:48Z

Yes, this is a backend related problem. I'm working on it.

…surface-for-volume-tracing

philippotto · 2020-08-13T08:14:07Z

Really cool to see the isosurfaces for own annotations 👍 However, testing didn't went totally smooth for me. Here are my observations:

Initially, I had trouble to get the rendering to work at all. I think, I expected that the current cell ID for isosurface rendering was the same as the "brush ID". However, the two ids seem to be different? I know that the isosurface code also needs to work in "view"-mode where no brush id exists, but when being in a volume tracing this shouldn't be a problem? Either way, it's not really transparent right now which cell ID will be rendered. Maybe we should make this more visible somehow?
When only annotating one slice and then using the isosurface feature, nothing is rendered. I think this is because the rendering always considers blocks of X voxels and takes the median of these (is this correct, @youri-k?). So, right now this is expected behavior. However, this can be very confusing to the user.
- Idea 1: When taking the mode of X voxels within a block, ignore cell id 0. I think this should always result in intuitive results. But maybe I'm missing something?
- Idea 2: Communicate to the user why nothing shows up. However, I think this will be very hard to explain and also not intuitive. I hope idea 1 works
The reload button can be improved in multiple ways:
- before requesting new isosurfaces, the front-end should save everything so that the rendered isosurfaces are up-to-date
- as soon as the button is clicked, a spinner should be shown (also, it should be disabled). after the reload has been completed, the spinner should be hidden again obviously.
- right now, the old isosurfaces are immediately removed. then, the request takes some time and the isosurfaces are rendered again. I suggest to only remove the isosurfaces shortly before the new ones are added.

Probably out of scope for this PR, but I'm starting to think that we should probably list all the isosurfaces explicitly in the "meshes" tab. Then, we could have functionality for reloading, removing etc. very transparently. Something like this:

Maybe it makes sense to combine this with a general segment tab, so that all segments are listed (and when you click it you jump to it). And if you want, you can activate the isosurface rendering for one particular cell. Also see #4667.

However, a first version of this feature could simply list the cells for which isosurfaces have been loaded (that should be rather easy to do).

What do you think?

MichaelBuessemeyer

I applied your feedback for the frontend @philippotto. Could you please check the newest changes?

frontend/javascripts/oxalis/api/api_latest.js

philippotto · 2020-08-18T09:41:23Z

Cool stuff! I'll look through the code to give feedback in a bit, @MichaelBuessemeyer!

What's the status of volume (or hybrid) annotations with fallback segmentation? Since this will be the new default soon, there should also be isosurface support. However, I couldn't get it to work? It's probably also not trivial, since the back-end can only compute isosurfaces for volume tracings or for segmentation layers, but not for the overlay via fallbacks, right? @youri-k

We probably need to discuss this in person together with @youri-k, when he's back again.

philippotto

Awesome! This already looks very good 🕺 Just left some mostly minor feedback :)

frontend/javascripts/oxalis/default_state.js

frontend/javascripts/oxalis/view/td_view_controls.js

frontend/javascripts/oxalis/api/api_latest.js

frontend/javascripts/oxalis/model/sagas/isosurface_saga.js

philippotto · 2020-08-18T15:39:02Z

frontend/javascripts/oxalis/model/sagas/isosurface_saga.js

+    let shouldBeRemoved = true;
+    for (const [, position] of isoSurfacePositions) {
+      // Reload the Isosurface.
+      yield* call(ensureSuitableIsosurface, null, position, cellId, shouldBeRemoved);


I'd add a comment which explains that no redundant work will be done, since the isosurface-loading-code always checks whether a request has already been performed. I myself just stumbled here a bit, since the code requests a new isosurface for each position in the three-d-map, even though ensureSuitableIsosurface will also traverse the 3D space. However, your approach is still very correct (since multiple seeds might still be necessary to reconstruct everything). I'd just add the comment so that the next reader does not stumble here :)

philippotto · 2020-08-18T15:42:42Z

frontend/javascripts/oxalis/model/sagas/isosurface_saga.js

  yield _takeEvery("TRIGGER_ISOSURFACE_DOWNLOAD", downloadActiveIsosurfaceCell);
  yield _takeEvery("IMPORT_ISOSURFACE_FROM_STL", importIsosurfaceFromStl);
  yield _takeEvery("REMOVE_ISOSURFACE", removeIsosurface);
+  yield _takeEvery("REFRESH_ISOSURFACES", refreshIsosurfaces);


When the refresh button is clicked, I think it would also make sense to fetch the isosurfaces at the current position (as a seed) with the cell id at that position. That way, the button can also be used to render the very first isosurface without needing to know about shift+click.

philippotto · 2020-08-31T12:53:21Z

What's the status of volume (or hybrid) annotations with fallback segmentation? Since this will be the new default soon, there should also be isosurface support. However, I couldn't get it to work? It's probably also not trivial, since the back-end can only compute isosurfaces for volume tracings or for segmentation layers, but not for the overlay via fallbacks, right? @youri-k

I had a discussion with Florian and Norman and we agreed for this version of the feature to simply follow this logic: if the current tracing uses a fallback segmentation layer, fetch the isosurfaces from the datastore (i.e, the traced data gets ignored completely) and if there's no fallback layer, use the tracingstore.
Consequently, the isosurfaces rendering will work well for the case of simply "opening a fresh tracing for a dataset with a segmentation layer". It will also work for "opening a new tracing for a dataset without a segmentation layer". And editing a volume tracing will only work well if there's no fallback, but since this hasn't been possible before at all, this seems like a fine trade-off for me.

@MichaelBuessemeyer Could you make sure that the logic explained above is implemented? Also see my other PR feedback :)

philippotto · 2020-08-31T12:54:25Z

@fm3 Could you also review the back-end code in case you haven't done it already? :)

fm3

Backend mostly looks good to me, I added some small comments.
I like the holder structure :)

...-datastore/app/com/scalableminds/webknossos/datastore/services/BinaryDataServiceHolder.scala

...tore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala

...nossos-datastore/app/com/scalableminds/webknossos/datastore/services/IsosurfaceService.scala

…surface-for-volume-tracing

MichaelBuessemeyer · 2020-09-01T12:01:08Z

@philippotto I thin I covered all your feedback

philippotto

Excellent 👍 Works well for me :)

Now, only the back-end's feedback needs to be integrated, I think.

MichaelBuessemeyer · 2020-09-08T14:36:02Z

@fm3 @youri-k ping

fm3

I had another look and merged the master. I’d say backend LGTM now! Let’s do a quick final round of testing after the CI is done, then this should be good to go. Edit: did some testing, seemed to work :) I agree with philippotto that progress/overview UI would be beneficial, though.

Youri K added 5 commits April 23, 2020 17:34

[WIP] isosurfaces for volume tracings

8badcdd

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

9bdaf8f

…surface-for-volume-tracing

update changelog

b1ceca2

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

b442c03

…surface-for-volume-tracing

make datasource optional

894eb94

youri-k self-assigned this Apr 30, 2020

Youri K added 4 commits May 13, 2020 11:38

merge master

dc50d98

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

ee28593

…surface-for-volume-tracing

add frontend part for isosurfaces for volumes

e0767ad

merge master

6d98b8c

philippotto reviewed May 29, 2020

View reviewed changes

...nossos-datastore/app/com/scalableminds/webknossos/datastore/services/IsosurfaceService.scala Outdated Show resolved Hide resolved

Youri K and others added 2 commits June 2, 2020 17:28

send scale to correctly display isosurfaces

a1470c2

Add button to reload Isosurfaces of modified cells

f3a6600

save before reloading & reload isosurface of all cubes

16f9a29

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

7a6e801

…surface-for-volume-tracing

youri-k requested a review from fm3 August 9, 2020 14:18

youri-k marked this pull request as ready for review August 9, 2020 14:19

youri-k changed the title ~~[WIP] isosurfaces for volume tracings~~ isosurfaces for volume tracings Aug 9, 2020

Youri K added 3 commits August 11, 2020 12:29

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

0877cf8

…surface-for-volume-tracing

fix isosurface dependency injection

19cec69

merge master

7eb494d

MichaelBuessemeyer added frontend new feature labels Aug 17, 2020

fix flow definition

ad680ec

MichaelBuessemeyer reviewed Aug 18, 2020

View reviewed changes

frontend/javascripts/oxalis/api/api_latest.js Outdated Show resolved Hide resolved

Merge branch 'master' into isosurface-for-volume-tracing

33110aa

philippotto requested changes Aug 18, 2020

View reviewed changes

philippotto mentioned this pull request Aug 28, 2020

How to organize segments? #4796

Closed

fm3 requested changes Aug 31, 2020

View reviewed changes

MichaelBuessemeyer added 6 commits September 1, 2020 09:39

apply some feedback

f4eb119

Merge branch 'master' of github.com:scalableminds/webknossos into iso…

763413e

…surface-for-volume-tracing

apply feedback & load isosurface at flycam position on refresh

1e02ec0

fetch from datastore if fallback layer exists

25ca0b2

only refetch isosurfaces if there is no fallback layer

bf8a1bc

adjust refesh button tooltip

cbb9333

fix linting

ff62171

philippotto approved these changes Sep 1, 2020

View reviewed changes

Youri K and others added 3 commits September 10, 2020 11:36

implement pr feedback

491a971

pr feedback 2

8013a4f

merge master

44f9dbf

fm3 approved these changes Sep 14, 2020

View reviewed changes

Merge branch 'master' into isosurface-for-volume-tracing

c9729db

fm3 added the automerge label Sep 17, 2020

bulldozer-boy bot merged commit ae10f2d into master Sep 17, 2020

bulldozer-boy bot deleted the isosurface-for-volume-tracing branch September 17, 2020 12:45

philippotto mentioned this pull request Sep 23, 2020

Improve isosurface loading for rendering a single cell #4835

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

isosurfaces for volume tracings #4567

isosurfaces for volume tracings #4567

youri-k commented Apr 24, 2020 •

edited

Loading

MichaelBuessemeyer commented Jul 29, 2020 •

edited

Loading

MichaelBuessemeyer commented Jul 30, 2020

MichaelBuessemeyer commented Jul 30, 2020 •

edited

Loading

philippotto commented Jul 31, 2020

MichaelBuessemeyer commented Jul 31, 2020

youri-k commented Aug 9, 2020

philippotto commented Aug 10, 2020

youri-k commented Aug 11, 2020

philippotto commented Aug 13, 2020 •

edited by MichaelBuessemeyer

Loading

MichaelBuessemeyer left a comment

philippotto commented Aug 18, 2020

philippotto left a comment

philippotto Aug 18, 2020

philippotto Aug 18, 2020

philippotto commented Aug 31, 2020

philippotto commented Aug 31, 2020

fm3 left a comment

MichaelBuessemeyer commented Sep 1, 2020

philippotto left a comment

MichaelBuessemeyer commented Sep 8, 2020 •

edited

Loading

fm3 left a comment •

edited

Loading

isosurfaces for volume tracings #4567

isosurfaces for volume tracings #4567

Conversation

youri-k commented Apr 24, 2020 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

MichaelBuessemeyer commented Jul 29, 2020 • edited Loading

MichaelBuessemeyer commented Jul 30, 2020

MichaelBuessemeyer commented Jul 30, 2020 • edited Loading

philippotto commented Jul 31, 2020

MichaelBuessemeyer commented Jul 31, 2020

youri-k commented Aug 9, 2020

philippotto commented Aug 10, 2020

youri-k commented Aug 11, 2020

philippotto commented Aug 13, 2020 • edited by MichaelBuessemeyer Loading

MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

philippotto commented Aug 18, 2020

philippotto left a comment

Choose a reason for hiding this comment

philippotto Aug 18, 2020

Choose a reason for hiding this comment

philippotto Aug 18, 2020

Choose a reason for hiding this comment

philippotto commented Aug 31, 2020

philippotto commented Aug 31, 2020

fm3 left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Sep 1, 2020

philippotto left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Sep 8, 2020 • edited Loading

fm3 left a comment • edited Loading

Choose a reason for hiding this comment

youri-k commented Apr 24, 2020 •

edited

Loading

MichaelBuessemeyer commented Jul 29, 2020 •

edited

Loading

MichaelBuessemeyer commented Jul 30, 2020 •

edited

Loading

philippotto commented Aug 13, 2020 •

edited by MichaelBuessemeyer

Loading

MichaelBuessemeyer commented Sep 8, 2020 •

edited

Loading

fm3 left a comment •

edited

Loading