From 3f6628f1cca1839a0a0cfed3d1a2a2f4507b177c Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 20 Jul 2019 04:53:18 +0900 Subject: [PATCH] restorable: Replace Fill with Clear --- internal/restorable/image.go | 66 ++++++++---------------------- internal/restorable/images_test.go | 53 ++++++------------------ internal/shareable/shareable.go | 2 +- 3 files changed, 31 insertions(+), 90 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index dc707ed64..64b4358e5 100644 --- a/internal/restorable/image.go +++ b/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") - } - - 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 + panic("restorable: clearImage cannot be called on emptyImage") } // 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 - if a == 0 { - // The first DrawTriangles must be clear mode for initialization. - // TODO: Can the graphicscommand package hide this knowledge? - c = driver.CompositeModeClear - } - img.DrawTriangles(emptyImage.image, vs, is, nil, c, driver.FilterNearest, driver.AddressClampToZero) + // The first DrawTriangles must be clear mode for initialization. + // TODO: Can the graphicscommand package hide this knowledge? + img.DrawTriangles(emptyImage.image, vs, is, nil, driver.CompositeModeClear, 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{ - color: color.RGBA{r, g, b, a}, - } + i.basePixels = &Pixels{} 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) } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index 6e2fbc9f1..8f2e8932a 100644 --- a/internal/restorable/images_test.go +++ b/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) diff --git a/internal/shareable/shareable.go b/internal/shareable/shareable.go index fabd8d844..512449628 100644 --- a/internal/shareable/shareable.go +++ b/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) {