Skip to content

Libvips cropping #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
tonytw1 wants to merge 56 commits into
base: libvips
Choose a base branch
from
Draft

Libvips cropping #16

tonytw1 wants to merge 56 commits into from

Conversation

@tonytw1
Copy link
Member

@tonytw1 tonytw1 commented Jul 27, 2025 •
edited
Loading

What does this change?

Cropping is implemented using libvips with the image processing operations happening in memory rather than forking out to shell commands.

isGraphic decision for crop type is inferred from the number of bands in the source image.
Do not know how to implement this colour depth query in libvibs.

 // val isGraphic = !colourType.matches("True[ ]?Color.*")

Seems to yield noticeably more predictable timings for large TIFF files.
Can crop these test images while running in a container with 2Gb of memory.

Time for "POST /crops to return after making a fullset of 5:3 full frame crops of various test images.

imagemagick: 
7360x4912 image/tiff 64676976: 24068ms !!!
3301x2173 image/jpeg 1324968 : 1290ms
3500x2334 image/jpeg 2492334:  2050ms
4165x3407 image/png 26741926 : 8904ms
8640x5760 image/jpeg 27134591: 5279ms
5000x3333 image/jpeg 8674990: 2521ms

vips:
7360x4912 image/tiff 64676976 : 3982ms
3301x2173 image/jpeg 1324968 : 1127ms
3500x2334 image/jpeg 2492334 : 1307ms
4165x3407 image/png 26741926 : 5036ms
8640x5760 image/jpeg 27134591 : 4001ms
5000x3333 image/jpeg 8674990 : 1619ms

How should a reviewer test this change?

How can success be measured?

Who should look at this?

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

@tonytw1 tonytw1 force-pushed the libvips branch 2 times, most recently from c8f8217 to c96ff82 Compare August 11, 2025 19:06
@tonytw1 tonytw1 force-pushed the libvips branch 4 times, most recently from d296629 to 25c0148 Compare September 28, 2025 10:12
@tonytw1 tonytw1 force-pushed the libvips-cropping branch 2 times, most recently from 5997767 to 7ced3c4 Compare September 28, 2025 11:22
@tonytw1 tonytw1 force-pushed the libvips-cropping branch 6 times, most recently from c916363 to dcd3779 Compare December 30, 2025 11:08
tonytw1 added 29 commits January 26, 2026 09:04
@tonytw1
Refactor. S3 store of master crop pushes up. MasterCrop is a simpler …
1561201 
…opject with no Futures in it.

Grouping the vips images operations in 1 tight block with no eventual side effects.
@tonytw1
Refactor. File create for mastercrop pushes up to be next to the s3 s…
a490435 
…tore.
@tonytw1
Master file create order; before arena close.
083fd0a
@tonytw1
Comment; master crop if different from the the full dimensions crop; …
91da546 
…it's higher quality for CDN resizer origin.
@tonytw1
Refactor; split creation of crop files from publishing to S3.
e571e52
@tonytw1
Palettise PNG crops.
cc1c031
@tonytw1
Do not quantise master crop; only the resizes.
2df15d3
@tonytw1
isGraphic was sendint all TIFFs to PNG.
51399f1 
TODO restore this decision.
@tonytw1
isGraphic was sendint all TIFFs to PNG.
f0e188c 
TODO restore this decision.
@tonytw1
Correct CMYK renders too light in crops.
8b23bed
@tonytw1
Match depth between icc_transform and pngsave for visble saves.
ad5a420 
TODO jpeg?
@tonytw1
Extract getDepthFor function to infer the depth of a VImage.
a91323c
@tonytw1
INTERPRETATION_RGB16 is 16 bit.
074a7d6
@tonytw1
palette and bitdepth should be mutually exclusive.
d6a5141
@tonytw1
Unused parameters.
4d21018
@tonytw1
Unused parameters.
ac9c4ef
@tonytw1
Bypass icc_transform for LAB images.
a6ad060
@tonytw1
Remove non used non vips functions.
3af1815
@tonytw1
Remove non used runConvert command.
9667f1c
@tonytw1
Remove unused iccColourSpace val.
11e8417
@tonytw1
Reapply png master quality.
654408d
@tonytw1
CropType decision can be deferred until after the master crop has bee…
086f80a 
…n loaded.

So that hasAlpha and colour depth can possibly come from the actual master crop image.
@tonytw1
Refactor. Push the isGraphic? decision up out of cropType so that is …
754cfd9 
…can be made at a more implementation specific level.
@tonytw1
Spike. vips based implementation of isGraphic? is likely to involve t…
0db6cbf 
…he number of bands in the image.

image_get_typeof palette as an indication that the image had a palette is another good signal for isGraphic.
format and bands are likely to be required to infer colour depth.
@tonytw1
VIPS enabled image loader and cropper can probably use a smaller heap…
48a5450 
…; leaving more RAM for off heap VIPS operations.

Cropped 20% heap.
@tonytw1
png Q option is not needed if we are not actually quantising the image.
0994c8f
@tonytw1
Crop quality values can be int.
1a1a582
@tonytw1
Test to exercise ImageOperations resize.
ce4886d
@tonytw1
Clarify master crop quality values.
8454725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tonytw1