internal/atlas: let callers retry ReadPixels instead of blocking

This is a preparation to implement HandleInput, which might call
(*Image).At in its callback.

Updates #1704

internal/atlas/image.go

@@ -539,33 +539,23 @@ 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) chan error {
+func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) (ok bool, err error) {
 	backendsM.Lock()
 	defer backendsM.Unlock()
 
 	if !inFrame {
-		ch := make(chan error)
+		// Not ready to read pixels. Try this later.
-		deferredM.Lock()
+		return false, nil
-		deferred = append(deferred, func() {
-			if err := i.readPixels(graphicsDriver, pixels, region); err != nil {
-				ch <- err
-			}
-			close(ch)
-		})
-		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)
 	if err := i.readPixels(graphicsDriver, pixels, region); err != nil {
-		ch <- err
+		return false, err
 	}
-	close(ch)
+	return true, nil
-	return ch
 }
 
 func (i *Image) readPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {

internal/atlas/image_test.go

@@ -117,9 +117,13 @@ 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 {
+	ok, err := img4.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < size; j++ {
 		for i := 0; i < size; i++ {
 			r := pix[4*(size*j+i)]
@@ -212,9 +216,13 @@ 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 {
+	ok, err := img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < size; j++ {
 		for i := 0; i < size; i++ {
 			want := color.RGBA{R: byte(i + j), G: byte(i + j), B: byte(i + j), A: byte(i + j)}
@@ -236,9 +244,13 @@ 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 {
+	ok, err = img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < size; j++ {
 		for i := 0; i < size; i++ {
 			want := color.RGBA{R: byte(i + j), G: byte(i + j), B: byte(i + j), A: byte(i + j)}
@@ -324,9 +336,13 @@ 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 {
+	ok, err := img0.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix0, image.Rect(0, 0, w0, h0))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < h0; j++ {
 		for i := 0; i < w0; i++ {
 			r := pix0[4*(w0*j+i)]
@@ -343,9 +359,13 @@ 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 {
+	ok, err = img1.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix1, image.Rect(0, 0, w1, h1))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < h1; j++ {
 		for i := 0; i < w1; i++ {
 			r := pix1[4*(w1*j+i)]
@@ -385,9 +405,13 @@ 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 {
+	ok, err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < h; j++ {
 		for i := 0; i < w; i++ {
 			r := pix[4*(w*j+i)]
@@ -427,9 +451,13 @@ 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 {
+	ok, err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < h; j++ {
 		for i := 0; i < w; i++ {
 			r := pix[4*(w*j+i)]
@@ -470,9 +498,13 @@ 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 {
+	ok, err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, dstW, dstH))
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	for j := 0; j < h; j++ {
 		for i := 0; i < w*scale; i++ {
 			r := pix[4*(dstW*j+i)]
@@ -684,9 +716,13 @@ 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 {
+		ok, err := img.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, size, size))
+		if err != nil {
 			t.Fatal(err)
 		}
+		if !ok {
+			t.Fatal("ReadPixels failed")
+		}
 		for j := 0; j < size; j++ {
 			for i := 0; i < size; i++ {
 				want := color.RGBA{R: byte(i + j), G: byte(i + j), B: byte(i + j), A: byte(i + j)}

internal/atlas/shader_test.go

@@ -44,9 +44,13 @@ 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 {
+	ok, err := dst.ReadPixels(g, pix, image.Rect(0, 0, w, h))
+	if err != nil {
 		t.Error(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	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 {
 		t.Errorf("got: %v, want: %v", got, want)
 	}
@@ -71,9 +75,13 @@ 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 {
+	ok, err := dst.ReadPixels(ui.Get().GraphicsDriverForTesting(), pix, image.Rect(0, 0, w, h))
+	if err != nil {
 		t.Error(err)
 	}
+	if !ok {
+		t.Fatal("ReadPixels failed")
+	}
 	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 {
 		t.Errorf("got: %v, want: %v", got, want)
 	}

internal/buffered/image.go

@@ -57,24 +57,24 @@ func (i *Image) MarkDisposed() {
 	i.img.MarkDisposed()
 }
 
-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) (ok bool, err 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 {
+		ok, err := i.img.ReadPixels(graphicsDriver, pixels, region)
-			return err
+		if err != nil {
+			return false, err
 		}
-		return nil
+		return ok, 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 {
+		ok, err := i.img.ReadPixels(graphicsDriver, pix, image.Rect(0, 0, i.width, i.height))
-			return err
+		if err != nil {
+			return false, err
+		}
+		if !ok {
+			return false, nil
 		}
 		i.pixels = pix
 	}
@@ -85,7 +85,7 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte
 		srcX := 4 * ((region.Min.Y+j)*i.width + region.Min.X)
 		copy(pixels[dstX:dstX+lineWidth], i.pixels[srcX:srcX+lineWidth])
 	}
-	return nil
+	return true, nil
 }
 
 func (i *Image) DumpScreenshot(graphicsDriver graphicsdriver.Graphics, name string, blackbg bool) (string, error) {

internal/mipmap/mipmap.go

@@ -61,7 +61,7 @@ func (m *Mipmap) WritePixels(pix []byte, region image.Rectangle) {
 	m.disposeMipmaps()
 }
 
-func (m *Mipmap) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) error {
+func (m *Mipmap) ReadPixels(graphicsDriver graphicsdriver.Graphics, pixels []byte, region image.Rectangle) (ok bool, err error) {
 	return m.orig.ReadPixels(graphicsDriver, pixels, region)
 }
 

internal/processtest/testdata/issue2815.go

@@ -17,6 +17,8 @@
 package main
 
 import (
+	"fmt"
+	"image/color"
 	"time"
 
 	"github.com/hajimehoshi/ebiten/v2"
@@ -27,20 +29,29 @@ type Game struct {
 	count int
 	end0  chan struct{}
 	end1  chan struct{}
+	errCh chan error
 }
 
 func (g *Game) Update() error {
 	if !g.init {
 		g.end0 = make(chan struct{})
 		g.end1 = make(chan struct{})
+		g.errCh = make(chan error)
 		img := ebiten.NewImage(1, 1)
+		img.WritePixels([]byte{0xff, 0xff, 0xff, 0xff})
 		go func() {
 			t := time.Tick(time.Microsecond)
 		loop:
 			for {
 				select {
 				case <-t:
-					img.At(0, 0)
+					got := img.At(0, 0).(color.RGBA)
+					want := color.RGBA{0xff, 0xff, 0xff, 0xff}
+					if got != want {
+						g.errCh <- fmt.Errorf("got: %v, want: %v", got, want)
+						close(g.errCh)
+						return
+					}
 				case <-g.end0:
 					close(g.end1)
 					break loop
@@ -49,6 +60,13 @@ func (g *Game) Update() error {
 		}()
 		g.init = true
 	}
+
+	select {
+	case err := <-g.errCh:
+		return err
+	default:
+	}
+
 	g.count++
 	if g.count >= 60 {
 		close(g.end0)

internal/ui/context.go

@@ -59,6 +59,9 @@ type context struct {
 	skipCount int
 
 	setContextOnce sync.Once
+
+	deferred  []func()
+	deferredM sync.Mutex
 }
 
 func newContext(game Game) *context {
@@ -108,6 +111,11 @@ func (c *context) updateFrameImpl(graphicsDriver graphicsdriver.Graphics, update
 		}
 	}()
 
+	// Flush deferred functions, like reading pixels from GPU.
+	if err := c.flushDeferredFuncs(ui); err != nil {
+		return err
+	}
+
 	// ForceUpdate can be invoked even if the context is not initialized yet (#1591).
 	if w, h := c.layoutGame(outsideWidth, outsideHeight, deviceScaleFactor); w == 0 || h == 0 {
 		return nil
@@ -282,3 +290,31 @@ func (c *context) screenScaleAndOffsets() (scale, offsetX, offsetY float64) {
 func (u *UserInterface) LogicalPositionToClientPosition(x, y float64) (float64, float64) {
 	return u.context.logicalPositionToClientPosition(x, y, u.DeviceScaleFactor())
 }
+
+// appendDeferredFunc appends a function.
+// An appended function is called at the next frame.
+func (c *context) appendDeferredFunc(f func()) {
+	c.deferredM.Lock()
+	defer c.deferredM.Unlock()
+	c.deferred = append(c.deferred, f)
+}
+
+func (c *context) flushDeferredFuncs(ui *UserInterface) error {
+	c.deferredM.Lock()
+	fs := c.deferred
+	c.deferred = nil
+	c.deferredM.Unlock()
+
+	for _, f := range fs {
+		f()
+	}
+
+	if len(fs) > 0 {
+		// Catch the error that happened at (*Image).At.
+		if err := ui.error(); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

internal/ui/ui.go

@@ -120,7 +120,35 @@ func newUserInterface() (*UserInterface, error) {
 }
 
 func (u *UserInterface) readPixels(mipmap *mipmap.Mipmap, pixels []byte, region image.Rectangle) error {
-	return mipmap.ReadPixels(u.graphicsDriver, pixels, region)
+	ok, err := mipmap.ReadPixels(u.graphicsDriver, pixels, region)
+	if err != nil {
+		return err
+	}
+
+	// ReadPixels failed since this was called in between two frames.
+	// Try this again at the next frame.
+	if !ok {
+		ch := make(chan error)
+		u.context.appendDeferredFunc(func() {
+			ok, err := mipmap.ReadPixels(u.graphicsDriver, pixels, region)
+			if !ok {
+				// This never reaches since this function must be called in a frame.
+				panic("ui: ReadPixels unexpectedly failed")
+			}
+			if err != nil {
+				ch <- err
+			}
+			close(ch)
+		})
+
+		// If this function is called from the game (Update/Draw) goroutine in between two frames,
+		// this channel never returns and causes a dead lock.
+		// This never happens so far, but if handling inputs after EndFrame is implemented,
+		// this might be possible (#1704).
+		return <-ch
+	}
+
+	return nil
 }
 
 func (u *UserInterface) dumpScreenshot(mipmap *mipmap.Mipmap, name string, blackbg bool) (string, error) {