mobie / mobie-viewer-fiji

BSD 2-Clause "Simplified" License
31 stars 12 forks source link

3D viewer doesn't work well on large segmentation #137

Closed jhennies closed 3 years ago

jhennies commented 3 years ago

Hi all,

We noticed that the 3D visualization of the Covid data does not render very nicely: mobie_3dviewer

Is this intended, e.g. for computational reasons, and only meant to show a smaller number of selected objects?

constantinpape commented 3 years ago

Is this intended, e.g. for computational reasons, and only meant to show a smaller number of selected objects?

I think the problem is that we have used this mainly for larger connected objects. For these, it makes sense to compute the mesh on a lower resolution (for computational reasons, but also for smoother meshes).

In this case, where we have one label that comprises a lot of small disconnected objects, the resulting meshes don't look good.

tischi commented 3 years ago

@constantinpape @jhennies

If you click on the MoBIE main UI there is a menu item Preferences:

image

There you can set the voxel spacing in 3D:

image

However, we never used this much and thus the code is not well maintained. In fact, I found a bug:

In addition, it is a bit hard to use, because sensible numbers here depend on the resolution layers of the sources. Maybe I should add the resolution levels of the currently shown sources to the log window...

Let me know whether this in principle is the feature you are looking for and then I can work on improving it.

Another issue is, imho:

constantinpape commented 3 years ago

Let me know whether this in principle is the feature you are looking for and then I can work on improving it.

Yes, this is what we are looking for. Let us know when you fixed the caching bug, then we ca test it.

jhennies commented 3 years ago

Yes, @tischi, this is exacly what we are looking for. Thanks for looking into it!

tischi commented 3 years ago

I am on it. What kind of image is it? We have those:

    public enum Type
    {
        Image,
        Segmentation,
        Mask
    }
jhennies commented 3 years ago

It's a segmentation

tischi commented 3 years ago

Thanks, but then currently only "selected segments" are shown in 3D, right? How did you select them to make them appear in the 3D viewer?

jhennies commented 3 years ago

We actually put all DMVs as one label. all golgi in another, and so forth. What I selected was the DMV label

tischi commented 3 years ago

This is how it looks now. I fetches the resolution layer that's closest to the asked for voxel spacing and then replaces all images, also the ones that are not segmentations in the 3D view. The users need to be careful, because showing the "wrong" sources and asking for a high resolution may lead to downloading TBs of data (and out-of-memory crash).

image

  1. Any further suggestions?
  2. Should I try to add this to the context menu in the bdv rather than the hidden preferences?
  3. Should this setting become part of bookmarks? But this would be a future feature request, because then I think I would first have to write code such that different sources can be displayed at different resolutions. Also in general we would have to think how a user could chose the resolution of a source that is shown in 3D. Maybe at the moment they click the checkbox [X]V, a UI could pop up asking for the resolution?
jhennies commented 3 years ago

How about a warning dialog if a high resolution is selected, such that a confirmation of a 'dangerous' setting is required?

constantinpape commented 3 years ago
  • Should I try to add this to the context menu in the bdv rather than the hidden preferences?

Yes, I think it's a good idea to move it in the context menu. If you do that, would it maybe make sense to set it as a per source option instead of changing it for all sources globally?

  • Should this setting become part of bookmarks? But this would be a future feature request, because then I think I would first have to write code such that different sources can be displayed at different resolutions.

Ok, I think this answers my question above :D. So yes, I agree we should have the option for different sources at different resolutions first.

Also in general we would have to think how a user could chose the resolution of a source that is shown in 3D. Maybe at the moment they click the checkbox [X]V, a UI could pop up asking for the resolution?

Yes, I think that would be good. But I would still propose a conservative default value like we have now.

tischi commented 3 years ago

@K-Meech @constantinpape

@jhennies would like to add something like resolution3Dview to the bookmarks.

This would determine the default resolution of the 3D rendering. This could be changed later in the UI, via the context menu, probably something like

Set 3D rendering voxel size...

voxelSize [um] = 
source: [ selected from dropDown with all currently shown sources ]*

Do we all want this?

tischi commented 3 years ago

(ok in fact I thought the answers from Constantin were from Julian ;-) So that already makes for a majority vote.

@K-Meech: Since you are the Bookmarks god by now, could add the code to support this new metadata field? I would write the code that consumes it.

K-Meech commented 3 years ago

@tischi sure - will do! :)

tischi commented 3 years ago

Cool! Thanks! 🚀 I just updated imagej-utils and mobie with some changes towards this, please pull and let me know once you have something and then I will take it from there.

K-Meech commented 3 years ago

Ok - I added it to the metadata (pull requests on both repos now). I made a note here: https://github.com/mobie/mobie-viewer-fiji/pull/143, but you will need to add a getter for the current value of this 3d resolution parameter so I can save it to a bookmark.

tischi commented 3 years ago

@K-Meech Seems to work: image I pushed all changes. Could you please:

  1. Play with it to find bugs?
  2. Check whether this works for the bookmarks?
K-Meech commented 3 years ago

@tischi I have problems running the 3d parts locally in intellij. When I tick 'V' to show objects in 3d it errors with:

Exception in thread "Thread-29" java.lang.NoSuchMethodError: java.nio.ByteBuffer.rewind()Ljava/nio/ByteBuffer;
    at org.scijava.java3d.JoglPipeline.<clinit>(JoglPipeline.java:4338)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:264)
    at org.scijava.java3d.Pipeline$PipelineCreator.run(Pipeline.java:74)
    at org.scijava.java3d.Pipeline$PipelineCreator.run(Pipeline.java:61)
    at java.security.AccessController.doPrivileged(Native Method)
    at org.scijava.java3d.Pipeline.createPipeline(Pipeline.java:91)
    at org.scijava.java3d.MasterControl.loadLibraries(MasterControl.java:837)
    at org.scijava.java3d.VirtualUniverse.<clinit>(VirtualUniverse.java:274)
    at de.embl.cba.mobie.ui.viewer.SourcesPanel.init3DUniverse(SourcesPanel.java:248)
    at de.embl.cba.mobie.ui.viewer.SourcesPanel.initAndShowUniverseIfNecessary(SourcesPanel.java:241)
    at de.embl.cba.mobie.ui.viewer.SourcesPanel.showSourceInVolumeViewer(SourcesPanel.java:185)
    at de.embl.cba.mobie.ui.viewer.SourcesPanel.updateSource3dView(SourcesPanel.java:70)
    at de.embl.cba.mobie.ui.viewer.SourcesDisplayUI$2.lambda$actionPerformed$0(SourcesDisplayUI.java:58)
    at java.lang.Thread.run(Thread.java:748)

Any ideas? I'm pretty sure the 3d has never worked in my intellij, so I think it's something to do with my local setup. I'm running jdk1.8.0_271

tischi commented 3 years ago

Interesting, I am using the same JDK. Google: java.lang.NoSuchMethodError: java.nio.ByteBuffer.rewind()Ljava/nio/ByteBuffer; ...indeed indicates it has to do with the JDK...

Could you just try running this DevelopAnnotationCorrection in the test folder of imagej-utils? If you select (Ctrl + Left-click) on an object in bdv it should show it in 3D. Are you getting the error as well?

tischi commented 3 years ago
tischer@Christians-MBP ~ $ /Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk/Contents/Home/bin/java -version
openjdk version "1.8.0_272"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_272-b10)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.272-b10, mixed mode)
K-Meech commented 3 years ago

Actually it works for DevelopAnnotationCorrection! Weird, not sure why it's an issue in mobie then...

tischi commented 3 years ago
  1. Maybe you are using a different jdk for MoBIE?
  2. Which version of java is used to mvn clean install imagej-utils? Frankly I would not know for myself, but something to check!
K-Meech commented 3 years ago

I checked and it's the same jdk for both, and the same for maven. Hmmmm... I'll do some more searching.

K-Meech commented 3 years ago

aha - I found the issue! I'd built your version of java3d-core https://github.com/tischi/java3d-core with a newer jdk, hence the error. Works now :)

K-Meech commented 3 years ago

@tischi I played around with the 3d changes, some possible issues:

  1. If you change the 3d resolution (while they are being shown in 3D viewer), you lose the colour of the sources in the 3D view.
  2. 3d resolution doesn't work with saving bookmarks. This is an issue with how the bookmark code is structured - I can fix this (should only be a line or two change)
  3. setting the 3d resolution persists when you remove sources and add them again. You probably want this right? But, if you load the source from a bookmark, it won't persist! This could be made consistent by modifying both the current and default metadata (if you want persistence), or by making the current metadata a copy (if you don't want persistence)??
tischi commented 3 years ago

Thanks a lot for testing!!!

K-Meech commented 3 years ago

Sounds good! If you figure out 1., then I'll look at 2. and 3. :)

K-Meech commented 3 years ago

@jhennies @constantinpape Do you have a preference for persistence of setting the 3D resolution through the menu? i.e. if you add a source, change the 3d resolution, then remove it. Would you like it to keep that resolution the next time you add it? (within that session) Or instead load the default from images.json again?

constantinpape commented 3 years ago

Do you have a preference for persistence of setting the 3D resolution through the menu? i.e. if you add a source, change the 3d resolution, then remove it. Would you like it to keep that resolution the next time you add it? (within that session) Or instead load the default from images.json again?

I think I would prefer to use the default from the images.json again; the other option introduces some fairly hidden state. But I don't feel strongly about this, so I won' object if you solve it the other way.

tischi commented 3 years ago

If you change the 3d resolution (while they are being shown in 3D viewer), you lose the colour of the sources in the 3D view.

@K-Meech In fact, could you provide me with a concrete example when this happens?

jhennies commented 3 years ago

Do you have a preference for persistence of setting the 3D resolution through the menu? i.e. if you add a source, change the 3d resolution, then remove it. Would you like it to keep that resolution the next time you add it? (within that session) Or instead load the default from images.json again?

I think I would prefer to use the default from the images.json again; the other option introduces some fairly hidden state. But I don't feel strongly about this, so I won' object if you solve it the other way.

For the sake of the user experience, I would prefer keeping the changed setting for that session. However, I also don't feel too strongly about this, so if there is a high risk for bugs then I would be happy with it loading the defaults.

K-Meech commented 3 years ago

@tischi sure

You should see that colour then goes back to gray.

K-Meech commented 3 years ago

ok - thanks @jhennies @constantinpape . I think I'll go for loading from defaults for now. None of the other settings persist, so it's more consistent if the 3d resolution also doesn't. It can always be changed later, if it's too annoying - it's a pretty minor change to the code.

tischi commented 3 years ago

If you change the 3d resolution (while they are being shown in 3D viewer), you loose the colour of the sources in the 3D view.

@K-Meech should be fixed now in latest master.

K-Meech commented 3 years ago

@tischi colour changes work now! Also, I made a pull request that fixes issues 2 and 3

tischi commented 3 years ago

I'll close this for now. Please reopen if needed.