Navigation Menu

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

image/color: NRGBA provides alpha-premultiplied values, and RGBA provides non-alpha-premultiplied values. The doc claims the exact opposite. #55327

Closed
ScienceMarc opened this issue Sep 21, 2022 · 5 comments

Comments

@ScienceMarc
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.19 linux/amd64

Does this issue reproduce with the latest release?

Yes. This issue has been present in every version since weekly.2011-10-06.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/marc/.cache/go-build"
GOENV="/home/marc/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/marc/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/marc/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3829980571=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://go.dev/play/p/ckINKpbqQ2L

What did you expect to see?

This should be alpha-premultiplied (0 0 0 0): 0 0 0 0
This should be alpha-premultiplied (0 0 0 0): 0 0 0 0

This shouldn't be alpha-premultiplied (0 32639 65535 0): 0 32639 65535 0
This shouldn't be alpha-premultiplied (0 2147483647 4294967295 0): 0 2147483647 4294967295 0

What did you see instead?

This should be alpha-premultiplied (0 0 0 0): 0 32639 65535 0
This should be alpha-premultiplied (0 0 0 0): 0 32767 65535 0

This shouldn't be alpha-premultiplied (0 32639 65535 0): 0 0 0 0
This shouldn't be alpha-premultiplied (0 2147483647 4294967295 0): 0 0 0 0


I may be completely misinterpreting the documentation, or even misinterpreting the concept of alpha-premultiplication, but based on the Wikipedia page for Alpha compositing the current behavior is inverted.

@ScienceMarc
Copy link
Author

Also, the 64bit color.RGBA64 and color.NRGB64 return uint32's. This does not feel correct and is not what I expected. However, the documentation does not specify the range of color.RGB64() so this may be the intended behavior. Either way, I think the documentation could be made clearer.
Would I need to start a new issue for this problem specifically?

@cherrymui
Copy link
Member

I think you indeed misinterpreted the documentation. For RGBA, the pre-multiplication means that the R, G, B components in the struct are already multiplied with A, so it doesn't need to multiply again. For NRGBA, it is the opposite, i.e. the components in the struct is not already multiplied, so it needs to be multiplied.

Also, see https://pkg.go.dev/image/color#RGBA64

// An alpha-premultiplied color component c has been scaled by alpha (a),
// so has valid values 0 <= c <= a.

So color.RGBA{0, 0x7F, 0xFF, 0} is not actually valid.

For your second comment, the RGBA method satisfies the https://pkg.go.dev/image/color#Color interface. Note that for RGBA64 and NRGBA64, the total bits of R+G+B+A is 64, not each individual component is 64-bit, so returning uint32 is still enough.

Closing as working as intended. Thanks.

@cherrymui cherrymui closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2022
@ScienceMarc
Copy link
Author

Thanks, I had already started working all of this out in the time after submitting this. I still feel like something could be changed to make it harder to accidentally create an invalid struct. Maybe it'd be worth it to add a NewColor function that takes straight r,g,b,a values and returns a valid color.RGBA object, doing the multiplication for the developer.

Otherwise, developers will keep making their own structs manually which often results in incorrect behavior. But, it could be argued that it's up to the developers to avoid making this kind of mistake.

@cherrymui
Copy link
Member

Since https://pkg.go.dev/image/color#RGBA already documents that

An alpha-premultiplied color component C has been scaled by alpha (A), so has valid values 0 <= C <= A.

I think it is expected that the user would not create invalid values. For a NewColor (or perhaps NewRGBA) function, it is unclear what to do if the given input is invalid. Panic? Maybe, but doesn't sound much better than just documenting the requirement. So maybe not worth adding a new API. Thanks.

@Zyl9393
Copy link

Zyl9393 commented Sep 28, 2022

Commenting here since this currently is the first Google result for golang nrgba premultiplication.

All the handling of alpha values by the standard library is a bit funky. The premultiplication done by method RGBA() of type color.NRGBA occurs without any color space conversions, which while that isn't in conflict with the documentation, is not usually what you would want. The alpha value generally indicates amount of coverage over the pixel below it, whatever it is going to be, so it must not be biased towards either end of the luminosity spectrum and thus needs to be applied in linear RGB. However, when I use the png package to load a PNG with an alpha channel, I get the unaltered values as NRGBA color model, meaning NRGBA is then storing sRGB data. I had been confused for the past hour or so why an image of mine with transparency had dark outlines when magnifying interpolation is happening, while the non-premultiplied equivalent looked fine, and I confirmed this to be the cause. As I said, considering the documentation, this is not a bug (there is no indication that NRGBA is in any color space of any sort), it's just a subtle design detail that I did not expect, and I think other developers should be aware of as well.

@golang golang locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants