From 4dad64f5f1530a02404d562c7e6992d285bd9087 Mon Sep 17 00:00:00 2001 From: Nigel Tao Date: Thu, 21 Mar 2019 15:08:41 +1100 Subject: [PATCH] image/gif: don't assume Encode src's origin is (0, 0) When gif.Encode is given an "m image.Image" argument that isn't an *image.Paletted, it creates a temporary *image.Paletted (called pm) that is intended to be a copy of this image, only with fewer colors. That creation process, and specifically the opts.Drawer.Draw call that does the copy, incorrectly assumed that m.Bounds().Min is the zero point (0, 0). This commit fixes that. Fixes #30887 Change-Id: Ie03bddec359e2dcc52f18451049452105514e179 Reviewed-on: https://go-review.googlesource.com/c/go/+/168418 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/image/gif/writer.go | 5 +++- src/image/gif/writer_test.go | 51 +++++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/image/gif/writer.go b/src/image/gif/writer.go index 5e80feb33f..5819df0dd0 100644 --- a/src/image/gif/writer.go +++ b/src/image/gif/writer.go @@ -435,12 +435,15 @@ func Encode(w io.Writer, m image.Image, o *Options) error { pm, ok := m.(*image.Paletted) if !ok || len(pm.Palette) > opts.NumColors { + // Set pm to be a palettedized copy of m, including its bounds, which + // might not start at (0, 0). + // // TODO: Pick a better sub-sample of the Plan 9 palette. pm = image.NewPaletted(b, palette.Plan9[:opts.NumColors]) if opts.Quantizer != nil { pm.Palette = opts.Quantizer.Quantize(make(color.Palette, 0, opts.NumColors), m) } - opts.Drawer.Draw(pm, b, m, image.ZP) + opts.Drawer.Draw(pm, b, m, b.Min) } // When calling Encode instead of EncodeAll, the single-frame image is diff --git a/src/image/gif/writer_test.go b/src/image/gif/writer_test.go index 69042ec674..91275bb907 100644 --- a/src/image/gif/writer_test.go +++ b/src/image/gif/writer_test.go @@ -339,7 +339,10 @@ func TestEncodeNonZeroMinPoint(t *testing.T) { {+2, +2}, } for _, p := range points { - src := image.NewPaletted(image.Rectangle{Min: p, Max: p.Add(image.Point{6, 6})}, palette.Plan9) + src := image.NewPaletted(image.Rectangle{ + Min: p, + Max: p.Add(image.Point{6, 6}), + }, palette.Plan9) var buf bytes.Buffer if err := Encode(&buf, src, nil); err != nil { t.Errorf("p=%v: Encode: %v", p, err) @@ -354,6 +357,52 @@ func TestEncodeNonZeroMinPoint(t *testing.T) { t.Errorf("p=%v: got %v, want %v", p, got, want) } } + + // Also test having a source image (gray on the diagonal) that has a + // non-zero Bounds().Min, but isn't an image.Paletted. + { + p := image.Point{+2, +2} + src := image.NewRGBA(image.Rectangle{ + Min: p, + Max: p.Add(image.Point{6, 6}), + }) + src.SetRGBA(2, 2, color.RGBA{0x22, 0x22, 0x22, 0xFF}) + src.SetRGBA(3, 3, color.RGBA{0x33, 0x33, 0x33, 0xFF}) + src.SetRGBA(4, 4, color.RGBA{0x44, 0x44, 0x44, 0xFF}) + src.SetRGBA(5, 5, color.RGBA{0x55, 0x55, 0x55, 0xFF}) + src.SetRGBA(6, 6, color.RGBA{0x66, 0x66, 0x66, 0xFF}) + src.SetRGBA(7, 7, color.RGBA{0x77, 0x77, 0x77, 0xFF}) + + var buf bytes.Buffer + if err := Encode(&buf, src, nil); err != nil { + t.Errorf("gray-diagonal: Encode: %v", err) + return + } + m, err := Decode(&buf) + if err != nil { + t.Errorf("gray-diagonal: Decode: %v", err) + return + } + if got, want := m.Bounds(), image.Rect(0, 0, 6, 6); got != want { + t.Errorf("gray-diagonal: got %v, want %v", got, want) + return + } + + rednessAt := func(x int, y int) uint32 { + r, _, _, _ := m.At(x, y).RGBA() + // Shift by 8 to convert from 16 bit color to 8 bit color. + return r >> 8 + } + + // Round-tripping a still (non-animated) image.Image through + // Encode+Decode should shift the origin to (0, 0). + if got, want := rednessAt(0, 0), uint32(0x22); got != want { + t.Errorf("gray-diagonal: rednessAt(0, 0): got 0x%02x, want 0x%02x", got, want) + } + if got, want := rednessAt(5, 5), uint32(0x77); got != want { + t.Errorf("gray-diagonal: rednessAt(5, 5): got 0x%02x, want 0x%02x", got, want) + } + } } func TestEncodeImplicitConfigSize(t *testing.T) { -- 2.50.0