Skip to content

perf(android): Hit-test gestures without getLocationOnScreen#5595

Draft
runningcode wants to merge 2 commits into
mainfrom
no/java-534-findtarget-point-transform
Draft

perf(android): Hit-test gestures without getLocationOnScreen#5595
runningcode wants to merge 2 commits into
mainfrom
no/java-534-findtarget-point-transform

Conversation

@runningcode

@runningcode runningcode commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

📜 Description

ViewUtils.findTarget (run on every tap and at every scroll start) called View.getLocationOnScreen for every visited view to hit-test it. getLocationOnScreen walks 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 ViewGroup itself 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's left/top, then apply the child's inverse matrix for transformed views). Each view is then a cheap O(1) bounds check against its own [0,0,width,height], making the whole traversal O(N).

Notes:

  • The locators still receive the original decor-view-relative x,y, because ComposeGestureTargetLocator hit-tests against window coordinates.
  • Because the traversal now stays in window/local space instead of mixing in screen space, hit-testing is also slightly more correct when the window isn't at the screen origin (status bar offset, split-screen).
  • Keeps the BFS order, so the selected target for overlapping clickable views is unchanged.

The sentry-compose locator gets the same LinkedListArrayDeque cleanup (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 getLocationOnScreen call, replacing an O(N·depth) traversal with O(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?

  • Rewrote the gesture test harness (ViewHelpers) to mock local geometry; all existing SentryGestureListenerClickTest / SentryGestureListenerScrollTest cases pass unchanged.
  • Added ViewUtilsTest coverage 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.
  • On-device (Pixel 3, Android 12): scroll target still resolves (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 that getLocationOnScreen is present in the call path on main and absent after this change (replaced by mapToChild), confirming the per-view ancestor walk is eliminated. I could not measure a findTarget duration 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) findTarget is 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

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

None.

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>
@linear-code

linear-code Bot commented Jun 22, 2026

Copy link
Copy Markdown

JAVA-534

@sentry

sentry Bot commented Jun 22, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.44.1 (1) release

⚙️ sentry-android Build Distribution Settings

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