graphicsutil: Avoid duplicated copying

Fixes #521

image.go

@@ -542,12 +542,7 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) {
 	}
 	runtime.SetFinalizer(i, (*Image).Dispose)
 
-	rgbaImg := graphicsutil.CopyImage(source)
+	_ = i.ReplacePixels(graphicsutil.CopyImage(source))
-	p := make([]byte, 4*width*height)
-	for j := 0; j < height; j++ {
-		copy(p[j*width*4:(j+1)*width*4], rgbaImg.Pix[j*rgbaImg.Stride:])
-	}
-	_ = i.ReplacePixels(p)
 	return i, nil
 }
 

internal/graphicsutil/copy.go

@@ -20,55 +20,56 @@ import (
 	"image/draw"
 )
 
-// CopyImage copies origImg to a new RGBA image.
+// CopyImage copies img to a new RGBA image.
 //
 // Basically CopyImage just calls draw.Draw.
-// If origImg is a paletted image, an optimized copying method is used.
+// If img is a paletted image, an optimized copying method is used.
 //
 // CopyImage is used only internally but it is exposed for testing.
-//
+func CopyImage(img image.Image) []byte {
-// TODO: CopyImage should return []byte (#521)
+	size := img.Bounds().Size()
-func CopyImage(origImg image.Image) *image.RGBA {
-	size := origImg.Bounds().Size()
 	w, h := size.X, size.Y
-	newImg := image.NewRGBA(image.Rect(0, 0, w, h))
+	bs := make([]byte, 4*w*h)
-	switch origImg := origImg.(type) {
+
+	switch img := img.(type) {
 	case *image.Paletted:
-		b := origImg.Bounds()
+		b := img.Bounds()
 		x0 := b.Min.X
 		y0 := b.Min.Y
 		x1 := b.Max.X
 		y1 := b.Max.Y
-		palette := make([]uint8, len(origImg.Palette)*4)
+
-		for i, c := range origImg.Palette {
+		palette := make([]uint8, len(img.Palette)*4)
+		for i, c := range img.Palette {
 			rgba := color.RGBAModel.Convert(c).(color.RGBA)
 			palette[4*i] = rgba.R
 			palette[4*i+1] = rgba.G
 			palette[4*i+2] = rgba.B
 			palette[4*i+3] = rgba.A
 		}
-		index0 := 0
+		// Even img is a subimage of another image, Pix starts with 0-th index.
-		index1 := 0
+		idx0 := 0
-		d0 := origImg.Stride - (x1 - x0)
+		idx1 := 0
-		d1 := newImg.Stride - (x1-x0)*4
+		d := img.Stride - (x1 - x0)
-		// Even origImg is a subimage of another image, Pix starts with 0-th index.
-		pix0 := origImg.Pix
-		pix1 := newImg.Pix
 		for j := 0; j < y1-y0; j++ {
 			for i := 0; i < x1-x0; i++ {
-				p := int(pix0[index0])
+				p := int(img.Pix[idx0])
-				pix1[index1] = palette[4*p]
+				bs[idx1] = palette[4*p]
-				pix1[index1+1] = palette[4*p+1]
+				bs[idx1+1] = palette[4*p+1]
-				pix1[index1+2] = palette[4*p+2]
+				bs[idx1+2] = palette[4*p+2]
-				pix1[index1+3] = palette[4*p+3]
+				bs[idx1+3] = palette[4*p+3]
-				index0++
+				idx0++
-				index1 += 4
+				idx1 += 4
 			}
-			index0 += d0
+			idx0 += d
-			index1 += d1
 		}
 	default:
-		draw.Draw(newImg, image.Rect(0, 0, w, h), origImg, origImg.Bounds().Min, draw.Src)
+		dstImg := &image.RGBA{
+			Pix:    bs,
+			Stride: 4 * w,
+			Rect:   image.Rect(0, 0, w, h),
+		}
+		draw.Draw(dstImg, image.Rect(0, 0, w, h), img, img.Bounds().Min, draw.Src)
 	}
-	return newImg
+	return bs
 }

internal/graphicsutil/copy_test.go

@@ -15,6 +15,7 @@
 package graphicsutil_test
 
 import (
+	"bytes"
 	"image"
 	"image/color"
 	"image/color/palette"
@@ -39,7 +40,7 @@ func TestCopyImage(t *testing.T) {
 	bigPalette := color.Palette(p)
 	cases := []struct {
 		In  image.Image
-		Out *image.RGBA
+		Out []uint8
 	}{
 		{
 			In: &image.Paletted{
@@ -50,19 +51,11 @@ func TestCopyImage(t *testing.T) {
 					color.Transparent, color.White,
 				}),
 			},
-			Out: &image.RGBA{
+			Out: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0},
-				Pix:    []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0},
-				Stride: 8,
-				Rect:   image.Rect(0, 0, 2, 2),
-			},
 		},
 		{
-			In: image.NewPaletted(image.Rect(0, 0, 240, 160), pal).SubImage(image.Rect(238, 158, 240, 160)),
+			In:  image.NewPaletted(image.Rect(0, 0, 240, 160), pal).SubImage(image.Rect(238, 158, 240, 160)),
-			Out: &image.RGBA{
+			Out: []uint8{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
-				Pix:    []uint8{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
-				Stride: 8,
-				Rect:   image.Rect(0, 0, 2, 2),
-			},
 		},
 		{
 			In: &image.RGBA{
@@ -70,11 +63,15 @@ func TestCopyImage(t *testing.T) {
 				Stride: 8,
 				Rect:   image.Rect(0, 0, 2, 2),
 			},
-			Out: &image.RGBA{
+			Out: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0},
-				Pix:    []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0},
+		},
+		{
+			In: &image.NRGBA{
+				Pix:    []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0x80, 0x80, 0x80, 0x80, 0x80, 0, 0, 0, 0},
 				Stride: 8,
 				Rect:   image.Rect(0, 0, 2, 2),
 			},
+			Out: []uint8{0, 0, 0, 0, 0x80, 0x80, 0x80, 0x80, 0x40, 0x40, 0x40, 0x80, 0, 0, 0, 0},
 		},
 		{
 			In: &image.Paletted{
@@ -83,11 +80,7 @@ func TestCopyImage(t *testing.T) {
 				Rect:    image.Rect(0, 0, 2, 2),
 				Palette: bigPalette,
 			},
-			Out: &image.RGBA{
+			Out: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0, 0, 0, 0, 0},
-				Pix:    []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0, 0, 0, 0, 0},
-				Stride: 8,
-				Rect:   image.Rect(0, 0, 2, 2),
-			},
 		},
 		{
 			In: (&image.Paletted{
@@ -96,28 +89,14 @@ func TestCopyImage(t *testing.T) {
 				Rect:    image.Rect(0, 0, 2, 2),
 				Palette: bigPalette,
 			}).SubImage(image.Rect(1, 0, 2, 1)),
-			Out: &image.RGBA{
+			Out: []uint8{0xff, 0xff, 0xff, 0xff},
-				Pix:    []uint8{0xff, 0xff, 0xff, 0xff},
-				Stride: 4,
-				Rect:   image.Rect(0, 0, 1, 1),
-			},
 		},
 	}
-	for _, c := range cases {
+	for i, c := range cases {
 		got := CopyImage(c.In)
-		if got.Rect != c.Out.Rect {
+		want := c.Out
-			t.Errorf("Rect: %v, want: %v", got.Rect, c.Out.Rect)
+		if !bytes.Equal(got, want) {
-		}
+			t.Errorf("Test %d: got: %v, want: %v", i, got, want)
-		size := got.Rect.Size()
-		w, h := size.X, size.Y
-		for j := 0; j < h; j++ {
-			for i := 0; i < w; i++ {
-				got := got.At(i, j)
-				want := c.Out.At(i, j)
-				if got != want {
-					t.Errorf("At(%d, %d): %v, want: %v", i, j, got, want)
-				}
-			}
 		}
 	}
 }