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

Updates #805

internal/atlas/image.go

@@ -817,11 +817,6 @@ 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(graphicsDriver)
 

internal/buffered/image.go

@@ -21,7 +21,6 @@ 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"
 )
 
 type Image struct {
@@ -50,14 +49,6 @@ func (i *Image) Deallocate() {
 }
 
 func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
-	// If restorable.AlwaysReadPixelsFromGPU() returns false, the pixel data is cached in the restorable package.
-	if !restorable.AlwaysReadPixelsFromGPU() {
-		if err := i.img.ReadPixels(graphicsDriver, pixels, region); err != nil {
-			return err
-		}
-		return nil
-	}
-
 	if i.pixels == nil {
 		pix := make([]byte, 4*i.width*i.height)
 		if err := i.img.ReadPixels(graphicsDriver, pix, image.Rect(0, 0, i.width, i.height)); err != nil {

internal/restorable/export_test.go

@@ -16,14 +16,8 @@ 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

@@ -219,7 +219,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
 	}
 
@@ -278,36 +278,7 @@ func (i *Image) WritePixels(pixels *graphics.ManagedBytes, region image.Rectangl
 	}
 
 	// Even if the image is already stale, call makeStale to extend the stale region.
-	if !needsRestoring() || !i.needsRestoring() || i.stale {
+	i.makeStale(region)
-		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.
@@ -328,24 +299,8 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageCount]*Image, vertices [
 	}
 	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.
-	if srcstale || !needsRestoring() || !i.needsRestoring() || i.stale {
+	i.makeStale(dstRegion)
-		i.makeStale(dstRegion)
-	} else {
-		i.appendDrawTrianglesHistory(srcs, vertices, indices, blend, dstRegion, srcRegions, shader, uniforms, fillRule)
-	}
 
 	var imgs [graphics.ShaderImageCount]*graphicscommand.Image
 	for i, src := range srcs {
@@ -362,7 +317,7 @@ func (i *Image) appendDrawTrianglesHistory(srcs [graphics.ShaderImageCount]*Imag
 	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")
 	}
 
@@ -408,7 +363,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,
@@ -504,20 +459,6 @@ 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,22 +20,8 @@ import (
 	"github.com/hajimehoshi/ebiten/v2/internal/graphicsdriver"
 )
 
-// forceRestoring reports whether restoring forcely happens or not.
+func alwaysReadPixelsFromGPU() bool {
-var forceRestoring = false
+	return true
-
-// 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.
@@ -60,56 +46,10 @@ func SwapBuffers(graphicsDriver graphicsdriver.Graphics) error {
 		}
 		graphicscommand.LogImagesInfo(imgs)
 	}
-	return resolveStaleImages(graphicsDriver, true)
+	if err := graphicscommand.FlushCommands(graphicsDriver, true); err != nil {
-}
-
-// 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
 	}
-	if !needsRestoring() {
+	return nil
-		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
-
-		// As isInvalidated() is expensive, call this only for one image.
-		// This assumes that if there is one image that is invalidated, all images are invalidated.
-		for img := range theImages.images {
-			// The screen image might not have a texture. Skip this.
-			if img.imageType == ImageTypeScreen {
-				continue
-			}
-			var err error
-			r, err = img.isInvalidated(graphicsDriver)
-			if err != nil {
-				return err
-			}
-			break
-		}
-
-		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.
@@ -144,17 +84,6 @@ 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.
@@ -182,83 +111,6 @@ 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

@@ -1,200 +0,0 @@
-// 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.ShaderImageCount]*restorable.Image{emptyImage}, vs, is, graphicsdriver.BlendClear, dr, [graphics.ShaderImageCount]image.Rectangle{}, restorable.NearestFilterShader, nil, graphicsdriver.FillAll)
-}
-
-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.ShaderImageCount]*restorable.Image{}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillAll)
-
-	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.ShaderImageCount]*restorable.Image{imgs[i]}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillAll)
-	}
-
-	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.ShaderImageCount]*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.ShaderImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillAll)
-
-	// 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.ShaderImageCount]*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.ShaderImageCount]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.FillAll)
-
-	// 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.ShaderImageCount]*restorable.Image{}, quadVertices(1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.BlendCopy, dr, [graphics.ShaderImageCount]image.Rectangle{}, s, nil, graphicsdriver.FillAll)
-
-	// 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)
-	}
-}