Skip to content

Commit 19cc46b

Browse files
committed
ebiten, vector: bug fix: race conditions
This change fixes these race conditions in * (*ebiten.Image).invokeUsageCallbacks concurrent invocations * (*ebiten.Image).usageCallbacks usages * vector.theCallbackTokens usages * vector's global shader initializations Closes #3333
1 parent 7ecd403 commit 19cc46b

File tree

5 files changed

+213
-90
lines changed

5 files changed

+213
-90
lines changed

image.go

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"image"
2020
"image/color"
2121
"math"
22+
"slices"
2223
"sync"
2324
"sync/atomic"
2425
"unsafe"
@@ -72,15 +73,23 @@ type Image struct {
7273

7374
// usageCallbacks are callbacks that are invoked when the image is used.
7475
// usageCallbacks is valid only when the image is not a sub-image.
75-
usageCallbacks map[int64]func()
76+
usageCallbacks map[int64]usageCallback
7677

7778
// inUsageCallbacks reports whether the image is in usageCallbacks.
78-
inUsageCallbacks bool
79+
inUsageCallbacks atomic.Bool
80+
81+
// usageCallbacksM is a mutex for usageCallbacks.
82+
usageCallbacksM sync.Mutex
7983

8084
// Do not add a 'buffering' member that are resolved lazily.
8185
// This tends to forget resolving the buffer easily (#2362).
8286
}
8387

88+
type usageCallback struct {
89+
image *Image
90+
fn func(image *Image)
91+
}
92+
8493
func (i *Image) copyCheck() {
8594
if i.addr != i {
8695
panic("ebiten: illegal use of non-zero Image copied by value")
@@ -1597,19 +1606,26 @@ func (*Image) private() {
15971606
var currentCallbackToken atomic.Int64
15981607

15991608
//go:linkname addUsageCallback
1600-
func addUsageCallback(img *Image, callback func()) int64 {
1601-
return img.addUsageCallback(callback)
1609+
func addUsageCallback(img *Image, callback func(image *Image)) int64 {
1610+
return img.addUsageCallback(img, callback)
16021611
}
16031612

1604-
func (i *Image) addUsageCallback(callback func()) int64 {
1613+
func (i *Image) addUsageCallback(image *Image, callback func(image *Image)) int64 {
16051614
if i.isSubImage() {
1606-
return i.original.addUsageCallback(callback)
1615+
return i.original.addUsageCallback(image, callback)
16071616
}
1617+
token := currentCallbackToken.Add(1)
1618+
1619+
i.usageCallbacksM.Lock()
1620+
defer i.usageCallbacksM.Unlock()
1621+
16081622
if i.usageCallbacks == nil {
1609-
i.usageCallbacks = map[int64]func(){}
1623+
i.usageCallbacks = map[int64]usageCallback{}
1624+
}
1625+
i.usageCallbacks[token] = usageCallback{
1626+
image: image,
1627+
fn: callback,
16101628
}
1611-
token := currentCallbackToken.Add(1)
1612-
i.usageCallbacks[token] = callback
16131629
return token
16141630
}
16151631

@@ -1623,25 +1639,45 @@ func (i *Image) removeUsageCallback(token int64) {
16231639
i.original.removeUsageCallback(token)
16241640
return
16251641
}
1642+
1643+
i.usageCallbacksM.Lock()
1644+
defer i.usageCallbacksM.Unlock()
16261645
delete(i.usageCallbacks, token)
16271646
}
16281647

1648+
var theTmpUsageCallbackSlicePool = sync.Pool{
1649+
New: func() any {
1650+
slice := make([]usageCallback, 0, 16)
1651+
return &slice
1652+
},
1653+
}
1654+
16291655
func (i *Image) invokeUsageCallbacks() {
16301656
if i.isSubImage() {
16311657
i.original.invokeUsageCallbacks()
16321658
return
16331659
}
16341660

1635-
if i.inUsageCallbacks {
1661+
// Do not allow recursive calls.
1662+
if !i.inUsageCallbacks.CompareAndSwap(false, true) {
16361663
return
16371664
}
1665+
defer i.inUsageCallbacks.Store(false)
16381666

1639-
i.inUsageCallbacks = true
1640-
defer func() {
1641-
i.inUsageCallbacks = false
1667+
tmpUsageCallbackSlice := theTmpUsageCallbackSlicePool.Get().(*[]usageCallback)
1668+
1669+
func() {
1670+
i.usageCallbacksM.Lock()
1671+
defer i.usageCallbacksM.Unlock()
1672+
for _, cb := range i.usageCallbacks {
1673+
*tmpUsageCallbackSlice = append(*tmpUsageCallbackSlice, cb)
1674+
}
16421675
}()
16431676

1644-
for _, cb := range i.usageCallbacks {
1645-
cb()
1677+
for _, cb := range *tmpUsageCallbackSlice {
1678+
cb.fn(cb.image)
16461679
}
1680+
1681+
*tmpUsageCallbackSlice = slices.Delete(*tmpUsageCallbackSlice, 0, len(*tmpUsageCallbackSlice))
1682+
theTmpUsageCallbackSlicePool.Put(tmpUsageCallbackSlice)
16471683
}

vector/fill.go

Lines changed: 24 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package vector
1616

1717
import (
18+
"fmt"
1819
"slices"
1920

2021
"github.com/hajimehoshi/ebiten/v2"
@@ -121,6 +122,8 @@ var (
121122

122123
// theAtlas manages the atlas for stencil buffer images.
123124
// theAtlas is a singleton to avoid unnecessary texture allocations.
125+
//
126+
// theAtlas methods are used only at fillPathsState.fillPaths, and should be protected by theFillPathM.
124127
var theAtlas atlas
125128

126129
type fillPathsState struct {
@@ -147,6 +150,7 @@ func (f *fillPathsState) addPath(path *Path, clr ebiten.ColorScale) {
147150
if path == nil {
148151
return
149152
}
153+
150154
f.paths = slices.Grow(f.paths, 1)[:len(f.paths)+1]
151155
if f.paths[len(f.paths)-1] == nil {
152156
f.paths[len(f.paths)-1] = &Path{}
@@ -163,62 +167,13 @@ func (f *fillPathsState) addPath(path *Path, clr ebiten.ColorScale) {
163167
}
164168

165169
// fillPaths fills the specified path with the specified color.
170+
//
171+
// fillPaths callers must be protected by theFillPathM.
166172
func (f *fillPathsState) fillPaths(dst *ebiten.Image) {
167173
if len(f.paths) != len(f.colors) {
168174
panic("vector: the number of paths and colors must be the same")
169175
}
170176

171-
if stencilBufferFillShader == nil {
172-
s, err := ebiten.NewShader([]byte(stencilBufferFillShaderSrc))
173-
if err != nil {
174-
panic(err)
175-
}
176-
stencilBufferFillShader = s
177-
}
178-
if stencilBufferBezierShader == nil {
179-
s, err := ebiten.NewShader([]byte(stencilBufferBezierShaderSrc))
180-
if err != nil {
181-
panic(err)
182-
}
183-
stencilBufferBezierShader = s
184-
}
185-
if !f.antialias && f.fillRule == FillRuleNonZero {
186-
if stencilBufferNonZeroShader == nil {
187-
s, err := ebiten.NewShader([]byte(stencilBufferNonZeroShaderSrc))
188-
if err != nil {
189-
panic(err)
190-
}
191-
stencilBufferNonZeroShader = s
192-
}
193-
}
194-
if f.antialias && f.fillRule == FillRuleNonZero {
195-
if stencilBufferNonZeroAAShader == nil {
196-
s, err := ebiten.NewShader([]byte(stencilBufferNonZeroAAShaderSrc))
197-
if err != nil {
198-
panic(err)
199-
}
200-
stencilBufferNonZeroAAShader = s
201-
}
202-
}
203-
if !f.antialias && f.fillRule == FillRuleEvenOdd {
204-
if stencilBufferEvenOddShader == nil {
205-
s, err := ebiten.NewShader([]byte(stencilBufferEvenOddShaderSrc))
206-
if err != nil {
207-
panic(err)
208-
}
209-
stencilBufferEvenOddShader = s
210-
}
211-
}
212-
if f.antialias && f.fillRule == FillRuleEvenOdd {
213-
if stencilBufferEvenOddAAShader == nil {
214-
s, err := ebiten.NewShader([]byte(stencilBufferEvenOddAAShaderSrc))
215-
if err != nil {
216-
panic(err)
217-
}
218-
stencilBufferEvenOddAAShader = s
219-
}
220-
}
221-
222177
vs := f.vertices[:0]
223178
is := f.indices[:0]
224179
defer func() {
@@ -343,7 +298,11 @@ func (f *fillPathsState) fillPaths(dst *ebiten.Image) {
343298
}
344299
op := &ebiten.DrawTrianglesShaderOptions{}
345300
op.Blend = ebiten.BlendLighter
346-
stencilBufferImage.DrawTrianglesShader32(vs, is, stencilBufferFillShader, op)
301+
shader, err := ensureStencilBufferShaders()
302+
if err != nil {
303+
panic(fmt.Sprintf("vector: failed to create stencil buffer shader: %v", err))
304+
}
305+
stencilBufferImage.DrawTrianglesShader32(vs, is, shader, op)
347306
}
348307
}
349308

@@ -415,7 +374,11 @@ func (f *fillPathsState) fillPaths(dst *ebiten.Image) {
415374
}
416375
op := &ebiten.DrawTrianglesShaderOptions{}
417376
op.Blend = ebiten.BlendLighter
418-
stencilBufferImage.DrawTrianglesShader32(vs, is, stencilBufferBezierShader, op)
377+
shader, err := ensureStencilBufferBezierShader()
378+
if err != nil {
379+
panic(fmt.Sprintf("vector: failed to create stencil buffer bezier shader: %v", err))
380+
}
381+
stencilBufferImage.DrawTrianglesShader32(vs, is, shader, op)
419382
}
420383
}
421384

@@ -506,16 +469,16 @@ func (f *fillPathsState) fillPaths(dst *ebiten.Image) {
506469
var shader *ebiten.Shader
507470
switch f.fillRule {
508471
case FillRuleNonZero:
509-
if f.antialias {
510-
shader = stencilBufferNonZeroAAShader
511-
} else {
512-
shader = stencilBufferNonZeroShader
472+
var err error
473+
shader, err = ensureStencilBufferNonZeroShader(f.antialias)
474+
if err != nil {
475+
panic(fmt.Sprintf("vector: failed to create stencil buffer non-zero shader: %v", err))
513476
}
514477
case FillRuleEvenOdd:
515-
if f.antialias {
516-
shader = stencilBufferEvenOddAAShader
517-
} else {
518-
shader = stencilBufferEvenOddShader
478+
var err error
479+
shader, err = ensureStencilBufferEvenOddShader(f.antialias)
480+
if err != nil {
481+
panic(fmt.Sprintf("vector: failed to create stencil buffer even-odd shader: %v", err))
519482
}
520483
}
521484
dst.DrawTrianglesShader32(vs, is, shader, op)

vector/stencilshader.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
package vector
1616

1717
import (
18+
"sync"
19+
1820
"github.com/hajimehoshi/ebiten/v2"
1921
)
2022

@@ -30,8 +32,94 @@ var (
3032
stencilBufferNonZeroAAShader *ebiten.Shader
3133
stencilBufferEvenOddShader *ebiten.Shader
3234
stencilBufferEvenOddAAShader *ebiten.Shader
35+
36+
stencilBufferM sync.Mutex
3337
)
3438

39+
func ensureStencilBufferShaders() (*ebiten.Shader, error) {
40+
stencilBufferM.Lock()
41+
defer stencilBufferM.Unlock()
42+
43+
if stencilBufferFillShader != nil {
44+
return stencilBufferFillShader, nil
45+
}
46+
s, err := ebiten.NewShader([]byte(stencilBufferFillShaderSrc))
47+
if err != nil {
48+
return nil, err
49+
}
50+
stencilBufferFillShader = s
51+
return stencilBufferFillShader, err
52+
}
53+
54+
func ensureStencilBufferBezierShader() (*ebiten.Shader, error) {
55+
stencilBufferM.Lock()
56+
defer stencilBufferM.Unlock()
57+
58+
if stencilBufferBezierShader != nil {
59+
return stencilBufferBezierShader, nil
60+
}
61+
s, err := ebiten.NewShader([]byte(stencilBufferBezierShaderSrc))
62+
if err != nil {
63+
return nil, err
64+
}
65+
stencilBufferBezierShader = s
66+
return stencilBufferBezierShader, nil
67+
}
68+
69+
func ensureStencilBufferNonZeroShader(antialias bool) (*ebiten.Shader, error) {
70+
stencilBufferM.Lock()
71+
defer stencilBufferM.Unlock()
72+
73+
if antialias {
74+
if stencilBufferNonZeroAAShader != nil {
75+
return stencilBufferNonZeroAAShader, nil
76+
}
77+
s, err := ebiten.NewShader([]byte(stencilBufferNonZeroAAShaderSrc))
78+
if err != nil {
79+
return nil, err
80+
}
81+
stencilBufferNonZeroAAShader = s
82+
return stencilBufferNonZeroAAShader, nil
83+
}
84+
85+
if stencilBufferNonZeroShader != nil {
86+
return stencilBufferNonZeroShader, nil
87+
}
88+
s, err := ebiten.NewShader([]byte(stencilBufferNonZeroShaderSrc))
89+
if err != nil {
90+
return nil, err
91+
}
92+
stencilBufferNonZeroShader = s
93+
return stencilBufferNonZeroShader, nil
94+
}
95+
96+
func ensureStencilBufferEvenOddShader(antialias bool) (*ebiten.Shader, error) {
97+
stencilBufferM.Lock()
98+
defer stencilBufferM.Unlock()
99+
100+
if antialias {
101+
if stencilBufferEvenOddAAShader != nil {
102+
return stencilBufferEvenOddAAShader, nil
103+
}
104+
s, err := ebiten.NewShader([]byte(stencilBufferEvenOddAAShaderSrc))
105+
if err != nil {
106+
return nil, err
107+
}
108+
stencilBufferEvenOddAAShader = s
109+
return stencilBufferEvenOddAAShader, nil
110+
}
111+
112+
if stencilBufferEvenOddShader != nil {
113+
return stencilBufferEvenOddShader, nil
114+
}
115+
s, err := ebiten.NewShader([]byte(stencilBufferEvenOddShaderSrc))
116+
if err != nil {
117+
return nil, err
118+
}
119+
stencilBufferEvenOddShader = s
120+
return stencilBufferEvenOddShader, nil
121+
}
122+
35123
//ebitengine:shadersource
36124
const stencilBufferFillShaderSrc = `//kage:unit pixels
37125

0 commit comments

Comments
 (0)