From ac212f40241898f844326896d1aaca88d9efc272 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:28:17 +0000 Subject: [PATCH] Fix the preview canvas stretching the image on HiDPI screens A 4032x3024 photo came out visibly too wide - 85% too wide on a 1920x1080 screen at 2x, and wrong by some amount on every display with a pixel ratio above 1. The engine was never at fault: `ops::fit_within` returns an aspect-correct frame, and the preview it hands back has the right shape. The layer that got it wrong was the layout. `#preview-canvas` is a flex item of `.canvas-frame`, and its two axes were being sized by two mechanisms that never spoke to each other: the width by flex-shrink against `max-width: 100%`, the height by the default `align-items: stretch` against the frame's line. Nothing tied them to the canvas's own aspect ratio. That only stayed hidden at a pixel ratio of 1, where the backing store happens to equal the CSS box, so both axes land on the same scale by coincidence. Above 1 the backing store is measured in device pixels and the CSS box in CSS pixels, the coincidence breaks, and the picture stretches. So the canvas is now given an explicit display size instead: - `previewBox()` reports the box in both units - CSS pixels for layout, device pixels for the render request - rather than conflating them. It also measures with `getBoundingClientRect()`, since `clientHeight` rounds to whole pixels while the padding it subtracts does not, which could overstate the box by most of a pixel. - `layoutCanvas()` fits the frame that actually arrived into the box that actually exists and sets `style.width`/`style.height` from it. Fitting the received frame rather than the requested one is self-correcting: a resize mid-render costs a little sharpness, never a stretched image. - The window `resize` handler re-fits immediately, then re-renders on the existing debounce to recover the resolution the new box is worth. `.canvas-frame` loses its `max-width`/`max-height` so it wraps the canvas exactly. The crop overlay is stretched across that box with `inset: 0`, so slack between the two put the selection somewhere the image was not - the crop tool was misaligned by the same amount the image was stretched. Verified in Chromium across 70 viewport/pixel-ratio/image-size combinations: worst aspect-ratio error drops from 100.6% to 0.21% (integer rounding of the frame's device dimensions, sub-pixel on screen), with the overlay landing on the image to within 0.000px and no case overflowing the viewport. Also checked that dragging the window through a range of shapes keeps the image proportioned with no re-render in between. Engine suite still 38/38. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A8jTvVudtvCzWAqgG2MkJf --- src/editor.css | 20 ++++++++---- src/main.ts | 83 +++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/editor.css b/src/editor.css index 8c4ff53..2cfc6fc 100644 --- a/src/editor.css +++ b/src/editor.css @@ -200,20 +200,28 @@ overflow: hidden; } +/* Wraps the canvas exactly, with nothing here to size it independently: the + crop overlay is stretched across this box (inset: 0), so any slack between + the two would put the selection somewhere the image is not. `layoutCanvas()` + is what keeps the canvas inside the viewport; the viewport's own + `overflow: hidden` is the backstop if it ever fails to. */ .canvas-frame { position: relative; line-height: 0; - max-width: 100%; - max-height: 100%; display: flex; } +/* Sized from JS, in `layoutCanvas()`, because CSS on its own gets it wrong + here: the canvas's intrinsic size is its backing store, measured in *device* + pixels, so on a HiDPI screen `max-width: 100%` shrinks the width against a + box the height never sees while flex stretches the height to the line. The + two axes land on different scales and the image comes out stretched - a + 4032x3024 photo on a 1920x1080 screen at 2x was drawn 85% too wide. + `flex: 0 0 auto` keeps flex from having an opinion about either axis. */ #preview-canvas { display: block; - max-width: 100%; - max-height: 100%; - width: auto; - height: auto; + flex: 0 0 auto; + align-self: flex-start; border-radius: 4px; } diff --git a/src/main.ts b/src/main.ts index 536b809..15a7536 100644 --- a/src/main.ts +++ b/src/main.ts @@ -189,15 +189,79 @@ function displayPipeline(): Pipeline { return pipeline; } -/** How many device pixels the preview box can actually show. */ -function previewBox(): { width: number; height: number } { +/** + * The preview box, in both the units that matter. + * + * `css` is the space on screen the image has to fit into; `device` is how many + * real pixels that space is worth, which is what the engine renders. On a + * HiDPI screen the two differ by the pixel ratio, and conflating them is what + * makes a canvas come out stretched: the backing store is sized in device + * pixels but laid out in CSS ones. + */ +function previewBox(): { cssWidth: number; cssHeight: number; width: number; height: number } { + // `clientWidth`/`clientHeight` round to whole pixels while the padding + // does not, which can overstate the box by most of a pixel - enough for + // the canvas to poke out from under the crop overlay. The rect is exact. + const rect = ui.canvasViewport.getBoundingClientRect(); const style = getComputedStyle(ui.canvasViewport); - const padX = parseFloat(style.paddingLeft) + parseFloat(style.paddingRight); - const padY = parseFloat(style.paddingTop) + parseFloat(style.paddingBottom); + const padX = + parseFloat(style.paddingLeft) + + parseFloat(style.paddingRight) + + parseFloat(style.borderLeftWidth) + + parseFloat(style.borderRightWidth); + const padY = + parseFloat(style.paddingTop) + + parseFloat(style.paddingBottom) + + parseFloat(style.borderTopWidth) + + parseFloat(style.borderBottomWidth); + + const cssWidth = Math.max(0, rect.width - padX); + const cssHeight = Math.max(0, rect.height - padY); + + // The render resolution is a separate question: never ask the engine for + // a postage stamp, and never for more pixels than a screen can show. const dpr = Math.min(window.devicePixelRatio || 1, 2); - const width = Math.max(160, Math.round((ui.canvasViewport.clientWidth - padX) * dpr)); - const height = Math.max(160, Math.round((ui.canvasViewport.clientHeight - padY) * dpr)); - return { width: Math.min(width, 2400), height: Math.min(height, 2400) }; + return { + cssWidth, + cssHeight, + width: Math.min(Math.max(160, Math.round(cssWidth * dpr)), 2400), + height: Math.min(Math.max(160, Math.round(cssHeight * dpr)), 2400), + }; +} + +/** + * Scale `w` x `h` down until it fits inside the box, keeping the ratio and + * never enlarging. The mirror of `ops::fit_within` in the engine, so the + * layout maths and the render maths agree on what "fits" means. + */ +function fitWithin( + w: number, + h: number, + maxW: number, + maxH: number, +): { width: number; height: number } { + if (w <= 0 || h <= 0) return { width: 0, height: 0 }; + const scale = Math.min(maxW / w, maxH / h, 1); + return { width: w * scale, height: h * scale }; +} + +/** + * Give the canvas an explicit display size instead of leaving it to `max-width` + * and the flex algorithm, which size the two axes independently and so throw + * the aspect ratio away. Fitting the frame we actually received into the box + * we actually have is also self-correcting: a window resize between asking for + * a frame and drawing it costs a little sharpness, never a stretched image. + */ +function layoutCanvas(): void { + if (!ui.canvas.width || !ui.canvas.height) return; + const box = previewBox(); + // A collapsed box means the stage is hidden; leave the last good size in + // place rather than flattening the canvas to nothing. + if (box.cssWidth <= 0 || box.cssHeight <= 0) return; + + const size = fitWithin(ui.canvas.width, ui.canvas.height, box.cssWidth, box.cssHeight); + ui.canvas.style.width = `${size.width}px`; + ui.canvas.style.height = `${size.height}px`; } /** @@ -253,6 +317,7 @@ function onFrame(frame: PreviewResult): void { 0, 0, ); + layoutCanvas(); // The overlay tracks the frame it is drawn on, which in crop mode is the // untrimmed image and therefore exactly crop space. @@ -927,8 +992,12 @@ function wireStage(): void { ui.compareBtn.addEventListener('keyup', endCompare); ui.compareBtn.addEventListener('blur', endCompare); + // Re-fit what is already on screen straight away so the image never lags + // the box it sits in, then re-render once the drag settles to recover the + // resolution the new box is worth. let resizeTimer: number | undefined; window.addEventListener('resize', () => { + layoutCanvas(); window.clearTimeout(resizeTimer); resizeTimer = window.setTimeout(() => refresh(), 150); });