Discussion:
[PATCH V2 0/8] Add weston randr protocol
(too old to reply)
Quanxian Wang
2014-03-24 11:39:12 UTC
Permalink
Important Changes:

1) Adding randr_commit, randr_delmode, randr_newmode, randr_start
randr_commit: will commit all requests in one time.
randr_delmode: delete one mode
randr_newmode: new one mode
randr_start: as the randr beginning, it will generate a callback
interface to keep tunning the commit.
2) Add wrandr_callback interface for tunning. (randr_start)
3) Add new request structure in compositor to store all the requests.
This structure is easy to control every client request set on one output.
4) Delete original design: one request has one default commit. It is not convenient
for group operations. Using commit request to submit all the request
in one time.
5) Take wrandr as a module
6) Add --enable-wrandr option to enable wrandr.
7) For 8th patch, it is still under consideration. It is only one choice.
But I am not sure if other agrees with that.

Hints:
This patches series has been tested. Until now it works fine.
If you want to have a try, you need two patches I send before.
1) [PATCH 0/3] Add wl_output name event
2) [PATCH] desktop-shell: Bug fix client apps because of output change
When you build weston, add option --enable-wrandr to enable the build of weston randr.
From Pq's comment, delete mode or set mode could not identify one unique mode using
parameters width, height and refresh. Currently there is no method to get drmModeinfo
from compistor. (wl_output_send_mode only send width, height and refresh).
Using new mode, user knows the parameter. However mode delete or mode set,
user could not get the detailed parameters from client. I will check with others to
get more idea to make sure how to implement that. Any comment is welcome.

By the way, currently we are focus on the protocol design. The implemented code is just
a reference. However it will be helpful for reviewer to have a test and get the clear
idea of protocol. I like your comment to make this protocol stronger. Thanks.
Especially thanks to Pq. He gives me more valuable information. I will continue keep
tuning your comment and make the change.

Thanks for your support.

Regards

Quanxian Wang

General Information:

Objective:
Randr interface will not be exposed. It is only for weston-specific.
and it is only for testing and configuration.

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 to 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) Mode set of output (scale, transform, mode)
2) Position of output (currently support leftof, rightof)
3) New a custom mode
4) Delete mode

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
6. weston-randton-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync,-vsync'
7. weston-randton-wrandr --output HDMI3 --delmode 3
or weston-randton-wrandr --output HDMI3 --delmode 1920x1200 at 60

Quanxian Wang (8):
wesston: Add weston randr protocol
weston: Add the weston randr support in compositor.h
weston: Add enable-wrandr option in compositor to load wrandr.so
Add new mode function in drm backend
weston:Add the weston randr protocol implementation
weston: Add configure to support wrandr.
Apps: Add weston-randr application
Add request_id for request set to be distinguished from others

Makefile.am | 5 +
clients/Makefile.am | 9 +
clients/wrandr.c | 879 ++++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 8 +
module/Makefile.am | 3 +
module/wrandr/Makefile.am | 32 ++
module/wrandr/wrandr.c | 803 ++++++++++++++++++++++++++++++++++++++++++
protocol/Makefile.am | 1 +
protocol/randr.xml | 236 +++++++++++++
src/compositor-drm.c | 67 ++++
src/compositor.c | 14 +-
src/compositor.h | 28 ++
12 files changed, 2083 insertions(+), 2 deletions(-)
create mode 100644 clients/wrandr.c
create mode 100644 module/Makefile.am
create mode 100644 module/wrandr/Makefile.am
create mode 100644 module/wrandr/wrandr.c
create mode 100644 protocol/randr.xml
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:13 UTC
Permalink
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"/>
+ </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.
+ </description>
+ <entry name="custom" value="0x1000"
+ summary="indicates this is the custom mode"/>
+ <entry name="del" value="0x2000"
+ summary="indicates the mode will be deleted"/>
+ </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"/>
+ <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"/>
+ </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.
+ </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"/>
+ <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"/>
+ <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"/>
+ </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"/>
+ </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.
+ </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"/>
+ </event>
+ </interface>
+</protocol>
--
1.8.1.2
Pekka Paalanen
2014-03-28 15:00:19 UTC
Permalink
On Mon, 24 Mar 2014 19:39:13 +0800
Post by Quanxian Wang
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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </description>
+ <entry name="custom" value="0x1000"
+ summary="indicates this is the custom mode"/>
Only one custom mode ever possible? Per output?
Post by Quanxian Wang
+ <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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ <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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ <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.
Post by Quanxian Wang
+ <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?
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </event>
+ </interface>
+</protocol>
I'm still personally not quite sure in which direction to develop this
protocol with the custom modes thing.


Thanks,
pq
Wang, Quanxian
2014-03-31 02:48:09 UTC
Permalink
-----Original Message-----
From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
Sent: Friday, March 28, 2014 11:00 PM
To: Wang, Quanxian
Cc: wayland-devel at lists.freedesktop.org
Subject: Re: [PATCH V2 1/8] wesston: Add weston randr protocol
On Mon, 24 Mar 2014 19:39:13 +0800
Post by Quanxian Wang
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
+++++++++++++++++++++++++++++++++++++++++++++++++++
Post by Quanxian Wang
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 (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
Post by Quanxian Wang
+ SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
AND
Post by Quanxian Wang
+ 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
Post by Quanxian Wang
+ 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
Post by Quanxian Wang
+ 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.
[Wang, Quanxian] Currently I delete randr_start interface, and will return callback in randr_commit.
We support multiple outputs now. But the return results will base on every output and will send result to user who are interested with that one by one.
With output argument in callback, It will tell user what operation on that and what results is for every operations.

For example: 'run this complex randr command', it will return the a little detailed information that what is be done? What is error? What is not done? It will be helpful for client to check their operations.
weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
output:HDMI3
TRANSFORM:SUCCESS!
MODE:SUCCESS!
SCALE:SUCCESS!
MOVE:Same as current, no change!
NEWMODE:SUCCESS!
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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </description>
+ <entry name="custom" value="0x1000"
+ summary="indicates this is the custom mode"/>
Only one custom mode ever possible? Per output?
[Wang, Quanxian] right.
Post by Quanxian Wang
+ <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.
[Wang, Quanxian] If we want to add delete action, client should know that and update their process that some modes are deleted. Otherwise it will be in-consistent with compositor.
Custom flag will tell user, this mode is custom by client instead of from edid or hardware information. Why do I add here? because I am thinking if we should control the delete action, client could only delete the custom mode instead of the modes from hardware or EDID. Currently I will not add the flags in wl_output, because this weston randr will be only for QA and developers, admin to use. Weston randr should be a right place to be put in for special case.
Post by Quanxian Wang
+ </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"/>
Post by Quanxian Wang
+ <arg name="height" type="int" summary="height of the mode in
+hardware units"/>
- 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.
[Wang, Quanxian] I will expose two mode setting method. One is for mode timing with detailed information.
Another is the width, height, refresh parameter(first matched, first set).
Post by Quanxian Wang
+ <arg name="refresh" type="int" summary="vertical refresh rate in
mHz"/>
Post by Quanxian Wang
+ </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">
Post by Quanxian Wang
+ 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.
[Wang, Quanxian] Yes. Compositor doesn't support above or below. If we add it, do we add null function for that? When they are ready, we could put detailed content there?
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Sorry, it is changed now in my private branch. In next version, it will be updated. Sorry about that.
Post by Quanxian Wang
+ </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"/>
Post by Quanxian Wang
+ <arg name="height" type="int" summary="height of the mode in
hardware units"/>
Post by Quanxian Wang
+ <arg name="refresh" type="int" summary="vertical refresh rate in
mHz"/>
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Generally user will use mHz, but in mode timing from hardware, it uses kHz(for example, timing from edid), so we will provide the interface with user custom.
Before setting and showing them, we need to transfer them. (done in code for that)
Post by Quanxian Wang
+ <arg name="hdisplay" type="int"
+ summary="The number of pixel for one horticontal line in units
Hz"/>
Post by Quanxian Wang
+ <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.
[Wang, Quanxian] I will recheck that.
Post by Quanxian Wang
+ <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?
[Wang, Quanxian] Currently it is changed to flags. The flags will contain vsync, hsync, csync, and others. If client input the char parameters, we will translate them into flags and then talk with weston randr.
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] It has been deleted. But callback done event will have.
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.
[Wang, Quanxian] yes, done.
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Reuse the words and make a little change. If you think it is not right here, I will write them again using my words. Anyway, I like official words accepted by developers, it will have less syntax error.:)
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] No, it is for all operations. Flag will contain all the actions you have done from weston-randr. The result will contain all the results of actions you have done.
Still the same example:

Command: (transform, scale, set mode, move, newmode, ...)
weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'

Results: (it will return the result in one commit, done event is received by user, user could parse the result based on the weston randr protocol)
output:HDMI3
TRANSFORM:SUCCESS!
MODE:SUCCESS!
SCALE:SUCCESS!
MOVE:Same as current, no change!
NEWMODE:SUCCESS!
Post by Quanxian Wang
+ </event>
+ </interface>
+</protocol>
I'm still personally not quite sure in which direction to develop this protocol with
the custom modes thing.
[Wang, Quanxian]
Custom mode, my understanding is that it should be the same format as mode from EDID or hardware information. For example new mode, client should input the right timing parameters to help compositor create a custom mode.
We have defined that in weston randr protocol. Client should clearly know that what every field of timing means and then custom the mode. After that, the new mode should act as the mode gotten from hardware.
The mode format will be standard for xserver or others display server. If the format provided by client is not right, we will tell them it is invalid.

For example, the following parameters list. It stands for "clock(mHz), hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start, vsync_end, vtotal, flags string".
newmode='105,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
Thanks,
pq
Pekka Paalanen
2014-03-31 07:13:15 UTC
Permalink
On Mon, 31 Mar 2014 02:48:09 +0000
Post by Wang, Quanxian
-----Original Message-----
From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
Sent: Friday, March 28, 2014 11:00 PM
To: Wang, Quanxian
Cc: wayland-devel at lists.freedesktop.org
Subject: Re: [PATCH V2 1/8] wesston: Add weston randr protocol
On Mon, 24 Mar 2014 19:39:13 +0800
Post by Quanxian Wang
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
+++++++++++++++++++++++++++++++++++++++++++++++++++
Post by Quanxian Wang
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 (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
Post by Quanxian Wang
+ SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
AND
Post by Quanxian Wang
+ 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
Post by Quanxian Wang
+ 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
Post by Quanxian Wang
+ 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.
[Wang, Quanxian] Currently I delete randr_start interface, and will return callback in randr_commit.
We support multiple outputs now. But the return results will base on every output and will send result to user who are interested with that one by one.
With output argument in callback, It will tell user what operation on that and what results is for every operations.
For example: 'run this complex randr command', it will return the a little detailed information that what is be done? What is error? What is not done? It will be helpful for client to check their operations.
weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
output:HDMI3
TRANSFORM:SUCCESS!
MODE:SUCCESS!
SCALE:SUCCESS!
MOVE:Same as current, no change!
NEWMODE:SUCCESS!
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.
Post by Quanxian Wang
+ </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.
Post by Quanxian Wang
+ </description>
+ <entry name="custom" value="0x1000"
+ summary="indicates this is the custom mode"/>
Only one custom mode ever possible? Per output?
[Wang, Quanxian] right.
Post by Quanxian Wang
+ <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.
[Wang, Quanxian] If we want to add delete action, client should know that and update their process that some modes are deleted. Otherwise it will be in-consistent with compositor.
Custom flag will tell user, this mode is custom by client instead of from edid or hardware information. Why do I add here? because I am thinking if we should control the delete action, client could only delete the custom mode instead of the modes from hardware or EDID. Currently I will not add the flags in wl_output, because this weston randr will be only for QA and developers, admin to use. Weston randr should be a right place to be put in for special case.
Post by Quanxian Wang
+ </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"/>
Post by Quanxian Wang
+ <arg name="height" type="int" summary="height of the mode in
+hardware units"/>
- 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.
[Wang, Quanxian] I will expose two mode setting method. One is for mode timing with detailed information.
Another is the width, height, refresh parameter(first matched, first set).
Post by Quanxian Wang
+ <arg name="refresh" type="int" summary="vertical refresh rate in
mHz"/>
Post by Quanxian Wang
+ </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">
Post by Quanxian Wang
+ 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.
[Wang, Quanxian] Yes. Compositor doesn't support above or below. If we add it, do we add null function for that? When they are ready, we could put detailed content there?
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Sorry, it is changed now in my private branch. In next version, it will be updated. Sorry about that.
Post by Quanxian Wang
+ </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"/>
Post by Quanxian Wang
+ <arg name="height" type="int" summary="height of the mode in
hardware units"/>
Post by Quanxian Wang
+ <arg name="refresh" type="int" summary="vertical refresh rate in
mHz"/>
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Generally user will use mHz, but in mode timing from hardware, it uses kHz(for example, timing from edid), so we will provide the interface with user custom.
Before setting and showing them, we need to transfer them. (done in code for that)
Post by Quanxian Wang
+ <arg name="hdisplay" type="int"
+ summary="The number of pixel for one horticontal line in units
Hz"/>
Post by Quanxian Wang
+ <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.
[Wang, Quanxian] I will recheck that.
Post by Quanxian Wang
+ <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?
[Wang, Quanxian] Currently it is changed to flags. The flags will contain vsync, hsync, csync, and others. If client input the char parameters, we will translate them into flags and then talk with weston randr.
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] It has been deleted. But callback done event will have.
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.
[Wang, Quanxian] yes, done.
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] Reuse the words and make a little change. If you think it is not right here, I will write them again using my words. Anyway, I like official words accepted by developers, it will have less syntax error.:)
Post by Quanxian Wang
+ </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.
[Wang, Quanxian] No, it is for all operations. Flag will contain all the actions you have done from weston-randr. The result will contain all the results of actions you have done.
Command: (transform, scale, set mode, move, newmode, ...)
weston-wrandr --output HDMI3 --transform 1 --scale 2 --mode 2 --rightof VGA1 --newmode='1000,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
Results: (it will return the result in one commit, done event is received by user, user could parse the result based on the weston randr protocol)
output:HDMI3
TRANSFORM:SUCCESS!
MODE:SUCCESS!
SCALE:SUCCESS!
MOVE:Same as current, no change!
NEWMODE:SUCCESS!
Well, I guess that could be interesting somehow, but I think even for
the kernel modesetting interfaces, it is not yet clear what should be
communicated on a failure. At least I haven't seen an explicit
discussion about that, although it could also be already in the patches
for all I know. You should prepare for the case, where the platform/OS
APIs do not tell the server in sufficient detail about what failed, and
write guidelines on what should be reported in that case to the Wayland
client. Or at least keep that in mind when developing this. That is the
reason why I suggested just a free-form human readable string as the
"why" argument.

What happens if only some part of the operations fail? Do you then
abort/revert everything, or do you let the succeeded changes stay?

FWIW, I think it should be all or nothing. If even one bit fails, then
the server should revert to the original state as if the requests never
happened, and communicate the failure to the client.
Post by Wang, Quanxian
Post by Quanxian Wang
+ </event>
+ </interface>
+</protocol>
I'm still personally not quite sure in which direction to develop this protocol with
the custom modes thing.
[Wang, Quanxian]
Custom mode, my understanding is that it should be the same format as mode from EDID or hardware information. For example new mode, client should input the right timing parameters to help compositor create a custom mode.
We have defined that in weston randr protocol. Client should clearly know that what every field of timing means and then custom the mode. After that, the new mode should act as the mode gotten from hardware.
The mode format will be standard for xserver or others display server. If the format provided by client is not right, we will tell them it is invalid.
For example, the following parameters list. It stands for "clock(mHz), hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start, vsync_end, vtotal, flags string".
newmode='105,300,100,120,150,400,350,370,399,+hsync -vsync csync +csync -csync'
You are right for a real modeline for a real monitor case.

I meant more like the two very different uses cases: arbitrary size on
"virtual monitor" vs. a real modeline on a real monitor. Do we need to
cater for both? Can they both be sufficiently supported by the real
modeline on a real monitor approach?

I would hope to forget the virtual case here.


Thanks,
pq
Quanxian Wang
2014-03-24 11:39:14 UTC
Permalink
1) Add weston_randr definition and randr_backend intreface.
2) Export functions used in compositor.c so that module wrandr
could use them. For example, weston_output_transform_scale_init.
3) Support new_mode backend interface in output structure.

Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
src/compositor.h | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/src/compositor.h b/src/compositor.h
index 22a485f..19d0f7b 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -172,6 +172,17 @@ enum weston_mode_switch_op {
WESTON_MODE_SWITCH_RESTORE_NATIVE
};

+struct wrandr {
+ struct weston_compositor *compositor;
+ struct wl_global *global;
+ struct wl_resource *resource;
+ struct wl_listener destroy_listener;
+ struct {
+ struct wl_list request_list;
+ struct wl_list randr_callback_list;
+ } pending;
+};
+
struct weston_output {
uint32_t id;
char *name;
@@ -218,6 +229,18 @@ struct weston_output {
void (*destroy)(struct weston_output *output);
void (*assign_planes)(struct weston_output *output);
int (*switch_mode)(struct weston_output *output, struct weston_mode *mode);
+ struct weston_mode * (*new_mode)(struct weston_output *output,
+ int fclock,
+ int hdisplay,
+ int hsync_start,
+ int hsync_end,
+ int htotal,
+ int vdisplay,
+ int vsync_start,
+ int vsync_end,
+ int vtotal,
+ const char *hsync,
+ const char *vsync);

/* backlight values are on 0-255 range, where higher is brighter */
int32_t backlight_current;
@@ -632,6 +655,7 @@ struct weston_compositor {

/* Raw keyboard processing (no libxkbcommon initialization or handling) */
int use_xkbcommon;
+ struct wrandr *randr;
};

struct weston_buffer {
@@ -1356,6 +1380,10 @@ weston_surface_set_color(struct weston_surface *surface,
void
weston_surface_destroy(struct weston_surface *surface);

+void
+weston_output_transform_scale_init(struct weston_output *output,
+ uint32_t transform, uint32_t scale);
+
int
weston_output_switch_mode(struct weston_output *output, struct weston_mode *mode,
int32_t scale, enum weston_mode_switch_op op);
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:15 UTC
Permalink
When starting weston with parameter --enable-wrandr,
it will automatically load wrandr.so module. This is
for QA testing and Admin configuration.

Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
src/compositor.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index c031edc..0d37ab8 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -88,7 +88,7 @@ sigchld_handler(int signal_number, void *data)
return 1;
}

-static void
+WL_EXPORT void
weston_output_transform_scale_init(struct weston_output *output,
uint32_t transform, uint32_t scale);

@@ -3297,7 +3297,7 @@ weston_output_update_matrix(struct weston_output *output)
output->dirty = 0;
}

-static void
+WL_EXPORT void
weston_output_transform_scale_init(struct weston_output *output, uint32_t transform, uint32_t scale)
{
output->transform = transform;
@@ -4140,6 +4140,7 @@ int main(int argc, char *argv[])
char *log = NULL;
int32_t idle_time = 300;
int32_t help = 0;
+ int32_t enable_wrandr = 0;
char *socket_name = "wayland-0";
int32_t version = 0;
struct weston_config *config;
@@ -4151,6 +4152,7 @@ int main(int argc, char *argv[])
{ WESTON_OPTION_STRING, "socket", 'S', &socket_name },
{ WESTON_OPTION_INTEGER, "idle-time", 'i', &idle_time },
{ WESTON_OPTION_STRING, "modules", 0, &option_modules },
+ { WESTON_OPTION_STRING, "enable-wrandr", 0, &enable_wrandr},
{ WESTON_OPTION_STRING, "log", 0, &log },
{ WESTON_OPTION_BOOLEAN, "help", 'h', &help },
{ WESTON_OPTION_BOOLEAN, "version", 0, &version },
@@ -4233,6 +4235,14 @@ int main(int argc, char *argv[])
ec->idle_time = idle_time;
ec->default_pointer_grab = NULL;

+ /* Add wrandr modules */
+ if (enable_wrandr) {
+ if (!option_modules)
+ option_modules = strdup("wrandr.so");
+ else
+ option_modules = strcat(option_modules, ",wrandr.so");
+ }
+
setenv("WAYLAND_DISPLAY", socket_name, 1);

if (option_shell)
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:16 UTC
Permalink
Provide drm_output_new_mode interface to create new mode
from outsite instead of only from edid or configure.

Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
src/compositor-drm.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 154e15e..577d02f 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -1389,6 +1389,72 @@ drm_output_add_mode(struct drm_output *output, drmModeModeInfo *info)
return mode;
}

+static struct weston_mode *
+drm_output_new_mode(struct weston_output *output,
+ int fclock,
+ int hdisplay,
+ int hsync_start,
+ int hsync_end,
+ int htotal,
+ int vdisplay,
+ int vsync_start,
+ int vsync_end,
+ int vtotal,
+ const char *hsync,
+ const char *vsync)
+{
+ drmModeModeInfo *modeinfo;
+ struct drm_mode *mode = NULL;
+
+ modeinfo = malloc(sizeof(*modeinfo));
+ if (modeinfo == NULL)
+ return NULL;
+ memset(modeinfo, 0x0, sizeof(*modeinfo));
+
+ modeinfo->type = DRM_MODE_TYPE_USERDEF;
+ modeinfo->hskew = 0;
+ modeinfo->vscan = 0;
+ modeinfo->vrefresh = 0;
+ modeinfo->flags = 0;
+
+ modeinfo->hdisplay = hdisplay;
+ modeinfo->hsync_start = hsync_start;
+ modeinfo->hsync_end = hsync_end;
+ modeinfo->htotal = htotal;
+ modeinfo->vdisplay = vdisplay;
+ modeinfo->vsync_start = vsync_start;
+ modeinfo->vsync_end = vsync_end;
+ modeinfo->vtotal = vtotal;
+
+ modeinfo->clock = fclock * 1000;
+
+ if (strcmp(hsync, "+hsync") == 0)
+ modeinfo->flags |= DRM_MODE_FLAG_PHSYNC;
+ else if (strcmp(hsync, "-hsync") == 0)
+ modeinfo->flags |= DRM_MODE_FLAG_NHSYNC;
+ else {
+ weston_log("Invalid hsync parameter %s.\n", hsync);
+ free(modeinfo);
+ return NULL;
+ }
+
+ if (strcmp(vsync, "+vsync") == 0)
+ modeinfo->flags |= DRM_MODE_FLAG_PVSYNC;
+ else if (strcmp(vsync, "-vsync") == 0)
+ modeinfo->flags |= DRM_MODE_FLAG_NVSYNC;
+ else {
+ weston_log("Invalid vsync parameter %s.\n", vsync);
+ free(modeinfo);
+ return NULL;
+ }
+
+ mode = drm_output_add_mode((struct drm_output *)output, modeinfo);
+ if (mode)
+ return &mode->base;
+ else
+ return NULL;
+}
+
static int
drm_subpixel_to_wayland(int drm_value)
{
@@ -2046,6 +2112,7 @@ create_output_for_connector(struct drm_compositor *ec,
output->base.assign_planes = drm_assign_planes;
output->base.set_dpms = drm_set_dpms;
output->base.switch_mode = drm_output_switch_mode;
+ output->base.new_mode = drm_output_new_mode;

output->base.gamma_size = output->original_crtc->gamma_size;
output->base.set_gamma = drm_output_set_gamma;
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:17 UTC
Permalink
Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
module/Makefile.am | 3 +
module/wrandr/Makefile.am | 32 ++
module/wrandr/wrandr.c | 792 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 827 insertions(+)
create mode 100644 module/Makefile.am
create mode 100644 module/wrandr/Makefile.am
create mode 100644 module/wrandr/wrandr.c

diff --git a/module/Makefile.am b/module/Makefile.am
new file mode 100644
index 0000000..1a10e65
--- /dev/null
+++ b/module/Makefile.am
@@ -0,0 +1,3 @@
+SUBDIRS =
+
+SUBDIRS += wrandr
diff --git a/module/wrandr/Makefile.am b/module/wrandr/Makefile.am
new file mode 100644
index 0000000..b0f2e6b
--- /dev/null
+++ b/module/wrandr/Makefile.am
@@ -0,0 +1,32 @@
+moduledir = $(libdir)/weston
+module_LTLIBRARIES = $(wrandr)
+
+AM_CPPFLAGS = \
+ -I$(top_srcdir)/shared \
+ -I$(top_srcdir)/src \
+ -I$(top_builddir)/src \
+ -DDATADIR='"$(datadir)"' \
+ -DMODULEDIR='"$(moduledir)"' \
+ -DLIBEXECDIR='"$(libexecdir)"' \
+ -DIN_WESTON
+
+if BUILD_WRANDR
+wrandr = wrandr.la
+wrandr_la_LDFLAGS = -module -avoid-version
+wrandr_la_LIBADD = $(COMPOSITOR_LIBS) \
+ $(top_srcdir)/shared/libshared.la
+wrandr_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
+wrandr_la_SOURCES = \
+ wrandr.c \
+ randr-protocol.c \
+ randr-server-protocol.h
+endif
+
+BUILT_SOURCES = \
+ randr-protocol.c \
+ randr-server-protocol.h
+
+CLEANFILES = $(BUILT_SOURCES)
+
+wayland_protocoldir = $(top_srcdir)/protocol
+include $(top_srcdir)/wayland-scanner.mk
diff --git a/module/wrandr/wrandr.c b/module/wrandr/wrandr.c
new file mode 100644
index 0000000..b97d101
--- /dev/null
+++ b/module/wrandr/wrandr.c
@@ -0,0 +1,792 @@
+/*
+ * Copyright ? 2012-2013 Collabora, Ltd.
+
+ * 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.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+
+#include "compositor.h"
+#include "randr-server-protocol.h"
+
+#define RESULT_MASK 0x3
+
+struct randr_request {
+ struct wl_list link;
+ int flags;
+ struct wl_resource *callback;
+
+ /* Client and output compose unique id
+ * of these request.*/
+ struct wl_client *client;
+ struct weston_output *output;
+
+ /* output move */
+ struct weston_output *loutput;
+ struct weston_output *routput;
+
+ /* mode set */
+ int height;
+ int width;
+ int refresh;
+
+ int scale;
+ int32_t transform;
+
+ struct {
+ int clock;
+ int hdisplay;
+ int hsync_start;
+ int hsync_end;
+ int htotal;
+ int vdisplay;
+ int vsync_start;
+ int vsync_end;
+ int vtotal;
+ char *hsync;
+ char *vsync;
+ } newmode;
+
+ struct {
+ int height;
+ int width;
+ int refresh;
+ } delmode;
+};
+
+static void
+del_request(struct randr_request *request)
+{
+ wl_list_remove(&request->link);
+
+ if (request->callback)
+ wl_resource_destroy(request->callback);
+
+ if (request->newmode.hsync)
+ free(request->newmode.hsync);
+
+ if (request->newmode.vsync)
+ free(request->newmode.vsync);
+
+ free(request);
+}
+
+static struct randr_request *
+get_request(struct wl_resource *resource,
+ struct wl_client *client,
+ struct weston_output *output,
+ int create)
+{
+ struct wrandr *randr =
+ wl_resource_get_user_data(resource);
+ struct randr_request *request;
+
+ wl_list_for_each(request, &randr->pending.request_list, link)
+ {
+ if (request->client == client &&
+ request->output == output)
+ return request;
+ }
+
+ if (create) {
+ request = zalloc(sizeof(*request));
+ if (request == NULL) {
+ wl_resource_post_no_memory(resource);
+ return NULL;
+ }
+ memset(request, 0x0, sizeof(*request));
+
+ request->client = client;
+ request->output = output;
+ wl_list_insert(&randr->pending.request_list, &request->link);
+ return request;
+ } else
+ return NULL;
+}
+
+static void
+adjust_pointer(struct weston_output *output,
+ pixman_region32_t *old_output_region)
+{
+ struct weston_seat *seat;
+ int32_t x, y;
+ struct weston_pointer *pointer;
+
+ /* If a pointer falls outside the outputs new geometry, move it to its
+ * lower-right corner. */
+ wl_list_for_each(seat, &output->compositor->seat_list, link) {
+ pointer = seat->pointer;
+ if (!pointer)
+ continue;
+
+ x = wl_fixed_to_int(pointer->x);
+ y = wl_fixed_to_int(pointer->y);
+
+ if (!pixman_region32_contains_point(old_output_region,
+ x, y, NULL) ||
+ pixman_region32_contains_point(&output->region,
+ x, y, NULL))
+ continue;
+
+ if (x >= output->x + output->width)
+ x = output->x + output->width - 1;
+ if (y >= output->y + output->height)
+ y = output->y + output->height - 1;
+
+ pointer->x = wl_fixed_from_int(x);
+ pointer->y = wl_fixed_from_int(y);
+ }
+}
+
+static void
+notify_change(struct weston_output *output,
+ int mode_changed,
+ int scale_changed,
+ int transform_changed)
+{
+ struct wl_resource *resource;
+ struct weston_mode *mode = output->current_mode;
+
+ if (!mode_changed && !scale_changed &&
+ !transform_changed)
+ return;
+
+ /* Notify clients of the changes */
+ wl_resource_for_each(resource,
+ &output->resource_list) {
+ if (mode_changed) {
+ wl_output_send_mode(resource,
+ mode->flags |
+ WL_OUTPUT_MODE_CURRENT,
+ mode->width,
+ mode->height,
+ mode->refresh);
+ }
+
+ if (transform_changed) {
+ wl_output_send_geometry(resource,
+ output->x,
+ output->y,
+ output->mm_width,
+ output->mm_height,
+ output->subpixel,
+ output->make,
+ output->model,
+ output->transform);
+ }
+
+ if (scale_changed &&
+ wl_resource_get_version(resource) >= 2)
+ wl_output_send_scale(resource,
+ output->current_scale);
+
+ if (wl_resource_get_version(resource) >= 2)
+ wl_output_send_done(resource);
+ }
+}
+
+static void
+randr_destroy(struct wl_client *client,
+ struct wl_resource *resource)
+{
+ wl_resource_destroy(resource);
+ return;
+}
+
+static struct weston_mode *
+randr_found_mode(struct weston_output *output,
+ int width, int height, int refresh)
+{
+ struct weston_mode *mode;
+
+ wl_list_for_each(mode, &output->mode_list, link) {
+ if (width != mode->width ||
+ height != mode->height)
+ continue;
+
+ if (!refresh || refresh == (int)mode->refresh)
+ return mode;
+ }
+ return NULL;
+}
+
+static void
+update_region(struct weston_compositor *ec,
+ struct weston_output *target_output)
+{
+ pixman_region32_t old_output_region;
+
+ /* Update output region and transformation matrix. */
+ pixman_region32_init(&old_output_region);
+ pixman_region32_copy(&old_output_region, &target_output->region);
+ weston_output_transform_scale_init(target_output,
+ target_output->transform,
+ target_output->current_scale);
+
+ pixman_region32_init(&target_output->previous_damage);
+ pixman_region32_init_rect(&target_output->region,
+ target_output->x, target_output->y,
+ target_output->width, target_output->height);
+
+ weston_output_update_matrix(target_output);
+
+ adjust_pointer(target_output, &old_output_region);
+ pixman_region32_fini(&old_output_region);
+
+ /* damage the output region in primary_plane */
+ pixman_region32_union(&ec->primary_plane.damage,
+ &ec->primary_plane.damage,
+ &target_output->region);
+
+ target_output->dirty = 1;
+}
+
+static int32_t
+randr_modeset(struct wrandr *randr,
+ struct randr_request *request)
+{
+ struct weston_compositor *ec = randr->compositor;
+ struct weston_output *output = NULL;
+ struct weston_mode *mode = NULL;
+ int move = 0, offset = 0;
+ uint32_t results = 0;
+ int32_t result = 0;
+ uint32_t mode_change = 0, trans_change = 0;
+ uint32_t scale_change = 0;
+ uint32_t move_change = 0;
+ struct weston_output *toutput = request->output;
+
+ /* Mode Setting Operation */
+ if (request->width > 0 && request->height > 0) {
+ mode = randr_found_mode(toutput,
+ request->width,
+ request->height,
+ request->refresh);
+
+ if (!mode)
+ result = WRANDR_CALLBACK_RESULT_FAIL;
+ else {
+ if (!(mode->flags & WL_OUTPUT_MODE_CURRENT) &&
+ toutput->switch_mode) {
+ result = toutput->switch_mode(toutput, mode);
+ if (result < 0)
+ result = WRANDR_CALLBACK_RESULT_FAIL;
+ else
+ result = WRANDR_CALLBACK_RESULT_SUCCESS;
+ } else
+ result = WRANDR_CALLBACK_RESULT_NOACT;
+ }
+
+ results |= result<<(WRANDR_CALLBACK_ACTION_MODE * 2);
+ }
+
+ /* Transform Operation */
+ if (request->transform >= 0) {
+ if ((uint32_t)request->transform != toutput->transform) {
+ toutput->transform = (uint32_t)request->transform;
+ result = WRANDR_CALLBACK_RESULT_SUCCESS;
+ } else
+ result = WRANDR_CALLBACK_RESULT_NOACT;
+
+ results |= result<<(WRANDR_CALLBACK_ACTION_TRANSFORM * 2);
+ }
+
+ /* Scale Operation */
+ if (request->scale > 0) {
+ if (request->scale && request->scale !=
+ toutput->current_scale) {
+ toutput->current_scale = request->scale;
+ result = WRANDR_CALLBACK_RESULT_SUCCESS;
+ } else
+ result = WRANDR_CALLBACK_RESULT_NOACT;
+
+ results |= result<<(WRANDR_CALLBACK_ACTION_SCALE * 2);
+ }
+
+ /* Move Operation */
+ if (request->loutput || request->routput) {
+ if (request->loutput == toutput ||
+ request->routput == toutput)
+ result = WRANDR_CALLBACK_RESULT_FAIL;
+ else {
+ if (request->loutput &&
+ &toutput->link !=
+ request->loutput->link.prev) {
+ wl_list_remove(&toutput->link);
+ wl_list_insert(request->loutput->link.prev,
+ &toutput->link);
+ move = 1;
+ }
+
+ if (request->routput &&
+ &toutput->link !=
+ request->routput->link.next) {
+ wl_list_remove(&toutput->link);
+ wl_list_insert(&request->routput->link,
+ &toutput->link);
+ move = 1;
+ }
+
+ if (move != 1)
+ result = WRANDR_CALLBACK_RESULT_NOACT;
+ else
+ result = WRANDR_CALLBACK_RESULT_SUCCESS;
+ }
+ results |= result<<(WRANDR_CALLBACK_ACTION_MOVE * 2);
+ }
+
+ /* Check if any action to be done. */
+ mode_change = (results>>(WRANDR_CALLBACK_ACTION_MODE * 2) &
+ RESULT_MASK) == WRANDR_CALLBACK_RESULT_SUCCESS;
+ scale_change = (results>>(WRANDR_CALLBACK_ACTION_SCALE * 2) &
+ RESULT_MASK) == WRANDR_CALLBACK_RESULT_SUCCESS;
+ trans_change = (results>>(WRANDR_CALLBACK_ACTION_TRANSFORM * 2) &
+ RESULT_MASK) == WRANDR_CALLBACK_RESULT_SUCCESS;
+ move_change = (results>>(WRANDR_CALLBACK_ACTION_MOVE * 2) &
+ RESULT_MASK) == WRANDR_CALLBACK_RESULT_SUCCESS;
+
+ if (!move_change && !scale_change &&
+ !trans_change && !mode_change)
+ return results;
+
+ update_region(randr->compositor, toutput);
+
+ /* Adjust all outputs. */
+ wl_list_for_each(output, &ec->output_list, link) {
+ weston_output_move(output, offset, 0);
+ offset += output->width;
+ pixman_region32_union(&randr->compositor->primary_plane.damage,
+ &randr->compositor->primary_plane.damage,
+ &output->region);
+ }
+
+ /* Notify clients of the changes. */
+ notify_change(toutput, mode_change,
+ scale_change, trans_change);
+
+ return results;
+}
+
+static void
+randr_set_scale(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *output_resource,
+ int32_t scale)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(output_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_SCALE;
+ request->scale = scale;
+ }
+}
+
+static void
+randr_set_transform(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *output_resource,
+ uint32_t transform)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(output_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_TRANSFORM;
+ request->transform = transform % 8;
+ }
+}
+
+static void
+randr_start(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *output_resource,
+ uint32_t callback)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(output_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (!request) {
+ wl_resource_post_no_memory(resource);
+ return;
+ }
+
+ request->callback =
+ wl_resource_create(client, &wrandr_callback_interface, 1,
+ callback);
+ if (request->callback == NULL) {
+ free(request);
+ wl_resource_post_no_memory(request->callback);
+ return;
+ }
+}
+
+static void
+randr_set_mode(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *output_resource,
+ int width, int height, int refresh)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(output_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_MODE;
+ request->width = width;
+ request->height = height;
+ request->refresh = refresh;
+ }
+}
+
+static void
+randr_move(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *target_resource,
+ struct wl_resource *source_resource, int move)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(target_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_MOVE;
+ if (move == WESTON_RANDR_MOVE_LEFTOF) {
+ request->loutput =
+ wl_resource_get_user_data(source_resource);
+ } else {
+ request->routput =
+ wl_resource_get_user_data(source_resource);
+ }
+ }
+}
+
+static int
+output_newmode(struct weston_output *output,
+ int clock,
+ int hdisplay,
+ int hsync_start,
+ int hsync_end,
+ int htotal,
+ int vdisplay,
+ int vsync_start,
+ int vsync_end,
+ int vtotal,
+ const char *hsync,
+ const char *vsync)
+{
+ struct weston_mode *mode = NULL;
+ struct wl_resource *resource;
+ uint64_t refresh;
+
+ if ((strcmp(hsync, "+hsync") != 0 &&
+ strcmp(hsync, "-hsync") != 0) ||
+ (strcmp(vsync, "+vsync") != 0 &&
+ strcmp(vsync, "-vsync") != 0)) {
+ weston_log("Invalid hsync format:%s.\n", hsync);
+ return WRANDR_CALLBACK_RESULT_FAIL;
+ }
+
+ /* Calculate higher precision (mHz) refresh rate */
+ refresh = (clock * 1000 * 1000000LL / htotal +
+ vtotal / 2) / vtotal;
+
+ /* Same width, height and refresh in mode list */
+ mode = randr_found_mode(output, hdisplay, vdisplay, refresh);
+ if (mode)
+ return WRANDR_CALLBACK_RESULT_NOACT;
+
+ if (output->new_mode) {
+ mode = output->new_mode(output,
+ clock,
+ hdisplay,
+ hsync_start,
+ hsync_end,
+ htotal,
+ vdisplay,
+ vsync_start,
+ vsync_end,
+ vtotal,
+ hsync,
+ vsync);
+ if (mode) {
+ mode->flags |= WESTON_RANDR_MODE_CUSTOM;
+ /* Notify clients of the change. */
+ wl_resource_for_each(resource,
+ &output->resource_list) {
+ wl_output_send_mode(resource,
+ mode->flags,
+ mode->width,
+ mode->height,
+ mode->refresh);
+ if (wl_resource_get_version(resource) >= 2)
+ wl_output_send_done(resource);
+ }
+ return WRANDR_CALLBACK_RESULT_SUCCESS;
+ }
+ }
+
+ return WRANDR_CALLBACK_RESULT_FAIL;
+}
+
+static void
+randr_newmode(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *target_resource,
+ int clock,
+ int hdisplay,
+ int hsync_start,
+ int hsync_end,
+ int htotal,
+ int vdisplay,
+ int vsync_start,
+ int vsync_end,
+ int vtotal,
+ const char *hsync,
+ const char *vsync)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(target_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_NEWMODE;
+ request->newmode.clock = clock;
+ request->newmode.hdisplay = hdisplay;
+ request->newmode.hsync_start = hsync_start;
+ request->newmode.hsync_end = hsync_end;
+ request->newmode.htotal = htotal;
+ request->newmode.vdisplay = vdisplay;
+ request->newmode.vsync_start = vsync_start;
+ request->newmode.vsync_end = vsync_end;
+ request->newmode.vtotal = vtotal;
+ request->newmode.hsync = strdup(hsync);
+ request->newmode.vsync = strdup(vsync);
+ }
+}
+
+static int
+output_delmode(struct weston_output *output,
+ int width,
+ int height,
+ int refresh)
+{
+ struct wl_resource *resource;
+ struct weston_mode *mode =
+ randr_found_mode(output, width, height, refresh);
+
+ if (!mode)
+ return WRANDR_CALLBACK_RESULT_FAIL;
+ else {
+ mode->flags |= WESTON_RANDR_MODE_DEL;
+ /* Notify clients of the change. */
+ wl_resource_for_each(resource, &output->resource_list) {
+ wl_output_send_mode(resource,
+ mode->flags,
+ mode->width,
+ mode->height,
+ mode->refresh);
+ if (wl_resource_get_version(resource) >= 2)
+ wl_output_send_done(resource);
+ }
+
+ wl_list_remove(&mode->link);
+ free(mode);
+ return WRANDR_CALLBACK_RESULT_SUCCESS;
+ }
+}
+
+static void
+randr_delmode(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *target_resource,
+ int width,
+ int height,
+ int refresh)
+{
+ struct weston_output *output =
+ wl_resource_get_user_data(target_resource);
+ struct randr_request *request;
+
+ request = get_request(resource, client, output, 1);
+ if (request) {
+ request->flags |= 1<<WRANDR_CALLBACK_ACTION_DELMODE;
+ request->delmode.width = width;
+ request->delmode.height = height;
+ request->delmode.refresh = refresh;
+ }
+}
+
+static void
+randr_commit(struct wl_client *client,
+ struct wl_resource *resource,
+ struct wl_resource *target_resource)
+{
+ struct wrandr *randr = wl_resource_get_user_data(resource);
+ struct weston_output *output =
+ wl_resource_get_user_data(target_resource);
+ struct randr_request *request;
+ int results = 0;
+ int result = 0;
+
+ /* process pending request for this client */
+ request = get_request(resource, client, output, 0);
+ if (!request) {
+ weston_log("No action happens when commit?!\n");
+ } else {
+ if (request->flags & 1<<WRANDR_CALLBACK_ACTION_NEWMODE) {
+ result = output_newmode(output,
+ request->newmode.clock,
+ request->newmode.hdisplay,
+ request->newmode.hsync_start,
+ request->newmode.hsync_end,
+ request->newmode.htotal,
+ request->newmode.vdisplay,
+ request->newmode.vsync_start,
+ request->newmode.vsync_end,
+ request->newmode.vtotal,
+ request->newmode.hsync,
+ request->newmode.vsync);
+ results |= result<<(WRANDR_CALLBACK_ACTION_NEWMODE * 2);
+ }
+
+ if (request->flags & 1<<WRANDR_CALLBACK_ACTION_DELMODE) {
+ result = output_delmode(output,
+ request->delmode.width,
+ request->delmode.height,
+ request->delmode.refresh);
+ results |= result<<(WRANDR_CALLBACK_ACTION_DELMODE * 2);
+ }
+
+ if (request->width || request->height ||
+ request->scale || request->transform >= 0 ||
+ request->loutput || request->routput)
+ results |= randr_modeset(randr, request);
+ }
+
+ wrandr_callback_send_done(request->callback,
+ request->flags, results);
+ del_request(request);
+}
+
+static const struct weston_randr_interface randr_interface = {
+ randr_destroy,
+ randr_start,
+ randr_set_mode,
+ randr_set_transform,
+ randr_set_scale,
+ randr_move,
+ randr_delmode,
+ randr_newmode,
+ randr_commit
+};
+
+static void
+bind_randr(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct wrandr *randr = data;
+
+ randr->resource = wl_resource_create(client,
+ &weston_randr_interface,
+ 1, id);
+
+ if (randr->resource == NULL) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+
+ wl_resource_set_implementation(randr->resource,
+ &randr_interface,
+ randr, NULL);
+}
+
+static void
+handle_randr_destroy(struct wl_listener *listener, void *data)
+{
+ struct wrandr *randr =
+ container_of(listener, struct wrandr, destroy_listener);
+ struct randr_request *request;
+
+ if (randr) {
+ if (randr->global)
+ wl_global_destroy(randr->global);
+
+ wl_list_for_each(request, &randr->pending.request_list, link)
+ del_request(request);
+
+ free(randr);
+ }
+}
+
+static void
+weston_randr_init(struct weston_compositor *ec)
+{
+ /* Initialize randr */
+ ec->randr = zalloc(sizeof *(ec->randr));
+ if (!ec->randr) {
+ weston_log("No memory for wrandr in backend compositor.\n");
+ return;
+ }
+ memset(ec->randr, 0, sizeof *(ec->randr));
+
+ ec->randr->compositor = ec;
+
+ ec->randr->global = wl_global_create(ec->wl_display,
+ &weston_randr_interface, 1,
+ ec->randr, bind_randr);
+
+ if (ec->randr->global == NULL) {
+ weston_log("Failed to create global weston_randr_interface.\n");
+ return;
+ }
+
+ wl_list_init(&ec->randr->pending.request_list);
+ wl_list_init(&ec->randr->pending.randr_callback_list);
+}
+
+WL_EXPORT int
+module_init(struct weston_compositor *ec,
+ int *argc, char *argv[])
+{
+ weston_randr_init(ec);
+ if (!ec->randr) {
+ weston_log("Failed to initialize wrandr module.\n");
+ return -1;
+ }
+ ec->randr->destroy_listener.notify = handle_randr_destroy;
+ wl_signal_add(&ec->destroy_signal, &ec->randr->destroy_listener);
+
+ return 0;
+}
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:18 UTC
Permalink
Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
Makefile.am | 5 +++++
configure.ac | 8 ++++++++
2 files changed, 13 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index f22c542..254cde7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -6,11 +6,16 @@ if ENABLE_XWAYLAND
xwayland_subdir = xwayland
endif

+if BUILD_WRANDR
+randr_dir = module/wrandr
+endif
+
SUBDIRS = \
shared \
src \
$(xwayland_subdir) \
desktop-shell \
+ $(randr_dir) \
clients \
data \
protocol \
diff --git a/configure.ac b/configure.ac
index cce1850..01e7429 100644
--- a/configure.ac
+++ b/configure.ac
@@ -409,6 +409,12 @@ if test "x$enable_dbus" != "xno"; then
fi
AM_CONDITIONAL(ENABLE_DBUS, test "x$enable_dbus" = "xyes")

+AC_ARG_ENABLE(wrandr, [ --disable-wrandr],, enable_wrandr=no)
+AM_CONDITIONAL(BUILD_WRANDR, test x$enable_wrandr = xyes)
+if test x$enable_wrandr = xyes; then
+ AC_DEFINE([BUILD_WRANDR], [1], [Build the wrandr])
+fi
+
AC_ARG_ENABLE(wcap-tools, [ --disable-wcap-tools],, enable_wcap_tools=yes)
AM_CONDITIONAL(BUILD_WCAP_TOOLS, test x$enable_wcap_tools = xyes)
if test x$enable_wcap_tools = xyes; then
@@ -498,6 +504,8 @@ AC_CONFIG_FILES([Makefile
src/Makefile
xwayland/Makefile
desktop-shell/Makefile
+ module/Makefile
+ module/wrandr/Makefile
src/version.h
src/weston.pc
clients/Makefile
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:19 UTC
Permalink
Functions implemented in this application
1) Query output mode list
2) Update properties of output (transform, scale, mode setting)
3) Position of output (leftof and rightof are supported.)
4) Custom a mode for output.
5) Delete mode of output.
6) Combination of above 2-5 in one shot.

Note:
Does not support operations on multiple outputs in one time.

More details, please run "weston-wrandr -h"

Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
clients/Makefile.am | 9 +
clients/wrandr.c | 862 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 871 insertions(+)
create mode 100644 clients/wrandr.c

diff --git a/clients/Makefile.am b/clients/Makefile.am
index 4f8d4a6..757dba3 100644
--- a/clients/Makefile.am
+++ b/clients/Makefile.am
@@ -60,6 +60,7 @@ libexec_PROGRAMS = \
weston-desktop-shell \
weston-screenshooter \
$(screensaver) \
+ weston-wrandr \
weston-keyboard \
weston-simple-im

@@ -101,6 +102,12 @@ libtoytoolkit_la_LIBADD = \
weston_flower_SOURCES = flower.c
weston_flower_LDADD = libtoytoolkit.la

+weston_wrandr_SOURCES = \
+ wrandr.c \
+ randr-protocol.c \
+ randr-client-protocol.h
+weston_wrandr_LDADD = libtoytoolkit.la $(CLIENT_LIBS)
+
weston_screenshooter_SOURCES = \
screenshot.c \
screenshooter-protocol.c \
@@ -211,6 +218,8 @@ BUILT_SOURCES = \
text-cursor-position-protocol.c \
text-protocol.c \
text-client-protocol.h \
+ randr-protocol.c \
+ randr-client-protocol.h \
input-method-protocol.c \
input-method-client-protocol.h \
desktop-shell-client-protocol.h \
diff --git a/clients/wrandr.c b/clients/wrandr.c
new file mode 100644
index 0000000..86e58a1
--- /dev/null
+++ b/clients/wrandr.c
@@ -0,0 +1,862 @@
+/*
+ * 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.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <math.h>
+#include <assert.h>
+#include <signal.h>
+
+#include "randr-client-protocol.h"
+#include <wayland-client.h>
+#include <wayland-server.h>
+#include "../shared/config-parser.h"
+
+#ifndef HZ
+#define HZ 1000
+#endif
+
+#define ARRAY_LENGTH(a) (sizeof(a) / sizeof(a)[0])
+
+enum randr_status {
+ RANDR_EXIT = 0,
+ RANDR_COMMIT = 1,
+};
+
+static int running = 1;
+
+struct output;
+struct mode;
+
+struct randr {
+ struct wl_display *display;
+ struct wl_registry *registry;
+ struct wl_compositor *compositor;
+ struct weston_randr *randr;
+ struct wl_list output_list;
+ struct wl_output *loutput;
+ struct wl_output *routput;
+ struct output *output;
+ struct wl_output *wayland_output;
+ struct mode *delmode;
+ struct mode *mode;
+ struct {
+ int clock;
+ int hdisplay;
+ int hsync_start;
+ int hsync_end;
+ int htotal;
+ int vdisplay;
+ int vsync_start;
+ int vsync_end;
+ int vtotal;
+ char hsync[128];
+ char vsync[128];
+ } newmode;
+ struct wrandr_callback *callback;
+};
+
+struct mode {
+ struct wl_list link;
+ int height;
+ int width;
+ int refresh;
+ uint32_t flags;
+};
+
+struct output {
+ struct wl_list link;
+ struct wl_list mode_list;
+ struct randr *randr;
+ struct wl_output *output;
+ char *name;
+ int x;
+ int y;
+ int physical_width;
+ int physical_height;
+ int subpixel;
+ char *make;
+ char *model;
+ int transform;
+ int scale;
+ int server_output_id;
+};
+
+struct argument {
+ char *output;
+ char *leftof;
+ char *rightof;
+ char *mode;
+ char *newmode;
+ char *delmode;
+ int query;
+ int scale;
+ int transform;
+ int help;
+};
+
+static void
+delete_argument(struct argument *argument)
+{
+ if (argument->output)
+ free(argument->output);
+
+ if (argument->leftof)
+ free(argument->leftof);
+
+ if (argument->rightof)
+ free(argument->rightof);
+
+ if (argument->mode)
+ free(argument->mode);
+
+ if (argument->newmode)
+ free(argument->newmode);
+
+ if (argument->delmode)
+ free(argument->delmode);
+}
+
+#define RESULT_MASK 0x3
+
+static void
+print_line(int flags, int results, int flag, char *name)
+{
+ if (!(flags & 1<<flag))
+ return;
+
+ printf("%s:", name);
+ switch (results>>(flag * 2) & RESULT_MASK) {
+ case WRANDR_CALLBACK_RESULT_SUCCESS:
+ printf("SUCCESS!\n");
+ break;
+ case WRANDR_CALLBACK_RESULT_FAIL:
+ printf("FAIL!\n");
+ break;
+ case WRANDR_CALLBACK_RESULT_NOACT:
+ printf("Same as current, no change!\n");
+ break;
+ }
+}
+
+static void
+randr_done(void *data,
+ struct wrandr_callback *callback,
+ uint32_t flags,
+ uint32_t results)
+{
+ print_line(flags, results,
+ WRANDR_CALLBACK_ACTION_TRANSFORM, "TRANSFORM");
+ print_line(flags, results, WRANDR_CALLBACK_ACTION_MODE, "MODE");
+ print_line(flags, results, WRANDR_CALLBACK_ACTION_SCALE, "SCALE");
+ print_line(flags, results, WRANDR_CALLBACK_ACTION_MOVE, "MOVE");
+ print_line(flags, results, WRANDR_CALLBACK_ACTION_NEWMODE, "NEWMODE");
+ print_line(flags, results, WRANDR_CALLBACK_ACTION_DELMODE, "DELMODE");
+
+ running = 0;
+}
+
+static const struct wrandr_callback_listener wrandr_listener = {
+ randr_done
+};
+
+static void
+output_handle_geometry(void *data,
+ struct wl_output *wl_output,
+ int x, int y,
+ int physical_width,
+ int physical_height,
+ int subpixel,
+ const char *make,
+ const char *model,
+ int transform)
+{
+ struct output *output = data;
+
+ output->output = wl_output;
+ output->x = x;
+ output->y = y;
+ output->physical_height = physical_height;
+ output->physical_width = physical_width;
+ output->subpixel = subpixel;
+ output->make = strdup(make);
+ output->model = strdup(model);
+ output->transform = transform;
+}
+
+static void
+output_handle_done(void *data,
+ struct wl_output *wl_output)
+{
+}
+
+static void
+output_handle_name(void *data,
+ struct wl_output *wl_output,
+ const char *name)
+{
+ struct output *output = data;
+ if (name)
+ output->name = strdup(name);
+}
+
+static void
+output_handle_scale(void *data,
+ struct wl_output *wl_output,
+ int32_t scale)
+{
+ struct output *output = data;
+
+ output->scale = scale;
+}
+
+static void
+output_handle_mode(void *data,
+ struct wl_output *wl_output,
+ uint32_t flags,
+ int width,
+ int height,
+ int refresh)
+{
+ struct output *output = data;
+ struct mode *mode;
+
+ wl_list_for_each(mode, &output->mode_list, link) {
+ if (mode->width == width &&
+ mode->height == height &&
+ mode->refresh == refresh) {
+ if (flags != mode->flags)
+ mode->flags = flags;
+ return;
+ }
+ }
+
+ mode = (struct mode *)malloc(sizeof(*mode));
+ if (!mode)
+ return;
+
+ mode->width = width;
+ mode->height = height;
+ mode->refresh = refresh;
+ mode->flags = flags;
+
+ wl_list_insert(output->mode_list.prev, &mode->link);
+}
+
+static const struct wl_output_listener output_listener = {
+ output_handle_geometry,
+ output_handle_mode,
+ output_handle_done,
+ output_handle_scale,
+ output_handle_name
+};
+
+static void
+randr_add_output(struct randr *randr, uint32_t id)
+{
+ struct output *output;
+
+ output = (struct output *)malloc(sizeof(*output));
+ if (!output)
+ return;
+
+ output->randr = randr;
+ output->scale = 1;
+ output->name = NULL;
+ output->server_output_id = id;
+ output->output = wl_registry_bind(randr->registry,
+ id,
+ &wl_output_interface,
+ 2);
+
+ wl_list_init(&output->mode_list);
+ wl_list_insert(randr->output_list.prev, &output->link);
+ wl_output_add_listener(output->output, &output_listener, output);
+}
+
+static void
+mode_destroy(struct mode *mode)
+{
+ wl_list_remove(&mode->link);
+ free(mode);
+}
+
+static void
+output_destroy(struct output *output)
+{
+ struct mode *mode;
+
+ wl_list_for_each(mode, &output->mode_list, link)
+ mode_destroy(mode);
+
+ wl_list_remove(&output->link);
+
+ free(output->name);
+ free(output->make);
+ free(output->model);
+ free(output);
+}
+
+static void
+randr_destroy_output(struct randr *randr)
+{
+ struct output *output;
+
+ wl_list_for_each(output, &randr->output_list, link)
+ output_destroy(output);
+}
+
+static void
+registry_handle_global(void *data, struct wl_registry *registry, uint32_t id,
+ const char *interface, uint32_t version)
+{
+ struct randr *randr = data;
+
+ if (strcmp(interface, "weston_randr") == 0) {
+ randr->randr = wl_registry_bind(registry, id,
+ &weston_randr_interface,
+ 1);
+ } else if (strcmp(interface, "wl_compositor") == 0) {
+ randr->compositor = wl_registry_bind(registry, id,
+ &wl_compositor_interface,
+ 3);
+ } else if (strcmp(interface, "wl_output") == 0) {
+ randr_add_output(randr, id);
+ }
+}
+
+static void
+registry_handle_global_remove(void *data,
+ struct wl_registry *registry,
+ uint32_t name)
+{
+ struct randr *randr = data;
+
+ randr_destroy_output(randr);
+}
+
+static const struct wl_registry_listener registry_listener = {
+ registry_handle_global,
+ registry_handle_global_remove
+};
+
+static void
+transform_usage(void)
+{
+ fprintf(stderr,
+ " -R (0-7)\n"
+ " 0: TRANSFORM NORMAL\n"
+ " 1: TRANSFORM 90\n"
+ " 2: TRANSFORM 180\n"
+ " 3: TRANSFORM 270\n"
+ " 4: TRANSFORM FLIP 0\n"
+ " 5: TRANSFORM FLIP 90\n"
+ " 6: TRANSFORM FLIP 180\n"
+ " 7: TRANSFORM FLIP 270\n");
+}
+
+static void
+newmode_format(void)
+{
+ fprintf(stderr,
+ "\t\nNewmode Format:"
+ "\t--newmode='d,d,d,d,d,d,d,d,d,s,s'"
+ "(d:digit, s:string)\n\n"
+ "\tclock: Pixel clock (Mhz)\n\n"
+ "\thdisplay: Pixel number of one horticontal line(Hz)\n\n"
+ "\thsync_start: Start of horizontal sync signal(Hz)\n\n"
+ "\thsycn_end: End of horizontal sync signal(Hz)\n\n"
+ "\thtotal: Whole cycle of a horizontal line(Hz)\n\n"
+ "\tvdisplay: Pixel number of one vertical line(Hz)\n\n"
+ "\tvsync_start: Start of vertical sync signal(Hz)\n\n"
+ "\tvsycn_end: End of vertical sync signal(Hz)\n\n"
+ "\tvtotal: Whole cycle of a vertical line(Hz)\n\n"
+ "\thsync: Polarity of the horizontal sync pulses\n"
+ "\t\tvalue:(+-)hsync\n"
+ "\tvsync: Polarity of the vertical sync pulses\n"
+ "\t\tvsync value:(+-)vsync\n");
+}
+static void
+usage(int error_code)
+{
+ fprintf(stderr, "Usage: weston-randr [OPTIONS]\n\n"
+ " --leftof \tleft output\n"
+ " --rightof \tright output\n"
+ " --output \ttarget output\n"
+ " --newmode \tcustome one mode\n"
+ " --delmode \tdelete a mode like 800x600 at 58 or number\n"
+ " -q|--query \tquery all outputs\n"
+ " --mode \tset mode like 800x600 at 60 or number"
+ " --scale \tthe value of scale"
+ "(Firstly use -q to get mode list and find number).\n"
+ " --transform \toutput trasform(0-7)\n"
+ " -h|--help \tThis help text\n\n");
+
+ transform_usage();
+ newmode_format();
+ exit(error_code);
+}
+
+static void
+randr_init(struct randr *randr)
+{
+ wl_list_init(&randr->output_list);
+ randr->display = wl_display_connect(NULL);
+ assert(randr->display);
+
+ randr->registry = wl_display_get_registry(randr->display);
+ wl_registry_add_listener(randr->registry,
+ &registry_listener, randr);
+
+ wl_display_dispatch(randr->display);
+}
+
+static void
+randr_query(struct randr *randr)
+{
+ struct output *output;
+ struct mode *mode;
+ char *state;
+ int i;
+
+ wl_list_for_each(output, &randr->output_list, link) {
+ printf("%s\n", output->name);
+ i = 1;
+
+ wl_list_for_each(mode, &output->mode_list, link) {
+ state = "";
+ if (mode->flags & WL_OUTPUT_MODE_CURRENT)
+ state = "(current)";
+ else if (mode->flags & WL_OUTPUT_MODE_PREFERRED)
+ state = "(preferred)";
+
+ printf("%d)%dx%d@%d%s\n",
+ i++,
+ mode->width,
+ mode->height,
+ (mode->refresh + HZ/2)/HZ,
+ state);
+ }
+ printf("\n");
+ }
+}
+
+static int
+get_newmode(char *str, struct randr *randr)
+{
+ if (sscanf(str, "%d,%d,%d,%d,%d,%d,%d,%d,%d,%6s,%6s",
+ &randr->newmode.clock,
+ &randr->newmode.hdisplay,
+ &randr->newmode.hsync_start,
+ &randr->newmode.hsync_end,
+ &randr->newmode.htotal,
+ &randr->newmode.vdisplay,
+ &randr->newmode.vsync_start,
+ &randr->newmode.vsync_end,
+ &randr->newmode.vtotal,
+ randr->newmode.hsync,
+ randr->newmode.vsync) == 11) {
+
+ if ((strcmp(randr->newmode.hsync, "+hsync") != 0 &&
+ strcmp(randr->newmode.hsync, "-hsync") != 0) ||
+ (strcmp(randr->newmode.vsync, "+vsync") != 0 &&
+ strcmp(randr->newmode.vsync, "-vsync") != 0)) {
+ fprintf(stderr,
+ "Invalid format of hsync and vsync\n"
+ "hsync:%s, vsync:%s\n",
+ randr->newmode.hsync,
+ randr->newmode.vsync);
+
+ return WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ return WRANDR_CALLBACK_RESULT_SUCCESS;
+ }
+ return WRANDR_CALLBACK_RESULT_FAIL;
+}
+
+static struct mode *
+found_mode(struct output *output,
+ const char *modestr)
+{
+ int width = -1, height = -1, refresh = -1;
+ int num = -1;
+ int i = 0;
+ struct mode *mode;
+
+ if (sscanf(modestr, "%dx%d@%d", &width, &height, &refresh) != 3) {
+ if (sscanf(modestr, "%dx%d", &width, &height) != 2) {
+ if (sscanf(modestr, "%d", &num) != 1) {
+ printf("Error formatting for mode.\n");
+ return NULL;
+ }
+ }
+ }
+
+ if (num >= 0) {
+ wl_list_for_each(mode, &output->mode_list, link) {
+ i++;
+ if (i == num)
+ return mode;
+ }
+ } else if (width > 0 && height > 0)
+ wl_list_for_each(mode, &output->mode_list, link) {
+ if (mode->width == width && mode->height == height)
+ if (refresh <= 0 || mode->refresh == refresh)
+ return mode;
+ }
+
+ return NULL;
+}
+
+static void
+randr_printmode(struct output *output)
+{
+ struct mode *mode;
+ char *state;
+ int i = 1;
+
+ printf("%s\n", output->name);
+ wl_list_for_each(mode, &output->mode_list, link) {
+ if (mode->flags & WL_OUTPUT_MODE_CURRENT)
+ state = "(current)";
+ else if (mode->flags & WL_OUTPUT_MODE_PREFERRED)
+ state = "(preferred)";
+ else
+ state = "";
+ printf("%d)%dx%d@%d%s\n",
+ i++,
+ mode->width,
+ mode->height,
+ (mode->refresh + HZ/2)/HZ,
+ state);
+ }
+}
+
+static void
+randr_commit(struct randr *randr,
+ struct wl_output *wayland_output)
+{
+ weston_randr_commit(randr->randr,
+ wayland_output);
+}
+
+static void
+get_output_handle(struct randr *randr,
+ struct argument *argument)
+{
+ struct output *output;
+
+ /* Get all output handles. */
+ wl_list_for_each(output, &randr->output_list, link) {
+ if (argument->leftof &&
+ !strcmp(output->name, argument->leftof)) {
+ randr->loutput = output->output;
+ }
+
+ if (argument->rightof &&
+ !strcmp(output->name, argument->rightof)) {
+ randr->routput = output->output;
+ }
+
+ if (argument->output &&
+ !strcmp(output->name, argument->output)) {
+ randr->output = output;
+ randr->wayland_output = output->output;
+ }
+ }
+}
+
+static int
+verify_params(struct randr *randr,
+ struct argument *argument)
+{
+ int ret = WRANDR_CALLBACK_RESULT_SUCCESS;
+ /* Verify output paramerters */
+ if (argument->output && !randr->wayland_output) {
+ printf("%s does not exists or not connected.\n",
+ argument->output);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+
+ if (argument->newmode) {
+ if (get_newmode(argument->newmode, randr) ==
+ WRANDR_CALLBACK_RESULT_FAIL) {
+ printf("Invalid newmode format!\n");
+ newmode_format();
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->delmode) {
+ randr->delmode = found_mode(randr->output, argument->delmode);
+ if (!randr->delmode) {
+ printf("%s not exists\n", argument->delmode);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->leftof) {
+ if (!randr->loutput) {
+ printf("%s not exists\n", argument->leftof);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->rightof) {
+ if (!randr->routput) {
+ printf("%s not exists\n", argument->rightof);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->mode) {
+ randr->mode = found_mode(randr->output, argument->mode);
+ if (randr->mode) {
+ if (randr->mode->flags & WL_OUTPUT_MODE_CURRENT) {
+ printf("%dx%d@%d has been the current.\n",
+ randr->mode->width,
+ randr->mode->height,
+ randr->mode->refresh);
+
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ } else {
+ printf("%s not exists\n", argument->mode);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->scale != -1) {
+ if (argument->scale <= 0) {
+ printf("scale %d should be great than 0.\n",
+ argument->scale);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+
+ if (argument->scale == randr->output->scale) {
+ printf("scale %d is the same as current.\n",
+ argument->scale);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ if (argument->transform != -1) {
+ if (argument->transform >= 0 && argument->transform <= 7) {
+ if (argument->transform == randr->output->transform) {
+ printf("transform %d is the same as current.\n",
+ argument->transform);
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ } else {
+ transform_usage();
+ ret = WRANDR_CALLBACK_RESULT_FAIL;
+ }
+ }
+
+ return ret;
+}
+
+static void
+query_output(struct randr *randr,
+ struct argument *argument)
+{
+ /* Query mode */
+ if (argument->query > 0) {
+ if (randr->output)
+ randr_printmode(randr->output);
+ else
+ randr_query(randr);
+ }
+}
+
+static int
+del_new_mode(struct randr *randr,
+ struct argument *argument)
+{
+ int ret = RANDR_EXIT;
+
+ /* Delete mode */
+ if (argument->delmode) {
+ weston_randr_del_mode(randr->randr,
+ randr->wayland_output,
+ randr->delmode->width,
+ randr->delmode->height,
+ randr->delmode->refresh);
+ ret = RANDR_COMMIT;
+ }
+
+ /* New mode */
+ if (argument->newmode) {
+ weston_randr_new_mode(randr->randr,
+ randr->wayland_output,
+ randr->newmode.clock,
+ randr->newmode.hdisplay,
+ randr->newmode.hsync_start,
+ randr->newmode.hsync_end,
+ randr->newmode.htotal,
+ randr->newmode.vdisplay,
+ randr->newmode.vsync_start,
+ randr->newmode.vsync_end,
+ randr->newmode.vtotal,
+ randr->newmode.hsync,
+ randr->newmode.vsync);
+
+ ret = RANDR_COMMIT;
+ }
+
+ return ret;
+}
+
+static int
+output_modeset(struct randr *randr,
+ struct argument *argument)
+{
+ int ret = RANDR_EXIT;
+
+ /* Single action for output. */
+ if (randr->mode) {
+ weston_randr_set_mode(randr->randr,
+ randr->wayland_output,
+ randr->mode->width,
+ randr->mode->height,
+ randr->mode->refresh);
+ ret = RANDR_COMMIT;
+ }
+
+ if (argument->scale != -1) {
+ weston_randr_set_scale(randr->randr,
+ randr->wayland_output,
+ argument->scale);
+ ret = RANDR_COMMIT;
+ }
+
+ if (argument->transform >= 0) {
+ weston_randr_set_transform(randr->randr,
+ randr->wayland_output,
+ argument->transform);
+ ret = RANDR_COMMIT;
+
+ }
+
+ if (randr->loutput) {
+ weston_randr_move(randr->randr,
+ randr->wayland_output,
+ randr->loutput,
+ WESTON_RANDR_MOVE_LEFTOF);
+ ret = RANDR_COMMIT;
+ }
+
+ if (randr->routput) {
+ weston_randr_move(randr->randr,
+ randr->wayland_output,
+ randr->routput,
+ WESTON_RANDR_MOVE_RIGHTOF);
+ ret = RANDR_COMMIT;
+ }
+
+ return ret;
+}
+
+int
+main(int argc, char **argv)
+{
+ struct randr randr = { 0 };
+ struct argument argument = {NULL, NULL, NULL, NULL,
+ NULL, NULL, -1, -1, -1, -1};
+ int ret = 0;
+ int commit = 0;
+
+ const struct weston_option randr_options[] = {
+ { WESTON_OPTION_STRING, "output", 0, &argument.output},
+ { WESTON_OPTION_STRING, "mode", 0, &argument.mode},
+ { WESTON_OPTION_STRING, "newmode", 0, &argument.newmode},
+ { WESTON_OPTION_STRING, "delmode", 0, &argument.delmode},
+ { WESTON_OPTION_STRING, "leftof", 0, &argument.leftof},
+ { WESTON_OPTION_STRING, "rightof", 0, &argument.rightof},
+ { WESTON_OPTION_BOOLEAN, "query", 'q', &argument.query},
+ { WESTON_OPTION_INTEGER, "scale", 0, &argument.scale},
+ { WESTON_OPTION_INTEGER, "transform", 0, &argument.transform},
+ { WESTON_OPTION_BOOLEAN, "help", 'h', &argument.help },
+ };
+
+ parse_options(randr_options, ARRAY_LENGTH(randr_options), &argc, argv);
+
+ if (argument.help > 0)
+ usage(EXIT_SUCCESS);
+
+ randr_init(&randr);
+
+ wl_display_roundtrip(randr.display);
+
+ /* Check if weston_randr is enable or not. */
+ if (!randr.randr) {
+ printf("weston_randr is not available in compositor.\n");
+ goto exit;
+ }
+
+ get_output_handle(&randr, &argument);
+
+ if (verify_params(&randr, &argument) ==
+ WRANDR_CALLBACK_RESULT_FAIL)
+ goto exit;
+
+ query_output(&randr, &argument);
+
+ if (!randr.output)
+ goto exit;
+
+ randr.callback =
+ weston_randr_start(randr.randr, randr.wayland_output);
+ wrandr_callback_add_listener(randr.callback,
+ &wrandr_listener, &randr);
+
+ if (del_new_mode(&randr, &argument) == RANDR_COMMIT)
+ commit += 1;
+
+ if (del_new_mode(&randr, &argument) == RANDR_COMMIT)
+ commit += 1;
+
+ if (output_modeset(&randr, &argument) == RANDR_COMMIT)
+ commit += 1;
+
+ if (commit > 0) {
+ randr_commit(&randr,
+ randr.wayland_output);
+ } else
+ goto exit;
+
+ while (running && ret != -1)
+ ret = wl_display_dispatch(randr.display);
+
+exit:
+ delete_argument(&argument);
+ wl_registry_destroy(randr.registry);
+ wl_display_flush(randr.display);
+ wl_display_disconnect(randr.display);
+
+ return 0;
+}
--
1.8.1.2
Quanxian Wang
2014-03-24 11:39:20 UTC
Permalink
For example, when you run start randr, will send 5 request,
while you go to 3th request, somebody operates on the same output
and commit his request, it will interrupt you. This will affect
your complete actions on the output.

In order to avoid this case, take request_id, client, output as the
unique id to generate a request set in compositor randr, others
will not do any operation on your private request structure.
After you commit, you only commit your request and without
submitting others requests.

If only one thread/process uses client and output, just take
request_id as 0 is enough. If more threads/process use the same
client and output at the same time, client app needs generate
a request id to be distinguished from others.

Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
---
clients/wrandr.c | 35 ++++++++++++++++++++++++++---------
module/wrandr/wrandr.c | 31 +++++++++++++++++++++----------
protocol/randr.xml | 8 ++++++++
3 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/clients/wrandr.c b/clients/wrandr.c
index 86e58a1..05cbd21 100644
--- a/clients/wrandr.c
+++ b/clients/wrandr.c
@@ -558,10 +558,12 @@ randr_printmode(struct output *output)

static void
randr_commit(struct randr *randr,
- struct wl_output *wayland_output)
+ struct wl_output *wayland_output,
+ uint32_t request_id)
{
weston_randr_commit(randr->randr,
- wayland_output);
+ wayland_output,
+ request_id);
}

static void
@@ -695,7 +697,8 @@ query_output(struct randr *randr,

static int
del_new_mode(struct randr *randr,
- struct argument *argument)
+ struct argument *argument,
+ uint32_t request_id)
{
int ret = RANDR_EXIT;

@@ -703,6 +706,7 @@ del_new_mode(struct randr *randr,
if (argument->delmode) {
weston_randr_del_mode(randr->randr,
randr->wayland_output,
+ request_id,
randr->delmode->width,
randr->delmode->height,
randr->delmode->refresh);
@@ -713,6 +717,7 @@ del_new_mode(struct randr *randr,
if (argument->newmode) {
weston_randr_new_mode(randr->randr,
randr->wayland_output,
+ request_id,
randr->newmode.clock,
randr->newmode.hdisplay,
randr->newmode.hsync_start,
@@ -733,7 +738,8 @@ del_new_mode(struct randr *randr,

static int
output_modeset(struct randr *randr,
- struct argument *argument)
+ struct argument *argument,
+ uint32_t request_id)
{
int ret = RANDR_EXIT;

@@ -741,6 +747,7 @@ output_modeset(struct randr *randr,
if (randr->mode) {
weston_randr_set_mode(randr->randr,
randr->wayland_output,
+ request_id,
randr->mode->width,
randr->mode->height,
randr->mode->refresh);
@@ -750,6 +757,7 @@ output_modeset(struct randr *randr,
if (argument->scale != -1) {
weston_randr_set_scale(randr->randr,
randr->wayland_output,
+ request_id,
argument->scale);
ret = RANDR_COMMIT;
}
@@ -757,6 +765,7 @@ output_modeset(struct randr *randr,
if (argument->transform >= 0) {
weston_randr_set_transform(randr->randr,
randr->wayland_output,
+ request_id,
argument->transform);
ret = RANDR_COMMIT;

@@ -765,6 +774,7 @@ output_modeset(struct randr *randr,
if (randr->loutput) {
weston_randr_move(randr->randr,
randr->wayland_output,
+ request_id,
randr->loutput,
WESTON_RANDR_MOVE_LEFTOF);
ret = RANDR_COMMIT;
@@ -773,6 +783,7 @@ output_modeset(struct randr *randr,
if (randr->routput) {
weston_randr_move(randr->randr,
randr->wayland_output,
+ request_id,
randr->routput,
WESTON_RANDR_MOVE_RIGHTOF);
ret = RANDR_COMMIT;
@@ -789,6 +800,11 @@ main(int argc, char **argv)
NULL, NULL, -1, -1, -1, -1};
int ret = 0;
int commit = 0;
+ uint32_t request_id = 0;
+
+ /* Get a request id to unique request process */
+ srandom(10000);
+ request_id = random();

const struct weston_option randr_options[] = {
{ WESTON_OPTION_STRING, "output", 0, &argument.output},
@@ -830,22 +846,23 @@ main(int argc, char **argv)
goto exit;

randr.callback =
- weston_randr_start(randr.randr, randr.wayland_output);
+ weston_randr_start(randr.randr, randr.wayland_output, request_id);
wrandr_callback_add_listener(randr.callback,
&wrandr_listener, &randr);

- if (del_new_mode(&randr, &argument) == RANDR_COMMIT)
+ if (del_new_mode(&randr, &argument, request_id) == RANDR_COMMIT)
commit += 1;

- if (del_new_mode(&randr, &argument) == RANDR_COMMIT)
+ if (del_new_mode(&randr, &argument, request_id) == RANDR_COMMIT)
commit += 1;

- if (output_modeset(&randr, &argument) == RANDR_COMMIT)
+ if (output_modeset(&randr, &argument, request_id) == RANDR_COMMIT)
commit += 1;

if (commit > 0) {
randr_commit(&randr,
- randr.wayland_output);
+ randr.wayland_output,
+ request_id);
} else
goto exit;

diff --git a/module/wrandr/wrandr.c b/module/wrandr/wrandr.c
index b97d101..f17edc2 100644
--- a/module/wrandr/wrandr.c
+++ b/module/wrandr/wrandr.c
@@ -42,6 +42,7 @@ struct randr_request {
* of these request.*/
struct wl_client *client;
struct weston_output *output;
+ uint32_t request_id;

/* output move */
struct weston_output *loutput;
@@ -97,6 +98,7 @@ static struct randr_request *
get_request(struct wl_resource *resource,
struct wl_client *client,
struct weston_output *output,
+ uint32_t request_id,
int create)
{
struct wrandr *randr =
@@ -106,6 +108,7 @@ get_request(struct wl_resource *resource,
wl_list_for_each(request, &randr->pending.request_list, link)
{
if (request->client == client &&
+ request->request_id == request_id &&
request->output == output)
return request;
}
@@ -120,6 +123,7 @@ get_request(struct wl_resource *resource,

request->client = client;
request->output = output;
+ request->request_id = request_id;
wl_list_insert(&randr->pending.request_list, &request->link);
return request;
} else
@@ -393,13 +397,14 @@ static void
randr_set_scale(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *output_resource,
+ uint32_t request_id,
int32_t scale)
{
struct weston_output *output =
wl_resource_get_user_data(output_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_SCALE;
request->scale = scale;
@@ -410,13 +415,14 @@ static void
randr_set_transform(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *output_resource,
+ uint32_t request_id,
uint32_t transform)
{
struct weston_output *output =
wl_resource_get_user_data(output_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_TRANSFORM;
request->transform = transform % 8;
@@ -427,13 +433,13 @@ static void
randr_start(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *output_resource,
- uint32_t callback)
+ uint32_t request_id, uint32_t callback)
{
struct weston_output *output =
wl_resource_get_user_data(output_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (!request) {
wl_resource_post_no_memory(resource);
return;
@@ -453,13 +459,14 @@ static void
randr_set_mode(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *output_resource,
+ uint32_t request_id,
int width, int height, int refresh)
{
struct weston_output *output =
wl_resource_get_user_data(output_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_MODE;
request->width = width;
@@ -472,13 +479,14 @@ static void
randr_move(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *target_resource,
+ uint32_t request_id,
struct wl_resource *source_resource, int move)
{
struct weston_output *output =
wl_resource_get_user_data(target_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_MOVE;
if (move == WESTON_RANDR_MOVE_LEFTOF) {
@@ -563,6 +571,7 @@ static void
randr_newmode(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *target_resource,
+ uint32_t request_id,
int clock,
int hdisplay,
int hsync_start,
@@ -579,7 +588,7 @@ randr_newmode(struct wl_client *client,
wl_resource_get_user_data(target_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_NEWMODE;
request->newmode.clock = clock;
@@ -631,6 +640,7 @@ static void
randr_delmode(struct wl_client *client,
struct wl_resource *resource,
struct wl_resource *target_resource,
+ uint32_t request_id,
int width,
int height,
int refresh)
@@ -639,7 +649,7 @@ randr_delmode(struct wl_client *client,
wl_resource_get_user_data(target_resource);
struct randr_request *request;

- request = get_request(resource, client, output, 1);
+ request = get_request(resource, client, output, request_id, 1);
if (request) {
request->flags |= 1<<WRANDR_CALLBACK_ACTION_DELMODE;
request->delmode.width = width;
@@ -651,7 +661,8 @@ randr_delmode(struct wl_client *client,
static void
randr_commit(struct wl_client *client,
struct wl_resource *resource,
- struct wl_resource *target_resource)
+ struct wl_resource *target_resource,
+ uint32_t request_id)
{
struct wrandr *randr = wl_resource_get_user_data(resource);
struct weston_output *output =
@@ -661,7 +672,7 @@ randr_commit(struct wl_client *client,
int result = 0;

/* process pending request for this client */
- request = get_request(resource, client, output, 0);
+ request = get_request(resource, client, output, request_id, 0);
if (!request) {
weston_log("No action happens when commit?!\n");
} else {
diff --git a/protocol/randr.xml b/protocol/randr.xml
index 07f83a4..555e6bf 100644
--- a/protocol/randr.xml
+++ b/protocol/randr.xml
@@ -56,6 +56,7 @@
posted for one randr request unless requested again.
</description>
<arg name="output" type="object" interface="wl_output"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<arg name="callback" type="new_id" interface="wrandr_callback"/>
</request>

@@ -78,6 +79,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<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"/>
@@ -92,6 +94,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<arg name="transform" type="uint"
summary="the value should be between 0-7"/>
</request>
@@ -102,6 +105,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<arg name="scale" type="int"
summary="Scale the output"/>
</request>
@@ -121,6 +125,7 @@
</description>
<arg name="target_output" type="object" interface="wl_output"
summary="the target output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<arg name="source_output" type="object" interface="wl_output"
summary="the source output object"/>
<arg name="move" type="int"/>
@@ -132,6 +137,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<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"/>
@@ -143,6 +149,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
<arg name="fclock" type="int"
summary="Pixel clock in hardware units Mhz"/>
<arg name="hdisplay" type="int"
@@ -176,6 +183,7 @@
</description>
<arg name="output" type="object" interface="wl_output"
summary="the output object"/>
+ <arg name="request_id" type="uint" summary="request id of randr"/>
</request>
</interface>
--
1.8.1.2
Bryce W. Harrington
2014-04-02 02:52:51 UTC
Permalink
Post by Quanxian Wang
By the way, currently we are focus on the protocol design. The implemented code is just
a reference. However it will be helpful for reviewer to have a test and get the clear
idea of protocol. I like your comment to make this protocol stronger. Thanks.
Especially thanks to Pq. He gives me more valuable information. I will continue keep
tuning your comment and make the change.
I haven't done a new review of the latest version of the protocol, but
have been following the discussions so far. I'd like to step back and
offer some thoughts based on my experience with randr as Ubuntu's X.org
maintainer for a good 7 years. Sorry this is so long and rambling; skip
down to the end for my specific suggestions.


I remember when randr was first introduced, and it was a godsend,
because it allowed changing resolutions on the fly. This was very
important with the old CRTs because you often didn't have EDID
information, and the best resolution was not always the highest, and
you'd need to fiddle with refresh rates to get something that was easy
to look at. IOW, there was a lot of fiddling we expected of users.
librandr and the xrandr tool gave users a way to do this fiddling
without having to restart X. GUI applications wrappering either
librandr or xrandr sprung up over time, and after a lot of debugging we
got to the relatively robust display configuration tools we have today.

It's worth noting that librandr is a *very* low level protocol, compared
with the more user-friendly CUI interface provided by the xrandr tool.
It describes CRTCs, outputs, and so on. As GUI interfaces came into
being, they had to reinvent the user-oriented logic already in the
xrandr tool, and besides the duplication of effort we saw the same
mistakes made over and over. If there had been a higher level library
that implemented xrandr's user-friendly interface as a proper structured
API, it would have made life much easier for the GUI configuration tool
developers, and would have ensured better consistency and faster
stabilization across the development community.

So, I think that just reimplementing librandr's API as-is in Wayland
isn't necessarily the best way to go. Instead I think you want a high
level API that says, "Put monitor A on the left at max resolution, and
monitor B to the right at max res," as you would with xrandr, and leave
all the detailed CRTC and whatnot operations as hidden internal
details.

When xrandr was developed, many things in display were non-standard.
It was not unusual for monitors to not provide EDID, or to provide
incorrect EDID, and mess up X's detection (there were lengthy lists of
quirks and heuristics in the X drivers and Xserver to fixup specific
hardware). A lot of these issues are in the past. You can assume the
kernel handles a lot of this mess these days, and when there are
discrepancies you treat them as simple bugs and don't really need tools
to configure your way around them.

The needs of users have also changed. When randr was introduced, users
wanted better configuration tools do set up their displays. Having
access to the configuration innards was a good thing. Today's users
want *no* configuration tools to be needed; the displays should come up
at maximum resolution supported by the hardware and "just work" when
plugged together with other hardware.

For many of today's computing use cases, that is pretty much how it
works. Tablets, embedded devices, single-display laptops, phones...
Other than screen rotation the user won't have much need for display
configuration.

The one glaring exception is external display. Plug a tablet into an
external monitor, or a laptop into a projector. randr's approach to
give the user the tools to set these up, and it does give a very
flexible set of controls. However, users these days don't even want to
do this degree of configuration. At best, they want to hit a Fn key
combo to toggle between mirrored, extended, etc.

You've indicated the intended audience for this as testers and
administrators. I think as a tester or administrator I would not care
about on-the-fly configuration changes, and actually would prefer
locking down the settings in the wayland.ini file. But I may not
understand the full breadth of uses you intend, so I'll assume you have
good reasons for wanting on-the-fly changes, and offer these
suggestions:


1. Keep the API very, very high level.

Rather than providing individual routines to set this and that, define a
data structure that will describe a static configuration of monitors,
which you pass to Wayland for parsing and validating (and perhaps
persisting it to disk in XML or whatnot). A second call lets you
activate a validated configuration (with 0 being a known-good default).
A third one to delete a configuration. Another to return a list of all
stored configs.


2. Automatic detection of external configs

When multiple video cards and/or monitors are present, there should be a
way to request a set of automatically determined static configurations,
that can be selected from and passed directly to the API call in #1.
For instance, given two physically identical monitors we can assume the
user wants to lay them out side-by-side or above-and-below (or vice
versa), and provide the four combinations. The user can then hit a
button or key combo to toggle between the four until it's right, and
then click save. Or if one of the displays looks like it might be a
projector, provide a set of configs for letterboxed, mirrored, extended,
etc.

There should be a hashing function used to id the various configs, so
when you have a volatile combination (a laptop in a docking station, or
a tablet periodically hooked in via USB, or etc.) the IDs of the
displays are hashed together and used to look up which config to use.


3. No modeline fiddling

Yes, even recently I've seen cases where users needed the ability to add
their own modelines, and used xrandr's addmode/setmode stuff to do so.
But I think we can do better.

First, 90% of these cases are due to broken or missing EDIDs. Modern
kernels now permit adding edid's to /lib/firmware. I think this ends up
being a much more holistic solution, because you can supply them via the
normal packaging system. Then we'd just need to have this API provide a
way to select amongst available EDIDs and notify the kernel to switch.

Also, for the (hopefully increasingly rare) cases where users do need to
fiddle modelines, this should rather be done by having them specify the
desired resolution, refresh rates, etc. as the inputs, and calculate the
modeline internally, saving it either to EDID or placing it into the
data structure in #1.


I think that covers everything. Hopefully the above gives some useful
food for thought in designing this, although maybe I'm just throwing a
smelly fish on the table! :-) But I do think we can do better than
just reimplementing randr, and come up with something that avoids many
of the issues that have been driving users to need to configure things
at all.

Bryce
Fu Michael
2014-04-02 03:42:11 UTC
Permalink
Post by Bryce W. Harrington
Post by Quanxian Wang
By the way, currently we are focus on the protocol design. The implemented code is just
a reference. However it will be helpful for reviewer to have a test and get the clear
idea of protocol. I like your comment to make this protocol stronger. Thanks.
Especially thanks to Pq. He gives me more valuable information. I will continue keep
tuning your comment and make the change.
I haven't done a new review of the latest version of the protocol, but
have been following the discussions so far. I'd like to step back and
offer some thoughts based on my experience with randr as Ubuntu's X.org
maintainer for a good 7 years. Sorry this is so long and rambling; skip
down to the end for my specific suggestions.
I remember when randr was first introduced, and it was a godsend,
because it allowed changing resolutions on the fly. This was very
important with the old CRTs because you often didn't have EDID
information, and the best resolution was not always the highest, and
you'd need to fiddle with refresh rates to get something that was easy
to look at. IOW, there was a lot of fiddling we expected of users.
librandr and the xrandr tool gave users a way to do this fiddling
without having to restart X. GUI applications wrappering either
librandr or xrandr sprung up over time, and after a lot of debugging we
got to the relatively robust display configuration tools we have today.
It's worth noting that librandr is a *very* low level protocol, compared
with the more user-friendly CUI interface provided by the xrandr tool.
It describes CRTCs, outputs, and so on. As GUI interfaces came into
being, they had to reinvent the user-oriented logic already in the
xrandr tool, and besides the duplication of effort we saw the same
mistakes made over and over. If there had been a higher level library
that implemented xrandr's user-friendly interface as a proper structured
API, it would have made life much easier for the GUI configuration tool
developers, and would have ensured better consistency and faster
stabilization across the development community.
So, I think that just reimplementing librandr's API as-is in Wayland
isn't necessarily the best way to go. Instead I think you want a high
level API that says, "Put monitor A on the left at max resolution, and
monitor B to the right at max res," as you would with xrandr, and leave
all the detailed CRTC and whatnot operations as hidden internal
details.
When xrandr was developed, many things in display were non-standard.
It was not unusual for monitors to not provide EDID, or to provide
incorrect EDID, and mess up X's detection (there were lengthy lists of
quirks and heuristics in the X drivers and Xserver to fixup specific
hardware). A lot of these issues are in the past. You can assume the
kernel handles a lot of this mess these days, and when there are
discrepancies you treat them as simple bugs and don't really need tools
to configure your way around them.
The needs of users have also changed. When randr was introduced, users
wanted better configuration tools do set up their displays. Having
access to the configuration innards was a good thing. Today's users
want *no* configuration tools to be needed; the displays should come up
at maximum resolution supported by the hardware and "just work" when
plugged together with other hardware.
For many of today's computing use cases, that is pretty much how it
works. Tablets, embedded devices, single-display laptops, phones...
Other than screen rotation the user won't have much need for display
configuration.
The one glaring exception is external display. Plug a tablet into an
external monitor, or a laptop into a projector. randr's approach to
give the user the tools to set these up, and it does give a very
flexible set of controls. However, users these days don't even want to
do this degree of configuration. At best, they want to hit a Fn key
combo to toggle between mirrored, extended, etc.
You've indicated the intended audience for this as testers and
administrators. I think as a tester or administrator I would not care
about on-the-fly configuration changes, and actually would prefer
locking down the settings in the wayland.ini file. But I may not
understand the full breadth of uses you intend, so I'll assume you have
good reasons for wanting on-the-fly changes, and offer these
1. Keep the API very, very high level.
Rather than providing individual routines to set this and that, define a
data structure that will describe a static configuration of monitors,
which you pass to Wayland for parsing and validating (and perhaps
persisting it to disk in XML or whatnot). A second call lets you
activate a validated configuration (with 0 being a known-good default).
A third one to delete a configuration. Another to return a list of all
stored configs.
2. Automatic detection of external configs
When multiple video cards and/or monitors are present, there should be a
way to request a set of automatically determined static configurations,
that can be selected from and passed directly to the API call in #1.
For instance, given two physically identical monitors we can assume the
user wants to lay them out side-by-side or above-and-below (or vice
versa), and provide the four combinations. The user can then hit a
button or key combo to toggle between the four until it's right, and
then click save. Or if one of the displays looks like it might be a
projector, provide a set of configs for letterboxed, mirrored, extended,
etc.
There should be a hashing function used to id the various configs, so
when you have a volatile combination (a laptop in a docking station, or
a tablet periodically hooked in via USB, or etc.) the IDs of the
displays are hashed together and used to look up which config to use.
3. No modeline fiddling
Yes, even recently I've seen cases where users needed the ability to add
their own modelines, and used xrandr's addmode/setmode stuff to do so.
But I think we can do better.
First, 90% of these cases are due to broken or missing EDIDs. Modern
kernels now permit adding edid's to /lib/firmware. I think this ends up
being a much more holistic solution, because you can supply them via the
normal packaging system. Then we'd just need to have this API provide a
way to select amongst available EDIDs and notify the kernel to switch.
Also, for the (hopefully increasingly rare) cases where users do need to
fiddle modelines, this should rather be done by having them specify the
desired resolution, refresh rates, etc. as the inputs, and calculate the
modeline internally, saving it either to EDID or placing it into the
data structure in #1.
I think that covers everything. Hopefully the above gives some useful
food for thought in designing this, although maybe I'm just throwing a
smelly fish on the table! :-) But I do think we can do better than
just reimplementing randr, and come up with something that avoids many
of the issues that have been driving users to need to configure things
at all.
Indeed. Most importantly, giving user more confident when connecting a
Linux Wayland PC to a projector, rather than joking with it as a ice
breaker. ;)
Post by Bryce W. Harrington
Bryce
_______________________________________________
wayland-devel mailing list
wayland-devel at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Wang, Quanxian
2014-04-02 07:34:11 UTC
Permalink
Thanks Bryce's information and suggestion. Your suggestion and randr history are really appreciated.
As GUI interfaces came into being, they
had to reinvent the user-oriented logic already in the xrandr tool, and besides the
duplication of effort we saw the same mistakes made over and over. If there had
been a higher level library that implemented xrandr's user-friendly interface as a
proper structured API, it would have made life much easier for the GUI
configuration tool developers, and would have ensured better consistency and
faster stabilization across the development community.
[Wang, Quanxian] I am very interested with these words. What is a proper structure?
Is it a whole layout or a xml stature to contain all configuration request? I found you like xml to store all requests.
When you want to configure, just generate a xml with your request and send it to display server. Server will help
parse xml and do what user want to do. Is it right?

More comments below.

Regards

Thanks

Quanxian Wang
-----Original Message-----
From: Bryce W. Harrington [mailto:b.harrington at samsung.com]
Sent: Wednesday, April 02, 2014 10:53 AM
To: Wang, Quanxian
Cc: wayland-devel at lists.freedesktop.org; ppaalanen at gmail.com
Subject: Re: [PATCH V2 0/8] Add weston randr protocol
Post by Quanxian Wang
By the way, currently we are focus on the protocol design. The
implemented code is just a reference. However it will be helpful for
reviewer to have a test and get the clear idea of protocol. I like your comment to
make this protocol stronger. Thanks.
Post by Quanxian Wang
Especially thanks to Pq. He gives me more valuable information. I will
continue keep tuning your comment and make the change.
I haven't done a new review of the latest version of the protocol, but have been
following the discussions so far. I'd like to step back and offer some thoughts
based on my experience with randr as Ubuntu's X.org maintainer for a good 7
years. Sorry this is so long and rambling; skip down to the end for my specific
suggestions.
I remember when randr was first introduced, and it was a godsend, because it
allowed changing resolutions on the fly. This was very important with the old
CRTs because you often didn't have EDID information, and the best resolution was
not always the highest, and you'd need to fiddle with refresh rates to get
something that was easy to look at. IOW, there was a lot of fiddling we expected
of users.
librandr and the xrandr tool gave users a way to do this fiddling without having to
restart X. GUI applications wrappering either librandr or xrandr sprung up over
time, and after a lot of debugging we got to the relatively robust display
configuration tools we have today.
It's worth noting that librandr is a *very* low level protocol, compared with the
more user-friendly CUI interface provided by the xrandr tool.
It describes CRTCs, outputs, and so on. As GUI interfaces came into being, they
had to reinvent the user-oriented logic already in the xrandr tool, and besides the
duplication of effort we saw the same mistakes made over and over. If there had
been a higher level library that implemented xrandr's user-friendly interface as a
proper structured API, it would have made life much easier for the GUI
configuration tool developers, and would have ensured better consistency and
faster stabilization across the development community.
So, I think that just reimplementing librandr's API as-is in Wayland isn't necessarily
the best way to go. Instead I think you want a high level API that says, "Put
monitor A on the left at max resolution, and monitor B to the right at max res," as
you would with xrandr, and leave all the detailed CRTC and whatnot operations as
hidden internal details.
When xrandr was developed, many things in display were non-standard.
It was not unusual for monitors to not provide EDID, or to provide incorrect EDID,
and mess up X's detection (there were lengthy lists of quirks and heuristics in the
X drivers and Xserver to fixup specific hardware). A lot of these issues are in the
past. You can assume the kernel handles a lot of this mess these days, and when
there are discrepancies you treat them as simple bugs and don't really need tools
to configure your way around them.
The needs of users have also changed. When randr was introduced, users
wanted better configuration tools do set up their displays. Having access to the
configuration innards was a good thing. Today's users want *no* configuration
tools to be needed; the displays should come up at maximum resolution
supported by the hardware and "just work" when plugged together with other
hardware.
For many of today's computing use cases, that is pretty much how it works.
Tablets, embedded devices, single-display laptops, phones...
Other than screen rotation the user won't have much need for display
configuration.
The one glaring exception is external display. Plug a tablet into an external
monitor, or a laptop into a projector. randr's approach to give the user the tools
to set these up, and it does give a very flexible set of controls. However, users
these days don't even want to do this degree of configuration. At best, they
want to hit a Fn key combo to toggle between mirrored, extended, etc.
You've indicated the intended audience for this as testers and administrators. I
think as a tester or administrator I would not care about on-the-fly configuration
changes, and actually would prefer locking down the settings in the wayland.ini file.
But I may not understand the full breadth of uses you intend, so I'll assume you
have good reasons for wanting on-the-fly changes, and offer these
1. Keep the API very, very high level.
Rather than providing individual routines to set this and that, define a data
structure that will describe a static configuration of monitors, which you pass to
Wayland for parsing and validating (and perhaps persisting it to disk in XML or
whatnot). A second call lets you activate a validated configuration (with 0 being a
known-good default).
A third one to delete a configuration. Another to return a list of all stored
configs.
[Wang, Quanxian] As a whole, the randr protocol data structure for configuration request should be consistent with xml layout. Whatever xml or data structure, the only objective is to store all the configuration requests from client. Xml will contain all the configuration request and routine exists for atomic operations. Everyone has their advantage. For xml, it will be useful for static configuration just like xorg.conf or weston.ini, therefore if it is for static, I prefer to use xml in weston.ini instead of randr protocol. For dynamic, atomic operation will be easy to store every request together. But your idea gives me some hints. If randr provides an interface to transfer xml file and parse it, then put the results into data structure which contain configuration requests(merge together). Will it be better?
2. Automatic detection of external configs
When multiple video cards and/or monitors are present, there should be a way to
request a set of automatically determined static configurations, that can be
selected from and passed directly to the API call in #1.
For instance, given two physically identical monitors we can assume the user
wants to lay them out side-by-side or above-and-below (or vice versa), and
provide the four combinations. The user can then hit a button or key combo to
toggle between the four until it's right, and then click save. Or if one of the
displays looks like it might be a projector, provide a set of configs for letterboxed,
mirrored, extended, etc.
[Wang, Quanxian] It is the whole layout of all outputs. Interesting.
There should be a hashing function used to id the various configs, so when you
have a volatile combination (a laptop in a docking station, or a tablet periodically
hooked in via USB, or etc.) the IDs of the displays are hashed together and used to
look up which config to use.
3. No modeline fiddling
Yes, even recently I've seen cases where users needed the ability to add their own
modelines, and used xrandr's addmode/setmode stuff to do so.
But I think we can do better.
First, 90% of these cases are due to broken or missing EDIDs. Modern kernels
now permit adding edid's to /lib/firmware. I think this ends up being a much
more holistic solution, because you can supply them via the normal packaging
system. Then we'd just need to have this API provide a way to select amongst
available EDIDs and notify the kernel to switch.
Also, for the (hopefully increasingly rare) cases where users do need to fiddle
modelines, this should rather be done by having them specify the desired
resolution, refresh rates, etc. as the inputs, and calculate the modeline internally,
saving it either to EDID or placing it into the data structure in #1.
I think that covers everything. Hopefully the above gives some useful food for
thought in designing this, although maybe I'm just throwing a smelly fish on the
table! :-) But I do think we can do better than just reimplementing randr, and
come up with something that avoids many of the issues that have been driving
users to need to configure things at all.
[Wang, Quanxian] Very useful. Different view and experience will remind what missed, what to be considered more. Thank Bryce.
Bryce
Bryce W. Harrington
2014-04-02 08:29:58 UTC
Permalink
Post by Wang, Quanxian
Thanks Bryce's information and suggestion. Your suggestion and randr history are really appreciated.
As GUI interfaces came into being, they
had to reinvent the user-oriented logic already in the xrandr tool, and besides the
duplication of effort we saw the same mistakes made over and over. If there had
been a higher level library that implemented xrandr's user-friendly interface as a
proper structured API, it would have made life much easier for the GUI
configuration tool developers, and would have ensured better consistency and
faster stabilization across the development community.
[Wang, Quanxian] I am very interested with these words. What is a proper structure?
By that I mean a structured data format. That could be a C struct, an
XML schema, a JSON file, INI file... As a starting point I would
recommend looking at GNOME's ~/.config/monitors.xml file as an example.
(Probably worth looking at what KDE's configuration tools use too, and
maybe even Mir.) Basically it's a static layout describing how the
given set of display equipment needs to be set up.
Post by Wang, Quanxian
Is it a whole layout or a xml stature to contain all configuration request? I found you like xml to store all requests.
When you want to configure, just generate a xml with your request and send it to display server. Server will help
parse xml and do what user want to do. Is it right?
Right. It wouldn't have to be XML, although that might be the most
logical format, but if there's already an established wire format for
structured data use that. But that's the idea - make it a single
transaction, rather than a series of pokes and peeks.
Post by Wang, Quanxian
More comments below.
1. Keep the API very, very high level.
Rather than providing individual routines to set this and that, define a data
structure that will describe a static configuration of monitors, which you pass to
Wayland for parsing and validating (and perhaps persisting it to disk in XML or
whatnot). A second call lets you activate a validated configuration (with 0 being a
known-good default).
A third one to delete a configuration. Another to return a list of all stored
configs.
[Wang, Quanxian] As a whole, the randr protocol data structure for configuration request should be consistent with xml layout. Whatever xml or data structure, the only objective is to store all the configuration requests from client. Xml will contain all the configuration request and routine exists for atomic operations. Everyone has their advantage. For xml, it will be useful for static configuration just like xorg.conf or weston.ini, therefore if it is for static, I prefer to use xml in weston.ini instead of randr protocol. For dynamic, atomic operation will be easy to store every request together. But your idea gives me some hints. If randr provides an interface to transfer xml file and parse it, then put the results into data structure which contain configuration requests(merge together). Will it be better?
Right, you'd receive the configuration layout in some structured data
format, then internally you'd work out how to achieve that layout by
setting resolutions, rotations, translations, CRTC shuffling, and so
on.

Where this will show itself as better, I believe, is in the debugging.
When you provide the user with a bunch of atomic operations to do their
configuration, then there can be multiple paths they might take to get
from config A to config B. One user might translate the display, then
scale it to correct resolution; another might do it the other way
round. And you could have a bug that crops up when you do it one way
but not the other. Testers would need to parametrically test every
possible path from config A to B.

Now consider that we're starting to see graphics cards able to drive 3+
monitors, and hybrid graphics systems, and multi-video cards, and
hotplugging amongst all the above. All this adds more and more
variables for the user to go from A to B - and a combitorial nightmare
of test cases for the tester!

But with a transactional approach, the user isn't involved in any of
these decisions, and your implementation can establish a single way to
get from A to B, all under your programmatic control. Testing becomes
majorly simplified. Bug reproduction becomes quite straightforward -
you don't need much more than their config structure.

None of this is really new thinking - the fewer knobs and levers you
give the user, the less chance for things to go wrong.

And even though your targeted use case is administrators and testers,
well I think it works to their advantage too. The tester might
programmatically generate a set of configuration xml files for the range
of things they want to test, and then just iteratively toss them at
wayland and see what happens. The administrator would identify the
optimial 2 or 3 configs she wants to provide to her users, store the XML
in her configuration control system, and disseminate configuration
updates through her normal update processes.
Post by Wang, Quanxian
2. Automatic detection of external configs
When multiple video cards and/or monitors are present, there should be a way to
request a set of automatically determined static configurations, that can be
selected from and passed directly to the API call in #1.
For instance, given two physically identical monitors we can assume the user
wants to lay them out side-by-side or above-and-below (or vice versa), and
provide the four combinations. The user can then hit a button or key combo to
toggle between the four until it's right, and then click save. Or if one of the
displays looks like it might be a projector, provide a set of configs for letterboxed,
mirrored, extended, etc.
[Wang, Quanxian] It is the whole layout of all outputs. Interesting.
I'll elaborate on the 'letterboxed' option. Currently if you hook up
some unknown projector to your laptop and fire it up, you'll typically
find that both get mirrored, since X doesn't know anything about how
they should be configured.

But quite typically, the projector is going to be at a 4:3 aspect ratio,
vs. the laptop's 16:9 or 16:10, so none of the native resolutions of
either projector or laptop are going to match up. X drops down to the
lowest common denominator - one of the VESA resolutions, like 1024x768.
Yet this is pretty much guaranteed to be the *last* thing the user
wants.

What I think the user likely wants is for their content to be displayed
on the projector at the best possible resolution, and for it to be
displayed on their laptop at their laptop LVDS's best resolution. On
the laptop it should show the content in the same aspect ratio, by
adding black bars on either side. So that's what I mean by
letterboxing.
Post by Wang, Quanxian
There should be a hashing function used to id the various configs, so when you
have a volatile combination (a laptop in a docking station, or a tablet periodically
hooked in via USB, or etc.) the IDs of the displays are hashed together and used to
look up which config to use.
3. No modeline fiddling
Yes, even recently I've seen cases where users needed the ability to add their own
modelines, and used xrandr's addmode/setmode stuff to do so.
But I think we can do better.
First, 90% of these cases are due to broken or missing EDIDs. Modern kernels
now permit adding edid's to /lib/firmware. I think this ends up being a much
more holistic solution, because you can supply them via the normal packaging
system. Then we'd just need to have this API provide a way to select amongst
available EDIDs and notify the kernel to switch.
Also, for the (hopefully increasingly rare) cases where users do need to fiddle
modelines, this should rather be done by having them specify the desired
resolution, refresh rates, etc. as the inputs, and calculate the modeline internally,
saving it either to EDID or placing it into the data structure in #1.
I think that covers everything. Hopefully the above gives some useful food for
thought in designing this, although maybe I'm just throwing a smelly fish on the
table! :-) But I do think we can do better than just reimplementing randr, and
come up with something that avoids many of the issues that have been driving
users to need to configure things at all.
[Wang, Quanxian] Very useful. Different view and experience will remind what missed, what to be considered more. Thank Bryce.
Bryce
You should also check out how display configuration is currently being
done on OSX and Windows. Last I looked at them, they both had
streamlined it a good bit, and that's the style of operation users
expect now. But I think there's still room to do even better, still.

Bryce

Loading...