feat: Screenshot tool can now annotate 3D physics raycast candidates#1661
Merged
Conversation
…nd coordinate metadata Ported main's final-state screenshot extension onto v3-beta's per-tool architecture: ScreenshotSchema/ScreenshotResponse gain AnnotateRaycastGrid, RaycastLayerMask, GameViewWidth/Height, ScreenshotToInputFormula, UnityInputFormula, RaycastGridPoints, and RaycastLayerSummaries. ScreenshotResponse's CoordinateSystem/YOffset fields were renamed to ImageCoordinateSystem/ImageToInputOffsetY to match main and support both window and rendering capture modes explicitly. EditorWindowCaptureUtility.CaptureGameRenderingAsync now shares a GameRenderingImageInfo struct with a new GetGameRenderingImageInfoAsync, so the raycast-grid path and the PNG-capture path reuse the same settled RenderTexture geometry without recomputing it twice, while preserving v3-beta's EditorFrameWaiter timeout-safety mechanism (not present in main). EditorWindowCaptureService, the public ToolContracts extension-API facade's internal bridge, now adapts to the new signature without changing its own public (texture, yOffset, timedOut) contract. UIElementAnnotator.CollectInteractiveElements threads a UiRaycastHelper.RaycastContext through element collection (replacing v3-beta's ad-hoc SharedPointerEventData reachability check), and CreateAnnotationOverlay now draws in two passes (all borders, then all labels) with a new outline-based border branch for PhysicsCollider elements, ported from main's e67dcba. ScreenshotUseCase.CaptureRenderingAsync integrates raycast-grid collection (grid points, layer summaries, or clustered physics-collider elements depending on RaycastLayerMask) gated on AnnotateRaycastGrid, with matching parameter validation and a default-tools.json schema update needed immediately because DefaultToolsCatalogDriftTests compares the live tool schema against it. Verified: uloop compile (0 errors/0 warnings), full EditMode suite (1774 passed, 0 failed, 7 pre-existing skips), and PlayMode regression for SimulateMouseUi/InputReplayVerification to confirm no behavior change in existing UiRaycastHelper callers.
Contributor
|
Warning Review limit reached
Next review available in: 33 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 (6)
📝 WalkthroughWalkthroughThis PR extends the screenshot tool with raycast-grid and physics-collider annotation support, plus richer image/window coordinate metadata. It updates capture utilities, use-case orchestration and validation, public tool contracts, and UI annotation rendering. ChangesScreenshot Coordinate and Raycast Annotation Refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ScreenshotTool
participant ScreenshotUseCase
participant EditorWindowCaptureUtility
participant UIElementAnnotator
Client->>ScreenshotTool: ExecuteAsync(parameters)
ScreenshotTool->>ScreenshotUseCase: ValidateParameters(schema)
alt invalid combination
ScreenshotUseCase-->>ScreenshotTool: throw UnityCliLoopToolParameterValidationException
else valid
ScreenshotUseCase->>EditorWindowCaptureUtility: CaptureGameRenderingAsync
EditorWindowCaptureUtility-->>ScreenshotUseCase: (texture, GameRenderingImageInfo, timedOut)
ScreenshotUseCase->>UIElementAnnotator: collect/annotate elements
UIElementAnnotator-->>ScreenshotUseCase: annotated elements + raycast grid data
ScreenshotUseCase->>ScreenshotUseCase: ApplyRenderingCoordinateMetadata
ScreenshotUseCase-->>ScreenshotTool: ScreenshotResponse
end
Possibly related PRs
🚥 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.cs`:
- Around line 387-405: The screenshot response contract has changed in
ScreenshotUseCase, so update the CLI protocol version to match the breaking
wire-format change. Bump the protocol in cli/common/clicontract/contract.json
and keep Packages/src/Editor/Domain/CliConstants.cs
REQUIRED_CLI_PROTOCOL_VERSION in sync, since ScreenshotInfo now uses the new
coordinate and metadata fields exposed by ApplyRenderingCoordinateMetadata and
ApplyWindowCoordinateMetadata.
- Around line 185-187: The annotation conversion in ScreenshotUseCase is using
the initially sampled Game View height instead of the settled capture height,
which can make the response metadata and ScreenshotToInputFormula disagree if
the Game View changes during the wait. Update the ConvertToSimCoordinates call
in the screenshot flow to use captureRenderingInfo.GameViewSize consistently,
and keep the annotated elements conversion aligned with the same settled height
used for the response data.
In `@Packages/src/Editor/FirstPartyTools/Screenshot/UIElementAnnotator.cs`:
- Around line 499-512: The physics-collider overlay is re-reading the current
Game View height instead of using the captured render height, which can desync
the Y flip from the screenshot geometry. Update `CreateAnnotationOverlay` to
accept and pass through the captured `GameViewSize.y` from `ScreenshotUseCase`,
and change `CalculatePhysicsAnnotationScreenHeight` to use that value instead of
calling `GameViewCoordinateUtility.GetMainGameViewSize()`. Keep the existing
`IsPhysicsColliderElement` flow, but ensure all physics-collider bounds/outline
conversion uses the screenshot’s original Game View height.
In `@Packages/src/Editor/ToolContracts/ScreenshotResponse.cs`:
- Around line 16-29: The screenshot response contract changed field names
without a protocol update, breaking older consumers. In ScreenshotResponse, keep
the existing wire names compatible by adding aliases or preserving the
serialized names for CoordinateSystem and YOffset equivalents, and make sure the
protocol version is bumped if the contract is intentionally changed. Update the
related contract definition and CliConstants.REQUIRED_CLI_PROTOCOL_VERSION
together so ScreenshotInfo remains interoperable across versions.
🪄 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: b42e88d5-dae1-4147-8a98-cd537453c21c
⛔ Files ignored due to path filters (2)
Assets/Tests/Editor/EditorWindowCaptureUtilityTests.cs.metais excluded by none and included by noneAssets/Tests/Editor/ScreenshotUseCaseTests.cs.metais excluded by none and included by none
📒 Files selected for processing (12)
Assets/Tests/Editor/EditorWindowCaptureUtilityTests.csAssets/Tests/Editor/ScreenshotUseCaseTests.csAssets/Tests/Editor/UIElementAnnotatorTests.csPackages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureService.csPackages/src/Editor/FirstPartyTools/Screenshot/EditorWindowCaptureUtility.csPackages/src/Editor/FirstPartyTools/Screenshot/ScreenshotUseCase.csPackages/src/Editor/FirstPartyTools/Screenshot/UIElementAnnotator.csPackages/src/Editor/ToolContracts/ScreenshotResponse.csPackages/src/Editor/ToolContracts/ScreenshotSchema.csPackages/src/Editor/ToolContracts/UnityCliLoopConstants.csPackages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cscli/common/tools/default-tools.json
💤 Files with no reviewable changes (1)
- Packages/src/Editor/ToolContracts/UnityCliLoopScreenshotTypes.cs
…w size CodeRabbit flagged that ConvertToSimCoordinates used the pre-capture gameViewSize sample instead of captureRenderingInfo.GameViewSize, which is already used for the response's GameViewWidth/Height metadata a few lines later. If the Game View resizes during the overlay-render wait, the two values diverge and the returned AnnotatedElements coordinates become inconsistent with the formula the response advertises.
CoordinateSystem/YOffset were renamed to ImageCoordinateSystem/ ImageToInputOffsetY earlier in this PR, but the screenshot skill docs (and their .claude/.agents generated copies) still referenced the old field names, actively guiding agents toward fields that no longer exist on the response. Regenerated the installed copies via `uloop skills install --claude --agents`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ScreenshotToInputFormula) for converting image pixels into simulate-mouse-input/raycast coordinates, removing the need to reverse-engineer the previousCoordinateSystem/YOffsetfields.User Impact
CoordinateSystem/YOffset.AnnotateRaycastGrid=trueoverlays a 5x5 grid of physics raycast hits (or, withRaycastLayerMaskset, clustered outlines of the actual hit colliders) on the screenshot, and the response includes the exact formula plus Game View size needed to drivesimulate-mouse-input/raycastfrom the image.Changes
ScreenshotSchema/ScreenshotResponsegainedAnnotateRaycastGrid,RaycastLayerMask,GameViewWidth/Height,ScreenshotToInputFormula,UnityInputFormula,RaycastGridPoints, andRaycastLayerSummaries.CoordinateSystem/YOffsetwere renamed toImageCoordinateSystem/ImageToInputOffsetY.EditorWindowCaptureUtilitynow shares aGameRenderingImageInfostruct between the raycast-grid path and the PNG capture path so both reuse the same settled RenderTexture geometry, while keeping the existing frame-wait timeout safety net.UIElementAnnotatornow shares oneUiRaycastHelper.RaycastContextwhile collecting elements (replacing an ad-hoc per-call raycast check) and draws physics-collider outlines in a separate pass from box borders/labels.ScreenshotUseCasewires raycast-grid collection into rendering capture with matching parameter validation, anddefault-tools.json'sscreenshotentry was updated in the same change (required immediately byDefaultToolsCatalogDriftTests).Verification
uloop compile— 0 errors, 0 warningsSimulateMouseUiTests,SimulateMouseUiInputSystemTests,InputReplayVerificationE2ETests) — all passed, confirming no behavior change for existingUiRaycastHelpercallers