[RFC wayland] protocol: Add wl_surface.buffer_damage

[Daniel Stone]

Hi,

On 7 November 2015 at 17:49, Jasper St. Pierre <jstpierre at mecheye.net> wrote:
> OK. So the bug *wasn't* fixed two years ago like you were saying, and
> under transformations, mesa is still wrong.

It was fixed two years ago for clients using plain eglSwapBuffers,
which I posit is functionally equivalent to all of them. Games don't
use SwapBuffersWithDamage, nothing in GNOME uses it (well, Clutter has
support, but when was the last time you saw a partial redraw in a
Clutter scene ... ?), Chrome doesn't use it, etc.

There are a lot of other bugs in Mesa, too; which other parts of a
declared-stable protocol which are necessary for a reasonably
performant client do you want to throw overboard?

> So we fix the bug by adding wl_surface.buffer_damage, since it's
> impossible for Mesa to know about buffer transformations... and then
> what?

Mesa starts using it. People with the combination of old Mesa and
actually using SwapBuffersWithDamage encounter bugs. People encounter
bugs with Mesa all the time; either they upgrade, or they get the fix
backported, or they live with it. Life goes on.

> The only way we can properly interpret the wl_surface.damage event is
> it being correct under no buffer transformations. Which is fine to me.
> And in any other case, we effectively have to treat it as a
> full-surface damage, since the coordinates are junk.

Sure, do that in Mutter if you want: it'll make prerotated software
clients perhaps unusably slow. To which the answer is, 'oh yeah,
that's a bug which we fixed, just upgrade your GTK+'. Sounds oddly
like 'that's a bug in Mesa, just upgrade Mesa'.

> And technically, it doesn't matter which of the two you pick -- it's
> an optimization for the compositor to redraw less. At that point, I
> think it makes more sense to say that we should just treat every
> wl_surface.damage event as redrawing the full surface.

The tradeoff is between clients using the interface incorrectly (only
people using Mesa and Mali-4xx's eglSwapBuffersWithDamage; the latter
of which is not officially supported by the vendor), and clients who
have done it correctly.

> Given how surprising and unintuitive the rectangles behavior is (and
> given how it only says it in passing in the spec), I personally don't
> believe many 2D clients went through the trouble to make sure their
> rectangles were correct in all cases. Given that the error case is
> "app gets tearing and then when you click on it and switches away from
> the backdrop theme, it works fine", I think many people probably just
> blamed cairo and got on with it. Especially when it remained wrong in
> Weston's own clients for a few years as well until somebody spotted
> it.
>
> Yes, this is what happens wheny ou introduce a broken protocol and let
> it stay there in the name of stability -- you get nonsense results and
> have to throw it out.

It's not broken. It's insufficient to cover some cases (EGL), it's in
some ways unintuitive, but it does provably work: there is a way to
handle it. buffer_damage isn't necessarily any more intuitive, since
you have a disconnect between input and output events. Yes, that does
make more sense when you sit down and think about it, but people will
still get it wrong that way. So what do we do when someone
accidentally writes a broken client then? Throw everyone who's ported
to buffer_damage under the bus and give up on the idea of damage
forever?

It's not ideal, but throwing those who went to the trouble of getting
it right under the bus, in favour of pessimising towards a hell of a
corner case caused by one buggy client, isn't to my mind responsible
stewardship of stable protocol.

Introduce buffer_damage to help EGL, modify the surface damage
documentation to nudge people towards buffer_damage, log something to
the journal when you detect that a client seems to be pushing damage
in the wrong co-ordinate space (via whichever interface); all of these
things are helpful. Binning critical (draw a large enough UI on a
constrained device and damage becomes critical rather than useful)
functionality because of one bug once, isn't helpful. Do it in Mutter
if you feel so passionately about it, but I'll NAK anything which does
this for Weston.

Cheers,
Daniel

> On Sat, Nov 7, 2015 at 9:41 AM, Daniel Stone <daniel at fooishbar.org> wrote:
>> Hi,
>>
>> On 7 November 2015 at 16:59, Jasper St. Pierre <jstpierre at mecheye.net> wrote:
>>> I don't see where. I see eglSwapBuffersWithDamage still looking like
>>> it shoves the rects across the wire in buffer space, without any
>>> modification.
>>>
>>> http://cgit.freedesktop.org/mesa/mesa/tree/src/egl/drivers/dri2/platform_wayland.c#n705
>>
>> A bug which should be fixed ...
>>
>>> I checked the Mali driver implementation I have, and it does something similar.
>>
>> Mali 6xx doesn't do that.
>>
>> Cheers,
>> Daniel
>>
>>> On Sat, Nov 7, 2015 at 2:32 AM, Daniel Stone <daniel at fooishbar.org> wrote:
>>>> On 6 November 2015 at 19:08, Jasper St. Pierre <jstpierre at mecheye.net> wrote:
>>>>> To help clear things up, I think we should deprecate the
>>>>> wl_surface.damage request and document that the coordinates are
>>>>> effectively undefined -- for legacy reasons, if you see
>>>>> wl_surface.damage, it should be considered a damage for the entire
>>>>> surface.
>>>>
>>>> Why? Mesa fixed that bug two years ago, I don't know of a non-Mesa EGL
>>>> implementation with that bug, and non-EGL clients are perfectly
>>>> capable of implementing it correctly themselves. Whilst introducing
>>>> buffer_damage, we could obliterate surface damage out of spite, but
>>>> that seems like a giant middle finger to everyone who wrote conformant
>>>> and performant apps, and now gets their entire buffer smashed through
>>>> TexImage2D every time they want to blink a cursor.
>>>>
>>>> Cheers,
>>>> Daniel
>>>
>>>
>>>
>>> --
>>>   Jasper
>
>
>
> --
>   Jasper