weston: weston randr protocol for testing and configuration

Wang, Quanxian quanxian.wang at intel.com
Sat Mar 22 17:23:10 PDT 2014


Hi, Pq

Except the below comments. I want to mention several key points here.

I have added the following request in protocol. 
1) randr_commit (commit all requests in one shot)
2) randr_newmode (create mode with hdisplay, hsync_start, ... following weston.config definition to add custom mode)
3) randr_delmode (delete mode)
And create a request structure to store all requests from client. And then when randr_commit, this request could be sent out in one time.

By the way, in the coming version, I will provide a complete solution including the randr protocol and its implementation.
The interested developers could have a try and provide more comments.

The time will be a little longer mainly because of testing and code review. I am sorry about that.

Thanks for your support.

>-----Original Message-----
>From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
>Sent: Saturday, March 22, 2014 8:20 PM
>To: Wang, Quanxian
>Cc: wayland-devel at lists.freedesktop.org
>Subject: Re: weston: weston randr protocol for testing and configuration
>
>Hi
>
>On Fri, 14 Mar 2014 11:18:48 +0800
>Quanxian Wang <quanxian.wang at intel.com> wrote:
>
>> Objective:
>> With discussion in mail list, currently we have an agreement. Randr interfaces
>will not be exposed public.
>> The objective will be only for testing and configuration. Thanks Pq, Jason, Jasper,
>Hardening, and other's comment.
>>
>> Before sending the email, the function list below is implementedi(tested)
>except new mode creating(not testing).
>>
>> Thanks for your support.
>>
>> Updates:
>> 1) Change wl_randr to weston_randr for weston-specific.
>> 2) Unique operation issue
>> Given that one client has two more threads to do mode setting on the same
>output at the same time, how to identify what response (done event) is belong to
>one or another request when they want to get response?
>>
>
>Talking about threads here is misleading. Such threaded operations do not make
>sense to begin with. The question is more about ambiguity between operations
>and acknowledgements, and preferring explicit correspondence than relying on
>protocol message ordering.
>So the goal is good, but the rationale is slightly wrong.
>
>> This is a design flaw in the first version of randr protocol.
>>
>> The solution is to use the wayland/weston mechanism, every request will
>generate a resource which be used to send done event to the owner who start it.
>Owner could set the listener on that and keep tuning on the response event.
>>
>> For example
>> In client:
>> randr_resource =
>> wl_randr_set_transform(randr.randr,wayland_output,argument.transform);
>> wl_randr_add_listener(randr_resource, &randr_transform_listener,
>> &randr);
>>
>> In compositor:
>> randr_resource = wl_resource_create(client,&wl_randr_interface,1,
>> action); ...
>> wl_randr_send_action_done(randr_resource,
>> 1<<WL_RANDR_ACTION_TRANSFORM, ret, action);
>> wl_resource_destroy(randr_resource);
>
>I do not want a reply for every single part of output state, I want a reply for the
>final commit request that applies all the state updates gathered so far. That will
>also solve the atomicity problem.
>
>See how wl_surface works, and add a reply object only to the commit request.
[Wang, Quanxian] I am working on that. randr_commit will be added for this.

>
>> 3) Mess up issue
>> Will take randr protocol implementation as a module. This will keep compositor
>clear and avoid messing up in compositor.c.
>>
>> You can enable it from command line
>> 'weston --tty=1 --enable-wrandr' or put wrandr.so in to module list in weston.ini.
>
>This kind of "messing up" was never a problem. Making it a module is a solution to
>the security concerns. Again, the right goal, but not the right reason.
>
>> 4) Add group randr operations
>> After talking with Pq, in order to avoid glitches, group operation is needed.
>However, if operating on two outputs more at one time, it will bring more issues
>which could not control. In this randr design, will not provide group operation on
>multiple outputs. We provide atomic operation on one output, multi outputs
>operation logic are left to designer/developers. Group operation on one output
>will be designed. For example, you can set mode, scale, and transform at one time
>with one randr request.
>>
>
>This seems to contradict your example above about the reply objects.
[Wang, Quanxian] yes.
>
>> 5) Mode setting parameters control
>> Output name will be under the control, for mode setting, from suggestion of
>Hardening and Jasper St. Pierre, it'd better to let user new mode. Defaultly I agree
>that. If someone has other suggestion and reason, please let me know.
>>
>
>Sorry, what?
>
>If this means you will support custom modes, that's good. It's not in your proposal
>below, though.
[Wang, Quanxian] I will add newmode and delmode request for that.
>
>> 6) Disconnected outputs
>> Currently I have disable that. But basically, when user query output information,
>showing connected and disconnected output as a whole will be fine for user and
>QA people. Sometime, QA people or user like that information. It will be helpful
>for them to identify how many outputs are provided by their machine. Which is
>connected and which is not connected.
>> So when user has requirement, that is fine. I could enable it.
>>
>> 7) wl_output property name event
>> I have sent patches to enable it. Not only for randr, for any client of wayland, it
>will be helpful to get a sensible name instead of only wl_output object.
>>
>> 8) adding set_scale request
>> Mode, scale, transform are the basic properties of output, I will add them as a
>whole.
>>
>> Original information:
>>
>> The idea is from xrandr provided by xserver. *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. If you want to make sure if it is successful, you
>> need keep tuning done event.
>> after all, it is a protocol communication.
>>
>> With this protocol, weston-wrandr application is developped to help
>> implement randr protocol in command line just like xrandr command in
>> xserver.
>>
>> Weston protocol wrandr will provide interface to
>> 1) set output mode, scale, transform
>> 2) move output to relative position
>>
>> Here are some use cases.
>>
>> 1. weston-randr -q # query all output mode info and disconnected
>> output
>>
>> HDMI3
>> 1)1440x900 at 60 (current)
>> 2)1920x1200 at 60
>> 3)1680x1050 at 60
>> ...
>>
>> VGA1
>> 1)1280x1024 at 60 (current)
>> 2)1152x864 at 60
>> 3)1024x768 at 60
>> ...
>>
>> 2. weston-randr --output HDMI3 # query HDMI3 output mode info
>>
>> HDMI3
>> 1)1440x900 at 60 (current)
>> 2)1920x1200 at 60
>> 3)1680x1050 at 60
>>
>> 3. weston-randr --output HDMI3 --mode 2 # which will set mode as
>> 1920x1200 or weston-randr --output HDMI3 --mode 1920x1200 at 60 or
>> weston-randr --output HDMI3 --mode 1920x1200
>>
>> 4. weston-randr --output HDMI3 --transform 1 # rotate HDMI3 output 90
>> degree
>>
>> 5. weston-randr --output HDMI3 --leftof VGA1 # put HDMI3 output leftof
>> VGA1
>>
>> Group operations example:
>> 5. weston-randr --output HDMI3 --transform 3 --scale 2 --mode 2
>> -leftof VGA1
>
>Nice.
>
>>
>> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
>> ---
>>  protocol/randr.xml   | 166
>+++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 166 insertions(+)
>>  create mode 100644 protocol/randr.xml
>>
>> diff --git a/protocol/randr.xml b/protocol/randr.xml new file mode
>> 100644 index 0000000..5c1d2d2
>> --- /dev/null
>> +++ b/protocol/randr.xml
>> @@ -0,0 +1,166 @@
>> +<?xml version="1.0" encoding="UTF-8"?> <protocol name="randr">
>> +
>> +  <copyright>
>> +    Copyright (c) 2014 Quanxian Wang
>> +    Copyright (c) 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>
>> +
>> +    <enum name="error">
>> +      <entry name="bad_randr" value="0"
>> +             summary="the to-be weston_randr is invalid"/>
>
>When does this error happen?
[Wang, Quanxian] Delete it. Thanks
>
>While I cannot understand what "bad_randr" could mean, I see no definition on
>what happens if a client sends illegal values, like scale -400.
[Wang, Quanxian] This has been rejected if scale is set to minus number.
>
>> +    </enum>
>> +
>> +    <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="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"/>
>> +      <arg name="refresh" type="int" summary="vertical refresh rate
>> +in mHz"/>
>
>So this is the simple mode set request.
>
>Do you think width/height/refresh is really enough to identify a mode? I don't
>think so. I think in the early days of X11 RandR, NVidia hit the same problem, and
>started to expose fake refresh values, only to be able to distinguish modes that
>were identical in width/height/refresh but still different in timings. Or actually I
>think it was much more complicated than that, but this is the point in simple terms.
>
>So we need something else here to identify a mode.
>
>Check what kind of protocol GNOME uses, and how current RandR protocol works.
[Wang, Quanxian] Yes, only height, width, refresh could not be identified as one unique mode. Refresh could be calculated out as the same value with different parameter.
>
>> +      <arg name="randr_mode" type="new_id" interface="weston_randr"
>> +           summary="weston_randr resource handle"/>
>
>See earlier on what I said about the reply objects and committing.
>
>weston_randr is your global interface, it makes no sense for its requests to create
>more weston_randr objects. You should not use weston_randr here as the object
>type to be created, but another interface written only for the reply.
>
>> +    </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.
>
>Just refer to the wl_output specification where these come from, right? These
>values are not invented here, they specified elsewhere, so say what specifies
>them.
[Wang, Quanxian] right. It is from wl_output specification. It should be consistent with definition of wl_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"/>
>> +      <arg name="randr_transform" type="new_id"
>interface="weston_randr"
>> +           summary="weston_randr resource handle"/>
>> +    </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"/>
>> +      <arg name="randr_scale" type="new_id" interface="weston_randr"
>> +           summary="weston_randr resource handle"/>
>> +    </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"/>
>
>What about above/below?
[Wang, Quanxian] Currently in Weston compositor, there is no such requirement. If supported, I like to add that. It is extensible for future change.
>
>> +    </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"/>
>> +      <arg name="randr_move" type="new_id" interface="weston_randr"
>> +           summary="weston_randr resource handle"/>
>> +    </request>
>> +
>> +    <request name="operations">
>> +      <description summary="multiple operations on one output">
>> +	Do multiple operations on target output. This allows uer
>> +	to do mode, scale , transform, move at the same time.
>> +	It will stop glitches for multiple actions
>> +	one by one when mode setting.
>> +      </description>
>> +      <arg name="target_output" type="object" interface="wl_output"
>> +           summary="the target output object"/>
>> +      <arg name="left" type="object" interface="wl_output"
>allow-null="true"/>
>> +      <arg name="right" type="object" interface="wl_output"
>allow-null="true"/>
>> +      <arg name="width" type="int"/>
>> +      <arg name="height" type="int"/>
>> +      <arg name="refresh" type="int"/>
>> +      <arg name="transform" type="int"
>> +           summary="The value should be between 0-7"/>
>> +      <arg name="scale" type="int"
>> +           summary="Scale the output"/>
>> +      <arg name="randr_operations" type="new_id"
>interface="weston_randr"
>> +           summary="weston_randr resource handle"/>
>
>No, no, not like this. Look at how wl_surface collects several pieces of state, and
>then applies them all in one go.
>
>Fixing this, all the reply object stuff, and supporting atomic setting over multiple
>outputs will actually make your protocol much simpler, and at the same time more
>powerful and extensible.
>
>Extendability is important, we may get more output properties later, just like scale
>was added after wl_output interface was already stable.
[Wang, Quanxian] Yes, I have noticed that. I have implemented randr_commit in next version of weston randr. New commit structure will be used for storing of request and then commit in one shot not like that.
>
>> +    </request>
>> +
>> +    <enum name="status">
>> +      <description summary="flags">
>> +	These flags show the status of every action excuted by randr.
>> +      </description>
>> +      <entry name="mode" value="0"/>
>> +      <entry name="move" value="4"/>
>> +      <entry name="transform" value="8"/>
>> +      <entry name="scale" value="12"/>
>> +    </enum>
>
>It is unclear how this would work or what it means.
[Wang, Quanxian] Let me detailed that.
>
>> +
>> +    <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="action_done">
>> +      <description summary="action done event">
>> +	Notify the randr client that mode setting has been done.
>> +      </description>
>> +      <arg name="flags" type="uint"
>> +           summary="flag action and result"/>
>> +      <arg name="action_id" type="uint"/>
>
>This is the old kind of stuff you had, which I thought you replaced with a reply
>object interface. But you didn't define any reply object interface anywhere yet.
>
>The reply interface could be something like this:
>
><interface name="weston_randr_update" version="1">
>  <event name="done">
>    <arg name="status" type="uint"/>
>  </event>
></interface>
>
>Of course you need to add all the details about object destruction, etc.
>
>That is very similar to wl_callback, except this one is specific to weston_randr and
>you can extend it if needed, and define what the status codes are: success/fail;
>maybe add some reason-why-failed if it makes sense...
>
>Or instead of one "done" event there could be events "success" and "failure",
>where only failure would return the extra info.
[Wang, Quanxian] Sorry, I am not familiar with them. I need to check how to achieve that. Basically I will send done event with result of all actions in one commit.
I will send version 2 soon and use randr_commit to return a new randr interface, and then add new listener to it. 
And then in compositor, randr_commit will get the newid and then send it to done event. It will promise the commit send the done event to right committer.
This will implement who commit who get done event instead of create a new special interface(just like Weston_randr_update mentioned above)

You can check version 2 of Weston randr. I will send later. I am testing and review them. I will provide the whole solution including the implementation for that.

Thanks for your comment.

>
>> +    </event>
>> +  </interface>
>> +</protocol>
>> --
>> 1.8.1.2
>>
>
>
>Thanks,
>pq


More information about the wayland-devel mailing list