From fbf700705610bbd319696d9f15b4eedbf49ba176 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Wed, 26 Dec 2018 20:10:12 +0900
Subject: [PATCH] restorable: Pixels() should return immediately when it
 doesn't have to access GPU

Fixes #763
---
 internal/restorable/image.go       | 14 ++++++++++----
 internal/restorable/images_test.go | 22 ++++++++++++++++++++++
 internal/shareable/shareable.go    |  5 ++++-
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/internal/restorable/image.go b/internal/restorable/image.go
index f3cf551ef..fee936064 100644
--- a/internal/restorable/image.go
+++ b/internal/restorable/image.go
@@ -106,6 +106,10 @@ func (i *Image) BasePixelsForTesting() []byte {
 	return i.basePixels
 }
 
+// Pixels returns the image's pixel bytes.
+//
+// Pixels tries to read pixels from GPU if needed.
+// It is assured that GPU is not accessed if only the image's ReplacePixels is called.
 func (i *Image) Pixels() []byte {
 	i.readPixelsFromGPUIfNeeded()
 	return i.basePixels
@@ -189,14 +193,16 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		i.stale = false
 		return
 	}
-	if i.basePixels == nil {
-		i.makeStale()
-		return
-	}
 	if len(i.drawImageHistory) > 0 {
 		i.makeStale()
 		return
 	}
+	if i.stale {
+		return
+	}
+	if i.basePixels == nil {
+		i.basePixels = make([]byte, 4*w*h)
+	}
 	idx := 4 * (y*w + x)
 	if pixels != nil {
 		for j := 0; j < height; j++ {
diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go
index 67664d086..d2c220ff9 100644
--- a/internal/restorable/images_test.go
+++ b/internal/restorable/images_test.go
@@ -657,4 +657,26 @@ func TestClear(t *testing.T) {
 	}
 }
 
+func TestReplacePixelsOnly(t *testing.T) {
+	const w, h = 128, 128
+	img := NewImage(w, h, false)
+	defer img.Dispose()
+
+	for i := 0; i < w*h; i += 5 {
+		img.ReplacePixels([]byte{1, 2, 3, 4}, i%w, i/w, 1, 1)
+	}
+
+	// BasePixelsForTesting is available without GPU accessing.
+	for i := 0; i < w*h; i++ {
+		var want color.RGBA
+		if i%5 == 0 {
+			want = color.RGBA{1, 2, 3, 4}
+		}
+		got := byteSliceToColor(img.BasePixelsForTesting(), i)
+		if !sameColors(got, want, 0) {
+			t.Errorf("got %v, want %v", got, want)
+		}
+	}
+}
+
 // TODO: How about volatile/screen images?
diff --git a/internal/shareable/shareable.go b/internal/shareable/shareable.go
index ac4493f19..f74313ecc 100644
--- a/internal/shareable/shareable.go
+++ b/internal/shareable/shareable.go
@@ -69,7 +69,10 @@ func (b *backend) TryAlloc(width, height int) (*packing.Node, bool) {
 	// ReplacePixels on a part of image deletes other region that are rendered by DrawImage (#593, 758).
 	// TODO: Add validations to ensure that an image cannot accept DrawImage after ReplacePixels on a part of
 	// it.
-	newImg.ReplacePixels(oldImg.Pixels(), 0, 0, w, h)
+	//
+	// Pixels() returns immediately as long as only oldImg.ReplacePixels is called.
+	pix := oldImg.Pixels()
+	newImg.ReplacePixels(pix, 0, 0, w, h)
 	oldImg.Dispose()
 	b.restorable = newImg