[PATCH 2/2] protocol: Support scaled outputs and surfaces

Pekka Paalanen ppaalanen at gmail.com
Mon May 20 00:49:27 PDT 2013


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.

>        </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.

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?

>  
> @@ -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.

>     </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



More information about the wayland-devel mailing list