fix: Screenshot annotations only mark reachable controls as clickable#1465
Conversation
Use the same raycast reachability check for screenshot annotations as UI input simulation so disabled canvases and covered controls are not reported as clickable.
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors UI raycast handling to reuse a cached ChangesRaycast reuse and annotation flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Avoid repeated Canvas and Graphic scans while collecting screenshot annotations so reachability filtering stays aligned with UI input behavior without adding per-element scene-wide work.
Render all annotation borders before creating labels so label tabs remain visible when they overlap another element's outline.
Drop unused raycast helper entry points and keep annotation draw values shared between border and label passes so the overlay code stays narrow after the review cleanup.
Clarify that RaycastContext snapshots Canvas and Graphic state so future callers keep it scoped to one frame instead of caching stale UI state.
![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
🧹 Nitpick comments (1)
Assets/Tests/PlayMode/SimulateMouseUiTests.cs (1)
341-356: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing explanatory comment for the regression scenario.
Every other non-trivial regression test in this file (e.g. lines 115-117, 171-173, 235-237) has a comment explaining the specific gotcha being tested. Consider adding one here noting that a front
Graphicblocks raycast reachability so covered controls are correctly excluded from annotation.🤖 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 `@Assets/Tests/PlayMode/SimulateMouseUiTests.cs` around lines 341 - 356, Add a short explanatory comment in CollectInteractiveElements_Should_SkipButtonCoveredByFrontGraphic to describe the regression being covered: a front Graphic intercepts raycasts, so the covered control should be excluded from UIElementAnnotator.CollectInteractiveElements. Place it near the blocker setup/assertion so the intent matches the other regression tests in SimulateMouseUiTests.
🤖 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 `@Assets/Tests/Editor/UIElementAnnotatorTests.cs`:
- Around line 15-21: The SetUp fixture in UIElementAnnotatorTests relies on
AddComponent<EventSystem>() to become current, which can leave
UIElementAnnotator.CollectInteractiveElements() using a stale EventSystem.
Update SetUp to explicitly assign EventSystem.current to the newly created
EventSystem on UIElementAnnotatorTestsEventSystem, and clear or replace any
existing current system before running the test so the raycast context is always
the fixture’s own.
---
Nitpick comments:
In `@Assets/Tests/PlayMode/SimulateMouseUiTests.cs`:
- Around line 341-356: Add a short explanatory comment in
CollectInteractiveElements_Should_SkipButtonCoveredByFrontGraphic to describe
the regression being covered: a front Graphic intercepts raycasts, so the
covered control should be excluded from
UIElementAnnotator.CollectInteractiveElements. Place it near the blocker
setup/assertion so the intent matches the other regression tests in
SimulateMouseUiTests.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26196f51-58b2-4d08-b530-a1765e7baafd
📒 Files selected for processing (4)
Assets/Tests/Editor/UIElementAnnotatorTests.csAssets/Tests/PlayMode/SimulateMouseUiTests.csPackages/src/Editor/Api/McpTools/SimulateMouseUi/UiRaycastHelper.csPackages/src/Editor/Utils/UIElementAnnotator.cs
Disable and restore any pre-existing EventSystem during UIElementAnnotatorTests so annotation collection does not raycast through stale editor scene state.
1 participant
Summary
User Impact
Changes
Verification
uloop compile --wait-for-domain-reload trueuloop run-tests --test-mode EditMode --filter-type regex --filter-value "UIElementAnnotatorTests"uloop run-tests --test-mode PlayMode --filter-type regex --filter-value "SimulateMouseUiTests"git diff --checkSummary
UiRaycastHelperreachability logic used by UI mouse click simulation.UiRaycastHelperto centralize UI hit-testing in a frame-scopedRaycastContextthat snapshots EventSystem pointer/Canvas+Graphic raycast inputs and selects the preferred hit using the existing priority rule.Tests
UIElementAnnotatorTests): added fixture setup/teardown cleanup and new coroutine coverage to verifyCollectInteractiveElements()skips buttons under disabled canvases and includes buttons when visible; added coverage for overlay render ordering (labels above borders).SimulateMouseUiTests): added regression tests to ensure clicks ignore disabled ScreenSpaceOverlay contents and to ensure interactables covered by a front UI graphic are excluded from collected interactive elements.