perf(android): Hit-test gestures without getLocationOnScreen#5595
Draft
runningcode wants to merge 2 commits into
Draft
perf(android): Hit-test gestures without getLocationOnScreen#5595runningcode wants to merge 2 commits into
runningcode wants to merge 2 commits into
Conversation
ViewUtils.findTarget called View.getLocationOnScreen for every visited view, and that walks from the view up to the root each time, making the traversal O(N*depth) per tap and scroll start. Instead, map the touch point down into each child's local coordinate space as we descend the tree — the same way ViewGroup dispatches touch events — so each view costs O(1) and the whole traversal is O(N). The locators still receive the original decor-view-relative coordinates, since the Compose locator hit-tests against window coordinates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
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
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.
📜 Description
ViewUtils.findTarget(run on every tap and at every scroll start) calledView.getLocationOnScreenfor every visited view to hit-test it.getLocationOnScreenwalks from the view up to the root each call, so hit-testing the whole tree was O(N·depth) per gesture, repeatedly re-walking the same ancestor chains.This replaces that with the technique
ViewGroupitself uses to dispatch touch events: map the touch point down into each child's local coordinate space as we descend (offset by the parent's scroll and the child'sleft/top, then apply the child's inverse matrix for transformed views). Each view is then a cheapO(1)bounds check against its own[0,0,width,height], making the whole traversal O(N).Notes:
x,y, becauseComposeGestureTargetLocatorhit-tests against window coordinates.The
sentry-composelocator gets the sameLinkedList→ArrayDequecleanup (it already hit-tests in window space, so no coordinate change there).💡 Motivation and Context
JAVA-534 / #5481. Follow-up to #5594 — that PR removed the queue-node allocations; this one removes the per-view
getLocationOnScreencall, replacing anO(N·depth)traversal withO(N).Tradeoff: carrying the per-view local coordinates through the BFS needs a small holder object per node, so this re-introduces a per-node allocation that #5594 removed. That is an intentional trade — avoiding the repeated
O(depth)ancestor walk per view in exchange for a short-lived gen-0 object.This is an algorithmic improvement, not a measured one — see testing notes below for why I could not get a reliable timing number on the available hardware.
💚 How did you test it?
ViewHelpers) to mock local geometry; all existingSentryGestureListenerClickTest/SentryGestureListenerScrollTestcases pass unchanged.ViewUtilsTestcoverage that exercises the actual transform — a child offset within its parent is hit when the point maps inside it and missed when it maps outside, even though the point is inside the decor view.Scroll target found: scrolling_container) and a click on a button near the bottom of the layout resolves correctly (view.id: scrolling_crash), confirming the transform is correct for a real non-trivial offset.Timing — not conclusively measured. I compared ART method-trace captures (
am profile, 30-tap burst) before and after. The only robust signal is thatgetLocationOnScreenis present in the call path onmainand absent after this change (replaced bymapToChild), confirming the per-view ancestor walk is eliminated. I could not measure afindTargetduration delta: instrumented method tracing produces empty traces on this device, fine-grained sampling overflows the buffer, and at the only working sampling rate (1 ms)findTargetis below the resolution — run-to-run variance (~0.2–1.6 ms/tap on the same build) dwarfs any real difference. So this PR is justified on algorithmic + call-path grounds, not on a measured ms improvement.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
None.