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. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
