<div dir="ltr"><div>I believe wl_surface.damage should be defined as "the same as w_surface.buffer_damage if the only transforms are due to an integer buffer scale and the 8 possible buffer transforms". This means it is undefined if the wl_scalar proposal is used, and avoids the need to define it for any future transform possibilities. </div><div>For very similar reasons I believe the wl_scalar and the subsurface apis will need to be changed to be in buffer coordinates. So a defined suffix (such as _buffer or _pixels) is probably a good idea so these can be fixed in a similar way. </div></div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Nov 9, 2015 at 7:06 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, 6 Nov 2015 12:55:19 -0600 Derek Foreman <<a href="mailto:derekf@osg.samsung.com">derekf@osg.samsung.com</a>> wrote: > wl_surface.damage uses surface local co-ordinates. > > Buffer scale and buffer transforms came along, and EGL surfaces > have no understanding of them. > > Theoretically, clients pass damage rectangles - in Y-inverted surface > co-ordinates) to EGLSwapBuffersWithDamage, and the EGL implementation > passed them on to wayland. However, for this to work the EGL > implementation must be able to flip those rectangles into the space > the compositor is expecting, but it's unable to do so because it > doesn't know the height of the transformed buffer. > > So, currently, EGLSwapBuffersWithDamage is unusable and EGLSwapBuffers > has to pass (0,0) - (INT32_MAX, INT32_MAX) damage to function. > > wl_surface.buffer_damage allows damage to be registered on a surface > in buffer co-ordinates, avoiding this problem. > > Credit where it's due, these ideas are not entirely my own: > Over a year ago the idea of changing damage co-ordinates to buffer > co-ordinates was suggested (by Jason Ekstrand), and it was at least > partially rejected and abandoned. At the time it was also suggested > (by Pekka Paalanen) that adding a new wl_surface.buffer_damage request > was another option. > Hi Derek, please mention <a href="https://bugs.freedesktop.org/show_bug.cgi?id=78190" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=78190</a> in this patch. > Signed-off-by: Derek Foreman <<a href="mailto:derekf@osg.samsung.com">derekf@osg.samsung.com</a>> > --- > > Necro-posting on: > <a href="http://lists.freedesktop.org/archives/wayland-devel/2014-February/013440.html" rel="noreferrer" target="_blank">http://lists.freedesktop.org/archives/wayland-devel/2014-February/013440.html</a> > and > <a href="http://lists.freedesktop.org/archives/wayland-devel/2014-February/013442.html" rel="noreferrer" target="_blank">http://lists.freedesktop.org/archives/wayland-devel/2014-February/013442.html</a> > > This came up on IRC again the other day, and it's still an unsolved problem... > I'm posting this as RFC to see if anyone's interested in it - I'll do an > implementation if we can get an agreement on the protocol text. Thanks for picking this up! > protocol/wayland.xml | 44 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index 9c22d45..1cb2f66 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -176,7 +176,7 @@ > </event> > </interface> > > - <interface name="wl_compositor" version="3"> > + <interface name="wl_compositor" version="4"> > <description summary="the compositor singleton"> > A compositor. This object is a singleton global. The > compositor is in charge of combining the contents of multiple > @@ -986,7 +986,7 @@ > </event> > </interface> > > - <interface name="wl_surface" version="3"> > + <interface name="wl_surface" version="4"> > <description summary="an onscreen surface"> > A surface is a rectangular area that is displayed on the screen. > It has a location, size and pixel contents. > @@ -1327,6 +1327,46 @@ > </description> > <arg name="scale" type="int"/> > </request> I know Jasper suggested deprecating wl_surface.damage, but I see no reason for that. wl_surface.damage is well-defined - it is only misused. We could add some wording to have both refer to each other as an alternative way to post damage. Especially to wl_surface.damage should mention buffer_damage as doing what you'd usually expect. > + > +  > + <request name="buffer_damage" since="4"> The name "buffer_damage" is slightly unfortunate. See: <a href="https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_swap_buffers_with_damage.txt" rel="noreferrer" target="_blank">https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_swap_buffers_with_damage.txt</a> What we are doing in Wayland is always "surface damage"[*], while that EGL extension reserves "buffer damage" for a different purpose. I feel it might be confusing. Could we come up with a another name for this request? - wl_surface.damage_pixels - wl_surface.damage_by_buffer eh... buffer_damage is fine if nothing else sticks. The specification language below is clear anyway, IMO. > + <description summary="mark part of the surface damaged using buffer co-ordinates"> > + This request is used to describe the regions where the pending > + buffer is different from the current surface contents, and where > + the surface therefore needs to be repainted. The pending buffer > + must be set by wl_surface.attach before sending damage. The > + compositor ignores the parts of the damage that fall outside of > + the surface. > + > + Damage is double-buffered state, see wl_surface.commit. > + > + The damage rectangle is specified in buffer coordinates. > + > + The initial value for pending damage is empty: no damage. > + wl_surface.damage adds pending damage: the new pending damage > + is the union of old pending damage and the given rectangle. > + > + wl_surface.commit assigns pending damage as the current damage, > + and clears pending damage. The server will clear the current > + damage as it repaints the surface. Essentially a copy from wl_surface.damage without changing anything but the coordinate system. Ok. > + > + This request differs from wl_surface.damage in only one way - it > + takes damage in buffer co-ordinates instead of surface local > + co-ordinates. This is desirable because EGL implementations > + are unaware of buffer scale and buffer transform and can only > + provide damage in buffer co-ordinates. Damage in buffer > + co-ordinates is required for EGLSwapBuffersWithDamage to work > + efficiently. Not sure if explaining the EGL side is needed here. Besides EGL, it could be any drawing library, and with wl_viewport there are much more use cases where buffer_damage is preferable. > + Mixing wl_surface.buffer_damage and wl_surface.damage requests > + on the same surface will result in undefined behaviour. Why undefined? The compositor will always transform between surface and buffer coordinate systems: surface to buffer for texture updates, and buffer to surface for repaint damage. I suppose you can avoid accumulating two different regions depending on the coordinate space until wl_surface.commit by saying only one coordinate space is guaranteed to work at a time. Is that reason enough, or the only reason? > + </description> > + > + <arg name="x" type="int"/> > + <arg name="y" type="int"/> > + <arg name="width" type="int"/> > + <arg name="height" type="int"/> > + </request> > </interface> > > <interface name="wl_seat" version="5"> Thanks, pq [*] There is an off-topic rabbit hole to be dug here, if we would allow the compositor to cache shm-based textures... ;-) _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> </div>