fix: simulate-mouse-ui clicks visible overlay UI instead of hidden targets at scaled Game view resolutions - #1324
Closed
hatayama wants to merge 4 commits into
Closed
Conversation
EventSystem clips GraphicRaycaster hits at Screen.width/height, which can be smaller than the Canvas layout space (scaled Game view resolution). When a non-UI raycaster (e.g. PhysicsRaycaster) still hit at such a position, RaycastUI dispatched to that first result even though the ScreenSpaceOverlay UI element in front was the target a real user would reach. Prefer the existing Canvas-space hit test whenever the first EventSystem result does not come from a GraphicRaycaster; UI-vs-UI ordering stays delegated to Unity's own RaycastComparer. - Unify the drag path's duplicated raycast-plus-fallback logic onto UiRaycastHelper.RaycastUI so DragMove resolves targets the same way - Add a PlayMode regression test that reproduces the clipped-UI case with a test-only BaseRaycaster, since this project disables the physics modules
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughSimulateMouseUi now selects raycast targets using a priority-aware comparison of Canvas-space UI candidates against EventSystem results, ordering by module sort/render priorities, then sorting layer, order, and depth. The tool delegates raycast selection to an enhanced UiRaycastHelper, and Physics2D dependency support is added. Three new PlayMode tests validate that clipped overlay UI, Physics2D conflicts, and layered modal scenarios all dispatch clicks to the frontmost interactive UI element rather than lower-priority raycast targets. ChangesUI Raycast Priority Selection
Sequence Diagram(s)sequenceDiagram
participant SimTool as SimulateMouseUiTool
participant UiHelper as UiRaycastHelper
participant EventSys as EventSystem
participant Canvas as RaycastCanvasSpace
participant CompPrio as CompareRaycastPriority
SimTool->>UiHelper: RaycastUI(screenPosition, EventSystem.current)
UiHelper->>EventSys: RaycastAll(screenPosition, results)
UiHelper->>Canvas: RaycastCanvasSpace(screenPosition)
Canvas->>Canvas: Build RaycastResult candidates
Canvas->>CompPrio: Compare by module priority, layer, order, depth
CompPrio-->>Canvas: Best candidate
alt EventSystem results present
UiHelper->>CompPrio: ShouldPreferCanvasSpaceHit
CompPrio-->>UiHelper: Preferred result
else No EventSystem results
UiHelper-->>UiHelper: Use Canvas-space candidate
end
UiHelper-->>SimTool: RaycastResult
SimTool->>SimTool: pointerData.pointerCurrentRaycast = result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🤖 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/PlayMode/SimulateMouseUiTests.cs`:
- Around line 143-148: The test currently only asserts the first RaycastResult
isn't from a GraphicRaycaster but doesn't ensure the clipped overlay isn't
present later; update the assertions after calling
EventSystem.current.RaycastAll(pointerData, raycastResults) to validate that no
entry in raycastResults has module is GraphicRaycaster (or specifically that the
overlay GameObject/target is not contained in raycastResults), e.g., iterate
raycastResults and Assert.IsFalse for any r.module is GraphicRaycaster (or
r.gameObject == overlayTarget) so the overlay hit is proven to be fully
excluded.
🪄 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: ec39e33a-7037-4498-afe4-2a9cb913aa48
📒 Files selected for processing (3)
Assets/Tests/PlayMode/SimulateMouseUiTests.csPackages/src/Editor/Api/McpTools/SimulateMouseUi/SimulateMouseUiTool.csPackages/src/Editor/Api/McpTools/SimulateMouseUi/UiRaycastHelper.cs
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Rank canvas-space fallback candidates by sorting layer before sorting order, since sortingOrder only orders canvases within the same sorting layer. Also assert in the regression test that the overlay element is fully clipped out of EventSystem results, so the test provably exercises the canvas-space fallback path instead of relying on RaycastComparer internals to keep UI hits in first place.
Contributor
|
Thanks for putting this alternative fix together. I tested this branch with a minimal saved-scene repro project: https://github.com/prog-k-dev/uloop-raycast-repro Full verification details are in #1317: Short version: this branch still fails in the UI-vs-UI ordering case where the first |
Compare Canvas-space overlay candidates against EventSystem hits using UI raycast priority so clipped frontmost overlay controls can still win over lower-priority GraphicRaycaster results.
Cover the real Physics2D raycaster path in the test Unity project while keeping the package manifest free of Physics2D dependencies.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
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/Api/McpTools/SimulateMouseUi/UiRaycastHelper.cs">
<violation number="1" location="Packages/src/Editor/Api/McpTools/SimulateMouseUi/UiRaycastHelper.cs:26">
P1: Manual Canvas-space fallback now overrides existing UI hits, which can reorder UI-vs-UI selection versus EventSystem. Restrict fallback preference to cases where EventSystem first hit is non-UI.
(Based on your team's feedback about limiting Canvas-space fallback scope to non-UI/no-usable-UI cases.) [FEEDBACK_USED].</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return results[0]; | ||
| RaycastResult firstHit = results[0]; | ||
|
|
||
| if (canvasSpaceHit != null && ShouldPreferCanvasSpaceHit(canvasSpaceHit.Value, firstHit)) |
There was a problem hiding this comment.
P1: Manual Canvas-space fallback now overrides existing UI hits, which can reorder UI-vs-UI selection versus EventSystem. Restrict fallback preference to cases where EventSystem first hit is non-UI.
(Based on your team's feedback about limiting Canvas-space fallback scope to non-UI/no-usable-UI cases.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/src/Editor/Api/McpTools/SimulateMouseUi/UiRaycastHelper.cs, line 26:
<comment>Manual Canvas-space fallback now overrides existing UI hits, which can reorder UI-vs-UI selection versus EventSystem. Restrict fallback preference to cases where EventSystem first hit is non-UI.
(Based on your team's feedback about limiting Canvas-space fallback scope to non-UI/no-usable-UI cases.) .</comment>
<file context>
@@ -17,38 +17,31 @@ internal static class UiRaycastHelper
RaycastResult firstHit = results[0];
- if (firstHit.module is GraphicRaycaster)
+ if (canvasSpaceHit != null && ShouldPreferCanvasSpaceHit(canvasSpaceHit.Value, firstHit))
{
- return firstHit;
</file context>
Suggested change
| if (canvasSpaceHit != null && ShouldPreferCanvasSpaceHit(canvasSpaceHit.Value, firstHit)) | |
| if (canvasSpaceHit != null && !IsGraphicRaycast(firstHit)) |
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
2 participants
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
simulate-mouse-ui(and input replay) now dispatches to the visible ScreenSpaceOverlay UI element instead of a target behind it when the Game view runs at a scaled target resolution.User Impact
EventSystemclipped UI raycasts atScreen.width/Screen.height. If a non-UI raycaster (e.g.PhysicsRaycaster) still hit at that position, the click went to that hidden target — or reported the wrong element — even though a real user would reach the frontmost button.Changes
UiRaycastHelper.RaycastUI: when the firstEventSystemresult does not come from aGraphicRaycaster, prefer the existing Canvas-space hit test (ScreenSpaceOverlay UI always renders in front of world-space content). UI-vs-UI ordering stays delegated to Unity's ownRaycastComparer.SimulateMouseUiTool.UpdatePointerRaycast: unified the drag path's duplicated raycast-plus-fallback logic onto the shared helper, soDragMoveresolves targets the same way.BaseRaycaster, since this project intentionally disables the physics modules.Relationship to #1318
Alternative approach to #1318 (thanks @prog-k-dev for the report and investigation). On Unity 2022.3.62f3 and 6000.3,
EventSystem.RaycastAllalready orders overlay UI ahead of physics hits whenever theGraphicRaycasteractually hits the element (raycastersortOrderPriorityresolves first), so this PR keeps Unity's ordering for UI-vs-UI and only covers the case where the UI hit is clipped away entirely.Closes #1317.
Verification
main("Overlay UI should receive the click") and passes with this change.uloop compile: 0 errors / 0 warnings.uloop run-tests --test-mode PlayModeforTests.PlayMode.SimulateMouseUiTests: 22/22 passed.SimulateKeyboardTests.Press_Should_CreateOverlayCanvas_WithUnitScale) also fails on a cleanmaincheckout in this environment (Game view scale dependent) and is unrelated.Overview
This PR fixes simulate-mouse-ui so simulated clicks at scaled Game view resolutions (where Screen bounds can clip EventSystem raycasts) target the visually frontmost
ScreenSpaceOverlayuGUI element instead of selecting behind-the-overlay or non-UI raycast targets.What changed
UiRaycastHelper.RaycastUI
RaycastCanvasSpace.EventSystem.RaycastAllreturns results, it no longer blindly usesresults[0]; instead it conditionally prefers the Canvas-space hit overresults[0]via a newShouldPreferCanvasSpaceHithelper.EventSystem.RaycastAllreturns no results, it falls back to the Canvas-space hit.UiRaycastHelper.RaycastCanvasSpace
RaycastResults that include:sortingLayersortingOrderdepthCompareRaycastPriority, incorporating:RaycastResultis populated with the chosen graphic andsortingOrder.SimulateMouseUiTool.UpdatePointerRaycast
pointerData.pointerCurrentRaycastfromUiRaycastHelper.RaycastUI(position, EventSystem.current), defaulting to an emptyRaycastResultwhen null.Tests (PlayMode regression coverage)
Updated
Assets/Tests/PlayMode/SimulateMouseUiTests.cswith new PlayMode regression tests covering:ScreenSpaceOverlayover a non-UI raycast hit (using test-onlyBaseRaycaster/RaycastResultinjection).Physics2DRaycasterhit when the physics raycaster is made higher-priority.GraphicRaycasterhit.The test file also adds helper raycasters:
AlwaysHitRaycaster : BaseRaycasterAlwaysHitGraphicRaycaster : GraphicRaycasterHighPriorityPhysics2DRaycaster : Physics2DRaycasterDependencies
com.unity.modules.physics2dtoPackages/manifest.jsonand lockfile to ensure PlayMode tests can usePhysics2DRaycaster.Verification
SimulateMouseUiTests:22/22passed.60/61passed (one unrelated existing failure on main).Class Diagram