From bc8a96eda7ada66a997c67d5dbf3e099ffaa44a8 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Thu, 8 Mar 2018 23:59:37 +0900 Subject: [PATCH] graphicsutil: Avoid duplicated copying Fixes #521 --- image.go | 7 +--- internal/graphicsutil/copy.go | 59 +++++++++++++++--------------- internal/graphicsutil/copy_test.go | 55 +++++++++------------------- 3 files changed, 48 insertions(+), 73 deletions(-) diff --git a/image.go b/image.go index 067325aa7..1bbdc3628 100644 --- a/image.go +++ b/image.go @@ -542,12 +542,7 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) { } runtime.SetFinalizer(i, (*Image).Dispose) - rgbaImg := 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) + _ = i.ReplacePixels(graphicsutil.CopyImage(source)) return i, nil } diff --git a/internal/graphicsutil/copy.go b/internal/graphicsutil/copy.go index c4cfc1a9a..8d3f27ef7 100644 --- a/internal/graphicsutil/copy.go +++ b/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. -// -// TODO: CopyImage should return []byte (#521) -func CopyImage(origImg image.Image) *image.RGBA { - size := origImg.Bounds().Size() +func CopyImage(img image.Image) []byte { + size := img.Bounds().Size() w, h := size.X, size.Y - newImg := image.NewRGBA(image.Rect(0, 0, w, h)) - switch origImg := origImg.(type) { + bs := make([]byte, 4*w*h) + + 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 - index1 := 0 - d0 := origImg.Stride - (x1 - x0) - d1 := newImg.Stride - (x1-x0)*4 - // Even origImg is a subimage of another image, Pix starts with 0-th index. - pix0 := origImg.Pix - pix1 := newImg.Pix + // Even img is a subimage of another image, Pix starts with 0-th index. + idx0 := 0 + idx1 := 0 + d := img.Stride - (x1 - x0) for j := 0; j < y1-y0; j++ { for i := 0; i < x1-x0; i++ { - p := int(pix0[index0]) - pix1[index1] = palette[4*p] - pix1[index1+1] = palette[4*p+1] - pix1[index1+2] = palette[4*p+2] - pix1[index1+3] = palette[4*p+3] - index0++ - index1 += 4 + p := int(img.Pix[idx0]) + bs[idx1] = palette[4*p] + bs[idx1+1] = palette[4*p+1] + bs[idx1+2] = palette[4*p+2] + bs[idx1+3] = palette[4*p+3] + idx0++ + idx1 += 4 } - index0 += d0 - index1 += d1 + idx0 += d } 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 } diff --git a/internal/graphicsutil/copy_test.go b/internal/graphicsutil/copy_test.go index 6b13dbf5f..85304d79e 100644 --- a/internal/graphicsutil/copy_test.go +++ b/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{ - 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), - }, + Out: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0}, }, { - In: image.NewPaletted(image.Rect(0, 0, 240, 160), pal).SubImage(image.Rect(238, 158, 240, 160)), - Out: &image.RGBA{ - 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.NewPaletted(image.Rect(0, 0, 240, 160), pal).SubImage(image.Rect(238, 158, 240, 160)), + Out: []uint8{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { In: &image.RGBA{ @@ -70,11 +63,15 @@ func TestCopyImage(t *testing.T) { Stride: 8, Rect: image.Rect(0, 0, 2, 2), }, - Out: &image.RGBA{ - Pix: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0}, + Out: []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{ - 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), - }, + Out: []uint8{0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0, 0, 0, 0, 0, 0, 0, 0}, }, { 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{ - Pix: []uint8{0xff, 0xff, 0xff, 0xff}, - Stride: 4, - Rect: image.Rect(0, 0, 1, 1), - }, + Out: []uint8{0xff, 0xff, 0xff, 0xff}, }, } - for _, c := range cases { + for i, c := range cases { got := CopyImage(c.In) - if got.Rect != c.Out.Rect { - t.Errorf("Rect: %v, want: %v", got.Rect, c.Out.Rect) - } - 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) - } - } + want := c.Out + if !bytes.Equal(got, want) { + t.Errorf("Test %d: got: %v, want: %v", i, got, want) } } }