An image cropper has to satisfy three rules on every single pointer move:
- the crop stays inside the image
- neither side goes below a minimum
- if a ratio is locked, width ÷ height equals it exactly
Each is trivial alone. Together they are a small constrained-optimisation problem, and the order you enforce them in changes the answer.
Try it, with the fuzz results computed live: https://dev48.infy.uk/design/day65-image-cropper.html
The bug
The natural implementation applies the ratio, then clamps:
rect.w = Math.min(rect.w, image.w - rect.x); // keep inside horizontally
rect.h = Math.min(rect.h, image.h - rect.y); // keep inside vertically
Two clamps. Each obviously correct. And they silently break the ratio, because clamping width and height by different amounts changes w/h.
Here is what makes it survive review: the rectangle is still inside the image. It is still above the minimum. Every constraint you can see holds. The only one that broke is the invisible one — so it passes a smoke test, ships, and a "1:1" avatar exports at something else. The first person to notice is a designer looking at a grid of faces.
The fix is to stop having two numbers
A locked ratio leaves one degree of freedom, not two. So compute how much of the requested size actually fits — as a single scale factor — and apply it to both sides:
const grow = Math.max(1, min / w, (min / ratio) / h); // minimum, as a scale
w *= grow; h *= grow;
const fit = Math.min(1, maxW / w, maxH / h); // bounds, as a scale
w *= fit; h *= fit;
The ratio cannot drift because it never becomes two independent quantities. That is the general shape: when a constraint says two variables move together, do not give the code a chance to move them separately.
Measured, both ways, 20,000 drags
| clamping | worst aspect drift | drags that left the image |
|---|---|---|
| width and height separately | 1.52 | 0 |
| one scale factor | 4.4e-16 | 0 |
Identical containment. That column is the whole point — the buggy version is not wrong, it is wrong about the thing nobody is checking. Only the drift number separates them, and only because something is watching it.
A concrete case: dragging the SE handle from an anchor near the right edge with a 1:1 lock produces 0.438:1 under the buggy clamp.
Order matters, and the impossible case is honest
Grow to the minimum first, then shrink to fit. The reverse leaves a rectangle that satisfies the minimum and hangs outside the image, because the growth step has nothing after it to catch the overflow.
When the image genuinely cannot hold the minimum at that ratio, the fit step wins and the result is smaller than the minimum. That is the honest outcome — the alternative is a rectangle outside the image — and the function returns it rather than pretending.
Two more things the tests pin
The anchor is captured once. Dragging the NW handle must hold the SE corner still. Store the anchor at pointerdown, not per-frame — recompute it during the drag and the rectangle creeps, because each frame anchors to a corner the previous frame already moved. 8,000 corner drags, the anchored corner moved 0px.
Rotation carries the crop. Rotating 90° swaps the image's width and height, so a crop expressed in the old frame is meaningless in the new one. Of 5,000 random rects, 4,156 would land outside the rotated bounds if left alone — and it is the one case where none of your clamping logic runs, because nothing moved.
Export in source pixels, always
const k = img.naturalWidth / stage.clientWidth;
ctx.drawImage(img, crop.x * k, crop.y * k, crop.w * k, crop.h * k, 0, 0, out.width, out.height);
One scalar on four numbers. Forget it and the export is silently correct-looking and a third of the resolution — a bug that never throws, never gets reported, and just makes everything slightly worse.
The technique worth stealing
The resize is a pure function: anchor, pointer, handle, ratio, bounds, minimum in; a rectangle out. No element, no event, no state. That is what lets 20,000 drags run in a few milliseconds under node with no browser at all — and it is why the invisible constraint could be measured rather than argued about.
Part of a from-scratch series — one component a day, vanilla JS, one file, offline: https://dev48.infy.uk/designfromzero.php
Top comments (0)