fix(app): stabilize retained canvas diff rendering
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
||||
measureFragments,
|
||||
resolveScrollAnchor,
|
||||
resolveRelayoutScrollTop,
|
||||
shouldApplyRelayoutScroll,
|
||||
} from "./model";
|
||||
import type { BuildDiffDocumentModelInput, TextMeasurer } from "./types";
|
||||
|
||||
@@ -65,6 +66,12 @@ function input(overrides: Partial<BuildDiffDocumentModelInput> = {}): BuildDiffD
|
||||
}
|
||||
|
||||
describe("diff document model", () => {
|
||||
it("does not imperatively scroll for a no-op relayout", () => {
|
||||
expect(shouldApplyRelayoutScroll(320, 320)).toBe(false);
|
||||
expect(shouldApplyRelayoutScroll(320, 320.4)).toBe(false);
|
||||
expect(shouldApplyRelayoutScroll(320, 321)).toBe(true);
|
||||
});
|
||||
|
||||
it("segments graphemes when Intl.Segmenter is unavailable", () => {
|
||||
const originalSegmenter = Intl.Segmenter;
|
||||
Object.defineProperty(Intl, "Segmenter", { configurable: true, value: undefined });
|
||||
|
||||
@@ -172,10 +172,9 @@ export function buildDiffDocumentModel(input: BuildDiffDocumentModelInput): Diff
|
||||
};
|
||||
}
|
||||
|
||||
export function finalExpandedBodyBorderTop(model: DiffDocumentModel): number | null {
|
||||
export function expandedBodyBorderTop(file: DiffFileSection): number | null {
|
||||
"worklet";
|
||||
const file = model.files[model.files.length - 1];
|
||||
return file && !file.isCollapsed ? file.bottom - DIFF_BODY_BORDER_HEIGHT : null;
|
||||
return file.isCollapsed ? null : file.bottom - DIFF_BODY_BORDER_HEIGHT;
|
||||
}
|
||||
|
||||
export function retainReusableModels(
|
||||
@@ -579,6 +578,13 @@ export function resolveRelayoutScrollTop(
|
||||
return anchor ? resolveScrollAnchor(next, anchor) : scrollTop;
|
||||
}
|
||||
|
||||
export function shouldApplyRelayoutScroll(
|
||||
currentScrollTop: number,
|
||||
nextScrollTop: number,
|
||||
): boolean {
|
||||
return Math.abs(nextScrollTop - currentScrollTop) > 0.5;
|
||||
}
|
||||
|
||||
function sameSourceIdentity(left: DiffSourceIdentity, right: DiffSourceIdentity): boolean {
|
||||
return (
|
||||
left.hunkIndex === right.hunkIndex &&
|
||||
|
||||
@@ -78,4 +78,25 @@ describe("native canvas slabs", () => {
|
||||
expect(nativeCanvasWindowBucket(899, 600)).toBe(2);
|
||||
expect(nativeCanvasWindowTop(2, 600)).toBe(600);
|
||||
});
|
||||
|
||||
it("keeps slab identities stable when inline review geometry moves files", () => {
|
||||
const before = buildNativeCanvasSlabs(
|
||||
modelWithBodies([
|
||||
{ path: "first.ts", bodyTop: 30, bottom: 230 },
|
||||
{ path: "second.ts", bodyTop: 260, bottom: 3460 },
|
||||
]),
|
||||
600,
|
||||
);
|
||||
const after = buildNativeCanvasSlabs(
|
||||
modelWithBodies([
|
||||
{ path: "first.ts", bodyTop: 30, bottom: 378 },
|
||||
{ path: "second.ts", bodyTop: 408, bottom: 3608 },
|
||||
]),
|
||||
600,
|
||||
);
|
||||
|
||||
expect(after.filter((slab) => slab.path === "second.ts").map((slab) => slab.key)).toEqual(
|
||||
before.filter((slab) => slab.path === "second.ts").map((slab) => slab.key),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,10 +19,11 @@ export function buildNativeCanvasSlabs(
|
||||
const slabHeight = Math.max(768, viewportHeight * 2);
|
||||
return model.files.flatMap((file) => {
|
||||
const slabs: NativeCanvasSlab[] = [];
|
||||
for (let top = file.bodyTop; top < file.bottom; top += slabHeight) {
|
||||
let slabIndex = 0;
|
||||
for (let top = file.bodyTop; top < file.bottom; top += slabHeight, slabIndex += 1) {
|
||||
const height = Math.min(slabHeight, file.bottom - top);
|
||||
slabs.push({
|
||||
key: `${file.path}:${top}:${height}`,
|
||||
key: `${file.path}:${slabIndex}`,
|
||||
path: file.path,
|
||||
fileIndex: file.fileIndex,
|
||||
top,
|
||||
|
||||
@@ -5,7 +5,7 @@ import {
|
||||
type SkPaint,
|
||||
type SkPicture,
|
||||
} from "@shopify/react-native-skia";
|
||||
import { DIFF_BODY_BORDER_HEIGHT, finalExpandedBodyBorderTop, visibleRowRange } from "./model";
|
||||
import { DIFF_BODY_BORDER_HEIGHT, expandedBodyBorderTop, visibleRowRange } from "./model";
|
||||
import { nativeTextRuns } from "./native-text-runs";
|
||||
import { horizontalOffsetForPath, type DiffHorizontalOffsets } from "./horizontal-offsets";
|
||||
import { codeLineNumberTone } from "./palette";
|
||||
@@ -102,6 +102,7 @@ export function recordNativeSlabPictures(input: {
|
||||
textLayout: input.textLayout,
|
||||
paints: input.paints,
|
||||
layer: "fixed",
|
||||
fileIndex: input.fileIndex,
|
||||
});
|
||||
|
||||
const recordContent = (contentCell: "unified" | "left" | "right") => {
|
||||
@@ -193,6 +194,7 @@ interface PaintNativeRangeInput {
|
||||
paints: NativePaints;
|
||||
layer: "all" | "fixed" | "content";
|
||||
contentCell?: "unified" | "left" | "right";
|
||||
fileIndex?: number;
|
||||
}
|
||||
|
||||
function paintNativeRange(input: PaintNativeRangeInput): void {
|
||||
@@ -257,15 +259,20 @@ function paintNativeRange(input: PaintNativeRangeInput): void {
|
||||
}
|
||||
});
|
||||
}
|
||||
const finalBorderTop = paintsFixedContent ? finalExpandedBodyBorderTop(input.model) : null;
|
||||
if (finalBorderTop !== null) {
|
||||
let borderFiles = input.model.files;
|
||||
if (input.fileIndex !== undefined) {
|
||||
const file = input.model.files[input.fileIndex];
|
||||
borderFiles = file ? [file] : [];
|
||||
}
|
||||
for (const file of paintsFixedContent ? borderFiles : []) {
|
||||
const borderTop = expandedBodyBorderTop(file);
|
||||
const isVisible =
|
||||
borderTop !== null &&
|
||||
borderTop < input.scrollTop + input.viewportHeight &&
|
||||
borderTop + DIFF_BODY_BORDER_HEIGHT > input.scrollTop;
|
||||
if (!isVisible) continue;
|
||||
input.canvas.drawRect(
|
||||
Skia.XYWHRect(
|
||||
0,
|
||||
finalBorderTop - input.scrollTop,
|
||||
input.viewportWidth,
|
||||
DIFF_BODY_BORDER_HEIGHT,
|
||||
),
|
||||
Skia.XYWHRect(0, borderTop - input.scrollTop, input.viewportWidth, DIFF_BODY_BORDER_HEIGHT),
|
||||
input.paints.border,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -84,7 +84,7 @@ 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", () => {
|
||||
it("paints every expanded file body's bottom border", () => {
|
||||
const fills: Array<{ color: string; x: number; y: number; width: number; height: number }> = [];
|
||||
let fillStyle = "";
|
||||
const context = {
|
||||
@@ -111,8 +111,22 @@ describe("web diff text shaping", () => {
|
||||
} as unknown as CanvasRenderingContext2D;
|
||||
const borderedModel: DiffDocumentModel = {
|
||||
...model,
|
||||
height: 19,
|
||||
files: [{ ...model.files[0]!, bodyHeight: 19, bottom: 19 }],
|
||||
height: 38,
|
||||
files: [
|
||||
{ ...model.files[0]!, bodyHeight: 19, bottom: 19 },
|
||||
{
|
||||
...model.files[0]!,
|
||||
file: { ...model.files[0]!.file, path: "src/b.ts" },
|
||||
fileIndex: 1,
|
||||
path: "src/b.ts",
|
||||
top: 19,
|
||||
bodyTop: 19,
|
||||
bodyHeight: 19,
|
||||
bottom: 38,
|
||||
rowStart: 1,
|
||||
rowEnd: 1,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
paintWebViewport({
|
||||
@@ -129,7 +143,10 @@ describe("web diff text shaping", () => {
|
||||
devicePixelRatio: 1,
|
||||
});
|
||||
|
||||
expect(fills).toContainEqual({ color: "border", x: 0, y: 18, width: 200, height: 1 });
|
||||
expect(fills.filter((fill) => fill.color === "border" && fill.x === 0)).toEqual([
|
||||
{ color: "border", x: 0, y: 18, width: 200, height: 1 },
|
||||
{ color: "border", x: 0, y: 37, width: 200, height: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("clips a horizontally scrolled unified selection to the code viewport", () => {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { selectionRectangles } from "./hit-testing";
|
||||
import {
|
||||
DIFF_BODY_BORDER_HEIGHT,
|
||||
finalExpandedBodyBorderTop,
|
||||
expandedBodyBorderTop,
|
||||
fragmentWidthForRange,
|
||||
visibleRowRange,
|
||||
} from "./model";
|
||||
@@ -69,15 +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,
|
||||
);
|
||||
context.fillStyle = input.palette.border;
|
||||
const paintDocumentTop = input.scrollTop + paintTop;
|
||||
const paintDocumentBottom = paintDocumentTop + paintHeight;
|
||||
for (const file of input.model.files) {
|
||||
const borderTop = expandedBodyBorderTop(file);
|
||||
const isVisible =
|
||||
borderTop !== null &&
|
||||
borderTop < paintDocumentBottom &&
|
||||
borderTop + DIFF_BODY_BORDER_HEIGHT > paintDocumentTop;
|
||||
if (!isVisible) continue;
|
||||
context.fillRect(0, borderTop - input.scrollTop, input.viewportWidth, DIFF_BODY_BORDER_HEIGHT);
|
||||
}
|
||||
|
||||
if (input.selection) paintSelection(input, input.selection);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Canvas, Picture, type SkPicture } from "@shopify/react-native-skia";
|
||||
import { Canvas, Group, Picture, type SkPicture } from "@shopify/react-native-skia";
|
||||
import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import {
|
||||
@@ -9,26 +9,34 @@ import {
|
||||
type LayoutChangeEvent,
|
||||
type ViewStyle,
|
||||
} from "react-native";
|
||||
import {
|
||||
import Animated, {
|
||||
createAnimatedComponent,
|
||||
useAnimatedReaction,
|
||||
useAnimatedScrollHandler,
|
||||
useAnimatedStyle,
|
||||
useDerivedValue,
|
||||
useSharedValue,
|
||||
type SharedValue,
|
||||
} from "react-native-reanimated";
|
||||
import { scheduleOnRN } from "react-native-worklets";
|
||||
import { InlineReviewThread } from "@/review";
|
||||
import { useKeyboardShift } from "@/hooks/keyboard-shift-context";
|
||||
import { inlineUnistylesStyle } from "@/styles/unistyles-inline-style";
|
||||
import { DocumentFileHeader } from "./document-file-header";
|
||||
import { hitTestDiffBodyPoint } from "./native-hit-testing";
|
||||
import { retainDiffViewport } from "./viewport";
|
||||
import { HorizontalScroll } from "./horizontal-scroll.native";
|
||||
import {
|
||||
horizontalOffsetForPath,
|
||||
retainHorizontalOffsetsForPaths,
|
||||
type DiffHorizontalOffsets,
|
||||
} from "./horizontal-offsets";
|
||||
import { buildDiffDocumentModel, resolveRelayoutScrollTop, retainReusableModels } from "./model";
|
||||
import {
|
||||
buildDiffDocumentModel,
|
||||
resolveRelayoutScrollTop,
|
||||
retainReusableModels,
|
||||
shouldApplyRelayoutScroll,
|
||||
} from "./model";
|
||||
import {
|
||||
buildNativeCanvasSlabs,
|
||||
nativeCanvasSlabsForViewport,
|
||||
@@ -54,7 +62,6 @@ import type {
|
||||
} from "./types";
|
||||
|
||||
const AnimatedScrollView = createAnimatedComponent(ScrollView);
|
||||
const AnimatedCodeLayer = createAnimatedComponent(View);
|
||||
const SYSTEM_MONO = "monospace";
|
||||
const CODE_LEFT_PADDING = 8;
|
||||
|
||||
@@ -69,6 +76,11 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
} | null>(null);
|
||||
const consumedFocusRef = useRef<string | null>(null);
|
||||
const scrollTop = useSharedValue(0);
|
||||
const { shift: keyboardShift } = useKeyboardShift();
|
||||
const keyboardSpacerStyle = useAnimatedStyle(
|
||||
() => ({ height: keyboardShift.value }),
|
||||
[keyboardShift],
|
||||
);
|
||||
const horizontalOffsets = useSharedValue<DiffHorizontalOffsets>({});
|
||||
const family = props.displayPreferences.monoFontFamily.trim() || SYSTEM_MONO;
|
||||
const typography = useMemo<DiffTypography>(
|
||||
@@ -101,7 +113,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
);
|
||||
const reuseFrom = canReuse ? previous?.models : undefined;
|
||||
const next = buildDiffDocumentModel({
|
||||
files: props.files,
|
||||
files: viewport.width > 0 ? props.files : [],
|
||||
collapsedFilePaths: props.collapsedFilePaths,
|
||||
layout: props.displayPreferences.layout,
|
||||
wrapLines: props.displayPreferences.wrapLines,
|
||||
@@ -156,16 +168,16 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
|
||||
const layout = useCallback((event: LayoutChangeEvent) => {
|
||||
const { width, height } = event.nativeEvent.layout;
|
||||
setViewport((current) =>
|
||||
current.width === width && current.height === height ? current : { width, height },
|
||||
);
|
||||
setViewport((current) => retainDiffViewport(current, { width, height }));
|
||||
}, []);
|
||||
useLayoutEffect(() => {
|
||||
const previous = previousModelRef.current;
|
||||
if (previous && previous !== model) {
|
||||
const nextScrollTop = resolveRelayoutScrollTop(previous, model, scrollTop.value);
|
||||
scrollTop.value = nextScrollTop;
|
||||
scrollRef.current?.scrollTo({ y: nextScrollTop, animated: false });
|
||||
if (shouldApplyRelayoutScroll(scrollTop.value, nextScrollTop)) {
|
||||
scrollTop.value = nextScrollTop;
|
||||
scrollRef.current?.scrollTo({ y: nextScrollTop, animated: false });
|
||||
}
|
||||
}
|
||||
previousModelRef.current = model;
|
||||
}, [model, scrollTop]);
|
||||
@@ -201,7 +213,6 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
() => nativeStickyHeaderIndices(model.files.length),
|
||||
[model.files.length],
|
||||
);
|
||||
|
||||
return (
|
||||
<View style={[styles.root, { backgroundColor: props.palette.surface }]} onLayout={layout}>
|
||||
<AnimatedScrollView
|
||||
@@ -222,11 +233,10 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
paints={paints}
|
||||
horizontalOffsets={horizontalOffsets}
|
||||
/>
|
||||
{model.files.flatMap((file, index) => [
|
||||
{model.files.flatMap((file) => [
|
||||
<NativeStickyFileHeader
|
||||
key={`${file.path}:header`}
|
||||
file={file}
|
||||
showTopBorder={index > 0 && !model.files[index - 1]?.isCollapsed}
|
||||
selectedPath={props.selectedPath}
|
||||
mode={props.mode}
|
||||
onToggleFile={props.onToggleFile}
|
||||
@@ -241,6 +251,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
/>,
|
||||
])}
|
||||
<NativeReviewOverlays model={model} mode={props.mode} />
|
||||
<Animated.View pointerEvents="none" style={keyboardSpacerStyle} />
|
||||
</AnimatedScrollView>
|
||||
</View>
|
||||
);
|
||||
@@ -282,6 +293,7 @@ function NativeCanvasSlabLayer({
|
||||
() => nativeCanvasSlabsForViewport(descriptors, windowTop, viewportHeight),
|
||||
[descriptors, viewportHeight, windowTop],
|
||||
);
|
||||
if (model.viewportWidth <= 0 || viewportHeight <= 0) return null;
|
||||
return visible.map((slab) => (
|
||||
<NativeCanvasSlabView
|
||||
key={slab.key}
|
||||
@@ -296,14 +308,12 @@ function NativeCanvasSlabLayer({
|
||||
|
||||
function NativeStickyFileHeader({
|
||||
file,
|
||||
showTopBorder,
|
||||
selectedPath,
|
||||
mode,
|
||||
onToggleFile,
|
||||
onSelectPath,
|
||||
}: {
|
||||
file: DiffFileSection;
|
||||
showTopBorder: boolean;
|
||||
selectedPath: DiffSurfaceProps["selectedPath"];
|
||||
mode: DiffSurfaceProps["mode"];
|
||||
onToggleFile: DiffSurfaceProps["onToggleFile"];
|
||||
@@ -313,7 +323,6 @@ function NativeStickyFileHeader({
|
||||
<View style={styles.header}>
|
||||
<DocumentFileHeader
|
||||
file={file}
|
||||
showTopBorder={showTopBorder}
|
||||
selectedPath={selectedPath}
|
||||
mode={mode}
|
||||
onToggleFile={onToggleFile}
|
||||
@@ -491,7 +500,6 @@ function NativeCanvasSlabView({
|
||||
wrapLines={model.wrapLines}
|
||||
clipX={unifiedClip.x}
|
||||
clipWidth={unifiedClip.width}
|
||||
contentWidth={file.contentWidth}
|
||||
height={slab.height}
|
||||
/>
|
||||
{model.layout === "split" ? (
|
||||
@@ -503,7 +511,6 @@ function NativeCanvasSlabView({
|
||||
wrapLines={model.wrapLines}
|
||||
clipX={file.gutterWidth + CODE_LEFT_PADDING}
|
||||
clipWidth={splitClipWidth}
|
||||
contentWidth={file.contentWidth}
|
||||
height={slab.height}
|
||||
/>
|
||||
<NativeSlabCode
|
||||
@@ -513,7 +520,6 @@ function NativeCanvasSlabView({
|
||||
wrapLines={model.wrapLines}
|
||||
clipX={columnWidth + file.gutterWidth + CODE_LEFT_PADDING}
|
||||
clipWidth={splitClipWidth}
|
||||
contentWidth={file.contentWidth}
|
||||
height={slab.height}
|
||||
/>
|
||||
</>
|
||||
@@ -532,7 +538,6 @@ function NativeSlabCode({
|
||||
wrapLines,
|
||||
clipX,
|
||||
clipWidth,
|
||||
contentWidth,
|
||||
height,
|
||||
}: {
|
||||
picture: SkPicture;
|
||||
@@ -541,14 +546,13 @@ function NativeSlabCode({
|
||||
wrapLines: boolean;
|
||||
clipX: number;
|
||||
clipWidth: number;
|
||||
contentWidth: number;
|
||||
height: number;
|
||||
}) {
|
||||
const transform = useAnimatedStyle(() => ({
|
||||
transform: [
|
||||
{ translateX: wrapLines ? 0 : -horizontalOffsetForPath(horizontalOffsets.value, path) },
|
||||
],
|
||||
}));
|
||||
const pictureTransform = useDerivedValue(() => [
|
||||
{
|
||||
translateX: -clipX - (wrapLines ? 0 : horizontalOffsetForPath(horizontalOffsets.value, path)),
|
||||
},
|
||||
]);
|
||||
const clipStyle = useMemo<ViewStyle>(
|
||||
() => ({
|
||||
position: "absolute",
|
||||
@@ -560,17 +564,14 @@ function NativeSlabCode({
|
||||
}),
|
||||
[clipWidth, clipX, height],
|
||||
);
|
||||
const contentStyle = useMemo<ViewStyle>(
|
||||
() => ({ position: "absolute", top: 0, left: -clipX, width: contentWidth, height }),
|
||||
[clipX, contentWidth, height],
|
||||
);
|
||||
if (clipWidth <= 0 || height <= 0) return null;
|
||||
return (
|
||||
<View pointerEvents="none" style={clipStyle}>
|
||||
<AnimatedCodeLayer style={[contentStyle, transform]}>
|
||||
<Canvas style={StyleSheet.absoluteFill}>
|
||||
<Canvas pointerEvents="none" style={StyleSheet.absoluteFill}>
|
||||
<Group transform={pictureTransform}>
|
||||
<Picture picture={picture} />
|
||||
</Canvas>
|
||||
</AnimatedCodeLayer>
|
||||
</Group>
|
||||
</Canvas>
|
||||
</View>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
} from "./model";
|
||||
import { paintWebViewport } from "./paint.web";
|
||||
import { hasPointerDragStarted } from "./pointer-gesture";
|
||||
import { retainDiffViewport } from "./viewport";
|
||||
import type {
|
||||
DiffHit,
|
||||
DiffSelection,
|
||||
@@ -48,6 +49,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
startX: number;
|
||||
startY: number;
|
||||
moved: boolean;
|
||||
dismissSelectionOnClick: boolean;
|
||||
} | null>(null);
|
||||
const frameRef = useRef<number | null>(null);
|
||||
const resizeSettleTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
@@ -223,7 +225,8 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
useLayoutEffect(() => {
|
||||
const root = rootRef.current;
|
||||
if (!root) return;
|
||||
const commitPendingViewport = () => setViewport(pendingViewportRef.current);
|
||||
const commitPendingViewport = () =>
|
||||
setViewport((current) => retainDiffViewport(current, pendingViewportRef.current));
|
||||
const clearResizeSettleTimer = () => {
|
||||
if (resizeSettleTimerRef.current === null) return;
|
||||
clearTimeout(resizeSettleTimerRef.current);
|
||||
@@ -248,12 +251,12 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
if (!entry) return;
|
||||
const nextViewport = { width: entry.contentRect.width, height: entry.contentRect.height };
|
||||
pendingViewportRef.current = nextViewport;
|
||||
setViewport((currentViewport) =>
|
||||
currentViewport.width === 0 || currentViewport.height === 0
|
||||
? nextViewport
|
||||
: currentViewport,
|
||||
);
|
||||
if (resizePointerActiveRef.current) return;
|
||||
setViewport((currentViewport) =>
|
||||
currentViewport.width > 0 && currentViewport.height > 0
|
||||
? currentViewport
|
||||
: retainDiffViewport(currentViewport, nextViewport),
|
||||
);
|
||||
clearResizeSettleTimer();
|
||||
resizeSettleTimerRef.current = setTimeout(() => {
|
||||
resizeSettleTimerRef.current = null;
|
||||
@@ -403,11 +406,10 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
)
|
||||
)
|
||||
return;
|
||||
if (selectionRef.current) {
|
||||
const dismissSelectionOnClick = selectionRef.current !== null;
|
||||
if (dismissSelectionOnClick) {
|
||||
selectionRef.current = null;
|
||||
dragRef.current = null;
|
||||
schedulePaint();
|
||||
return;
|
||||
}
|
||||
const hit = pointHit(event);
|
||||
if (hit?.kind !== "cell") return;
|
||||
@@ -416,6 +418,7 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
startX: event.clientX,
|
||||
startY: event.clientY,
|
||||
moved: false,
|
||||
dismissSelectionOnClick,
|
||||
};
|
||||
selectionRef.current = { anchor: hit.position, focus: hit.position };
|
||||
event.currentTarget.focus();
|
||||
@@ -486,6 +489,11 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
alreadyDragging: drag.moved,
|
||||
})
|
||||
: false;
|
||||
if (drag && !moved && drag.dismissSelectionOnClick) {
|
||||
selectionRef.current = null;
|
||||
schedulePaint();
|
||||
return;
|
||||
}
|
||||
if (drag && !moved && hit?.kind === "cell" && hit.target && reviewActions) {
|
||||
selectionRef.current = null;
|
||||
reviewActions.onStartComment(hit.target);
|
||||
@@ -552,11 +560,10 @@ export function DiffSurface(props: DiffSurfaceProps) {
|
||||
>
|
||||
<div style={contentStyle} onMouseDown={preventDocumentMouseSelection}>
|
||||
<canvas ref={canvasRef} data-testid="git-diff-canvas" style={canvasStyle} />
|
||||
{model.files.map((file, index) => (
|
||||
{model.files.map((file) => (
|
||||
<WebFileHeaderSection key={file.path} file={file}>
|
||||
<DocumentFileHeader
|
||||
file={file}
|
||||
showTopBorder={index > 0 && !model.files[index - 1]?.isCollapsed}
|
||||
selectedPath={props.selectedPath}
|
||||
mode={props.mode}
|
||||
onToggleFile={props.onToggleFile}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
createCachedAsciiTextMetrics,
|
||||
createFallbackAwareTextMeasurer,
|
||||
requiresNativeParagraph,
|
||||
type PrimaryTextFace,
|
||||
@@ -40,6 +41,39 @@ describe("native paragraph retention", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("cached ASCII text metrics", () => {
|
||||
it("measures each distinct grapheme once across lines", () => {
|
||||
const measured: string[] = [];
|
||||
const metrics = createCachedAsciiTextMetrics({
|
||||
glyphIds: (text) => Array.from(text, () => 1),
|
||||
measure(text) {
|
||||
measured.push(text);
|
||||
return text.length * 10;
|
||||
},
|
||||
});
|
||||
|
||||
expect(metrics.measureAdvances(["a", "b", "a"])).toEqual([10, 20, 30]);
|
||||
expect(metrics.measureAdvances(["b", "a"])).toEqual([10, 20]);
|
||||
expect(measured).toEqual(["a", "b"]);
|
||||
});
|
||||
|
||||
it("caches glyph coverage per distinct ASCII character", () => {
|
||||
const checked: string[] = [];
|
||||
const metrics = createCachedAsciiTextMetrics({
|
||||
glyphIds(text) {
|
||||
checked.push(text);
|
||||
return [text === "?" ? 0 : 1];
|
||||
},
|
||||
measure: (text) => text.length * 10,
|
||||
});
|
||||
|
||||
expect(metrics.hasEveryGlyph("abba")).toBe(true);
|
||||
expect(metrics.hasEveryGlyph("bad?")).toBe(false);
|
||||
expect(metrics.hasEveryGlyph("cab")).toBe(true);
|
||||
expect(checked).toEqual(["a", "b", "d", "?", "c"]);
|
||||
});
|
||||
});
|
||||
|
||||
function primaryFaceWithout(...missing: string[]): PrimaryTextFace {
|
||||
return {
|
||||
glyphIds(text) {
|
||||
|
||||
@@ -5,12 +5,47 @@ export interface PrimaryTextFace {
|
||||
measure(text: string): number;
|
||||
}
|
||||
|
||||
export interface CachedAsciiTextMetrics {
|
||||
hasEveryGlyph(text: string): boolean;
|
||||
measureAdvances(graphemes: readonly string[]): number[];
|
||||
}
|
||||
|
||||
const CODE_LIGATURE_CANDIDATE = /(?:===?|!==?|=>|<=|>=|->|::|\+\+|--)/;
|
||||
|
||||
export function requiresNativeParagraph(text: string): boolean {
|
||||
return /[^\x20-\x7e\t]/u.test(text) || CODE_LIGATURE_CANDIDATE.test(text);
|
||||
}
|
||||
|
||||
export function createCachedAsciiTextMetrics(primary: PrimaryTextFace): CachedAsciiTextMetrics {
|
||||
const glyphCoverage = new Map<string, boolean>();
|
||||
const widths = new Map<string, number>();
|
||||
return {
|
||||
hasEveryGlyph(text) {
|
||||
for (const character of text) {
|
||||
let hasGlyph = glyphCoverage.get(character);
|
||||
if (hasGlyph === undefined) {
|
||||
hasGlyph = primary.glyphIds(character).every((glyph) => glyph !== 0);
|
||||
glyphCoverage.set(character, hasGlyph);
|
||||
}
|
||||
if (!hasGlyph) return false;
|
||||
}
|
||||
return true;
|
||||
},
|
||||
measureAdvances(graphemes) {
|
||||
let advance = 0;
|
||||
return graphemes.map((grapheme) => {
|
||||
let width = widths.get(grapheme);
|
||||
if (width === undefined) {
|
||||
width = primary.measure(grapheme);
|
||||
widths.set(grapheme, width);
|
||||
}
|
||||
advance += width;
|
||||
return advance;
|
||||
});
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function createFallbackAwareTextMeasurer(input: {
|
||||
primary: PrimaryTextFace;
|
||||
measureWithSystemFallback: (text: string) => number;
|
||||
|
||||
@@ -7,7 +7,12 @@ import {
|
||||
} from "@shopify/react-native-skia";
|
||||
import { fragmentTextForRange } from "./model";
|
||||
import { codeTextColor } from "./palette";
|
||||
import { createFallbackAwareTextMeasurer, requiresNativeParagraph } from "./text-measurement";
|
||||
import {
|
||||
createCachedAsciiTextMetrics,
|
||||
createFallbackAwareTextMeasurer,
|
||||
requiresNativeParagraph,
|
||||
type CachedAsciiTextMetrics,
|
||||
} from "./text-measurement";
|
||||
import type { DiffCell, DiffDocumentModel, DiffFragment, DiffPalette, TextMeasurer } from "./types";
|
||||
|
||||
const PARAGRAPH_WIDTH = 100_000;
|
||||
@@ -19,6 +24,7 @@ export interface NativeTextLayout {
|
||||
|
||||
export interface NativeTextLayoutStore {
|
||||
font: SkFont;
|
||||
asciiMetrics: CachedAsciiTextMetrics;
|
||||
paragraphsByCell: WeakMap<DiffCell, Array<SkParagraph | null>>;
|
||||
ownedParagraphs: Set<SkParagraph>;
|
||||
families: string[];
|
||||
@@ -43,6 +49,10 @@ export function createNativeTextLayoutStore(input: {
|
||||
const font = primaryFont(families, input.fontSize);
|
||||
return {
|
||||
font,
|
||||
asciiMetrics: createCachedAsciiTextMetrics({
|
||||
glyphIds: (text) => font.getGlyphIDs(text),
|
||||
measure: (text) => font.getTextWidth(text),
|
||||
}),
|
||||
paragraphsByCell: new WeakMap(),
|
||||
ownedParagraphs: new Set(),
|
||||
families,
|
||||
@@ -63,7 +73,7 @@ export function prepareNativeTextLayout(
|
||||
const cached = store.paragraphsByCell.get(cell);
|
||||
if (cached) return cached;
|
||||
const cellParagraphs = cell.fragments.map((fragment) => {
|
||||
if (!requiresRetainedParagraph(fragment.text, store.font)) return null;
|
||||
if (!requiresRetainedParagraph(fragment.text, store.asciiMetrics)) return null;
|
||||
const paragraph = createFragmentParagraph({
|
||||
cell,
|
||||
fragment,
|
||||
@@ -88,11 +98,13 @@ export function createNativeTextMeasurer(input: {
|
||||
}): TextMeasurer {
|
||||
const families = nativeFontFamilies(input.configuredFamily);
|
||||
const font = primaryFont(families, input.fontSize);
|
||||
const primary = {
|
||||
glyphIds: (text: string) => font.getGlyphIDs(text),
|
||||
measure: (text: string) => font.getTextWidth(text),
|
||||
};
|
||||
const asciiMetrics = createCachedAsciiTextMetrics(primary);
|
||||
const fallbackAware = createFallbackAwareTextMeasurer({
|
||||
primary: {
|
||||
glyphIds: (text) => font.getGlyphIDs(text),
|
||||
measure: (text) => font.getTextWidth(text),
|
||||
},
|
||||
primary,
|
||||
measureWithSystemFallback(text) {
|
||||
const paragraph = createParagraph({
|
||||
text,
|
||||
@@ -110,13 +122,8 @@ export function createNativeTextMeasurer(input: {
|
||||
...fallbackAware,
|
||||
measureAdvances(graphemes) {
|
||||
const text = graphemes.join("");
|
||||
if (!requiresRetainedParagraph(text, font)) {
|
||||
let prefix = "";
|
||||
return graphemes.map((grapheme) => {
|
||||
prefix += grapheme;
|
||||
return font.getTextWidth(prefix);
|
||||
});
|
||||
}
|
||||
if (!requiresRetainedParagraph(text, asciiMetrics))
|
||||
return asciiMetrics.measureAdvances(graphemes);
|
||||
const paragraph = createParagraph({
|
||||
text,
|
||||
families,
|
||||
@@ -139,8 +146,8 @@ export function createNativeTextMeasurer(input: {
|
||||
};
|
||||
}
|
||||
|
||||
function requiresRetainedParagraph(text: string, font: SkFont): boolean {
|
||||
return requiresNativeParagraph(text) || font.getGlyphIDs(text).some((glyph) => glyph === 0);
|
||||
function requiresRetainedParagraph(text: string, asciiMetrics: CachedAsciiTextMetrics): boolean {
|
||||
return requiresNativeParagraph(text) || !asciiMetrics.hasEveryGlyph(text);
|
||||
}
|
||||
|
||||
function createFragmentParagraph(input: {
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { retainDiffViewport } from "./viewport";
|
||||
|
||||
describe("retainDiffViewport", () => {
|
||||
it("waits for the first positive layout", () => {
|
||||
const initial = { width: 0, height: 0 };
|
||||
|
||||
expect(retainDiffViewport(initial, { width: 0, height: 700 })).toBe(initial);
|
||||
expect(retainDiffViewport(initial, { width: 390, height: 0 })).toBe(initial);
|
||||
expect(retainDiffViewport(initial, { width: 390, height: 700 })).toEqual({
|
||||
width: 390,
|
||||
height: 700,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves the completed viewport while a retained panel is hidden", () => {
|
||||
const visible = { width: 390, height: 700 };
|
||||
|
||||
expect(retainDiffViewport(visible, { width: 0, height: 0 })).toBe(visible);
|
||||
});
|
||||
|
||||
it("accepts a real resize and preserves identity for duplicate layouts", () => {
|
||||
const current = { width: 390, height: 700 };
|
||||
|
||||
expect(retainDiffViewport(current, current)).toBe(current);
|
||||
expect(retainDiffViewport(current, { width: 700, height: 390 })).toEqual({
|
||||
width: 700,
|
||||
height: 390,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,10 @@
|
||||
export interface DiffViewport {
|
||||
width: number;
|
||||
height: number;
|
||||
}
|
||||
|
||||
export function retainDiffViewport(current: DiffViewport, next: DiffViewport): DiffViewport {
|
||||
if (next.width <= 0 || next.height <= 0) return current;
|
||||
if (current.width === next.width && current.height === next.height) return current;
|
||||
return next;
|
||||
}
|
||||
Reference in New Issue
Block a user