mobie / mobie-viewer-fiji

BSD 2-Clause "Simplified" License
33 stars 13 forks source link

Add support for BigVolumeViewer #415

Open constantinpape opened 3 years ago

constantinpape commented 3 years ago

Add support for scenery / BigVolumeViewer (to potentially replace the current 3d viewer?!). This can be done via BDV-playground, relevant links: https://github.com/bigdataviewer/bigdataviewer-playground/tree/master/src/main/java/sc/fiji/bdvpg/bvv https://github.com/bigdataviewer/bigdataviewer-playground/tree/master/src/main/java/sc/fiji/bdvpg/scijava/command/bvv

This was first discussed in https://github.com/mobie/mobie-viewer-fiji/issues/237.

tischi commented 1 month ago

Hi @ekatrukha,

It would be amazing if you could give it a shot to integrate BVV into MoBIE.

This is the place where the current volume viewing could be replaced by the new one:

https://github.com/mobie/mobie-viewer-fiji/blob/81e8fab7cb4085ab28894c36091ca5809cacab1d/src/main/java/org/embl/mobie/ui/UserInterfaceHelper.java#L1145

And this is the class where the current volume viewer is defined; the constructor just needs a Collection of SourceAndConverter and a handle on the current instance of the volume viewer:

https://github.com/mobie/mobie-viewer-fiji/blob/81e8fab7cb4085ab28894c36091ca5809cacab1d/src/main/java/org/embl/mobie/lib/volume/ImageVolumeViewer.java#L70

Thanks a lot for your interest!

ekatrukha commented 1 month ago

Hello @tischi et al.,

I've started to play around with it. For now, I am stuck with just running it. I am using OpenPlatybrowser.java as a test, but it gives me an error libblosc.so: cannot open shared object file I guess I need to install it? Or should I just add to my debug JVM something like -Djna.library.path=/..Fiji.app/lib/linux64/lib? Gonna try that

ekatrukha commented 1 month ago

Ok, I figured it out

tischi commented 1 month ago

Ok, I figured it out

Actually, I am curious, what did you need to do?

tischi commented 1 month ago

...and @ekatrukha the web address is wrong in the OpenPlatyBrowser.java.

Please use: https://github.com/mobie/platybrowser-project

ekatrukha commented 1 month ago

Ok, I figured it out

Actually, I am curious, what did you need to do?

I saw recently a similar issue on zulip. In my Eclipse in the "Debug Configuration -> Arguments -> VM arguments" I've first tried to add FIJI library folder -Djna.library.path=/..Fiji.app/lib/linux64/lib, but that did not work. So I've installed homebrew on my Ubuntu and from there brew install c-blosc, which made a library and I've now added its location to VM arguments -Djna.library.path=/home/linuxbrew/.linuxbrew/Cellar/c-blosc/1.21.6/lib

and after that debug worked.

ekatrukha commented 1 month ago

Here is a first attempt on using BVV to render 3D images (that works). I should have made another branch, gonna correct this later. For now I use original BVV without extra things.

I use your trick to convert a specific resolution of the source to RAI, but it should be possible to use Source directly. Somehow when I try it directly, it hangs for a long time. I suspect BVV tries to load everything, all levels, if the data does not come as 'nice spimdata'. How would you show this list of SourceAndConverter in Bdv normally? I am going to look/dig into it later.

tischi commented 1 month ago

Somehow when I try it directly, it hangs for a long time.

That is very strange, I would think the having the multiple resolutions should help...

BdvFunctions.show( ... ) has an implementation with a List< SourceAndConverter > as input argument. Does BvvFunctions.show() have that, too? If not, maybe we can ask Tobias to add it...?

ekatrukha commented 1 month ago

No, not yet, it is more fun to figure it out.

tischi commented 1 month ago

And I think we should for now not replace the old way of showing volumes but rather add another [ ] BV checkbox next to the existing [ ] V checkbox. Is that something you could do? Otherwise I can make a branch where I do it and then you could continue from there.

Another consideration is that I already have a scijava39 branch. We could consider using this as a base for our developments. I am not sure it matters though...what do you think?

ekatrukha commented 1 month ago

For now, I am going to try experimenting and writing things in a clean way, so it is not important. Append transforms and deal with sources addition and removal (gonna use logic from 3D viewer). I think an additional BV checkbox would be better.

Some extra questions, does Mobie work with 32bit/float data?

My preliminary plan is to make things work in 'classic' bvv, then switch to bvv-playground that is a bit more advanced (LUTs/volumetric render), should be easy. Another bonus is that playground should work with pom 38/39 without problems. I also thought to make a bvv-playground-minimal with a minimal GUI for clipping/scale/zoom and maybe leave an animation panel. What do you think about it? I mean, once something is ready, you can look at it, test and decide.

tischi commented 1 month ago

bvv-playground-minimal: Would you then depend on this yourself? If so, then yes!

MoBIE: Float data: Yes, for instance the X-Ray tomography data that we deal with is float.

Everything else: Sounds awesome! Thanks so much! This is exciting!!

ekatrukha commented 1 month ago

Ok, I finally figured out, I think, what was wrong. Your Source seems always have a time point, even if it is a single volume. BVV loads them differently than BDV, it needs to be wrapped up in spimdata in this case. Otherwise it first loads everything to the memory. Time option is tricky.

Btw, if it is a 'true' time data, should we load just a single time point or allow user to change it? Not sure what will happen if there are multiple sources with different number of time points.

This wrapping also should take care of the 8-bit data, since BVV natively shows only 16-bit. And in principle, float too. Maybe you can point me in the direction of the float data?

This bvv-minimal yes, should be handy, I am going to reuse in a number of our projects too. But it will take a month or so.

tischi commented 1 month ago

Good question about the time.... I think we did not have volumetric time-lapse data as a use-case yet. In general, of course, it would be good to link the time-sliders of BDV and BVV, I guess using some listener pattern.

BTW: this reminds me that Nico Chiaruttini shared a link with me how he is doing things in bdv-playground:

https://github.com/search?q=repo%3Abigdataviewer%2Fbigdataviewer-playground%20bvv&type=code

Probably it would be useful to see how he does it....

I will look for publicly accessible float data.

ekatrukha commented 1 month ago

Yes, I saw and tried the way Nicolas is doing it, but it A) in principle works only with 16bit sources and B) does not work with sources supplied by Mobie (in my hands). BVV does not recognize them as volatile multires, so it loads everything as Simple Stack, i.e. reads all data into memory.

After a couple of days reading code I am coming to the conclusion that to show them in BVV via spimdata wrap is a good solution, but then it is re-caching. I will try to finish this implementation till the end of the week and see.

Spimdata seems to provide control over cache, which is important for BVV, probably because of the way it uploads things to GPU cache.

While 'regular' caching (via cache-examples) does not expose cache control explicitly (or I could not find it) and probably cache access is organized differently. But BDV is less picky or does its own caching differently, since it is just one plane.

It is exciting problem to work on. I thought it will be super easy and quick, but here we go. But I learned a lot about 'internals' behind.

Nicolas definitely should know a lot about it, he does reflection cache substitutions and other things I do not understand yet in BDV playground. But I will contact him next week, since I did not hit a wall yet. And (mainly) because at that time I will finish some other project that I promised to him 🙈

Probably he knows how to expose cachecontrols, so no need to recache.

Worse case scenario, instead of RGB copy of resulution level loaded to 3DViewer we can load 'raw' RAI to BVV.

tischi commented 1 month ago

Hi @ekatrukha,

Nice! Thanks for all the exploration! In the end, if wrapping into SpimData works, why not. Maybe it is even useful that it is re-caching there, because for the 3-D display very different data will need to be loaded than for the 2-D display?

In general though, I have no clue about BVV, but I am very surprised that one cannot simply add a SourceAndConverter to it, which, as you probably know, exposes the cached volatile version of the data via sourceAndConverter.asVolatile(). Thus I would hope that at a simple BvvFunction.show( sac ) should do the trick? I think we should discuss this with Nico and Tobi. Maybe via a Zulip thread?

Regarding the 16-bit: We could do something like

RandomAccessibleInterval< UnsignedShortType > shortTypes = Converters.convert( floatTypes, ( floatType, shortType ) -> shortType.setReal( scale * ( floatType.get() - min ) ), new UnsignedShortType() );

We would then have to do this for the whole sourceAndConverter, i.e. all resolution levels, both volatile and non-volatile. But before implemeting this it would be good to check with Nico, because chances are that he has something for this already in bdv-playground.

tischi commented 1 month ago

Hi,

Here's a link to a float image:

https://s3.embl.de/imatrec/ATH_20to200_20240703_PM_01_epo_02_P3_32bit.ome.zarr

Metadata of highest resolution is:

> zarr_overview('https://s3.embl.de/imatrec/ATH_20to200_20240703_PM_01_epo_02_P3_32bit.ome.zarr/0')
Type: Array
Path: https://s3.embl.de/imatrec/ATH_20to200_20240703_PM_01_epo_02_P3_32bit.ome.zarr/0/
Shape: 407 x 4096 x 4096
Chunk Shape: 128 x 128 x 128
No. of Chunks: 4096 (4 x 32 x 32)
Data Type: float32
Endianness: little
Compressor: blosc
ekatrukha commented 1 month ago

Hello @tischi,

yoohoo, the first milestone, BVV version works, take a look at this branch. It works with 16-bit and 8-bit images, but float can be implemented too, provided some min and max. Do you do this estimation somewhere? Or we can take it from BDV settings. Or we can do iteration over the whole volume to find them, maybe not so fast, but efficient.

In this branch, I am overriding the same checkbox as for 3DViewer. If you make a branch with a new extra checkbox, I can plug the code there.

The next step is to include bvv-playground for volumetric rendering and LUTs, it should be straightforward since all interfaces are the same, just change package names. Then bvv-minimal, but that will take some weeks.

At some point I was doing debug for 16-bit images using DebugAngelikaOMEZarrIssue, but I think something is wrong with that source/transforms. Since it is jumping both in BVV and BDV when switching between different mipmap levels.

tischi commented 4 weeks ago

Great 🥳 ! I am travelling this week and will probably not get around trying it..

Do you do this estimation somewhere?

I do this here, but this is specifically for 2-D. I think for 3-D we may better copy the code from LabKit.

If you make a branch with a new extra checkbox, I can plug the code there.

I made a new branch, based on the current master, which also contains a couple of other changes as I was working on it the last couple of days: https://github.com/mobie/mobie-viewer-fiji/tree/bvv I marked the line where you can add BVV with // FIXME: replace with display.bigVolumeViewer

Then bvv-minimal, but that will take some weeks.

No worries, there is no timeline! I am just super happy that this will happen at all! Maybe for a nice X-mas release 🎄

ekatrukha commented 3 weeks ago

I've added BVV Manager to handle additions of different sources without BVV restart and I also figured checkbox thing (I see you also added it in the new branch, thank youuuu).

Now the only thing that is left is 1) Float data. 2) For Platy project I tried opening segmentation and raw data volumes in BVV and somehow raw is very small, I guess I am missing some scaling factor somewhere, gonna look at it later.

For now, just for convenience, I will still be working on this branch in my fork. At least until the top two parts are finished. And when it is ready, I will move it/add it to the desired location.

ekatrukha commented 3 weeks ago

Hello again, @tischi

I've finished the points from the previous comment.

FloatType source range estimation is taken from LabKit. Matthias came up with a rather clever way, I must say, and it works relatively fast. But since we use the lowest pyramid level, it always underestimates ranges (since values are averaged). In principle, an alternative could be to take min and max from the current BDV settings. And to update them, one needs to uncheck BVV checkbox and check again, but that would lead to full reloading of the dataset. I do not know what is better, you decide. I think for the first iteration it is ok and then we see.

I've moved everything to a new package for convenience.

For a BVV there is a set of parameters that needs to be defined. I've put some values there, but potentially there should be some window/dialog/settings somewhere in Mobie that is gonna change them.

In short, I think it is a workable version. Can you please test it and run it on some data/typical usage examples? I do not have much experience using Mobie and looking at the data. Or do you prefer me to update root bvv branch?

If it passes your tests, I will update everything to bvv-playground next week.

tischi commented 3 weeks ago

Dear @ekatrukha,

Thanks a lot again! I am on a conference this week. But I will try to squeeze in a quick test! Could you please update the bvv branch? I think it will be easier.

ekatrukha commented 3 weeks ago

Mmm, the pom of that branch depends on mobie-io.version 3.1.0-SNAPSHOT that is not in maven, I guess it is on your PC. And if I change it to 3.0.4, which is the latest on maven, it gives me build errors (since code changed). Can you share 3.1.0-SNAPSHOT jar? Or cut the release to maven?

upd: I will do minor fixes to work with 3.0.4 and then you can revert it back.

tischi commented 3 weeks ago

I fixed the bvv branch to compile with mobie-io 3.0.4 and pushed the changes. Can you try with that?

ekatrukha commented 3 weeks ago

Yep, I've pushed all the recent changes, it should be ok to test if you pull the latest changes.

tischi commented 3 weeks ago

Works!

image

Questions:

  1. Do you also support a rendering mode that would look like in the ImageJ 3D Viewer?
  2. Is there a way to rotate around the mouse pointer position rather than (0,0,0)?
  3. There is no menu in BVV, I was looking for a "Help" menu that could have the keyboard shortcuts.
tischi commented 3 weeks ago

Do you also get that error (it does not seem to affect anything):

Exception in thread "PainterThread" com.jogamp.opengl.GLException: java.lang.InterruptedException
    at com.jogamp.opengl.awt.GLJPanel.display(GLJPanel.java:465)
    at bvv.core.InteractiveGLDisplayCanvas.display(InteractiveGLDisplayCanvas.java:320)
    at bvv.core.VolumeViewerPanel.paint(VolumeViewerPanel.java:517)
    at bdv.viewer.render.PainterThread.run(PainterThread.java:87)
Caused by: java.lang.InterruptedException
    at java.lang.Object.wait(Native Method)
    at java.lang.Object.wait(Object.java:502)
    at java.awt.EventQueue.invokeAndWait(EventQueue.java:1343)
    at java.awt.EventQueue.invokeAndWait(EventQueue.java:1324)
    at com.jogamp.opengl.awt.GLJPanel.display(GLJPanel.java:463)
    ... 3 more
ekatrukha commented 3 weeks ago

Niiice.

1) "Volumetric" rendering is supported in bvv-playground. Check it out, I've made a new bvvpg branch. Shortcut O (letter) switched between "volume" and max projection. Also in the sources brightness/color dialog mouse right click on the color square allows to select/apply LUT.

2) and 3. No, I do not think there is a mode to rotate differently right now, but it can be implemented, if you have ideas. The shortcuts are almost all the same as BDV. So arrows to zoom in/out and rotate. X,Y and Z to specify rotation axis, etc. The list of keymaps is available via Ctrl + , (comma).

Yeah, that error I also have, not sure how to shut down BVV properly. I tried many things, but I think we need to ask Tobias. Indeed, it does not affect things, but it is not nice.

tischi commented 3 weeks ago

Trying bvvpg; the 0 key does not do anything for me.

Is that message related?

Keymap list file /Users/tischer/Library/Application Support/sc.fiji.bigvolumeviewer/keymaps/keymaps.yaml not found. Using builtin styles.
Appearance settings file /Users/tischer/Library/Application Support/sc.fiji.bigvolumeviewer/appearance.yaml not found. Using defaults.
ekatrukha commented 3 weeks ago

No, the message is not related. Do you try 0 or O? :) It should be letter O, like in the beginning of Open.

tischi commented 3 weeks ago

Sorry, my bad O works 🥳

image

Where exactly should I right click to change the LUT?

tischi commented 3 weeks ago
image

Are those strip patterns some rendering artefact? It looks cool anyway!

tischi commented 3 weeks ago

When showing two sources and pressing O only one of them is volume rendered and the other one is max projected; it behaves like a toggle switch...

image

Is that on purpose or maybe a bug?

ekatrukha commented 3 weeks ago

You need to click on this square. You can also do the same at the Cards panel (shortcut P) for groups and etc.

Those patterns yeah, I am not sure, but they add "lighting" atmosphere :) I think it is probably an artifact of current raytracing with an adaptive step. I need to check that.

tischi commented 3 weeks ago

How hard would it be to add a Glasbey LUT for the label mask images?

ekatrukha commented 3 weeks ago

So it is a good catch with different sources being rendered differently. I've just pushed some update to this branch to have them all rendered consistently. In principle, bvv-playground allows mixing them, but it is rarely needed and confusing.

You can use any LUT that is installed in your ImageJ/FIJI if you run it from ImageJ/FIJI. If you run it from the IDE, it picks only default LUTs supplied in the ij.jar.

tischi commented 3 weeks ago

Very cool!

image

I guess you don't have anything for mesh rendering, do you? This is what would be needed for the current implementation of the rendering of a SegmentationDisplay.

To explore the current behaviour you have to add the sbem-segmentation cells to the view and then Ctrl + Left Click in BDV to select one segment and then click the V checkbox:

image

That is, only "selected" segments will be 3-D rendered as meshes.

ekatrukha commented 3 weeks ago

I have everything for mesh rendering :) This is how it happens in BigTrace, for example.

Do you have already generated meshes? If yes, in which object do you store them, imagej-mesh? Or do you generate them by marching cubes from masks somewhere?

It should not be a problem. The problem is time, I need to finish some other projects first. This can be a good addition to bvv-minimal version with clipping and animation. I thought to call it BVVBrowser.

tischi commented 3 weeks ago

Wow, that would be amazing!

The mesh code is all here:

https://github.com/mobie/mobie-viewer-fiji/tree/main/src/main/java/org/embl/mobie/lib/volume

Here is where a specific mesh is added to the volume viewer:

https://github.com/mobie/mobie-viewer-fiji/blob/5d95facf26350d278b787049fe2480f3cc7f3090/src/main/java/org/embl/mobie/lib/volume/SegmentVolumeViewer.java#L283

From a practical point of view, maybe we can create a separate issue for the segment mesh rendering such that we could close this issue here when we released the "normal" BVV rendering.

And, again, thanks a lot for working on any of this at all! This is amazing and, from my side, there is no rush for anything. I am just super grateful to have you on board!

ekatrukha commented 3 weeks ago

I need to take a look at how to convert this CustomTriangleMesh to something like imagej/imglib2-mesh. After that it should be easy. A similar problem happens in another project that I am working on. Yes, I agree that it would be a separate issue, we can make a new branch to experiment with it, just like we did with this one.

Thank you for your kind words! It is a fun project and I am happy to participate. I do not have much experience working on a joint large codebase, I think it shows, but I am catching up.

What do you think is missing from the "normal" BVV rendering? From my point of view, just one thing: There should be some dialog/options to change BVV settings somewhere in Mobie. All parameters default values are living here now Some of them can be changed at runtime, some will require BVV to restart (I marked them).

Another possible issue: In bvv-playground I use range slider from jide-oss that has a specific license. One of them is the same as OpenJDK, but I do not know if it is compatible with BSD2 license of Mobie. And actually bvv-playground too (right now under the same BSD2 license). I think it is better to raise this problem now than later. So we need to 1) look into it; 2) ignore it; 3) I should switch bvv-playground to another range slider. In the last case, I can use range slider from Tobias/BDV, but then I want to have a regular slider in the same style. SInce it breaks consistency and good looks :)

ekatrukha commented 3 weeks ago

Going back to your remark:

Is there a way to rotate around the mouse pointer position rather than (0,0,0)?

That is an important point.

It does rotate around the mouse pointer position (when you first click the left mouse button). The same way as in BDV.

I guess the difference with 3DViewer is that the last always applies rotation around "center" (of the universe/current view transform), when you drag the mouse, independently on where the first mouse click happened. Practically, this results in the situation when the volumes tend to stay "on the canvas". Do you find it more intuitive?

While in BVV the rotation depends on where you first clicked on the canvas.
If you first click close to the center of the canvas, it will behave similar to 3DViewer. If you click close to (any) corner of a window, it would be a new rotation center. And it would be far from the current "universe/viewTransform" center. In this case, large mouse movement will make more probable the volume to leave the canvas.

I guess it can be confusing if you are used to 3DViewer navigation approach.

In BigTrace I handle it with extra "centering" features/options. I plan to include it in the upcoming BVVBrowser. But we can implement 3DViewer navigation style in BVV. Also, if you have better ideas on this topic, let me know. I think navigation of 3D data is challenging and we can try different things and see what works best.

ekatrukha commented 3 weeks ago

In principle, we can think about and implement some "Navigation presets" schemes in bvv-playground, like, choose your style: "BDV" or "3DViewer" or "Blender" or "Imaris" or something else, depending on what you are familiar with. We can ask it at the first launch, but it can be changed somewhere in the settings later.

For translation, for example, "BDV" uses "mouse wheel click", while "3DViewer" uses Shift+wheel click, but it is all a matter of preferences, I guess. But if you are used to one way, it is pretty hard to switch to another. Same with rotation center.

But even for "3DViewer" style rotation there could be some crucial details of implementation, see these two cases ("Maya" style and "BDV" style) for example.

Maybe rendering of "trackball" position/rotation center and its rotational orientation in each case can be helpful to guide users.

It all seems like a minor detail, but it has a huge influence on whether a user is going to use plugin/software or not. So for me it is important.

tischi commented 3 weeks ago

Meshes

https://github.com/mobie/mobie-viewer-fiji/issues/1186


Navigation

As I understand it, the centre of rotation is where you click in the current canvas. I do not find this very intuitive for 3D, because this location can be, along the current viewing axis, very far away from the volume that one is looking at.

Could we add something where, e.g. Ctrl + Left Drag is rotating around the centre of the "current source"?


BVV Settings

There is the Settings cock wheel that the user can click on for each displayed source in MoBIE. For intensity images that leads to ConfigureImageRenderingCommand.class, this is where we could add some of the BVV settings.


Shipping BVV with MoBIE

I think already what we have right now would be nice to ship with MoBIE. I think we only have to figure out the dependencies. Whether we leave it as is or whether we want to wait for the bvv-minimal refactoing.

ekatrukha commented 3 weeks ago

Navigation

I've added "3DViewer style rotation" to the BVV window. In addition, shift+left mouse drag is fast, and ctrl+left mouse drag is slow rotation.

ekatrukha commented 3 weeks ago

Shipping BVV with Mobie

to FIJI update site you would need to add three jars: 1) bvv-playground 0.2.9, 2) jide-oss 3) flat-laf for it

tischi commented 3 weeks ago

Awesome!

Before shipping, I would wait a couple of weeks to see whether we can get also some label mask rendering working, because then we can announce both in one go. If that turns out to be a show-stopper we could decide to only ship the volume rendering.