From 1464c6ae12c02e4ec60cf71370f13a1e80b2c449 Mon Sep 17 00:00:00 2001 From: Paola De Bartolo Date: Mon, 20 Jul 2026 16:21:33 -0300 Subject: [PATCH 1/5] fix: make px crops export at a deterministic size Close #33 --- .../vaadin/addons/imagecrop/Crop.java | 5 +- .../vaadin/addons/imagecrop/ImageCrop.java | 19 +- .../resources/frontend/src/image-crop.tsx | 181 ++++++++---------- 3 files changed, 103 insertions(+), 102 deletions(-) diff --git a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.java b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.java index ccb2b65..ced0a85 100644 --- a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.java +++ b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.java @@ -26,8 +26,9 @@ * The crop dimensions are defined by the unit, x and y coordinates, width, and * height. * - * @param unit the unit of the crop dimensions, can be 'px' (pixels) or '%' - * (percentage). + * @param unit the unit of the crop dimensions, can be 'px' or '%'. A '%' crop + * is resolution-independent; a 'px' crop is interpreted in the + * image's source (natural) pixels (see issue #33). * @param x the x-coordinate of the cropped area. * @param y the y-coordinate of the cropped area. * @param width the width of the cropped area diff --git a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java index d93706c..1f6d769 100644 --- a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java +++ b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java @@ -135,7 +135,15 @@ public String getImageAlt() { /** * Defines the crop dimensions. - * + * + *

+ * A {@code %} crop is resolution-independent and is recommended for + * programmatic use. A {@code px} crop is interpreted in the image's + * source (natural) pixels, so the cropped output has a deterministic + * size regardless of how the image is scaled on screen (see issue #33). Note + * that this differs from {@link #setCropMinWidth(Integer) the min/max crop + * constraints}, which are expressed in rendered (on-screen) pixels. + * * @param crop the crop dimensions */ public void setCrop(Crop crop) { @@ -247,8 +255,13 @@ public boolean isLocked() { } /** - * Sets a minimum crop width, in pixels. - * + * Sets a minimum crop width, in rendered (on-screen) pixels. + * + *

+ * This constraint is applied by react-image-crop in the image's displayed + * pixels, unlike a {@code px} {@link #setCrop(Crop) crop}, which is expressed in + * source (natural) pixels. + * * @param minWidth the minimum crop width */ public void setCropMinWidth(Integer minWidth) { diff --git a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx index 3c92c4f..f57b248 100644 --- a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx +++ b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx @@ -106,82 +106,75 @@ class ImageCropElement extends ReactAdapterElement { const [outputMimeType] = hooks.useState("outputMimeType"); const [outputQuality] = hooks.useState("outputQuality", 1.0); - // Track previous image dimensions to adjust crop proportionally when resizing - const prevImgSize = useRef<{ width: number; height: number } | null>(null); // Skip the first run of the output-format effect (initial encoding is handled on image load) const didMountRef = useRef(false); /** - * Handles intial calculations on image load. + * Converts a value expressed in source (natural) pixels to a percentage of + * the given dimension. */ - const onImageLoad = () => { - if (imgRef.current) { - const { width, height } = imgRef.current; - prevImgSize.current = { width, height }; - if (crop) { - const newcrop = centerCrop( - makeAspectCrop( - { - unit: crop.unit, - width: crop.width, - height: crop.height, - x: crop.x, - y: crop.y - }, - aspect, - width, - height - ), - width, - height - ) - setCrop(newcrop); - this._updateCroppedImage(newcrop); - } - } - }; + const toPercent = (value: number, dimension: number) => + dimension ? (value / dimension) * 100 : 0; /** - * Adjusts the crop size proportionally when the image is resized. + * Normalizes the configured crop when the image loads. The crop is kept as a + * percentage of the image's natural size, so both the on-screen selection and + * the exported image are independent of how the browser scales the image on + * screen. A configured "px" crop is interpreted as source (natural) pixels, + * which makes the exported size deterministic (see issue #33). */ - const resizeCrop = (newWidth: number, newHeight: number) => { - if (!crop || !prevImgSize.current) return; - const { width: oldWidth, height: oldHeight } = prevImgSize.current; - - const scaleX = newWidth / oldWidth; - const scaleY = newHeight / oldHeight; - - const resizedCrop: Crop = { - unit: crop.unit, - width: crop.width * scaleX, - height: crop.height * scaleY, - x: crop.x * scaleX, - y: crop.y * scaleY, - }; + const onImageLoad = () => { + const img = imgRef.current; + if (!img || !crop) { + return; + } + const { naturalWidth, naturalHeight } = img; + + // Work in "%": a "px" crop is treated as source pixels and converted. + let normalized: PercentCrop = crop.unit === "%" + ? { unit: "%", x: crop.x, y: crop.y, width: crop.width, height: crop.height } + : { + unit: "%", + x: toPercent(crop.x, naturalWidth), + y: toPercent(crop.y, naturalHeight), + width: toPercent(crop.width, naturalWidth), + height: toPercent(crop.height, naturalHeight), + }; + + // Enforce the aspect ratio when configured, then center the selection. + if (aspect) { + normalized = makeAspectCrop( + { unit: "%", width: normalized.width, x: normalized.x, y: normalized.y }, + aspect, + naturalWidth, + naturalHeight + ); + } + normalized = centerCrop(normalized, naturalWidth, naturalHeight); - setCrop(resizedCrop); - prevImgSize.current = { width: newWidth, height: newHeight }; + setCrop(normalized); + this._updateCroppedImage(normalized); }; /** - * Observes image resizing and updates crop size dynamically. + * Normalizes a programmatic "px" crop that the server sets after the image + * has loaded. onImageLoad only runs on the initial load, so without this a + * later setCrop("px", ...) would be rendered by ReactCrop as on-screen pixels + * and the selection box would diverge from the natural-pixel export (issue + * #33). The configured x/y are preserved (no centering) since the crop is + * explicitly positioned. */ useEffect(() => { - if (!imgRef.current) return; - - const resizeObserver = new ResizeObserver(() => { - if (imgRef.current && prevImgSize.current) { - const { width, height } = imgRef.current; - if (width != prevImgSize.current.width && - height != prevImgSize.current.height) { - resizeCrop(width, height); - } - } - }); - - resizeObserver.observe(imgRef.current); - - return () => resizeObserver.disconnect(); + const img = imgRef.current; + if (crop && crop.unit !== "%" && img && img.naturalWidth) { + setCrop({ + unit: "%", + x: toPercent(crop.x, img.naturalWidth), + y: toPercent(crop.y, img.naturalHeight), + width: toPercent(crop.width, img.naturalWidth), + height: toPercent(crop.height, img.naturalHeight), + }); + } }, [crop]); /** @@ -199,19 +192,22 @@ class ImageCropElement extends ReactAdapterElement { } }, [outputMimeType, outputQuality]); - const onChange = (c: Crop) => { - setCrop(c); + // Keep the crop state in "%" so it stays valid regardless of the image's + // on-screen size; that scale-invariance is why no ResizeObserver is needed + // to rescale it on layout changes (see issue #33). + const onChange = (_pixelCrop: PixelCrop, percentCrop: PercentCrop) => { + setCrop(percentCrop); }; - const onComplete = (c: PixelCrop) => { - this._updateCroppedImage(c); + const onComplete = (_pixelCrop: PixelCrop, percentCrop: PercentCrop) => { + this._updateCroppedImage(percentCrop); }; return ( onChange(c)} - onComplete={(c: PixelCrop) => onComplete(c)} + onChange={(c: PixelCrop, pc: PercentCrop) => onChange(c, pc)} + onComplete={(c: PixelCrop, pc: PercentCrop) => onComplete(c, pc)} circularCrop={circularCrop} aspect={aspect} keepSelection={keepSelection} @@ -246,43 +242,34 @@ class ImageCropElement extends ReactAdapterElement { * Draws the selected crop region onto an off-screen canvas and dispatches the * resulting data URI through a {@code cropped-image} event. * - *

The crop rectangle reported by react-image-crop is expressed in the - * image's displayed (rendered) pixels, which can be smaller or larger - * than the image's intrinsic resolution when the browser scales it to fit the - * layout. The selected region is mapped back to the source's natural - * pixels using {@code scaleX}/{@code scaleY} for both the source rectangle and - * the output canvas, so the cropped image keeps the original resolution of the - * selected area rather than the (smaller or larger) on-screen size (see issue - * #26).

+ *

The crop is mapped to the image's natural (intrinsic) pixels: + * {@code convertToPixelCrop} scales a {@code %} crop against + * {@code naturalWidth}/{@code naturalHeight}, while a {@code px} crop is taken + * as source (natural) pixels directly. Because the mapping never depends on the + * image's on-screen size, the exported dimensions are deterministic regardless + * of how the browser scaled the image when the crop was set (see issues #26 and + * #33).

* - *

Note: a {@code px} crop is measured in rendered pixels, so the exported - * size is the rendered crop scaled to natural resolution, not necessarily the - * configured pixel value. The output is not multiplied by - * {@code window.devicePixelRatio}, so the original pixels are used verbatim - * instead of being upsampled on high-density displays (see issue #21).

+ *

Note: the output is not multiplied by {@code window.devicePixelRatio}, so + * the original pixels are used verbatim instead of being upsampled on + * high-density displays (see issue #21).

*/ public _updateCroppedImage(crop: PixelCrop|PercentCrop) { const image = this.querySelector("img"); if (crop && image) { - crop = convertToPixelCrop(crop, image.width, image.height); + // Map the crop to the image's natural pixels. A "%" crop scales to the + // natural resolution; a "px" crop is interpreted as source pixels. + const ccrop = convertToPixelCrop(crop, image.naturalWidth, image.naturalHeight); // create a canvas element to draw the cropped image const canvas = document.createElement("canvas"); - - // draw the image on the canvas - const ccrop = crop; - - // Ratio between the image's natural resolution and its displayed size. - // Greater than 1 when the image is scaled down to fit the screen. - const scaleX = image.naturalWidth / image.width; - const scaleY = image.naturalHeight / image.height; const ctx = canvas.getContext("2d"); - // Size the output in the crop region's natural pixels so the cropped + // The output is sized in the crop region's natural pixels, so the cropped // image keeps the source's resolution rather than the on-screen size. - const outWidth = Math.round(ccrop.width * scaleX); - const outHeight = Math.round(ccrop.height * scaleY); + const outWidth = Math.round(ccrop.width); + const outHeight = Math.round(ccrop.height); // Setting canvas dimensions resets the 2D context, so it must happen // before any drawing/clipping state is configured below. @@ -302,10 +289,10 @@ class ImageCropElement extends ReactAdapterElement { ctx.drawImage( image, - ccrop.x * scaleX, - ccrop.y * scaleY, - ccrop.width * scaleX, - ccrop.height * scaleY, + ccrop.x, + ccrop.y, + ccrop.width, + ccrop.height, 0, 0, outWidth, From 4ec6120b85105108655a3ea7bee6f6b703072262 Mon Sep 17 00:00:00 2001 From: Paola De Bartolo Date: Fri, 14 Aug 2026 11:26:54 -0300 Subject: [PATCH 2/5] wip: skip the export when the crop maps to an empty region --- .../META-INF/resources/frontend/src/image-crop.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx index f57b248..c00f0e1 100644 --- a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx +++ b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx @@ -271,6 +271,16 @@ class ImageCropElement extends ReactAdapterElement { const outWidth = Math.round(ccrop.width); const outHeight = Math.round(ccrop.height); + // Nothing to export: the image has no intrinsic size (naturalWidth / + // naturalHeight are still 0 before it loads, and stay 0 for a source + // without an intrinsic size, which collapses any crop to zero) or the + // selection itself is empty. Bail out instead of drawing a 0x0 canvas + // and firing an event with a blank "data:," URI. + if (!Number.isFinite(outWidth) || !Number.isFinite(outHeight) + || outWidth <= 0 || outHeight <= 0) { + return; + } + // Setting canvas dimensions resets the 2D context, so it must happen // before any drawing/clipping state is configured below. canvas.width = outWidth; From 4bed3bad9f31f7ab2e239399902d47e2af130c1b Mon Sep 17 00:00:00 2001 From: Paola De Bartolo Date: Fri, 14 Aug 2026 12:11:23 -0300 Subject: [PATCH 3/5] wip: reuse react-image-crop's converter for px-to-% normalization --- .../resources/frontend/src/image-crop.tsx | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx index c00f0e1..a10bb98 100644 --- a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx +++ b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx @@ -21,7 +21,7 @@ import { ReactAdapterElement, RenderHooks } from 'Frontend/generated/flow/ReactAdapter'; import { JSXElementConstructor, ReactElement, useRef, useEffect } from "react"; import React from 'react'; -import { type Crop, ReactCrop, PixelCrop, PercentCrop, makeAspectCrop, centerCrop, convertToPixelCrop } from "react-image-crop"; +import { type Crop, ReactCrop, PixelCrop, PercentCrop, makeAspectCrop, centerCrop, convertToPixelCrop, convertToPercentCrop } from "react-image-crop"; // MIME types that HTMLCanvasElement.toDataURL can actually encode across browsers. // Anything else silently falls back to image/png, so we never emit it. @@ -110,11 +110,22 @@ class ImageCropElement extends ReactAdapterElement { const didMountRef = useRef(false); /** - * Converts a value expressed in source (natural) pixels to a percentage of - * the given dimension. + * Normalizes a configured crop to a "%" crop of the image's natural size: a + * "px" crop is interpreted as source (natural) pixels, while a "%" crop is + * already resolution-independent and is returned unchanged. + * + * Returns null while the image has no intrinsic size (naturalWidth / + * naturalHeight are 0 before it loads, and stay 0 for a source without an + * intrinsic size), since every conversion against a zero dimension is + * meaningless. */ - const toPercent = (value: number, dimension: number) => - dimension ? (value / dimension) * 100 : 0; + const toPercentCrop = (configured: Crop, img: HTMLImageElement): PercentCrop | null => { + const { naturalWidth, naturalHeight } = img; + if (!naturalWidth || !naturalHeight) { + return null; + } + return convertToPercentCrop(configured, naturalWidth, naturalHeight); + }; /** * Normalizes the configured crop when the image loads. The crop is kept as a @@ -128,18 +139,12 @@ class ImageCropElement extends ReactAdapterElement { if (!img || !crop) { return; } - const { naturalWidth, naturalHeight } = img; - // Work in "%": a "px" crop is treated as source pixels and converted. - let normalized: PercentCrop = crop.unit === "%" - ? { unit: "%", x: crop.x, y: crop.y, width: crop.width, height: crop.height } - : { - unit: "%", - x: toPercent(crop.x, naturalWidth), - y: toPercent(crop.y, naturalHeight), - width: toPercent(crop.width, naturalWidth), - height: toPercent(crop.height, naturalHeight), - }; + let normalized = toPercentCrop(crop, img); + if (!normalized) { + return; + } + const { naturalWidth, naturalHeight } = img; // Enforce the aspect ratio when configured, then center the selection. if (aspect) { @@ -166,14 +171,12 @@ class ImageCropElement extends ReactAdapterElement { */ useEffect(() => { const img = imgRef.current; - if (crop && crop.unit !== "%" && img && img.naturalWidth) { - setCrop({ - unit: "%", - x: toPercent(crop.x, img.naturalWidth), - y: toPercent(crop.y, img.naturalHeight), - width: toPercent(crop.width, img.naturalWidth), - height: toPercent(crop.height, img.naturalHeight), - }); + if (!crop || crop.unit === "%" || !img) { + return; + } + const normalized = toPercentCrop(crop, img); + if (normalized) { + setCrop(normalized); } }, [crop]); From 26b6213fc15d5c50324550f3646acdce79f29e8f Mon Sep 17 00:00:00 2001 From: Paola De Bartolo Date: Fri, 14 Aug 2026 12:34:21 -0300 Subject: [PATCH 4/5] wip: enforce the configured aspect on a post-load px crop --- .../resources/frontend/src/image-crop.tsx | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx index a10bb98..cdb627b 100644 --- a/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx +++ b/src/main/resources/META-INF/resources/frontend/src/image-crop.tsx @@ -127,6 +127,23 @@ class ImageCropElement extends ReactAdapterElement { return convertToPercentCrop(configured, naturalWidth, naturalHeight); }; + /** + * Enforces the configured aspect ratio on a "%" crop by deriving its height + * from its width, leaving the position untouched. + * + * No-op when no aspect is configured: makeAspectCrop divides the width by the + * aspect, so passing an undefined one collapses the crop to zero. + */ + const applyAspect = (percentCrop: PercentCrop, img: HTMLImageElement): PercentCrop => + aspect + ? makeAspectCrop( + { unit: "%", width: percentCrop.width, x: percentCrop.x, y: percentCrop.y }, + aspect, + img.naturalWidth, + img.naturalHeight + ) + : percentCrop; + /** * Normalizes the configured crop when the image loads. The crop is kept as a * percentage of the image's natural size, so both the on-screen selection and @@ -144,18 +161,9 @@ class ImageCropElement extends ReactAdapterElement { if (!normalized) { return; } - const { naturalWidth, naturalHeight } = img; - // Enforce the aspect ratio when configured, then center the selection. - if (aspect) { - normalized = makeAspectCrop( - { unit: "%", width: normalized.width, x: normalized.x, y: normalized.y }, - aspect, - naturalWidth, - naturalHeight - ); - } - normalized = centerCrop(normalized, naturalWidth, naturalHeight); + normalized = applyAspect(normalized, img); + normalized = centerCrop(normalized, img.naturalWidth, img.naturalHeight); setCrop(normalized); this._updateCroppedImage(normalized); @@ -167,7 +175,9 @@ class ImageCropElement extends ReactAdapterElement { * later setCrop("px", ...) would be rendered by ReactCrop as on-screen pixels * and the selection box would diverge from the natural-pixel export (issue * #33). The configured x/y are preserved (no centering) since the crop is - * explicitly positioned. + * explicitly positioned, but the aspect ratio is enforced just as it is on + * load, so a crop that does not match the configured aspect is corrected + * instead of staying off-ratio until the user drags a handle. */ useEffect(() => { const img = imgRef.current; @@ -176,7 +186,7 @@ class ImageCropElement extends ReactAdapterElement { } const normalized = toPercentCrop(crop, img); if (normalized) { - setCrop(normalized); + setCrop(applyAspect(normalized, img)); } }, [crop]); From 0af81acaa12157baacf3343aef18dc6404f05e07 Mon Sep 17 00:00:00 2001 From: Paola De Bartolo Date: Tue, 18 Aug 2026 15:49:53 -0300 Subject: [PATCH 5/5] wip: document that the crop is normalized to % on load --- .../vaadin/addons/imagecrop/ImageCrop.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java index 1f6d769..23bc596 100644 --- a/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java +++ b/src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java @@ -144,6 +144,13 @@ public String getImageAlt() { * that this differs from {@link #setCropMinWidth(Integer) the min/max crop * constraints}, which are expressed in rendered (on-screen) pixels. * + *

+ * A {@code %} crop is measured against the image's natural size when the output + * is generated, but against its rendered box when the selection is drawn. Both + * agree as long as the rendered image keeps the source's aspect ratio; forcing + * both a width and a height on the image (or an {@code object-fit} that crops or + * stretches it) makes the on-screen selection diverge from the exported region. + * * @param crop the crop dimensions */ public void setCrop(Crop crop) { @@ -153,6 +160,15 @@ public void setCrop(Crop crop) { /** * Gets the crop dimensions. + * + *

+ * Once the image has loaded, the crop is normalized to a percentage of the + * image's natural size, so the returned unit is {@code %} even when + * {@link #setCrop(Crop)} was given a {@code px} crop. Because {@link Crop} holds + * integer values, those percentages are rounded to whole units, which on a large + * image is coarser than the configured pixel values. + * + * @return the crop dimensions */ public Crop getCrop() { return getState("crop", Crop.class);