Skip to content

fix(annotation): prevent closed freehand ROI stat mismatch between stack and volume viewports#2717

Open
Devu-trenser wants to merge 6 commits into
cornerstonejs:mainfrom
Devu-trenser:fix--planar-freehand-ROI
Open

fix(annotation): prevent closed freehand ROI stat mismatch between stack and volume viewports#2717
Devu-trenser wants to merge 6 commits into
cornerstonejs:mainfrom
Devu-trenser:fix--planar-freehand-ROI

Conversation

@Devu-trenser
Copy link
Copy Markdown
Contributor

@Devu-trenser Devu-trenser commented May 3, 2026

Context

Fixes issue: #2646

Fixes the issue where Closed Freehand ROI statistics (especially max) changed when switching between Stack Viewport and Volume Viewport.

The root cause was floating-point precision drift during world-to-index coordinate conversion.
The previous implementation used Math.floor for Z index calculation, which could incorrectly map planar annotations to the previous slice in volume viewports.

This caused ROI statistics to be computed from slightly different voxel regions between stack and volume rendering modes, even though the annotation itself remained visually unchanged.

Changes & Results

  • Updated Z index calculation from Math.floor to Math.round
  • Ensured planar annotations stay constrained to the nearest slice
  • Prevented unintended slice shifts caused by floating-point precision issues
  • Fixed inconsistency in ROI statistics between Stack and Volume viewports

Before

  • Closed Freehand ROI max value changed after switching viewport types
  • ROI could be evaluated on an adjacent slice in volume mode

After

  • ROI statistics remain consistent across Stack and Volume viewports
  • Annotation mapping stays aligned with the intended slice
2026-05-04.12-35-47.mp4

Testing

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.

Tested Environment

  • "OS: Ubuntu 24.04
  • "Node version: version: 22
  • "Browser: Chrome 139

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@Devu-trenser Devu-trenser marked this pull request as draft May 3, 2026 05:57
@Devu-trenser Devu-trenser force-pushed the fix--planar-freehand-ROI branch from 6c4ec85 to c56b2c2 Compare May 3, 2026 06:26
@Devu-trenser Devu-trenser marked this pull request as ready for review May 4, 2026 05:14
@sen-trenser
Copy link
Copy Markdown

@sedghi Could you please take a look at this PR ?
Thanks!

const zi = Math.round(worldPosIndex[2]);

kMin = Math.min(kMin, zi);
kMax = Math.max(kMax, zi);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure about your solution, this might fix in one plane, but in other planes where aquisitino is sag or cor, the issue still exists since you are taking the round on the z no?

Comment thread packages/tools/src/tools/annotation/PlanarFreehandROITool.ts Outdated
@wayfarer3130
Copy link
Copy Markdown
Collaborator

@sedghi @sen-trenser - I can't reproduce this on viewer-dev

@Devu-trenser
Copy link
Copy Markdown
Contributor Author

I can't reproduce this on viewer-dev

This can be reproduced in viewer-dev while switching between Stack and Volume viewports for a Closed Freehand ROI.

Test data: https://viewer-dev.ohif.org/viewer?StudyInstanceUIDs=1.3.6.1.4.1.5962.99.1.2968617883.1314880426.1493322302363.3.0

Attaching a video for reference.

Volume-2717.mp4

@Devu-trenser Devu-trenser force-pushed the fix--planar-freehand-ROI branch from 9f8bcf9 to 88a6c19 Compare May 25, 2026 06:41
@sen-trenser
Copy link
Copy Markdown

@wayfarer3130 Could you please take a look at the above comments from Devu and check the changes?
Thanks!

@Devu-trenser
Copy link
Copy Markdown
Contributor Author

@sedghi Could you please merge this PR?

cc: @wayfarer3130 @sen-trenser

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.

4 participants