fix middle slice circle 3d - #2775
Conversation
📝 WalkthroughWalkthroughIn ChangesMiddle Slice Tolerance Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/tools/src/tools/segmentation/CircleROIStartEndThresholdTool.ts`:
- Around line 429-435: The middle-slice spacing check in
CircleROIStartEndThresholdTool should use the annotation’s persisted spacing
instead of reloading the volume from cache. Remove the cache.getVolume lookup
based on getTargetId(viewport), and read spacingInNormal from the annotation
metadata already attached to the ROI state. Update the Math.abs(...) comparison
in the render-path logic to pass that stored spacing into the threshold check so
it remains stable after decaching or viewport target switches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e9b2832a-a44d-4cb3-9c53-2558f1b1a049
📒 Files selected for processing (1)
packages/tools/src/tools/segmentation/CircleROIStartEndThresholdTool.ts
| let isNearMiddleSlice = false; | ||
| const targetId = this.getTargetId(viewport); | ||
| const imageVolume = cache.getVolume(targetId.split(/volumeId:|\?/)[1]); | ||
| if ( | ||
| Math.abs(roundedCameraCoordinate - middleCoordinate) < | ||
| csUtils.getSpacingInNormalDirection(imageVolume, viewplaneNormal) | ||
| ) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use the annotation’s stored spacing here instead of reloading the viewport volume.
This render-path lookup can now throw if the volume was decached, and it can also pick the wrong spacing after a viewport target switch. spacingInNormal is already persisted on the annotation metadata, so use that instead of cache.getVolume(...) here.
Proposed fix
- let isNearMiddleSlice = false;
- const targetId = this.getTargetId(viewport);
- const imageVolume = cache.getVolume(targetId.split(/volumeId:|\?/)[1]);
- if (
- Math.abs(roundedCameraCoordinate - middleCoordinate) <
- csUtils.getSpacingInNormalDirection(imageVolume, viewplaneNormal)
- ) {
- isNearMiddleSlice = true;
- }
+ const isNearMiddleSlice =
+ Math.abs(roundedCameraCoordinate - middleCoordinate) <
+ metadata.spacingInNormal;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let isNearMiddleSlice = false; | |
| const targetId = this.getTargetId(viewport); | |
| const imageVolume = cache.getVolume(targetId.split(/volumeId:|\?/)[1]); | |
| if ( | |
| Math.abs(roundedCameraCoordinate - middleCoordinate) < | |
| csUtils.getSpacingInNormalDirection(imageVolume, viewplaneNormal) | |
| ) { | |
| const isNearMiddleSlice = | |
| Math.abs(roundedCameraCoordinate - middleCoordinate) < | |
| metadata.spacingInNormal; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/tools/src/tools/segmentation/CircleROIStartEndThresholdTool.ts`
around lines 429 - 435, The middle-slice spacing check in
CircleROIStartEndThresholdTool should use the annotation’s persisted spacing
instead of reloading the volume from cache. Remove the cache.getVolume lookup
based on getTargetId(viewport), and read spacingInNormal from the annotation
metadata already attached to the ROI state. Update the Math.abs(...) comparison
in the render-path logic to pass that stored spacing into the threshold check so
it remains stable after decaching or viewport target switches.
1 participant
Context
We had an issue for circleStartEnd, as the central slice is the only one interactable, if we had an even number of slice the camera position was never able to match the center of the ROI.
This PR changes behaviour to activate handles for closest slices of the center (determined by slice position is lower than thickness, can select two slices for even number of slice in ROI)
Changes & Results
Handles are not lost if even number of slices in the ROI
Testing
Manually through test
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Linux and chrome latest
Summary by CodeRabbit