[PATCH V2 1/8] wesston: Add weston randr protocol
Pekka Paalanen
ppaalanen at gmail.com
Fri Mar 28 08:00:19 PDT 2014
On Mon, 24 Mar 2014 19:39:13 +0800
Quanxian Wang <quanxian.wang at intel.com> wrote:
> Weston protocol wrandr will provide interface to
> 1) Mode set of output (scale, transform, mode)
> 2) Position of output (currently support leftof, rightof)
> 3) New a custom mode
> 4) Delete mode
>
> This protocol is not expose public. It is only for
> QA testing and Admin configuration currently.
>
> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
> ---
> protocol/Makefile.am | 1 +
> protocol/randr.xml | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 229 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..07f83a4
> --- /dev/null
> +++ b/protocol/randr.xml
> @@ -0,0 +1,228 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="randr">
> +
> + <copyright>
> + Copyright © 2014 Quanxian Wang
> + Copyright © 2014 Intel Corporation
> +
> + 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="weston_randr" version="1">
> + <description summary="randr">
> + The global interface exposing randr capabilities.
> + As a weston_randr, that provides the interfaces for apps to more operations
> + on output.
> +
> + The aim of weston_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>
> +
> + <request name="destroy" type="destructor">
> + <description summary="unbind from the weston_randr interface">
> + Informs the server that the client will not be using this
> + protocol object anymore. This does not affect any other
> + objects, weston_randr objects included.
> + </description>
> + </request>
> +
> + <request name="start">
> + <description summary="randr request callback">
> + It is request notification when the next output randr commit
> + is coming and is useful for notifying client the result of
> + operations on the output. The randr request will take effect
> + on the next weston_randr.commit. The notification will only be
> + posted for one randr request unless requested again.
> + </description>
> + <arg name="output" type="object" interface="wl_output"/>
> + <arg name="callback" type="new_id" interface="wrandr_callback"/>
Ok, the reply object is created as the first thing. What happens if you
create multiple of them without sending commit in between? They all
return the same result?
Why do you have an output argument here?
Are you explicitly forbidding the very useful case, where one could
gather changes to multiple outputs into the same batch to be committed?
I think that would be a misfeature. I don't see any reason to not
allow changing multiple outputs in the same batch. Quite the contrary,
when atomic modesetting becomes available in the kernel, we will be
able to use it only if we allow changing multiple outputs at the
same time in the protocol.
It does not matter if the kernel or platform do not support atomic mode
setting over multiple outputs, the protocol can and should still have
it.
> + </request>
> +
> + <enum name="mode">
> + <description summary="mode information">
> + These flags describe properties of an output mode.
> + They are used in the flags bitfield of the mode event.
> + Here we take the last 28-32th bit as additional flags
> + which is different with original output mode.
Sorry, how does this work? What mode event?
If you mean wl_output.mode event, I'm not sure you can extend the flags
there from here. wl_output would need to specify, that flags not
defined there should be ignored by the client.
> + </description>
> + <entry name="custom" value="0x1000"
> + summary="indicates this is the custom mode"/>
Only one custom mode ever possible? Per output?
> + <entry name="del" value="0x2000"
> + summary="indicates the mode will be deleted"/>
How do you know, that a client can handle this flag properly?
I mean, that the client does not just ignore an unknown flag and add a
mode instead of deleting one.
> + </enum>
> +
> + <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" summary="width of the mode in hardware units"/>
> + <arg name="height" type="int" summary="height of the mode in hardware units"/>
I've come to realize there are two different use cases for "modes":
- the usual definition, the video mode for a monitor, which includes
all timing details either implicitly or explicitly
- the VNC/RDP case, where there is not so much a "mode" but only the
size of an output, although in these cases it is better controlled
from the remote.
For the latter case, I only now realized that it would be silly to have
to create a new custom mode for every single output size you want to
set, when the output really is just a window on another system. Or is
it?
I guess there is some ground for having two different paths.
OTOH it makes me wonder if a VNC/RDP-backed compositor should ever even
expose weston_randr. It quite clearly does not fit the intended use
case.
> + <arg name="refresh" type="int" summary="vertical refresh rate in mHz"/>
> + </request>
> +
> + <request name="set_transform">
> + <description summary="set the transform of output">
> + Set the transform of output, the valuable value should
> + be 0-7. This means 0, 90, 180, 270, FLIP-0, FLIP-90,
> + FLIP-180, FLIP-270. The values are referred from
> + the wl_output specification.
> + </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>
> +
> + <request name="set_scale">
> + <description summary="set the scale of output">
> + Set the scale of output.
> + </description>
> + <arg name="output" type="object" interface="wl_output"
> + summary="the output object"/>
> + <arg name="scale" type="int"
> + summary="Scale the output"/>
> + </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"/>
Still missing above/below at least.
> + </enum>
> +
> + <request name="move">
> + <description summary="move target output">
> + Move the target output to be 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>
> +
> + <request name="del_mode">
> + <description summary="set the mode of output">
> + Set the mode of output.
I think you forgot to document this.
> + </description>
> + <arg name="output" type="object" interface="wl_output"
> + summary="the output object"/>
> + <arg name="width" type="int" summary="width of the mode in hardware units"/>
> + <arg name="height" type="int" summary="height of the mode in hardware units"/>
> + <arg name="refresh" type="int" summary="vertical refresh rate in mHz"/>
> + </request>
> +
> + <request name="new_mode">
> + <description summary="new custom mode">
> + Set the mode of output.
> + </description>
> + <arg name="output" type="object" interface="wl_output"
> + summary="the output object"/>
> + <arg name="fclock" type="int"
> + summary="Pixel clock in hardware units Mhz"/>
int MHz? hardware units? That does not sound too precise, maybe it
should be kHz? That would still go up to 2 THz.
> + <arg name="hdisplay" type="int"
> + summary="The number of pixel for one horticontal line in units Hz"/>
> + <arg name="hsync_start" type="int"
> + summary="Start of horizontal sync signal in units Hz"/>
> + <arg name="hsync_end" type="int"
> + summary="End of horizontal sync signal in units Hz"/>
> + <arg name="htotal" type="int"
> + summary="The whole cycle of a horizontal line in units Hz"/>
> + <arg name="vdisplay" type="int"
> + summary="The number of pixel for one vertical line in units Hz"/>
> + <arg name="vsync_start" type="int"
> + summary="Start of vertical sync signal in units Hz"/>
> + <arg name="vsync_end" type="int"
> + summary="End of vertical sync signal in units Hz"/>
> + <arg name="vtotal" type="int"
> + summary="The whole cycle of a vorizontal line in units Hz"/>
Careful with the units there.
> + <arg name="hsync" type="string"
> + summary="the polarity of the horizontal sync pulses"/>
> + <arg name="vsync" type="string"
> + summary="the polarity of the vertical sync pulses"/>
The strings need to be documented.
Are you assuming there can be only one custom mode for an output?
And if the current mode happens to be custom, is there a way e.g. to
read it back, so that if you try another mode, you can return back to
it?
> + </request>
> +
> + <request name="commit">
> + <description summary="multiple operations on one output">
> + Commit all the previous output changes from the client.
> + Before the commit, the chagne request will be stored in wrandr interface.
> + Once randr interface commit, it will update the change and clear up
> + the requests from this client.
> + </description>
> + <arg name="output" type="object" interface="wl_output"
> + summary="the output object"/>
Here, the same comments apply as to the "start" request. We should not
be restricted to just one output, hence the output argument is not
needed here.
You could also just remove "start" request completely, and create the
reply object here. If we even want an explicit reply object at all. I'm
starting to lean on Jasper's side here after all.
> + </request>
> + </interface>
> +
> + <interface name="wrandr_callback" version="1">
> + <description summary="weston randr callback object">
> + Clients can handle the 'done' event to get notified when
> + the related request is done.
> + </description>
> +
> + <request name="destroy" type="destructor">
> + <description summary="unbind from the interface">
> + Informs the server that the client will not be using this
> + protocol object anymore. This does not affect any other
> + objects, wrandr_callback objects included.
Copy-pasta doc.
> + </description>
> + </request>
> +
> + <enum name="action">
> + <description summary="flags">
> + These flags show the status of every action excuted by randr.
> + </description>
> + <entry name="mode" value="0"/>
> + <entry name="move" value="1"/>
> + <entry name="transform" value="2"/>
> + <entry name="scale" value="3"/>
> + <entry name="newmode" value="4"/>
> + <entry name="delmode" value="5"/>
> + </enum>
> +
> + <enum name="result">
> + <description summary="action result">
> + Action result
> + </description>
> + <entry name="SUCCESS" value="1"/>
> + <entry name="FAIL" value="2"/>
> + <entry name="NOACT" value="3"/>
> + </enum>
> +
> + <event name="done">
> + <description summary="action done event">
> + Notify the randr client that mode setting has been done.
> + Flag shows the action executed. every bit stands for one action.
> + Result shows the final status for every action, every 2
> + bit will show that.
> + </description>
> + <arg name="flags" type="uint" summary="flag action"/>
> + <arg name="result" type="uint" summary="flag result"/>
The probably only interesting failure case is the "mode" case. Move,
transform, and scale can only come from illegal values in the requests,
right? For newmode and delmode I'm not sure they do here, just
for illegal values?
If illegal values can be known before-hand to be illegal, you could just
do a fatal protocol error.
Extending this to cover multiple outputs does not seem to work too
well, either. For now, I would be satisfied with a simple "succeeded"
or "failed" reply. The failure reply might carry a string with an
explanation for the user. It is very hard to design what kind of
failures would need to be communicated and what the compositor could
even detect, so using a free-form string seems the best. Presumably the
only thing the client would do with the failure information is to show
it to the user.
> + </event>
> + </interface>
> +</protocol>
I'm still personally not quite sure in which direction to develop this
protocol with the custom modes thing.
Thanks,
pq
More information about the wayland-devel
mailing list