On 04/26/2018 11:56 AM, Michel Dänzer wrote:
On 2018-04-26 11:33 AM, Thomas Hellstrom wrote:
On 04/26/2018 10:30 AM, Michel Dänzer wrote:
On 2018-04-25 07:49 PM, Deepak Rawat wrote:
Similar to what is done in dri2_x11_swap_buffers_msc send invalidate
to the driver because egl/X11 is not watching for for server's
invalidate events. The dri2_copy_region path is trigerred when
server supports DRI2 version minor 1.

Tested with piglit egl tests for regression.

Cc: <mesa-sta...@lists.freedesktop.org>
Signed-off-by: Deepak Rawat <dra...@vmware.com>
Reviewed-by: Thomas Hellstrom <thellst...@vmware.com>
---
   src/egl/drivers/dri2/platform_x11.c | 7 +++++++
   1 file changed, 7 insertions(+)

diff --git a/src/egl/drivers/dri2/platform_x11.c
b/src/egl/drivers/dri2/platform_x11.c
index 6c287b4d06..e99434ea3a 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -841,6 +841,13 @@ dri2_copy_region(_EGLDriver *drv, _EGLDisplay
*disp,
                          render_attachment);
      free(xcb_dri2_copy_region_reply(dri2_dpy->conn, cookie, NULL));
   +   /*
+    * Just like as done in dri2_x11_swap_buffers_msc we aren't
watching for
+    * server's invalidate events, so just send invalidate to driver.
+    */
+   if (dri2_dpy->flush->base.version >= 3 &&
dri2_dpy->flush->invalidate)
+      dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
+
      return EGL_TRUE;
   }
Why is invalidate needed after copy_region? That's surprising, I suspect
it just papers over the real problem.


I had a quick look into the platform_x11 code. dri2_copy_region is
called only from the various swap_buffers() paths,
dri2_x11_swap_buffers() and dri2_x11_swap_buffers_region(). Explicit
invalidation is, before this patch, done only if the server supports
dri2 swaps. Probably because most drivers do, vmware does not, so we can
hit swapbuffers without doing explicit invalidation.

In comparison, GLX does explicit invalidation on swapbuffers,
bind_context and bind_texture for the same protocol version, so this
patch should only bring the EGL swapbuffer paths closer to what GLX is
doing, while still not addressing bind_context and bind_texture.

FWIW, EGL platform_x11 (dri2) seems to not parse server invalidate
events for the higher protocol versions, relying solely on explicit
invalidation.
The purpose of the invalidate callback is to trigger updating the DRI2
buffers before drawing the next frame. This isn't necessary after a
copy_region operation, so you seem to be relying on some kind of side
effect of the invalidate callback. Which may be okay, but I think it
would be clearer not to put this code in dri2_copy_region itself.


The purpose in this case is to update the dri2 buffers after a swapbuffer operation, which *is* needed, but I agree that there might be cases where the dri2_copy_region could theoretically be used without requiring an invalidation.

So we could of course move out the invalidation to the swapbuffer functions.

/Thomas


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to