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

Add support for BigVolumeViewer #415

Closed
constantinpape opened this issue Aug 12, 2021 · 74 comments
Closed

Add support for BigVolumeViewer #415

constantinpape opened this issue Aug 12, 2021 · 74 comments
Labels
enhancement New feature or request

Comments

@constantinpape
Copy link
Contributor

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 #237.

@tischi
Copy link
Contributor

tischi commented Oct 25, 2024

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:

display.imageVolumeViewer.showImages( checkBox.isSelected() );

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:

Thanks a lot for your interest!

@ekatrukha
Copy link
Collaborator

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
Copy link
Collaborator

Ok, I figured it out

@tischi
Copy link
Contributor

tischi commented Oct 28, 2024

Ok, I figured it out

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

@tischi
Copy link
Contributor

tischi commented Oct 28, 2024

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

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

@ekatrukha
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Contributor

tischi commented Oct 28, 2024

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
Copy link
Collaborator

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

@tischi
Copy link
Contributor

tischi commented Oct 29, 2024

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
Copy link
Collaborator

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
Copy link
Contributor

tischi commented Oct 29, 2024

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
Copy link
Collaborator

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
Copy link
Contributor

tischi commented Oct 30, 2024

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
Copy link
Collaborator

ekatrukha commented Oct 30, 2024

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
Copy link
Contributor

tischi commented Oct 31, 2024

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
Copy link
Contributor

tischi commented Oct 31, 2024

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
Copy link
Collaborator

ekatrukha commented Nov 1, 2024

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
Copy link
Contributor

tischi commented Nov 4, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Contributor

tischi commented Nov 6, 2024

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
Copy link
Collaborator

ekatrukha commented Nov 6, 2024

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
Copy link
Contributor

tischi commented Nov 6, 2024

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

@ekatrukha
Copy link
Collaborator

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

@tischi
Copy link
Contributor

tischi commented Nov 6, 2024

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
Copy link
Contributor

tischi commented Nov 6, 2024

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
Copy link
Collaborator

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
Copy link
Contributor

tischi commented Nov 11, 2024

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.

@ekatrukha
Copy link
Collaborator

So I think one last thing before the final minimal BVV 'alpha' version, in my opinion is BVV settings.

Some of them can be changed 'on-the-fly' (projection matrix, camera position) and some (render quality, GPU cache size) will require BVV restart and reloading of data.

You mentioned that it can be done at the 'source settings wheel', if I remember correctly.

Are there maybe some 'general Mobie settings' somewhere?

It is just that BVV settings are kind of global (like projection matrix details), not sure it is convenient to add them to the each source settings.
What do you think?

@tischi
Copy link
Contributor

tischi commented Dec 4, 2024

Good point.

I think the easiest (and many only option) right now would be to add another Command here:
https://github.com/mobie/mobie-viewer-fiji/tree/main/src/main/java/org/embl/mobie/command/context

This Command could be then be made accessible to the user as another entry in the Context menu that one sees upon right-click in the BDV window. We could call it "BigVolumeViewer Settings".

I can add a skeleton of this for you to the bvvpg branch, shall I?

@ekatrukha
Copy link
Collaborator

Yes, please, that would be very helpful.

@tischi
Copy link
Contributor

tischi commented Dec 5, 2024

The question would be where to store those global settings....

I think the easiest right now would be to make all the fields in BVVManager public static.

Would that be OK for now?

@tischi
Copy link
Contributor

tischi commented Dec 5, 2024

...then, if we want changes to be immediate, once the user changed any of those values we would need to do something like

MoBIE.getInstance().getViewManager().bvvManager.get().coloringChanged()

In the coloringChanged() method you would need to add code that ensures that all the relevant settings from BVVManager are applied.

Or, if those changes are too massive, maybe we actually need a new ImageBVVViewer.bvvSettingsChanged() method that reinitialises the current viewer more substantially than the current coloringChanged().

@tischi
Copy link
Contributor

tischi commented Dec 5, 2024

...and could you please give me one example of a setting that you would like to put there?

@ekatrukha
Copy link
Collaborator

Hello @tischi,

yes, we can make them static.
But if a user changes them, will they be the same after Mobie restarts?
That would be preferable. I usually store these kind of things using ij.Prefs.set() and ij.Prefs.get().
I do not know how it is organized in Mobie.
I mean, if a user has 10 Gb GPU and he wants to change maxCacheSizeInMB,
it should be possible to do it once.

I am in favor of ImageBVVViewer.bvvSettingsChanged().
There we can decide:

  1. if changed parameters only those that can be changed during runtime, we do small update.
    For example dCam, dClipNear and dClipFar can be update 'on-the-fly' via simple bvv.getBvvHandle().getViewerPanel().setCamParams( ... ) .
  2. If other parameters change, we have to close BVV and re-add sources again, it would be large update.

@ekatrukha
Copy link
Collaborator

BTW, I've caught this annoying exception upon closing of BVV, I need to add it to a new version of bvvpg.

I've talked to Tobias and indeed, as you mentioned, show(final SourceAndConverter< T > source ) is absent there, but can be easily added in a manner similar to bdv.

It needs to be tested, I will work on it soon.
It will prevent re-caching of UnsignedShort volumes (at least).
For the other data types we still have to wrap it in 'spimData', so the code that was written is still useful.

Cheers,
Eugene

@ekatrukha
Copy link
Collaborator

Do you have somewhere 'in the cloud' UnsignedShort dataset to test straightforward SourceAndConverter addition to bvv?

@tischi
Copy link
Contributor

tischi commented Dec 6, 2024

I added

image

It turns out that we can directly update BVV without going through ImageBVVViewer and I therefore implemented a minimal example that only does that.

@tischi
Copy link
Contributor

tischi commented Dec 6, 2024

I added test.debug.DebugOpenOpenOrganelle.class. If you run this you should have a uint16 image from Janelia's OpenOrganelle to play with :-)

Note that you'll also have to rebuild the project, because I merged the latest version of the main branch, which depends on a newer version of mobie-io.

@ekatrukha
Copy link
Collaborator

Thank youuu! I will work on it next week and keep you updated!

@tischi
Copy link
Contributor

tischi commented Dec 6, 2024

I forgot to mention one thing: Since ConfigureBVVRenderingCommand.class implements Command the parameter values persist, I think also when closing and re-opening Fiji (or the IDE). That's functionality one get's for free by the SciJava Command framework. Thus, we will not need ij.Prefs.set().

However, what is missing is that we would automatically apply the recent settings of ConfigureBVVRenderingCommand to BVV upon startup of MoBIE. I don't know how to fetch those values without actually running the Command. I will create a new issue for this and ask Curtis.

@ekatrukha
Copy link
Collaborator

ekatrukha commented Dec 11, 2024

I've implemented parameters that do not require BVV restart,
gonna see now how I can restart BVV with new settings.
One way would be to kill it and reload all sac s,
but I guess it is not very efficient.
I think I would need to unload BvvSource 's somewhere and load them back.

@tischi
Copy link
Contributor

tischi commented Dec 11, 2024

I think killing it could be OK, if necessary. The data needed for rendering should hopefully still be cached in the SourceAndConverters....

@ekatrukha
Copy link
Collaborator

I have a trouble.
To restart it, in the ConfigureBVVRenderingCommand here I need access to the imageBVViewer, so I can reload the list of added SourceAndConverter s.
Do you know how to do it?
Or should we add a link to the imageBVViewer in the BVVManager class?

@tischi
Copy link
Contributor

tischi commented Dec 11, 2024

Does this work?

Bvv bvv = MoBIE.getInstance().getViewManager().getBvvManager().get();
List< SourceAndConverter< ? > > sources = bvv.getBvvHandle().getViewerPanel().state().getSources();	

@ekatrukha
Copy link
Collaborator

Since it seems to be harder than I though, I moved the discussion on BVV restart to a new issue.

@ekatrukha
Copy link
Collaborator

Hello @tischi,

In my opinion, we are done with a minimal implementation, what do you think?
I've added BVV view sync with the current BDV shortcut (D).
In principle, we can add an option to do it continuously (I think Nicolas does it in bdvpg),
but I am not sure it is needed.
We can wait and see if someone will ask for it.

@tischi
Copy link
Contributor

tischi commented Dec 13, 2024

I would be happy to ship this with MoBIE 🐳 !
I guess I would need to upload a couple of additional jars to the update site.
Do you know which jars those would be?

@ekatrukha
Copy link
Collaborator

Shipping BVV with Mobie

to FIJI update site you would need to add three jars:

  1. bvv-playground 0.3.3,
  2. jide-oss
  3. flat-laf for it

@tischi
Copy link
Contributor

tischi commented Dec 16, 2024

I will try to publish this on the update site today or tomorrow.

@tischi
Copy link
Contributor

tischi commented Dec 17, 2024

image

I guess I should also upload jide-oss ?

@tischi
Copy link
Contributor

tischi commented Dec 17, 2024

@ekatrukha I hope I uploaded everything could you try?

@ekatrukha
Copy link
Collaborator

I've tried on a fresh Fiji install and it works great!
I was able to run it on our server now, with a high bandwidth network connection
and 16 Gb GPU.
It is pretty smooth experience, compared to the laptop!
Yoohoo!

@tischi
Copy link
Contributor

tischi commented Dec 17, 2024

Awesome! I will close this thread now in favour of smaller issues.

@tischi tischi closed this as completed Dec 17, 2024
@ekatrukha
Copy link
Collaborator

Hello @tischi

and happy new 2025!

Should we merge bvvpg branch to the main?

@tischi
Copy link
Contributor

tischi commented Jan 9, 2025

Hi,

Happy new year!

It is merged, afaik...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants