Skip to content

fix(annotation): stabilize planar freehand ROI statistics for oblique volume data - #2718

Closed
Devu-trenser wants to merge 4 commits into
cornerstonejs:mainfrom
Devu-trenser:fix--oblique-closed-freehand
Closed

fix(annotation): stabilize planar freehand ROI statistics for oblique volume data#2718
Devu-trenser wants to merge 4 commits into
cornerstonejs:mainfrom
Devu-trenser:fix--oblique-closed-freehand

Conversation

@Devu-trenser

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

Copy link
Copy Markdown
Contributor

Context

Fixes issue: #2386

Fixes an issue where Closed Freehand ROI statistics could become inconsistent or invalid when viewing annotations on oblique volume data.

The problem was caused by floating-point precision differences during scanline processing in worldToCanvas projection space.
Small subpixel variations in canvas Y coordinates caused repeated scanline resets during voxel traversal, which could result in:

  • Missing voxels inside the ROI
  • Incorrect min/max/mean statistics
  • Inconsistent values between viewport orientations
  • Invalid Infinity or NaN values being propagated into SR measurements

Changes & Results

  • Stabilized scanline row comparison for oblique projections
  • Prevented unnecessary scanline resets caused by floating-point precision differences
  • Improved voxel inclusion consistency during ROI statistics calculation
  • Added validation checks to prevent invalid numeric values (Infinity / NaN) from being written into SR reports
  • Improved consistency of ROI statistics across stack and volume viewports

Before

  • ROI statistics could fluctuate for oblique data
  • Scanline processing could skip valid voxels
  • Infinity / NaN values could appear in exported SR measurements
  • Max/min values could differ between viewport types

After

  • Stable and consistent ROI statistics across viewport types
  • Reliable voxel traversal for oblique projections
  • Invalid numeric values are filtered before SR generation
  • Improved measurement robustness for volume rendering workflows

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: 22
  • "Browser: Chrome 139

@claude claude Bot left a comment

Copy link
Copy Markdown

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 09:01
@Devu-trenser
Devu-trenser marked this pull request as ready for review May 4, 2026 07:16
@sen-trenser

Copy link
Copy Markdown

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

Comment thread packages/adapters/src/adapters/helpers/validateNumericValue.ts Outdated
@Devu-trenser
Devu-trenser force-pushed the fix--oblique-closed-freehand branch from 37b9f9c to 6a00e71 Compare May 22, 2026 09:40
@Devu-trenser
Devu-trenser force-pushed the fix--oblique-closed-freehand branch from 5fc9b35 to c26fe97 Compare May 25, 2026 06:35
@sen-trenser

Copy link
Copy Markdown

@sedghi Could you please review the changes?
Thanks!

*/
const TOLERANCE = 0.5;

const isOblique =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This isn't a good oblique detection - to do this correctly, you will have to use the dot product of the VPN with the view plane normal of the images. Perhaps oblique isn't what you want to detect? By oblique, I mean that the VPN of the display isn't orthogonal and isn't co-linear with the image VPN.

There is another PR that converts the internal points to use single-voxel delta distances - I THINK that one will just address this change directly without you needing to do anything here, but I don't really know. The way it works is that it converts everything to a voxel-relative x/y "canvas" like space to perform the calculations.

@wayfarer3130 wayfarer3130 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@sedghi - I think we should reject this PR in favour of fixing the oblique tests to use the voxel-centric x/y two dimensional coordinate system instead as that fixes it fundamentally rather than a work around.

@wayfarer3130

Copy link
Copy Markdown
Collaborator

The PR I'm thinking of that was just recently merged is https://github.com/cornerstonejs/cornerstone3D/pull/2717/changes
I'm hoping that either directly addresses the issue, or at a minimum can be used as a pattern to address the issue.

@Devu-trenser

Devu-trenser commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

This issue is not fixed as part of PR https://github.com/cornerstonejs/cornerstone3D/pull/2717/changes. However, it has been resolved in the open PR #2744.

We verified the fix locally by linking against the branch arul-trenser:fix-freehand-mpr-layout-oblique, and the issue no longer reproduces.

@wayfarer3130, could you please confirm whether we can close this PR in favor of the changes implemented in https://github.com/cornerstonejs/cornerstone3D/pull/2744/changes#diff-30e53f5adb45d004036f0a67ea62e0729ce6f25b65136476867fc977ad5fabf0R954

A reference video has been attached for verification.

PR-2718-Resolved.mp4

@sedghi

sedghi commented Jul 8, 2026

Copy link
Copy Markdown
Member

Thanks for the detailed analysis of the scanline precision issue. Per the review discussion, we are pursuing the voxel-centric fix in #2744 instead of stabilizing the projection-space scanlines, and you confirmed #2744 resolves #2386 in your testing, so closing this in its favor

@sedghi sedghi closed this Jul 8, 2026
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