From 8f62d21bd72059d723f4626f76de6fec4a569616 Mon Sep 17 00:00:00 2001 From: Chad Versace Date: Thu, 11 May 2017 21:49:14 -0700 Subject: [PATCH] egl: Partially revert 23c86c74, fix eglMakeCurrent MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fixes regressions in Android CtsVerifier.apk on Intel Chrome OS devices due to incorrect error handling in eglMakeCurrent. See below on how to confirm the regression is fixed. This partially reverts commit 23c86c74cc450a23848b85cfe914376caede1cdf Author: Chad Versace Subject: egl: Emit error when EGLSurface is lost The problem with commit 23c86c74 is that, once an EGLSurface became lost, the app could never unbind the bad surface. Each attempt to unbind the bad surface with eglMakeCurrent failed with EGL_BAD_CURRENT_SURFACE. Specificaly, the bad commit added the error handling below. #2 and #3 were right, but #1 was wrong. 1. eglMakeCurrent emits EGL_BAD_CURRENT_SURFACE if the calling thread has unflushed commands and either previous surface is no longer valid. 2. eglMakeCurrent emits EGL_BAD_NATIVE_WINDOW if either new surface is no longer valid. 3. eglSwapBuffers emits EGL_BAD_NATIVE_WINDOW if the swapped surface is no longer valid. Whe I wrote the bad commit, I misunderstood the EGL spec language for #1. The correct behavior is, if I understand correctly now, is below. This patch doesn't implement the correct behavior, though, it just reverts the broken behavior. - Assume a bound EGLSurface is no longer valid. - Assume the bound EGLContext has unflushed commands. - The app calls eglMakeCurrent. The spec requires eglMakeCurrent to implicitly flush. After flushing, eglMakeCurrent emits EGL_BAD_CURRENT_SURFACE and does *not* alter the thread's current bindings. - If the app calls eglMakeCurrent again, and the app inserts no commands into the GL command stream between the two eglMakeCurrent calls, then this second eglMakeCurrent succeeds without emitting an error. How to confirm this fixes the regression: Download android-cts-verifier-7.1_r5-linux_x86-x86.zip from source.android.com, unpack, and `adb install CtsVerifier.apk`. Run test "Projection Cube". Click the Pass button (a green checkmark). Then run test "Projection Widget". Confirm that widgets are visible and that logcat does not complain about eglMakeCurrent failure. Then confirm there are no regressions in the cts-traded module that commit 263243b1 fixed: cts-tf > run cts --skip-preconditions --skip-device-info \ -m CtsCameraTestCases \ -t android.hardware.camera2.cts.RobustnessTest Tested with Chrome OS board "reef". Fixes: 23c86c74 (egl: Emit error when EGLSurface is lost) Acked-by: Tapani Pälli Cc: "17.1" Cc: Tomasz Figa Cc: Nicolas Boichat Cc: Emil Velikov --- src/egl/main/eglapi.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c index aa0eb94..9cea2f4 100644 --- a/src/egl/main/eglapi.c +++ b/src/egl/main/eglapi.c @@ -828,25 +828,6 @@ eglMakeCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface read, RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_FALSE); } - _EGLThreadInfo *t =_eglGetCurrentThread(); - _EGLContext *old_ctx = t->CurrentContext; - _EGLSurface *old_draw_surf = old_ctx ? old_ctx->DrawSurface : NULL; - _EGLSurface *old_read_surf = old_ctx ? old_ctx->ReadSurface : NULL; - - /* From the EGL 1.5 spec, Section 3.7.3 Binding Context and Drawables: - * - * If the previous context of the calling thread has unflushed commands, - * and the previous surface is no longer valid, an - * EGL_BAD_CURRENT_SURFACE error is generated. - * - * It's difficult to check if the context has unflushed commands, but it's - * easy to check if the surface is no longer valid. - */ - if (old_draw_surf && old_draw_surf->Lost) - RETURN_EGL_ERROR(disp, EGL_BAD_CURRENT_SURFACE, EGL_FALSE); - if (old_read_surf && old_read_surf->Lost) - RETURN_EGL_ERROR(disp, EGL_BAD_CURRENT_SURFACE, EGL_FALSE); - /* If a native window underlying either draw or read is no longer valid, * an EGL_BAD_NATIVE_WINDOW error is generated. */ -- 2.7.4