simple-damage: fix --rotating-transform
When the flag --rotating-transform is set, the client changes the whole content of the buffer. Still, only the area with the ball get damaged, leading to heavy glitches.
Fixes #155
Merge request reports
Activity
mentioned in issue #155
added Clients label
added 14 commits
-
3b5878fb...1f3fae2f - 13 commits from branch
wayland:master - c7e23a3a - simple-damage: fix --rotating-transform
-
3b5878fb...1f3fae2f - 13 commits from branch
@derekf I remember you having something to do with this at some point ... does it ring any bells?
Ugh, yes. I tried to fix this on the compositor side years ago, but there was some question as to whether the fix actually belonged there.
This example has been broken forever, and the proposed fix renders correctly, however it is not "perfect" - it posts damage for a much larger area than it needs to, and as such isn't necessarily a good "reference" for how a client should handle this...
I don't mind so much the extra damage in the non damage_buffer case - and wonder if by now we can simply remove the non damage_buffer code from all weston demo clients entirely? :)
But I think for the damage_buffer path we need our damage region to exactly match the minimal area possible, which I think means re-transforming the previous ball location to where it would have been in the current frame of reference.
This way someone using this code as a reference (as I feel all code in the weston repo should be) will see the minimal possible update.
comments would be great too. :)
I have a gut feeling that doing the perfect update might expose problems in texture map upload from shm to gl, which is another reason to aim for perfection here...
But I think for the damage_buffer path we need our damage region to exactly match the minimal area possible, which I think means re-transforming the previous ball location to where it would have been in the current frame of reference.
Strictly speaking that's not enough, as the whole buffer is transformed. In the normal case, that's not visible and your proposal is visual equal, but if
--use-viewportis set, the red part of the texture creeps in. So from a technical perspective, I'd say this solution is more 'correct'.I don't mind so much the extra damage in the non damage_buffer case - and wonder if by now we can simply remove the non damage_buffer code from all weston demo clients entirely? :)
Does surface-damage get deprecated? Because as long as the protocol requires it, why would you remove it?
By the way, I found this issue while working on a mutter implementation on viewports and transforms: https://gitlab.gnome.org/GNOME/mutter/merge_requests/121
Buffer-damage is preferred over surface-damage. Maybe we should update the protocol to deprecate surface-damage? Having two ways to set damage is a little annoying.
Totally agree that buffer damage is the better option and having to care for surface damage is quite annoying (took me a while to get it right with viewports in mutter). But afaik even XWayland uses surface damage atm, so there would need to be a longer period of deprecation. And as long as the it's a valid protocol option, shouldn't a demo client support it?
Any way, this MR is probably not the right place to discuss it :)
Edited by Robert Mader
If I understand this MR correctly, NAK.
The whole point of the test app is that it uses the true minimal damage region while changing all the parameters that affect how damage must be transformed. Therefore making it use full surface damage when that is not actually what is visually changing makes the whole test moot.
I believe the same goes for buffer damage. This client must mark the area of the buffer, that will be different when mapped to the surface coordinates. If a compositor maintains a copy of the buffer and implements partial damage support with changing buffer transform, the compositor will read only the damage region of the provided buffer, transform it according to the internal copy of the buffer it maintains and uploads the transformed contents into the copy. If a compositor does not implement partial damage support when buffer transform changes, it will just replace the whole copy it maintains with the new content.
Note, that even if the compositor does not implement partial damage support while buffer transform changes, it may still use the damage region to optimize composition. Therefore the accurate buffer damage must not be replaced with full buffer damage either, as that would make the composition testing moot.
mentioned in merge request !51 (closed)
mentioned in merge request !222 (closed)
