shiny/driver/gldriver: add a mutex for groups of OpenGL operations.
Methods like Texture.Upload or Window.Draw are conceptually one
operation but are implemented by multiple OpenGL calls. OpenGL is a
stateful API, so interleaving OpenGL calls from separate higher-level
operations causes inconsistencies.
Change-Id: I66ccf8844d276b82586620c5bfeadb50ba0f5bef
Reviewed-on: https://go-review.googlesource.com/14861
Reviewed-by: David Crawshaw <crawshaw@golang.org>
diff --git a/shiny/driver/gldriver/gldriver.go b/shiny/driver/gldriver/gldriver.go
index cc5134c..ae64b98 100644
--- a/shiny/driver/gldriver/gldriver.go
+++ b/shiny/driver/gldriver/gldriver.go
@@ -40,6 +40,7 @@
}
}
+// writeAff3 must only be called while holding glMu.
func writeAff3(u gl.Uniform, a f64.Aff3) {
var m [9]float32
m[0*3+0] = float32(a[0*3+0])
@@ -84,6 +85,7 @@
return b
}
+// compileProgram must only be called while holding glMu.
func compileProgram(vSrc, fSrc string) (gl.Program, error) {
program := gl.CreateProgram()
if program.Value == 0 {
@@ -115,6 +117,7 @@
return program, nil
}
+// compileShader must only be called while holding glMu.
func compileShader(shaderType gl.Enum, src string) (gl.Shader, error) {
shader := gl.CreateShader(shaderType)
if shader.Value == 0 {
diff --git a/shiny/driver/gldriver/screen.go b/shiny/driver/gldriver/screen.go
index 1b589a5..62583ac 100644
--- a/shiny/driver/gldriver/screen.go
+++ b/shiny/driver/gldriver/screen.go
@@ -13,13 +13,22 @@
"golang.org/x/mobile/gl"
)
+// glMu is a mutex that enforces the atomicity of methods like Texture.Upload
+// or Window.Draw that are conceptually one operation but are implemented by
+// multiple OpenGL calls. OpenGL is a stateful API, so interleaving OpenGL
+// calls from separate higher-level operations causes inconsistencies.
+//
+// glMu does not need to be held when accessing gl.WorkAvailable or gl.DoWork.
+//
+// TODO: is this affected by changing the x/mobile/gl package from an
+// (implicit) global context to a per-window context?
+var glMu sync.Mutex
+
var theScreen = &screenImpl{
windows: make(map[uintptr]*windowImpl),
}
type screenImpl struct {
- mu sync.Mutex
- windows map[uintptr]*windowImpl
texture struct {
program gl.Program
pos gl.Attrib
@@ -36,6 +45,9 @@
color gl.Uniform
quad gl.Buffer
}
+
+ mu sync.Mutex
+ windows map[uintptr]*windowImpl
}
func (s *screenImpl) NewBuffer(size image.Point) (retBuf screen.Buffer, retErr error) {
@@ -46,9 +58,10 @@
}
func (s *screenImpl) NewTexture(size image.Point) (screen.Texture, error) {
- s.mu.Lock()
- defer s.mu.Unlock()
+ glMu.Lock()
+ defer glMu.Unlock()
+ // TODO: can we compile these programs eagerly instead of lazily?
if !gl.IsProgram(s.texture.program) {
p, err := compileProgram(textureVertexSrc, textureFragmentSrc)
if err != nil {
diff --git a/shiny/driver/gldriver/texture.go b/shiny/driver/gldriver/texture.go
index 4e7246d..3572e41 100644
--- a/shiny/driver/gldriver/texture.go
+++ b/shiny/driver/gldriver/texture.go
@@ -23,27 +23,36 @@
func (t *textureImpl) Bounds() image.Rectangle { return image.Rectangle{Max: t.size} }
func (t *textureImpl) Release() {
+ glMu.Lock()
+ defer glMu.Unlock()
+
gl.DeleteTexture(t.id)
t.id = gl.Texture{}
}
func (t *textureImpl) Upload(dp image.Point, src screen.Buffer, sr image.Rectangle, sender screen.Sender) {
- t.upload(dp, src, sr, sender, t)
+ t.upload(dp, src, sr)
+ if sender != nil {
+ sender.Send(screen.UploadedEvent{Buffer: src, Uploader: t})
+ }
}
-func (t *textureImpl) upload(dp image.Point, src screen.Buffer, sr image.Rectangle, sender screen.Sender, u screen.Uploader) {
+func (t *textureImpl) upload(dp image.Point, src screen.Buffer, sr image.Rectangle) {
+ glMu.Lock()
+ defer glMu.Unlock()
+
// TODO: adjust if dp is outside dst bounds, or r is outside src bounds.
gl.BindTexture(gl.TEXTURE_2D, t.id)
m := src.RGBA().SubImage(sr).(*image.RGBA)
b := m.Bounds()
// TODO check m bounds smaller than t.size
gl.TexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, b.Dx(), b.Dy(), gl.RGBA, gl.UNSIGNED_BYTE, m.Pix)
- if sender != nil {
- sender.Send(screen.UploadedEvent{Buffer: src, Uploader: u})
- }
}
func (t *textureImpl) Fill(dr image.Rectangle, src color.Color, op draw.Op) {
+ glMu.Lock()
+ defer glMu.Unlock()
+
// TODO.
}
diff --git a/shiny/driver/gldriver/window.go b/shiny/driver/gldriver/window.go
index 1b59e07..2cf537f 100644
--- a/shiny/driver/gldriver/window.go
+++ b/shiny/driver/gldriver/window.go
@@ -71,7 +71,10 @@
if err != nil {
panic(err)
}
- t.(*textureImpl).upload(dp, src, sr, sender, w)
+ t.(*textureImpl).upload(dp, src, sr)
+ if sender != nil {
+ sender.Send(screen.UploadedEvent{Buffer: src, Uploader: w})
+ }
w.Draw(f64.Aff3{
1, 0, float64(dp.X),
0, 1, float64(dp.Y),
@@ -80,6 +83,9 @@
}
func (w *windowImpl) Fill(dr image.Rectangle, src color.Color, op draw.Op) {
+ glMu.Lock()
+ defer glMu.Unlock()
+
if !gl.IsProgram(w.s.fill.program) {
p, err := compileProgram(fillVertexSrc, fillFragmentSrc)
if err != nil {
@@ -126,6 +132,9 @@
}
func (w *windowImpl) Draw(src2dst f64.Aff3, src screen.Texture, sr image.Rectangle, op draw.Op, opts *screen.DrawOptions) {
+ glMu.Lock()
+ defer glMu.Unlock()
+
gl.UseProgram(w.s.texture.program)
// Start with src-space left, top, right and bottom.
@@ -242,7 +251,10 @@
//
// This enforces that the final receive (for this paint cycle) on
// gl.WorkAvailable happens before the send on publish.
+ glMu.Lock()
gl.Flush()
+ glMu.Unlock()
+
w.publish <- struct{}{}
// TODO(crawshaw): wait for an ack before returning.
return screen.PublishResult{}
diff --git a/shiny/driver/gldriver/x11.go b/shiny/driver/gldriver/x11.go
index 78ce138..2d5f1ab 100644
--- a/shiny/driver/gldriver/x11.go
+++ b/shiny/driver/gldriver/x11.go
@@ -172,7 +172,11 @@
// TODO: should this really be done on the receiving end of the w.Events()
// channel, in the same goroutine as other GL calls in the app's 'business
// logic'?
- go gl.Viewport(0, 0, int(width), int(height))
+ go func() {
+ glMu.Lock()
+ gl.Viewport(0, 0, int(width), int(height))
+ glMu.Unlock()
+ }()
sz := size.Event{
WidthPx: int(width),
