Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 135 additions & 49 deletions packages/upload-media/src/store/private-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { StubFile } from '../stub-file';
import { UploadError } from '../upload-error';
import {
vipsResizeImage,
vipsBatchResizeImage,
vipsRotateImage,
vipsConvertImageFormat,
vipsHasTransparency,
Expand Down Expand Up @@ -1167,6 +1168,35 @@ export function generateThumbnails( id: QueueItemId ) {
);
}

// Determine the actual output type for thumbnails.
// If transcoding is configured, use that format; otherwise
// use the source format.
const thumbnailOutputType = thumbnailTranscodeOperation
? `image/${ thumbnailTranscodeOperation[ 1 ].outputFormat }`
: sourceType;

const quality = thumbnailTranscodeOperation
? thumbnailTranscodeOperation[ 1 ].outputQuality ??
DEFAULT_OUTPUT_QUALITY
: DEFAULT_OUTPUT_QUALITY;

// Collect all resize configs for batch processing.
const batchConfigs: Array< {
name: string;
resize: {
width: number;
height: number;
crop?:
| boolean
| [
'left' | 'center' | 'right',
'top' | 'center' | 'bottom',
];
};
quality: number;
scaledSuffix?: boolean;
} > = [];

for ( const name of sizesToGenerate ) {
const imageSize = allImageSizes[ name ];
if ( ! imageSize ) {
Expand All @@ -1177,65 +1207,122 @@ export function generateThumbnails( id: QueueItemId ) {
continue;
}

// Build operations list for this thumbnail.
const thumbnailOperations: Operation[] = [
[ OperationType.ResizeCrop, { resize: imageSize } ],
];

// Add transcoding if format conversion is configured and
// the transparency check passed.
if ( thumbnailTranscodeOperation ) {
thumbnailOperations.push( thumbnailTranscodeOperation );
}

thumbnailOperations.push( OperationType.Upload );

dispatch.addSideloadItem( {
file,
onChange: ( [ updatedAttachment ] ) => {
// If the sub-size is still being generated, there is no need
// to invoke the callback below. It would just override
// the main image in the editor with the sub-size.
if ( isBlobURL( updatedAttachment.url ) ) {
return;
}

// This might be confusing, but the idea is to update the original
// image item in the editor with the new one with the added sub-size.
item.onChange?.( [ updatedAttachment ] );
},
batchId,
parentId: item.id,
additionalData: {
// Sideloading does not use the parent post ID but the
// attachment ID as the image sizes need to be added to it.
post: attachment.id,
image_size: name,
convert_format: false,
},
operations: thumbnailOperations,
batchConfigs.push( {
name,
resize: imageSize,
quality,
} );
}

// Create and sideload the scaled version.
// Check if a scaled version is needed.
const { bigImageSizeThreshold } = settings;
let needsScaling = false;
if ( bigImageSizeThreshold && attachment.id ) {
// Check if the image actually exceeds the threshold.
// Only create a scaled version for images larger than the threshold,
// matching WordPress core's wp_create_image_subsizes() behavior.
const bitmap = await createImageBitmap( item.sourceFile );
const needsScaling =
needsScaling =
bitmap.width > bigImageSizeThreshold ||
bitmap.height > bigImageSizeThreshold;
bitmap.close();

if ( needsScaling ) {
// Rename sourceFile to match the server attachment filename.
const sourceForScaled = attachment.filename
? renameFile( item.sourceFile, attachment.filename )
: item.sourceFile;
batchConfigs.push( {
name: 'scaled',
resize: {
width: bigImageSizeThreshold,
height: bigImageSizeThreshold,
},
quality,
scaledSuffix: true,
} );
}
}

// Batch resize: decode source once via copyMemory(),
// generate all sub-sizes with thumbnailImage(), and
// write each directly to the output format.
// Falls back to per-thumbnail processing on failure.
let batchResults: Awaited<
ReturnType< typeof vipsBatchResizeImage >
> | null = null;

if ( batchConfigs.length > 0 ) {
try {
batchResults = await vipsBatchResizeImage(
item.id,
file,
thumbnailOutputType,
batchConfigs,
false
);
} catch {
// eslint-disable-next-line no-console
console.warn(
'Batch resize failed, falling back to per-thumbnail processing'
);
}
}

if ( batchResults ) {
// Batch succeeded — enqueue upload-only sideloads.
for ( const result of batchResults ) {
dispatch.addSideloadItem( {
file: result.file,
onChange: ( [ updatedAttachment ] ) => {
if ( isBlobURL( updatedAttachment.url ) ) {
return;
}
item.onChange?.( [ updatedAttachment ] );
},
batchId,
parentId: item.id,
additionalData: {
post: attachment.id,
image_size: result.name,
convert_format: false,
},
operations: [ OperationType.Upload ],
} );
}
} else {
// Fallback: per-thumbnail processing (original approach
// without batch resize).
for ( const name of sizesToGenerate ) {
Comment on lines +1286 to +1289

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which cases would we expect the batch resize to fail?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if some parts of the batch fail, eg a corrupted image is uploaded? Makes me realize we should be sure to add more failure tests where we expect a certain type of failure (probably on the errror handling PR)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, interesting. Maybe not for this PR, then, but I'm curious if there's a case where batch will fail but per-thumbnail will succeed. My assumption (e.g. if there's a corrupt image) is that we'd bail entirely rather than trying another approach to procesing/uploading. But not a blocker to go with this!

Mostly my thinking was if this else condition isn't required then we could remove some code 😄

const imageSize = allImageSizes[ name ];
if ( ! imageSize ) {
continue;
}

const thumbnailOperations: Operation[] = [
[ OperationType.ResizeCrop, { resize: imageSize } ],
];

if ( thumbnailTranscodeOperation ) {
thumbnailOperations.push( thumbnailTranscodeOperation );
}

thumbnailOperations.push( OperationType.Upload );

dispatch.addSideloadItem( {
file,
onChange: ( [ updatedAttachment ] ) => {
if ( isBlobURL( updatedAttachment.url ) ) {
return;
}
item.onChange?.( [ updatedAttachment ] );
},
batchId,
parentId: item.id,
additionalData: {
post: attachment.id,
image_size: name,
convert_format: false,
},
operations: thumbnailOperations,
} );
}

// Add scaling to queue.
// Fallback scaled version.
if ( needsScaling && bigImageSizeThreshold && attachment.id ) {
const scaledOperations: Operation[] = [
[
OperationType.ResizeCrop,
Expand All @@ -1249,15 +1336,14 @@ export function generateThumbnails( id: QueueItemId ) {
],
];

// Add transcoding if format conversion is configured.
if ( thumbnailTranscodeOperation ) {
scaledOperations.push( thumbnailTranscodeOperation );
}

scaledOperations.push( OperationType.Upload );

dispatch.addSideloadItem( {
file: sourceForScaled,
file,
onChange: ( [ updatedAttachment ] ) => {
if ( isBlobURL( updatedAttachment.url ) ) {
return;
Expand Down
19 changes: 19 additions & 0 deletions packages/upload-media/src/store/test/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,25 @@ jest.mock( '../utils', () => ( {
} )
)
),
vipsBatchResizeImage: jest.fn( ( _id, file, outputType, configs ) =>
Promise.resolve(
configs.map(
( c: {
name: string;
resize: { width: number; height: number };
} ) => ( {
name: c.name,
file: new File(
[ 'batch-resized' ],
`example-${ c.resize.width }x${ c.resize.height }.${
outputType.split( '/' )[ 1 ]
}`,
{ type: outputType }
),
} )
)
)
),
vipsRotateImage: jest.fn(),
vipsHasTransparency: jest.fn( () => Promise.resolve( false ) ),
vipsConvertImageFormat: jest.fn(),
Expand Down
Loading
Loading