Hi Harish,
Overall looks great, a few comments/questions inline.
On 13 October 2017 at 19:49, Harish Krupo <harish.krupo....@intel.com> wrote:
> This passes 33/37 deqp tests related to partial_update, 4 are not
> supported.
>
> Signed-off-by: Harish Krupo <harish.krupo....@intel.com>
> ---
> src/egl/drivers/dri2/platform_wayland.c | 68
> ++++++++++++++++++++++++++++-----
> 1 file changed, 59 insertions(+), 9 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index 14db55ca74..483d588b92 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -810,15 +810,39 @@ try_damage_buffer(struct dri2_egl_surface *dri2_surf,
> }
> return EGL_TRUE;
> }
> +
> /**
> - * Called via eglSwapBuffers(), drv->API.SwapBuffers().
> + * Called via eglSetDamageRegionKHR(), drv->API.SetDamageRegion().
> */
> static EGLBoolean
> -dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> - _EGLDisplay *disp,
> - _EGLSurface *draw,
> - const EGLint *rects,
> - EGLint n_rects)
> +wl_set_damage_region(_EGLDriver *drv,
Yes, it might be a bit confusing, but please stay consistent and call
this dri2_wl_set_damage_region.
> + _EGLDisplay *dpy,
> + _EGLSurface *surf,
> + const EGLint *rects,
> + EGLint n_rects)
> +{
> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +
> + /* 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 || !try_damage_buffer(dri2_surf, rects, n_rects)) {
> + wl_surface_damage(dri2_surf->wl_surface_wrapper,
> + 0, 0, INT32_MAX, INT32_MAX);
> + return EGL_TRUE;
Drop this return - it's only confusing the reader.
> + }
> +
> + return EGL_TRUE;
... since this should suffice.
> +}
> +
> +static EGLBoolean
> +dri2_wl_swap_buffers_common(_EGLDriver *drv,
> + _EGLDisplay *disp,
> + _EGLSurface *draw,
> + const EGLint *rects,
> + EGLint n_rects,
> + EGLBoolean with_damage)
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
> @@ -876,7 +900,17 @@ 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))
> +
> + if (!with_damage) {
> +
> + /* If called from swapBuffers, check if the damage region
> + * is already set, if not set to full damage
> + */
> + if (!dri2_surf->base.SetDamageRegionCalled)
> + wl_surface_damage(dri2_surf->wl_surface_wrapper,
> + 0, 0, INT32_MAX, INT32_MAX);
> + }
> + else 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);
>
> @@ -912,6 +946,20 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> return EGL_TRUE;
> }
>
> +/**
> + * Called via eglSwapBuffers(), drv->API.SwapBuffers().
> + */
> +static EGLBoolean
> +dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
> + _EGLDisplay *disp,
> + _EGLSurface *draw,
> + const EGLint *rects,
> + EGLint n_rects)
> +{
> + return dri2_wl_swap_buffers_common(drv, disp, draw,
> + rects, n_rects, EGL_TRUE);
Hmm what will happen if use calls:
eglSetDamageRegionKHR(dpy, surf, rects, n_rects)
...
eglSwapBuffersWithDamageKHR(dpy, surf, NULL, 0)
eglSwapBuffersWithDamageKHR should behave identical as eglSwapBuffers, correct?
If so, the proposed code seems buggy - we might as well drop the
explicit with_damage and derive it from rects/n_rects locally.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
