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 71 commits into
base: libvips
Choose a base branch
from
Draft

Libvips cropping #16

tonytw1 wants to merge 71 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.

Aims to only load the image once.
The image processing operations happen in memory rather than forking out to shell commands.

Nudges the Crops.cropType crop file type decision towards a more deterministic set of parameters.

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.*")

IPTC tagging

Moves to to the XMP block with the XMP block been built in memory.

Performance

Some marginal reductions on cropping time and predictable memory use.
These test images can be cropped by 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: 
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:
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 25 commits February 8, 2026 21:08
@tonytw1
isGraphic was sendint all TIFFs to PNG.
9155748 
TODO restore this decision.
@tonytw1
Correct CMYK renders too light in crops.
f1468f4
@tonytw1
Match depth between icc_transform and pngsave for visble saves.
fe3bcfd 
TODO jpeg?
@tonytw1
Extract getDepthFor function to infer the depth of a VImage.
c860245 
INTERPRETATION_RGB16 is 16 bit.
@tonytw1
palette and bitdepth should be mutually exclusive.
2f2ec70
@tonytw1
Unused parameters.
c20c269
@tonytw1
Unused parameters.
b25acc4
@tonytw1
Bypass icc_transform for LAB images.
b331a90
@tonytw1
Remove non used non vips functions.
a6d88a0
@tonytw1
Remove non used runConvert command.
244f860
@tonytw1
Remove unused iccColourSpace val.
ed2f583
@tonytw1
Reapply png master quality.
4bc0bbd
@tonytw1
CropType decision can be deferred until after the master crop has bee…
41a1dfa 
…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 …
ab09a15 
…can be made at a more implementation specific level.
@tonytw1
Spike. vips based implementation of isGraphic? is likely to involve t…
1232958 
…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…
2d42c34 
…; 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.
04c9903
@tonytw1
Crop quality values can be int.
7df12e4
@tonytw1
Test to exercise ImageOperations resize.
3bcf188
@tonytw1
Clarify master crop quality values.
8408a9d
@tonytw1
[libvips-cropping] Allow embedded icc profile to be used in crop icc …
b2a7563 
…transform.
@tonytw1
[libvips-cropping] Test name.
aa9c28e
@tonytw1
[libvips-cropping] Test to exercise create thumbnail.
69f00ea
@tonytw1
[libvips-cropping] Image operations resize takes the output file as a…
740266c 
… parameter for easier testing.
@tonytw1
[libvips-cropping] Cropping of LAB fixed by making non icc_transform …
c9027cf 
…decision based on no icc profile seen.

Nice and clean; cribbed from vips thumbnail source code.
@tonytw1
vips-ffm 1.9.5.
3b9c410
@tonytw1
[libvips-cropping] Show that it is possible to manually trim metadata…
8d415ed 
… without relying in strip on save.

Leads to been able to set metadata on crops.
If not removed, orientation will express itself on save if not removed even though it's not in a exif prefixed field.
@tonytw1
[libvips-cropping] Bake the credit, copyright and supplier transmissi…
ce06adf 
…on reference into the XMP using IPTC metadata fields.

Explicitly use compact XMP format.
IPTC has moved from the app13 block into XMP.
@tonytw1
[libvips-cropping] While manually stripping metadata might be useful,…
6411aa7 
… vips strip with keep for XMP feels safer.
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.

1 participant

@tonytw1