image/draw: Possible precision loss for NRGBA images

[nvinuesa]

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

$ go version
go version go1.18 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/nicolas/.cache/go-build"
GOENV="/home/nicolas/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/nicolas/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/nicolas/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2624768179=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I simply created an NRGBA image with a single pixel (with a small value color (1 in uint8) and an alpha less than 0.5 (127 in uint8)), and tried to copy it using it as source of draw.Draw():

package main

import (
	"image"
	"image/color"
	"image/draw"
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestImgDrawNRGBA(t *testing.T) {
	pixel := color.NRGBA{1, 0, 0, 127}
	expected := image.NewNRGBA(image.Rect(0, 0, 1, 1))
	expected.SetNRGBA(0, 0, pixel)

	actual := image.NewNRGBA(expected.Rect)
	draw.Draw(actual, expected.Rect, expected, image.Point{0, 0}, draw.Src)
	assert.Equal(t, expected, actual)
}

Go playground link: https://go.dev/play/p/IYelc0epkM7

What did you expect to see?

I expected the test to pass because this test passes using go-1.17.8 (Go playground link: https://go.dev/play/p/IYelc0epkM7?v=goprev).

What did you see instead?

Instead I saw a difference between the original image (expected) and the actual one (drawn using draw.Draw()):

Diff:
--- Expected
+++ Actual
@@ -2,3 +2,3 @@
	Pix: ([]uint8) (len=4) {
-  00000000  01 00 00 7f                                       |....|
+  00000000  00 00 00 7f                                       |....|
	},

Even though this is a non-alpha-premultiplied image, it looks like a possible precision loss due to the value of the alpha channel: if I use an alpha higher or equal than 0.5 (128 in uint8) the test passes.

[nvinuesa]

After some investigation, I found that when the operation is draw.Src (image copy) in https://github.com/golang/go/blob/master/src/image/draw/draw.go#L251 the destination image is written using .SetRGBA64():

dst0.SetRGBA64(x, y, src0.RGBA64At(sx, sy))

regardless the image type, which in our case is NRGBA.

First, the RGBA64 color is retrieved from the source image, therefore each channel is multiplied by the alpha channel.

Then, the retrieved RGBA64 color is passed to (p *NRGBA) SetRGBA64(x, y int, c color.RGBA64), and therefore the alpha channel needs to be de-multiplied (this is done on https://github.com/golang/go/blob/master/src/image/image.go#L394).

After having each channel de-multiplied, a shift is performed in order to go from uint16 (of color.RGBA64) to uint8.

These steps seem to be responsible for a possible precision loss.

[mknyszek]

There seems to have been some movement in the image/draw package in 1.18, so my first instinct is that this is probably a regression.

CC @nigeltao

[gopherbot]

Change https://go.dev/cl/396795 mentions this issue: image/draw: have draw.Src preserve NRGBA colors