Hi, On Tue, May 16, 2017 at 11:35 PM, Tapani Pälli <[email protected]> wrote: > > > On 05/16/2017 08:10 PM, Chad Versace wrote: >> >> On Tue 16 May 2017, Tapani Pälli wrote: >>> >>> >>> >>> 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? >> >> >> I'm unsure. I haven't yet tried that experiment. I'll give it a try when >> I'm back at my desk. >> >> Which EGL function is emitting EGL_BAD_NATIVE_WINDOW in logcat? >> eglMakeCurrent or eglSwapBuffers, or something else? > > > doh sorry, I actually meant 'Projection Cube' test, not the widget test. But > yep, when tapping switcher button eglMakeCurrent starts to emit the error > (likely the app just continues to submit frames even when it's > 'minimized'?). When switching back to app it cannot draw anymore as it keeps > getting the error.
I don't remember exactly and I'm not an Android app expert either, but I remember hearing something that the app needs to stop drawing when the activity is paused (stopped?). Isn't that what's happening when an app gets minimized? Best regards, Tomasz > >> What was the behavior before this patch? And before >> commit 23c86c74 (egl: Emit error when EGLSurface is lost)? > > > Without this patch (and before commit 23c86c74) I can minimize app to > switcher and bring it back and it continues to draw. > > Note that everything else seems to work just fine though (tested lots of GL > apps) so is this just something specific what cts-verifier is doing or not > handling properly? > > > >>>> 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 > > _______________________________________________ > mesa-stable mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-stable _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
