shareable: Bug fix: Protect critical sections

image_test.go

@@ -238,6 +238,7 @@ func TestImageScale(t *testing.T) {
 		}
 		op := &DrawImageOptions{}
 		op.GeoM.Scale(float64(scale), float64(scale))
+
 		if err := img1.DrawImage(img0, op); err != nil {
 			t.Fatal(err)
 			return
@@ -317,7 +318,7 @@ func TestImageDotByDotInversion(t *testing.T) {
 	}
 }
 
-func TestReplacePixels(t *testing.T) {
+func TestImageReplacePixels(t *testing.T) {
 	// Create a dummy image so that the shared texture is used and origImg's position is shfited.
 	dummyImg, _ := NewImageFromImage(image.NewRGBA(image.Rect(0, 0, 16, 16)), FilterDefault)
 	defer dummyImg.Dispose()

internal/packing/packing.go

@@ -15,10 +15,6 @@
 // Package packing offers a packing algorithm in 2D space.
 package packing
 
-import (
-	"github.com/hajimehoshi/ebiten/internal/sync"
-)
-
 const (
 	minSize = 1
 )
@@ -26,7 +22,6 @@ const (
 type Page struct {
 	root    *Node
 	maxSize int
-	m       sync.Mutex
 }
 
 func NewPage(maxSize int) *Page {
@@ -36,14 +31,10 @@ func NewPage(maxSize int) *Page {
 }
 
 func (p *Page) IsEmpty() bool {
-	p.m.Lock()
 	if p.root == nil {
-		p.m.Unlock()
 		return true
 	}
-	r := !p.root.used && p.root.child0 == nil && p.root.child1 == nil
+	return !p.root.used && p.root.child0 == nil && p.root.child1 == nil
-	p.m.Unlock()
-	return r
 }
 
 type Node struct {
@@ -145,7 +136,6 @@ func (p *Page) alloc(n *Node, width, height int) *Node {
 }
 
 func (p *Page) Alloc(width, height int) *Node {
-	p.m.Lock()
 	if width <= 0 || height <= 0 {
 		panic("bsp: width and height must > 0")
 	}
@@ -162,17 +152,10 @@ func (p *Page) Alloc(width, height int) *Node {
 		height = minSize
 	}
 	n := p.alloc(p.root, width, height)
-	p.m.Unlock()
 	return n
 }
 
 func (p *Page) Free(node *Node) {
-	p.m.Lock()
-	p.free(node)
-	p.m.Unlock()
-}
-
-func (p *Page) free(node *Node) {
 	if node.child0 != nil || node.child1 != nil {
 		panic("bsp: can't free the node including children")
 	}
@@ -186,6 +169,6 @@ func (p *Page) free(node *Node) {
 	if node.parent.child0.canFree() && node.parent.child1.canFree() {
 		node.parent.child0 = nil
 		node.parent.child1 = nil
-		p.free(node.parent)
+		p.Free(node.parent)
 	}
 }

internal/shareable/shareable.go

@@ -33,6 +33,9 @@ type backend struct {
 }
 
 var (
+	// backendsM is a mutex for critical sections of the backend and packing.Node objects.
+	backendsM sync.Mutex
+
 	// theBackends is a set of actually shared images.
 	theBackends = []*backend{}
 )
@@ -53,7 +56,7 @@ func (s *Image) ensureNotShared() {
 	newImg := restorable.NewImage(w, h, false)
 	newImg.DrawImage(s.backend.restorable, x, y, w, h, nil, nil, opengl.CompositeModeCopy, graphics.FilterNearest)
 
-	s.Dispose()
+	s.dispose()
 	s.backend = &backend{
 		restorable: newImg,
 	}
@@ -68,11 +71,19 @@ func (s *Image) region() (x, y, width, height int) {
 }
 
 func (s *Image) Size() (width, height int) {
+	backendsM.Lock()
 	_, _, w, h := s.region()
+	backendsM.Unlock()
 	return w, h
 }
 
 func (s *Image) DrawImage(img *Image, sx0, sy0, sx1, sy1 int, geom *affine.GeoM, colorm *affine.ColorM, mode opengl.CompositeMode, filter graphics.Filter) {
+	backendsM.Lock()
+	defer backendsM.Unlock()
+	s.drawImage(img, sx0, sy0, sx1, sy1, geom, colorm, mode, filter)
+}
+
+func (s *Image) drawImage(img *Image, sx0, sy0, sx1, sy1 int, geom *affine.GeoM, colorm *affine.ColorM, mode opengl.CompositeMode, filter graphics.Filter) {
 	s.ensureNotShared()
 
 	// Compare i and img after ensuring i is not shared, or
@@ -90,6 +101,9 @@ func (s *Image) DrawImage(img *Image, sx0, sy0, sx1, sy1 int, geom *affine.GeoM,
 }
 
 func (s *Image) ReplacePixels(p []byte) {
+	backendsM.Lock()
+	defer backendsM.Unlock()
+
 	x, y, w, h := s.region()
 	if l := 4 * w * h; len(p) != l {
 		panic(fmt.Sprintf("shareable: len(p) was %d but must be %d", len(p), l))
@@ -98,11 +112,17 @@ func (s *Image) ReplacePixels(p []byte) {
 }
 
 func (s *Image) At(x, y int) (color.Color, error) {
+	backendsM.Lock()
+
 	ox, oy, w, h := s.region()
 	if x < 0 || y < 0 || x >= w || y >= h {
+		backendsM.Unlock()
 		return color.RGBA{}, nil
 	}
-	return s.backend.restorable.At(x+ox, y+oy)
+
+	clr, err := s.backend.restorable.At(x+ox, y+oy)
+	backendsM.Unlock()
+	return clr, err
 }
 
 func (s *Image) isDisposed() bool {
@@ -110,6 +130,12 @@ func (s *Image) isDisposed() bool {
 }
 
 func (s *Image) Dispose() {
+	backendsM.Lock()
+	defer backendsM.Unlock()
+	s.dispose()
+}
+
+func (s *Image) dispose() {
 	if s.isDisposed() {
 		return
 	}
@@ -144,16 +170,17 @@ func (s *Image) Dispose() {
 }
 
 func (s *Image) IsInvalidated() (bool, error) {
-	return s.backend.restorable.IsInvalidated()
+	backendsM.Lock()
+	v, err := s.backend.restorable.IsInvalidated()
+	backendsM.Unlock()
+	return v, err
 }
 
-var shareableImageLock sync.Mutex
-
 func NewImage(width, height int) *Image {
 	const maxSize = 2048
 
-	shareableImageLock.Lock()
+	backendsM.Lock()
-	defer shareableImageLock.Unlock()
+	defer backendsM.Unlock()
 
 	if width > maxSize || height > maxSize {
 		s := &backend{