[PATCH xserver] xwayland: use wl_surface_damage_buffer

[Pekka Paalanen]

On Mon, 05 Nov 2018 11:58:26 +0000
Simon Ser <contact at emersion.fr> wrote:

> wl_surface_damage can be unoptimal on some compositors, damaging the whole
> buffer instead of the provided region. wl_surface_damage_buffer is preferred.
> Since xwayland doesn't set a surface offset, scale or transform, surface
> damage is effectively equivalent to buffer damage.
> 
> Signed-off-by: Simon Ser <contact at emersion.fr>
> ---
> 
> See https://lists.freedesktop.org/archives/wayland-devel/2018-November/039608.html
> 
>  hw/xwayland/xwayland-cursor.c  | 12 ++++++------
>  hw/xwayland/xwayland-present.c |  6 +++---
>  hw/xwayland/xwayland.c         | 12 ++++++------
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index cf8395f1d..98c7fd207 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -165,9 +165,9 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat)
>                            xwl_seat->x_cursor->bits->yhot);
>      wl_surface_attach(xwl_cursor->surface,
>                        xwl_shm_pixmap_get_wl_buffer(pixmap), 0, 0);
> -    wl_surface_damage(xwl_cursor->surface, 0, 0,
> -                      xwl_seat->x_cursor->bits->width,
> -                      xwl_seat->x_cursor->bits->height);
> +    wl_surface_damage_buffer(xwl_cursor->surface, 0, 0,
> +                             xwl_seat->x_cursor->bits->width,
> +                             xwl_seat->x_cursor->bits->height);
>  
>      xwl_cursor->frame_cb = wl_surface_frame(xwl_cursor->surface);
>      wl_callback_add_listener(xwl_cursor->frame_cb, &frame_listener, xwl_cursor);
> @@ -215,9 +215,9 @@ xwl_tablet_tool_set_cursor(struct xwl_tablet_tool *xwl_tablet_tool)
>                                    xwl_seat->x_cursor->bits->yhot);
>      wl_surface_attach(xwl_cursor->surface,
>                        xwl_shm_pixmap_get_wl_buffer(pixmap), 0, 0);
> -    wl_surface_damage(xwl_cursor->surface, 0, 0,
> -                      xwl_seat->x_cursor->bits->width,
> -                      xwl_seat->x_cursor->bits->height);
> +    wl_surface_damage_buffer(xwl_cursor->surface, 0, 0,
> +                             xwl_seat->x_cursor->bits->width,
> +                             xwl_seat->x_cursor->bits->height);
>  
>      xwl_cursor->frame_cb = wl_surface_frame(xwl_cursor->surface);
>      wl_callback_add_listener(xwl_cursor->frame_cb, &frame_listener, xwl_cursor);
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index 792eaa69c..afd9e67a9 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -521,9 +521,9 @@ xwl_present_flip(WindowPtr present_window,
>                                   xwl_present_window);
>      }
>  
> -    wl_surface_damage(xwl_window->surface, 0, 0,
> -                      damage_box->x2 - damage_box->x1,
> -                      damage_box->y2 - damage_box->y1);
> +    wl_surface_damage_buffer(xwl_window->surface, 0, 0,
> +                             damage_box->x2 - damage_box->x1,
> +                             damage_box->y2 - damage_box->y1);
>  
>      wl_surface_commit(xwl_window->surface);
>  
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 605c9f56b..76211ef4a 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -684,13 +684,13 @@ xwl_window_post_damage(struct xwl_window *xwl_window)
>       */
>      if (RegionNumRects(region) > 256) {
>          box = RegionExtents(region);
> -        wl_surface_damage(xwl_window->surface, box->x1, box->y1,
> -                          box->x2 - box->x1, box->y2 - box->y1);
> +        wl_surface_damage_buffer(xwl_window->surface, box->x1, box->y1,
> +                                 box->x2 - box->x1, box->y2 - box->y1);
>      } else {
>          box = RegionRects(region);
>          for (i = 0; i < RegionNumRects(region); i++, box++)
> -            wl_surface_damage(xwl_window->surface, box->x1, box->y1,
> -                              box->x2 - box->x1, box->y2 - box->y1);
> +            wl_surface_damage_buffer(xwl_window->surface, box->x1, box->y1,
> +                                     box->x2 - box->x1, box->y2 - box->y1);
>      }

Hi,

you need to make this conditional to the wl_surface version, and use
the old calls otherwise.

>  
>      xwl_window->frame_callback = wl_surface_frame(xwl_window->surface);
> @@ -737,9 +737,9 @@ registry_global(void *data, struct wl_registry *registry, uint32_t id,
>  {
>      struct xwl_screen *xwl_screen = data;
>  
> -    if (strcmp(interface, "wl_compositor") == 0) {
> +    if (strcmp(interface, "wl_compositor") == 0 && version >= 4) {
>          xwl_screen->compositor =
> -            wl_registry_bind(registry, id, &wl_compositor_interface, 1);
> +            wl_registry_bind(registry, id, &wl_compositor_interface, 4);
>      }
>      else if (strcmp(interface, "wl_shm") == 0) {
>          xwl_screen->shm = wl_registry_bind(registry, id, &wl_shm_interface, 1);

I don't think it's a good idea to break Xwayland completely on
compositors that don't implement wl_surface version 4 or greater.

It would make sense to bind wl_compositor min(version, 4).


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20181105/4ab99026/attachment.sig>