fix(app): stabilize canvas refresh and borders
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.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
},
|
||||
[],
|
||||
|
||||
Reference in New Issue
Block a user