Skip to content

fix(wsi): stop scaling canvas points by devicePixelRatio in the generic WSI viewport - #2942

Open
namespaceMarcello wants to merge 1 commit into
cornerstonejs:mainfrom
namespaceMarcello:fix/wsi-generic-dpr
Open

namespaceMarcello wants to merge 1 commit into
cornerstonejs:mainfrom
namespaceMarcello:fix/wsi-generic-dpr

Conversation

@namespaceMarcello

@namespaceMarcello namespaceMarcello commented Sep 26, 2026 •

Copy link
Copy Markdown

Context

Fixes #2754 for the generic (planar "next") WSI viewport.

#2769 fixed the legacy WSIViewport: its canvas is sized in CSS pixels and its transform works in CSS pixels, so scaling canvas points by devicePixelRatio applied the ratio twice and misplaced annotations on HiDPI displays. The generic WSI path in GenericViewport/WSI still does the same scaling:

  • canvasToIndexForWSI multiplies the canvas point by devicePixelRatio and indexToCanvasForWSI divides the result by it, while the transform is centred on canvasWidth / 2, canvasHeight / 2, and the viewport passes element.clientWidth / clientHeight, which are CSS pixels.
  • WSIResolvedView.getCenterIndexForCanvasPoint, used to zoom at the cursor, multiplies the canvas point the same way.

With a ratio of 2, the canvas centre maps to a point half a canvas away from the view centre. buildICamera then puts the camera focal point there, and the annotations, which are drawn through this transform, move relative to the slide as the view zooms. Clicks still round-trip, which is why placement looks right until the zoom changes. That is the behaviour reported in #2754.

Changes & Results

  • wsiTransformUtils.ts: canvasToIndexForWSI and indexToCanvasForWSI no longer scale by devicePixelRatio, and the unused devicePixelRatio argument goes (the functions are internal, and no caller passed it).
  • WSIResolvedView.getCenterIndexForCanvasPoint: the canvas point is used as is, in the same CSS-pixel space as the transform.
  • test/wsiTransformUtils.jest.js: new cases at device pixel ratios 1, 2 and 3. The canvas centre maps to the view centre and back; one CSS pixel moves the index by one resolution step; zooming at a canvas point keeps the index under the cursor. The existing test "divides the whole transformed point by an explicit devicePixelRatio argument" described the double scaling (its comment notes that the whole point, centre offset included, scales by 1/dpr). It goes with the argument.

At a device pixel ratio of 2, on a 300 × 150 canvas with the view centred on index (30, 70), resolution 2 and no rotation:

  • Before: canvas (150, 75), the centre, maps to index (330, -80), and canvas (160, 75), 10 pixels to its right, to x = 370 instead of 50.
  • After: they map to (30, 70) and x = 50.

Testing

Verified:

  • On main, the centre and step cases fail at ratios 2 and 3 and pass at 1. With the fix all 33 tests in the file pass.
  • Breaking one thing at a time: dividing by the ratio again in indexToCanvasForWSI alone fails only the centre case at ratios 2 and 3. Fixing the utilities but leaving the ratio in getCenterIndexForCanvasPoint fails only the zoom-at-cursor case at ratios 2 and 3.
  • prettier --check, oxlint, tsc --noEmit for core, jest, and the full karma suite pass locally.

To try it:

  1. Open the WSI annotation example with the generic viewport on a HiDPI display (or with browser zoom at 200%).
  2. Draw an annotation, then zoom with the mouse wheel.
  3. Before, the annotation drifts away from the tissue it was drawn on; now it stays on it.

Checklist

PR

  • My Pull Request title is descriptive, accurate and follows the
    semantic-release format and guidelines.

Code

  • My code has been well-documented (function documentation, inline comments,
    etc.)

Public Documentation Updates

  • The documentation page has been updated as necessary for any public API
    additions or removals. (No public API change.)

Tested Environment

  • "OS: Windows 11"
  • "Node version: 24.13.0"
  • "Browser: Chrome Headless (karma)"

Written by Claude Code (AI assistant) on behalf of @namespaceMarcello, who directs this work.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KdTu6m4ycpVjrxdZbMtztk

Summary by CodeRabbit

  • Bug Fixes
    • Corrected coordinate mapping in whole-slide image views so canvas positions align consistently across different display pixel ratios.
    • Improved rotation, panning, and zoom behavior, including keeping the image position under the cursor during zoom.

…ic WSI viewport

The generic WSI path sizes its transform on the element's clientWidth and
clientHeight, CSS pixels, like the legacy WSIViewport, whose double
devicePixelRatio scaling cornerstonejs#2769 removed. canvasToIndexForWSI still
multiplied the canvas point by the ratio, indexToCanvasForWSI divided by
it, and WSIResolvedView.getCenterIndexForCanvasPoint did the same when
zooming at the cursor. At a ratio of 2 the canvas centre mapped half a
canvas away from the view centre, and annotations drifted off the tissue
as the view zoomed.

Use the canvas point as is, and drop the devicePixelRatio argument that
no caller passed. The test that described the double scaling goes with
it; new cases at ratios 1, 2 and 3 check the centre, the step per pixel
and zooming at the cursor.

Fixes cornerstonejs#2754

Assisted-by: Claude Code
Generated-By: Claude
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KdTu6m4ycpVjrxdZbMtztk
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f9f627b6-c9a0-4610-96aa-a6cda4776cc0

📥 Commits

Reviewing files that changed from the base of the PR and between 071b8b5 and 49b55bc.

📒 Files selected for processing (3)
  • packages/core/src/RenderingEngine/GenericViewport/WSI/WSIResolvedView.ts
  • packages/core/src/RenderingEngine/GenericViewport/WSI/wsiTransformUtils.ts
  • packages/core/test/wsiTransformUtils.jest.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

WSI canvas-to-index and index-to-canvas transforms now use CSS pixels without device-pixel-ratio scaling. Tests cover coordinate mapping, movement, and zoom behavior across device pixel ratios.

Changes

WSI Coordinate Handling

Layer / File(s) Summary
CSS-pixel transform functions
packages/core/src/RenderingEngine/GenericViewport/WSI/wsiTransformUtils.ts
canvasToIndexForWSI and indexToCanvasForWSI no longer accept or apply a device pixel ratio.
Viewport coordinates and transform tests
packages/core/src/RenderingEngine/GenericViewport/WSI/WSIResolvedView.ts, packages/core/test/wsiTransformUtils.jest.js
getCenterIndexForCanvasPoint calculates offsets from CSS-pixel coordinates. Tests check mapping, movement, and zoom behavior at device pixel ratios 1, 2, and 3.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 49b55

The CSS-pixel coordinate changes appear ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix and follows the semantic-release format: it removes incorrect devicePixelRatio scaling in the generic WSI viewport.
Description check ✅ Passed The description is complete and relevant. It includes context, issue reference, implementation changes, expected results, detailed testing, reproduction steps, checklist completion, and tested environ…
Linked Issues check ✅ Passed Issue #2754 requires annotations to remain at their slide locations after zoom and pan for device pixel ratios other than 1. The PR removes device-pixel-ratio scaling from canvasToIndexForWSI, `inde…
Out of Scope Changes check ✅ Passed The changed source code directly implements the coordinate-space fix for issue #2754. The test changes replace coverage for the removed scaling behavior with coverage for CSS-pixel mapping and cursor-…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

[Bug] WSI Viewport - Annotation coordinate is wrong unless DevicePixelRatio = 1

1 participant