[PATCH xserver] xwayland: use wl_surface_damage_buffer

[Simon Ser]

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 <***@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);
}

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);

[Pekka Paalanen]

On Mon, 05 Nov 2018 11:58:26 +0000

Post by Simon Ser
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.
---
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.

Post by Simon Ser
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

[Simon Ser]

Post by Pekka Paalanen
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).

This is intentional. I think it's reasonable to expect wl_compositor v4, it's
pretty old (3 years old). For instance, compositors not implementing v4 have no
damage tracking with EGL.

I think it's worth it to require v4 rather than cluttering the codebase with
version checks.
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xor

[Adam Jackson]

Post by Simon Ser

Post by Pekka Paalanen
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).

This is intentional. I think it's reasonable to expect wl_compositor v4, it's
pretty old (3 years old). For instance, compositors not implementing v4 have no
damage tracking with EGL.
I think it's worth it to require v4 rather than cluttering the codebase with
version checks.

I tend to agree. weston, mutter, and wlroots seem to all support it.
wlc doesn't but it's been deprecated. I don't know how to look this up
for KDE easily short of installing and running it; if someone could
verify that it implements v4 then I'd be happy to see this merged.

- ajax

_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/

[Roman Gilg]

Post by Simon Ser

Post by Pekka Paalanen
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).

This is intentional. I think it's reasonable to expect wl_compositor v4, it's
pretty old (3 years old). For instance, compositors not implementing v4 have no
damage tracking with EGL.

In KWin we still only support v3, but I currently have a patch up for
review to support v4: https://phabricator.kde.org/D15910. Until the
next Xserver is released, a version of KWin with this support should
have been released and systems with older KWin versions wouldn't use
the latest Xserver version. So from KWin's side I think it shouldn't
be a problem.

Of course there are other compositors, but one would assume that a
compositor with Xwayland support on latest Xserver is able to speak v4
or should be made ready to do this quickly.
_______________________________________________
xorg-***@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.