From 9e6e148ba24dd96b9db5ef948c26539f047e0fba Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 17 Aug 2026 13:42:37 +0200 Subject: [PATCH] fix(app): stabilize canvas refresh and borders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fast Refresh preserves refs while rerunning effects, so canceled paint requests must release their retained IDs or unseen diff regions stop rendering. Paint the final expanded body’s reserved boundary pixel because no following header owns that seam. --- packages/app/src/git/diff-document/model.ts | 12 +++-- .../app/src/git/diff-document/paint.native.ts | 14 +++++- .../src/git/diff-document/paint.web.test.ts | 48 +++++++++++++++++++ .../app/src/git/diff-document/paint.web.ts | 18 ++++++- .../app/src/git/diff-document/surface.web.tsx | 12 ++--- 5 files changed, 90 insertions(+), 14 deletions(-) diff --git a/packages/app/src/git/diff-document/model.ts b/packages/app/src/git/diff-document/model.ts index 106ca9e21..75cee369e 100644 --- a/packages/app/src/git/diff-document/model.ts +++ b/packages/app/src/git/diff-document/model.ts @@ -24,7 +24,7 @@ import type { } from "./types"; export const FILE_HEADER_HEIGHT = 30; -const BODY_BORDER_HEIGHT = 1; +export const DIFF_BODY_BORDER_HEIGHT = 1; const CODE_HORIZONTAL_PADDING = 16; interface CellSource { @@ -84,7 +84,7 @@ export function buildDiffDocumentModel(input: BuildDiffDocumentModelInput): Diff documentTop += reusedRow.height; } maximumHorizontalOverflow = Math.max(0, reusableFile.contentWidth - input.viewportWidth); - documentTop += BODY_BORDER_HEIGHT; + documentTop += DIFF_BODY_BORDER_HEIGHT; } else if (!isCollapsed) { if (file.status === "binary" || file.status === "too_large") { const height = input.typography.lineHeight + 24; @@ -139,7 +139,7 @@ export function buildDiffDocumentModel(input: BuildDiffDocumentModelInput): Diff documentTop += height; } } - documentTop += BODY_BORDER_HEIGHT; + documentTop += DIFF_BODY_BORDER_HEIGHT; } files.push({ @@ -172,6 +172,12 @@ export function buildDiffDocumentModel(input: BuildDiffDocumentModelInput): Diff }; } +export function finalExpandedBodyBorderTop(model: DiffDocumentModel): number | null { + "worklet"; + const file = model.files[model.files.length - 1]; + return file && !file.isCollapsed ? file.bottom - DIFF_BODY_BORDER_HEIGHT : null; +} + export function retainReusableModels( previous: readonly DiffDocumentModel[] | undefined, next: DiffDocumentModel, diff --git a/packages/app/src/git/diff-document/paint.native.ts b/packages/app/src/git/diff-document/paint.native.ts index fb1116041..d80902925 100644 --- a/packages/app/src/git/diff-document/paint.native.ts +++ b/packages/app/src/git/diff-document/paint.native.ts @@ -5,7 +5,7 @@ import { type SkPaint, type SkPicture, } from "@shopify/react-native-skia"; -import { visibleRowRange } from "./model"; +import { DIFF_BODY_BORDER_HEIGHT, finalExpandedBodyBorderTop, visibleRowRange } from "./model"; import { nativeTextRuns } from "./native-text-runs"; import { horizontalOffsetForPath, type DiffHorizontalOffsets } from "./horizontal-offsets"; import { codeLineNumberTone } from "./palette"; @@ -257,6 +257,18 @@ function paintNativeRange(input: PaintNativeRangeInput): void { } }); } + const finalBorderTop = paintsFixedContent ? finalExpandedBodyBorderTop(input.model) : null; + if (finalBorderTop !== null) { + input.canvas.drawRect( + Skia.XYWHRect( + 0, + finalBorderTop - input.scrollTop, + input.viewportWidth, + DIFF_BODY_BORDER_HEIGHT, + ), + input.paints.border, + ); + } } function paintsNativeContentCell( diff --git a/packages/app/src/git/diff-document/paint.web.test.ts b/packages/app/src/git/diff-document/paint.web.test.ts index aac7837ec..33c81833a 100644 --- a/packages/app/src/git/diff-document/paint.web.test.ts +++ b/packages/app/src/git/diff-document/paint.web.test.ts @@ -84,6 +84,54 @@ describe("web diff text shaping", () => { expect(fills).toContainEqual({ color: "border", x: 20, y: 0, width: 1, height: 58 }); }); + it("paints the final expanded file body's bottom border", () => { + const fills: Array<{ color: string; x: number; y: number; width: number; height: number }> = []; + let fillStyle = ""; + const context = { + setTransform() {}, + clearRect() {}, + fillRect(x: number, y: number, width: number, height: number) { + fills.push({ color: fillStyle, x, y, width, height }); + }, + save() {}, + restore() {}, + beginPath() {}, + rect() {}, + clip() {}, + fillText() {}, + get fillStyle() { + return fillStyle; + }, + set fillStyle(value: string | CanvasGradient | CanvasPattern) { + fillStyle = String(value); + }, + globalAlpha: 1, + font: "", + textBaseline: "alphabetic", + } as unknown as CanvasRenderingContext2D; + const borderedModel: DiffDocumentModel = { + ...model, + height: 19, + files: [{ ...model.files[0]!, bodyHeight: 19, bottom: 19 }], + }; + + paintWebViewport({ + context, + model: borderedModel, + palette, + typography: { family: "monospace", size: 12, lineHeight: 18 }, + measureText: { measure: () => 0 }, + scrollTop: 0, + viewportWidth: 200, + viewportHeight: 100, + horizontalOffsets: new Map(), + selection: null, + devicePixelRatio: 1, + }); + + expect(fills).toContainEqual({ color: "border", x: 0, y: 18, width: 200, height: 1 }); + }); + it("clips a horizontally scrolled unified selection to the code viewport", () => { const selectionModel = createSelectionModel("unified"); const paints = paintSelection(selectionModel, selectionForCell(0)); diff --git a/packages/app/src/git/diff-document/paint.web.ts b/packages/app/src/git/diff-document/paint.web.ts index 09610ddf3..776fd8aee 100644 --- a/packages/app/src/git/diff-document/paint.web.ts +++ b/packages/app/src/git/diff-document/paint.web.ts @@ -1,5 +1,10 @@ import { selectionRectangles } from "./hit-testing"; -import { fragmentWidthForRange, visibleRowRange } from "./model"; +import { + DIFF_BODY_BORDER_HEIGHT, + finalExpandedBodyBorderTop, + fragmentWidthForRange, + visibleRowRange, +} from "./model"; import { codeLineNumberTone, codeTextColor } from "./palette"; import { reviewBackgroundPaint, reviewDividerHeight, reviewGapTop } from "./review-paint"; import type { @@ -64,6 +69,17 @@ export function paintWebViewport(input: PaintWebViewportInput): void { }); } + const finalBorderTop = finalExpandedBodyBorderTop(input.model); + if (finalBorderTop !== null) { + context.fillStyle = input.palette.border; + context.fillRect( + 0, + finalBorderTop - input.scrollTop, + input.viewportWidth, + DIFF_BODY_BORDER_HEIGHT, + ); + } + if (input.selection) paintSelection(input, input.selection); context.restore(); } diff --git a/packages/app/src/git/diff-document/surface.web.tsx b/packages/app/src/git/diff-document/surface.web.tsx index 60a160fab..ae1ad9e51 100644 --- a/packages/app/src/git/diff-document/surface.web.tsx +++ b/packages/app/src/git/diff-document/surface.web.tsx @@ -209,15 +209,7 @@ export function DiffSurface(props: DiffSurfaceProps) { }, [loadedTypography, measurement, props.palette, viewport.height, viewport.width]); const schedulePaint = useCallback( (force = true) => { - if (force) { - forcePaintRef.current = true; - // Fast Refresh preserves refs. A request owned by the previous module can leave a stale - // id behind, which would otherwise prevent every subsequent canvas repaint. - if (frameRef.current !== null) { - cancelAnimationFrame(frameRef.current); - frameRef.current = null; - } - } + if (force) forcePaintRef.current = true; if (frameRef.current === null) frameRef.current = requestAnimationFrame(paint); }, [paint], @@ -267,6 +259,8 @@ export function DiffSurface(props: DiffSurfaceProps) { useEffect( () => () => { if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + // Fast Refresh preserves refs while rerunning effects. Release ownership of the canceled + // request so the refreshed renderer can schedule its first paint. frameRef.current = null; }, [],