Revert "restorable: Do not record pixels if restoring is not requried"

This reverts commit 5e1f263d6d.

Reason: Issue #1304

internal/graphicscommand/image.go

@@ -26,6 +26,15 @@ import (
 	"github.com/hajimehoshi/ebiten/internal/png"
 )
 
+type lastCommand int
+
+const (
+	lastCommandNone lastCommand = iota
+	lastCommandClear
+	lastCommandDrawTriangles
+	lastCommandReplacePixels
+)
+
 // Image represents an image that is implemented with OpenGL.
 type Image struct {
 	image          driver.Image
@@ -42,6 +51,8 @@ type Image struct {
 	id int
 
 	bufferedRP []*driver.ReplacePixelsArgs
+
+	lastCommand lastCommand
 }
 
 var nextID = 1
@@ -140,6 +151,12 @@ func (i *Image) InternalSize() (int, int) {
 // If the source image is not specified, i.e., src is nil and there is no image in the uniform variables, the
 // elements for the source image are not used.
 func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [graphics.ShaderImageNum - 1][2]float32, vertices []float32, indices []uint16, clr *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, sourceRegion driver.Region, shader *Shader, uniforms []interface{}) {
+	if i.lastCommand == lastCommandNone {
+		if !i.screen && mode != driver.CompositeModeClear {
+			panic("graphicscommand: the image must be cleared first")
+		}
+	}
+
 	for _, src := range srcs {
 		if src == nil {
 			continue
@@ -152,6 +169,12 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [gra
 	i.resolveBufferedReplacePixels()
 
 	theCommandQueue.EnqueueDrawTrianglesCommand(i, srcs, offsets, vertices, indices, clr, mode, filter, address, sourceRegion, shader, uniforms)
+
+	if i.lastCommand == lastCommandNone && !i.screen {
+		i.lastCommand = lastCommandClear
+	} else {
+		i.lastCommand = lastCommandDrawTriangles
+	}
 }
 
 // Pixels returns the image's pixels.
@@ -170,6 +193,12 @@ func (i *Image) Pixels() ([]byte, error) {
 }
 
 func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
+	// ReplacePixels for a part might invalidate the current image that are drawn by DrawTriangles (#593, #738).
+	if i.lastCommand == lastCommandDrawTriangles {
+		if x != 0 || y != 0 || i.width != width || i.height != height {
+			panic("graphicscommand: ReplacePixels for a part after DrawTriangles is forbidden")
+		}
+	}
 	i.bufferedRP = append(i.bufferedRP, &driver.ReplacePixelsArgs{
 		Pixels: pixels,
 		X:      x,
@@ -177,6 +206,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		Width:  width,
 		Height: height,
 	})
+	i.lastCommand = lastCommandReplacePixels
 }
 
 func (i *Image) IsInvalidated() bool {

internal/graphicscommand/image_test.go

@@ -63,6 +63,11 @@ func TestClear(t *testing.T) {
 }
 
 func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("ReplacePixels must panic but not")
+		}
+	}()
 	const w, h = 32, 32
 	clr := NewImage(w, h)
 	src := NewImage(w/2, h/2)
@@ -72,8 +77,6 @@ func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) {
 	dst.DrawTriangles([graphics.ShaderImageNum]*Image{clr}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressUnsafe, driver.Region{}, nil, nil)
 	dst.DrawTriangles([graphics.ShaderImageNum]*Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressUnsafe, driver.Region{}, nil, nil)
 	dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1)
-
-	// TODO: Check the result.
 }
 
 func TestShader(t *testing.T) {

internal/restorable/image.go

@@ -153,18 +153,6 @@ func NewImage(width, height int, volatile bool) *Image {
 	return i
 }
 
-func newImageWithColor(width, height int, volatile bool, clr color.RGBA) *Image {
-	i := &Image{
-		image:    graphicscommand.NewImage(width, height),
-		width:    width,
-		height:   height,
-		volatile: volatile,
-	}
-	fillImage(i.image, clr)
-	theImages.add(i)
-	return i
-}
-
 // Extend extends the image by the given size.
 // Extend creates a new image with the given size and copies the pixels of the given source image.
 // Extend disposes itself after its call.
@@ -179,22 +167,21 @@ func (i *Image) Extend(width, height int) *Image {
 		panic(fmt.Sprintf("restorable: the original size (%d, %d) cannot be extended to (%d, %d)", i.width, i.height, width, height))
 	}
 
-	newImg := newImageWithColor(width, height, i.volatile, i.basePixels.baseColor)
+	if i.stale {
+		panic("restorable: Extend at a stale image is forbidden")
+	}
 
-	// Use DrawTriangles instead of ReplacePixels because the image i might be stale and not have its pixels
+	if len(i.drawTrianglesHistory) > 0 {
-	// information.
+		panic("restorable: Extend after DrawTriangles is forbidden")
-	srcs := [graphics.ShaderImageNum]*Image{i}
+	}
-	var offsets [graphics.ShaderImageNum - 1][2]float32
-	sw, sh := i.image.InternalSize()
-	vs := quadVertices(0, 0, float32(sw), float32(sh), 0, 0, float32(sw), float32(sh), 1, 1, 1, 1)
-	is := graphics.QuadIndices()
-	newImg.DrawTriangles(srcs, offsets, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, driver.Region{}, nil, nil)
 
-	// Overwrite the history as if the image newImg is created only by ReplacePixels. Now drawTrianglesHistory
+	newImg := NewImage(width, height, i.volatile)
-	// and basePixels cannot be mixed.
+	i.basePixels.Apply(newImg.image)
-	newImg.drawTrianglesHistory = nil
+
+	if i.basePixels.baseColor != (color.RGBA{}) {
+		panic("restorable: baseColor must be empty at Extend")
+	}
 	newImg.basePixels = i.basePixels
-	newImg.stale = i.stale
 
 	i.Dispose()
 
@@ -325,11 +312,6 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height)
 	}
 
-	if !needsRestoring() || i.screen || i.volatile {
-		i.makeStale()
-		return
-	}
-
 	if x == 0 && y == 0 && width == w && height == h {
 		if pixels != nil {
 			i.basePixels.AddOrReplace(pixels, 0, 0, w, h)
@@ -341,7 +323,9 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		return
 	}
 
-	// drawTrianglesHistory and basePixels cannot be mixed.
+	// It looked like ReplacePixels on a part of image deletes other region that are rendered by DrawTriangles
+	// (#593, #758).
+
 	if len(i.drawTrianglesHistory) > 0 {
 		panic("restorable: ReplacePixels for a part after DrawTriangles is forbidden")
 	}