[RFC PATCH v2] Add xdg-output protocol

[Olivier Fourdan]

Hi Pekka,

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

Well, for now, it will be used by Xwayland, might be of some interest for other clients who might need to know about the output "logical" size (e.g. screencast maybe? not even sure...).
 
> 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?

Yes, I kind of took for granted that those concepts where defined in wl_output, but considering this protocol is aimed at complementing wl_output for the desktop concept of an ouptut, it indeed makes sense to clarify further in xdg_output.

> > +
> > +    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)?

Mostly because it started as an additional wl_output event I guess, but I agree a "conventional" design is more appropriate for a separate xdg_output protocol.

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

Right.

> > +    </description>
> > +
> > +    <event name="position">
> 
> To mirror logical_size, should this be called logical_position?

OK

> > +      <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?

Yes, but not just that, this is also to cope with the different ways weston and mutter (< 3.26) deal with Xwayland surfaces, what you mentioned about Xwayland not downscaling the advertised output size (needed for weston, but not for mutter).

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

Yes, instead of "normal", I would that that most Wayland clients should not need this and would rely on xdg_shell interfaces.
 
> 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,

Well, X11 clients could never use that directly of course.

> 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 Xwayland issue this is aimed at solving is current, no need to go as far as achieving compositor bypass or direct scan out (I am not sure either if the logical_size would help at all with that)

Let's take an example, a hidpi monitor 3840×2160 at scale 2.

Right now, advertised as its mode resolution in Xwayland (which doesn't take into account the wl_output scale), so Xrandr reports:

  XWAYLAND1 connected 3840x2160+0+0
     3840x2160     60*+

In weston, an X client trying to configure its window the size based on what Xrandr reports for the monitor ends up with a window of size 7860×4320 because weston will scale the Xwayland surface buffer by 2, whereas in mutter, the resulting window is 3840×2160 because mutter (up to 3.26) doesn't scale Xwayland surfaces. But that will eventually be configurable in mutter, so there is no "one fits all" solution other than the compositor itself telling the "logical" output size.

In such a case, weston would set the "logical_size" as 1920×1080 whereas mutter current or future version without "scale-monitor-framebuffer" set would set a logical size of 3840×2160.

With mutter implementing fractional scaling and using a scale of 1.5 (for example), it would set a wl_output.scale of 2 and set a logical size of 2560×1620.

I'll add that example in the description for logical_size.

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

Yes, but that's for "regular" Wayland clients, Xwayland surface aren't scaled in mutter even though Xwayland doesn't use wl_surface.set_buffer_scale (mutter doesn't support set_buffer_transform, either)

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

I think I kinda see what you mean, I would remove that sentence altogether (it's convoluted and confusing) and rephrase the introductory sentence as:

 "For example, a surface without any buffer scale, transformation
  nor rotation set, with the size matching the logical_size will
  have the same size as the corresponding output when displayed."

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

Not mine of course, this is from wl_output :)

> > +      </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 for the feedback! I'll post an updated patch soon(ish).

Cheers,
Olivier