Skip to content

scroll: Let hosts ask whether a strip under a point can still scroll - #3797

Closed
yuanzhuang-svg wants to merge 1 commit into
longbridge:mainfrom
yuanzhuang-svg:feat/horizontal-strip-hit-test
Closed

yuanzhuang-svg wants to merge 1 commit into
longbridge:mainfrom
yuanzhuang-svg:feat/horizontal-strip-hit-test

Conversation

@yuanzhuang-svg

Copy link
Copy Markdown
Contributor

Description

Follow-up to #3621. Some hosts put their own pan gesture on top of GPUI content, for example a mobile app where swiping right opens a side drawer. With #3621 the host learns that a horizontal strip (a markdown table, …) owns a swipe only after the strip has moved. So the host has to hold its own gesture still for a short window to see whether the strip moves first. In our iOS app that pause makes the drawer feel like it needs a moment before it follows the finger.

With this PR the host can ask on touch down. Each horizontal ScrollableMask registers, as it paints, its visible bounds in window coordinates (clipped by the enclosing content mask) and whether it can still scroll right and left. horizontal_strip_can_scroll_at(window, point, direction) answers from the frame on screen. Root starts every render by forgetting the strips its window registered in the previous frame, so the list only holds what is on screen. Strips scrolled out of a list viewport no longer claim points.

The registry is thread-local and keyed by window. Scrolling behavior is unchanged, and last_horizontal_scroll_claim() stays as it is.

Public API

gpui-base (gpui_kit::base)

  • horizontal_strip_can_scroll_at(window: WindowId, point: Point<Pixels>, direction: i32) -> bool: whether a horizontal strip under point in window can still scroll its content in direction (> 0 right, < 0 left, 0 either way).

How to Test

cargo test -p gpui-base --features test-support --lib scrollable_mask

The new horizontal_strip_reports_room_to_scroll_at_a_point test scrolls a strip to the left edge, the middle and the right edge, and checks the room it reports each way. It also checks that a point outside the strip does not match and that a new frame forgets the old strips.

I also tested it on the iOS simulator. A right swipe now opens the drawer from the first move. Over a table that can still scroll, the table takes the swipe.

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
  • Passed cargo run for story tests related to the changes.
  • Tested macOS, Windows and Linux platforms performance (if the change is platform-specific)

🤖 Generated with Claude Code

Hosts with their own pan over GPUI content (a swipe that opens a side
drawer on mobile) had to wait for a strip to move before knowing it owned
the swipe. Horizontal masks now register their visible bounds and room
to scroll each frame, and horizontal_strip_can_scroll_at answers on touch
down. Root starts each frame by forgetting the window's previous strips.
@yuanzhuang-svg

Copy link
Copy Markdown
Contributor Author

@huacnlee Could you take a look when you have a moment? It's a follow-up to #3621: the iOS app's drawer swipe can now ask on touch down whether a table under the finger can still scroll, instead of waiting for the table to move first. CI is green.

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host gesture requirement is valid: the drawer should follow the first movement without waiting to discover whether a table scrolls. However, I would hold this public API in its current form.

Two correctness issues need addressing:

  1. Root::render clears all registered strips for the window, but GPUI can replay a cached child view without calling ScrollableMask::paint again. A visible scrollable strip then disappears from the registry. Dock panels already use cached views. Please cover a cached strip surviving an unrelated redraw.
  2. The query checks bounds and remaining scroll room without accounting for occlusion. A strip underneath a dialog or menu can return true even though the actual scroll handler rejects the event through its hitbox check. Please cover an occluded strip.

Before adding this public API, please investigate handling arbitration in the iOS host: synchronously dispatch the first real scroll movement to GPUI, check whether last_horizontal_scroll_claim() produced a new claim during that dispatch, and immediately use the same movement for the drawer when no strip moved. Lock ownership for the remainder of the gesture. This needs verification against the host event pipeline; it is a proposed approach, not a confirmed replacement.

If the host cannot implement that behavior with existing interfaces, please document the specific limitation so we can design the smallest reliable public contract. The current horizontal_strip terminology and magic i32 direction also need reconsideration before becoming public API.

Reviewed by Codex (AI agent).

@yuanzhuang-svg

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Closing this: the iOS host now arbitrates the drawer swipe as you suggested, with no new public API. When the drawer pan begins, the same movement is handed to GPUI first (gpui-mobile dispatches touchesMoved synchronously), and on the next main-queue turn the host checks the existing ai_chat_ios_horizontal_scroll_claimed_within (backed by last_horizontal_scroll_claim). A table that moved right, or a chart crosshair moving sideways, keeps the swipe; otherwise the drawer takes it immediately and follows the finger. The 120 ms wait is gone. The ai-chat side (#527) will be reverted separately.

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.

2 participants