restorable: Unify locks with shareable's backendsM and fix the deadlock

This change should make Image operations cuncurrent safe.

Updates #913
Fixes #915

internal/restorable/image.go

@@ -121,9 +121,6 @@ func init() {
 //
 // Note that Dispose is not called automatically.
 func NewImage(width, height int) *Image {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	i := &Image{
 		image: graphicscommand.NewImage(width, height),
 	}
@@ -175,9 +172,6 @@ func (i *Image) MakeVolatile() {
 //
 // Note that Dispose is not called automatically.
 func NewScreenFramebufferImage(width, height int) *Image {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	i := &Image{
 		image:  graphicscommand.NewScreenFramebufferImage(width, height),
 		screen: true,
@@ -188,8 +182,6 @@ func NewScreenFramebufferImage(width, height int) *Image {
 }
 
 func (i *Image) Clear() {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
 	theImages.makeStaleIfDependingOn(i)
 	i.clear()
 }
@@ -300,9 +292,6 @@ func (i *Image) ClearPixels(x, y, width, height int) {
 //
 // ReplacePixels for a part is forbidden if the image is rendered with DrawTriangles or Fill.
 func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	w, h := i.image.Size()
 	if width <= 0 || height <= 0 {
 		panic("restorable: width/height must be positive")
@@ -356,9 +345,6 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 
 // DrawTriangles draws a given image img to the image.
 func (i *Image) DrawTriangles(img *Image, vertices []float32, indices []uint16, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address) {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	if i.priority {
 		panic("restorable: DrawTriangles cannot be called on a priority image")
 	}
@@ -413,9 +399,6 @@ func (i *Image) readPixelsFromGPUIfNeeded() {
 //
 // Note that this must not be called until context is available.
 func (i *Image) At(x, y int) (byte, byte, byte, byte) {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	w, h := i.image.Size()
 	if x < 0 || y < 0 || w <= x || h <= y {
 		return 0, 0, 0, 0
@@ -541,9 +524,6 @@ func (i *Image) restore() {
 //
 // After disposing, calling the function of the image causes unexpected results.
 func (i *Image) Dispose() {
-	theImages.m.Lock()
-	defer theImages.m.Unlock()
-
 	theImages.remove(i)
 	i.image.Dispose()
 	i.image = nil

internal/restorable/images.go

@@ -16,7 +16,6 @@ package restorable
 
 import (
 	"path/filepath"
-	"sync"
 
 	"github.com/hajimehoshi/ebiten/internal/graphicscommand"
 )
@@ -41,9 +40,6 @@ func EnableRestoringForTesting() {
 type images struct {
 	images     map[*Image]struct{}
 	lastTarget *Image
-
-	m    sync.Mutex
-	once sync.Once
 }
 
 // theImages represents the images for the current process.
@@ -56,9 +52,6 @@ var theImages = &images{
 //
 // ResolveStaleImages is intended to be called at the end of a frame.
 func ResolveStaleImages() {
-	// Until the begin o the frame (by RestoreIfNeeded, any operations are locked.
-	theImages.m.Lock()
-
 	graphicscommand.FlushCommands()
 	if !needsRestoring() {
 		return
@@ -70,24 +63,6 @@ func ResolveStaleImages() {
 //
 // Restoring means to make all *graphicscommand.Image objects have their textures and framebuffers.
 func RestoreIfNeeded() error {
-	// Unlock except for the first time.
-	//
-	// In each frame, restoring images and resolving images happen respectively:
-	//
-	//   [Restore -> Resolve] -> [Restore -> Resolve] -> ...
-	//
-	// Between each frame, any image operations are not permitted, or stale images would remain when restoring
-	// (#913).
-	defer func() {
-		firsttime := false
-		theImages.once.Do(func() {
-			firsttime = true
-		})
-		if !firsttime {
-			theImages.m.Unlock()
-		}
-	}()
-
 	if !needsRestoring() {
 		return nil
 	}

internal/shareable/shareable.go

@@ -143,6 +143,8 @@ var (
 	// backendsM is a mutex for critical sections of the backend and packing.Node objects.
 	backendsM sync.Mutex
 
+	backendsOnce sync.Once
+
 	// theBackends is a set of actually shared images.
 	theBackends = []*backend{}
 
@@ -570,15 +572,30 @@ func InitializeGraphicsDriverState() error {
 	return restorable.InitializeGraphicsDriverState()
 }
 
-func ResolveStaleImages() {
+func EndFrame() error {
 	backendsM.Lock()
-	defer backendsM.Unlock()
 	restorable.ResolveStaleImages()
+	return restorable.Error()
 }
 
-func RestoreIfNeeded() error {
-	backendsM.Lock()
-	defer backendsM.Unlock()
+func BeginFrame() error {
+	// Unlock except for the first time.
+	//
+	// In each frame, restoring images and resolving images happen respectively:
+	//
+	//   [Restore -> Resolve] -> [Restore -> Resolve] -> ...
+	//
+	// Between each frame, any image operations are not permitted, or stale images would remain when restoring
+	// (#913).
+	defer func() {
+		firsttime := false
+		backendsOnce.Do(func() {
+			firsttime = true
+		})
+		if !firsttime {
+			backendsM.Unlock()
+		}
+	}()
 	return restorable.RestoreIfNeeded()
 }
 
@@ -587,9 +604,3 @@ func DumpImages(dir string) error {
 	defer backendsM.Unlock()
 	return restorable.DumpImages(dir)
 }
-
-func Error() error {
-	backendsM.Lock()
-	defer backendsM.Unlock()
-	return restorable.Error()
-}

uicontext.go

@@ -81,9 +81,6 @@ func (c *uiContext) initializeIfNeeded() error {
 		}
 		c.initialized = true
 	}
-	if err := c.restoreIfNeeded(); err != nil {
-		return err
-	}
 	return nil
 }
 
@@ -96,6 +93,9 @@ func (c *uiContext) Update(afterFrameUpdate func()) error {
 	if err := c.initializeIfNeeded(); err != nil {
 		return err
 	}
+	if err := shareable.BeginFrame(); err != nil {
+		return err
+	}
 	for i := 0; i < updateCount; i++ {
 		c.offscreen.Clear()
 		// Mipmap images should be disposed by fill.
@@ -141,16 +141,7 @@ func (c *uiContext) Update(afterFrameUpdate func()) error {
 	}
 	_ = c.screen.DrawImage(c.offscreen, op)
 
-	shareable.ResolveStaleImages()
-
-	if err := shareable.Error(); err != nil {
-		return err
-	}
-	return nil
-}
-
-func (c *uiContext) restoreIfNeeded() error {
-	if err := shareable.RestoreIfNeeded(); err != nil {
+	if err := shareable.EndFrame(); err != nil {
 		return err
 	}
 	return nil