shiny/driver/gldriver: be less racy wrt size events.
Set the windowImpl.sz cached value on the receiver side of the channel
communication, not spawned by a goroutine 'soon' after the send.
This CL only affects x11. The same change applies in principle to cocoa
and win32, but I have not (manually) tested it there yet.
Change-Id: Ibe4bad965fe0c059378daef2f535dcae362447a9
Reviewed-on: https://go-review.googlesource.com/37918
Reviewed-by: David Crawshaw <crawshaw@golang.org>
diff --git a/shiny/driver/gldriver/cocoa.go b/shiny/driver/gldriver/cocoa.go
index cc01582..8e44d5b 100644
--- a/shiny/driver/gldriver/cocoa.go
+++ b/shiny/driver/gldriver/cocoa.go
@@ -48,6 +48,9 @@
const useLifecycler = true
+// TODO: change this to true, after manual testing on OS X.
+const handleSizeEventsAtChannelReceive = false
+
var initThreadID C.uint64_t
func init() {
@@ -193,9 +196,11 @@
PixelsPerPt: ppp,
}
- w.szMu.Lock()
- w.sz = sz
- w.szMu.Unlock()
+ if !handleSizeEventsAtChannelReceive {
+ w.szMu.Lock()
+ w.sz = sz
+ w.szMu.Unlock()
+ }
w.Send(sz)
}
diff --git a/shiny/driver/gldriver/win32.go b/shiny/driver/gldriver/win32.go
index 9e73f71..343fb45 100755
--- a/shiny/driver/gldriver/win32.go
+++ b/shiny/driver/gldriver/win32.go
@@ -23,8 +23,12 @@
"golang.org/x/mobile/gl"
)
+// TODO: change this to true, after manual testing on Win32.
const useLifecycler = false
+// TODO: change this to true, after manual testing on Win32.
+const handleSizeEventsAtChannelReceive = false
+
func main(f func(screen.Screen)) error {
return win32.Main(func() { f(theScreen) })
}
@@ -170,6 +174,7 @@
return
}
+ // TODO: the paint.Event should have External: true.
w.Send(paint.Event{})
}
@@ -187,12 +192,18 @@
go drawLoop(w)
}
- w.szMu.Lock()
- w.sz = e
- w.szMu.Unlock()
+ if !handleSizeEventsAtChannelReceive {
+ w.szMu.Lock()
+ w.sz = e
+ w.szMu.Unlock()
+ }
w.Send(e)
+ if handleSizeEventsAtChannelReceive {
+ return
+ }
+
// Screen is dirty, generate a paint event.
//
// The sizeEvent function is called on the goroutine responsible for
diff --git a/shiny/driver/gldriver/window.go b/shiny/driver/gldriver/window.go
index 522fcca..82baf2c 100644
--- a/shiny/driver/gldriver/window.go
+++ b/shiny/driver/gldriver/window.go
@@ -65,6 +65,22 @@
sz size.Event
}
+// NextEvent implements the screen.EventDeque interface.
+func (w *windowImpl) NextEvent() interface{} {
+ e := w.Deque.NextEvent()
+ if handleSizeEventsAtChannelReceive {
+ if sz, ok := e.(size.Event); ok {
+ w.glctxMu.Lock()
+ w.backBufferBound = false
+ w.szMu.Lock()
+ w.sz = sz
+ w.szMu.Unlock()
+ w.glctxMu.Unlock()
+ }
+ }
+ return e
+}
+
func (w *windowImpl) Release() {
// There are two ways a window can be closed: the Operating System or
// Desktop Environment can initiate (e.g. in response to a user clicking a
diff --git a/shiny/driver/gldriver/x11.go b/shiny/driver/gldriver/x11.go
index 28ef104..2a3b8f6 100644
--- a/shiny/driver/gldriver/x11.go
+++ b/shiny/driver/gldriver/x11.go
@@ -42,6 +42,8 @@
const useLifecycler = true
+const handleSizeEventsAtChannelReceive = true
+
var theKeysyms x11key.KeysymTable
func init() {
@@ -274,18 +276,6 @@
return
}
- // 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 func() {
- w.glctxMu.Lock()
- // Force a w.glctx.Viewport call.
- //
- // TODO: is this racy? See also the TODO immediately above.
- w.backBufferBound = false
- w.glctxMu.Unlock()
- }()
-
w.lifecycler.SetVisible(x+width > 0 && y+height > 0)
w.lifecycler.SendEvent(w, w.glctx)
@@ -294,19 +284,13 @@
ptPerInch = 72
)
pixelsPerMM := float32(displayWidth) / float32(displayWidthMM)
- sz := size.Event{
+ w.Send(size.Event{
WidthPx: int(width),
HeightPx: int(height),
WidthPt: geom.Pt(width),
HeightPt: geom.Pt(height),
PixelsPerPt: pixelsPerMM * mmPerInch / ptPerInch,
- }
-
- w.szMu.Lock()
- w.sz = sz
- w.szMu.Unlock()
-
- w.Send(sz)
+ })
}
//export onDeleteWindow
