Modify extension to request raw tiles when possible #1

melissalinkert · 2023-11-10T17:27:09Z

If the server can supply raw tiles via microservices, request them directly. This expands the range of data types supported to more than just 3 channel/uint8.

/cc @muhanadz

This probably needs to be configurable, but works for initial testing.

As noted in comments, OMERO expects resolution indexes to be specified in reverse order compared to Bio-Formats/QuPath.

pixelbuffer is what we actually use to load raw tiles.

melissalinkert · 2023-11-10T17:54:28Z

I think the .github/workflows/gradle.yml will do what we need for testing now, but I suspect this needs to be manually enabled (and I do not seem to have permission to do so).

melissalinkert · 2024-03-06T16:45:39Z

@muhanadz : build is now passing here.

muhanadz

Few issues I've encountered while testing:

Login always fails first time:

However the server is added to the list of server and works if clicked from the extentions menu:

I cannot load none uint8 + 3 channel images:
Without PR:

With this PR:

I'd expect to be able to load all images since we're getting the raw tiles.

Otherwise valid images load as expected, but will need to further test and confirm that the microservices are being used.

Testing on QuPath 0.4.4 macOS 13.5.1, M1. Happy to switch to a windows environment for less unknown variables if needed.

melissalinkert · 2024-03-08T20:02:53Z

All of the work here was against QuPath 0.3.2 as agreed approximately a year ago; I don't think we had discussed moving to a newer version of QuPath.

I can reproduce the issues above, all of which are a result of not being able to find the session cookie after login. That means there is no communication with the microservice at all because the sessionid can't be set. I suspect (but am not 100% certain) that this is a result of QuPath requiring Java 17 with 0.4.0+. Note https://github.com/qupath/qupath-extension-omero/releases/tag/v0.1.0-rc4 with QuPath 0.5.0 also cannot use the microservice.

If you test with QuPath 0.3.2, I'd expect all of this to work (keep in mind this is a public PR if adding screenshots). Before I update this PR, let's decide for sure which QuPath version we're now targeting, as I'd expect 0.4.x and 0.5.x to require different amounts of work.

…Path 0.5.1

melissalinkert · 2024-03-13T00:19:27Z

I believe the last 3 commits here are sufficient to upgrade from QuPath 0.3.2 to 0.5.1. @muhanadz, if you see any issues with 0.5.1 and the latest build (https://github.com/glencoesoftware/qupath-extension-omero/actions/runs/8257393991?pr=1) just let me know.

muhanadz · 2024-03-13T10:54:08Z

Introduced in 4e819fe: When adding the extension for the first time and trying to add a server, since there are no servers added by default, the JSON deserialisation fails and the usedServers remains null which throws this exception:

10:40:07.111	[JavaFX Application Thread]	INFO	qupath.lib.gui.ExtensionClassLoader	Adding jar: /QuPath/v0.5/extensions/qupath-extension-omero-0.3.1-SNAPSHOT 2.jar
10:40:07.115	[JavaFX Application Thread]	INFO	qupath.lib.gui.ExtensionManager	Loaded extension OMERO extension (2 ms)
10:40:10.249	[JavaFX Application Thread]	ERROR	qupath.lib.gui.QuPathUncaughtExceptionHandler	Cannot invoke "java.util.List.iterator()" because "usedServers" is null	java.lang.NullPointerException: Cannot invoke "java.util.List.iterator()" because "usedServers" is null
	at qupath.lib.images.servers.omero.OmeroExtension.lambda$createServerListMenu$3(OmeroExtension.java:148)
	at com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.event.Event.fireEvent(Event.java:198)
	at javafx.scene.control.Menu.setShowing(Menu.java:205)
	at javafx.scene.control.Menu.show(Menu.java:412)
	at com.sun.javafx.scene.control.GlobalMenuAdapter.lambda$new$1(GlobalMenuAdapter.java:78)
	at com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:136)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
	at javafx.beans.property.ReadOnlyBooleanPropertyBase.fireValueChangedEvent(ReadOnlyBooleanPropertyBase.java:78)
	at javafx.beans.property.ReadOnlyBooleanWrapper.fireValueChangedEvent(ReadOnlyBooleanWrapper.java:103)
	at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:111)
	at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:145)
	at javafx.scene.control.Menu.setShowing(Menu.java:209)
	at javafx.scene.control.Menu.show(Menu.java:412)
	at com.sun.javafx.tk.quantum.GlassMenuEventHandler.handleMenuOpening(GlassMenuEventHandler.java:41)
	at com.sun.glass.ui.Menu.notifyMenuOpening(Menu.java:186)

No way currently to avoid the exception and add a server since it's trying to iterate null every time I try to add a server.

melissalinkert · 2024-03-13T14:40:32Z

660edd4 should fix the issue noted in #1 (comment).

muhanadz · 2024-03-14T12:49:42Z

Thank Melissa for the changes.

I can now connect to an omero server and retrieve images fine. I'll continue with the thorough testing on that. Meanwhile a small issue I noticed when logging in. The server gets duplicated from the Browse Server, the first time I connect to it. Starting with the following 3 servers as an example:

Then when I connect to a server, qupath-testing for example, I see:

melissalinkert · 2024-03-14T14:49:00Z

5d64b83 should fix the issue noted in #1 (comment).

muhanadz

Tested with connections to different valid servers. Microservices are used when detected, otherwise falls back to previous behaviour. If Microservices are present, able to load multi-channel images not only uint8 but also uint16 uint32 uint64. Tile loading takes 2-3 seconds depends on connection and distance to the server but that's expected and doesn't seem to be an issue. Downstream analysis with QuPath works as expected such as cell detection. Sending ROIs from QuPath to OMERO via the extension work as expected as long as iviewer is installed and configured correctly.

sbesson

At first glance the proposed changes make sense. Talking to Muhanad, there has been some recent evolution upstream as this extension has been renamed as qupath-extension-omero-web (but the artifact name is still qupath-extension-omero) and work has started on a new experimental https://github.com/qupath/qupath-extension-omero extension still released below 0.1.0 that should include native support for OMERO deployments with micro-services amongst others. The QuPath documentation has been updated accordingly - see https://qupath.readthedocs.io/en/stable/docs/advanced/omero.html#omero-extension.

As the above is already confusing, my best suggestion would be to match the upstream name i.e.

rename this fork (and any of our forks) as qupath-extension-omero-web
keep the artifact name as qupath-extension-omero like upstream
use a version with a suffix indicating this is our custom build on top of the official QuPath extension release e.g. 0.4.0-gs or 0.4.1-gs
update the readme to describe the intent of this fork i.e. adding micro-service support to the qupath-extension-omero-web extension
also include the blurb above to the release notes on the GitHub release page

In addition to the above, should we try to sync the changes made upstream in v0.4.0? Looking at the [diff
(https://github.com/glencoesoftware/qupath-extension-omero/compare/main...qupath:qupath-extension-omero-web:v0.4.0), the main changes are:

refactoring of the top-level build configuration
Gradle wrapper upgrade to 8.6
miscellaneous changes to the source code - some of them overlapping with changes on this branch e.g. OmeroWebClientsCommand.java or OmeroWebImageServerBrowserCommand.java

...so it's clearer that builds are from our fork, not upstream.

…ero into raw-tiles

melissalinkert · 2024-03-22T19:15:06Z

At first glance the proposed changes make sense. Talking to Muhanad, there has been some recent evolution upstream as this extension has been renamed as qupath-extension-omero-web (but the artifact name is still qupath-extension-omero) and work has started on a new experimental https://github.com/qupath/qupath-extension-omero extension still released below 0.1.0 that should include native support for OMERO deployments with micro-services amongst others. The QuPath documentation has been updated accordingly - see https://qupath.readthedocs.io/en/stable/docs/advanced/omero.html#omero-extension.

As the above is already confusing, my best suggestion would be to match the upstream name i.e.

rename this fork (and any of our forks) as qupath-extension-omero-web

No objection, but it doesn't look like I have permission to do this.

keep the artifact name as qupath-extension-omero like upstream

👍

use a version with a suffix indicating this is our custom build on top of the official QuPath extension release e.g. 0.4.0-gs or 0.4.1-gs

0095d5a updates to 0.4.1-gs-SNAPSHOT; as with everything else, the -SNAPSHOT would be dropped when we tag a release.

update the readme to describe the intent of this fork i.e. adding micro-service support to the qupath-extension-omero-web extension

Done in b178bd2.

also include the blurb above to the release notes on the GitHub release page

Can do when we merge this and tag the release.

In addition to the above, should we try to sync the changes made upstream in v0.4.0? Looking at the [diff (https://github.com/glencoesoftware/qupath-extension-omero/compare/main...qupath:qupath-extension-omero-web:v0.4.0), the main changes are:

See #2. I had intended to push directly to main, but don't have permission to do so. That means that in order:

Sync with upstream #2 should be merged
this PR should be rebuilt
the new artifacts for this PR should be re-tested (so that changes in Sync with upstream #2 and recent changes here are all tested together)

…ero into raw-tiles

melissalinkert · 2024-03-26T15:01:35Z

Conflicts fixed and latest build is passing, so ready for re-testing as discussed. The gradle.yml workflow should start on every push, so I don't believe further changes are needed to build tags.

muhanadz

Performed similar tests mentioned in the previous comment #1 (review), all looks good to me ✅

The OMERO extension version is now listed correctly in the Extension manager which was "unknown" before:

The GitHub button link redirects to https://github.com/qupath/qupath-extension-omero. Might be worth making a change to redirect it to this repository.

melissalinkert · 2024-03-27T18:12:49Z

eedc8c5 should fix the GitHub button in the extension manager to point to this repository. Note that is an actual code change, not a build configuration change.

muhanadz

The button now redirects to this repository, hopefully this change clears any confusion.
Thanks, looks great.

melissalinkert · 2024-03-28T16:13:53Z

Thanks @muhanadz. Merging as discussed, in preparation for release.

melissalinkert added 12 commits November 2, 2022 11:26

Work in progress to support raw tiles

1fd1c91

Fix the port used to connect to ms-pixel-buffer

98498d4

This probably needs to be configurable, but works for initial testing.

Connect to microservice with http instead of https

3e2fbc3

Copy channel names and colors from OMERO

e998d95

Fix tile XY coordinates

cb0d553

Fix resolution indexing

00b4a0c

As noted in comments, OMERO expects resolution indexes to be specified in reverse order compared to Bio-Formats/QuPath.

Save server URLs so they don't need to be typed every time

c2983f3

Make microservice optional, fall back to webgateway if not available

201bda8

Add some clarifying comments and error handling

bea3cd8

Fix microservice check

2e6486c

Check for pixelbuffer microservice instead of imageregion

238c122

pixelbuffer is what we actually use to load raw tiles.

Update gradle workflow to retain artifact

e2a5262

melissalinkert requested a review from muhanadz November 10, 2023 17:54

melissalinkert closed this Mar 6, 2024

melissalinkert reopened this Mar 6, 2024

melissalinkert added 2 commits March 6, 2024 10:35

Use repository that includes cisd:jhdf:19.04.0

294bae1

Reintroduce scijava releases repo so that qupath-gui-fx is found

b5f83bf

melissalinkert added 2 commits March 6, 2024 15:37

Fix path of artifacts to upload

f0dd726

One more artifact path fix

2135dfe

muhanadz suggested changes Mar 7, 2024

View reviewed changes

melissalinkert added 3 commits March 12, 2024 18:27

Update dependencies and fix build for compatibility with Java 17 / Qu…

c7e025c

…Path 0.5.1

Fix server list property parsing

4e819fe

Always use a new CookieManager instead of the default

a1da8fc

Fix a few more issues with server list property handling

660edd4

Handle trailing / in saved server names

5d64b83

muhanadz approved these changes Mar 18, 2024

View reviewed changes

sbesson self-requested a review March 20, 2024 16:25

sbesson reviewed Mar 21, 2024

View reviewed changes

melissalinkert mentioned this pull request Mar 22, 2024

Sync with upstream #2

Merged

melissalinkert added 3 commits March 22, 2024 14:01

Update version number to 0.4.1-gs-SNAPSHOT

0095d5a

...so it's clearer that builds are from our fork, not upstream.

Update README to clarify what this fork does

b178bd2

Merge branch 'main' of github.com:glencoesoftware/qupath-extension-om…

92e8610

…ero into raw-tiles

Merge branch 'main' of github.com:glencoesoftware/qupath-extension-om…

4f66ae2

…ero into raw-tiles

muhanadz self-requested a review March 26, 2024 15:02

muhanadz approved these changes Mar 26, 2024

View reviewed changes

Fix GitHub link reported in extension manager

eedc8c5

muhanadz approved these changes Mar 28, 2024

View reviewed changes

melissalinkert merged commit 6738f1b into glencoesoftware:main Mar 28, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify extension to request raw tiles when possible #1

Modify extension to request raw tiles when possible #1

melissalinkert commented Nov 10, 2023

melissalinkert commented Nov 10, 2023

melissalinkert commented Mar 6, 2024

muhanadz left a comment •

edited

Loading

melissalinkert commented Mar 8, 2024

melissalinkert commented Mar 13, 2024

muhanadz commented Mar 13, 2024

melissalinkert commented Mar 13, 2024

muhanadz commented Mar 14, 2024

melissalinkert commented Mar 14, 2024

muhanadz left a comment

sbesson left a comment

melissalinkert commented Mar 22, 2024

melissalinkert commented Mar 26, 2024

muhanadz left a comment •

edited

Loading

melissalinkert commented Mar 27, 2024

muhanadz left a comment

melissalinkert commented Mar 28, 2024

Modify extension to request raw tiles when possible #1

Modify extension to request raw tiles when possible #1

Conversation

melissalinkert commented Nov 10, 2023

melissalinkert commented Nov 10, 2023

melissalinkert commented Mar 6, 2024

muhanadz left a comment • edited Loading

Choose a reason for hiding this comment

melissalinkert commented Mar 8, 2024

melissalinkert commented Mar 13, 2024

muhanadz commented Mar 13, 2024

melissalinkert commented Mar 13, 2024

muhanadz commented Mar 14, 2024

melissalinkert commented Mar 14, 2024

muhanadz left a comment

Choose a reason for hiding this comment

sbesson left a comment

Choose a reason for hiding this comment

melissalinkert commented Mar 22, 2024

melissalinkert commented Mar 26, 2024

muhanadz left a comment • edited Loading

Choose a reason for hiding this comment

melissalinkert commented Mar 27, 2024

muhanadz left a comment

Choose a reason for hiding this comment

melissalinkert commented Mar 28, 2024

muhanadz left a comment •

edited

Loading

muhanadz left a comment •

edited

Loading