[RFC PATCH v2] Add xdg-output protocol

Pekka Paalanen ppaalanen at gmail.com
Thu Jul 6 08:01:47 UTC 2017


On Tue,  4 Jul 2017 14:13:43 +0200
Olivier Fourdan <ofourdan at redhat.com> wrote:

> This protocol aims at describing outputs in way which is more in line
> with the concept of an output on desktop oriented systems.
> 
> Some information are more specific to the concept of an output for a
> desktop oriented system and may not make sense in other applications,
> such as IVI systems for example.
> 
> The goal is to gradually move the desktop specific concepts out of the
> core wl_output protocol.
> 
> For now it just features the position and logical size which describe
> the output position and size in the global compositor space.
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  v2: use "destroy" instead of "release" for destructor
> 
>  Makefile.am                                    |   1 +
>  unstable/xdg-output/README                     |   4 +
>  unstable/xdg-output/xdg-output-unstable-v1.xml | 135 +++++++++++++++++++++++++
>  3 files changed, 140 insertions(+)
>  create mode 100644 unstable/xdg-output/README
>  create mode 100644 unstable/xdg-output/xdg-output-unstable-v1.xml

Hi Olivier,

I think I can offer only mostly mechanical comments, as I am still
quite confused about how this will actually be used.

> 
> diff --git a/Makefile.am b/Makefile.am
> index e693afa..6c696aa 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -12,6 +12,7 @@ unstable_protocols =								\
>  	unstable/tablet/tablet-unstable-v2.xml			                \
>  	unstable/xdg-foreign/xdg-foreign-unstable-v1.xml			\
>  	unstable/idle-inhibit/idle-inhibit-unstable-v1.xml			\
> +	unstable/xdg-output/xdg-output-unstable-v1.xml				\
>  	$(NULL)
>  
>  stable_protocols =								\
> diff --git a/unstable/xdg-output/README b/unstable/xdg-output/README
> new file mode 100644
> index 0000000..e42b711
> --- /dev/null
> +++ b/unstable/xdg-output/README
> @@ -0,0 +1,4 @@
> +xdg_output protocol
> +
> +Maintainers:
> +Olivier Fourdan <ofourdan at redhat.com>
> diff --git a/unstable/xdg-output/xdg-output-unstable-v1.xml b/unstable/xdg-output/xdg-output-unstable-v1.xml
> new file mode 100644
> index 0000000..09d03eb
> --- /dev/null
> +++ b/unstable/xdg-output/xdg-output-unstable-v1.xml
> @@ -0,0 +1,135 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="xdg_output_unstable_v1">
> +
> +  <copyright>
> +    Copyright © 2017 Red Hat Inc.
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <description summary="Protocol to describe output regions">
> +    This protocol aims at describing outputs in way which is more in line
> +    with the concept of an output on desktop oriented systems.
> +
> +    Some information are more specific to the concept of an output for
> +    a desktop oriented system and may not make sense in other applications,
> +    such as IVI systems for example.

Would it be good to explain here, that on desktop the outputs are
expected to form a single connected 2D region? Exactly like RandR
outputs inside a single X11 SCREEN are, IOW window spanning and input
(pointer traveling) wise. Essentially this is that "global compositor
space" implies in practice.

And that each output is rectangular, able to show the complete
described area?

Or are these not actually always true?

> +
> +    Some of the information provided in this protocol might be identical
> +    to their counterpart already available from wl_output, in which case
> +    the information provided by this protocol should be preferred to their
> +    equivalent in wl_output. The goal is to move the desktop specific
> +    concepts (such as output location within the global compositor space,
> +    the connector name and types, etc.) out of the core wl_output protocol.

Good.

> +
> +    Warning! The protocol described in this file is experimental and
> +    backward incompatible changes may be made. Backward compatible
> +    changes may be added together with the corresponding interface
> +    version bump.
> +    Backward incompatible changes are done by bumping the version
> +    number in the protocol and interface names and resetting the
> +    interface version. Once the protocol is to be declared stable,
> +    the 'z' prefix and the version number in the protocol and
> +    interface names are removed and the interface version number is
> +    reset.
> +  </description>
> +
> +  <interface name="zxdg_output" version="1">
> +    <description summary="Compositor logical output region">
> +      An xdg_output describes part of the compositor geometry.
> +
> +      This typically corresponds to a monitor that displays part of the
> +      compositor space.
> +
> +      This object is published during start up, when a monitor is hot
> +      plugged or when the logical size of a wl_output is changed.

Do you mean that zxgd_output interface is a global interface advertised
via wl_registry?

Can you explain why picked this design instead of the (IMO) more
conventional design of advertising a global factory interface in
wl_registry, and the factory interface having a request
get_xdg_output(xdg_output new_id, wl_output base)?

I see some inconveniences with the design of using wl_output as an
event argument:

A client may bind to the same wl_output global many times. This may
happen from independent software components (libraries). The compositor
cannot know which events should be sent with which wl_output
wl_resource, so it must send events for all wl_resources pointing to
the same output. Likewise, client components then must be able to cope
with getting a wl_output proxy they did not create or set up, the first
issue there being identifying whether the user data of the proxy is of
the expected type.

Every single event in the interface must carry an explicit wl_output
argument, while this could be implicit from the xdg_output proxy
instance if it was created for a specific wl_output proxy instance to
begin with.


The more conventional design I mentioned above does impose any
additional roundtrips, the client binding to a wl_output and also
create a xdg_output immediately without waiting for any reply. The
events from both wl_output and xdg_output still come to the client in
the same burst.

> +    </description>
> +
> +    <event name="position">

To mirror logical_size, should this be called logical_position?

> +      <description summary="Position of the output within the global compositor space">
> +	The position event describes the location of the wl_output within the
> +	global compositor space.
> +
> +	The event is sent when binding to the xdg_output interface and whenever
> +	the location of the output changes within the global compositor
> +	space.
> +      </description>
> +      <arg name="output" type="object" interface="wl_output"
> +           summary="corresponding wl_output"/>
> +      <arg name="x" type="int"
> +	   summary="x position within the global compositor space"/>
> +      <arg name="y" type="int"
> +	   summary="y position within the global compositor space"/>
> +    </event>
> +
> +    <event name="logical_size">
> +      <description summary="Size of the output in the global compositor space">
> +	The logical_size event describes the size of the output in the global
> +	compositor space.
> +
> +	For example, a surface with the size matching the logical_size will
> +	have the same size as the corresponding output when displayed.
> +
> +	Clients such as Xwayland need this to configure their surfaces in
> +	the global compositor space as the compositor may apply a different
> +	scale from what is advertised by the output scaling property (to
> +	achieve fractional scaling, for example).

IOW, this event is a different way of sending the output scale,
supporting arbitrary fractional values. Except that we expect most
clients to ignore this and draw with the integer scale factor, instead
of using this information and wp_viewport to realize the fractional
scale factor. Is that correct?

Should there be wording to say that "normal" - however you define that -
clients should not use this information for sizing their buffers and
surfaces? And instead rely on the xdg_shell family of interfaces to
provide a target size.

I suppose we could really use a good write-up (with examples and
pictures!) on how Xwayland and compositors are intended to use this
information, how it affects RandR parameters and how X11 clients will
use this, and how their windows go through Xwayland to the compositor,
to achieve compositor-bypass and hit the direct scanout path in a
mixed-dpi environment. I get dizzy when I try to think of that. IOW, I
feel I cannot really evaluate the suitability of this solution for the
underlying Xwayland issue.


> +
> +	The logical size being in global compositor space implies that the
> +	client does not need to apply any wl_output.scale or wl_output.rotation
> +	to the given logical_size to figure the size of a surface when
> +	displayed on the output.

The only way you can actually directly set the size of a surface is via
wp_viewport, and in all other cases the surface size is computed from
the buffer size via the client-set buffer_scale and buffer_transform.

I think you wanted to talk about the output size here, somehow, that
you are given it directly and not need to take into account
output_scale and output_transform. If one then arranges a wl_surface to
have that size, it will be the same size as the output area. So it's
about finding out what size a client wants to have, not how the size of
a wl_surface is determined. Can you see what I mean?

> +
> +	The event is sent when binding to the xdg_output interface and whenever
> +	the logical size of the output changes, either as a result of a
> +	change in the applied scale or because of a change in the corresponding
> +	output mode (see wl_output.mode) or transform (see wl_output.transform).

Ok.

> +      </description>
> +      <arg name="output" type="object" interface="wl_output"
> +	   summary="corresponding wl_output"/>
> +      <arg name="width" type="int"
> +	   summary="width of the mode in global compositor space"/>
> +      <arg name="height" type="int"
> +	   summary="height of the mode in global compositor space"/>
> +    </event>
> +
> +    <event name="done">
> +      <description summary="Sent all information about output">
> +	This event is sent after all other properties have been	sent, after
> +	binding to an xdg_output interface and after any other property changes
> +	done after that.
> +
> +	This allows changes to the xdg_output properties to be seen as atomic,
> +	even if they happen via multiple events.

A good idea.

> +      </description>
> +      <arg name="output" type="object" interface="wl_output"
> +	   summary="corresponding wl_output"/>
> +    </event>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="Destroy the xdg_output object">
> +	Using this request a client can tell the server that it is not going to
> +	use the xdg_output object anymore.
> +      </description>
> +    </request>
> +
> +  </interface>
> +</protocol>
> +


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20170706/2aaf62ff/attachment.sig>


More information about the xorg-devel mailing list