fix: re-enable THUMBNAIL images in the volume pyramid (#318)#389
fix: re-enable THUMBNAIL images in the volume pyramid (#318)#389igoroctaviano wants to merge 7 commits into
Conversation
Removes the skipThumbnails workaround that was added to avoid annotation misplacement when a THUMBNAIL image is present (#318). The underlying cause was in dicom-microscopy-viewer, which rounded the per-level zoom factor and thereby mis-scaled THUMBNAIL levels. With that fixed upstream (ImagingDataCommons/dicom-microscopy-viewer#245), thumbnails can be part of the rendered pyramid again, restoring fast initial load without misplacing annotations. Requires dicom-microscopy-viewer >= 0.48.22 (the release containing the fix). Refs #318
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Jul 15, 2026 4:38p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
…318-reenable-thumbnails
|
|
In this one, I am unable to zoom out to see the entire slide: https://andrey-slim-test.web.app/studies/2.25.101421708741469260194990472062574324362/series/1.3.6.1.4.1.5962.99.1.3273152385.1327370468.1755619841921.4.0 
Works outside of thisPR! https://viewer.imaging.datacommons.cancer.gov/slim/studies/2.25.101421708741469260194990472062574324362/series/1.3.6.1.4.1.5962.99.1.3273152385.1327370468.1755619841921.4.0 |
Re-enabling thumbnails made DMV lock the OL view to tile-grid resolutions, which clips zoom so the full slide cannot fit. Pass useTileGridResolutions: false so zoom stays free while thumbnails still load for a fast first paint.
|
Pushed a follow-up for the zoom-out issue in #389 (comment): Cause: with thumbnails back in the pyramid ( Fix: pass Please re-check: https://andrey-slim-test.web.app/studies/2.25.101421708741469260194990472062574324362/series/1.3.6.1.4.1.5962.99.1.3273152385.1327370468.1755619841921.4.0 (after redeploy of this branch). |
…318-reenable-thumbnails
|
|
Visit the preview URL for this PR (updated for commit 2dee600): https://idc-external-006--pr389-fix-318-reenable-thu-vl7k0ma6.web.app (expires Wed, 22 Jul 2026 16:40:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307 |
|
@fedorov please try again |
2 participants
Important
Draft — depends on ImagingDataCommons/dicom-microscopy-viewer#245.
Do not merge until that fix is released. This PR also needs a follow-up bump of
dicom-microscopy-viewerto the released version (expected^0.48.22) plus a lockfile update. Bumping now would desyncpnpm-lock.yaml, so it is intentionally left out.Summary
Removes the
skipThumbnails: trueworkaround (added in43406ea) soTHUMBNAILimages are rendered as part of the volume pyramid again. This restores the faster initial load that the thumbnail provides (requested in #318) without re-introducing the annotation misplacement.Background
#318: when a series contains a
THUMBNAIL, the slide "jumps" and annotations render in the wrong place while zooming. The root cause was in dicom-microscopy-viewer:_computeImagePyramidrounded each level's zoom factor to the nearest integer.VOLUMElevels are clean power-of-two downsamples (integer ratios), but aTHUMBNAILis not — e.g. a base/level column ratio of57.70was rounded to58, a ~0.5% scale error that drifts the upsampled thumbnail image relative to the (correctly placed) annotations. The workaround here simply dropped thumbnails from the pyramid, which also dodged the faster-load benefit.The upstream fix (DMV#245) computes exact, non-rounded resolutions so every level — including the thumbnail — stays aligned to the base coordinate system.
Test plan
tsc --noEmitpassesbiome checkpassesdicom-microscopy-viewer+ lockfile