From e25bcf989a3bf1355857674e477ab1dcf6f602f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Mon, 18 May 2026 14:04:53 -0400 Subject: [PATCH] =?UTF-8?q?fix(studio):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20split=20pan=20clamp,=20pin=20invariant,=20drop=20de?= =?UTF-8?q?ad=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- .../src/components/nle/NLEPreview.test.ts | 2 +- .../studio/src/components/nle/NLEPreview.tsx | 9 +- .../src/components/nle/previewZoom.test.ts | 87 +++++++++++++++---- .../studio/src/components/nle/previewZoom.ts | 40 ++++++--- 4 files changed, 110 insertions(+), 28 deletions(-) diff --git a/packages/studio/src/components/nle/NLEPreview.test.ts b/packages/studio/src/components/nle/NLEPreview.test.ts index e00eecf22..260906579 100644 --- a/packages/studio/src/components/nle/NLEPreview.test.ts +++ b/packages/studio/src/components/nle/NLEPreview.test.ts @@ -181,7 +181,7 @@ describe("NLEPreview", () => { ); }); - expect(view.stage.style.transform).toContain("translate3d(56px, 40px, 0)"); + expect(view.stage.style.transform).toContain("translate3d(48px, 40px, 0)"); view.cleanup(); }); diff --git a/packages/studio/src/components/nle/NLEPreview.tsx b/packages/studio/src/components/nle/NLEPreview.tsx index 6dac90100..8b51d8852 100644 --- a/packages/studio/src/components/nle/NLEPreview.tsx +++ b/packages/studio/src/components/nle/NLEPreview.tsx @@ -173,7 +173,13 @@ export const NLEPreview = memo(function NLEPreview({ zoomingRef.current = false; const final = zoomRef.current; writeStudioUiPreferences({ previewZoom: final }); - setSettledZoom(final); + setSettledZoom((prev) => + prev.zoomPercent === final.zoomPercent && + prev.panX === final.panX && + prev.panY === final.panY + ? prev + : final, + ); if (showHud) { const hud = hudRef.current; if (hud) { @@ -401,6 +407,7 @@ export const NLEPreview = memo(function NLEPreview({ width: `${stageSize.width}px`, height: `${stageSize.height}px`, transform: `translate3d(${toDomPrecision(initial.panX)}px, ${toDomPrecision(initial.panY)}px, 0) scale(${toDomPrecision(initial.zoomPercent / 100)})`, + // resolvePreviewWheelZoom cursor math assumes center-center pivot transformOrigin: "center center", }} data-testid="preview-zoom-stage" diff --git a/packages/studio/src/components/nle/previewZoom.test.ts b/packages/studio/src/components/nle/previewZoom.test.ts index 5aa303f3a..e03e8e9da 100644 --- a/packages/studio/src/components/nle/previewZoom.test.ts +++ b/packages/studio/src/components/nle/previewZoom.test.ts @@ -99,23 +99,21 @@ describe("clampPreviewPan", () => { }); }); - it("allows pan range for under-fitting and overflowing axes", () => { - const result = clampPreviewPan({ - panX: 120, - panY: -90, - zoomPercent: 107.25, - viewportWidth: 1352, - viewportHeight: 682, - contentWidth: 1184, - contentHeight: 666, + it("allows overscroll even when only one axis overflows", () => { + expect( + clampPreviewPan({ + panX: 120, + panY: -90, + zoomPercent: 107.25, + viewportWidth: 1352, + viewportHeight: 682, + contentWidth: 1184, + contentHeight: 666, + }), + ).toEqual({ + panX: PREVIEW_PAN_OVERSCROLL_PX, + panY: -(16.142499999999984 + PREVIEW_PAN_OVERSCROLL_PX), }); - - const scale = 1.0725; - const expectedMaxPanX = Math.abs(1184 * scale - 1352) / 2 + PREVIEW_PAN_OVERSCROLL_PX; - const expectedMaxPanY = Math.abs(666 * scale - 682) / 2 + PREVIEW_PAN_OVERSCROLL_PX; - - expect(result.panX).toBeCloseTo(expectedMaxPanX, 4); - expect(result.panY).toBeCloseTo(-expectedMaxPanY, 4); }); }); @@ -236,6 +234,63 @@ describe("resolvePreviewWheelZoom", () => { expect(next.panX).toBeCloseTo(50 * ratio, 1); expect(next.panY).toBeCloseTo(30 * ratio, 1); }); + + it("keeps the content point under a non-center cursor fixed after zoom", () => { + const cursorX = 150; + const cursorY = -80; + const state: PreviewZoomState = { zoomPercent: 150, panX: 20, panY: -10 }; + const oldScale = state.zoomPercent / 100; + + const next = resolvePreviewWheelZoom({ + state, + deltaY: -5, + viewportWidth: 800, + viewportHeight: 600, + contentWidth: 800, + contentHeight: 450, + cursorX, + cursorY, + }); + + const newScale = next.zoomPercent / 100; + const contentXBefore = (cursorX - state.panX) / oldScale; + const contentXAfter = (cursorX - next.panX) / newScale; + const contentYBefore = (cursorY - state.panY) / oldScale; + const contentYAfter = (cursorY - next.panY) / newScale; + + expect(contentXAfter).toBeCloseTo(contentXBefore, 6); + expect(contentYAfter).toBeCloseTo(contentYBefore, 6); + }); + + it("uses wider pan range for cursor zoom than manual drag", () => { + let state: PreviewZoomState = { zoomPercent: 100, panX: 0, panY: 0 }; + for (let i = 0; i < 40; i++) { + state = resolvePreviewWheelZoom({ + state, + deltaY: 5, + viewportWidth: 800, + viewportHeight: 600, + contentWidth: 800, + contentHeight: 450, + cursorX: -300, + cursorY: 0, + }); + } + + expect(state.zoomPercent).toBeLessThan(100); + + const dragClamped = clampPreviewPan({ + panX: state.panX, + panY: state.panY, + zoomPercent: state.zoomPercent, + viewportWidth: 800, + viewportHeight: 600, + contentWidth: 800, + contentHeight: 450, + }); + + expect(Math.abs(state.panX)).toBeGreaterThan(Math.abs(dragClamped.panX)); + }); }); describe("resolvePreviewWheelPan", () => { diff --git a/packages/studio/src/components/nle/previewZoom.ts b/packages/studio/src/components/nle/previewZoom.ts index def64848e..be67db21b 100644 --- a/packages/studio/src/components/nle/previewZoom.ts +++ b/packages/studio/src/components/nle/previewZoom.ts @@ -69,15 +69,33 @@ export function clampPreviewPan(input: { const contentWidth = input.contentWidth ?? input.viewportWidth; const contentHeight = input.contentHeight ?? input.viewportHeight; const maxPanX = - Math.abs(contentWidth * scale - input.viewportWidth) / 2 + PREVIEW_PAN_OVERSCROLL_PX; + Math.max(0, (contentWidth * scale - input.viewportWidth) / 2) + PREVIEW_PAN_OVERSCROLL_PX; const maxPanY = - Math.abs(contentHeight * scale - input.viewportHeight) / 2 + PREVIEW_PAN_OVERSCROLL_PX; + Math.max(0, (contentHeight * scale - input.viewportHeight) / 2) + PREVIEW_PAN_OVERSCROLL_PX; return { panX: Math.min(maxPanX, Math.max(-maxPanX, input.panX)), panY: Math.min(maxPanY, Math.max(-maxPanY, input.panY)), }; } +function clampPreviewPanForZoom( + panX: number, + panY: number, + zoomPercent: number, + viewportWidth: number, + viewportHeight: number, + contentWidth: number, + contentHeight: number, +): Pick { + const scale = clampPreviewZoomPercent(zoomPercent) / 100; + const maxPanX = Math.abs(contentWidth * scale - viewportWidth) / 2 + PREVIEW_PAN_OVERSCROLL_PX; + const maxPanY = Math.abs(contentHeight * scale - viewportHeight) / 2 + PREVIEW_PAN_OVERSCROLL_PX; + return { + panX: Math.min(maxPanX, Math.max(-maxPanX, panX)), + panY: Math.min(maxPanY, Math.max(-maxPanY, panY)), + }; +} + export function resolvePreviewWheelZoom(input: { state: PreviewZoomState; deltaY: number; @@ -96,21 +114,23 @@ export function resolvePreviewWheelZoom(input: { let panX = input.state.panX; let panY = input.state.panY; - if (input.cursorX !== undefined && input.cursorY !== undefined && Math.abs(oldScale) > 1e-6) { + if (input.cursorX !== undefined && input.cursorY !== undefined) { const ratio = newScale / oldScale; panX = input.cursorX * (1 - ratio) + panX * ratio; panY = input.cursorY * (1 - ratio) + panY * ratio; } - const pan = clampPreviewPan({ + const cw = input.contentWidth ?? input.viewportWidth; + const ch = input.contentHeight ?? input.viewportHeight; + const pan = clampPreviewPanForZoom( panX, panY, - zoomPercent: nextZoomPercent, - viewportWidth: input.viewportWidth, - viewportHeight: input.viewportHeight, - contentWidth: input.contentWidth, - contentHeight: input.contentHeight, - }); + nextZoomPercent, + input.viewportWidth, + input.viewportHeight, + cw, + ch, + ); return { zoomPercent: nextZoomPercent,