restorable: Remove Fill and make (*ebiten.Image).Fill available for sub-images

Now a scissor (a clipping region) can be specified, we don't have to
worry about the rendering results out of the specified region.
Replace the implmenetation of the Fill with just a DrawTriangles with
an empty white image.

As a side effect, SubImage is avilable for Fill.

Fixes #1416

image.go

@@ -69,22 +69,39 @@ func (i *Image) Clear() {
 	i.Fill(color.Transparent)
 }
 
+var emptyImage = NewImage(3, 3)
+
+func init() {
+	w, h := emptyImage.Size()
+	pix := make([]byte, 4*w*h)
+	for i := range pix {
+		pix[i] = 0xff
+	}
+	// As emptyImage is used at Fill, use ReplacePixels instead.
+	emptyImage.ReplacePixels(pix)
+}
+
 // Fill fills the image with a solid color.
 //
 // When the image is disposed, Fill does nothing.
 func (i *Image) Fill(clr color.Color) {
-	i.copyCheck()
+	w, h := i.Size()
 
-	if i.isDisposed() {
+	op := &DrawImageOptions{}
-		return
+	op.GeoM.Scale(float64(w), float64(h))
+
+	r, g, b, a := clr.RGBA()
+	var rf, gf, bf, af float64
+	if a > 0 {
+		rf = float64(r) / float64(a)
+		gf = float64(g) / float64(a)
+		bf = float64(b) / float64(a)
+		af = float64(a) / 0xffff
 	}
+	op.ColorM.Scale(rf, gf, bf, af)
+	op.CompositeMode = CompositeModeCopy
 
-	// TODO: Implement this.
+	i.DrawImage(emptyImage.SubImage(image.Rect(1, 1, 2, 2)).(*Image), op)
-	if i.isSubImage() {
-		panic("ebiten: rendering to a sub-image is not implemented (Fill)")
-	}
-
-	i.mipmap.Fill(color.RGBAModel.Convert(clr).(color.RGBA))
 }
 
 func canSkipMipmap(geom GeoM, filter driver.Filter) bool {

internal/buffered/image.go

@@ -17,7 +17,6 @@ package buffered
 import (
 	"fmt"
 	"image"
-	"image/color"
 
 	"github.com/hajimehoshi/ebiten/v2/internal/affine"
 	"github.com/hajimehoshi/ebiten/v2/internal/driver"
@@ -31,9 +30,6 @@ type Image struct {
 	width  int
 	height int
 
-	hasFill   bool
-	fillColor color.RGBA
-
 	pixels               []byte
 	needsToResolvePixels bool
 }
@@ -105,13 +101,9 @@ func (i *Image) initializeAsScreenFramebuffer(width, height int) {
 func (i *Image) invalidatePendingPixels() {
 	i.pixels = nil
 	i.needsToResolvePixels = false
-	i.hasFill = false
 }
 
 func (i *Image) resolvePendingPixels(keepPendingPixels bool) {
-	if i.needsToResolvePixels && i.hasFill {
-		panic("buffered: needsToResolvePixels and hasFill must not be true at the same time")
-	}
 	if i.needsToResolvePixels {
 		i.img.ReplacePixels(i.pixels)
 		if !keepPendingPixels {
@@ -119,15 +111,6 @@ func (i *Image) resolvePendingPixels(keepPendingPixels bool) {
 		}
 		i.needsToResolvePixels = false
 	}
-	i.resolvePendingFill()
-}
-
-func (i *Image) resolvePendingFill() {
-	if !i.hasFill {
-		return
-	}
-	i.img.Fill(i.fillColor)
-	i.hasFill = false
 }
 
 func (i *Image) MarkDisposed() {
@@ -152,18 +135,6 @@ func (img *Image) Pixels(x, y, width, height int) (pix []byte, err error) {
 
 	pix = make([]byte, 4*width*height)
 
-	// If there are pixels or pending fillling that needs to be resolved, use this rather than resolving.
-	// Resolving them needs to access GPU and is expensive (#1137).
-	if img.hasFill {
-		for i := 0; i < len(pix)/4; i++ {
-			pix[4*i] = img.fillColor.R
-			pix[4*i+1] = img.fillColor.G
-			pix[4*i+2] = img.fillColor.B
-			pix[4*i+3] = img.fillColor.A
-		}
-		return pix, nil
-	}
-
 	if img.pixels == nil {
 		pix, err := img.img.Pixels(0, 0, img.width, img.height)
 		if err != nil {
@@ -183,22 +154,6 @@ func (i *Image) Dump(name string, blackbg bool) error {
 	return i.img.Dump(name, blackbg)
 }
 
-func (i *Image) Fill(clr color.RGBA) {
-	if maybeCanAddDelayedCommand() {
-		if tryAddDelayedCommand(func() error {
-			i.Fill(clr)
-			return nil
-		}) {
-			return
-		}
-	}
-
-	// Defer filling the image so that successive fillings will be merged into one (#1134).
-	i.invalidatePendingPixels()
-	i.fillColor = clr
-	i.hasFill = true
-}
-
 func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error {
 	if l := 4 * width * height; len(pix) != l {
 		panic(fmt.Sprintf("buffered: len(pix) was %d but must be %d", len(pix), l))
@@ -225,8 +180,6 @@ func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error {
 		return nil
 	}
 
-	i.resolvePendingFill()
-
 	// TODO: Can we use (*restorable.Image).ReplacePixels?
 	if i.pixels == nil {
 		pix, err := i.img.Pixels(0, 0, i.width, i.height)

internal/mipmap/mipmap.go

@@ -16,7 +16,6 @@ package mipmap
 
 import (
 	"fmt"
-	"image/color"
 	"math"
 
 	"github.com/hajimehoshi/ebiten/v2/internal/affine"
@@ -74,11 +73,6 @@ func (m *Mipmap) Dump(name string, blackbg bool) error {
 	return m.orig.Dump(name, blackbg)
 }
 
-func (m *Mipmap) Fill(clr color.RGBA) {
-	m.orig.Fill(clr)
-	m.disposeMipmaps()
-}
-
 func (m *Mipmap) ReplacePixels(pix []byte, x, y, width, height int) error {
 	if err := m.orig.ReplacePixels(pix, x, y, width, height); err != nil {
 		return err

internal/restorable/image.go

@@ -16,7 +16,6 @@ package restorable
 
 import (
 	"fmt"
-	"image/color"
 
 	"github.com/hajimehoshi/ebiten/v2/internal/affine"
 	"github.com/hajimehoshi/ebiten/v2/internal/driver"
@@ -25,16 +24,12 @@ import (
 )
 
 type Pixels struct {
-	baseColor    color.RGBA
 	rectToPixels *rectToPixels
 }
 
 // Apply applies the Pixels state to the given image especially for restoring.
 func (p *Pixels) Apply(img *graphicscommand.Image) {
 	// Pixels doesn't clear the image. This is a caller's responsibility.
-	if p.baseColor != (color.RGBA{}) {
-		fillImage(img, p.baseColor)
-	}
 
 	if p.rectToPixels == nil {
 		return
@@ -64,7 +59,7 @@ func (p *Pixels) At(i, j int) (byte, byte, byte, byte) {
 			return r, g, b, a
 		}
 	}
-	return p.baseColor.R, p.baseColor.G, p.baseColor.B, p.baseColor.A
+	return 0, 0, 0, 0
 }
 
 // drawTrianglesHistoryItem is an item for history of draw-image commands.
@@ -142,7 +137,7 @@ func NewImage(width, height int) *Image {
 		width:  width,
 		height: height,
 	}
-	fillImage(i.image, color.RGBA{})
+	clearImage(i.image)
 	theImages.add(i)
 	return i
 }
@@ -187,9 +182,6 @@ func (i *Image) Extend(width, height int) *Image {
 	newImg.SetVolatile(i.volatile)
 	i.basePixels.Apply(newImg.image)
 
-	if i.basePixels.baseColor != (color.RGBA{}) {
-		panic("restorable: baseColor must be empty at Extend")
-	}
 	newImg.basePixels = i.basePixels
 
 	i.Dispose()
@@ -209,7 +201,7 @@ func NewScreenFramebufferImage(width, height int) *Image {
 		height: height,
 		screen: true,
 	}
-	fillImage(i.image, color.RGBA{})
+	clearImage(i.image)
 	theImages.add(i)
 	return i
 }
@@ -224,49 +216,18 @@ func quadVertices(dx0, dy0, dx1, dy1, sx0, sy0, sx1, sy1, cr, cg, cb, ca float32
 	}
 }
 
-// Fill fills the specified part of the image with a solid color.
+func clearImage(i *graphicscommand.Image) {
-func (i *Image) Fill(clr color.RGBA) {
-	theImages.makeStaleIfDependingOn(i)
-	i.basePixels = Pixels{
-		baseColor: clr,
-	}
-	i.drawTrianglesHistory = nil
-	i.stale = false
-
-	// Do not call i.DrawTriangles as emptyImage is special (#928).
-	// baseColor is updated instead.
-	fillImage(i.image, i.basePixels.baseColor)
-}
-
-func fillImage(i *graphicscommand.Image, clr color.RGBA) {
 	if i == emptyImage.image {
 		panic("restorable: fillImage cannot be called on emptyImage")
 	}
 
-	var rf, gf, bf, af float32
-	if clr.A > 0 {
-		rf = float32(clr.R) / float32(clr.A)
-		gf = float32(clr.G) / float32(clr.A)
-		bf = float32(clr.B) / float32(clr.A)
-		af = float32(clr.A) / 0xff
-	}
-
-	// TODO: Use the previous composite mode if possible.
-	compositemode := driver.CompositeModeSourceOver
-	switch {
-	case af == 0.0:
-		compositemode = driver.CompositeModeClear
-	case af < 1.0:
-		compositemode = driver.CompositeModeCopy
-	}
-
 	// This needs to use 'InternalSize' to render the whole region, or edges are unexpectedly cleared on some
 	// devices.
 	//
 	// TODO: Can we unexport InternalSize()?
 	dw, dh := i.InternalSize()
 	sw, sh := emptyImage.width, emptyImage.height
-	vs := quadVertices(0, 0, float32(dw), float32(dh), 1, 1, float32(sw-1), float32(sh-1), rf, gf, bf, af)
+	vs := quadVertices(0, 0, float32(dw), float32(dh), 1, 1, float32(sw-1), float32(sh-1), 0, 0, 0, 0)
 	is := graphics.QuadIndices()
 	srcs := [graphics.ShaderImageNum]*graphicscommand.Image{emptyImage.image}
 	var offsets [graphics.ShaderImageNum - 1][2]float32
@@ -276,7 +237,7 @@ func fillImage(i *graphicscommand.Image, clr color.RGBA) {
 		Width:  float32(dw),
 		Height: float32(dh),
 	}
-	i.DrawTriangles(srcs, offsets, vs, is, nil, compositemode, driver.FilterNearest, driver.AddressUnsafe, dstRegion, driver.Region{}, nil, nil)
+	i.DrawTriangles(srcs, offsets, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressUnsafe, dstRegion, driver.Region{}, nil, nil)
 }
 
 // BasePixelsForTesting returns the image's basePixels for testing.
@@ -601,7 +562,7 @@ func (i *Image) restore() error {
 	}
 	if i.volatile {
 		i.image = graphicscommand.NewImage(w, h)
-		fillImage(i.image, color.RGBA{})
+		clearImage(i.image)
 		return nil
 	}
 	if i.stale {
@@ -611,9 +572,9 @@ func (i *Image) restore() error {
 	gimg := graphicscommand.NewImage(w, h)
 	// Clear the image explicitly.
 	if i != emptyImage {
-		// As fillImage uses emptyImage, fillImage cannot be called on emptyImage.
+		// As clearImage uses emptyImage, clearImage cannot be called on emptyImage.
 		// It is OK to skip this since emptyImage has its entire pixel information.
-		fillImage(gimg, color.RGBA{})
+		clearImage(gimg)
 	}
 	i.basePixels.Apply(gimg)
 

internal/restorable/images_test.go

@@ -862,69 +862,6 @@ func TestClearPixels(t *testing.T) {
 	img.ReplacePixels(make([]byte, 4*8*4), 0, 0, 8, 4)
 }
 
-func TestFill(t *testing.T) {
-	const w, h = 16, 16
-	img := NewImage(w, h)
-	img.Fill(color.RGBA{0xff, 0, 0, 0xff})
-	if err := ResolveStaleImages(); err != nil {
-		t.Fatal(err)
-	}
-	if err := RestoreIfNeeded(); err != nil {
-		t.Fatal(err)
-	}
-	for j := 0; j < h; j++ {
-		for i := 0; i < w; i++ {
-			r, g, b, a, err := img.At(i, j)
-			if err != nil {
-				t.Fatal(err)
-			}
-			got := color.RGBA{r, g, b, a}
-			want := color.RGBA{0xff, 0, 0, 0xff}
-			if got != want {
-				t.Errorf("img.At(%d, %d): got: %v, want: %v", i, j, got, want)
-			}
-		}
-	}
-}
-
-// Issue #1170
-func TestFill2(t *testing.T) {
-	const w, h = 16, 16
-	src := NewImage(w, h)
-	src.Fill(color.RGBA{0xff, 0, 0, 0xff})
-
-	dst := NewImage(w, h)
-	vs := quadVertices(w, h, 0, 0)
-	is := graphics.QuadIndices()
-	dr := driver.Region{
-		X:      0,
-		Y:      0,
-		Width:  w,
-		Height: h,
-	}
-	dst.DrawTriangles([graphics.ShaderImageNum]*Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil)
-
-	// Fill src with a different color. This should not affect dst.
-	src.Fill(color.RGBA{0, 0xff, 0, 0xff})
-
-	if err := ResolveStaleImages(); err != nil {
-		t.Fatal(err)
-	}
-	if err := RestoreIfNeeded(); err != nil {
-		t.Fatal(err)
-	}
-
-	for j := 0; j < h; j++ {
-		for i := 0; i < w; i++ {
-			got := pixelsToColor(dst.BasePixelsForTesting(), i, j)
-			want := color.RGBA{0xff, 0, 0, 0xff}
-			if got != want {
-				t.Errorf("img.At(%d, %d): got: %v, want: %v", i, j, got, want)
-			}
-		}
-	}
-}
-
 func TestMutateSlices(t *testing.T) {
 	const w, h = 16, 16
 	dst := NewImage(w, h)

internal/restorable/shader_test.go

@@ -24,6 +24,33 @@ import (
 	etesting "github.com/hajimehoshi/ebiten/v2/internal/testing"
 )
 
+var emptyImage = NewImage(3, 3)
+
+func clearImage(img *Image, w, h int) {
+	dx0 := float32(0)
+	dy0 := float32(0)
+	dx1 := float32(w)
+	dy1 := float32(h)
+	sx0 := float32(1)
+	sy0 := float32(1)
+	sx1 := float32(2)
+	sy1 := float32(2)
+	vs := []float32{
+		dx0, dy0, sx0, sy0, 0, 0, 0, 0,
+		dx1, dy0, sx1, sy0, 0, 0, 0, 0,
+		dx0, dy1, sx0, sy1, 0, 0, 0, 0,
+		dx1, dy1, sx1, sy1, 0, 0, 0, 0,
+	}
+	is := graphics.QuadIndices()
+	dr := driver.Region{
+		X:      0,
+		Y:      0,
+		Width:  float32(w),
+		Height: float32(h),
+	}
+	img.DrawTriangles([graphics.ShaderImageNum]*Image{emptyImage}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, nil, driver.CompositeModeClear, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, nil, nil)
+}
+
 func TestShader(t *testing.T) {
 	img := NewImage(1, 1)
 	defer img.Dispose()
@@ -114,7 +141,7 @@ func TestShaderMultipleSources(t *testing.T) {
 	dst.DrawTriangles(srcs, offsets, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, s, nil)
 
 	// Clear one of the sources after DrawTriangles. dst should not be affected.
-	srcs[0].Fill(color.RGBA{})
+	clearImage(srcs[0], 1, 1)
 
 	if err := ResolveStaleImages(); err != nil {
 		t.Fatal(err)
@@ -156,7 +183,7 @@ func TestShaderMultipleSourcesOnOneTexture(t *testing.T) {
 	dst.DrawTriangles(srcs, offsets, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, s, nil)
 
 	// Clear one of the sources after DrawTriangles. dst should not be affected.
-	srcs[0].Fill(color.RGBA{})
+	clearImage(srcs[0], 3, 1)
 
 	if err := ResolveStaleImages(); err != nil {
 		t.Fatal(err)

internal/shareable/image.go

@@ -16,7 +16,6 @@ package shareable
 
 import (
 	"fmt"
-	"image/color"
 	"runtime"
 	"sync"
 
@@ -421,26 +420,6 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, vertices []f
 	backendsM.Unlock()
 }
 
-func (i *Image) Fill(clr color.RGBA) {
-	backendsM.Lock()
-	defer backendsM.Unlock()
-
-	if i.disposed {
-		panic("shareable: the drawing target image must not be disposed (Fill)")
-	}
-	if i.backend == nil {
-		if _, _, _, a := clr.RGBA(); a == 0 {
-			return
-		}
-	}
-
-	i.ensureNotShared()
-
-	// As *restorable.Image is an independent image, it is fine to fill the entire image.
-	// TODO: Is it OK not to consider paddings?
-	i.backend.restorable.Fill(clr)
-}
-
 func (i *Image) ReplacePixels(pix []byte) {
 	backendsM.Lock()
 	defer backendsM.Unlock()