[PATCH 2/2] protocol: Support scaled outputs and surfaces
Kristian Høgsberg
hoegsberg at gmail.com
Wed May 22 18:49:25 PDT 2013
On Mon, May 20, 2013 at 10:49:27AM +0300, Pekka Paalanen wrote:
> Hi Alexander,
>
> nice to see this going forward, and sorry for replying so rarely and
> late.
>
> On Thu, 16 May 2013 15:49:36 +0200
> alexl at redhat.com wrote:
>
> > From: Alexander Larsson <alexl at redhat.com>
> >
> > This adds the wl_surface.set_buffer_scale request, and a wl_output.scale
> > event. These together lets us support automatic upscaling of "old"
> > clients on very high resolution monitors, while allowing "new" clients
> > to take advantage of this to render at the higher resolution when the
> > surface is displayed on the scaled output.
> >
> > It is similar to set_buffer_transform in that the buffer is stored in
> > a transformed pixels (in this case scaled). This means that if an output
> > is scaled we can directly use the pre-scaled buffer with additional data,
> > rather than having to scale it.
> >
> > Additionally this adds a "scaled" flag to the wl_output.mode flags
> > so that clients know which resolutions are native and which are scaled.
> >
> > Also, in places where the documentation was previously not clear as to
> > what coordinate system was used this was fleshed out.
> >
> > It also adds a scaling_factor event to wl_output that specifies the
> > scaling of an output.
> >
> > This is meant to be used for outputs with a very high DPI to tell the
> > client that this particular output has subpixel precision. Coordinates
> > in other parts of the protocol, like input events, relative window
> > positioning and output positioning are still in the compositor space
>
> We don't have a single "compositor space".
>
> This needs some way of explaining that surface coordinates are always
> the same, regardless of the attached buffer's scale. That is, surface
> coordinates always correspond to the size of a buffer with scale 1.
>
> > rather than the scaled space. However, input has subpixel precision
> > so you can still get input at full resolution.
> >
> > This setup means global properties like mouse acceleration/speed,
> > pointer size, monitor geometry, etc can be specified in a "mostly
> > similar" resolution even on a multimonitor setup where some monitors
> > are low dpi and some are e.g. retina-class outputs.
> > ---
> > protocol/wayland.xml | 107 ++++++++++++++++++++++++++++++++++++++++++++-------
> > 1 file changed, 93 insertions(+), 14 deletions(-)
> >
> > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> > index d3ae149..acfb140 100644
> > --- a/protocol/wayland.xml
> > +++ b/protocol/wayland.xml
> > @@ -173,7 +173,7 @@
> > </event>
> > </interface>
> >
> > - <interface name="wl_compositor" version="2">
> > + <interface name="wl_compositor" version="3">
>
> Ok.
>
> > <description summary="the compositor singleton">
> > A compositor. This object is a singleton global. The
> > compositor is in charge of combining the contents of multiple
> > @@ -709,7 +709,7 @@
> >
> > The x and y arguments specify the locations of the upper left
> > corner of the surface relative to the upper left corner of the
> > - parent surface.
> > + parent surface, in surface local coordinates.
> >
> > The flags argument controls details of the transient behaviour.
> > </description>
> > @@ -777,6 +777,10 @@
> > in any of the clients surfaces is reported as normal, however,
> > clicks in other clients surfaces will be discarded and trigger
> > the callback.
> > +
> > + The x and y arguments specify the locations of the upper left
> > + corner of the surface relative to the upper left corner of the
> > + parent surface, in surface local coordinates.
>
> Surface local coordinates are defined to have their origin in the
> surface top-left corner. If that is defined once and for all, you don't
> have to repeat "relative to upper left..." everywhere.
>
> Surface local coordinates relative to anything else do not exist.
>
> When these were originally written in the spec, the term surface
> coordinates had not settled yet.
>
> > </description>
> >
> > <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat whose pointer is used"/>
> > @@ -860,6 +864,9 @@
> >
> > The client is free to dismiss all but the last configure
> > event it received.
> > +
> > + The width and height arguments specify the size of the window
> > + in surface local coordinates.
>
> Yes, "window" is definitely the correct term here. Saying "surface"
> would be incorrect, due to sub-surfaces, if I recall my discussion with
> Giulio Camuffo right.
I think we need to introduce the "window geometry" concept before this
gets out of hand. I'm already a little uncomfortable with the idea of
including sub-surfaces in the surface bounding box - it seems like
something the client should control. So the suggestion is to add a
new wl_shell_surface request:
<request name="set_geometry">
<description summary="set logical surface geometry">
This request sets the the logical geometry of the surface.
The logical geometry is the rectangle that compositor uses for
window management placement decisions. For example, a buffer
often contains drop shadow for a surface or other content that
shouldn't be considered part of the surface geometry. By
setting the surface geometry, the client can communicate to
the compositor the sub-rectangle of the surface that it
considers the window geometry. The compositor will the use
this rectangle for initial placement, snapping and other
placement decisions. If the client has provided a geometry
for a surface, the new size suggested by the configure event
will refer to the window geometry.
The geometry is a rectangle in surface local coordinates.
Geometry is double-buffered state, see wl_surface.commit.
wl_shell_surface.set_geometry changes the pending geometry.
wl_surface.commit copies the pending geometry to the current
geometry. Otherwise, the pending and current geometry are
never changed.
Initially the geometry will track the bounding box of the
surface but once the geometry is set, it will only change when
the geomtry is set again.
</description>
<arg name="x" type="int"/>
<arg name="y" type="int"/>
<arg name="width" type="int"/>
<arg name="height" type="int"/>
</request>
> > </description>
> >
> > <arg name="edges" type="uint"/>
> > @@ -876,11 +883,16 @@
> > </event>
> > </interface>
> >
> > - <interface name="wl_surface" version="2">
> > + <interface name="wl_surface" version="3">
> > <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.
> >
> > + The size of a surface (and relative positions on it) is described
>
> "The size of the surface and positions on the surface are described..."?
>
> > + in surface local coordinates, which may differ from the buffer
> > + local coordinates of the pixel content, in case a buffer_transform
> > + or a buffer_scale is used.
>
> I think we could additionally define here, that surface local
> coordinates always correspond to the case buffer_scale=1. Would that
> define it once and for all?
>
> > +
> > Surfaces are also used for some special purposes, e.g. as
> > cursor images for pointers, drag icons, etc.
> > </description>
> > @@ -895,20 +907,25 @@
> > <description summary="set the surface contents">
> > Set a buffer as the content of this surface.
> >
> > + The new size of the surface is calculated based on the buffer
> > + size transformed by the inverse buffer_transform and the
> > + inverse buffer_scale. This means that the supplied buffer
> > + must be an integer multiple of the buffer_scale.
> > +
> > The x and y arguments specify the location of the new pending
> > - buffer's upper left corner, relative to the current buffer's
> > - upper left corner. In other words, the x and y, and the width
> > - and height of the wl_buffer together define in which directions
> > - the surface's size changes.
> > + buffer's upper left corner, relative to the current buffer's upper
> > + left corner, in surface local coordinates. In other words, the
> > + x and y, combined with the new surface size define in which
> > + directions the surface's size changes.
>
> In this paragraph, the current definition (program state) of surface
> coordinates is being changed: the origin is moved when this action is
> applied. Therefore I originally did not mention surface coordinates
> here, since we need to specify in which surface coordinates x,y are in,
> old or new.
>
> > Surface contents are double-buffered state, see wl_surface.commit.
> >
> > The initial surface contents are void; there is no content.
> > wl_surface.attach assigns the given wl_buffer as the pending
> > wl_buffer. wl_surface.commit makes the pending wl_buffer the new
> > - surface contents, and the size of the surface becomes the size of
> > - the wl_buffer, as described above. After commit, there is no
> > - pending buffer until the next attach.
> > + surface contents, and the size of the surface becomes the size
> > + calculated from the wl_buffer, as described above. After commit,
> > + there is no pending buffer until the next attach.
>
> Right.
>
> > Committing a pending wl_buffer allows the compositor to read the
> > pixels in the wl_buffer. The compositor may access the pixels at
> > @@ -945,6 +962,8 @@
> >
> > Damage is double-buffered state, see wl_surface.commit.
> >
> > + The damage rectangle is specified in surface local 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.
> > @@ -996,6 +1015,8 @@
> > behaviour, but marking transparent content as opaque will result
> > in repaint artifacts.
> >
> > + The opaque region is specified in surface local coordinates.
> > +
> > The compositor ignores the parts of the opaque region that fall
> > outside of the surface.
> >
> > @@ -1023,6 +1044,8 @@
> > surface in the server surface stack. The compositor ignores the
> > parts of the input region that fall outside of the surface.
> >
> > + The input region is specified in surface local coordinates.
> > +
> > Input region is double-buffered state, see wl_surface.commit.
>
> Alright.
>
> > wl_surface.set_input_region changes the pending input region.
> > @@ -1100,7 +1123,7 @@
> > according to the output transform, thus permiting the compositor to
> > use certain optimizations even if the display is rotated. Using
> > hardware overlays and scanning out a client buffer for fullscreen
> > - surfaces are examples of such optmizations. Those optimizations are
> > + surfaces are examples of such optimizations. Those optimizations are
> > highly dependent on the compositor implementation, so the use of this
> > request should be considered on a case-by-case basis.
>
> Heh, nice catch.
>
>
> I think we should change the wl_surface.commit description, so that
> buffer_transform and buffer_scale are taken into account first, then
> apply the new wl_buffer, and then everything else. This is because the
> new wl_buffer defines the surface coordinate system, where everything
> else is then described in.
I don't think so. The buffer doesn't change or define the surface
coordinate system. If you attach the same buffer with a different
scale, or bigger or smaller buffer with the same scale, that just
changes the surface size. If you set an opaque region it doesn't
matter whether you do it before or after the new buffer or scale
factor is set, the region is what it is.
> Btw. what happens, if a client sets a new buffer_transform or
> buffer_scale, but does not attach a new buffer? Or rather, what should
> we say about that?
That's fine, it'll look weird.
> >
> > @@ -1110,6 +1133,30 @@
> > </description>
> > <arg name="transform" type="int"/>
> > </request>
> > +
> > + <!-- Version 3 additions -->
> > +
> > + <request name="set_buffer_scale" since="3">
> > + <description summary="sets the buffer scaling factor">
> > + This request sets an optional scaling factor on how the compositor
> > + interprets the contents of the buffer attached to the window.
> > +
> > + Buffer scale is double-buffered state, see wl_surface.commit.
> > +
> > + A newly created surface has its buffer scale set to 1.
> > +
> > + The purpose of this request is to allow clients to supply higher
> > + resolution buffer data for use on high resolution outputs. Its
> > + intended that you pick the same buffer scale as the scale of the
> > + output that the surface is displayed on.This means the compositor
> > + can avoid scaling when rendering the surface on that output.
> > +
> > + Note that if the scale is larger than 1, then you have to attach
> > + a buffer that is larger (by a factor of scale in each dimension)
> > + than the desired surface size.
> > + </description>
> > + <arg name="scale" type="uint"/>
> > + </request>
>
> Very good.
> What if a client sets scale=0?
>
> Maybe the scale should also be signed here? I think all sizes are
> signed, too, even though a negative size does not make sense. We seem
> to have a convention, that numbers you compute with are signed, and
> enums and flags and bitfields and handles and such are unsigned. And
> timestamps, since there we need the overflow behaviour. I
> believe it's due to the C promotion or implicit cast rules more than
> anything else.
Yes, the problem is that if you make x and y signed and width and
height unsigned, you end up with something like
if (x2 - x1 > width)
where if x2 - x1 is -10, that becomes 4294967285 when it gets coerced
to an unsigned int because we're comparing against unsigned int width.
So the types of variables shouldn't be chosen for the values that it
may take ("a width can never be negative") but rather to match what
other variables you'll be using it with. So for width or scale, even
though they'll never be negative, int is the right choice as we'll
mixing them with other signed types.
Cases for unsigned ints are the timestamps where we need the
well-defined unsigned overflow behavior, tokens/enums where we always
only compare identity, or bitmasks where we only use bit-wise
manipulations.
Kristian
> > </interface>
> >
> > <interface name="wl_seat" version="1">
> > @@ -1197,7 +1244,8 @@
> > The parameters hotspot_x and hotspot_y define the position of
> > the pointer surface relative to the pointer location. Its
> > top-left corner is always at (x, y) - (hotspot_x, hotspot_y),
> > - where (x, y) are the coordinates of the pointer location.
> > + where (x, y) are the coordinates of the pointer location, in surface
> > + local coordinates.
> >
> > On surface.attach requests to the pointer surface, hotspot_x
> > and hotspot_y are decremented by the x and y parameters
> > @@ -1548,6 +1596,8 @@
> > summary="indicates this is the current mode"/>
> > <entry name="preferred" value="0x2"
> > summary="indicates this is the preferred mode"/>
> > + <entry name="scaled" value="0x4"
> > + summary="indicates that this is a scaled mode"/>
>
> What do we need the "scaled" flag for? And what does this flag mean?
> How is it used? I mean, can we get duplicate native modes that differ
> only by the scaled flag?
>
> Unfortunately I didn't get to answer that thread before, but I had some
> disagreement or not understanding there.
>
> > </enum>
> >
> > <event name="mode">
> > @@ -1559,10 +1609,15 @@
> > again if an output changes mode, for the mode that is now
> > current. In other words, the current mode is always the last
> > mode that was received with the current flag set.
> > +
> > + The size of a mode is given relative to the global compositor
> > + space. This is not necessarily the native size of the display,
> > + as the output might be scaled, as described in wl_output.scale.
> > + In this case the scaled flag will be set.
> > </description>
> > <arg name="flags" type="uint" summary="bitfield of mode flags"/>
> > - <arg name="width" type="int" summary="width of the mode in pixels"/>
> > - <arg name="height" type="int" summary="height of the mode in pixels"/>
> > + <arg name="width" type="int" summary="width of the mode in global coordinates"/>
> > + <arg name="height" type="int" summary="height of the mode in global coordinates"/>
> > <arg name="refresh" type="int" summary="vertical refresh rate in mHz"/>
> > </event>
>
> I wonder, could we invent a better term here than global coordinates?
> What you actually mean here, I guess, is that the scale of these
> coordinates is the same as in surface coordinates. That is, a pixel has
> the same area. Except of course that does not hold, if a compositor
> applies a transformation, so we'd have to assume no transformation.
>
> Clients do not know about the global coordinate system, so defining
> things in it in the protocol makes little sense.
>
> Basically, I guess, if a client makes a surface the size of the output,
> and uses the output_scale as the buffer_scale, and the
> buffer_transform too, then the compositor might scan out the client
> buffer directly, with pixels 1:1 from buffer to screen. But how do we
> say that?
>
> >
> > @@ -1575,6 +1630,30 @@
> > atomic, even if they happen via multiple events.
> > </description>
> > </event>
> > +
> > + <event name="scale" since="2">
>
> I believe the commit message should say, that this patch explicitly
> relies on the earlier patch for bumping the wl_output interface version.
> Therefore, maybe these two patches should be just one.
>
> > + <description summary="output scaling properties">
> > + This event contains scaling geometry information
> > + that is not in the geometry event. It may be sent after
> > + binding the output object or if the output scale changes
> > + later. If it is not sent, the client should assume a
> > + scale of 1.
> > +
> > + A scale larger than 1 means that the compositor will
> > + automatically scale surface buffers by this amount
> > + when rendering. This is used for very high resolution
> > + displays where applications rendering at the native
> > + resolution would be too small to be legible.
> > +
> > + It is intended that scaling aware clients track the
> > + current output of a surface, and if it is on a scaled
> > + output it should use wl_surface.set_buffer_scale with
> > + the scale of the output. That way the compositor can
> > + avoid scaling the surface, and the client can supply
> > + a higher detail image.
> > + </description>
> > + <arg name="factor" type="uint" summary="scaling factor of output"/>
> > + </event>
> > </interface>
> >
> > <interface name="wl_region" version="1">
>
> Ok.
>
> Some open questions, but looking better to me!
> I didn't read the other comments in this thread yet.
>
>
> Thanks,
> pq
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list