From 4c15c79e19fcb3cd583c314ca482221af5b6b87e Mon Sep 17 00:00:00 2001 From: Nigel Tao Date: Fri, 4 Sep 2015 17:13:09 +1000 Subject: [PATCH] app: change EndPaint to Publish. More than a name change, the painting model changes so that the app, not the library, is responsible for driving painting. If the app is animating and wants paint events at 60 Hz, it has to ask for that. If the app is not animating and doesn't need to update its screen, it shouldn't get any paint events. Plenty of TODOs, and this CL doesn't get us to a perfect place, but it is a checkpoint along the way. The darwin_*.go code changes were minimal. I don't even have a Mac or iOS device to test that this even builds. Even so, the TODOs about not sending paint.Events unconditionally are important TODOs. That's the whole point of switching to this model. I'll leave the actual implementation to you (crawshaw). Out of all the example apps, the change to example/network/main.go is probably the most interesting. It seems like there ought to be some way to reduce the copy/paste between all of the example app code, but I'll leave that for future CLs. Change-Id: I17e11c06174110c68e17f7183b2d8af19b6a170e Reviewed-on: https://go-review.googlesource.com/14300 Reviewed-by: David Crawshaw --- app/android.go | 13 +++++-------- app/app.go | 26 +++++++++++++++++--------- app/darwin_amd64.go | 32 ++++++++++++++++++++------------ app/darwin_armx.go | 5 ++++- app/doc.go | 2 +- app/internal/testapp/testapp.go | 3 ++- app/x11.go | 10 +++++++--- event/paint/paint.go | 2 ++ example/audio/main.go | 9 ++++++++- example/basic/main.go | 15 +++++++++++++-- example/network/main.go | 23 +++++++++++++++-------- example/sprite/main.go | 16 ++++++++++++++-- 12 files changed, 108 insertions(+), 48 deletions(-) diff --git a/app/android.go b/app/android.go index a88ba3c..145c58b 100644 --- a/app/android.go +++ b/app/android.go @@ -299,6 +299,8 @@ func mainUI(vm, jniEnv, ctx uintptr) error { // visible on <-endPaint unless Generation agrees. If possible, // windowRedrawDone is signalled, allowing onNativeWindowRedrawNeeded // to return. + // + // TODO: is this still needed? var redrawGen uint32 for { @@ -338,10 +340,8 @@ func mainUI(vm, jniEnv, ctx uintptr) error { sendLifecycle(lifecycle.StageAlive) case <-gl.WorkAvailable: gl.DoWork() - case p := <-endPaint: - if p.Generation != redrawGen { - continue - } + case <-publish: + // TODO: compare a generation number to redrawGen for stale paints? if C.surface != nil { // eglSwapBuffers blocks until vsync. if C.eglSwapBuffers(C.display, C.surface) == C.EGL_FALSE { @@ -352,10 +352,7 @@ func mainUI(vm, jniEnv, ctx uintptr) error { case windowRedrawDone <- struct{}{}: default: } - if C.surface != nil { - redrawGen++ - eventsIn <- paint.Event{redrawGen} - } + publishResult <- PublishResult{} } } } diff --git a/app/app.go b/app/app.go index 8c1cce2..3aab126 100644 --- a/app/app.go +++ b/app/app.go @@ -8,7 +8,6 @@ package app import ( "golang.org/x/mobile/event/lifecycle" - "golang.org/x/mobile/event/paint" "golang.org/x/mobile/event/size" "golang.org/x/mobile/gl" _ "golang.org/x/mobile/internal/mobileinit" @@ -38,17 +37,25 @@ type App interface { // Send sends an event on the events channel. It does not block. Send(event interface{}) - // EndPaint flushes any pending OpenGL commands or buffers to the screen. - // If EndPaint is called with an old generation number, it is ignored. - EndPaint(paint.Event) + // Publish flushes any pending drawing commands, such as OpenGL calls, and + // swaps the back buffer to the screen. + Publish() PublishResult +} + +// PublishResult is the result of an App.Publish call. +type PublishResult struct { + // BackBufferPreserved is whether the contents of the back buffer was + // preserved. If false, the contents are undefined. + BackBufferPreserved bool } var ( lifecycleStage = lifecycle.StageDead - eventsOut = make(chan interface{}) - eventsIn = pump(eventsOut) - endPaint = make(chan paint.Event, 1) + eventsOut = make(chan interface{}) + eventsIn = pump(eventsOut) + publish = make(chan struct{}) + publishResult = make(chan PublishResult) ) func sendLifecycle(to lifecycle.Stage) { @@ -72,7 +79,7 @@ func (app) Send(event interface{}) { eventsIn <- event } -func (app) EndPaint(e paint.Event) { +func (app) Publish() PublishResult { // gl.Flush is a lightweight (on modern GL drivers) blocking call // that ensures all GL functions pending in the gl package have // been passed onto the GL driver before the app package attempts @@ -81,7 +88,8 @@ func (app) EndPaint(e paint.Event) { // This enforces that the final receive (for this paint cycle) on // gl.WorkAvailable happens before the send on endPaint. gl.Flush() - endPaint <- e + publish <- struct{}{} + return <-publishResult } var filters []func(interface{}) interface{} diff --git a/app/darwin_amd64.go b/app/darwin_amd64.go index 2c1e561..b408fde 100644 --- a/app/darwin_amd64.go +++ b/app/darwin_amd64.go @@ -79,19 +79,24 @@ func loop(ctx C.GLintptr) { runtime.LockOSThread() C.makeCurrentContext(ctx) - for range draw { - eventsIn <- paint.Event{} - loop1: - for { - select { - case <-gl.WorkAvailable: - gl.DoWork() - case <-endPaint: - C.CGLFlushDrawable(C.CGLGetCurrentContext()) - break loop1 + for { + select { + case <-gl.WorkAvailable: + gl.DoWork() + case <-draw: + loop1: + for { + select { + case <-gl.WorkAvailable: + gl.DoWork() + case <-publish: + C.CGLFlushDrawable(C.CGLGetCurrentContext()) + publishResult <- PublishResult{} + break loop1 + } } + drawDone <- struct{}{} } - drawDone <- struct{}{} } } @@ -204,7 +209,10 @@ var mods = [...]struct { func lifecycleAlive() { sendLifecycle(lifecycle.StageAlive) } //export lifecycleVisible -func lifecycleVisible() { sendLifecycle(lifecycle.StageVisible) } +func lifecycleVisible() { + sendLifecycle(lifecycle.StageVisible) + eventsIn <- paint.Event{} +} //export lifecycleFocused func lifecycleFocused() { sendLifecycle(lifecycle.StageFocused) } diff --git a/app/darwin_armx.go b/app/darwin_armx.go index 8730119..d31f660 100644 --- a/app/darwin_armx.go +++ b/app/darwin_armx.go @@ -177,13 +177,16 @@ func drawgl(ctx uintptr) { sendLifecycle(lifecycle.StageFocused) } + // TODO(crawshaw): don't send a paint.Event unconditionally. Only send one + // if the window actually needs redrawing. eventsIn <- paint.Event{} for { select { case <-gl.WorkAvailable: gl.DoWork() - case <-endPaint: + case <-publish: + publishResult <- PublishResult{} return } } diff --git a/app/doc.go b/app/doc.go index 13a9195..91ae872 100644 --- a/app/doc.go +++ b/app/doc.go @@ -60,7 +60,7 @@ goroutine as other code that calls OpenGL. // ... case paint.Event: log.Print("Call OpenGL here.") - a.EndPaint(e) + a.Publish() } } }) diff --git a/app/internal/testapp/testapp.go b/app/internal/testapp/testapp.go index 3e2d417..b4672da 100644 --- a/app/internal/testapp/testapp.go +++ b/app/internal/testapp/testapp.go @@ -61,7 +61,7 @@ func main() { gl.ClearColor(0, 1, 0, 1) } gl.Clear(gl.COLOR_BUFFER_BIT) - a.EndPaint(e) + a.Publish() if sendPainting { comm.Send("paint", color) sendPainting = false @@ -76,6 +76,7 @@ func main() { } sendPainting = true } + // TODO: send our own paint Event?? } } }) diff --git a/app/x11.go b/app/x11.go index 0d70690..cb58d14 100644 --- a/app/x11.go +++ b/app/x11.go @@ -46,6 +46,10 @@ func main(f func(App)) { // TODO: send lifecycle events when e.g. the X11 window is iconified or moved off-screen. sendLifecycle(lifecycle.StageFocused) + // TODO: translate X11 expose events to shiny paint events, instead of + // sending this synthetic paint event as a hack. + eventsIn <- paint.Event{} + donec := make(chan struct{}) go func() { f(app{}) @@ -55,7 +59,7 @@ func main(f func(App)) { // TODO: can we get the actual vsync signal? ticker := time.NewTicker(time.Second / 60) defer ticker.Stop() - tc := ticker.C + var tc <-chan time.Time for { select { @@ -63,12 +67,12 @@ func main(f func(App)) { return case <-gl.WorkAvailable: gl.DoWork() - case <-endPaint: + case <-publish: C.swapBuffers() tc = ticker.C case <-tc: tc = nil - eventsIn <- paint.Event{} + publishResult <- PublishResult{} } C.processEvents() } diff --git a/event/paint/paint.go b/event/paint/paint.go index f63f758..1c9c557 100644 --- a/event/paint/paint.go +++ b/event/paint/paint.go @@ -11,5 +11,7 @@ package paint // import "golang.org/x/mobile/event/paint" // frame is completed by calling the App's EndPaint method. type Event struct { // Generation is a monotonically increasing generation number. + // + // TODO: is a generation number the right model for stale paints? Generation uint32 } diff --git a/example/audio/main.go b/example/audio/main.go index b7a2b9f..3c4a84d 100644 --- a/example/audio/main.go +++ b/example/audio/main.go @@ -71,20 +71,27 @@ var ( func main() { app.Main(func(a app.App) { + visible := false for e := range a.Events() { switch e := app.Filter(e).(type) { case lifecycle.Event: switch e.Crosses(lifecycle.StageVisible) { case lifecycle.CrossOn: + visible = true onStart() case lifecycle.CrossOff: + visible = false onStop() } case size.Event: sz = e case paint.Event: onPaint() - a.EndPaint(e) + a.Publish() + if visible { + // Keep animating. + a.Send(paint.Event{}) + } } } }) diff --git a/example/basic/main.go b/example/basic/main.go index 4cec6b8..abec164 100644 --- a/example/basic/main.go +++ b/example/basic/main.go @@ -57,14 +57,16 @@ var ( func main() { app.Main(func(a app.App) { - var sz size.Event + visible, sz := false, size.Event{} for e := range a.Events() { switch e := app.Filter(e).(type) { case lifecycle.Event: switch e.Crosses(lifecycle.StageVisible) { case lifecycle.CrossOn: + visible = true onStart() case lifecycle.CrossOff: + visible = false onStop() } case size.Event: @@ -73,7 +75,16 @@ func main() { touchY = float32(sz.HeightPx / 2) case paint.Event: onPaint(sz) - a.EndPaint(e) + a.Publish() + if visible { + // Drive the animation by preparing to paint the next frame + // after this one is shown. + // + // TODO: is paint.Event the right thing to send? Should we + // have a dedicated publish.Event type? Should App.Publish + // take an optional event sender and send a publish.Event? + a.Send(paint.Event{}) + } case touch.Event: touchX = e.X touchY = e.Y diff --git a/example/network/main.go b/example/network/main.go index 991b13a..81872e9 100644 --- a/example/network/main.go +++ b/example/network/main.go @@ -54,14 +54,21 @@ func main() { go checkNetwork() app.Main(func(a app.App) { - var sz size.Event - for e := range a.Events() { - switch e := app.Filter(e).(type) { - case size.Event: - sz = e - case paint.Event: - onDraw(sz) - a.EndPaint(e) + det, sz := determined, size.Event{} + for { + select { + case <-det: + a.Send(paint.Event{}) + det = nil + + case e := <-a.Events(): + switch e := app.Filter(e).(type) { + case size.Event: + sz = e + case paint.Event: + onDraw(sz) + a.Publish() + } } } }) diff --git a/example/sprite/main.go b/example/sprite/main.go index 0f832d5..a18d870 100644 --- a/example/sprite/main.go +++ b/example/sprite/main.go @@ -38,6 +38,7 @@ import ( "golang.org/x/mobile/app" "golang.org/x/mobile/asset" + "golang.org/x/mobile/event/lifecycle" "golang.org/x/mobile/event/paint" "golang.org/x/mobile/event/size" "golang.org/x/mobile/exp/app/debug" @@ -56,14 +57,25 @@ var ( func main() { app.Main(func(a app.App) { - var sz size.Event + visible, sz := false, size.Event{} for e := range a.Events() { switch e := app.Filter(e).(type) { + case lifecycle.Event: + switch e.Crosses(lifecycle.StageVisible) { + case lifecycle.CrossOn: + visible = true + case lifecycle.CrossOff: + visible = false + } case size.Event: sz = e case paint.Event: onPaint(sz) - a.EndPaint(e) + a.Publish() + if visible { + // Keep animating. + a.Send(paint.Event{}) + } } } })