graphics: Refactoring: Reduce error propagations

graphicscontext.go

@@ -82,6 +82,10 @@ func (c *graphicsContext) initializeIfNeeded() error {
 }
 
 func (c *graphicsContext) Update(afterFrameUpdate func()) error {
+	if err := shareable.Error(); err != nil {
+		return err
+	}
+
 	updateCount := clock.Update()
 
 	if err := c.initializeIfNeeded(); err != nil {
@@ -125,9 +129,7 @@ func (c *graphicsContext) Update(afterFrameUpdate func()) error {
 	op.Filter = filterScreen
 	_ = c.screen.DrawImage(c.offscreen, op)
 
-	if err := shareable.ResolveStaleImages(); err != nil {
+	shareable.ResolveStaleImages()
-		return err
-	}
 	return nil
 }
 

image.go

@@ -267,11 +267,7 @@ func (i *Image) At(x, y int) color.Color {
 	if i.isDisposed() {
 		return color.RGBA{}
 	}
-	clr, err := i.shareableImage.At(x, y)
+	return i.shareableImage.At(x, y)
-	if err != nil {
-		panic(err)
-	}
-	return clr
 }
 
 // Dispose disposes the image data. After disposing, most of image functions do nothing and returns meaningless values.

internal/graphics/command.go

@@ -58,6 +58,8 @@ type commandQueue struct {
 
 	tmpNumIndices int
 	nextIndex     int
+
+	err error
 }
 
 // theCommandQueue is the command queue for the current process.
@@ -136,7 +138,11 @@ func (q *commandQueue) Enqueue(command command) {
 }
 
 // Flush flushes the command queue.
-func (q *commandQueue) Flush() error {
+func (q *commandQueue) Flush() {
+	if q.err != nil {
+		return
+	}
+
 	// glViewport must be called at least at every frame on iOS.
 	opengl.GetContext().ResetViewportSize()
 	es := q.indices
@@ -168,7 +174,8 @@ func (q *commandQueue) Flush() error {
 		indexOffsetInBytes := 0
 		for _, c := range q.commands[:nc] {
 			if err := c.Exec(indexOffsetInBytes); err != nil {
-				return err
+				q.err = err
+				return
 			}
 			// TODO: indexOffsetInBytes should be reset if the command type is different
 			// from the previous one. This fix is needed when another drawing command is
@@ -186,12 +193,16 @@ func (q *commandQueue) Flush() error {
 	q.nindices = 0
 	q.tmpNumIndices = 0
 	q.nextIndex = 0
-	return nil
+}
+
+// Error returns an OpenGL error for the last command.
+func Error() error {
+	return theCommandQueue.err
 }
 
 // FlushCommands flushes the command queue.
-func FlushCommands() error {
+func FlushCommands() {
-	return theCommandQueue.Flush()
+	theCommandQueue.Flush()
 }
 
 // drawImageCommand represents a drawing command to draw an image on another image.

internal/graphics/image.go

@@ -90,15 +90,16 @@ func (i *Image) DrawImage(src *Image, vertices []float32, indices []uint16, clr
 	theCommandQueue.EnqueueDrawImageCommand(i, src, vertices, indices, clr, mode, filter)
 }
 
-func (i *Image) Pixels() ([]byte, error) {
+// Pixels returns the image's pixels.
+// Pixels might return nil when OpenGL error happens.
+func (i *Image) Pixels() []byte {
 	c := &pixelsCommand{
-		img: i,
+		result: nil,
+		img:    i,
 	}
 	theCommandQueue.Enqueue(c)
-	if err := theCommandQueue.Flush(); err != nil {
+	theCommandQueue.Flush()
-		return nil, err
+	return c.result
-	}
-	return c.result, nil
 }
 
 func (i *Image) ReplacePixels(p []byte, x, y, width, height int) {

internal/restorable/image.go

@@ -241,25 +241,27 @@ func (i *Image) appendDrawImageHistory(image *Image, vertices []float32, indices
 // At returns a color value at (x, y).
 //
 // Note that this must not be called until context is available.
-func (i *Image) At(x, y int) (color.RGBA, error) {
+func (i *Image) At(x, y int) color.RGBA {
 	w, h := i.image.Size()
 	if x < 0 || y < 0 || w <= x || h <= y {
-		return color.RGBA{}, nil
+		return color.RGBA{}
 	}
 
 	if i.basePixels == nil || i.drawImageHistory != nil || i.stale {
-		if err := graphics.FlushCommands(); err != nil {
+		graphics.FlushCommands()
-			return color.RGBA{}, err
+		i.readPixelsFromGPU()
-		}
-		if err := i.readPixelsFromGPU(); err != nil {
-			return color.RGBA{}, err
-		}
 		i.drawImageHistory = nil
 		i.stale = false
 	}
+
+	// Even after readPixelsFromGPU, basePixels might be nil when OpenGL error happens.
+	if i.basePixels == nil {
+		return color.RGBA{}
+	}
+
 	idx := 4*x + 4*y*w
 	r, g, b, a := i.basePixels[idx], i.basePixels[idx+1], i.basePixels[idx+2], i.basePixels[idx+3]
-	return color.RGBA{r, g, b, a}, nil
+	return color.RGBA{r, g, b, a}
 }
 
 // makeStaleIfDependingOn makes the image stale if the image depends on target.
@@ -273,33 +275,28 @@ func (i *Image) makeStaleIfDependingOn(target *Image) {
 }
 
 // readPixelsFromGPU reads the pixels from GPU and resolves the image's 'stale' state.
-func (i *Image) readPixelsFromGPU() error {
+func (i *Image) readPixelsFromGPU() {
-	var err error
+	i.basePixels = i.image.Pixels()
-	i.basePixels, err = i.image.Pixels()
-	if err != nil {
-		return err
-	}
 	i.drawImageHistory = nil
 	i.stale = false
-	return nil
 }
 
 // resolveStale resolves the image's 'stale' state.
-func (i *Image) resolveStale() error {
+func (i *Image) resolveStale() {
 	if !IsRestoringEnabled() {
-		return nil
+		return
 	}
 
 	if i.volatile {
-		return nil
+		return
 	}
 	if i.screen {
-		return nil
+		return
 	}
 	if !i.stale {
-		return nil
+		return
 	}
-	return i.readPixelsFromGPU()
+	i.readPixelsFromGPU()
 }
 
 // dependsOn returns a boolean value indicating whether the image depends on target.
@@ -369,11 +366,7 @@ func (i *Image) restore() error {
 	}
 	i.image = gimg
 
-	var err error
+	i.basePixels = gimg.Pixels()
-	i.basePixels, err = gimg.Pixels()
-	if err != nil {
-		return err
-	}
 	i.drawImageHistory = nil
 	i.stale = false
 	return nil
@@ -397,9 +390,7 @@ func (i *Image) Dispose() {
 // If an image is invalidated, GL context is lost and all the images should be restored asap.
 func (i *Image) IsInvalidated() (bool, error) {
 	// FlushCommands is required because c.offscreen.impl might not have an actual texture.
-	if err := graphics.FlushCommands(); err != nil {
+	graphics.FlushCommands()
-		return false, err
-	}
 	if !IsRestoringEnabled() {
 		return false, nil
 	}

internal/restorable/images.go

@@ -53,14 +53,12 @@ var theImages = &images{
 // all stale images.
 //
 // ResolveStaleImages is intended to be called at the end of a frame.
-func ResolveStaleImages() error {
+func ResolveStaleImages() {
-	if err := graphics.FlushCommands(); err != nil {
+	graphics.FlushCommands()
-		return err
-	}
 	if !restoringEnabled {
-		return nil
+		return
 	}
-	return theImages.resolveStaleImages()
+	theImages.resolveStaleImages()
 }
 
 // Restore restores the images.
@@ -73,7 +71,7 @@ func Restore() error {
 	return theImages.restore()
 }
 
-func Images() ([]image.Image, error) {
+func Images() []image.Image {
 	var imgs []image.Image
 	for img := range theImages.images {
 		if img.volatile {
@@ -87,10 +85,7 @@ func Images() ([]image.Image, error) {
 		pix := make([]byte, 4*w*h)
 		for j := 0; j < h; j++ {
 			for i := 0; i < w; i++ {
-				c, err := img.At(i, j)
+				c := img.At(i, j)
-				if err != nil {
-					return nil, err
-				}
 				pix[4*(i+j*w)] = byte(c.R)
 				pix[4*(i+j*w)+1] = byte(c.G)
 				pix[4*(i+j*w)+2] = byte(c.B)
@@ -103,7 +98,7 @@ func Images() ([]image.Image, error) {
 			Rect:   image.Rect(0, 0, w, h),
 		})
 	}
-	return imgs, nil
+	return imgs
 }
 
 // add adds img to the images.
@@ -118,14 +113,11 @@ func (i *images) remove(img *Image) {
 }
 
 // resolveStaleImages resolves stale images.
-func (i *images) resolveStaleImages() error {
+func (i *images) resolveStaleImages() {
 	i.lastTarget = nil
 	for img := range i.images {
-		if err := img.resolveStale(); err != nil {
+		img.resolveStale()
-			return err
-		}
 	}
-	return nil
 }
 
 // makeStaleIfDependingOn makes all the images stale that depend on target.
@@ -218,3 +210,7 @@ func (i *images) restore() error {
 func InitializeGLState() error {
 	return graphics.ResetGLState()
 }
+
+func Error() error {
+	return graphics.Error()
+}

internal/restorable/images_test.go

@@ -88,9 +88,7 @@ func TestRestore(t *testing.T) {
 
 	clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff}
 	fill(img0, clr0.R, clr0.G, clr0.B, clr0.A)
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -125,9 +123,7 @@ func TestRestoreChain(t *testing.T) {
 		vs := graphicsutil.QuadVertices(w, h, 0, 0, 1, 1, 1, 0, 0, 1, 0, 0)
 		imgs[i+1].DrawImage(imgs[i], vs, quadIndices, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 	}
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -172,9 +168,7 @@ func TestRestoreChain2(t *testing.T) {
 		imgs[i+1].DrawImage(imgs[i], vs, quadIndices, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 	}
 
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -217,9 +211,7 @@ func TestRestoreOverrideSource(t *testing.T) {
 	img3.DrawImage(img2, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
 	fill(img0, clr1.R, clr1.G, clr1.B, clr1.A)
 	img1.DrawImage(img0, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -316,9 +308,7 @@ func TestRestoreComplexGraph(t *testing.T) {
 	img7.DrawImage(img2, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
 	vs = graphicsutil.QuadVertices(w, h, 0, 0, w, h, 1, 0, 0, 1, 2, 0)
 	img7.DrawImage(img3, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -410,9 +400,7 @@ func TestRestoreRecursive(t *testing.T) {
 	vs := graphicsutil.QuadVertices(w, h, 0, 0, w, h, 1, 0, 0, 1, 1, 0)
 	img1.DrawImage(img0, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
 	img0.DrawImage(img1, vs, quadIndices, nil, opengl.CompositeModeSourceOver, graphics.FilterNearest)
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
@@ -463,28 +451,20 @@ func TestReplacePixels(t *testing.T) {
 	// Check the region (5, 7)-(9, 11). Outside state is indeterministic.
 	for j := 7; j < 11; j++ {
 		for i := 5; i < 9; i++ {
-			got, err := img.At(i, j)
+			got := img.At(i, j)
-			if err != nil {
-				t.Fatal(err)
-			}
 			want := color.RGBA{0xff, 0xff, 0xff, 0xff}
 			if got != want {
 				t.Errorf("img.At(%d, %d): got: %v, want: %v", i, j, got, want)
 			}
 		}
 	}
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
 	for j := 7; j < 11; j++ {
 		for i := 5; i < 9; i++ {
-			got, err := img.At(i, j)
+			got := img.At(i, j)
-			if err != nil {
-				t.Fatal(err)
-			}
 			want := color.RGBA{0xff, 0xff, 0xff, 0xff}
 			if got != want {
 				t.Errorf("img.At(%d, %d): got: %v, want: %v", i, j, got, want)
@@ -508,16 +488,11 @@ func TestDrawImageAndReplacePixels(t *testing.T) {
 	img1.DrawImage(img0, vs, quadIndices, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 	img1.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff}, 1, 0, 1, 1)
 
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
-	got, err := img1.At(0, 0)
+	got := img1.At(0, 0)
-	if err != nil {
-		t.Fatal(err)
-	}
 	want := color.RGBA{0xff, 0xff, 0xff, 0xff}
 	if !sameColors(got, want, 1) {
 		t.Errorf("got: %v, want: %v", got, want)
@@ -545,16 +520,11 @@ func TestDispose(t *testing.T) {
 	img0.DrawImage(img1, vs, quadIndices, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 	img1.Dispose()
 
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 	if err := Restore(); err != nil {
 		t.Fatal(err)
 	}
-	got, err := img0.At(0, 0)
+	got := img0.At(0, 0)
-	if err != nil {
-		t.Fatal(err)
-	}
 	want := color.RGBA{0xff, 0xff, 0xff, 0xff}
 	if !sameColors(got, want, 1) {
 		t.Errorf("got: %v, want: %v", got, want)
@@ -576,14 +546,10 @@ func TestDoubleResolve(t *testing.T) {
 	img0.DrawImage(img1, vs, quadIndices, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 	img0.ReplacePixels([]uint8{0x00, 0xff, 0x00, 0xff}, 1, 1, 1, 1)
 	// Now img0 is stale.
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 
 	img0.ReplacePixels([]uint8{0x00, 0x00, 0xff, 0xff}, 1, 0, 1, 1)
-	if err := ResolveStaleImages(); err != nil {
+	ResolveStaleImages()
-		t.Fatal(err)
-	}
 
 	if err := Restore(); err != nil {
 		t.Fatal(err)
@@ -595,10 +561,7 @@ func TestDoubleResolve(t *testing.T) {
 	wantImg.Set(1, 1, color.RGBA{0x00, 0xff, 0x00, 0xff})
 	for j := 0; j < 2; j++ {
 		for i := 0; i < 2; i++ {
-			got, err := img0.At(i, j)
+			got := img0.At(i, j)
-			if err != nil {
-				t.Fatal(err)
-			}
 			want := wantImg.At(i, j).(color.RGBA)
 			if !sameColors(got, want, 1) {
 				t.Errorf("got: %v, want: %v", got, want)

internal/shareable/shareable.go

@@ -184,21 +184,20 @@ func (i *Image) ReplacePixels(p []byte) {
 	i.backend.restorable.ReplacePixels(p, x, y, w, h)
 }
 
-func (i *Image) At(x, y int) (color.Color, error) {
+func (i *Image) At(x, y int) color.Color {
 	backendsM.Lock()
 	defer backendsM.Unlock()
 
 	if i.backend == nil {
-		return color.RGBA{}, nil
+		return color.RGBA{}
 	}
 
 	ox, oy, w, h := i.region()
 	if x < 0 || y < 0 || x >= w || y >= h {
-		return color.RGBA{}, nil
+		return color.RGBA{}
 	}
 
-	clr, err := i.backend.restorable.At(x+ox, y+oy)
+	return i.backend.restorable.At(x+ox, y+oy)
-	return clr, err
 }
 
 func (i *Image) Dispose() {
@@ -351,10 +350,10 @@ func InitializeGLState() error {
 	return restorable.InitializeGLState()
 }
 
-func ResolveStaleImages() error {
+func ResolveStaleImages() {
 	backendsM.Lock()
 	defer backendsM.Unlock()
-	return restorable.ResolveStaleImages()
+	restorable.ResolveStaleImages()
 }
 
 func IsRestoringEnabled() bool {
@@ -368,8 +367,14 @@ func Restore() error {
 	return restorable.Restore()
 }
 
-func Images() ([]image.Image, error) {
+func Images() []image.Image {
 	backendsM.Lock()
 	defer backendsM.Unlock()
 	return restorable.Images()
 }
+
+func Error() error {
+	backendsM.Lock()
+	defer backendsM.Unlock()
+	return restorable.Error()
+}

internal/shareable/shareable_test.go

@@ -88,10 +88,7 @@ func TestEnsureNotShared(t *testing.T) {
 
 	for j := 0; j < size; j++ {
 		for i := 0; i < size; i++ {
-			got, err := img4.At(i, j)
+			got := img4.At(i, j)
-			if err != nil {
-				t.Fatal(err)
-			}
 			var want color.RGBA
 			if i < dx0 || dx1 <= i || j < dy0 || dy1 <= j {
 				c := byte(i + j)

run.go

@@ -220,11 +220,7 @@ func (i *imageDumper) update(screen *Image) error {
 			return nil
 		}
 
-		images, err := shareable.Images()
+		for i, img := range shareable.Images() {
-		if err != nil {
-			return err
-		}
-		for i, img := range images {
 			if err := dump(img, i); err != nil {
 				return err
 			}