On 05/16/2017 02:04 AM, Chad Versace wrote:
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 <[email protected]>
     Subject: egl: Emit error when EGLSurface is lost

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:

     - 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".

both tests passed on Android-IA with this patch ... but if I minimize "Projection Widget" test it starts to bang EGL_BAD_NATIVE_WINDOW heavily. Is this expected behavior?

Cc: "17.1" <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: Nicolas Boichat <[email protected]>
Cc: Emil Velikov <[email protected]>
Fixes: 23c86c74 (egl: Emit error when EGLSurface is lost)
---
  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 aa0eb94666..9cea2f41ff 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.
      */

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to