[Mesa-dev] [PATCH] egl/wayland: Try to use wl_surface.damage_buffer for SwapBuffersWithDamage

[Emil Velikov]

On 12 February 2016 at 23:09, Derek Foreman <derekf at osg.samsung.com> wrote:
> On 12/02/16 04:46 PM, Emil Velikov wrote:
>> On 11 February 2016 at 16:34, Derek Foreman <derekf at osg.samsung.com> wrote:
>>> Since commit d1314de293e9e4a63c35f094c3893aaaed8580b4 we ignore
>>> damage passed to SwapBuffersWithDamage.
>>>
>>> Wayland 1.10 now has functionality that allows us to properly
>>> process those damage rectangles, and a way to query if it's
>>> available.
>>>
>>> Now we can use wl_surface.damage_buffer and interpret the incoming
>>> damage as being in buffer co-ordinates.
>>>
>>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>>> ---
>>>  src/egl/drivers/dri2/platform_wayland.c | 32 +++++++++++++++++++++++++++++---
>>>  1 file changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
>>> index c2438f7..b5a5b59 100644
>>> --- a/src/egl/drivers/dri2/platform_wayland.c
>>> +++ b/src/egl/drivers/dri2/platform_wayland.c
>>> @@ -653,6 +653,30 @@ create_wl_buffer(struct dri2_egl_surface *dri2_surf)
>>>                            &wl_buffer_listener, dri2_surf);
>>>  }
>>>
>>> +static EGLBoolean
>>> +try_damage_buffer(struct dri2_egl_surface *dri2_surf,
>>> +                  const EGLint *rects,
>>> +                  EGLint n_rects)
>>> +{
>>> +#ifdef WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION
>>> +   int i;
>>> +
>>> +   if (wl_proxy_get_version((struct wl_proxy *) dri2_surf->wl_win->surface)
>>> +       < WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION)
>>> +      return EGL_FALSE;
>>> +
>>> +   for (i = 0; i < n_rects; i++) {
>>> +      const int *rect = &rects[i * 4];
>>> +
>>> +      wl_surface_damage_buffer(dri2_surf->wl_win->surface,
>>> +                               rect[0],
>>> +                               dri2_surf->base.Height - rect[1] - rect[3],
>>> +                               rect[2], rect[3]);
>>> +   }
>>> +   return EGL_TRUE;
>>> +#endif
>>
>> I'm slightly worried about keeping this compile time. For example if
>> we compile against old wayland, and then run against a capable one...
>> this code won't exist thus will never get executed. Thus leading to a
>> handful of "wtf" moments.
>>
>> I would just ensure it's defined at the top.
>>
>> #ifndef WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION
>> #define WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION foo
>> #endif
>
> If your wayland headers don't have
> WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION then compilation will fail
> because wl_proxy_get_version() isn't present.
>
> wl_proxy_get_version() will be formally introduced in the same wayland
> release (next Tuesday's 1.10 release) as wl_surface_damage_buffer(), so
> I'm intentionally using that as a tricky way to avoid the check
> entirely.  Too tricky?
>
Slightly confusing/misleading, than anything else.

> If you prefer I can work out some autoconfy stuff to determine if
> wl_proxy_get_version() is available?
>
Personally I'd love to bump the wayland requirement... although we
cannot do that yet. Can you please add a small comment * above the
ifdef line. "wayland XX introduced wl_proxy_get_version() and
CRAZY_LONG_MACRO_NAME" or anything that you feel will make it dead
obvious.

Thanks
Emil