libvips / pyvips

python binding for libvips using cffi
MIT License
649 stars 50 forks source link

Unable to convert multiband image to srgb #478

Closed TWCCarlson closed 5 months ago

TWCCarlson commented 5 months ago

Hello,

I've been playing with styling of images, namely brightness and contrast. I've found that it mostly works correctly and gives good results except for one particular case in which the result is rendered unusable.

Here are the two images I'm working with to reproduce the problem (though it has occurred for every png image so far):

0_39_48 0_40_48

To replicate the problem I've made this minimal script:

import os

# Windows binaries are required: https://pypi.org/project/pyvips/, https://www.libvips.org/install.html
LIBVIPS_VERSION = "8.15"
vipsbin = os.path.join(os.getcwd(), f"vipsbin/vips-dev-{LIBVIPS_VERSION}/bin")
os.environ['PATH'] = os.pathsep.join((vipsbin, os.environ['PATH']))
import pyvips as pv

blank = pv.Image.black(256, 256, bands=3)

basetile1 = f"./osrs-wiki-maps/out/mapgen/versions/2024-05-29_a/tiles/base/2/0_33_80.png"
basetile1 = pv.Image.new_from_file(basetile1)

basetile2 = f"./osrs-wiki-maps/out/mapgen/versions/2024-05-29_a/tiles/base/2/0_33_79.png"
basetile2 = pv.Image.new_from_file(basetile2)

baseImage = pv.Image.arrayjoin([basetile1, basetile2, blank, blank], across=4)

brightness = 0
contrast = 1.0

baseImage = contrast * (baseImage - 127) + (127 + brightness)
baseImage.write_to_file(f"debug.png")

For these settings the tiles should be unaltered compared to the original images, and be output as a row of tiles with blanks in the last two positions. And that's what I get:

image

If instead I change the tile order to:

baseImage = pv.Image.arrayjoin([blank, basetile1, basetile2, blank], across=4)

I get:

image

When I load the tile images from file they are assigned the srgb colorspace. The pv.Image.black() tiles are assigned multiband instead.

Printing the images after array joining showed me that arrayjoin seems to take the colorspace of the first image in the list and use that as the output colorspace:

print(blank)
print(basetile1)
print(basetile2)
baseImage = pv.Image.arrayjoin([basetile1, basetile2, blank, blank], across=4)
print(baseImage)

>>> <pyvips.Image 256x256 uchar, 3 bands, multiband>
>>> <pyvips.Image 256x256 uchar, 3 bands, srgb>
>>> <pyvips.Image 256x256 uchar, 3 bands, srgb>
>>> <pyvips.Image 1024x256 uchar, 3 bands, srgb>
print(blank)
print(basetile1)
print(basetile2)
baseImage = pv.Image.arrayjoin([blank, basetile1, basetile2, blank], across=4)
print(baseImage)

>>> <pyvips.Image 256x256 uchar, 3 bands, multiband>
>>> <pyvips.Image 256x256 uchar, 3 bands, srgb>
>>> <pyvips.Image 256x256 uchar, 3 bands, srgb>
>>> <pyvips.Image 1024x256 uchar, 3 bands, multiband>

So I figured I could simply convert the colorspace of the output whenever I put a multiband image in the first position. But it doesn't seem to work:

print(baseImage)
baseImage = baseImage.colourspace("srgb")
print(baseImage)

>>> <pyvips.Image 1024x256 uchar, 3 bands, multiband>
>>> <pyvips.Image 1024x256 uchar, 3 bands, multiband>

But if I convert to some other colorspace (I tried hsv, lab, and lch) than srgb first, it does:

print(baseImage)
baseImage = baseImage.colourspace("lab").colourspace("srgb")
print(baseImage)

>>> <pyvips.Image 1024x256 uchar, 3 bands, multiband>
>>> <pyvips.Image 1024x256 uchar, 3 bands, srgb>

image

On the other hand, trying to convert from the multiband output to rgb gives:

print(baseImage)
baseImage = baseImage.colourspace("rgb")
print(baseImage)

>>> pyvips.error.Error: unable to call colourspace
            vips_colourspace: no known route from 'srgb' to 'rgb'

So I guess the multiband image thinks it is already in srgb which explains why .colourspace("srgb") isn't doing anything for me? I also tried splitting the bands without converting the colorspace and got the same results. Have I overlooked something?

TWCCarlson commented 5 months ago

Forgot to add, I replicated this process in nip2 and it had no problems: image

jcupitt commented 5 months ago

Hi again,

Just like numpy, libvips images are huge uninterpreted 3D arrays, with a hint attached to them (called interpretation) suggesting how the numeric values should be converted into colour. But most operations ignore it -- the interpretation hint is only used by loaders, savers, and the colourspace operator (I think).

black makes a multiband image (the most generic interpretation), because it doesn't know what you want. You set the interpretation hint with eg.

blank = pyvips.Image.black(bands=3).copy(interpretation="srgb")

This applies to alphas as well -- an RGBA PNG is a 4-band image with the srgb tag. If you do:

baseImage = contrast * (baseImage - 127) + (127 + brightness)

You're also modifying the A band (if there is one), which might not be what you want.

jcupitt commented 5 months ago

... you can either split off the alpha and reattach it later:

alpha = image[3] if image.hasalpha() else nil
rgb = image[0:3]
rgb = rgb * scale + offset
image = rgb.bandjoin(alpha) if alpha else rgb

Or use array constants:

rgb = rgb * [scale, scale, scale, 1] + [offset, offset, offset, 0]

ie. don't touch alpha. It depends a bit on the context.

jcupitt commented 5 months ago

And nice job with nip2, by the way, heh.

TWCCarlson commented 5 months ago

Just like numpy, libvips images are huge uninterpreted 3D arrays, with a hint attached to them (called interpretation) suggesting how the numeric values should be converted into colour. But most operations ignore it -- the interpretation hint is only used by loaders, savers, and the colourspace operator (I think).

I think this is how I understood it; maybe some of the implications are slipping past me. I thought that if I used the colourspace operator to convert to srgb I would achieve the same thing you suggest with .copy(interpretation=srgb), only done in-place instead of creating a deep copy of the image. Is it expected that this doesn't work, then?

You're also modifying the A band (if there is one), which might not be what you want.

Fortunately I'm assured that there isn't one at this stage. Although I should consider the implications of a future maintainer changing that...

And nice job with nip2, by the way, heh.

It's extremely useful to try things out without running a script over and over 😁 Although I am still confused, as I explicitly told nip2 to produce a black multiband image, I would have thought the outcome would be identical to what I did in the script.

jcupitt commented 5 months ago

I thought that if I used the colourspace operator to convert to srgb I would achieve the same thing you suggest with .copy(interpretation=srgb), only done in-place instead of creating a deep copy of the image. Is it expected that this doesn't work, then?

They are two different things.

.copy(interpretation="xxx") copies the image exactly and just changes the interpretation hint. Pixels are copied with a just a pointer copy, so it's instant, no matter how large the image is. If you had pixels [0, 0, 0] before, this is what you'll have after too.

.colourspace("yyy") is something like a meta operator. It looks at the current interpretation hint, looks at the interpretation you've asked for, then picks the best sequence of colour conversions to use to get from the source to the destination. For example, if the image is tagged as srgb and you ask for lch, it'll do image.sRGB2scRGB().scRGB2XYZ().XYZ2Lab().Lab2LCh().

The description section in the colour docs explains the background:

https://www.libvips.org/API/current/libvips-colour.html#libvips-colour.description

Though the useful diagram hasn't been generated, annoyingly.

I explicitly told nip2 to produce a black multiband image, I would have thought the outcome would be identical to what I did in the script.

nip2 is still a vips7 program and the rules around tagging were slightly different. The vips8 rules are supposed to be simpler and more consistent.

TWCCarlson commented 5 months ago

Interesting; then the problem is that vips isn't sure what I want a value in a generic multiband image to be interpreted as when I write it to a file?

nip2 is still a vips7 program and the rules around tagging were slightly different. The vips8 rules are supposed to be simpler and more consistent.

Ahh, okay.

jcupitt commented 5 months ago

Yes, vips7 tried to set and guess interpretation for you to cover the most common cases, but while it often helped, there were a large number of edges where it guessed wrongly and caused intense confusion or extra work.

For example, black used to set different values for interpretation for different numbers of bands, but that meant that when using black you could never be quite sure which one it had chosen, so you ended up setting it anyway. Savers would do more guessing about what interpretation should be, but different savers had to have different rules, because image formats have different conventions (eg. TIFF allows more than one alpha), and that made behaviour complex and hard to predict.

vips8 is always multiband for things like black, and all savers use the same rules based on requiring interpretation to be set correctly rather than trying to guess. vips8 behaviour is supposed to be more predictable.

TWCCarlson commented 5 months ago

I see! Thank you as always for explaining.