From b08b7409144710d8342655fb823700318257ca9c Mon Sep 17 00:00:00 2001
From: Hajime Hoshi <hajimehoshi@gmail.com>
Date: Tue, 21 Feb 2023 15:51:46 +0900
Subject: [PATCH] internal/buffered: bug fix: functions were not
 concurrent-safe

Before this change, `delayedCommandsFlushed` was set as 1 BEFORE
buffered commands were flushed. This meant that a function call and
a command being flushed were not concurrent-safe.

This change fixes this issue by changing the timing of setting
`delayedCommandsFlushed` as 1 to the later time after flushing the
buffered commands.

Closes #2580
---
 internal/buffered/command.go | 28 +++++++---------------------
 internal/buffered/image.go   | 34 ++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/internal/buffered/command.go b/internal/buffered/command.go
index 67aea1ea8..881d5e2a1 100644
--- a/internal/buffered/command.go
+++ b/internal/buffered/command.go
@@ -30,34 +30,22 @@ var (
 )
 
 func flushDelayedCommands() {
-	fs := getDelayedFuncsAndClear()
-	for _, f := range fs {
-		f()
-	}
-}
-
-func getDelayedFuncsAndClear() []func() {
 	if atomic.LoadUint32(&delayedCommandsFlushed) == 0 {
 		// Outline the slow-path to expect the fast-path is inlined.
-		return getDelayedFuncsAndClearSlow()
+		flushDelayedCommandsSlow()
 	}
-	return nil
 }
 
-func getDelayedFuncsAndClearSlow() []func() {
+func flushDelayedCommandsSlow() {
 	delayedCommandsM.Lock()
 	defer delayedCommandsM.Unlock()
 
 	if delayedCommandsFlushed == 0 {
-		defer atomic.StoreUint32(&delayedCommandsFlushed, 1)
-
-		fs := make([]func(), len(delayedCommands))
-		copy(fs, delayedCommands)
-		delayedCommands = nil
-		return fs
+		for _, f := range delayedCommands {
+			f()
+		}
+		delayedCommandsFlushed = 1
 	}
-
-	return nil
 }
 
 // maybeCanAddDelayedCommand returns false if the delayed commands cannot be added.
@@ -72,9 +60,7 @@ func tryAddDelayedCommand(f func()) bool {
 	defer delayedCommandsM.Unlock()
 
 	if delayedCommandsFlushed == 0 {
-		delayedCommands = append(delayedCommands, func() {
-			f()
-		})
+		delayedCommands = append(delayedCommands, f)
 		return true
 	}
 
diff --git a/internal/buffered/image.go b/internal/buffered/image.go
index 72cc328f0..638581385 100644
--- a/internal/buffered/image.go
+++ b/internal/buffered/image.go
@@ -56,11 +56,15 @@ func NewImage(width, height int, imageType atlas.ImageType) *Image {
 func (i *Image) initialize(imageType atlas.ImageType) {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			i.initialize(imageType)
+			i.initializeImpl(imageType)
 		}) {
 			return
 		}
 	}
+	i.initializeImpl(imageType)
+}
+
+func (i *Image) initializeImpl(imageType atlas.ImageType) {
 	i.img = atlas.NewImage(i.width, i.height, imageType)
 }
 
@@ -71,11 +75,15 @@ func (i *Image) invalidatePixels() {
 func (i *Image) MarkDisposed() {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			i.MarkDisposed()
+			i.markDisposedImpl()
 		}) {
 			return
 		}
 	}
+	i.markDisposedImpl()
+}
+
+func (i *Image) markDisposedImpl() {
 	i.invalidatePixels()
 	i.img.MarkDisposed()
 }
@@ -127,12 +135,15 @@ func (i *Image) WritePixels(pix []byte, x, y, width, height int) {
 		copied := make([]byte, len(pix))
 		copy(copied, pix)
 		if tryAddDelayedCommand(func() {
-			i.WritePixels(copied, x, y, width, height)
+			i.writePixelsImpl(copied, x, y, width, height)
 		}) {
 			return
 		}
 	}
+	i.writePixelsImpl(pix, x, y, width, height)
+}
 
+func (i *Image) writePixelsImpl(pix []byte, x, y, width, height int) {
 	i.invalidatePixels()
 	i.img.WritePixels(pix, x, y, width, height)
 }
@@ -155,12 +166,15 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageCount]*Image, vertices [
 		us := make([]uint32, len(uniforms))
 		copy(us, uniforms)
 		if tryAddDelayedCommand(func() {
-			i.DrawTriangles(srcs, vs, is, blend, dstRegion, srcRegion, subimageOffsets, shader, us, evenOdd)
+			i.drawTrianglesImpl(srcs, vs, is, blend, dstRegion, srcRegion, subimageOffsets, shader, us, evenOdd)
 		}) {
 			return
 		}
 	}
+	i.drawTrianglesImpl(srcs, vertices, indices, blend, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
+}
 
+func (i *Image) drawTrianglesImpl(srcs [graphics.ShaderImageCount]*Image, vertices []float32, indices []uint16, blend graphicsdriver.Blend, dstRegion, srcRegion graphicsdriver.Region, subimageOffsets [graphics.ShaderImageCount - 1][2]float32, shader *Shader, uniforms []uint32, evenOdd bool) {
 	var imgs [graphics.ShaderImageCount]*atlas.Image
 	for i, img := range srcs {
 		if img == nil {
@@ -186,22 +200,30 @@ func NewShader(ir *shaderir.Program) *Shader {
 func (s *Shader) initialize(ir *shaderir.Program) {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			s.initialize(ir)
+			s.initializeImpl(ir)
 		}) {
 			return
 		}
 	}
+	s.initializeImpl(ir)
+}
+
+func (s *Shader) initializeImpl(ir *shaderir.Program) {
 	s.shader = atlas.NewShader(ir)
 }
 
 func (s *Shader) MarkDisposed() {
 	if maybeCanAddDelayedCommand() {
 		if tryAddDelayedCommand(func() {
-			s.MarkDisposed()
+			s.markDisposedImpl()
 		}) {
 			return
 		}
 	}
+	s.markDisposedImpl()
+}
+
+func (s *Shader) markDisposedImpl() {
 	s.shader.MarkDisposed()
 	s.shader = nil
 }