Skip to content

Commit

Permalink
internal/buffered: bug fix: functions were not concurrent-safe
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hajimehoshi committed Feb 21, 2023
1 parent 8c4b29e commit ade7b8c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 27 deletions.
28 changes: 7 additions & 21 deletions internal/buffered/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}

Expand Down
34 changes: 28 additions & 6 deletions internal/buffered/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,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)
}

Expand All @@ -72,11 +76,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()
}
Expand Down Expand Up @@ -128,12 +136,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)
}
Expand All @@ -151,12 +162,15 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageCount]*Image, vertices [
if maybeCanAddDelayedCommand() {
if tryAddDelayedCommand(func() {
// 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
}
}
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 imgs [graphics.ShaderImageCount]*atlas.Image
if shader == nil {
Expand Down Expand Up @@ -190,22 +204,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
}

0 comments on commit ade7b8c

Please sign in to comment.