Skip to content
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.

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

x/image/draw: color space-correct interpolation #11420

Open
aclements opened this issue Jun 26, 2015 · 13 comments
Open

x/image/draw: color space-correct interpolation #11420

aclements opened this issue Jun 26, 2015 · 13 comments
Labels
Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@aclements
Copy link
Member

The BiLinear and CatmullRom interpolators assume the luminance of the source image's color space is linear. But the vast majority of images are in the sRGB color space, which has a highly non-linear luminance curve. As a result, scaled images can look quite different from the source image.

For example, on a reasonably calibrated monitor (and at 100% scaling in the browser), the left and right columns of the following image have the same average luminance:
gamma_colors
But scaling this down by a factor of 2 using x/image/draw yields the following:
out
Here, the two columns appear very different.

For an in-depth (possibly too in-depth) discussion of this problem and many test images, including the one I used above, see http://www.4p8.com/eric.brasseur/gamma.html. My test program is here: https://gist.github.com/aclements/599107a2e3f187f8a2c0.

A lot of software (including browsers) gets this wrong, and that's really unfortunate. It would be fantastic if Go software using x/image got this right out of the box.

Probably the best solution to this would be to thread color space information throughout the image library. At the other extreme, given the general recommendation to assume sRGB in the absence of other information (since virtually every image created in the past two decades is sRGB), it may make sense to simply assume sRGB when interpolating. We could also do the latter first and then later add more complete color space support, with the default being sRGB. Another option is to add this information to the x/image/draw.Options structure, though I fear that may interfere with later adding proper color space support to image.

/cc @nigeltao. (We discussed this a few weeks ago in person, but I figured I should open an issue so it doesn't get lost.)

@aclements aclements added the Suggested Issues that may be good for new contributors looking for work to do. label Jun 26, 2015
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jul 11, 2015
@ianlancetaylor
Copy link
Contributor

CC @nigeltao

@rselph
Copy link

rselph commented Dec 22, 2017

Just wanted to bump this issue. I'm working on creating test images from golang, and it's a constant headache having to do proper color space encoding myself. It's an issue that the vast majority of casual users are oblivious to, but one that affects image quality profoundly, as @aclements pointed out.

@nigeltao
Copy link
Contributor

nigeltao commented Jan 5, 2018

Yes, it's a valid feature request. No, I don't know anyone with all three of the time, expertise and inclination to work on it any time soon. Sorry.

@gopherbot
Copy link

Change https://golang.org/cl/253497 mentions this issue: x/image/draw: gamma corrected non linear interpolation

@eliasnaur
Copy link
Contributor

eliasnaur commented Dec 30, 2021

Note that this issue doesn't just affect scaling, but also blending; operations that involve image/draw.Over should also operate in linear space.

Also, I haven't seen a suggestion to fix this issue by simply introducing new types: image.SRGBA (and/or image.SNRGBA?), both defined as using the sRGB encoding. The reasons seem compelling:

  • Fully backwards compatible, in that image.RGBA/NRGBA retain their behaviour and performance characteristics.
  • Side-steps the need for a fully-fledged color space API. Arguably, sRGB is not even a separate color space as much as it is a particular encoding for the RGB color space.
  • sRGBA is only really useful for 8-bit color channels, so there won't be an explosion in image types.
  • Package image already have separate types for separate color spaces (e.g. image.CMYK).

@pjanx
Copy link

pjanx commented Dec 30, 2021

  • sRGB is a colour space of its own, given by its primaries and a white point, aside from being an encoding of it. There is no singular "RGB color space". Wikipedia suggests that the transfer function is also part of a colour space's definition.
  • sRGB is definitely useful with higher bit depths, if only because 8 bits cause very visible banding. Also, Display P3 uses the same transfer function, and could use the same treatment.
  • Note that CMYK is four-component and subtractive, major differences.

Another solution that hasn't been mentioned here yet is that you can create a simple adaptor around an arbitrary image.Image that un/applies the gamma-like transfer function in At/Set. Though it will presumably be very slow.

@eliasnaur
Copy link
Contributor

Another solution that hasn't been mentioned here yet is that you can create a simple adaptor around an arbitrary image.Image that un/applies the gamma-like transfer function in At/Set. Though it will presumably be very slow.

Indeed. Explicit image.SRGBA/SNRGBA types may be faster because they can be special-cased in Scale/Draw et al.

@eliasnaur
Copy link
Contributor

Another solution that hasn't been mentioned here yet is that you can create a simple adaptor around an arbitrary image.Image that un/applies the gamma-like transfer function in At/Set. Though it will presumably be very slow.

I implemented this as a proof of concept. Austin's program with sRGB wrappers.

@nigeltao
Copy link
Contributor

Nice proof of concept.

I'd be hesitant to check SRGBA and SNRGBA types into the stdlib (and maintain them forever) until we also have a good idea of how to handle "image.RGBA with Display P3" or "image.NRGBA with an arbitrary ICC color profile" in general. We might be missing out on image/draw and golang.org/x/image/draw special-case optimization, but colorspace.SRGBA doesn't need to live in the stdlib to do its job, for now.

If you want to take it beyond a proof of concept, I'd also try to implement the image.RGBA64Image and draw.RGBA64Image interfaces (added to the stdlib this year in Go 1.17). This should speed things up (avoiding the per-pixel color.Color allocations) once golang.org/x/image/draw gets new code to recognize them (doing that is on my long TODO list, but it'd also be easier once golang.org/x can drop pre-Go-1.17 support), similar to what https://go-review.googlesource.com/c/go/+/340049 and https://go-review.googlesource.com/c/go/+/351852 did for the stdlib (not x) image/draw.

@nigeltao
Copy link
Contributor

nigeltao commented Dec 30, 2021

I'd be hesitant to check SRGBA and SNRGBA types into the stdlib (and maintain them forever)

To elaborate... as aclements said, to do the stdlib API design properly, we'd need to think about whether color spaces are best associated with the image.Image type per se or are instead decoupled and travel around in some sort of Options struct-typed arg for the various functions (e.g. golang.org/x/image/draw functions but also JPEG / PNG / etc decode and encode).

Any new stdlib API should ideally still allow us to eventually have an "Image with general metadata" representation, not just "Image with color profile".

For example, if I Decode two images (one is SRGB PNG, the other is Display P3 WebP), Draw one over the other and Encode the result, how does the API make it all work? What if that WebP image also had EXIF metadata (orientation) for a 90 degree rotation (see #4341)?

This is obviously a non-trivial design problem. #33457 and https://go-review.googlesource.com/c/go/+/208559 and https://go-review.googlesource.com/c/go/+/216799 is an "image metadata API" discussion. #27830 is a related "image decoding options API" discussion.

@klauspost
Copy link
Contributor

I agree with Nigel here. Images does not know anything about the colorspace, which is the underlying problem.

The PoC assumes that the image is sRGB and gamma corrected. Certainly some parts (YCbCr for example) confirms this, but I am not so sure PNG input converts everything to this. Even if it does, it is an extremely limited colorspace, which will truncate nearly all other input colorspaces.

The wrapper is fine for interpolation, but for 16 bit inputs, you would loose maybe an unreasonable amount of precision. The thing with linear vs gamma corrected is that is isn't uniformly better to use linear. For instance applying "contrast" or denoising operations work much better in a gamma corrected space, since they will correspond much better with the visual expectations.

I think the basic stdlib "image" is a fine toybox, but maybe x/image should have an extended API, that allows specifying image gamma curve, a matrix for converting to or from XYZ (the colorspace) at a specific whitepoint (D65 for example).

The GetAt/SetAt is fine for a toy stdlib interface, but not really good for serious use. I would much rather see an interface that allows controlled full-image conversion to a known destination, or (with the information available) is able to do the conversion inline while performing operations on the input.

Subsampled images (YCbCr 4:2:0) operates badly with GetAt/SetAt interface as well, but again good enough for toying around. Subsampling on an API level is not really worth it, unless you specifically are working with videos. Similarly you could have CFA information, but it is also a big complication that shouldn't be part of an API, ie the readers could convert to planar grey images or non-subsampled 4:4:4 representation that is converted back on save.

@pjanx
Copy link

pjanx commented Dec 31, 2021

The generally valid assumption is that everything you meet is sRGB, or acts like sRGB (Display P3, essentially all iPhone photos since 2017). The PoC works for both of these very widely used colour spaces, and is optional. The current state of things works correctly for essentially nothing.

Yes, this is toy advancement. In the theme of "a bird in the hand is worth two in the bush".

Go does not have any Color Management Module at all, as far as I'm aware, much less complete PNG format handling. When converting between gamuts, you need to choose an intent.

@eliasnaur
Copy link
Contributor

If you want to take it beyond a proof of concept, I'd also try to implement the image.RGBA64Image and draw.RGBA64Image interfaces (added to the stdlib this year in Go 1.17). This should speed things up (avoiding the per-pixel color.Color allocations) once golang.org/x/image/draw gets new code to recognize them (doing that is on my long TODO list, but it'd also be easier once golang.org/x can drop pre-Go-1.17 support), similar to what https://go-review.googlesource.com/c/go/+/340049 and https://go-review.googlesource.com/c/go/+/351852 did for the stdlib (not x) image/draw.

Great suggestion, done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

8 participants