[Mesa-dev] [RFC] mesa: drop current draw/read buffer when ctx is released

[Tapani Pälli]

On 10/27/2016 01:48 PM, Rob Clark wrote:
> On Thu, Oct 27, 2016 at 2:59 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> On 10/27/2016 12:16 AM, Rob Clark wrote:
>>> So, not quite sure if this is the *correct* solution, but it is at least
>>> *a* solution to a problem with android wallpaper vs mesa that I've been
>>> debugging.  Basically, what happens is:
>>
>> Could you tell more how to trigger this, is this with some particular live
>> wallpaper and has this been working before (regression)? For me at least
>> these default wallpapers have been working ok.
> Actually, it is the default static wallpaper that is problematic.  And
> IIRC, it has never worked, at least not with any of the gallium
> drivers (freedreno, virgl, vc4, etc).  Live-wallpaper did work, but
> does not appear to exist in AOSP builds anymore.
>
> If this works with i965 on android, I'd be curious how.  Or if you're
> android build had some mesa patches that are not upstream?

I can confirm that default wallpaper is working on i965. Our Mesa tree 
currently looks like this:

https://github.com/android-ia/external-mesa

It's now quite a bit behind though because we were dealing with some 
non-graphics issues and are planning to rebase soon.

> BR,
> -R
>
>>>      EGLSurface tmpSurface = mEgl.eglCreatePbufferSurface(mEglDisplay,
>>> mEglConfig, attribs);
>>>      mEgl.eglMakeCurrent(mEglDisplay, tmpSurface, tmpSurface, mEglContext);
>>>
>>>      int[] maxSize = new int[1];
>>>      Rect frame = surfaceHolder.getSurfaceFrame();
>>>      glGetIntegerv(GL_MAX_TEXTURE_SIZE, maxSize, 0);
>>>
>>>      mEgl.eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE,
>>> EGL_NO_CONTEXT);
>>>      mEgl.eglDestroySurface(mEglDisplay, tmpSurface);
>>>
>>>      ... check maxSize vs frame size and bail if needed ...
>>>
>>>      mEglSurface = mEgl.eglCreateWindowSurface(mEglDisplay, mEglConfig,
>>> surfaceHolder, null);
>>>      ... error checking ...
>>>      mEgl.eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface,
>>> mEglContext);
>>>
>>> When the window-surface is created, it ends up with the same ptr address
>>> as the recently freed tmpSurface pbuffer surface.  Which after many
>>> levels of indirection, results in st_framebuffer_validate() ending up with
>>> the same/old framebuffer object, and in the end never calling the
>>> DRIimageLoaderExtension::getBuffers().  Then in droid_swap_buffers(), the
>>> dri2_surf is still the old pbuffer surface (with dri2_surf->buffer being
>>> NULL, obviously, so when wallpaper app calls eglSwapBuffers() nothing
>>> gets enqueued to the compositor).
>>>
>>> Maybe instead, eglDestroySurface() should clear any references the ctx
>>> has to the surface.  Not sure how that would work.  Did I mention there
>>> are many levels of indirection?
>>> ---
>>>    src/mesa/main/context.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>>> index 5e52065..83b8cc1 100644
>>> --- a/src/mesa/main/context.c
>>> +++ b/src/mesa/main/context.c
>>> @@ -1646,6 +1646,10 @@ ALOGE("%s:%d, drawBuffer=%p, readBuffer=%p",
>>> __func__, __LINE__, drawBuffer, rea
>>>         if (!newCtx) {
>>>          _glapi_set_dispatch(NULL);  /* none current */
>>> +      if (curCtx) {
>>> +         _mesa_reference_framebuffer(&curCtx->WinSysDrawBuffer, NULL);
>>> +         _mesa_reference_framebuffer(&curCtx->WinSysReadBuffer, NULL);
>>> +      }
>>>       }
>>>       else {
>>>          _glapi_set_dispatch(newCtx->CurrentDispatch);
>>
>>