refactor(crop): tighten architecture + fix zoom-after-crop
Some checks failed
Deploy to GitHub Pages / build (push) Has been cancelled
Deploy to GitHub Pages / deploy (push) Has been cancelled

Architectural debt addressed in one pass:

- CropViewer drops its five local refs (rotationDeg, cropLeft/Top/Right
  /Bottom) in favour of computed views over `store.cropRotate`. Drag and
  rotation handlers write straight to the store via thin setters; the
  store is now the canonical source of truth. localStorage is flushed on
  commit boundaries (drag end, rotation change, unmount, Next), not on
  every pointermove.
- `ImagePreTransform` deleted from `src/types/index.ts`. CorrectedImage-
  Viewer now takes `crop?: { state: CropRotateState; srcW; srcH }` and
  derives the pixel-space affine internally via a memoised `computed`.
  Cuts the second reactive ref in MeasureViewer and removes the
  redundant "type that exists only to ferry derived numbers across a
  prop boundary."
- `crop-transform.ts` is now pure geometry. The DOM-bound
  `renderRotatedCropped` moved to a new `src/lib/crop-render.ts`. The
  pure module is now safe to import from a worker / test without a
  canvas mock.

Bug fix shipped in the same commit because it fell out of the same
analysis:

- Zoom (wheel + pinch) anchored on a measurement-space point returned
  by `screenToImg`, but `viewOffset = screen - bitmap * scale` expects a
  bitmap-space point. With identity crop the two were equal, so the bug
  was invisible until the user cropped — at that point each zoom step
  shifted the image far off-screen ("canvas goes blank/black"). Pan was
  fine because it never converted screen↔image. Fix: a tiny
  `screenToBitmap` helper used by `onWheel` and the pinch branch of
  `onTouchMove`. Measurement placement / drag still go through the
  full `screenToImg` so coords stay in the canonical frame.
This commit is contained in:
Samuel Prevost 2026-05-01 00:29:24 +02:00
parent 9f54bc62bd
commit 6dc5454d46
6 changed files with 177 additions and 163 deletions

View File

@ -2,7 +2,7 @@
import { ref, computed, onMounted, onUnmounted, watch } from "vue"
import { useMediaQuery } from "@vueuse/core"
import { nanoid } from "nanoid"
import type { ImagePreTransform, Point } from "@/types"
import type { CropRotateState, Point } from "@/types"
import type {
LineMeasurement,
RectMeasurement,
@ -15,29 +15,32 @@ import { getDatumColor } from "@/lib/datums"
import { useAppStore } from "@/stores/app"
import { loadMeasurements, saveMeasurements } from "@/lib/measurement-cache"
import { loadZoom, saveZoom } from "@/lib/zoom-cache"
import { rotatedBboxSize, cropPixels } from "@/lib/crop-transform"
// `imageTransform` is an optional pre-transform from
// "deskewed-image space" (the original untransformed measurement
// coordinate frame) to the bitmap space of the image actually shown
// by `imageUrl`. Used by the Crop & Rotate step so measurements stay
// anchored to the deskewed image while the user views a rotated +
// cropped sub-bitmap. The mapping is:
// `crop` is an optional pre-transform from "deskewed-image space" (the
// original untransformed measurement coordinate frame) to the bitmap
// space of the image actually shown by `imageUrl`. Used by the Crop &
// Rotate step so measurements stay anchored to the deskewed image
// while the user views a rotated + cropped sub-bitmap. The mapping is:
//
// measurement_pt rotate around (srcW/2, srcH/2) by rotationDeg
// translate by (rotW/2, rotH/2)
// subtract (cropX, cropY) bitmap_pt
//
// When omitted (or set to identity values), behaviour is unchanged.
//
// The viewer does NOT rewrite stored measurement coordinates when the
// transform changes; it only adjusts the draw-time projection. This is
// option (b) from the task brief and means changing the crop/rotation
// never invalidates persisted measurements.
// When omitted, behaviour is identity. The viewer does NOT rewrite
// stored measurement coordinates when the transform changes; it only
// adjusts the draw-time projection (option (b) from the task brief),
// so changing the crop/rotation never invalidates persisted
// measurements.
const props = defineProps<{
imageUrl: string
scalePxPerMm: number
/** Optional pre-transform; identity when not supplied. */
imageTransform?: ImagePreTransform
/** Cropping/rotation transform applied between the deskewed image
* and the bitmap pointed at by `imageUrl`. The viewer derives the
* pixel-space affine internally callers only supply the
* canonical state plus the source dimensions of the deskewed
* bitmap. Omit for the legacy identity behaviour. */
crop?: { state: CropRotateState; srcW: number; srcH: number }
}>()
// Emit when the measurement set changes (add / mutate / delete) so the
@ -384,43 +387,55 @@ function makeLiveCtx(): RenderCtx {
}
}
// Pre-derived components of the crop pre-transform. Cached because
// `imgPreTransform` is called per-vertex in inner draw loops and the
// trig + bbox math don't change while the user is just panning the
// canvas. Recomputed only when `props.crop` itself changes.
const cropDerived = computed(() => {
const c = props.crop
if (!c) return null
const rot = rotatedBboxSize(c.srcW, c.srcH, c.state.rotationDeg)
const px = cropPixels(c.state, rot)
const r = (c.state.rotationDeg * Math.PI) / 180
return {
cx: c.srcW / 2,
cy: c.srcH / 2,
cos: Math.cos(r),
sin: Math.sin(r),
halfRotW: rot.rotW / 2,
halfRotH: rot.rotH / 2,
cropX: px.cropX,
cropY: px.cropY,
}
})
// Project a measurement point from deskewed-image space into the
// shown bitmap's coordinate frame. When no `imageTransform` is set we
// short-circuit to the identity for a tiny perf win and to keep the
// behaviour pixel-identical for the legacy non-cropped path.
// shown bitmap's coordinate frame. Returns the input unchanged when
// no crop transform is configured keeps the legacy non-cropped
// path pixel-identical.
function imgPreTransform(pt: Point): Point {
const tr = props.imageTransform
if (!tr) return pt
const r = (tr.rotationDeg * Math.PI) / 180
const cx = tr.srcW / 2
const cy = tr.srcH / 2
const dx = pt.x - cx
const dy = pt.y - cy
const cos = Math.cos(r)
const sin = Math.sin(r)
const rx = dx * cos - dy * sin + tr.rotW / 2
const ry = dx * sin + dy * cos + tr.rotH / 2
return { x: rx - tr.cropX, y: ry - tr.cropY }
const d = cropDerived.value
if (!d) return pt
const dx = pt.x - d.cx
const dy = pt.y - d.cy
const rx = dx * d.cos - dy * d.sin + d.halfRotW
const ry = dx * d.sin + dy * d.cos + d.halfRotH
return { x: rx - d.cropX, y: ry - d.cropY }
}
// Inverse of `imgPreTransform`: bitmap-space deskewed-image space.
// Used by `screenToImg` so pointer-driven placements / drags stay in
// the canonical measurement coordinate frame.
function imgPreTransformInverse(pt: Point): Point {
const tr = props.imageTransform
if (!tr) return pt
const r = (tr.rotationDeg * Math.PI) / 180
const cx = tr.srcW / 2
const cy = tr.srcH / 2
const rx = pt.x + tr.cropX
const ry = pt.y + tr.cropY
const dx = rx - tr.rotW / 2
const dy = ry - tr.rotH / 2
const cos = Math.cos(r)
const sin = Math.sin(r)
const d = cropDerived.value
if (!d) return pt
const rx = pt.x + d.cropX
const ry = pt.y + d.cropY
const dx = rx - d.halfRotW
const dy = ry - d.halfRotH
return {
x: dx * cos + dy * sin + cx,
y: -dx * sin + dy * cos + cy,
x: dx * d.cos + dy * d.sin + d.cx,
y: -dx * d.sin + dy * d.cos + d.cy,
}
}
@ -1874,6 +1889,19 @@ function pointerUp() {
dragState = null
}
// Zoom anchoring needs the bitmap-space point under the cursor, not the
// measurement-space point. `screenToImg` peels off the crop pre-transform
// to return the latter, which is correct for measurement placement but
// breaks the zoom-anchor math (`offset = screen - bitmap * scale`).
// Without crop the two are equal, which is why the bug only shows up
// once the user crops the deskew result.
function screenToBitmap(sx: number, sy: number): Point {
return {
x: (sx - viewOffsetX.value) / viewScale.value,
y: (sy - viewOffsetY.value) / viewScale.value,
}
}
function onWheel(e: WheelEvent) {
e.preventDefault()
const scaleBy = 1.08
@ -1884,11 +1912,11 @@ function onWheel(e: WheelEvent) {
const clamped = Math.max(0.05, Math.min(20, newScale))
const { x: px, y: py } = getCanvasXY(e)
const imgPt = screenToImg(px, py)
const bmp = screenToBitmap(px, py)
viewScale.value = clamped
viewOffsetX.value = px - imgPt.x * clamped
viewOffsetY.value = py - imgPt.y * clamped
viewOffsetX.value = px - bmp.x * clamped
viewOffsetY.value = py - bmp.y * clamped
redraw()
}
@ -2036,11 +2064,11 @@ function onTouchMove(e: TouchEvent) {
if (!rect) return
const cx = (t0.clientX + t1.clientX) / 2 - rect.left
const cy = (t0.clientY + t1.clientY) / 2 - rect.top
const imgPt = screenToImg(cx, cy)
const bmp = screenToBitmap(cx, cy)
viewScale.value = newScale
viewOffsetX.value = cx - imgPt.x * newScale
viewOffsetY.value = cy - imgPt.y * newScale
viewOffsetX.value = cx - bmp.x * newScale
viewOffsetY.value = cy - bmp.y * newScale
lastPinchDist = dist
redraw()

View File

@ -38,12 +38,30 @@ const imgUrl = ref<string | null>(null)
// permanently blank canvas.
const loadError = ref(false)
// Rotation in degrees. Crop fractions of the rotated bbox.
const rotationDeg = ref(0)
const cropLeft = ref(0)
const cropTop = ref(0)
const cropRight = ref(1)
const cropBottom = ref(1)
// Single source of truth: `store.cropRotate`. Rotations and crop drags
// update the store directly so the live state and the persisted state
// can never drift mid-interaction. localStorage is only written on
// explicit commit (drag end, rotation change, unmount) via
// `flushToCache()` that keeps the store hot without thrashing the
// disk on every pointermove.
const rotationDeg = computed(() => store.cropRotate.rotationDeg)
const cropLeft = computed(() => store.cropRotate.crop.left)
const cropTop = computed(() => store.cropRotate.crop.top)
const cropRight = computed(() => store.cropRotate.crop.right)
const cropBottom = computed(() => store.cropRotate.crop.bottom)
function setRotationOnly(deg: number) {
store.setCropRotate({ ...store.cropRotate, rotationDeg: deg })
}
function setCropOnly(crop: {
left: number
top: number
right: number
bottom: number
}) {
store.setCropRotate({ ...store.cropRotate, crop })
}
// Live-fit transform from rotated-bbox space screen canvas pixels.
// Recomputed on resize / rotation change so the user always sees the
@ -60,19 +78,9 @@ const rotBbox = computed(() => {
return rotatedBboxSize(i.naturalWidth, i.naturalHeight, rotationDeg.value)
})
function persist() {
function flushToCache() {
if (!store.fileHash) return
const state = {
rotationDeg: rotationDeg.value,
crop: {
left: cropLeft.value,
top: cropTop.value,
right: cropRight.value,
bottom: cropBottom.value,
},
}
store.setCropRotate(state)
saveCropRotate(store.fileHash, state)
saveCropRotate(store.fileHash, store.cropRotate)
}
function loadImage(url: string) {
@ -327,10 +335,7 @@ function onPointerMove(ev: PointerEvent) {
}
}
cropLeft.value = l
cropTop.value = t
cropRight.value = r2
cropBottom.value = b
setCropOnly({ left: l, top: t, right: r2, bottom: b })
drawOverlay()
}
@ -341,17 +346,13 @@ function onPointerUp(ev: PointerEvent) {
}
if (drag) {
drag = null
persist()
flushToCache()
}
}
function resetCrop() {
cropLeft.value = 0
cropTop.value = 0
cropRight.value = 1
cropBottom.value = 1
rotationDeg.value = 0
persist()
store.resetCropRotate()
flushToCache()
fitToContainer()
redraw()
}
@ -360,10 +361,10 @@ function onRotationInput(v: number) {
if (!Number.isFinite(v)) return
// Clamp to [-180, 180] and normalise so the slider/spinbox can't escape.
const clamped = Math.min(Math.max(v, -180), 180)
rotationDeg.value = clamped
setRotationOnly(clamped)
fitToContainer()
redraw()
persist()
flushToCache()
}
function rotateBy(delta: number) {
@ -377,15 +378,11 @@ onMounted(() => {
store.goToStep(4)
return
}
// Pre-fill from the store / cache so the user's previous crop &
// rotation come back when they re-enter the step.
// Hydrate the store from cache if we have one; otherwise the store
// already holds whatever was set the last time the user came through
// (or `IDENTITY_CROP_ROTATE` on a fresh session). Either way the
// computed refs above will surface the correct values.
const cached = store.fileHash ? loadCropRotate(store.fileHash) : null
const seed = cached ?? store.cropRotate
rotationDeg.value = seed.rotationDeg
cropLeft.value = seed.crop.left
cropTop.value = seed.crop.top
cropRight.value = seed.crop.right
cropBottom.value = seed.crop.bottom
if (cached) store.setCropRotate(cached)
imgUrl.value = URL.createObjectURL(store.deskewResult.correctedImageBlob)
@ -401,11 +398,11 @@ onMounted(() => {
})
onUnmounted(() => {
// If the user navigated away mid-drag (e.g. tapped Back during a corner
// pull), the local refs hold the latest crop coords but `persist()` only
// fires on `pointerup`. Flush here so cache + store match what was on
// screen.
persist()
// The store is already canonical (drag handlers wrote through), but
// localStorage only flushes on commit boundaries if the user navigates
// away mid-drag, push the current store state to disk so the cache
// matches what's on screen.
flushToCache()
resizeObs?.disconnect()
if (imgUrl.value) URL.revokeObjectURL(imgUrl.value)
})
@ -420,7 +417,7 @@ watch([cropLeft, cropTop, cropRight, cropBottom], () => {
})
function next() {
persist()
flushToCache()
store.goToStep(6)
}
</script>

View File

@ -15,7 +15,6 @@ import {
CardTitle,
} from "@/components/ui/card"
import CorrectedImageViewer from "@/components/CorrectedImageViewer.vue"
import type { ImagePreTransform } from "@/types"
// `defineExpose` in CorrectedImageViewer makes these methods available on
// the template ref, but Vue's ComponentPublicInstance type doesn't surface
// them automatically we type the ref explicitly so the call is checked.
@ -26,16 +25,17 @@ type CorrectedImageViewerRef = InstanceType<typeof CorrectedImageViewer> & {
}) => Promise<Blob>
}
import { loadSettings, saveSettings } from "@/lib/settings-cache"
import {
rotatedBboxSize,
cropPixels,
renderRotatedCropped,
} from "@/lib/crop-transform"
import { renderRotatedCropped } from "@/lib/crop-render"
import { patchUpload } from "@/lib/upload-cache"
const store = useAppStore()
const resultUrl = ref<string | null>(null)
const imageTransform = ref<ImagePreTransform | null>(null)
// Source dimensions of the deskewed bitmap (the input to rotate+crop).
// Captured once when we render the cropped output and passed to
// CorrectedImageViewer so it can derive the bitmap-space affine for
// projecting measurement coords. Null until the deskew bitmap is
// decoded for the first time.
const srcDims = ref<{ w: number; h: number } | null>(null)
const viewerRef = ref<CorrectedImageViewerRef | null>(null)
const error = ref("")
const includeScaleBar = ref(false)
@ -133,14 +133,7 @@ async function buildTransformedSource() {
el.src = url
},
)
const state = store.cropRotate
const rot = rotatedBboxSize(
image.naturalWidth,
image.naturalHeight,
state.rotationDeg,
)
const px = cropPixels(state, rot)
const out = renderRotatedCropped(image, state)
const out = renderRotatedCropped(image, store.cropRotate)
const blob = await new Promise<Blob>((resolve, reject) => {
out.toBlob((b) => {
if (b) resolve(b)
@ -149,15 +142,7 @@ async function buildTransformedSource() {
})
if (resultUrl.value) URL.revokeObjectURL(resultUrl.value)
resultUrl.value = URL.createObjectURL(blob)
imageTransform.value = {
rotationDeg: state.rotationDeg,
srcW: image.naturalWidth,
srcH: image.naturalHeight,
rotW: rot.rotW,
rotH: rot.rotH,
cropX: px.cropX,
cropY: px.cropY,
}
srcDims.value = { w: image.naturalWidth, h: image.naturalHeight }
schedulePreview()
} finally {
URL.revokeObjectURL(url)
@ -531,7 +516,15 @@ async function downloadMeasured(scope: "full" | "view") {
ref="viewerRef"
:image-url="resultUrl"
:scale-px-per-mm="store.scalePxPerMm"
:image-transform="imageTransform ?? undefined"
:crop="
srcDims
? {
state: store.cropRotate,
srcW: srcDims.w,
srcH: srcDims.h,
}
: undefined
"
@measurements-changed="schedulePreview"
/>
</CardContent>

36
src/lib/crop-render.ts Normal file
View File

@ -0,0 +1,36 @@
import type { CropRotateState } from "@/types"
import { rotatedBboxSize, cropPixels } from "@/lib/crop-transform"
// DOM rasterisation for the Crop & Rotate pipeline. Kept separate from
// `crop-transform.ts` so the pure geometry helpers (rotatedBboxSize,
// cropPixels) stay portable — no canvas / DOM dependency, importable from
// a worker or a Node-based test.
/** Render the deskewed bitmap onto a fresh canvas with rotation and
* fractional crop applied. Output canvas dims match the cropped sub-
* rectangle in pixels. Used to feed Measure/preview/exports. */
export function renderRotatedCropped(
image: HTMLImageElement | HTMLCanvasElement,
state: CropRotateState,
): HTMLCanvasElement {
const srcW = "naturalWidth" in image ? image.naturalWidth : image.width
const srcH = "naturalHeight" in image ? image.naturalHeight : image.height
const rot = rotatedBboxSize(srcW, srcH, state.rotationDeg)
const px = cropPixels(state, rot)
const out = document.createElement("canvas")
out.width = Math.round(px.cropW)
out.height = Math.round(px.cropH)
const ctx = out.getContext("2d")
if (!ctx) return out
// Draw with the deskewed-bitmap-centre→rotated-bbox-centre transform,
// then translate so the crop's top-left corner becomes (0, 0).
ctx.save()
ctx.translate(-px.cropX, -px.cropY)
ctx.translate(rot.rotW / 2, rot.rotH / 2)
ctx.rotate((state.rotationDeg * Math.PI) / 180)
ctx.drawImage(image, -srcW / 2, -srcH / 2)
ctx.restore()
return out
}

View File

@ -1,6 +1,6 @@
import type { CropRotateState } from "@/types"
// Geometry helpers for the Crop & Rotate step. The pipeline:
// Pure geometry helpers for the Crop & Rotate step. The pipeline:
// 1. Start in "deskewed image space" (the bitmap produced by the
// solver, dimensions deskewW × deskewH).
// 2. Rotate around the deskewed image's centre by `rotationDeg`.
@ -12,6 +12,9 @@ import type { CropRotateState } from "@/types"
// Measurements live in deskewed-image space; the renderer composes this
// transform on top of the live pan/zoom to project them onto the cropped
// bitmap, which is what the user is actually looking at.
//
// DOM rasterisation lives in `crop-render.ts`. This module stays pure so
// the geometry can run in a worker / be tested without a canvas mock.
interface RotatedSize {
rotW: number
@ -59,32 +62,3 @@ export function cropPixels(
cropH: Math.max(1, (bottom - top) * rot.rotH),
}
}
/** Render the deskewed bitmap onto a fresh canvas with rotation and
* fractional crop applied. Output canvas dims match the cropped sub-
* rectangle in pixels. Used to feed Measure/preview/exports. */
export function renderRotatedCropped(
image: HTMLImageElement | HTMLCanvasElement,
state: CropRotateState,
): HTMLCanvasElement {
const srcW = "naturalWidth" in image ? image.naturalWidth : image.width
const srcH = "naturalHeight" in image ? image.naturalHeight : image.height
const rot = rotatedBboxSize(srcW, srcH, state.rotationDeg)
const px = cropPixels(state, rot)
const out = document.createElement("canvas")
out.width = Math.round(px.cropW)
out.height = Math.round(px.cropH)
const ctx = out.getContext("2d")
if (!ctx) return out
// Draw with the deskewed-bitmap-centre→rotated-bbox-centre transform,
// then translate so the crop's top-left corner becomes (0, 0).
ctx.save()
ctx.translate(-px.cropX, -px.cropY)
ctx.translate(rot.rotW / 2, rot.rotH / 2)
ctx.rotate((state.rotationDeg * Math.PI) / 180)
ctx.drawImage(image, -srcW / 2, -srcH / 2)
ctx.restore()
return out
}

View File

@ -144,20 +144,6 @@ export const IDENTITY_CROP_ROTATE: CropRotateState = {
crop: { left: 0, top: 0, right: 1, bottom: 1 },
}
/** Pre-transform consumed by `CorrectedImageViewer`: maps measurement
* points from the original deskewed-image space onto the bitmap that
* is actually painted (which may be a rotated + cropped derivative).
* See `CorrectedImageViewer.vue` for the formula. */
export interface ImagePreTransform {
rotationDeg: number
srcW: number
srcH: number
rotW: number
rotH: number
cropX: number
cropY: number
}
/** Pixels per mm in the output image. Default 10 (= 100 px/cm). */
export const DEFAULT_SCALE_PX_PER_MM = 10