From 34f7142b5abea9cfc7c8cb981685152dc9527df4 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Fri, 17 Jul 2015 15:59:46 -0400 Subject: [PATCH] app: be more careful about android redraw events EGL is a poor source of window dimensions. On some fraction of rotations, it reported the old dimensions. Switch to extracting the data from ANativeWindow, which appears to be more reliable. While here, plumb through orientation, and follow the recommendation in the Android platform docs to block onNativeWindowRedrawNeeded until the draw is complete. Fixes golang/go#11741. Change-Id: I6b29b6a1e5743c612c9af2a8cce4260c5d7e9ca6 Reviewed-on: https://go-review.googlesource.com/12381 Reviewed-by: Nigel Tao --- app/android.c | 5 +-- app/android.go | 93 ++++++++++++++++++++++++++++++--------------- app/loop_android.go | 67 ++++++++++++++++---------------- 3 files changed, 99 insertions(+), 66 deletions(-) diff --git a/app/android.c b/app/android.c index 9cf940c..dae5873 100644 --- a/app/android.c +++ b/app/android.c @@ -90,15 +90,14 @@ void ANativeActivity_onCreate(ANativeActivity *activity, void* savedState, size_ activity->callbacks->onDestroy = onDestroy; activity->callbacks->onWindowFocusChanged = onWindowFocusChanged; activity->callbacks->onNativeWindowCreated = onNativeWindowCreated; - activity->callbacks->onNativeWindowResized = onNativeWindowResized; activity->callbacks->onNativeWindowRedrawNeeded = onNativeWindowRedrawNeeded; activity->callbacks->onNativeWindowDestroyed = onNativeWindowDestroyed; activity->callbacks->onInputQueueCreated = onInputQueueCreated; activity->callbacks->onInputQueueDestroyed = onInputQueueDestroyed; - // TODO(crawshaw): Type mismatch for onContentRectChanged. - //activity->callbacks->onContentRectChanged = onContentRectChanged; activity->callbacks->onConfigurationChanged = onConfigurationChanged; activity->callbacks->onLowMemory = onLowMemory; + // Note that onNativeWindowResized is not called on resize. Avoid it. + // https://code.google.com/p/android/issues/detail?id=180645 onCreate(activity); } diff --git a/app/android.go b/app/android.go index 68a3a2d..3d73b91 100644 --- a/app/android.go +++ b/app/android.go @@ -81,32 +81,8 @@ func callMain(mainPC uintptr) { //export onCreate func onCreate(activity *C.ANativeActivity) { - config := C.AConfiguration_new() - C.AConfiguration_fromAssetManager(config, activity.assetManager) - density := C.AConfiguration_getDensity(config) - C.AConfiguration_delete(config) - - var dpi int - switch density { - case C.ACONFIGURATION_DENSITY_DEFAULT: - dpi = 160 - case C.ACONFIGURATION_DENSITY_LOW, - C.ACONFIGURATION_DENSITY_MEDIUM, - 213, // C.ACONFIGURATION_DENSITY_TV - C.ACONFIGURATION_DENSITY_HIGH, - 320, // ACONFIGURATION_DENSITY_XHIGH - 480, // ACONFIGURATION_DENSITY_XXHIGH - 640: // ACONFIGURATION_DENSITY_XXXHIGH - dpi = int(density) - case C.ACONFIGURATION_DENSITY_NONE: - log.Print("android device reports no screen density") - dpi = 72 - default: - log.Print("android device reports unknown density: %d", density) - dpi = int(density) // This is a guess. - } - - pixelsPerPt = float32(dpi) / 72 + config := windowConfigRead(activity) + pixelsPerPt = config.pixelsPerPt } //export onStart @@ -143,13 +119,15 @@ func onNativeWindowCreated(activity *C.ANativeActivity, w *C.ANativeWindow) { windowCreated <- w } -//export onNativeWindowResized -func onNativeWindowResized(activity *C.ANativeActivity, window *C.ANativeWindow) { -} - //export onNativeWindowRedrawNeeded func onNativeWindowRedrawNeeded(activity *C.ANativeActivity, window *C.ANativeWindow) { + // Called on orientation change and window resize. + // Send a request for redraw, and block this function + // until a complete draw and buffer swap is completed. + // This is required by the redraw documentation to + // avoid bad draws. windowRedrawNeeded <- window + <-windowRedrawDone } //export onNativeWindowDestroyed @@ -173,8 +151,61 @@ func onInputQueueDestroyed(activity *C.ANativeActivity, q *C.AInputQueue) { func onContentRectChanged(activity *C.ANativeActivity, rect *C.ARect) { } +type windowConfig struct { + // TODO(crawshaw): report orientation + // ACONFIGURATION_ORIENTATION_ANY + // ACONFIGURATION_ORIENTATION_PORT + // ACONFIGURATION_ORIENTATION_LAND + // ACONFIGURATION_ORIENTATION_SQUARE + // Needs to be merged with iOS's notion of orientation first. + orientation int + pixelsPerPt float32 +} + +func windowConfigRead(activity *C.ANativeActivity) windowConfig { + aconfig := C.AConfiguration_new() + C.AConfiguration_fromAssetManager(aconfig, activity.assetManager) + orient := C.AConfiguration_getOrientation(aconfig) + density := C.AConfiguration_getDensity(aconfig) + C.AConfiguration_delete(aconfig) + + var dpi int + switch density { + case C.ACONFIGURATION_DENSITY_DEFAULT: + dpi = 160 + case C.ACONFIGURATION_DENSITY_LOW, + C.ACONFIGURATION_DENSITY_MEDIUM, + 213, // C.ACONFIGURATION_DENSITY_TV + C.ACONFIGURATION_DENSITY_HIGH, + 320, // ACONFIGURATION_DENSITY_XHIGH + 480, // ACONFIGURATION_DENSITY_XXHIGH + 640: // ACONFIGURATION_DENSITY_XXXHIGH + dpi = int(density) + case C.ACONFIGURATION_DENSITY_NONE: + log.Print("android device reports no screen density") + dpi = 72 + default: + log.Printf("android device reports unknown density: %d", density) + // All we can do is guess. + if density > 0 { + dpi = int(density) + } else { + dpi = 72 + } + } + + return windowConfig{ + orientation: int(orient), + pixelsPerPt: float32(dpi) / 72, + } +} + //export onConfigurationChanged func onConfigurationChanged(activity *C.ANativeActivity) { + // A rotation event first triggers onConfigurationChanged, then + // calls onNativeWindowRedrawNeeded. We extract the orientation + // here and save it for the redraw event. + windowConfigChange <- windowConfigRead(activity) } //export onLowMemory @@ -185,6 +216,8 @@ var ( windowDestroyed = make(chan bool) windowCreated = make(chan *C.ANativeWindow) windowRedrawNeeded = make(chan *C.ANativeWindow) + windowRedrawDone = make(chan struct{}) + windowConfigChange = make(chan windowConfig) ) func init() { diff --git a/app/loop_android.go b/app/loop_android.go index d9d6edf..3a717d0 100644 --- a/app/loop_android.go +++ b/app/loop_android.go @@ -7,6 +7,7 @@ package app /* #include #include +#include #include #include #include @@ -23,18 +24,11 @@ const EGLint RGB_888[] = { EGL_NONE }; -EGLint windowWidth; -EGLint windowHeight; EGLDisplay display; EGLSurface surface; #define LOG_ERROR(...) __android_log_print(ANDROID_LOG_ERROR, "Go", __VA_ARGS__) -void querySurfaceWidthAndHeight() { - eglQuerySurface(display, surface, EGL_WIDTH, &windowWidth); - eglQuerySurface(display, surface, EGL_HEIGHT, &windowHeight); -} - void createEGLWindow(ANativeWindow* window) { EGLint numConfigs, format; EGLConfig config; @@ -74,8 +68,6 @@ void createEGLWindow(ANativeWindow* window) { LOG_ERROR("eglMakeCurrent failed"); return; } - - querySurfaceWidthAndHeight(); } #undef LOG_ERROR @@ -92,46 +84,55 @@ import ( "golang.org/x/mobile/gl" ) -var firstWindowDraw = true - func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan struct{}) (done bool) { - C.createEGLWindow(w) - - // TODO: is this needed if we also have the "case <-windowRedrawNeeded:" below?? - sendLifecycle(lifecycle.StageFocused) - eventsIn <- config.Event{ - Width: geom.Pt(float32(C.windowWidth) / pixelsPerPt), - Height: geom.Pt(float32(C.windowHeight) / pixelsPerPt), - PixelsPerPt: pixelsPerPt, - } - if firstWindowDraw { - firstWindowDraw = false - // TODO: be more principled about when to send a paint event. - eventsIn <- paint.Event{} - } - + // Android can send a windowRedrawNeeded event any time, including + // in the middle of a paint cycle. The redraw event may have changed + // the size of the screen, so any partial painting is now invalidated. + // We must also not return to Android (via sending on windowRedrawDone) + // until a complete paint with the new configuration is complete. + // + // When a windowRedrawNeeded request comes in, we increment redrawGen + // (Gen is short for generation number), and do not make a paint cycle + // visible on <-endPaint unless paintGen agrees. If possible, + // windowRedrawDone is signalled, allowing onNativeWindowRedrawNeeded + // to return. + var redrawGen, paintGen uint32 for { processEvents(queue) select { case <-donec: return true - case <-windowRedrawNeeded: - // Re-query the width and height. - C.querySurfaceWidthAndHeight() + case cfg := <-windowConfigChange: + // TODO save orientation + pixelsPerPt = cfg.pixelsPerPt + case w := <-windowRedrawNeeded: sendLifecycle(lifecycle.StageFocused) eventsIn <- config.Event{ - Width: geom.Pt(float32(C.windowWidth) / pixelsPerPt), - Height: geom.Pt(float32(C.windowHeight) / pixelsPerPt), + Width: geom.Pt(float32(C.ANativeWindow_getWidth(w)) / pixelsPerPt), + Height: geom.Pt(float32(C.ANativeWindow_getHeight(w)) / pixelsPerPt), PixelsPerPt: pixelsPerPt, } + if paintGen == 0 { + paintGen++ + C.createEGLWindow(w) + eventsIn <- paint.Event{} + } + redrawGen++ case <-windowDestroyed: sendLifecycle(lifecycle.StageAlive) return false case <-gl.WorkAvailable: gl.DoWork() case <-endPaint: - // eglSwapBuffers blocks until vsync. - C.eglSwapBuffers(C.display, C.surface) + if paintGen == redrawGen { + // eglSwapBuffers blocks until vsync. + C.eglSwapBuffers(C.display, C.surface) + select { + case windowRedrawDone <- struct{}{}: + default: + } + } + paintGen = redrawGen eventsIn <- paint.Event{} } }