[PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

-----Original Message-----
From: Pekka Paalanen [mailto:ppaalanen at gmail.com] 
Sent: Thursday, February 27, 2014 3:28 PM
To: Wang, Quanxian
Cc: wayland-devel at lists.freedesktop.org; Zhang, Xiong Y
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Thu, 27 Feb 2014 11:28:00 +0800
Quanxian Wang <quanxian.wang at intel.com> wrote:

> Weston protocol wrandr will provide interface to
> 1) set output mode
> 2) set output transform
> 3) move output to relative position
> 4) provide disconnected display port information
> 
> *Dynamic* mode setting is the main objective of this protocol.
> Remember, it is one shot operation. For example, if setting the mode, 
> just call one request wl_randr_set_mode without any other operation.

Hi,

like I said in my other reply, this is a configuration interface, not something for all applications to use. Protocol comments below assuming, that this will be a configuration interface and that the generic idea is acceptable.

> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
> Reviewed-by: Zhang, Xiong Y <xiong.y.zhang at intel.com>
> ---
>  protocol/Makefile.am |   1 +
>  protocol/randr.xml   | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 152 insertions(+)
>  create mode 100644 protocol/randr.xml
> 
> diff --git a/protocol/Makefile.am b/protocol/Makefile.am index 
> 5e331a7..df2e070 100644
> --- a/protocol/Makefile.am
> +++ b/protocol/Makefile.am
> @@ -5,6 +5,7 @@ protocol_sources =				\
>  	text.xml				\
>  	input-method.xml			\
>  	workspaces.xml				\
> +	randr.xml				\
>  	text-cursor-position.xml		\
>  	wayland-test.xml			\
>  	xdg-shell.xml				\
> diff --git a/protocol/randr.xml b/protocol/randr.xml new file mode 
> 100644 index 0000000..f15e87a
> --- /dev/null
> +++ b/protocol/randr.xml
> @@ -0,0 +1,151 @@
> +<?xml version="1.0" encoding="UTF-8"?> <protocol name="randr">
> +
> +  <copyright>
> +    Copyright (c) 2012-2013 Collabora, Ltd.

Fix this.

> +
> +    Permission to use, copy, modify, distribute, and sell this
> +    software and its documentation for any purpose is hereby granted
> +    without fee, provided that the above copyright notice appear in
> +    all copies and that both that copyright notice and this permission
> +    notice appear in supporting documentation, and that the name of
> +    the copyright holders not be used in advertising or publicity
> +    pertaining to distribution of the software without specific,
> +    written prior permission.  The copyright holders make no
> +    representations about the suitability of this software for any
> +    purpose.  It is provided "as is" without express or implied
> +    warranty.
> +
> +    THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> +    SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> +    FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> +    SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
> +    AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> +    ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
> +    THIS SOFTWARE.
> +  </copyright>
> +
> +  <interface name="wl_randr" version="1">
> +    <description summary="randr">
> +      The global interface exposing randr capabilities.
> +      As a wl_randr, that provides the interfaces for apps to more operations
> +      on output.
> +
> +      The aim of wl_randr is to get modes list, choose preferred mode,
> +      layout the output including position, rotate, and en/disable.
> +      The idea is from xrandr protocoal of xserver. It is very convenient for
> +      weston/wayland user to operates on mode setting of output.
> +    </description>
> +
> +    <enum name="error">
> +      <entry name="bad_randr" value="0"
> +             summary="the to-be wl_randr is invalid"/>
> +    </enum>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="unbind from the wl_randr interface">
> +	Informs the server that the client will not be using this
> +	protocol object anymore. This does not affect any other
> +	objects, wl_randr objects included.
> +      </description>
> +    </request>
> +
> +    <request name="set_mode">
> +      <description summary="set the mode of output">
> +	set the mode of output
> +      </description>
> +      <arg name="output" type="object" interface="wl_output"
> +           summary="the output object"/>
> +      <arg name="width" type="int"/>
> +      <arg name="height" type="int"/>
> +      <arg name="refresh" type="int"/>
> +    </request>

Since you require a wl_output, there does not seem to be a way to force a disconnected output on? Should there be? Why else would you need a way to list disconnected outputs?
[Wang, Quanxian] list disconnected outputs, just let user get to know the status of display port in this current machine. Who are connected, who are not connected.

Should there be a way to force a connected output off?

What about detailed mode timings? Should the client be allowed to use e.g. CVT or GTF formulae to invent completely new modes that the hardware might support?
[Wang, Quanxian] wl_out or mode will be firstly be available in compositor. Otherwise, this interface will not allow that. We will check the parameter to make sure if user sets it rightly or under the control.

> +
> +    <request name="set_transform">
> +      <description summary="set the transform of output">
> +	set the transform of output
> +      </description>
> +      <arg name="output" type="object" interface="wl_output"
> +           summary="the output object"/>
> +      <arg name="transform" type="uint"
> +           summary="the value should be between 0-7"/>
> +    </request>
> +
> +    <enum name="move">
> +      <description summary="move target output to the position of source output">
> +	The purpose is mainly to allow clients move target output to
> +        the position of source output.
> +      </description>
> +      <entry name="rightof" value="0"/>
> +      <entry name="leftof" value="1"/>
> +    </enum>
> +
> +    <request name="move">
> +      <description summary="move target output">
> +	position the target output leftof/rightof source output
> +      </description>
> +      <arg name="target_output" type="object" interface="wl_output"
> +           summary="the target output object"/>
> +      <arg name="source_output" type="object" interface="wl_output"
> +           summary="the source output object"/>
> +      <arg name="move" type="int"/>
> +    </request>

Let's say I want to change a mode and move an output to another side.
How would I do that atomically? I mean, without the compositor e.g.
[Wang, Quanxian] live with compositor.

first showing me the output on the other side in original mode, and then switches the mode to what I asked for.
[Wang, Quanxian] :). Currently we don't support complex command in one shot. The request is an atomic action.
If you want to do that, just first change mode, and then call another atomic operation to move.

How do I do many operations, perhaps on multiple outputs, in a single shot without flickering or glitches?
[Wang, Quanxian] The above.

> +
> +    <request name="get_disoutputs">
> +      <description summary="disconnected output information">
> +	get the disconnected display port information
> +      </description>
> +    </request>

It seems you have modeled this interface like a function call API.
Function calls are synchronous and built on roundtrips (return values), while Wayland protocol is the exact opposite.
[Wang, Quanxian] Right. But I just want to get the information from compositor instead of from drm or others. It lives with compositor.

You could design a protocol, where the list of disconnected outputs is kept up to date automatically by the compositor just sending events when a client a) binds to the global interface, and b) when connector status changes. That could include also connected outputs/connectors.
[Wang, Quanxian] Yes. Good idea.

> +    <request name="get_output_name">
> +      <description summary="output name">
> +	get the output name
> +      </description>
> +      <arg name="wl_output" type="object" interface="wl_output"/>
> +    </request>

Yeah, would be better as a wl_output event indeed.
[Wang, Quanxian] core protocol is not easy to be changed. :)

> +
> +    <event name="output_name">
> +      <description summary="output name">
> +	get output name of wl_output
> +      </description>
> +      <arg name="output" type="object" interface="wl_output"/>
> +      <arg name="name" type="string"/>
> +    </event>
> +
> +    <enum name="action">
> +      <description summary="action">
> +	Actions have been taken.
> +      </description>
> +      <entry name="move" value="0"/>
> +      <entry name="transform" value="1"/>
> +      <entry name="mode" value="2"/>
> +    </enum>
> +
> +    <enum name="result">
> +      <description summary="action result">
> +	Action result
> +      </description>
> +      <entry name="FAIL" value="-1"/>
> +      <entry name="SUCCESS" value="1"/>
> +      <entry name="NOACT" value="2"/>
> +    </enum>
> +
> +    <event name="action_done">
> +      <description summary="action done event">
> +	Notify the randr client mode set done.
> +      </description>
> +      <arg name="flags" type="uint"
> +           summary="which action is done"/>
> +      <arg name="result" type="int"/>
> +    </event>

If I change the mode on two different outputs, how do I know which action_done corresponds to which request?
[Wang, Quanxian] right, add wl_output parameter.

What if move succeeds but mode change fails? Wouldn't that leave the output in an unwanted state which is neither the original nor the wanted setting?
[Wang, Quanxian] one by one. Not support complex. If you have such case, have to turn back. Call another move back. But firstly make sure the previous is successful.

The solution to this would tie in with the solution to take changes atomic. For instance, to prepare for a configuration change, one might create a change object in the protocol, store all changes in that object, and then commit that set of changes atomically. Then have one return value: the whole set either succeeds or fails.

I guess you could look for inspiration in the DRM atomic mode setting API. I don't know how the RandR X11 protocol works, if that would be a good example also.

> +
> +    <event name="disoutputs">
> +      <description summary="disconnected output string event">
> +	Send back disconnected outputs information.
> +      </description>
> +      <arg name="output" type="string"/>
> +    </event>
> +  </interface>
> +
> +</protocol>

Thanks,
pq