scenerygraphics / sciview

sciview is a tool for visualization and interaction with ND image and mesh data
BSD 2-Clause "Simplified" License
62 stars 18 forks source link

Ship sciview with Fiji? Or make the sciview update site stay in sync with Fiji seamlessly #520

Open ctrueden opened 1 year ago

ctrueden commented 1 year ago

Over time there have been various issues and dependency skew with the sciview update site(s) not matching what is currently shipped on the core update sites for vanilla Fiji. E.g. #422, #392, #179. This issue intends to document what the fundamental problem is, and ways we could/should resolve it.

The sciview dependencies must stay in sync with Fiji's dependencies. That's what the pom-scijava Bill of Materials is for: keeping dependencies at consistent versions. But it only fully works if: A) sciview and fiji releases extend the same (ideally latest release) version of pom-scijava; and B) the dependency versions uploaded to the sciview and Java-8 update sites respectively align with those releases.

This has been historically tricky for a couple of reasons. We have tooling to populate a Fiji installation with a Maven coordinate including its dependencies, and upload the resulting state to an update site, but there are a couple of wrinkles:

  1. Native dependencies need to be copied to the right places (jars/<platform> or lib/<platform>);
  2. We need to take care not to shadow the versions of any core libraries—i.e., every JAR should exist either on the Java-8 site (in which case it is "core") or the sciview update site;
  3. When a core dependency such as ImgLib2 changes in a backward-incompatible manner, we need to update all downstream code to accommodate the breakage, and then synchronize releases and uploads to avoid dependency skew across update sites.

To help scenery+sciview stay in better sync with Fiji henceforth, I propose that we add sciview to Fiji, so that it comes with Fiji. No extra update site needed—it works out of the box.

Needed steps to make this happen:

  1. Get scenery and sciview (re)added to the pom-scijava BOM.
    • This is important so that other projects can depend on scenery+sciview. It will let people e.g. write plugins that use sciview more easily.
    • It would also bring us closer to including scenery and sciview in the pom-scijava "mega melt" build+tests, to validate that changes to dependencies don't break scenery+sciview; unfortunately, because the builds are Gradle-based, the mega-melt tooling is incompatible—we would need to enhance the melting-pot.sh script to know how to override dependency versions in the scenery+sciview gradle builds.
    • We also need a release of sciview, which is what the pom-scijava BOM will reference.
  2. Add sciview as a runtime dep to the sc.fiji:fiji POM file.

And I think that's it?

@kephale @skalarproduktraum If you would rather keep scenery+sciview on a separate update site, I think that would also be OK. But we would then need to address the three wrinkles mentioned above: 1) fix any bugs in populate-app.sh, 2) stop shadowing core libs, 3) update scenery+sciview synchronously with Fiji whenever core libs break things. (Though ideally, core libs would not break things in the first place...)

kephale commented 1 year ago

To help scenery+sciview stay in better sync with Fiji henceforth, I propose that we add sciview to Fiji, so that it comes with Fiji. No extra update site needed—it works out of the box.

I am comfortable with doing this now. I think we're finally ready. Thank you for the suggestion @ctrueden

kephale commented 11 months ago

Doing this would close https://github.com/scenerygraphics/sciview/issues/402

kephale commented 11 months ago

I expect that the way we handle this issue will resolve/close https://github.com/scenerygraphics/sciview/pull/374.

ctrueden commented 11 months ago

I'm making progress on this. But there's one major issue I forgot about when I proposed this solution: scenery+sciview now require Java 11. So if we ship it with Fiji, we need to upgrade to Java 11, or at least somehow tolerate the scenery+sciview JARs being present at a bytecode version too new for Java 8. I have some ideas, and will keep poking at it.

ctrueden commented 11 months ago

Oh, the other issue is the Launcher. I need to make it really work with Java 11, and finally update it for all Fiji users. We'll give it another go and see who complains!

ctrueden commented 11 months ago

Updating to Java 11 will also require updating the JavaFX-dependent projects. It's time to start a new Java-11-based update site, and dead-end the Java-8 one. :+1:

ctrueden commented 5 months ago

1) fix any bugs in populate-app.sh, 2) stop shadowing core libs, 3) update scenery+sciview synchronously with Fiji whenever core libs break things.

I rewrote the populate-app logic, which is now as a Gradle task thanks to @elect86's initial prototyping; see e62041408fa6a4e906089bc5bda81c60945e7346. The logic is now more robust (addressing 1), avoids shadowing core libraries (addressing 2) unless the sciview site is already shadowing them, and warns when skipping a sciview dep that's newer than what's on the core sites (helping with, but not fully addressing, 3).

Oh, the other issue is the Launcher. I need to make it really work with Java 11

I am now mostly done with a new launcher for Fiji called Jaunch. This launcher is much smarter about discovering Java installations, as well as launching Java properly across the different major Java versions. For example, it will pass the needed --add-opens arguments when the Java being launched is v9+, but not when it's Java 8.

The plan is still to start shipping sciview with Fiji, so I'll leave this issue open till we achieve that.

kephale commented 1 month ago

This issue should be grouped with this issue: https://github.com/scenerygraphics/sciview/issues/563

kephale commented 1 month ago

Related: https://github.com/scenerygraphics/sciview/issues/498

kephale commented 1 month ago

Related: https://github.com/scenerygraphics/sciview/issues/392

kephale commented 3 weeks ago

@ctrueden should we be doing more Jaunch testing now?

ctrueden commented 3 weeks ago

Sure, if you like. I just tried briefly to combine the scijava-ops and sciview update sites. First, I followed the instructions to install SciJava Ops into Fiji (As explained on https://ops.scijava.org/). Then, I enabled the sciview update site, installed it, restarted Fiji, and tried to launch sciview. I encountered two small issues:

  1. When enabling the sciview update site, it wanted to install the newer (but still old at this point) ImageJ-linux64, and in so doing, wanted to set its executable bit, but for some reason failed to do so, resulting in an exception stack trace. To work around, I just marked ImageJ-linux64 with "Keep as-is" and was able to proceed with installing the rest of the sciview update site.
  2. After restarting and launching sciview, I got a snakeyaml error relating to the fact that the scijava-ops shadows snakeyaml at 2.0, but does not shadow ui-behaviour at 2.0.8. I fixed this by uploading ui-behaviour 2.0.8 to the scijava-ops site.

So it should now almost work to turn on both the scijava-ops and sciview update sites and enjoy the synergy. You might just have to do the same workaround I did in (1) above.

ctrueden commented 3 weeks ago

I did run into a novel issue when playing with the Game of Life 3D, though: trying to zoom around, the zoom behavior was very coarse. So then I tried "Circle camera around current object" while the GoL volume was actively animating, and it crashed Fiji outright with no errors on the console. Whee!