restorable: Replace Fill with Clear

internal/restorable/image.go

@@ -17,7 +17,6 @@ package restorable
 import (
 	"errors"
 	"fmt"
-	"image/color"
 
 	"github.com/hajimehoshi/ebiten/internal/affine"
 	"github.com/hajimehoshi/ebiten/internal/driver"
@@ -27,14 +26,11 @@ import (
 
 type Pixels struct {
 	rectToPixels *rectToPixels
-
-	// color is used only when rectToPixels is nil.
-	color color.RGBA
 }
 
 func (p *Pixels) Apply(img *graphicscommand.Image) {
 	if p.rectToPixels == nil {
-		fillImage(img, p.color.R, p.color.G, p.color.B, p.color.A)
+		clearImage(img)
 		return
 	}
 
@@ -46,7 +42,6 @@ func (p *Pixels) AddOrReplace(pix []byte, x, y, width, height int) {
 		p.rectToPixels = &rectToPixels{}
 	}
 	p.rectToPixels.addOrReplace(pix, x, y, width, height)
-	p.color = color.RGBA{}
 }
 
 func (p *Pixels) Remove(x, y, width, height int) {
@@ -63,9 +58,8 @@ func (p *Pixels) At(i, j int) (byte, byte, byte, byte) {
 		if r, g, b, a, ok := p.rectToPixels.at(i, j); ok {
 			return r, g, b, a
 		}
-		return 0, 0, 0, 0
 	}
-	return p.color.R, p.color.G, p.color.B, p.color.A
+	return 0, 0, 0, 0
 }
 
 // drawTrianglesHistoryItem is an item for history of draw-image commands.
@@ -90,6 +84,7 @@ type Image struct {
 	drawTrianglesHistory []*drawTrianglesHistoryItem
 
 	// stale indicates whether the image needs to be synced with GPU as soon as possible.
+	// TODO: Instead of this boolean value, can we represent the stale state with basePixels's state?
 	stale bool
 
 	// volatile indicates whether the image is cleared whenever a frame starts.
@@ -115,7 +110,7 @@ func init() {
 		pix[i] = 0xff
 	}
 
-	// As emptyImage is the source at fillImage, initialize this with ReplacePixels, not fillImage.
+	// As emptyImage is the source at clearImage, initialize this with ReplacePixels, not clearImage.
 	// This operation is also important when restoring emptyImage.
 	emptyImage.ReplacePixels(pix, 0, 0, w, h)
 	theImages.add(emptyImage)
@@ -158,10 +153,6 @@ func (i *Image) Extend(width, height int) *Image {
 		panic("restorable: Extend after DrawTriangles is forbidden")
 	}
 
-	if i.basePixels != nil && i.basePixels.color.A > 0 {
-		panic("restorable: Extend after Fill is forbidden")
-	}
-
 	newImg := NewImage(width, height)
 
 	i.basePixels.Apply(newImg.image)
@@ -192,33 +183,22 @@ func NewScreenFramebufferImage(width, height int) *Image {
 	return i
 }
 
-func (i *Image) Fill(r, g, b, a byte) {
+func (i *Image) Clear() {
 	theImages.makeStaleIfDependingOn(i)
-	i.fill(r, g, b, a)
+	i.clear()
 }
 
 // clearForInitialization clears the underlying image for initialization.
 func (i *Image) clearForInitialization() {
 	// As this is for initialization, drawing history doesn't have to be adjusted.
-	i.fill(0, 0, 0, 0)
+	i.clear()
 }
 
-// fillImage fills a graphicscommand.Image with the specified color.
+// clearImage clears a graphicscommand.Image.
 // This does nothing to do with a restorable.Image's rendering state.
-func fillImage(img *graphicscommand.Image, r, g, b, a byte) {
+func clearImage(img *graphicscommand.Image) {
 	if img == emptyImage.image {
-		panic("restorable: fillImage cannot be called on emptyImage")
+		panic("restorable: clearImage cannot be called on emptyImage")
-	}
-
-	rf := float32(0)
-	gf := float32(0)
-	bf := float32(0)
-	af := float32(0)
-	if a > 0 {
-		rf = float32(r) / float32(a)
-		gf = float32(g) / float32(a)
-		bf = float32(b) / float32(a)
-		af = float32(a) / 0xff
 	}
 
 	// There are not 'drawTrianglesHistoryItem's for this image and emptyImage.
@@ -231,27 +211,21 @@ func fillImage(img *graphicscommand.Image, r, g, b, a byte) {
 	vs := make([]float32, 4*graphics.VertexFloatNum)
 	graphics.PutQuadVertices(vs, emptyImage, 0, 0, sw, sh,
 		float32(dw)/float32(sw), 0, 0, float32(dh)/float32(sh), 0, 0,
-		rf, gf, bf, af)
+		0, 0, 0, 0)
 	is := graphics.QuadIndices()
-	c := driver.CompositeModeCopy
+	// The first DrawTriangles must be clear mode for initialization.
-	if a == 0 {
+	// TODO: Can the graphicscommand package hide this knowledge?
-		// The first DrawTriangles must be clear mode for initialization.
+	img.DrawTriangles(emptyImage.image, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressClampToZero)
-		// TODO: Can the graphicscommand package hide this knowledge?
-		c = driver.CompositeModeClear
-	}
-	img.DrawTriangles(emptyImage.image, vs, is, nil, c, driver.FilterNearest, driver.AddressClampToZero)
 }
 
-func (i *Image) fill(r, g, b, a byte) {
+func (i *Image) clear() {
 	if i.priority {
 		panic("restorable: clear cannot be called on a priority image")
 	}
 
-	fillImage(i.image, r, g, b, a)
+	clearImage(i.image)
 
-	i.basePixels = &Pixels{
+	i.basePixels = &Pixels{}
-		color: color.RGBA{r, g, b, a},
-	}
 	i.drawTrianglesHistory = nil
 	i.stale = false
 }
@@ -361,10 +335,6 @@ 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 {
 		// TODO: panic here?
 		return
@@ -539,7 +509,7 @@ func (i *Image) restore() error {
 
 	gimg := graphicscommand.NewImage(w, h)
 	// Clear the image explicitly.
-	fillImage(gimg, 0, 0, 0, 0)
+	clearImage(gimg)
 	if i.basePixels != nil {
 		i.basePixels.Apply(gimg)
 	}

internal/restorable/images_test.go

@@ -74,7 +74,7 @@ func TestRestore(t *testing.T) {
 	defer img0.Dispose()
 
 	clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff}
-	img0.Fill(clr0.R, clr0.G, clr0.B, clr0.A)
+	img0.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, 1, 1)
 	ResolveStaleImages()
 	if err := RestoreIfNeeded(); err != nil {
 		t.Fatal(err)
@@ -129,7 +129,7 @@ func TestRestoreChain(t *testing.T) {
 		}
 	}()
 	clr := color.RGBA{0x00, 0x00, 0x00, 0xff}
-	imgs[0].Fill(clr.R, clr.G, clr.B, clr.A)
+	imgs[0].ReplacePixels([]byte{clr.R, clr.G, clr.B, clr.A}, 0, 0, 1, 1)
 	for i := 0; i < num-1; i++ {
 		vs := quadVertices(imgs[i], 1, 1, 0, 0)
 		is := graphics.QuadIndices()
@@ -166,11 +166,11 @@ func TestRestoreChain2(t *testing.T) {
 	}()
 
 	clr0 := color.RGBA{0xff, 0x00, 0x00, 0xff}
-	imgs[0].Fill(clr0.R, clr0.G, clr0.B, clr0.A)
+	imgs[0].ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, w, h)
 	clr7 := color.RGBA{0x00, 0xff, 0x00, 0xff}
-	imgs[7].Fill(clr7.R, clr7.G, clr7.B, clr7.A)
+	imgs[7].ReplacePixels([]byte{clr7.R, clr7.G, clr7.B, clr7.A}, 0, 0, w, h)
 	clr8 := color.RGBA{0x00, 0x00, 0xff, 0xff}
-	imgs[8].Fill(clr8.R, clr8.G, clr8.B, clr8.A)
+	imgs[8].ReplacePixels([]byte{clr8.R, clr8.G, clr8.B, clr8.A}, 0, 0, w, h)
 
 	is := graphics.QuadIndices()
 	imgs[8].DrawTriangles(imgs[7], quadVertices(imgs[7], w, h, 0, 0), is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero)
@@ -212,11 +212,11 @@ func TestRestoreOverrideSource(t *testing.T) {
 	}()
 	clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff}
 	clr1 := color.RGBA{0x00, 0x00, 0x01, 0xff}
-	img1.Fill(clr0.R, clr0.G, clr0.B, clr0.A)
+	img1.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, w, h)
 	is := graphics.QuadIndices()
 	img2.DrawTriangles(img1, quadVertices(img1, w, h, 0, 0), is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero)
 	img3.DrawTriangles(img2, quadVertices(img2, w, h, 0, 0), is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero)
-	img0.Fill(clr1.R, clr1.G, clr1.B, clr1.A)
+	img0.ReplacePixels([]byte{clr1.R, clr1.G, clr1.B, clr1.A}, 0, 0, w, h)
 	img1.DrawTriangles(img0, quadVertices(img0, w, h, 0, 0), is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero)
 	ResolveStaleImages()
 	if err := RestoreIfNeeded(); err != nil {
@@ -666,7 +666,11 @@ func TestReadPixelsFromVolatileImage(t *testing.T) {
 	dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h)
 
 	// Second, draw src to dst. If the implementation is correct, dst becomes stale.
-	src.Fill(0xff, 0xff, 0xff, 0xff)
+	pix := make([]byte, 4*w*h)
+	for i := range pix {
+		pix[i] = 0xff
+	}
+	src.ReplacePixels(pix, 0, 0, w, h)
 	vs := quadVertices(src, 1, 1, 0, 0)
 	is := graphics.QuadIndices()
 	dst.DrawTriangles(src, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressClampToZero)
@@ -680,26 +684,6 @@ func TestReadPixelsFromVolatileImage(t *testing.T) {
 	}
 }
 
-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)
@@ -765,19 +749,6 @@ func TestExtend(t *testing.T) {
 	}
 }
 
-func TestFillAndExtend(t *testing.T) {
-	defer func() {
-		if r := recover(); r == nil {
-			t.Errorf("Extend after Fill must panic but not")
-		}
-	}()
-
-	const w, h = 16, 16
-	orig := NewImage(w, h)
-	orig.Fill(0x01, 0x02, 0x03, 0x04)
-	orig.Extend(w*2, h*2)
-}
-
 func TestClearPixels(t *testing.T) {
 	const w, h = 16, 16
 	img := NewImage(w, h)

internal/shareable/shareable.go

@@ -309,7 +309,7 @@ func (i *Image) ClearFramebuffer() {
 	}
 	i.ensureNotShared()
 
-	i.backend.restorable.Fill(0, 0, 0, 0)
+	i.backend.restorable.Clear()
 }
 
 func (i *Image) ReplacePixels(p []byte) {