Revert "internal/restorable: remove the case when the restoring is needed"

This reverts commit c08a2193a9.

Updates #3083

internal/atlas/image.go

@@ -833,6 +833,11 @@ func BeginFrame(graphicsDriver graphicsdriver.Graphics) error {
 		return err
 	}
 
+	// Restore images first before other image manipulations (#2075).
+	if err := restorable.RestoreIfNeeded(graphicsDriver); err != nil {
+		return err
+	}
+
 	flushDeferred()
 	putImagesOnSourceBackend()
 

internal/buffered/image.go

@@ -21,6 +21,7 @@ import (
 	"github.com/hajimehoshi/ebiten/v2/internal/atlas"
 	"github.com/hajimehoshi/ebiten/v2/internal/graphics"
 	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
+	"github.com/hajimehoshi/ebiten/v2/internal/restorable"
 )
 
 var whiteImage *Image
@@ -46,6 +47,8 @@ type Image struct {
 	// pixels is cached pixels for ReadPixels.
 	// pixels might be out of sync with GPU.
 	// The data of pixels is the secondary data of pixels for ReadPixels.
+	//
+	// pixels is always nil when restorable.AlwaysReadPixelsFromGPU() returns false.
 	pixels []byte
 
 	// pixelsUnsynced represents whether the pixels in CPU and GPU are not synced.
@@ -68,9 +71,6 @@ func (i *Image) Deallocate() {
 }
 
 func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) (bool, error) {
-	// Do not call flushDotsBufferIfNeeded here. This would slow (image/draw).Draw.
-	// See ebiten.TestImageDrawOver.
-
 	if region.Dx() == 1 && region.Dy() == 1 {
 		if c, ok := i.dotsBuffer[region.Min]; ok {
 			copy(pixels, c[:])
@@ -78,6 +78,19 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte
 		}
 	}
 
+	// If restorable.AlwaysReadPixelsFromGPU() returns false, the pixel data is cached in the restorable package.
+	if !restorable.AlwaysReadPixelsFromGPU() {
+		i.syncPixelsIfNeeded()
+		ok, err := i.img.ReadPixels(graphicsDriver, pixels, region)
+		if err != nil {
+			return false, err
+		}
+		return ok, nil
+	}
+
+	// Do not call syncPixelsIfNeeded here. This would slow (image/draw).Draw.
+	// See ebiten.TestImageDrawOver.
+
 	if i.pixels == nil {
 		pix := make([]byte, 4*i.width*i.height)
 		ok, err := i.img.ReadPixels(graphicsDriver, pix, image.Rect(0, 0, i.width, i.height))

internal/restorable/export_test.go

@@ -16,8 +16,14 @@ package restorable
 
 import (
 	"image"
+
+	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
 )
 
+func ResolveStaleImages(graphicsDriver graphicsdriver.Graphics) error {
+	return resolveStaleImages(graphicsDriver, false)
+}
+
 func AppendRegionRemovingDuplicates(regions *[]image.Rectangle, region image.Rectangle) {
 	appendRegionRemovingDuplicates(regions, region)
 }

internal/restorable/image.go

@@ -211,7 +211,7 @@ func (i *Image) makeStale(rect image.Rectangle) {
 	i.stale = true
 
 	// If ReadPixels always reads pixels from GPU, staleRegions are never used.
-	if alwaysReadPixelsFromGPU() {
+	if AlwaysReadPixelsFromGPU() {
 		return
 	}
 
@@ -270,7 +270,36 @@ func (i *Image) WritePixels(pixels *graphics.ManagedBytes, region image.Rectangl
 	}
 
 	// Even if the image is already stale, call makeStale to extend the stale region.
-	i.makeStale(region)
+	if !needsRestoring() || !i.needsRestoring() || i.stale {
+		i.makeStale(region)
+		return
+	}
+
+	if region.Eq(image.Rect(0, 0, w, h)) {
+		if pixels != nil {
+			// Clone a ManagedBytes as the package graphicscommand has a different lifetime management.
+			i.basePixels.AddOrReplace(pixels.Clone(), image.Rect(0, 0, w, h))
+		} else {
+			i.basePixels.Clear(image.Rect(0, 0, w, h))
+		}
+		i.clearDrawTrianglesHistory()
+		i.stale = false
+		i.staleRegions = i.staleRegions[:0]
+		return
+	}
+
+	// Records for DrawTriangles cannot come before records for WritePixels.
+	if len(i.drawTrianglesHistory) > 0 {
+		i.makeStale(region)
+		return
+	}
+
+	if pixels != nil {
+		// Clone a ManagedBytes as the package graphicscommand has a different lifetime management.
+		i.basePixels.AddOrReplace(pixels.Clone(), region)
+	} else {
+		i.basePixels.Clear(region)
+	}
 }
 
 // DrawTriangles draws triangles with the given image.
@@ -291,8 +320,24 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderSrcImageCount]*Image, vertice
 	}
 	theImages.makeStaleIfDependingOn(i)
 
+	// TODO: Add tests to confirm this logic.
+	var srcstale bool
+	for _, src := range srcs {
+		if src == nil {
+			continue
+		}
+		if src.stale || src.imageType == ImageTypeVolatile {
+			srcstale = true
+			break
+		}
+	}
+
 	// Even if the image is already stale, call makeStale to extend the stale region.
-	i.makeStale(dstRegion)
+	if srcstale || !needsRestoring() || !i.needsRestoring() || i.stale {
+		i.makeStale(dstRegion)
+	} else {
+		i.appendDrawTrianglesHistory(srcs, vertices, indices, blend, dstRegion, srcRegions, shader, uniforms, fillRule)
+	}
 
 	var imgs [graphics.ShaderSrcImageCount]*graphicscommand.Image
 	for i, src := range srcs {
@@ -309,7 +354,7 @@ func (i *Image) appendDrawTrianglesHistory(srcs [graphics.ShaderSrcImageCount]*I
 	if i.stale || !i.needsRestoring() {
 		panic("restorable: an image must not be stale or need restoring at appendDrawTrianglesHistory")
 	}
-	if alwaysReadPixelsFromGPU() {
+	if AlwaysReadPixelsFromGPU() {
 		panic("restorable: appendDrawTrianglesHistory must not be called when AlwaysReadPixelsFromGPU() returns true")
 	}
 
@@ -355,7 +400,7 @@ func (i *Image) readPixelsFromGPUIfNeeded(graphicsDriver graphicsdriver.Graphics
 }
 
 func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
-	if alwaysReadPixelsFromGPU() {
+	if AlwaysReadPixelsFromGPU() {
 		if err := i.image.ReadPixels(graphicsDriver, []graphicsdriver.PixelsArgs{
 			{
 				Pixels: pixels,
@@ -451,6 +496,20 @@ func (i *Image) readPixelsFromGPU(graphicsDriver graphicsdriver.Graphics) error
 	return nil
 }
 
+// resolveStale resolves the image's 'stale' state.
+func (i *Image) resolveStale(graphicsDriver graphicsdriver.Graphics) error {
+	if !needsRestoring() {
+		return nil
+	}
+	if !i.needsRestoring() {
+		return nil
+	}
+	if !i.stale {
+		return nil
+	}
+	return i.readPixelsFromGPU(graphicsDriver)
+}
+
 // dependsOn reports whether the image depends on target.
 func (i *Image) dependsOn(target *Image) bool {
 	for _, c := range i.drawTrianglesHistory {

internal/restorable/images.go

@@ -20,8 +20,22 @@ import (
 	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
 )
 
-func alwaysReadPixelsFromGPU() bool {
-	return true
+// forceRestoring reports whether restoring forcely happens or not.
+var forceRestoring = false
+
+// needsRestoring reports whether restoring process works or not.
+func needsRestoring() bool {
+	return forceRestoring
+}
+
+// AlwaysReadPixelsFromGPU reports whether ReadPixels always reads pixels from GPU or not.
+func AlwaysReadPixelsFromGPU() bool {
+	return !needsRestoring()
+}
+
+// EnableRestoringForTesting forces to enable restoring for testing.
+func EnableRestoringForTesting() {
+	forceRestoring = true
 }
 
 // images is a set of Image objects.
@@ -46,10 +60,43 @@ func SwapBuffers(graphicsDriver graphicsdriver.Graphics) error {
 		}
 		graphicscommand.LogImagesInfo(imgs)
 	}
-	if err := graphicscommand.FlushCommands(graphicsDriver, true); err != nil {
+	return resolveStaleImages(graphicsDriver, true)
+}
+
+// resolveStaleImages flushes the queued draw commands and resolves all stale images.
+// If endFrame is true, the current screen might be used to present when flushing the commands.
+func resolveStaleImages(graphicsDriver graphicsdriver.Graphics, endFrame bool) error {
+	if err := graphicscommand.FlushCommands(graphicsDriver, endFrame); err != nil {
 		return err
 	}
-	return nil
+	if !needsRestoring() {
+		return nil
+	}
+	return theImages.resolveStaleImages(graphicsDriver)
+}
+
+// RestoreIfNeeded restores the images.
+//
+// Restoring means to make all *graphicscommand.Image objects have their textures and framebuffers.
+func RestoreIfNeeded(graphicsDriver graphicsdriver.Graphics) error {
+	if !needsRestoring() {
+		return nil
+	}
+
+	if !forceRestoring {
+		var r bool
+
+		// TODO: Detect context lost explicitly on Android.
+
+		if !r {
+			return nil
+		}
+	}
+
+	if err := graphicscommand.ResetGraphicsDriverState(graphicsDriver); err != nil {
+		return err
+	}
+	return theImages.restore(graphicsDriver)
 }
 
 // DumpImages dumps all the current images to the specified directory.
@@ -84,6 +131,17 @@ func (i *images) removeShader(shader *Shader) {
 	delete(i.shaders, shader)
 }
 
+// resolveStaleImages resolves stale images.
+func (i *images) resolveStaleImages(graphicsDriver graphicsdriver.Graphics) error {
+	i.lastTarget = nil
+	for img := range i.images {
+		if err := img.resolveStale(graphicsDriver); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // makeStaleIfDependingOn makes all the images stale that depend on target.
 //
 // When target is modified, all images depending on target can't be restored with target.
@@ -111,6 +169,83 @@ func (i *images) makeStaleIfDependingOnShader(shader *Shader) {
 	}
 }
 
+// restore restores the images.
+//
+// Restoring means to make all *graphicscommand.Image objects have their textures and framebuffers.
+func (i *images) restore(graphicsDriver graphicsdriver.Graphics) error {
+	if !needsRestoring() {
+		panic("restorable: restore cannot be called when restoring is disabled")
+	}
+
+	// Dispose all the shaders ahead of restoring. A current shader ID and a new shader ID can be duplicated.
+	for s := range i.shaders {
+		s.shader.Dispose()
+		s.shader = nil
+	}
+	for s := range i.shaders {
+		s.restore()
+	}
+
+	// Dispose all the images ahead of restoring. A current texture ID and a new texture ID can be duplicated.
+	// TODO: Write a test to confirm that ID duplication never happens.
+	for i := range i.images {
+		i.image.Dispose()
+		i.image = nil
+	}
+
+	// Let's do topological sort based on dependencies of drawing history.
+	// It is assured that there are not loops since cyclic drawing makes images stale.
+	type edge struct {
+		source *Image
+		target *Image
+	}
+	images := map[*Image]struct{}{}
+	for i := range i.images {
+		images[i] = struct{}{}
+	}
+	edges := map[edge]struct{}{}
+	for t := range images {
+		for s := range t.dependingImages() {
+			edges[edge{source: s, target: t}] = struct{}{}
+		}
+	}
+
+	var sorted []*Image
+	for len(images) > 0 {
+		// current represents images that have no incoming edges.
+		current := map[*Image]struct{}{}
+		for i := range images {
+			current[i] = struct{}{}
+		}
+		for e := range edges {
+			if _, ok := current[e.target]; ok {
+				delete(current, e.target)
+			}
+		}
+		for i := range current {
+			delete(images, i)
+			sorted = append(sorted, i)
+		}
+		removed := []edge{}
+		for e := range edges {
+			if _, ok := current[e.source]; ok {
+				removed = append(removed, e)
+			}
+		}
+		for _, e := range removed {
+			delete(edges, e)
+		}
+	}
+
+	for _, img := range sorted {
+		if err := img.restore(graphicsDriver); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 var graphicsDriverInitialized bool
 
 // InitializeGraphicsDriverState initializes the graphics driver state.

internal/restorable/shader_test.go

@@ -0,0 +1,200 @@
+// Copyright 2018 The Ebiten Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package restorable_test
+
+import (
+	"image"
+	"image/color"
+	"testing"
+
+	"github.com/hajimehoshi/ebiten/v2/internal/graphics"
+	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
+	"github.com/hajimehoshi/ebiten/v2/internal/restorable"
+	etesting "github.com/hajimehoshi/ebiten/v2/internal/testing"
+	"github.com/hajimehoshi/ebiten/v2/internal/ui"
+)
+
+func clearImage(img *restorable.Image, w, h int) {
+	emptyImage := restorable.NewImage(3, 3, restorable.ImageTypeRegular)
+	defer emptyImage.Dispose()
+
+	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 := image.Rect(0, 0, w, h)
+	img.DrawTriangles([graphics.ShaderSrcImageCount]*restorable.Image{emptyImage}, vs, is, graphicsdriver.BlendClear, dr, [graphics.ShaderSrcImageCount]image.Rectangle{}, restorable.NearestFilterShader, nil, graphicsdriver.FillRuleFillAll)
+}
+
+func TestShader(t *testing.T) {
+	img := restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+	defer img.Dispose()
+
+	s := restorable.NewShader(etesting.ShaderProgramFill(0xff, 0, 0, 0xff))
+	dr := image.Rect(0, 0, 1, 1)
+	img.DrawTriangles([graphics.ShaderSrcImageCount]*restorable.Image{}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderSrcImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillRuleFillAll)
+
+	if err := restorable.ResolveStaleImages(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+	if err := restorable.RestoreIfNeeded(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+
+	want := color.RGBA{R: 0xff, A: 0xff}
+	got := pixelsToColor(img.BasePixelsForTesting(), 0, 0, 1, 1)
+	if !sameColors(got, want, 1) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestShaderChain(t *testing.T) {
+	const num = 10
+	imgs := []*restorable.Image{}
+	for i := 0; i < num; i++ {
+		img := restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+		defer img.Dispose()
+		imgs = append(imgs, img)
+	}
+
+	imgs[0].WritePixels(bytesToManagedBytes([]byte{0xff, 0, 0, 0xff}), image.Rect(0, 0, 1, 1))
+
+	s := restorable.NewShader(etesting.ShaderProgramImages(1))
+	for i := 0; i < num-1; i++ {
+		dr := image.Rect(0, 0, 1, 1)
+		imgs[i+1].DrawTriangles([graphics.ShaderSrcImageCount]*restorable.Image{imgs[i]}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderSrcImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillRuleFillAll)
+	}
+
+	if err := restorable.ResolveStaleImages(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+	if err := restorable.RestoreIfNeeded(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+
+	for i, img := range imgs {
+		want := color.RGBA{R: 0xff, A: 0xff}
+		got := pixelsToColor(img.BasePixelsForTesting(), 0, 0, 1, 1)
+		if !sameColors(got, want, 1) {
+			t.Errorf("%d: got %v, want %v", i, got, want)
+		}
+	}
+}
+
+func TestShaderMultipleSources(t *testing.T) {
+	var srcs [graphics.ShaderSrcImageCount]*restorable.Image
+	for i := range srcs {
+		srcs[i] = restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+	}
+	srcs[0].WritePixels(bytesToManagedBytes([]byte{0x40, 0, 0, 0xff}), image.Rect(0, 0, 1, 1))
+	srcs[1].WritePixels(bytesToManagedBytes([]byte{0, 0x80, 0, 0xff}), image.Rect(0, 0, 1, 1))
+	srcs[2].WritePixels(bytesToManagedBytes([]byte{0, 0, 0xc0, 0xff}), image.Rect(0, 0, 1, 1))
+
+	dst := restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+
+	s := restorable.NewShader(etesting.ShaderProgramImages(3))
+	dr := image.Rect(0, 0, 1, 1)
+	dst.DrawTriangles(srcs, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderSrcImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillRuleFillAll)
+
+	// Clear one of the sources after DrawTriangles. dst should not be affected.
+	clearImage(srcs[0], 1, 1)
+
+	if err := restorable.ResolveStaleImages(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+	if err := restorable.RestoreIfNeeded(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+
+	want := color.RGBA{R: 0x40, G: 0x80, B: 0xc0, A: 0xff}
+	got := pixelsToColor(dst.BasePixelsForTesting(), 0, 0, 1, 1)
+	if !sameColors(got, want, 1) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestShaderMultipleSourcesOnOneTexture(t *testing.T) {
+	src := restorable.NewImage(3, 1, restorable.ImageTypeRegular)
+	src.WritePixels(bytesToManagedBytes([]byte{
+		0x40, 0, 0, 0xff,
+		0, 0x80, 0, 0xff,
+		0, 0, 0xc0, 0xff,
+	}), image.Rect(0, 0, 3, 1))
+	srcs := [graphics.ShaderSrcImageCount]*restorable.Image{src, src, src}
+
+	dst := restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+
+	s := restorable.NewShader(etesting.ShaderProgramImages(3))
+	dr := image.Rect(0, 0, 1, 1)
+	srcRegions := [graphics.ShaderSrcImageCount]image.Rectangle{
+		image.Rect(0, 0, 1, 1),
+		image.Rect(1, 0, 2, 1),
+		image.Rect(2, 0, 3, 1),
+	}
+	dst.DrawTriangles(srcs, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, srcRegions, s, nil, graphicsdriver.FillRuleFillAll)
+
+	// Clear one of the sources after DrawTriangles. dst should not be affected.
+	clearImage(srcs[0], 3, 1)
+
+	if err := restorable.ResolveStaleImages(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+	if err := restorable.RestoreIfNeeded(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+
+	want := color.RGBA{R: 0x40, G: 0x80, B: 0xc0, A: 0xff}
+	got := pixelsToColor(dst.BasePixelsForTesting(), 0, 0, 1, 1)
+	if !sameColors(got, want, 1) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+func TestShaderDispose(t *testing.T) {
+	img := restorable.NewImage(1, 1, restorable.ImageTypeRegular)
+	defer img.Dispose()
+
+	s := restorable.NewShader(etesting.ShaderProgramFill(0xff, 0, 0, 0xff))
+	dr := image.Rect(0, 0, 1, 1)
+	img.DrawTriangles([graphics.ShaderSrcImageCount]*restorable.Image{}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderSrcImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillRuleFillAll)
+
+	// Dispose the shader. This should invalidate all the images using this shader i.e., all the images become
+	// stale.
+	s.Dispose()
+
+	if err := restorable.ResolveStaleImages(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+	if err := restorable.RestoreIfNeeded(ui.Get().GraphicsDriverForTesting()); err != nil {
+		t.Fatal(err)
+	}
+
+	want := color.RGBA{R: 0xff, A: 0xff}
+	got := pixelsToColor(img.BasePixelsForTesting(), 0, 0, 1, 1)
+	if !sameColors(got, want, 1) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}