x/exp/shiny/iconvg: crash when rasterizing icons with translucent palette

[eliasnaur]

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

$ go version
go version devel +3f30e0ee17 Wed Jun 10 21:25:14 2020 +0200 linux/amd64

Does this issue reproduce with the latest release?

Yes (as of golang/exp@0022984)

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/elias/.cache/go-build"
GOENV="/home/elias/.config/go/env"
GOEXE=""
GOFLAGS="-mod=readonly"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/elias/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/elias/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/elias/dev/go-tip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/elias/dev/go-tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/elias/go/src/golang.org/x/exp/go.mod"
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-build806979195=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Applied

diff --git shiny/iconvg/decode_test.go shiny/iconvg/decode_test.go
index fae6ca7..b752c99 100644
--- shiny/iconvg/decode_test.go
+++ shiny/iconvg/decode_test.go
@@ -131,6 +131,7 @@ var testdataTestCases = []struct {
        variants string
 }{
        {"testdata/action-info.lores", ""},
+       {"testdata/action-info.lores", ";translucent"},
        {"testdata/action-info.hires", ""},
        {"testdata/arcs", ""},
        {"testdata/blank", ""},
@@ -199,10 +200,15 @@ func TestRasterizer(t *testing.T) {
                        }
 
                        opts := &DecodeOptions{}
-                       if variant == "pink" {
+                       switch variant {
+                       case "pink":
                                pal := DefaultPalette
                                pal[0] = color.RGBA{0xfe, 0x76, 0xea, 0xff}
                                opts.Palette = &pal
+                       case "translucent":
+                               pal := DefaultPalette
+                               pal[0] = color.RGBA{0xfe, 0x76, 0xea, 0xf0}
+                               opts.Palette = &pal
                        }
 
                        got := image.NewRGBA(image.Rect(0, 0, width, height))

and ran go test

What did you expect to see?

An error because of the missing testdata reference image.

What did you see instead?

$ go test
--- FAIL: TestRasterizer (0.01s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x51880b]

goroutine 29 [running]:
testing.tRunner.func1.1(0x5566e0, 0x676560)
	/home/elias/dev/go-tip/src/testing/testing.go:1058 +0x30d
testing.tRunner.func1(0xc00009d500)
	/home/elias/dev/go-tip/src/testing/testing.go:1061 +0x41a
panic(0x5566e0, 0x676560)
	/home/elias/dev/go-tip/src/runtime/panic.go:969 +0x175
golang.org/x/image/vector.(*Rasterizer).rasterizeOpSrc(0xc000078000, 0x5b0f00, 0xc0004d4000, 0x0, 0x0, 0x100, 0x100, 0x0, 0x0, 0x0, ...)
	/home/elias/go/src/golang.org/x/image/vector/vector.go:459 +0x18b
golang.org/x/image/vector.(*Rasterizer).Draw(0xc000078000, 0x5b0f00, 0xc0004d4000, 0x0, 0x0, 0x100, 0x100, 0x0, 0x0, 0x0, ...)
	/home/elias/go/src/golang.org/x/image/vector/vector.go:298 +0x15b
golang.org/x/exp/shiny/iconvg.(*Rasterizer).ClosePathEndPath(0xc000078000)
	/home/elias/go/src/golang.org/x/exp/shiny/iconvg/rasterizer.go:263 +0xc5
golang.org/x/exp/shiny/iconvg.decodeDrawing(0x5b1d40, 0xc000078000, 0x0, 0xc0000c693f, 0x1, 0x201, 0x58a790, 0xc0000c693e, 0x1, 0x201, ...)
	/home/elias/go/src/golang.org/x/exp/shiny/iconvg/decode.go:551 +0x102a
golang.org/x/exp/shiny/iconvg.decode(0x5b1d40, 0xc000078000, 0x0, 0xc0000d3ad0, 0x4000, 0xc0000c6904, 0x3b, 0x23b, 0xc0000d3ee0, 0x7, ...)
	/home/elias/go/src/golang.org/x/exp/shiny/iconvg/decode.go:134 +0x3c2
golang.org/x/exp/shiny/iconvg.Decode(0x5b1d40, 0xc000078000, 0xc0000c6900, 0x3f, 0x23f, 0xc0000d3ee0, 0x100, 0x1)
	/home/elias/go/src/golang.org/x/exp/shiny/iconvg/decode.go:98 +0x115
golang.org/x/exp/shiny/iconvg.TestRasterizer(0xc00009d500)
	/home/elias/go/src/golang.org/x/exp/shiny/iconvg/decode_test.go:217 +0x5a7
testing.tRunner(0xc00009d500, 0x58a748)
	/home/elias/dev/go-tip/src/testing/testing.go:1109 +0xef
created by testing.(*T).Run
	/home/elias/dev/go-tip/src/testing/testing.go:1160 +0x386
exit status 2
FAIL	golang.org/x/exp/shiny/iconvg	0.014s

CC @nigeltao

[eliasnaur]

Original bug report: https://todo.sr.ht/~eliasnaur/gio/132.

[egonelbre]

This seems to be crashing because color.RGBA{0xfe, 0x76, 0xea, 0xf0} is not a valid premultiplied color.

https://github.com/golang/exp/blob/eab1b5eb1a030d481efa6d2d1362ece0ebdba8e9/shiny/iconvg/rasterizer.go#L230

The color is invalid, however crashing is not probably the best way to handle it. I guess the options are:

1. "try to fix it"
2. skip drawing it altogether

[kortschak]

It panics, exactly because of what @egonelbre is saying and that in this code that leaves the fill field as nil, setting up for the panic when the path is closed and drawn.

[egonelbre]

Just to clarify, the color was actually a valid premultiplied color, but a different kind. There are three variants in the wild:

1. linear premultiplied: premultiplied_c := linear_c * alpha
2. sRGB premultiplied: premultiplied_c := linearToSRGB(linear_c) * alpha
3. sRGB (early) premultiplied (variant 2): premultiplied_c := linearToSRGB(linear_c * alpha)

http://ssp.impulsetrain.com/gamma-premult.html

color.RGBA{0xfe, 0x76, 0xea, 0xf0} was the 3 kind.

[gopherbot]

Change https://golang.org/cl/291150 mentions this issue: shiny/iconvg: fix panic for out-of-range colors

[nigeltao]

1. "try to fix it"
2. skip drawing it altogether

I'm going to go with "skip drawing it altogether". We already re-purpose some of the "invalid alpha-premultiplied color space" to encode gradients, so I'm not sure if we can always fix it.

Sorry for the late reply.