diff --git a/.vscode/launch.json b/.vscode/launch.json index 3ba165a448..a624884534 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -47,6 +47,13 @@ "request": "launch", "type": "node-terminal" }, + { + "command": "pnpm run dev uploads", + "cwd": "${workspaceFolder}", + "name": "Run Dev Uploads", + "request": "launch", + "type": "node-terminal" + }, { "command": "PAYLOAD_BUNDLER=vite pnpm run dev fields", "cwd": "${workspaceFolder}", diff --git a/packages/payload/src/admin/components/elements/EditUpload/index.tsx b/packages/payload/src/admin/components/elements/EditUpload/index.tsx index 6fbf8bff0f..ea948ba3fd 100644 --- a/packages/payload/src/admin/components/elements/EditUpload/index.tsx +++ b/packages/payload/src/admin/components/elements/EditUpload/index.tsx @@ -83,7 +83,7 @@ export const EditUpload: React.FC<{ setFormQueryParams({ ...formQueryParams, uploadEdits: { - crop: crop ? crop : undefined, + crop: crop || undefined, focalPoint: pointPosition ? pointPosition : undefined, }, }) diff --git a/packages/payload/src/uploads/generateFileData.ts b/packages/payload/src/uploads/generateFileData.ts index e427c788dc..40bfc41269 100644 --- a/packages/payload/src/uploads/generateFileData.ts +++ b/packages/payload/src/uploads/generateFileData.ts @@ -99,7 +99,7 @@ export const generateFileData = async ({ let fileBuffer: { data: Buffer; info: OutputInfo } let ext let mime: string - const isSharpRequired = + const fileHasAdjustments = fileSupportsResize && Boolean(resizeOptions || formatOptions || trimOptions || file.tempFilePath) @@ -107,7 +107,7 @@ export const generateFileData = async ({ if (fileIsAnimated) sharpOptions.animated = true - if (isSharpRequired) { + if (fileHasAdjustments) { if (file.tempFilePath) { sharpFile = sharp(file.tempFilePath, sharpOptions).rotate() // pass rotate() to auto-rotate based on EXIF data. https://github.com/payloadcms/payload/pull/3081 } else { @@ -174,7 +174,7 @@ export const generateFileData = async ({ fileData.filename = fsSafeName let fileForResize = file - if (isSharpRequired && cropData) { + if (cropData) { const { data: croppedImage, info } = await cropImage({ cropData, dimensions, file }) filesToSave.push({ diff --git a/packages/payload/src/uploads/imageResizer.ts b/packages/payload/src/uploads/imageResizer.ts index 1174114fe2..24b07df5fe 100644 --- a/packages/payload/src/uploads/imageResizer.ts +++ b/packages/payload/src/uploads/imageResizer.ts @@ -113,36 +113,83 @@ const createResult = ( * * @param resizeConfig - object containing the requested dimensions and resize options * @param original - the original image size - * @returns true if the image needs to be resized, false otherwise + * @returns true if resizing is not needed, false otherwise */ -const needsResize = ( +const preventResize = ( { height: desiredHeight, width: desiredWidth, withoutEnlargement, withoutReduction }: ImageSize, original: ProbedImageSize, ): boolean => { - // allow enlargement or prevent reduction (our default is to prevent - // enlargement and allow reduction) - if (withoutEnlargement !== undefined || withoutReduction !== undefined) { - return true // needs resize + // default is to allow reduction + if (withoutReduction !== undefined) { + return false // needs resize + } + + // default is to prevent enlargement + if (withoutEnlargement !== undefined) { + return false // needs resize } const isWidthOrHeightNotDefined = !desiredHeight || !desiredWidth - if (isWidthOrHeightNotDefined) { // If with and height are not defined, it means there is a format conversion // and the image needs to be "resized" (transformed). - return true // needs resize + return false // needs resize } - const hasInsufficientWidth = original.width < desiredWidth - const hasInsufficientHeight = original.height < desiredHeight + const hasInsufficientWidth = desiredWidth > original.width + const hasInsufficientHeight = desiredHeight > original.height if (hasInsufficientWidth && hasInsufficientHeight) { // doesn't need resize - prevent enlargement. This should only happen if both width and height are insufficient. // if only one dimension is insufficient and the other is sufficient, resizing needs to happen, as the image // should be resized to the sufficient dimension. - return false + return true // do not create a new size } - return true // needs resize + return false // needs resize +} + +/** + * Check if the image should be passed directly to sharp without payload adjusting properties. + * + * @param resizeConfig - object containing the requested dimensions and resize options + * @param original - the original image size + * @returns true if the image should passed directly to sharp + */ +const applyPayloadAdjustments = ( + { height, width, withoutEnlargement, withoutReduction }: ImageSize, + original: ProbedImageSize, +) => { + if (!isNumber(height) && !isNumber(width)) return false + + const targetAspectRatio = width / height + const originalAspectRatio = original.width / original.height + if (originalAspectRatio === targetAspectRatio) return false + + const skipEnlargement = withoutEnlargement && (original.height < height || original.width < width) + const skipReduction = withoutReduction && (original.height > height || original.width > width) + if (skipEnlargement || skipReduction) return false + + return true +} + +/** + * Sanitize the resize config. If the resize config has the `withoutReduction` + * property set to true, the `fit` and `position` properties will be set to `contain` + * and `top left` respectively. + * + * @param resizeConfig - the resize config + * @returns a sanitized resize config + */ +const sanitizeResizeConfig = (resizeConfig: ImageSize): ImageSize => { + if (resizeConfig.withoutReduction) { + return { + ...resizeConfig, + // Why fit `contain` should also be set to https://github.com/lovell/sharp/issues/3595 + fit: resizeConfig?.fit || 'contain', + position: resizeConfig?.position || 'left top', + } + } + return resizeConfig } /** @@ -176,66 +223,65 @@ export default async function resizeAndTransformImageSizes({ const results: ImageSizesResult[] = await Promise.all( imageSizes.map(async (imageResizeConfig): Promise => { + imageResizeConfig = sanitizeResizeConfig(imageResizeConfig) + // This checks if a resize should happen. If not, the resized image will be // skipped COMPLETELY and thus will not be included in the resulting images. // All further format/trim options will thus be skipped as well. - if (!needsResize(imageResizeConfig, dimensions)) { + if (preventResize(imageResizeConfig, dimensions)) { return createResult(imageResizeConfig.name) } - let resized = sharpBase.clone() - const hasEdits = req.query?.uploadEdits + const imageToResize = sharpBase.clone() + let resized = imageToResize.resize(imageResizeConfig) - if (hasEdits && imageResizeConfig.width && imageResizeConfig.height) { - const { height, width } = imageResizeConfig - - const targetAspectRatio = width / height + if ( + req.query?.uploadEdits?.focalPoint && + applyPayloadAdjustments(imageResizeConfig, dimensions) + ) { + const { height: resizeHeight, width: resizeWidth } = imageResizeConfig + const resizeAspectRatio = resizeWidth / resizeHeight const originalAspectRatio = dimensions.width / dimensions.height + const prioritizeHeight = resizeAspectRatio < originalAspectRatio - if (originalAspectRatio === targetAspectRatio) { - resized = resized.resize(imageResizeConfig) - } else { - const focalPoint = { - x: 0.5, - y: 0.5, - } + // Scale the image up or down to fit the resize dimensions + const scaledImage = imageToResize.resize({ + height: prioritizeHeight ? resizeHeight : null, + width: prioritizeHeight ? null : resizeWidth, + }) + const { info: scaledImageInfo } = await scaledImage.toBuffer({ resolveWithObject: true }) - if (req.query.uploadEdits?.focalPoint) { - if (isNumber(req.query.uploadEdits.focalPoint?.x)) { - focalPoint.x = req.query.uploadEdits.focalPoint.x - } - if (isNumber(req.query.uploadEdits.focalPoint?.y)) { - focalPoint.y = req.query.uploadEdits.focalPoint.y - } - } - - const prioritizeHeight = originalAspectRatio > targetAspectRatio - - const { info } = await resized - .resize({ - height: prioritizeHeight ? height : null, - width: prioritizeHeight ? null : width, - }) - .toBuffer({ resolveWithObject: true }) - - const maxOffsetX = Math.max(info.width - width, 0) - const maxOffsetY = Math.max(info.height - height, 0) - - const focalPointX = Math.floor((info.width / 100) * focalPoint.x) - const focalPointY = Math.floor((info.height / 100) * focalPoint.y) - - const offsetX = Math.min(Math.max(focalPointX - width / 2, 0), maxOffsetX) - const offsetY = Math.min(Math.max(focalPointY - height / 2, 0), maxOffsetY) - - resized = resized.extract({ - height, - left: offsetX, - top: offsetY, - width, - }) + // Focal point adjustments + const focalPoint = { + x: isNumber(req.query.uploadEdits.focalPoint?.x) + ? req.query.uploadEdits.focalPoint.x + : 50, + y: isNumber(req.query.uploadEdits.focalPoint?.y) + ? req.query.uploadEdits.focalPoint.y + : 50, } - } else { - resized = resized.resize(imageResizeConfig) + + const safeResizeWidth = resizeWidth ?? scaledImageInfo.width + const maxOffsetX = scaledImageInfo.width - safeResizeWidth + const leftFocalEdge = Math.round( + scaledImageInfo.width * (focalPoint.x / 100) - safeResizeWidth / 2, + ) + const safeOffsetX = Math.min(Math.max(0, leftFocalEdge), maxOffsetX) + + const safeResizeHeight = resizeHeight ?? scaledImageInfo.height + const maxOffsetY = scaledImageInfo.height - safeResizeHeight + const topFocalEdge = Math.round( + scaledImageInfo.height * (focalPoint.y / 100) - safeResizeHeight / 2, + ) + const safeOffsetY = Math.min(Math.max(0, topFocalEdge), maxOffsetY) + + // extract the focal area from the scaled image + resized = scaledImage.extract({ + height: safeResizeHeight, + left: safeOffsetX, + top: safeOffsetY, + width: safeResizeWidth, + }) } if (imageResizeConfig.formatOptions) { diff --git a/test/uploads/config.ts b/test/uploads/config.ts index 819000d932..1421e47788 100644 --- a/test/uploads/config.ts +++ b/test/uploads/config.ts @@ -330,7 +330,6 @@ export default buildConfigWithDefaults({ height: 80, formatOptions: { format: 'jpg', options: { quality: 90 } }, withoutReduction: true, - fit: 'contain', }, { name: 'resizedLarger', @@ -341,9 +340,7 @@ export default buildConfigWithDefaults({ name: 'resizedSmaller', width: 180, height: 50, - // Why fit `contain` should also be set to https://github.com/lovell/sharp/issues/3595 withoutReduction: true, - fit: 'contain', }, ], }, diff --git a/test/uploads/e2e.spec.ts b/test/uploads/e2e.spec.ts index 70bfe920a3..9fb67f238b 100644 --- a/test/uploads/e2e.spec.ts +++ b/test/uploads/e2e.spec.ts @@ -10,11 +10,13 @@ import wait from '../../packages/payload/src/utilities/wait' import { saveDocAndAssert } from '../helpers' import { AdminUrlUtil } from '../helpers/adminUrlUtil' import { initPayloadE2E } from '../helpers/configHelpers' +import { RESTClient } from '../helpers/rest' import { adminThumbnailSrc } from './collections/admin-thumbnail' import { adminThumbnailSlug, audioSlug, mediaSlug, relationSlug } from './shared' const { beforeAll, describe } = test +let client: RESTClient let mediaURL: AdminUrlUtil let audioURL: AdminUrlUtil let relationURL: AdminUrlUtil @@ -27,6 +29,8 @@ describe('uploads', () => { beforeAll(async ({ browser }) => { const { serverURL } = await initPayloadE2E(__dirname) + client = new RESTClient(null, { serverURL, defaultSlug: 'users' }) + await client.login() mediaURL = new AdminUrlUtil(serverURL, mediaSlug) audioURL = new AdminUrlUtil(serverURL, audioSlug) @@ -47,7 +51,7 @@ describe('uploads', () => { }, }) - pngDoc = findPNG.docs[0] as Media + pngDoc = findPNG.docs[0] as unknown as Media const findAudio = await payload.find({ collection: audioSlug, @@ -55,7 +59,7 @@ describe('uploads', () => { pagination: false, }) - audioDoc = findAudio.docs[0] as Media + audioDoc = findAudio.docs[0] as unknown as Media }) test('should see upload filename in relation list', async () => { @@ -88,10 +92,6 @@ describe('uploads', () => { await saveDocAndAssert(page) }) - test('should update file upload', async () => { - await page.goto(mediaURL.edit(pngDoc.id)) - }) - test('should show resized images', async () => { await page.goto(mediaURL.edit(pngDoc.id)) @@ -192,4 +192,79 @@ describe('uploads', () => { const audioUploadImage = page.locator('tr.row-2 .thumbnail img') expect(await audioUploadImage.getAttribute('src')).toContain(adminThumbnailSrc) }) + + describe('image manipulation', () => { + test('should crop image correctly', async () => { + const positions = { + 'top-left': { + focalX: 25, + focalY: 25, + dragX: 0, + dragY: 0, + }, + 'bottom-right': { + focalX: 75, + focalY: 75, + dragX: 800, + dragY: 800, + }, + } + const createFocalCrop = async (page: Page, position: 'bottom-right' | 'top-left') => { + const { focalX, focalY, dragX, dragY } = positions[position] + await page.goto(mediaURL.create) + + // select and upload file + const fileChooserPromise = page.waitForEvent('filechooser') + await page.getByText('Select a file').click() + const fileChooser = await fileChooserPromise + await fileChooser.setFiles(path.join(__dirname, 'test-image.jpg')) + await page.locator('.file-field__edit').click() + + // set crop + await page.locator('.edit-upload__input input[name="Width (px)"]').fill('400') + await page.locator('.edit-upload__input input[name="Height (px)"]').fill('400') + // set focal point + await page.locator('.edit-upload__input input[name="X %"]').fill('25') // init left focal point + await page.locator('.edit-upload__input input[name="Y %"]').fill('25') // init top focal point + + // hover the crop selection, position mouse outside of focal point hitbox + await page.locator('.ReactCrop__crop-selection').hover({ position: { x: 100, y: 100 } }) + await page.mouse.down() // start drag + await page.mouse.move(dragX, dragY) // drag selection to the lower right corner + await page.mouse.up() // release drag + + // focal point should reset to center + await expect(page.locator('.edit-upload__input input[name="X %"]')).toHaveValue(`${focalX}`) + await expect(page.locator('.edit-upload__input input[name="Y %"]')).toHaveValue(`${focalY}`) + + await page.locator('button:has-text("Apply Changes")').click() + await page.waitForSelector('button#action-save') + await page.locator('button#action-save').click() + } + + await createFocalCrop(page, 'bottom-right') // green square + await wait(1000) // wait for edit view navigation (saving images) + // get the ID of the doc + const greenSquareMediaID = page.url().split('/').pop() + await createFocalCrop(page, 'top-left') // red square + await wait(1000) // wait for edit view navigation (saving images) + const redSquareMediaID = page.url().split('/').pop() + + const { doc: greenDoc } = await client.findByID({ + id: greenSquareMediaID, + slug: mediaSlug, + auth: true, + }) + + const { doc: redDoc } = await client.findByID({ + id: redSquareMediaID, + slug: mediaSlug, + auth: true, + }) + + // green and red squares should have different sizes (colors make the difference) + expect(greenDoc.filesize).toEqual(1205) + expect(redDoc.filesize).toEqual(1207) + }) + }) }) diff --git a/test/uploads/test-image.jpg b/test/uploads/test-image.jpg new file mode 100644 index 0000000000..3a0303dcf8 Binary files /dev/null and b/test/uploads/test-image.jpg differ