fix(gsps): restrict multi-frame overlays to referenced frames

[timcogan]

Summary by CodeRabbit

• New Features

  • GSPS overlays in multi-frame DICOM images now display graphics tied to specific frames, so annotations appear only on their intended frame.

• Improvements

  • More accurate overlay selection and rendering for presentation states that reference particular frames across single-view and grid displays.

• Tests

  • Updated tests to validate frame-specific overlay behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81b1b937-ad4e-44fe-966e-0c1f13ac0a29

📥 Commits

Reviewing files that changed from the base of the PR and between a4d9dca and 042243e.

📒 Files selected for processing (1)

• src/dicom.rs

---

📝 Walkthrough

Walkthrough

The changes add frame-specific GSPS overlay support: graphics are stored with optional referenced frames, parsing was refactored to track frame references, and rendering now filters and draws only graphics for the current frame index.

Changes

Cohort / File(s)	Summary
GSPS Overlay Rendering src/app.rs	Updated draw_gsps_overlay to accept a frame_index and updated all call sites to pass the current frame; rendering now uses overlay.graphics_for_frame(frame_index). Tests updated to use GspsOverlay::from_graphics.
GSPS Data Structures & Parsing src/dicom.rs	Replaced GspsOverlay.graphics: Vec<GspsGraphic> with Vec<GspsOverlayGraphic> (graphics + optional referenced_frames). Added GspsOverlay::from_graphics and graphics_for_frame(frame_index). Refactored parsing to produce ReferencedImageTarget entries, added helpers (collect_*_referenced_image_targets, referenced_image_target_from_item, read_item_multi_int) and dedup/sort logic. Tests extended to cover frame deduplication and mapping.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant App as App (render)
participant Overlay as GspsOverlay
participant Image as DicomImage
participant Painter as egui::Painter

App->>Image: request current image + frame_index
App->>Overlay: call draw_gsps_overlay(painter, rect, image, frame_index)
Overlay->>Overlay: graphics_for_frame(frame_index) — filter by referenced_frames
loop for each graphic
    Overlay->>Painter: render graphic primitives onto rect
end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat(viewer): add GSPS overlay support with toggle, docs, and tests #23: Earlier GSPS overlay work that this PR extends—both modify GSPS parsing/types and rendering to support per-frame referenced graphics.

Poem

🐰 I hop through frames with careful art,
Each graphic knows which slice it should start,
Wrapped and counted, drawn where it belongs,
A rabbit's cheer for precise overlay songs! 🎨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 34.62% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding frame filtering to GSPS overlays in multi-frame images. The changes directly implement restriction of overlays to referenced frames via frame_index parameter and graphics_for_frame() method.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/gsps-frame-filtering

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/dicom.rs`:
- Around line 397-410: The current chain turns a present-but-unusable
ReferencedFrameNumber tag into None by filtering out empty vectors, causing
graphics_for_frame() to treat it as “all frames”; instead preserve presence by
returning Some(empty_vec) when all values are filtered out. Locate the
referenced_frames construction using read_item_multi_int and
REFERENCED_FRAME_NUMBER and remove the .filter(|frames| !frames.is_empty()) step
so the code only maps to a Vec<usize> (sorting/deduping) and returns Some(vec)
even if empty; ensure downstream graphics_for_frame() can detect Some(vec![]) as
“no frames” rather than None meaning “all frames.”

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5b6f6f2e-72c0-4806-a1fa-9ba3b000434a

📥 Commits

Reviewing files that changed from the base of the PR and between 583ceee and a4d9dca.

📒 Files selected for processing (2)

• src/app.rs
• src/dicom.rs