Designwise, I think the main question is whether this extra gainmap processing code should go into each operation as I've done here, or whether it should all be in thumbnail.

Isolated in thumbnail

This feels cleaner and easier to understand, but it would be more work for downstream libraries like sharp.

In each operation

This would be much simpler for downstream libvips users -- potentially, they'd get working uhdr support for free.

However ... we'd be adding strange extra code to only some of the libvips operations, we'd need to document which ones did this extra work, and it'd be harder to make libvips do something different if you wanted to handle gainmaps yourself.

Analogy with other image features

Other image formats have extra metadata. For example, formats like GIF attach a palette on load and can use that palette again on save. libvips does not try to update the palette as it processes images, that's all left to downstream code. Likewise, JPEG can attach a thumbnail image and we don't try to update that during processing.

@kleisauke do you have an opinion on this? I'm starting to think we should have this extra gainmap code in thumbnail.

The code in question is pretty small. Crop has this:

https://github.com/libvips/libvips/pull/4709/files#diff-4db5e821a2fd0a9b9d0ee442b87e9f30e2e94e696262fd00006d4a6d4a3389c4R163-R180

And resize has this:

https://github.com/libvips/libvips/pull/4709/files#diff-e291fde7901012404ef4b8f1f475da90fba762c1508d320ae15ac8e5ec56f10dR304-R316

It's not that much more code for downstream libs.

I talked myself around heh. I moved the gainmap handling into thumbnail, so rot / flip / crop / resize are back to doing no extra magic.

This means downstream libraries that don't use thumbnail will need to add some extra code.

It's not a great deal of extra stuff. The change was:

Updating 44721eaec..42fb80237
Fast-forward
 libvips/conversion/extract.c | 20 --------------------
 libvips/conversion/flip.c    | 13 -------------
 libvips/conversion/rot.c     | 14 --------------
 libvips/resample/resize.c    | 14 --------------
 libvips/resample/thumbnail.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 42 insertions(+), 61 deletions(-)

So 60 lines come out of the operators, 40 go into thumbnail.

The extra code is pretty simple:

    if (vips_resize(in, &t[5], 1.0 / hshrink, "vscale", 1.0 / vshrink, NULL))
        return -1;
    in = t[5];
    
    /* Also process the gainmap, if any.
     */
    if ((gainmap = vips_image_get_gainmap(in))) {
        if (vips_resize(gainmap, &t[15], 1.0 / hshrink,
            "vscale", 1.0 / vshrink,
            "kernel", VIPS_KERNEL_LINEAR,
            NULL))
            return -1;
            
        vips_image_set_image(in, "gainmap", t[15]);
    }

So after calling some vips function that will change the image geometry (resize, crop, rotate, insert, flip, etc) you also need to fetch the optional gainmap and transform that in the same way.

We either put this extra code in the operators themselves, or we leave it for higher levels (thumbnail in this case, sharp is another obvious one) to handle. Which solution is better?

I tried cropping in python to see what it was like:

#!/usr/bin/env python3

import sys
import pyvips

def get_gainmap(image):
    if image.get_typeof("gainmap") != 0:
        return image.get("gainmap")
    elif image.get_typeof("gainmap-data") != 0:
        buf = image.get("gainmap-data")  
        gainmap = pyvips.Image.jpegload_buffer(buf)
        image.set_type(pyvips.GValue.image_type, "gainmap", gainmap)
        return gainmap
    else:
        return None

left = int(sys.argv[3])
top = int(sys.argv[4])
width = int(sys.argv[5])
height = int(sys.argv[6])

image = pyvips.Image.uhdrload(sys.argv[1])

image = image.copy()
gainmap = get_gainmap(image)
if gainmap != None:
    hscale = gainmap.width / image.width
    vscale = gainmap.width / image.width

# crop image
image = image.crop(left, top, width, height)

# crop gainmap, if any
gainmap = get_gainmap(image)
if gainmap != None:
    gainmap = gainmap.crop(left * hscale, top * vscale,
                           width * hscale, height * vscale)
    image.set("gainmap", gainmap)

image.uhdrsave(sys.argv[2])

We'd add get_gainmap() to pyvips, so that wouldn't be needed, but the remaining code is still quite tricky :( I think downstream users would not always get this right.

Perhaps this should be moved back into the operators again? At least we'd have one implementation in one place that we could check.

@lovell, what do you think? This would have a big effect on sharp.

... an easy way to test is with eg.:

$ vipsthumbnail ultra-hdr.jpg -o x.jpg --smartcrop centre --size 512x512
$ vipsheader -f gainmap-data x.jpg | base64 -d > gainmap.jpg

To thumbnail and then extract the transformed gainmap, then:

$ vipsdisp x.jpg gainmap.jpg 

And use alt-right and alt-left to flip between the image and the gainmap.