From a3b35e5fba55bba222b260ee6d1692bb1fd6bbdd Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sun, 21 Jul 2019 14:03:14 +0900 Subject: [PATCH] Revert "restorable: Refactoring" This reverts commit 777cab5cc32ab7ce10c81ed2b247b79dfec9f158. Reason: Test failures: https://travis-ci.org/hajimehoshi/ebiten/builds/561553656 --- internal/restorable/image.go | 55 +++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index b64a53878..925a2f0d2 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -28,13 +28,9 @@ type Pixels struct { rectToPixels *rectToPixels } -// Apply applies the Pixels state to the given image especially for restoring. -// -// Apply must work even with an image not initialized. func (p *Pixels) Apply(img *graphicscommand.Image) { - // TODO: Clearing is not needed when the image is for only ReplacePixels. - clearImage(img) if p.rectToPixels == nil { + clearImage(img) return } @@ -81,13 +77,14 @@ type drawTrianglesHistoryItem struct { type Image struct { image *graphicscommand.Image - basePixels Pixels + basePixels *Pixels // drawTrianglesHistory is a set of draw-image commands. // TODO: This should be merged with the similar command queue in package graphics (#433). 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. @@ -128,7 +125,7 @@ func NewImage(width, height int) *Image { i := &Image{ image: graphicscommand.NewImage(width, height), } - i.clear() + i.clearForInitialization() theImages.add(i) return i } @@ -181,7 +178,7 @@ func NewScreenFramebufferImage(width, height int) *Image { image: graphicscommand.NewScreenFramebufferImage(width, height), screen: true, } - i.clear() + i.clearForInitialization() theImages.add(i) return i } @@ -191,6 +188,12 @@ func (i *Image) Clear() { 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.clear() +} + // clearImage clears a graphicscommand.Image. // This does nothing to do with a restorable.Image's rendering state. func clearImage(img *graphicscommand.Image) { @@ -229,7 +232,7 @@ func (i *Image) clear() { // // After ResetRestoringState, the image is assumed to be cleared. func (i *Image) ResetRestoringState() { - i.basePixels = Pixels{} + i.basePixels = &Pixels{} i.drawTrianglesHistory = nil i.stale = false } @@ -240,7 +243,7 @@ func (i *Image) IsVolatile() bool { // BasePixelsForTesting returns the image's basePixels for testing. func (i *Image) BasePixelsForTesting() *Pixels { - return &i.basePixels + return i.basePixels } // Size returns the image's size. @@ -278,7 +281,7 @@ func (i *Image) PutVertex(vs []float32, dx, dy, sx, sy float32, bx0, by0, bx1, b // makeStale makes the image stale. func (i *Image) makeStale() { - i.basePixels = Pixels{} + i.basePixels = nil i.drawTrianglesHistory = nil i.stale = true @@ -319,6 +322,9 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } if x == 0 && y == 0 && width == w && height == h { + if i.basePixels == nil { + i.basePixels = &Pixels{} + } if pixels != nil { i.basePixels.AddOrReplace(pixels, 0, 0, w, h) } else { @@ -341,6 +347,9 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { return } + if i.basePixels == nil { + i.basePixels = &Pixels{} + } if pixels != nil { i.basePixels.AddOrReplace(pixels, x, y, width, height) } else { @@ -392,7 +401,7 @@ func (i *Image) appendDrawTrianglesHistory(image *Image, vertices []float32, ind } func (i *Image) readPixelsFromGPUIfNeeded() { - if len(i.drawTrianglesHistory) > 0 || i.stale { + if i.basePixels == nil || len(i.drawTrianglesHistory) > 0 || i.stale { graphicscommand.FlushCommands() i.readPixelsFromGPU() i.drawTrianglesHistory = nil @@ -411,6 +420,11 @@ func (i *Image) At(x, y int) (byte, byte, byte, byte) { i.readPixelsFromGPUIfNeeded() + // Even after readPixelsFromGPU, basePixels might be nil when OpenGL error happens. + if i.basePixels == nil { + return 0, 0, 0, 0 + } + return i.basePixels.At(x, y) } @@ -427,7 +441,7 @@ func (i *Image) makeStaleIfDependingOn(target *Image) { // readPixelsFromGPU reads the pixels from GPU and resolves the image's 'stale' state. func (i *Image) readPixelsFromGPU() { w, h := i.Size() - i.basePixels = Pixels{} + i.basePixels = &Pixels{} i.basePixels.AddOrReplace(i.image.Pixels(), 0, 0, w, h) i.drawTrianglesHistory = nil i.stale = false @@ -485,14 +499,14 @@ func (i *Image) restore() error { // The screen image should also be recreated because framebuffer might // be changed. i.image = graphicscommand.NewScreenFramebufferImage(w, h) - i.basePixels = Pixels{} + i.basePixels = nil i.drawTrianglesHistory = nil i.stale = false return nil } if i.volatile { i.image = graphicscommand.NewImage(w, h) - i.clear() + i.clearForInitialization() return nil } if i.stale { @@ -501,8 +515,11 @@ func (i *Image) restore() error { } gimg := graphicscommand.NewImage(w, h) - // gimg is not initialized (by clearing), but Apply should work. - i.basePixels.Apply(gimg) + // Clear the image explicitly. + clearImage(gimg) + if i.basePixels != nil { + i.basePixels.Apply(gimg) + } for _, c := range i.drawTrianglesHistory { if c.image.hasDependency() { @@ -512,7 +529,7 @@ func (i *Image) restore() error { } if len(i.drawTrianglesHistory) > 0 { - i.basePixels = Pixels{} + i.basePixels = &Pixels{} i.basePixels.AddOrReplace(gimg.Pixels(), 0, 0, w, h) } @@ -530,7 +547,7 @@ func (i *Image) Dispose() { i.image.Dispose() i.image = nil - i.basePixels = Pixels{} + i.basePixels = nil i.drawTrianglesHistory = nil i.stale = false }