Skip to content

Commit e25bcf9

Browse files
committed
fix(studio): address PR review — split pan clamp, pin invariant, drop dead guard
- Split pan clamping: clampPreviewPan (drag/wheel-pan) stays narrow (Math.max(0,...) — content pins to center when smaller than viewport). New clampPreviewPanForZoom (Math.abs) gives the wide range only to cursor-anchored zoom, preventing middle-mouse drag from pushing content off-screen at low zoom levels. - Pin transform-origin invariant: comment on the stage div noting that resolvePreviewWheelZoom cursor math depends on center-center pivot. New test verifies a non-center cursor keeps the same content-space point fixed across a zoom step. - Remove dead Math.abs(oldScale) > 1e-6 guard — oldScale >= 0.25 always (clampPreviewZoomPercent floors at MIN_PREVIEW_ZOOM_PERCENT = 25). - Skip setSettledZoom re-render when the value didn't change — uses a functional updater that returns the previous state object when all three fields match, avoiding a React re-render cascade through Player.
1 parent 0150a0a commit e25bcf9

4 files changed

Lines changed: 110 additions & 28 deletions

File tree

packages/studio/src/components/nle/NLEPreview.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ describe("NLEPreview", () => {
181181
);
182182
});
183183

184-
expect(view.stage.style.transform).toContain("translate3d(56px, 40px, 0)");
184+
expect(view.stage.style.transform).toContain("translate3d(48px, 40px, 0)");
185185
view.cleanup();
186186
});
187187

packages/studio/src/components/nle/NLEPreview.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,13 @@ export const NLEPreview = memo(function NLEPreview({
173173
zoomingRef.current = false;
174174
const final = zoomRef.current;
175175
writeStudioUiPreferences({ previewZoom: final });
176-
setSettledZoom(final);
176+
setSettledZoom((prev) =>
177+
prev.zoomPercent === final.zoomPercent &&
178+
prev.panX === final.panX &&
179+
prev.panY === final.panY
180+
? prev
181+
: final,
182+
);
177183
if (showHud) {
178184
const hud = hudRef.current;
179185
if (hud) {
@@ -401,6 +407,7 @@ export const NLEPreview = memo(function NLEPreview({
401407
width: `${stageSize.width}px`,
402408
height: `${stageSize.height}px`,
403409
transform: `translate3d(${toDomPrecision(initial.panX)}px, ${toDomPrecision(initial.panY)}px, 0) scale(${toDomPrecision(initial.zoomPercent / 100)})`,
410+
// resolvePreviewWheelZoom cursor math assumes center-center pivot
404411
transformOrigin: "center center",
405412
}}
406413
data-testid="preview-zoom-stage"

packages/studio/src/components/nle/previewZoom.test.ts

Lines changed: 71 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -99,23 +99,21 @@ describe("clampPreviewPan", () => {
9999
});
100100
});
101101

102-
it("allows pan range for under-fitting and overflowing axes", () => {
103-
const result = clampPreviewPan({
104-
panX: 120,
105-
panY: -90,
106-
zoomPercent: 107.25,
107-
viewportWidth: 1352,
108-
viewportHeight: 682,
109-
contentWidth: 1184,
110-
contentHeight: 666,
102+
it("allows overscroll even when only one axis overflows", () => {
103+
expect(
104+
clampPreviewPan({
105+
panX: 120,
106+
panY: -90,
107+
zoomPercent: 107.25,
108+
viewportWidth: 1352,
109+
viewportHeight: 682,
110+
contentWidth: 1184,
111+
contentHeight: 666,
112+
}),
113+
).toEqual({
114+
panX: PREVIEW_PAN_OVERSCROLL_PX,
115+
panY: -(16.142499999999984 + PREVIEW_PAN_OVERSCROLL_PX),
111116
});
112-
113-
const scale = 1.0725;
114-
const expectedMaxPanX = Math.abs(1184 * scale - 1352) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
115-
const expectedMaxPanY = Math.abs(666 * scale - 682) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
116-
117-
expect(result.panX).toBeCloseTo(expectedMaxPanX, 4);
118-
expect(result.panY).toBeCloseTo(-expectedMaxPanY, 4);
119117
});
120118
});
121119

@@ -236,6 +234,63 @@ describe("resolvePreviewWheelZoom", () => {
236234
expect(next.panX).toBeCloseTo(50 * ratio, 1);
237235
expect(next.panY).toBeCloseTo(30 * ratio, 1);
238236
});
237+
238+
it("keeps the content point under a non-center cursor fixed after zoom", () => {
239+
const cursorX = 150;
240+
const cursorY = -80;
241+
const state: PreviewZoomState = { zoomPercent: 150, panX: 20, panY: -10 };
242+
const oldScale = state.zoomPercent / 100;
243+
244+
const next = resolvePreviewWheelZoom({
245+
state,
246+
deltaY: -5,
247+
viewportWidth: 800,
248+
viewportHeight: 600,
249+
contentWidth: 800,
250+
contentHeight: 450,
251+
cursorX,
252+
cursorY,
253+
});
254+
255+
const newScale = next.zoomPercent / 100;
256+
const contentXBefore = (cursorX - state.panX) / oldScale;
257+
const contentXAfter = (cursorX - next.panX) / newScale;
258+
const contentYBefore = (cursorY - state.panY) / oldScale;
259+
const contentYAfter = (cursorY - next.panY) / newScale;
260+
261+
expect(contentXAfter).toBeCloseTo(contentXBefore, 6);
262+
expect(contentYAfter).toBeCloseTo(contentYBefore, 6);
263+
});
264+
265+
it("uses wider pan range for cursor zoom than manual drag", () => {
266+
let state: PreviewZoomState = { zoomPercent: 100, panX: 0, panY: 0 };
267+
for (let i = 0; i < 40; i++) {
268+
state = resolvePreviewWheelZoom({
269+
state,
270+
deltaY: 5,
271+
viewportWidth: 800,
272+
viewportHeight: 600,
273+
contentWidth: 800,
274+
contentHeight: 450,
275+
cursorX: -300,
276+
cursorY: 0,
277+
});
278+
}
279+
280+
expect(state.zoomPercent).toBeLessThan(100);
281+
282+
const dragClamped = clampPreviewPan({
283+
panX: state.panX,
284+
panY: state.panY,
285+
zoomPercent: state.zoomPercent,
286+
viewportWidth: 800,
287+
viewportHeight: 600,
288+
contentWidth: 800,
289+
contentHeight: 450,
290+
});
291+
292+
expect(Math.abs(state.panX)).toBeGreaterThan(Math.abs(dragClamped.panX));
293+
});
239294
});
240295

241296
describe("resolvePreviewWheelPan", () => {

packages/studio/src/components/nle/previewZoom.ts

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,33 @@ export function clampPreviewPan(input: {
6969
const contentWidth = input.contentWidth ?? input.viewportWidth;
7070
const contentHeight = input.contentHeight ?? input.viewportHeight;
7171
const maxPanX =
72-
Math.abs(contentWidth * scale - input.viewportWidth) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
72+
Math.max(0, (contentWidth * scale - input.viewportWidth) / 2) + PREVIEW_PAN_OVERSCROLL_PX;
7373
const maxPanY =
74-
Math.abs(contentHeight * scale - input.viewportHeight) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
74+
Math.max(0, (contentHeight * scale - input.viewportHeight) / 2) + PREVIEW_PAN_OVERSCROLL_PX;
7575
return {
7676
panX: Math.min(maxPanX, Math.max(-maxPanX, input.panX)),
7777
panY: Math.min(maxPanY, Math.max(-maxPanY, input.panY)),
7878
};
7979
}
8080

81+
function clampPreviewPanForZoom(
82+
panX: number,
83+
panY: number,
84+
zoomPercent: number,
85+
viewportWidth: number,
86+
viewportHeight: number,
87+
contentWidth: number,
88+
contentHeight: number,
89+
): Pick<PreviewZoomState, "panX" | "panY"> {
90+
const scale = clampPreviewZoomPercent(zoomPercent) / 100;
91+
const maxPanX = Math.abs(contentWidth * scale - viewportWidth) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
92+
const maxPanY = Math.abs(contentHeight * scale - viewportHeight) / 2 + PREVIEW_PAN_OVERSCROLL_PX;
93+
return {
94+
panX: Math.min(maxPanX, Math.max(-maxPanX, panX)),
95+
panY: Math.min(maxPanY, Math.max(-maxPanY, panY)),
96+
};
97+
}
98+
8199
export function resolvePreviewWheelZoom(input: {
82100
state: PreviewZoomState;
83101
deltaY: number;
@@ -96,21 +114,23 @@ export function resolvePreviewWheelZoom(input: {
96114
let panX = input.state.panX;
97115
let panY = input.state.panY;
98116

99-
if (input.cursorX !== undefined && input.cursorY !== undefined && Math.abs(oldScale) > 1e-6) {
117+
if (input.cursorX !== undefined && input.cursorY !== undefined) {
100118
const ratio = newScale / oldScale;
101119
panX = input.cursorX * (1 - ratio) + panX * ratio;
102120
panY = input.cursorY * (1 - ratio) + panY * ratio;
103121
}
104122

105-
const pan = clampPreviewPan({
123+
const cw = input.contentWidth ?? input.viewportWidth;
124+
const ch = input.contentHeight ?? input.viewportHeight;
125+
const pan = clampPreviewPanForZoom(
106126
panX,
107127
panY,
108-
zoomPercent: nextZoomPercent,
109-
viewportWidth: input.viewportWidth,
110-
viewportHeight: input.viewportHeight,
111-
contentWidth: input.contentWidth,
112-
contentHeight: input.contentHeight,
113-
});
128+
nextZoomPercent,
129+
input.viewportWidth,
130+
input.viewportHeight,
131+
cw,
132+
ch,
133+
);
114134

115135
return {
116136
zoomPercent: nextZoomPercent,

0 commit comments

Comments
 (0)