[Mesa-dev] [PATCH v3] egl/wayland: Support for KHR_partial_update

[Eric Engestrom]

On Monday, 2017-10-23 16:20:54 +0530, Harish Krupo wrote:
> This passes 33/37 deqp tests related to partial_update, 4 are not
> supported. Tests not supported:
> dEQP-EGL.functional.negative_partial_update.not_postable_surface
> dEQP-EGL.functional.negative_partial_update.not_current_surface
> dEQP-EGL.functional.negative_partial_update.buffer_preserved
> dEQP-EGL.functional.negative_partial_update.not_current_surface2
> Reason: No matching egl config found.
> 
> v2: Remove unnecessary return statement. Keep function names
>     consistent.  (Emil Velikov)
>     Add not supported list to commit message. (Eric Engestrom)
> 
> v3: Remove explicit with_damage variable. (Eric Engestrom)
> 
> Signed-off-by: Harish Krupo <harish.krupo.kps at intel.com>
> ---
>  src/egl/drivers/dri2/platform_wayland.c | 54 ++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> index b38eb1c335..8846099d57 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -790,27 +790,44 @@ create_wl_buffer(struct dri2_egl_display *dri2_dpy,
>     return ret;
>  }
>  
> +/**
> + * Called via eglSetDamageRegionKHR(), drv->API.SetDamageRegion().
> + */
>  static EGLBoolean
> -try_damage_buffer(struct dri2_egl_surface *dri2_surf,
> -                  const EGLint *rects,
> -                  EGLint n_rects)
> +dri2_wl_set_damage_region(_EGLDriver *drv,
> +                     _EGLDisplay *dpy,
> +                     _EGLSurface *surf,
> +                     const EGLint *rects,
> +                     EGLint n_rects)
>  {
> -   if (wl_proxy_get_version((struct wl_proxy *) dri2_surf->wl_surface_wrapper)
> -       < WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION)
> -      return EGL_FALSE;
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
>  
> -   for (int i = 0; i < n_rects; i++) {
> -      const int *rect = &rects[i * 4];
> +   /* The spec doesn't mention what should be returned in case of
> +    * failure in setting the damage buffer with the window system, so
> +    * setting the damage to maximum surface area
> +    */
> +   if (!n_rects ||
> +       wl_proxy_get_version((struct wl_proxy *) dri2_surf->wl_surface_wrapper)
> +       < WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION) {
> +      wl_surface_damage(dri2_surf->wl_surface_wrapper,
> +                        0, 0, INT32_MAX, INT32_MAX);
> +   } else {

I know Emil suggested you remove the `return` in an earlier version, but
if you add it back here you can drop the else, and the diff will look
much cleaner, keeping only the version check getting an `|| !n_rects`
and `return false` becoming `damage(everything)`.

Other than that, it looks good to me. Thanks :)

> +      for (int i = 0; i < n_rects; i++) {
> +         const int *rect = &rects[i * 4];
>  
> -      wl_surface_damage_buffer(dri2_surf->wl_surface_wrapper,
> -                               rect[0],
> -                               dri2_surf->base.Height - rect[1] - rect[3],
> -                               rect[2], rect[3]);
> +         wl_surface_damage_buffer(dri2_surf->wl_surface_wrapper,
> +                                  rect[0],
> +                                  dri2_surf->base.Height - rect[1] - rect[3],
> +                                  rect[2], rect[3]);
> +      }
>     }
> +
>     return EGL_TRUE;
>  }
> +
>  /**
> - * Called via eglSwapBuffers(), drv->API.SwapBuffers().
> + * Called via eglSwapBuffersWithDamage{KHR,EXT}(),
> + * drv->API.SwapBuffersWithDamageEXT().
>   */
>  static EGLBoolean
>  dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> @@ -875,9 +892,8 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
>     /* If the compositor doesn't support damage_buffer, we deliberately
>      * ignore the damage region and post maximum damage, due to
>      * https://bugs.freedesktop.org/78190 */
> -   if (!n_rects || !try_damage_buffer(dri2_surf, rects, n_rects))
> -      wl_surface_damage(dri2_surf->wl_surface_wrapper,
> -                        0, 0, INT32_MAX, INT32_MAX);
> +   if (n_rects || !dri2_surf->base.SetDamageRegionCalled)
> +      dri2_wl_set_damage_region(drv, disp, draw, rects, n_rects);
>  
>     if (dri2_dpy->is_different_gpu) {
>        _EGLContext *ctx = _eglGetCurrentContext();
> @@ -928,7 +944,8 @@ dri2_wl_query_buffer_age(_EGLDriver *drv,
>  static EGLBoolean
>  dri2_wl_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>  {
> -   return dri2_wl_swap_buffers_with_damage(drv, disp, draw, NULL, 0);
> +   return dri2_wl_swap_buffers_with_damage(drv, disp, draw,
> +                                      NULL, 0);
>  }
>  
>  static struct wl_buffer *
> @@ -1166,7 +1183,7 @@ static const struct dri2_egl_display_vtbl dri2_wl_display_vtbl = {
>     .swap_buffers = dri2_wl_swap_buffers,
>     .swap_buffers_with_damage = dri2_wl_swap_buffers_with_damage,
>     .swap_buffers_region = dri2_fallback_swap_buffers_region,
> -   .set_damage_region = dri2_fallback_set_damage_region,
> +   .set_damage_region = dri2_wl_set_damage_region,
>     .post_sub_buffer = dri2_fallback_post_sub_buffer,
>     .copy_buffers = dri2_fallback_copy_buffers,
>     .query_buffer_age = dri2_wl_query_buffer_age,
> @@ -1378,6 +1395,7 @@ dri2_initialize_wayland_drm(_EGLDriver *drv, _EGLDisplay *disp)
>     disp->Extensions.EXT_buffer_age = EGL_TRUE;
>  
>     disp->Extensions.EXT_swap_buffers_with_damage = EGL_TRUE;
> +   disp->Extensions.KHR_partial_update = EGL_TRUE;
>  
>     /* Fill vtbl last to prevent accidentally calling virtual function during
>      * initialization.
> -- 
> 2.14.2
>