From 02ef92f4cde1e9868dd9a45c06d431690d8a2ab3 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Fri, 3 Jul 2020 02:56:37 +0900 Subject: [PATCH] ebiten: Remove copying pixels from ReplacePixels and copyImage (renamed to imageToBytes) This optimization utilizes the fact that copying happens in the 'shareable' package to add paddings. Updates #1222 --- export_test.go | 2 +- image.go | 17 +++++-------- copy.go => imagetobytes.go | 37 ++++++++++++++++++++-------- copy_test.go => imagetobytes_test.go | 16 ++++++------ internal/buffered/image.go | 4 ++- 5 files changed, 45 insertions(+), 31 deletions(-) rename copy.go => imagetobytes.go (70%) rename copy_test.go => imagetobytes_test.go (91%) diff --git a/export_test.go b/export_test.go index 323c9fbf2..affa4ce57 100644 --- a/export_test.go +++ b/export_test.go @@ -15,5 +15,5 @@ package ebiten var ( - CopyImage = copyImage + ImageToBytes = imageToBytes ) diff --git a/image.go b/image.go index ccdccd09e..3e397acf1 100644 --- a/image.go +++ b/image.go @@ -540,7 +540,7 @@ func (i *Image) Dispose() error { // When the image is disposed, ReplacePixels does nothing. // // ReplacePixels always returns nil as of 1.5.0. -func (i *Image) ReplacePixels(pix []byte) error { +func (i *Image) ReplacePixels(pixels []byte) error { i.copyCheck() if i.isDisposed() { @@ -548,12 +548,10 @@ func (i *Image) ReplacePixels(pix []byte) error { } r := i.Bounds() - // Copy the pixels as restorable package might reuse the pixels later. - // TODO: Would it be possible to avoid copying? (#1222) - copied := make([]byte, len(pix)) - copy(copied, pix) - - if err := i.buffered.ReplacePixels(copied, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil { + // Do not need to copy pixels here. + // * In internal/buffered, pixels are copied when necessary. + // * In internal/shareable, pixels are copied to make its paddings. + if err := i.buffered.ReplacePixels(pixels, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil { theUIContext.setError(err) } return nil @@ -637,10 +635,7 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) { } i.addr = i - // Call (*buffered.Image).ReplacePixels directly to avoid copying. - if err := i.buffered.ReplacePixels(copyImage(source), 0, 0, width, height); err != nil { - theUIContext.setError(err) - } + i.ReplacePixels(imageToBytes(source)) return i, nil } diff --git a/copy.go b/imagetobytes.go similarity index 70% rename from copy.go rename to imagetobytes.go index 1808ea988..0e4c4e35d 100644 --- a/copy.go +++ b/imagetobytes.go @@ -20,17 +20,20 @@ import ( "image/draw" ) -// copyImage copies img to a new RGBA image. +// imageToBytes gets RGBA bytes from img. // -// Basically copyImage just calls draw.Draw. +// Basically imageToBytes just calls draw.Draw. // If img is a paletted image, an optimized copying method is used. -func copyImage(img image.Image) []byte { +// +// If img is *image.RGBA and its length is same as 4*width*height, imageToBytes returns its Pix. +func imageToBytes(img image.Image) []byte { size := img.Bounds().Size() w, h := size.X, size.Y - bs := make([]byte, 4*w*h) switch img := img.(type) { case *image.Paletted: + bs := make([]byte, 4*w*h) + b := img.Bounds() x0 := b.Min.X y0 := b.Min.Y @@ -61,13 +64,27 @@ func copyImage(img image.Image) []byte { } idx0 += d } - default: - dstImg := &image.RGBA{ - Pix: bs, - Stride: 4 * w, - Rect: image.Rect(0, 0, w, h), + return bs + case *image.RGBA: + if len(img.Pix) == 4*w*h { + return img.Pix } - draw.Draw(dstImg, image.Rect(0, 0, w, h), img, img.Bounds().Min, draw.Src) + return imageToBytesSlow(img) + default: + return imageToBytesSlow(img) } +} + +func imageToBytesSlow(img image.Image) []byte { + size := img.Bounds().Size() + w, h := size.X, size.Y + bs := make([]byte, 4*w*h) + + 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 bs } diff --git a/copy_test.go b/imagetobytes_test.go similarity index 91% rename from copy_test.go rename to imagetobytes_test.go index 5e2dfe036..a6618188b 100644 --- a/copy_test.go +++ b/imagetobytes_test.go @@ -24,7 +24,7 @@ import ( . "github.com/hajimehoshi/ebiten" ) -func TestCopyImage(t *testing.T) { +func TestImageToBytes(t *testing.T) { pal := make(color.Palette, 256) for i := range pal { pal[i] = color.White @@ -93,7 +93,7 @@ func TestCopyImage(t *testing.T) { }, } for i, c := range cases { - got := CopyImage(c.In) + got := ImageToBytes(c.In) want := c.Out if !bytes.Equal(got, want) { t.Errorf("Test %d: got: %v, want: %v", i, got, want) @@ -101,26 +101,26 @@ func TestCopyImage(t *testing.T) { } } -func BenchmarkCopyImageRGBA(b *testing.B) { +func BenchmarkImageToBytesRGBA(b *testing.B) { img := image.NewRGBA(image.Rect(0, 0, 4096, 4096)) b.ResetTimer() for i := 0; i < b.N; i++ { - CopyImage(img) + ImageToBytes(img) } } -func BenchmarkCopyImageNRGBA(b *testing.B) { +func BenchmarkImageToBytesNRGBA(b *testing.B) { img := image.NewNRGBA(image.Rect(0, 0, 4096, 4096)) b.ResetTimer() for i := 0; i < b.N; i++ { - CopyImage(img) + ImageToBytes(img) } } -func BenchmarkCopyImagePaletted(b *testing.B) { +func BenchmarkImageToBytesPaletted(b *testing.B) { img := image.NewPaletted(image.Rect(0, 0, 4096, 4096), palette.Plan9) b.ResetTimer() for i := 0; i < b.N; i++ { - CopyImage(img) + ImageToBytes(img) } } diff --git a/internal/buffered/image.go b/internal/buffered/image.go index 83dcdffa1..f45e17cda 100644 --- a/internal/buffered/image.go +++ b/internal/buffered/image.go @@ -192,8 +192,10 @@ func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error { } if maybeCanAddDelayedCommand() { + copied := make([]byte, len(pix)) + copy(copied, pix) if tryAddDelayedCommand(func() error { - i.ReplacePixels(pix, x, y, width, height) + i.ReplacePixels(copied, x, y, width, height) return nil }) { return nil