Repository navigation
Conversation
Until a frame is drawn, a display-sized view's canvas still holds attach()'s 640x480 placeholder, and layoutCanvasBox sized the CSS box from it. A press outside that box missed the canvas, and a press inside it was sent as a fraction of the placeholder rather than of the pane. Fill the display box until the first frame lands.
This branch has not been deployed
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.
What was observed
A resizable
YasSurfaceCanvasthat remounts, re-attaches or reconnects has a display size before it has drawn a frame. During that gap the first click often doesn't reach the remote app. We saw this in a browser-panel fuzz harness running against a real yas 0.5.3 server and Chrome. Instrumenting it showed the press does reachhandleMouse.drawnGeometry()returns a geometry and the press goes on the wire, but that geometry comes from the wrong box:attach()allocates a 640×480 placeholder backing buffer.layoutCanvasBoxtreats that placeholder as a frame and sizes the CSS box to 640×480 (divided by DPR).Root cause
Before its first frame, a display-sized view lays out and maps input against attach()'s 640×480 placeholder instead of its display box.
Fix
While no frame has been drawn (
_lastFrameSizeis null),layoutCanvasBoxfills the display box withobject-fit: fill.drawnGeometry()already reads that box and its fill mode, so a press anywhere in the pane reaches the canvas and maps against the pane. A press at the pane centre goes out at 0.5,0.5. When the first frame lands,presentFromStorerecords_lastFrameSizeand re-runs layout, so the existing frame-based geometry takes over.clearPresentation()resets_lastFrameSize, so a reused pane gets the same treatment.Unchanged:
The
drawnGeometryguardhandleMousereturns before any focus or modifier side effect whendrawnGeometry()is null. It also freezes the geometry used for both halves of a click. This protects a zero-size or remounting canvas from emitting half a click, and stops a later layout change from moving the press. The guard is untouched. A zero-size box still yields no geometry, and "does not focus or press through a zero-size canvas" still passes. The down and the up of a pre-frame press use the same display-box geometry.Tests
maps a press before the first frame against the display box. A display-sized 1200×700 view with no drawn frame must lay out its canvas at 1200×700 and send a centre press as down and up at 0.5,0.5. Onmainit fails withExpected "1200px", Received "640px".canvas.width/heightdirectly. That is indistinguishable from the placeholder, so they now use a smalldrawFramehelper that also records_lastFrameSize, aspresentFromStoredoes. Two tests encoded the old placeholder behaviour and were updated:keeps a smaller frame 1:1 at the top-leftnow draws an explicit 640×480 frame.positions a sized view absolutely…now expects the 800px display box.Commands run:
cd js && pnpm --filter @yas-run/core exec vitest run src/__tests__/YasSurfaceCanvas.test.ts: 270 passed. With only the test changes applied, the new repro fails.cd js && pnpm --filter @yas-run/core run test: 86 files, 1537 passed, 22 skipped.cd js && pnpm --filter @yas-run/core run typecheck: clean../bin/fmt --check: clean.End-to-end, I ran the fuzz harness with a scripted remount/reattach/drop sequence (12 steps) on a 1300×800 pane against a yas 0.5.3 server and Chrome, then repeated it with this branch's built
@yas-run/core.js/corediffers from v0.5.3 only inyas/fs.Known follow-ups (not in this PR)
YasNativeWorkspaceConnection.sendSurfacePointersilently drops input, both down and up, when the surface view isn't open yet.Tag
@indentto continue the conversation here.