Eric Engestrom <eric.engest...@imgtec.com> writes:
> On Monday, 2017-10-16 13:54:25 +0000, Emil Velikov wrote:
>> Hi Harish,
>>
>> Overall looks great, a few comments/questions inline.
>>
>
> I agree with everything Emil said :)
>
>> 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.
>
> Which 4 are not supported? It's definitely not obvious :)
> I'm guessing dEQP-EGL.functional.negative_partial_update.not_current_surface2
> isn't supported (pbuffer), but I can't tell which other ones would be
> unsupported.
>
Sorry, should have posted it.
Here are the ones 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.
I will update the commit in v2.
>> >
>> > 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
