From 91a4329f0dab727ac0a7fb8bc7e4e51cf5156b45 Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Wed, 17 Jul 2019 03:15:01 +0900
Subject: [PATCH] restorable: Disallow ReplacePixels for a part after Fill

This simplifies Pixels struct. This is a preparation to change
how to record pixels.

Updates #897
---
 internal/restorable/image.go       | 26 ++++++----------
 internal/restorable/images_test.go | 49 ++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/internal/restorable/image.go b/internal/restorable/image.go
index 958eebd1b..e5850a44a 100644
--- a/internal/restorable/image.go
+++ b/internal/restorable/image.go
@@ -34,24 +34,10 @@ type Pixels struct {
 	color color.RGBA
 }
 
-func (p *Pixels) ensurePixels() {
-	if p.pixels != nil {
-		return
-	}
-	p.pixels = make([]byte, p.length)
-	if p.color.A == 0 {
-		return
-	}
-	for i := 0; i < p.length/4; i++ {
-		p.pixels[4*i] = p.color.R
-		p.pixels[4*i+1] = p.color.G
-		p.pixels[4*i+2] = p.color.B
-		p.pixels[4*i+3] = p.color.A
-	}
-}
-
 func (p *Pixels) CopyFrom(pix []byte, from int) {
-	p.ensurePixels()
+	if p.pixels == nil {
+		p.pixels = make([]byte, p.length)
+	}
 	copy(p.pixels[from:from+len(pix)], pix)
 }
 
@@ -305,6 +291,8 @@ func (i *Image) ClearPixels(x, y, width, height int) {
 }
 
 // ReplacePixels replaces the image pixels with the given pixels slice.
+//
+// ReplacePixels for a part is forbidden if the image is rendered with DrawTriangles or Fill.
 func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 	if pixels == nil {
 		panic("restorable: pixels must not be nil")
@@ -345,6 +333,10 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		panic("restorable: ReplacePixels for a part after DrawTriangles is forbidden")
 	}
 
+	if i.basePixels != nil && i.basePixels.color.A > 0 {
+		panic("restorable: ReplacePixels for a part after Fill is forbidden")
+	}
+
 	if i.stale {
 		return
 	}
diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go
index dd26f20e1..ec734d868 100644
--- a/internal/restorable/images_test.go
+++ b/internal/restorable/images_test.go
@@ -664,3 +664,52 @@ func TestReadPixelsFromVolatileImage(t *testing.T) {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
 }
+
+func TestAllowReplacePixelsAfterFill(t *testing.T) {
+	const w, h = 16, 16
+	dst := NewImage(w, h)
+	dst.Fill(1, 1, 1, 1)
+	dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h)
+	// ReplacePixels for a whole image doesn't panic.
+}
+
+func TestDisallowReplacePixelsForPartAfterFill(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("ReplacePixels for a part after Fill must panic but not")
+		}
+	}()
+	const w, h = 16, 16
+	dst := NewImage(w, h)
+	dst.Fill(1, 1, 1, 1)
+	dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1)
+}
+
+func TestAllowReplacePixelsAfterDrawTriangles(t *testing.T) {
+	const w, h = 16, 16
+	src := NewImage(w, h)
+	dst := NewImage(w, h)
+
+	vs := quadVertices(src, w, h, 0, 0)
+	is := graphics.QuadIndices()
+	dst.DrawTriangles(src, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero)
+	dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h)
+	// ReplacePixels for a whole image doesn't panic.
+}
+
+func TestDisallowReplacePixelsForPartAfterDrawTriangles(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("ReplacePixels for a part after DrawTriangles must panic but not")
+		}
+	}()
+
+	const w, h = 16, 16
+	src := NewImage(w, h)
+	dst := NewImage(w, h)
+
+	vs := quadVertices(src, w, h, 0, 0)
+	is := graphics.QuadIndices()
+	dst.DrawTriangles(src, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero)
+	dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1)
+}