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 golang#30887

Change-Id: Ie03bddec359e2dcc52f18451049452105514e179
Reviewed-on: https://go-review.googlesource.com/c/go/+/168418
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rob Pike <[email protected]>

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

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) {