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

Calculate resolution sizes based on an integer scale factor, not the zoom level #9

Conversation

melissalinkert
Copy link
Member

Fixes #8. This scales each resolution by an integer amount, e.g. 2, 4, 8, etc. instead of directly using the zoom level returned by imgData which may introduce rounding errors that cause the calculated dimensions to be too large.

In addition to the synthetic data described in #8 (comment), the non-public dataset in curated/cellsens/qa-18870/ should demonstrate the original problem and the fix.

…zoom level

Fixes qupath#8. This scales each rseolution by an integer amount, e.g. 2, 4, 8, etc.
instead of directly using the zoom level returned by `imgData` which may introduce rounding errors.
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retested using the two sample files suggested i.e. the VSI dataset from curated/cellsens/qa-18870/ and the synthetic pyramid described in #8 which reproduces the dimensions of the original file.

Looking at the omero-ms-pixel-buffer logs at the ERROR level, without this PR, the synthetic file does not open and the VSI file displays errors in particular when looking at tiles on the right side.
With this PR included, errors are gone and I was able to look at both images without issue.

While testing other whole slide images, I noted this PR seems to introduce a regression in other scenarios. Taking the example of the public TCGA-OR-A5J1-01A-01-TS1.CFE08710-54B8-45B0-86AE-500D6E36D8A5.svs, zooming in shows a clear shift when going down one resolution level

Screenshot 2024-09-17 at 09 48 02 Screenshot 2024-09-17 at 09 48 05

The pyramid should have 4 resolutions level with the following dimensions

	Resolutions = 4
		sizeX[0] = 84727
		sizeX[1] = 21181
		sizeX[2] = 5295
		sizeX[3] = 2647

Increasing the verbosity of the QuPath logs at the DEBUG level seems to show the calculation introduces an off-by-one on the scaleFactor:

09:59:35.545	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	level = 0, rawZoomFactor = 1.0, scaleFactor = 1
09:59:35.545	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	  level width = 84727, level height = 21880
09:59:35.546	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	level = 1, rawZoomFactor = 0.2499911480401761, scaleFactor = 5
09:59:35.546	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	  level width = 16945, level height = 4376
09:59:35.546	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	level = 2, rawZoomFactor = 0.06249483635676939, scaleFactor = 17
09:59:35.546	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	  level width = 4983, level height = 1287
09:59:35.547	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	level = 3, rawZoomFactor = 0.031241516871835424, scaleFactor = 33
09:59:35.547	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	  level width = 2567, level height = 663
09:59:35.547	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	level = 4, rawZoomFactor = 0.012085875812904977, scaleFactor = 83
09:59:35.547	[JavaFX Application Thread]	DEBUG	qupath.lib.images.servers.omero.OmeroWebImageServer	  level width = 1020, level height = 263

@melissalinkert
Copy link
Member Author

See what you think of the approach in 2e3b745. That assumes that we can trust zoomLevelScaling to calculate the width, which I think is appropriate given that zoomLevelScaling is calculated from the widths. Instead of using zoomLevelScaling or its inverse to calculate the height, this approach instead assumes that the aspect ratio is the same at each resolution level.

This still isn't great, and the resolution heights may in some cases be one pixel too small, but in testing with the synthetic image, qa-18870, public QPTIFF data, and a large handful of the TCGA examples I have not yet found a case where the arithmetic in 2e3b745 results in failure to open the image.

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the different set of sample provided above, I am able to successfully open a variety of sample whole images (TCGA, qa-18870, QPTIFF samples...).

While navigating in QuPath and watching the pixel-buffer logs for errors, the only one that was raised was for the VSI fileset of qa-18870

omero@qupath-testing:~$ grep -A 2 -i error OMERO.ms/omero-ms-pixel-buffer/current/logs/omero-ms.log 
2024-09-20 10:04:58,245 [pixel-buffer-pool-2] ERROR c.g.o.m.p.TileRequestHandler - Exception while retrieving tile
java.lang.RuntimeException: loci.formats.FormatException: Invalid tile size: x=512, y=5120, w=512, h=452
	at ome.io.bioformats.BfPixelBuffer.getTileDirect(BfPixelBuffer.java:524)
--
2024-09-20 10:04:58,240 [pixel-buffer-pool-5] ERROR c.g.o.m.p.TileRequestHandler - Exception while retrieving tile
java.lang.RuntimeException: loci.formats.FormatException: Invalid tile size: x=1024, y=5120, w=104, h=452
	at ome.io.bioformats.BfPixelBuffer.getTileDirect(BfPixelBuffer.java:524)
--
2024-09-20 10:05:09,658 [pixel-buffer-pool-7] ERROR c.g.o.m.p.TileRequestHandler - Exception while retrieving tile
java.lang.RuntimeException: loci.formats.FormatException: Invalid tile size: x=0, y=5120, w=512, h=452
	at ome.io.bioformats.BfPixelBuffer.getTileDirect(BfPixelBuffer.java:524)

Looking at the resolution dimensions for the second series of this image as reported by Bio-Formats:

resolution 0: 9020 x 44566
resolution 1:  4510 x 22283
resolution 2: 2255 x 11142
resolution 3: 1128 x 5571
resolution 4: 564 x 2786
resolution 5: 282 x 1393
resolution 6: 141 x 697
resolution 7: 71 x 349

And indeed for resolution 3 1128 / 9020 * 44566 = 5573.220399113082... vs 5571 so we're possibly in the off-by-two pixels range. It's unclear to me how often this might happen and whether we can find a condition that solves most of the use cases without updating the imgData endpoint to expose the dimensions at each resolution level as discussed previously.

@melissalinkert
Copy link
Member Author

As discussed earlier today, holding off on further changes here for the moment, pending discussion of how to fix on the OMERO side.

@sbesson
Copy link
Member

sbesson commented Oct 3, 2024

See ome/omero-web#584 for a write-up of a possible extension of the OMERO.web imgData endpoint to expose the content of PixelBuffer.getResolutionDescriptions

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

Successfully merging this pull request may close these issues.

Potential off-by-one when calculating resolution sizes
2 participants