internal/atlas: bug fix: ReadPixels crashed when inFrame was false

This change fixes the issue by letting ReadPixels return a channel
and executing this asynchronously when needed.

Closes #2815

internal/atlas/image.go

@@ -539,18 +539,30 @@ func (i *Image) writePixels(pix []byte, region image.Rectangle) {
 	i.backend.restorable.WritePixels(pixb, r)
 }
 
-func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
+func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) chan error {
 	backendsM.Lock()
 	defer backendsM.Unlock()
 
 	if !inFrame {
-		panic("atlas: ReadPixels must be called in between BeginFrame and EndFrame")
+		ch := make(chan error)
+		deferredM.Lock()
+		deferred = append(deferred, func() {
+			ch <- i.readPixels(graphicsDriver, pixels, region)
+		})
+		deferredM.Unlock()
+		return ch
 	}
 
 	// In the tests, BeginFrame might not be called often and then images might not be disposed (#2292).
 	// To prevent memory leaks, flush the deferred functions here.
 	flushDeferred()
 
+	ch := make(chan error, 1)
+	ch <- i.readPixels(graphicsDriver, pixels, region)
+	return ch
+}
+
+func (i *Image) readPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
 	if i.backend == nil || i.backend.restorable == nil {
 		for i := range pixels {
 			pixels[i] = 0

internal/atlas/image_test.go

@@ -117,7 +117,7 @@ func TestEnsureIsolatedFromSourceBackend(t *testing.T) {
 	}
 
 	pix = make([]byte, 4*size*size)
-	if err := img4.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
+	if err := <-img4.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < size; j++ {
@@ -212,7 +212,7 @@ func TestReputOnSourceBackend(t *testing.T) {
 	atlas.PutImagesOnSourceBackendForTesting(ui.Get().GraphicsDriverForTesting())
 
 	pix = make([]byte, 4*size*size)
-	if err := img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
+	if err := <-img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < size; j++ {
@@ -236,7 +236,7 @@ func TestReputOnSourceBackend(t *testing.T) {
 	}
 
 	pix = make([]byte, 4*size*size)
-	if err := img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
+	if err := <-img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < size; j++ {
@@ -324,7 +324,7 @@ func TestExtend(t *testing.T) {
 	img1.WritePixels(p1, image.Rect(0, 0, w1, h1))
 
 	pix0 := make([]byte, 4*w0*h0)
-	if err := img0.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix0, image.Rect(0, 0, w0, h0)); err != nil {
+	if err := <-img0.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix0, image.Rect(0, 0, w0, h0)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < h0; j++ {
@@ -343,7 +343,7 @@ func TestExtend(t *testing.T) {
 	}
 
 	pix1 := make([]byte, 4*w1*h1)
-	if err := img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix1, image.Rect(0, 0, w1, h1)); err != nil {
+	if err := <-img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix1, image.Rect(0, 0, w1, h1)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < h1; j++ {
@@ -385,7 +385,7 @@ func TestWritePixelsAfterDrawTriangles(t *testing.T) {
 	dst.WritePixels(pix, image.Rect(0, 0, w, h))
 
 	pix = make([]byte, 4*w*h)
-	if err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
+	if err := <-dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < h; j++ {
@@ -427,7 +427,7 @@ func TestSmallImages(t *testing.T) {
 	dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src}, vs, is, graphicsdriver.BlendSourceOver, dr, [graphics.ShaderImageCount]image.Rectangle{}, atlas.NearestFilterShader, nil, false)
 
 	pix = make([]byte, 4*w*h)
-	if err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
+	if err := <-dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < h; j++ {
@@ -470,7 +470,7 @@ func TestLongImages(t *testing.T) {
 	dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src}, vs, is, graphicsdriver.BlendSourceOver, dr, [graphics.ShaderImageCount]image.Rectangle{}, atlas.NearestFilterShader, nil, false)
 
 	pix = make([]byte, 4*dstW*dstH)
-	if err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, dstW, dstH)); err != nil {
+	if err := <-dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, dstW, dstH)); err != nil {
 		t.Fatal(err)
 	}
 	for j := 0; j < h; j++ {
@@ -684,7 +684,7 @@ func TestImageWritePixelsModify(t *testing.T) {
 
 		// Check the pixels are the original ones.
 		pix = make([]byte, 4*size*size)
-		if err := img.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
+		if err := <-img.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size)); err != nil {
 			t.Fatal(err)
 		}
 		for j := 0; j < size; j++ {

internal/atlas/shader_test.go

@@ -44,7 +44,7 @@ func TestShaderFillTwice(t *testing.T) {
 	dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{}, vs, is, graphicsdriver.BlendCopy, dr, [graphics.ShaderImageCount]image.Rectangle{}, s1, nil, false)
 
 	pix := make([]byte, 4*w*h)
-	if err := dst.ReadPixels(g, pix, image.Rect(0, 0, w, h)); err != nil {
+	if err := <-dst.ReadPixels(g, pix, image.Rect(0, 0, w, h)); err != nil {
 		t.Error(err)
 	}
 	if got, want := (color.RGBA{R: pix[0], G: pix[1], B: pix[2], A: pix[3]}), (color.RGBA{R: 0x80, G: 0x80, B: 0x80, A: 0xff}); got != want {
@@ -71,7 +71,7 @@ func TestImageDrawTwice(t *testing.T) {
 	dst.DrawTriangles([graphics.ShaderImageCount]*atlas.Image{src1}, vs, is, graphicsdriver.BlendCopy, dr, [graphics.ShaderImageCount]image.Rectangle{}, atlas.NearestFilterShader, nil, false)
 
 	pix := make([]byte, 4*w*h)
-	if err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
+	if err := <-dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h)); err != nil {
 		t.Error(err)
 	}
 	if got, want := (color.RGBA{R: pix[0], G: pix[1], B: pix[2], A: pix[3]}), (color.RGBA{R: 0x80, G: 0x80, B: 0x80, A: 0xff}); got != want {

internal/buffered/image.go

@@ -58,9 +58,14 @@ func (i *Image) MarkDisposed() {
 }
 
 func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
+	// If this function is called from the game (Update/Draw) goroutine, and after EndFrame and before BeginFrame,
+	// (*atlas.Image).ReadPixels's channel never returns a value and causes a dead lock.
+	// It is assumed that this never happens so far, but if handling inputs after EndFrame is implemented,
+	// this might be possible (#1704).
+
 	// 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 {
+		if err := <-i.img.ReadPixels(graphicsDriver, pixels, region); err != nil {
 			return err
 		}
 		return nil
@@ -68,7 +73,7 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte
 
 	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 {
+		if err := <-i.img.ReadPixels(graphicsDriver, pix, image.Rect(0, 0, i.width, i.height)); err != nil {
 			return err
 		}
 		i.pixels = pix

internal/processtest/testdata/issue2815.go

@@ -0,0 +1,72 @@
+// Copyright 2023 The Ebitengine 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.
+
+//go:build ignore
+
+package main
+
+import (
+	"time"
+
+	"github.com/hajimehoshi/ebiten/v2"
+)
+
+type Game struct {
+	init  bool
+	count int
+	end0  chan struct{}
+	end1  chan struct{}
+}
+
+func (g *Game) Update() error {
+	if !g.init {
+		g.end0 = make(chan struct{})
+		g.end1 = make(chan struct{})
+		img := ebiten.NewImage(1, 1)
+		go func() {
+			t := time.Tick(time.Microsecond)
+		loop:
+			for {
+				select {
+				case <-t:
+					img.At(0, 0)
+				case <-g.end0:
+					close(g.end1)
+					break loop
+				}
+			}
+		}()
+		g.init = true
+	}
+	g.count++
+	if g.count >= 60 {
+		close(g.end0)
+		<-g.end1
+		return ebiten.Termination
+	}
+	return nil
+}
+
+func (g *Game) Draw(screen *ebiten.Image) {
+}
+
+func (g *Game) Layout(w, h int) (int, int) {
+	return 320, 240
+}
+
+func main() {
+	if err := ebiten.RunGame(&Game{}); err != nil {
+		panic(err)
+	}
+}