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
This commit is contained in:
Hajime Hoshi 2023-02-21 15:51:46 +09:00
parent 8c4b29ed1f
commit ade7b8ceec
2 changed files with 35 additions and 27 deletions

View File

@ -30,34 +30,22 @@ var (
) )
func flushDelayedCommands() { func flushDelayedCommands() {
fs := getDelayedFuncsAndClear()
for _, f := range fs {
f()
}
}
func getDelayedFuncsAndClear() []func() {
if atomic.LoadUint32(&delayedCommandsFlushed) == 0 { if atomic.LoadUint32(&delayedCommandsFlushed) == 0 {
// Outline the slow-path to expect the fast-path is inlined. // Outline the slow-path to expect the fast-path is inlined.
return getDelayedFuncsAndClearSlow() flushDelayedCommandsSlow()
} }
return nil
} }
func getDelayedFuncsAndClearSlow() []func() { func flushDelayedCommandsSlow() {
delayedCommandsM.Lock() delayedCommandsM.Lock()
defer delayedCommandsM.Unlock() defer delayedCommandsM.Unlock()
if delayedCommandsFlushed == 0 { if delayedCommandsFlushed == 0 {
defer atomic.StoreUint32(&delayedCommandsFlushed, 1) for _, f := range delayedCommands {
f()
fs := make([]func(), len(delayedCommands)) }
copy(fs, delayedCommands) delayedCommandsFlushed = 1
delayedCommands = nil
return fs
} }
return nil
} }
// maybeCanAddDelayedCommand returns false if the delayed commands cannot be added. // maybeCanAddDelayedCommand returns false if the delayed commands cannot be added.
@ -72,9 +60,7 @@ func tryAddDelayedCommand(f func()) bool {
defer delayedCommandsM.Unlock() defer delayedCommandsM.Unlock()
if delayedCommandsFlushed == 0 { if delayedCommandsFlushed == 0 {
delayedCommands = append(delayedCommands, func() { delayedCommands = append(delayedCommands, f)
f()
})
return true return true
} }

View File

@ -57,11 +57,15 @@ func NewImage(width, height int, imageType atlas.ImageType) *Image {
func (i *Image) initialize(imageType atlas.ImageType) { func (i *Image) initialize(imageType atlas.ImageType) {
if maybeCanAddDelayedCommand() { if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
i.initialize(imageType) i.initializeImpl(imageType)
}) { }) {
return return
} }
} }
i.initializeImpl(imageType)
}
func (i *Image) initializeImpl(imageType atlas.ImageType) {
i.img = atlas.NewImage(i.width, i.height, imageType) i.img = atlas.NewImage(i.width, i.height, imageType)
} }
@ -72,11 +76,15 @@ func (i *Image) invalidatePixels() {
func (i *Image) MarkDisposed() { func (i *Image) MarkDisposed() {
if maybeCanAddDelayedCommand() { if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
i.MarkDisposed() i.markDisposedImpl()
}) { }) {
return return
} }
} }
i.markDisposedImpl()
}
func (i *Image) markDisposedImpl() {
i.invalidatePixels() i.invalidatePixels()
i.img.MarkDisposed() i.img.MarkDisposed()
} }
@ -128,12 +136,15 @@ func (i *Image) WritePixels(pix []byte, x, y, width, height int) {
copied := make([]byte, len(pix)) copied := make([]byte, len(pix))
copy(copied, pix) copy(copied, pix)
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
i.WritePixels(copied, x, y, width, height) i.writePixelsImpl(copied, x, y, width, height)
}) { }) {
return return
} }
} }
i.writePixelsImpl(pix, x, y, width, height)
}
func (i *Image) writePixelsImpl(pix []byte, x, y, width, height int) {
i.invalidatePixels() i.invalidatePixels()
i.img.WritePixels(pix, x, y, width, height) i.img.WritePixels(pix, x, y, width, height)
} }
@ -151,12 +162,15 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageCount]*Image, vertices [
if maybeCanAddDelayedCommand() { if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
// Arguments are not copied. Copying is the caller's responsibility. // Arguments are not copied. Copying is the caller's responsibility.
i.DrawTriangles(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd) i.drawTrianglesImpl(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
}) { }) {
return return
} }
} }
i.drawTrianglesImpl(srcs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, shader, uniforms, evenOdd)
}
func (i *Image) drawTrianglesImpl(srcs [graphics.ShaderImageCount]*Image, vertices []float32, indices []uint16, colorm affine.ColorM, mode graphicsdriver.CompositeMode, filter graphicsdriver.Filter, address graphicsdriver.Address, dstRegion, srcRegion graphicsdriver.Region, subimageOffsets [graphics.ShaderImageCount - 1][2]float32, shader *Shader, uniforms [][]float32, evenOdd bool) {
var s *atlas.Shader var s *atlas.Shader
var imgs [graphics.ShaderImageCount]*atlas.Image var imgs [graphics.ShaderImageCount]*atlas.Image
if shader == nil { if shader == nil {
@ -190,22 +204,30 @@ func NewShader(ir *shaderir.Program) *Shader {
func (s *Shader) initialize(ir *shaderir.Program) { func (s *Shader) initialize(ir *shaderir.Program) {
if maybeCanAddDelayedCommand() { if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
s.initialize(ir) s.initializeImpl(ir)
}) { }) {
return return
} }
} }
s.initializeImpl(ir)
}
func (s *Shader) initializeImpl(ir *shaderir.Program) {
s.shader = atlas.NewShader(ir) s.shader = atlas.NewShader(ir)
} }
func (s *Shader) MarkDisposed() { func (s *Shader) MarkDisposed() {
if maybeCanAddDelayedCommand() { if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() { if tryAddDelayedCommand(func() {
s.MarkDisposed() s.markDisposedImpl()
}) { }) {
return return
} }
} }
s.markDisposedImpl()
}
func (s *Shader) markDisposedImpl() {
s.shader.MarkDisposed() s.shader.MarkDisposed()
s.shader = nil s.shader = nil
} }