[PATCH] protocol: deprecate wl

Discussion:

[PATCH] protocol: deprecate wl_surface.damage

Simon Ser

2018-11-04 08:38:16 UTC

This commit deprecates wl_surface.damage in favor of wl_surface.damage_buffer.

Having two requests makes it complicated for the compositor to handle damage,
making it necessary to transform one into the other's coordinates.

Moreover, integration with wp_viewporter is tricky.

Signed-off-by: Simon Ser <***@emersion.fr>
---

Based on the discussion in [1].

[1]: https://gitlab.freedesktop.org/wayland/weston/merge_requests/41#note_74071

protocol/wayland.xml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..c75ea3a 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1397,9 +1397,9 @@
and clears pending damage. The server will clear the current
damage as it repaints the surface.

- Alternatively, damage can be posted with wl_surface.damage_buffer
- which uses buffer coordinates instead of surface coordinates,
- and is probably the preferred and intuitive way of doing this.
+ Note! This request is deprecated and should not be used in new
+ clients. Instead damage can be posted with wl_surface.damage_buffer
+ which uses buffer coordinates instead of surface coordinates.
</description>
<arg name="x" type="int" summary="surface-local x coordinate"/>
<arg name="y" type="int" summary="surface-local y coordinate"/>

--
2.19.1

Derek Foreman

2018-11-04 15:23:29 UTC

Permalink

IMHO this is a win.

Reviewed-by: Derek Foreman <***@gmail.com>

I think we can follow it up by removing surface damage in surface
co-ordinates from weston clients - if we don't want people using it, we
shouldn't be giving then good examples of how.

I also think it's reasonable for compositor writers to take the easy way
out of handling legacy surface damage by just treating any
wl_surface.damage request as full surface damage...

Thanks,
Derek

Post by Simon Ser
This commit deprecates wl_surface.damage in favor of wl_surface.damage_buffer.
Having two requests makes it complicated for the compositor to handle damage,
making it necessary to transform one into the other's coordinates.
Moreover, integration with wp_viewporter is tricky.
---
Based on the discussion in [1].
[1]: https://gitlab.freedesktop.org/wayland/weston/merge_requests/41#note_74071
protocol/wayland.xml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..c75ea3a 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1397,9 +1397,9 @@
and clears pending damage. The server will clear the current
damage as it repaints the surface.
- Alternatively, damage can be posted with wl_surface.damage_buffer
- which uses buffer coordinates instead of surface coordinates,
- and is probably the preferred and intuitive way of doing this.
+ Note! This request is deprecated and should not be used in new
+ clients. Instead damage can be posted with wl_surface.damage_buffer
+ which uses buffer coordinates instead of surface coordinates.
</description>
<arg name="x" type="int" summary="surface-local x coordinate"/>
<arg name="y" type="int" summary="surface-local y coordinate"/>

Pekka Paalanen

2018-11-05 08:44:44 UTC

Permalink

On Sun, 04 Nov 2018 08:38:16 +0000

Hi,

what good does this do, when no compositor can ever stop implementing it?

I'm ok with adding a note that clients might have easier time using
damage_buffer, but I don't see anything that would allow compositors to
do otherwise. Err, well, that note is what you are replacing here.

Whatever the compositor, it will always have to translate from one
space to the other space, regardless of which request a client is
using. You need the buffer space damage to be able to update textures
(wl_shm path), and you need the surface space damage to repaint the
screen (for framebuffer damage).

Thanks,
pq

Simon Ser

2018-11-05 08:58:46 UTC

Permalink

Post by Pekka Paalanen
Hi,
what good does this do, when no compositor can ever stop implementing it?
I'm ok with adding a note that clients might have easier time using
damage_buffer, but I don't see anything that would allow compositors to
do otherwise. Err, well, that note is what you are replacing here.
Whatever the compositor, it will always have to translate from one
space to the other space, regardless of which request a client is
using. You need the buffer space damage to be able to update textures
(wl_shm path), and you need the surface space damage to repaint the
screen (for framebuffer damage).

As you said, the compositor needs to do two things with damage: (1) update
textures (buffer coords) and (2) damage outputs (surface coords).

If we only get buffer damage from clients, then (1) is easy and (2) requires a
single region transformation.

If we get surface damage and/or buffer damage from clients, then both (1) and
(2) require region transformations.

As said in the commit message, surface damage interactions with wp_viewporter
are hard to understand.

In the end, I still think having one code path for damage submission is a net
win. I don't see why clients would need to use surface damage instead of buffer
damage.

Pekka Paalanen

2018-11-05 10:07:36 UTC

Permalink

On Mon, 05 Nov 2018 08:58:46 +0000

Post by Simon Ser

As you said, the compositor needs to do two things with damage: (1) update
textures (buffer coords) and (2) damage outputs (surface coords).
If we only get buffer damage from clients, then (1) is easy and (2) requires a
single region transformation.
If we get surface damage and/or buffer damage from clients, then both (1) and
(2) require region transformations.
As said in the commit message, surface damage interactions with wp_viewporter
are hard to understand.
In the end, I still think having one code path for damage submission is a net
win. I don't see why clients would need to use surface damage instead of buffer
damage.

How about writing what Derek said: that the old damage request may be
unoptimal rather than deprecated.

I don't like "deprecated" because to me it implies that this request
will be removed (i.e. can be left unimplemented) some time in the future.

Xwayland is still using exclusively wl_surface.damage, for instance.

Thanks,
pq

Simon Ser

2018-11-05 13:43:16 UTC

Permalink

Post by Pekka Paalanen
How about writing what Derek said: that the old damage request may be
unoptimal rather than deprecated.
I don't like "deprecated" because to me it implies that this request
will be removed (i.e. can be left unimplemented) some time in the future.

Fair enough, I'll rephrase with words like "should" and "preferred".

It would be nice to really deprecate this request at some point.

Post by Pekka Paalanen
Xwayland is still using exclusively wl_surface.damage, for instance.

Sent a patch to fix this [1].

[1]: https://lists.x.org/archives/xorg-devel/2018-November/057678.html

Derek Foreman

2018-11-05 19:52:24 UTC

Permalink

Post by Simon Ser

Fair enough, I'll rephrase with words like "should" and "preferred".
It would be nice to really deprecate this request at some point.

Have we ever considered having a way to deprecate requests (beyond
deprecating entire xml files like the old xdg versions)?

We could generate compile time warnings and perhaps print a log message.

Actually removing protocol might be kind of scary, but we can give
people some way to know that they might get better performance with some
changes.

Or is this so infrequent an occurrence that it doesn't need to be
bothered with?

Thanks,
Derek

Post by Simon Ser

Post by Pekka Paalanen
Xwayland is still using exclusively wl_surface.damage, for instance.

Sent a patch to fix this [1].
[1]: https://lists.x.org/archives/xorg-devel/2018-November/057678.html

Pekka Paalanen

2018-11-06 08:48:07 UTC

Permalink

On Mon, 5 Nov 2018 13:52:24 -0600

Post by Derek Foreman

Post by Simon Ser

Fair enough, I'll rephrase with words like "should" and "preferred".
It would be nice to really deprecate this request at some point.

Have we ever considered having a way to deprecate requests (beyond
deprecating entire xml files like the old xdg versions)?
We could generate compile time warnings and perhaps print a log message.

Compile time warnings don't work with apps that will never be rebuilt
again (proprietary apps).

Log messages are compositor-specific and will get ignored, unless they
actually feed into a system that correctly identifies the application
and automatically submits a bug report at least somewhere.

You could define a deprecation XML attribute that will have the code
generator produce log messages, but where would those messages go?

Therefore I have no hope of ever actually stopping implementing a
request for something as fundamental as e.g. wl_surface.

For all the same reasons, deprecating "entire xml files", or a whole
tree of protocol object types, is risky, because there can be apps that
cannot be updated to not require it. This is one reason why the
wayland-protocols stabilization process takes so long.

Post by Derek Foreman
Actually removing protocol might be kind of scary, but we can give
people some way to know that they might get better performance with some
changes.

This would be possible given we assume that applications still have
active maintenance.

Post by Derek Foreman
Or is this so infrequent an occurrence that it doesn't need to be
bothered with?

A good plan would be nice, but I have no idea what it might be. Right
now the situation is the same as with library ABI stability rules,
except we do not have the option to keep an ancient library for an
ancient app, since this is about protocol. A middle-man Wayland proxy
daemon could probably convert between deprecated and current stuff.
That's obviously a very heavy solution, not much short from a
full-blown compositor.

In my opinion, ignoring the existence of binary-only applications would
be short-sighted.

Thanks,
pq