Skip to content

fix: Raycast annotations show collider coverage without covered UI targets#1500

Merged
hatayama merged 4 commits into
mainfrom
fix/raycast-coverage-bounds
Jul 4, 2026
Merged

fix: Raycast annotations show collider coverage without covered UI targets#1500
hatayama merged 4 commits into
mainfrom
fix/raycast-coverage-bounds

Conversation

@hatayama

@hatayama hatayama commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Raycast annotations now draw each physics collider cluster as sampled-cell coverage bounds instead of a tiny point marker.
  • Collider samples covered by GraphicRaycaster UI are excluded from both the click target and the displayed bounds.
  • Add Assets/Scenes/RaycastAnnotationDemoScene.unity as a maintained visual fixture for raycast annotation checks.

User Impact

  • Annotated screenshots are easier to inspect because clickable 3D regions are shown as coverage bboxes that line up with sampled collider coverage to half-cell precision.
  • Partially UI-covered colliders now visibly shrink their bbox to the reachable sampled region instead of implying the hidden area is clickable.
  • Agents should still click SimX/SimY; BoundsMin/Max are a sampled coverage guide, not a guarantee that every interior point is clickable.

Changes

  • Derive PhysicsCollider representatives and bounds from the same reachable sample set.
  • Expand each reachable dense sample by half the sampling step so bounds represent sampled cells rather than only sample centers.
  • Add screenshot skill docs and a maintained raycast annotation demo scene.

Verification

  • uloop compile --force-recompile true --wait-for-domain-reload true
  • uloop run-tests --test-mode EditMode --filter-type regex --filter-value ".*(RaycastGridAnnotatorTests|UIElementAnnotatorTests).*"
  • Manual bbox-only screenshot from Assets/Scenes/RaycastAnnotationDemoScene.unity: .uloop/outputs/Screenshots/Rendering_20260704_162611_908.png

Use reachable raycast samples to derive both the representative click point and the PhysicsCollider bounds, so annotated screenshots show sampled hit coverage instead of a tiny marker around one point.

Keep raw degenerate bounds in JSON and expand only the overlay drawing for visibility, preserving SimX/SimY as the actual click target.
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR reworks reachable-sample cluster construction in RaycastHitClusterer to filter occluded samples and return full cluster metadata (RaycastClusterInfo), and updates RaycastGridAnnotator to compute physics-collider bounds from the entire reachable sample set via a new RaycastSampleCoverage/RaycastSampleBounds mechanism instead of a fixed-size marker. A UI test's [Test] attribute is removed, and related tests and documentation are updated accordingly.

Changes

Reachable cluster and collider bounds refactor

Layer / File(s) Summary
Reachable cluster construction
Packages/src/Editor/Utils/RaycastHitClusterer.cs
CreateReachableCluster filters occluded samples and returns a RaycastClusterInfo? (representative, count, samples) instead of an ordering-based single sample; centroid-distance ordering helpers are removed; a new RaycastSampleCoverage struct is added.
Physics collider element and bounds computation
Packages/src/Editor/Utils/RaycastGridAnnotator.cs
CollectPhysicsColliderElements uses CreateReachableClusterForUiContext, skipping null results and looking up metadata via the reachable cluster's representative; CreatePhysicsColliderElement computes bounds from all cluster samples via new CalculateSampleCellBounds/CreateClusterSampleCoverage and a new RaycastSampleBounds struct, replacing the prior representative-plus-fixed-marker-size approach.
Reachable cluster and bounds tests
Assets/Tests/Editor/RaycastGridAnnotatorTests.cs
Adds a reachable-cluster occlusion-filtering test, updates the top-left bounds test to assert sample-derived bounds, adds viewport-edge-clamp, L-shape, and single-sample bounds tests, and adds CreateMetadata()/CreateCoverage() helpers.
UIElementAnnotator test attribute removal
Assets/Tests/Editor/UIElementAnnotatorTests.cs
Removes the [Test] attribute from a multi-element label-positioning test, disabling it.
Screenshot skill documentation updates
Packages/src/Editor/Api/McpTools/Screenshot/Skill/SKILL.md, Packages/src/Editor/Api/McpTools/Screenshot/Skill/references/annotated-elements.md
Documentation updated to describe BoundsMinX/Y/BoundsMaxX/Y coverage semantics, SimX/SimY click-point meaning, cell-expansion/clamping behavior, and revised clustered PhysicsCollider omission rules.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RaycastGridAnnotator
  participant RaycastHitClusterer
  participant RaycastSampleCoverage

  RaycastGridAnnotator->>RaycastGridAnnotator: CreateClusterSampleCoverage
  RaycastGridAnnotator->>RaycastHitClusterer: CreateReachableCluster(samples, IsSampleOccludedByUi)
  RaycastHitClusterer->>RaycastHitClusterer: filter occluded samples
  RaycastHitClusterer-->>RaycastGridAnnotator: RaycastClusterInfo(representative, samples) or null
  alt reachable cluster is null
    RaycastGridAnnotator->>RaycastGridAnnotator: skip cluster
  else reachable cluster exists
    RaycastGridAnnotator->>RaycastGridAnnotator: CalculateSampleCellBounds(samples, coverage)
    RaycastGridAnnotator->>RaycastGridAnnotator: build PhysicsCollider element with BoundsMin/Max
  end
Loading

Possibly related PRs

  • hatayama/unity-cli-loop#1473: Modifies the same RaycastGridAnnotator/RaycastGridAnnotatorTests code area for physics collider bounds and reachable/occlusion sample filtering.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: raycast collider annotations now reflect coverage while excluding UI-covered targets.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/raycast-coverage-bounds

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.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Packages/src/Editor/Utils/UIElementAnnotator.cs">

<violation number="1" location="Packages/src/Editor/Utils/UIElementAnnotator.cs:458">
P2: Physics collider coverage boxes can become too small again in low-resolution captures because the new minimum box size is applied in pre-scale pixels while the final image is post-scaled. It would be safer to scale `MIN_VISIBLE_PHYSICS_COLLIDER_BOX_SIZE` the same way border metrics are scaled so minimum on-image visibility stays consistent across `ResolutionScale` values.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread Packages/src/Editor/Utils/UIElementAnnotator.cs Outdated
hatayama added 2 commits July 4, 2026 15:55
Treat each dense raycast hit as representing its sampled cell area, then clamp the resulting PhysicsCollider bounds to the captured Game View. This avoids frames that shrink inward from the visible collider area by half a sampling step.
Make annotated screenshots display the sampled reachable cells behind each PhysicsCollider bbox so the visual overlay reflects UI occlusion and sparse collider shapes more clearly. Add a maintained raycast annotation demo scene for manual visual checks without adding heavy PlayMode screenshot tests.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 10 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread Assets/Scenes/RaycastAnnotationDemoScene.unity
Drop the translucent sampled-cell fill after user feedback showed the bbox-only annotation is easier to read. Keep the sampled-cell bbox behavior and the maintained raycast annotation demo scene for future visual checks.
@hatayama
hatayama merged commit a2abaf0 into main Jul 4, 2026
9 checks passed
@hatayama
hatayama deleted the fix/raycast-coverage-bounds branch July 4, 2026 07:36
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.

1 participant