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

Jump to bottom

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

Closed
eliasnaur opened this issue Jun 11, 2020 · 6 comments
Closed

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

eliasnaur opened this issue Jun 11, 2020 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@eliasnaur
Copy link
Contributor

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
@gopherbot gopherbot added this to the Unreleased milestone Jun 11, 2020
@eliasnaur
Copy link
Contributor Author

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

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 12, 2020
@egonelbre
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

egonelbre commented Jan 11, 2021 •
edited

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
Copy link

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

@nigeltao
Copy link
Contributor

  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.

@golang golang locked and limited conversation to collaborators Feb 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@toothrot @egonelbre @kortschak @eliasnaur @nigeltao @gopherbot