From 4c640d2500fb53baf6fad37a766eefdb47f23f55 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Wed, 1 Jul 2020 02:56:37 +0900
Subject: [PATCH] buffered, restorable: Remove copying pixels

Instead, the callers (ebiten.NewImageFromImage and
(*ebiten.Image).ReplacePixels) have responsibility to copy the
pixels now. This change should reduce unnecessary copying pixels.

Updates #1222
---
 image.go                     | 13 +++++++++++--
 internal/buffered/image.go   |  4 +---
 internal/restorable/image.go | 13 +++----------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/image.go b/image.go
index 39204b2ae..3843b99e9 100644
--- a/image.go
+++ b/image.go
@@ -548,7 +548,13 @@ func (i *Image) ReplacePixels(pix []byte) error {
 		return nil
 	}
 	r := i.Bounds()
-	if err := i.buffered.ReplacePixels(pix, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil {
+
+	// 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 {
 		theUIContext.setError(err)
 	}
 	return nil
@@ -632,7 +638,10 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) {
 	}
 	i.addr = i
 
-	_ = i.ReplacePixels(copyImage(source))
+	// Call (*buffered.Image).ReplacePixels directly to avoid copying.
+	if err := i.buffered.ReplacePixels(copyImage(source), 0, 0, width, height); err != nil {
+		theUIContext.setError(err)
+	}
 	return i, nil
 }
 
diff --git a/internal/buffered/image.go b/internal/buffered/image.go
index 6bd7592d9..c935fe1ea 100644
--- a/internal/buffered/image.go
+++ b/internal/buffered/image.go
@@ -192,10 +192,8 @@ 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(copied, x, y, width, height)
+			i.ReplacePixels(pix, x, y, width, height)
 			return nil
 		}) {
 			return nil
diff --git a/internal/restorable/image.go b/internal/restorable/image.go
index d4c733793..de74c4b04 100644
--- a/internal/restorable/image.go
+++ b/internal/restorable/image.go
@@ -300,15 +300,8 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 	// For this purpuse, images should remember which part of that is used for DrawTriangles.
 	theImages.makeStaleIfDependingOn(i)
 
-	// TODO: Avoid copying if possible (#983)
-	var copiedPixels []byte
 	if pixels != nil {
-		copiedPixels = make([]byte, len(pixels))
-		copy(copiedPixels, pixels)
-	}
-
-	if pixels != nil {
-		i.image.ReplacePixels(copiedPixels, x, y, width, height)
+		i.image.ReplacePixels(pixels, x, y, width, height)
 	} else {
 		// TODO: When pixels == nil, we don't have to care the pixel state there. In such cases, the image
 		// accepts only ReplacePixels and not Fill or DrawTriangles.
@@ -318,7 +311,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 
 	if x == 0 && y == 0 && width == w && height == h {
 		if pixels != nil {
-			i.basePixels.AddOrReplace(copiedPixels, 0, 0, w, h)
+			i.basePixels.AddOrReplace(pixels, 0, 0, w, h)
 		} else {
 			i.basePixels.Remove(0, 0, w, h)
 		}
@@ -340,7 +333,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 	}
 
 	if pixels != nil {
-		i.basePixels.AddOrReplace(copiedPixels, x, y, width, height)
+		i.basePixels.AddOrReplace(pixels, x, y, width, height)
 	} else {
 		i.basePixels.Remove(x, y, width, height)
 	}