When weston opens a device, why does it use two file descriptors?

[Wang, Quanxian]

Hi,

When I start weston in drm backend, I have found weston opens the every device 
with two fds. I am a little confused by that.
For example, for keyboard device /dev/input/event5, in device_added, it will 
open the device. Found it returns fd 34, however fd 35 is also used by this 
operation(from the below information). Anyone knows why?

Weston process number is 668.

root:/quanxian ls -l /proc/668/fd
lrwx--. 1 root root 64 Jul  3 19:53 0 - /dev/pts/0
lrwx--. 1 root root 64 Jul  3 19:53 1 - /dev/pts/0
lrwx--. 1 root root 64 Jul  3 19:53 13 - /dev/dri/card0
lrwx--. 1 root root 64 Jul  3 19:53 14 - /dev/dri/card0
lrwx--. 1 root root 64 Jul  3 19:53 15 - socket:[17763]
lrwx--. 1 root root 64 Jul  3 19:53 16 - socket:[17763]
lrwx--. 1 root root 64 Jul  3 19:53 17 - socket:[17764]
lrwx--. 1 root root 64 Jul  3 19:53 18 - socket:[17764]
lrwx--. 1 root root 64 Jul  3 19:53 22 - /dev/input/event3
lrwx--. 1 root root 64 Jul  3 19:53 23 - /dev/input/event3
lrwx--. 1 root root 64 Jul  3 19:53 24 - /dev/input/event0
lrwx--. 1 root root 64 Jul  3 19:53 25 - /dev/input/event0
lrwx--. 1 root root 64 Jul  3 19:53 26 - /dev/input/event1
lrwx--. 1 root root 64 Jul  3 19:53 27 - /dev/input/event1
lrwx--. 1 root root 64 Jul  3 19:53 28 - /dev/input/event7
lrwx--. 1 root root 64 Jul  3 19:53 29 - /dev/input/event7
lrwx--. 1 root root 64 Jul  3 19:53 30 - /dev/input/event8
lrwx--. 1 root root 64 Jul  3 19:53 31 - /dev/input/event8
lrwx--. 1 root root 64 Jul  3 19:53 32 - /dev/input/event4
lrwx--. 1 root root 64 Jul  3 19:53 33 - /dev/input/event4
lrwx--. 1 root root 64 Jul  3 19:53 34 - /dev/input/event5
lrwx--. 1 root root 64 Jul  3 19:53 35 - /dev/input/event5
lrwx--. 1 root root 64 Jul  3 19:53 36 - /dev/input/event6
lrwx--. 1 root root 64 Jul  3 19:53 37 - /dev/input/event6

Thanks

Regards

Quanxian Wang
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

Hi, Pq

Thanks for your comment and idea.

 I list several cases for discussion.

Case 1:
Original:
├──
│ B   │ 
└──
Action: Move A leftof B
├───┼───┤
│ A │ B │
└───┴───┘ 
As you said, B must not change, A will have negative coordinates. That is fine.

Case 2:
Original:
├───┼───┤
│ C │ D │E
└───┴───┘ 

Move D leftof C:
C must not change.
How about E? E does not need change.
├───┼───┤
│ D │ C  │ | E
└───┴───┘
D will have negative coordinates. A big hole is left. It is not accepted by 
user at least I think so.
An optimization is E will move left and is side with C 
├───┼───┤
│ D │ C  │E
└───┴───┘

Case 3:
├───┼───┤
│ C │ D │E  |  F
└───┴───┘
Action: Move E leftof D
C, D's coordinate will not change. E and C will have the same relation with D, 
however they are not clone.
F will be move forward. Right?
├───┼───┤
│C/E│D |  F
└───┴───┘

Case 4:
├───┼───┤
│ C │ D │E  |  F
└───┴───┘
││  │ G   |  
└───┴───┘
Action: Move E leftof D
C, D's coordinate will not change. E and C will have the same relation with D.
F will be move forward. The relation between F and G will be built up. Maybe 
you want G to be moved with E. That will be crazy. 
├───┼───┤
│C/E│D |  F |
└───┴───┘
│  ││ G|  
└───┴───┘

After the cases above, list a complex one. 

├───┼───┤
│  │H  |   I |
└───┴───┘
│  ││ G|  
├───┼───┤
│C/E│D |  F | K/L│M
└───┴─┘
│  ││J || | N|  
└───┴───┘

Move F leftof H, and find some output to take F's position. Then chain reaction 
will happen.
Here you will define an order who take the place. For example, K/L will take 
the place of F. What is the relation between G and K or L? what is the relation 
between J and K or L?
Things turns to be much more complex. Just have a think what will be. It is an 
interesting thing.

After that, you find a conclusion.
1) position to new place will be easy.
2) find replacer and rebuild the relations will be complex. 
3) Chain reaction will make you crazy. Because no one expect how many outputs 
will be available and how many links after that. Maybe 1, or 2 or 3 or more...
 It is very easy to be in a loop. And more hole in layout happens.

Anyway, it is a little hard to make everything happy. :)

Thanks

Regards

Quanxian Wang


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Wednesday, May 7, 2014 2:17 PM
 To: Wang, Quanxian
 Cc: Hardening (rdp.eff...@gmail.com); Bryce W. Harrington
 (b.harring...@samsung.com); wayland-devel@lists.freedesktop.org; Jason
 Ekstrand (ja...@jlekstrand.net); Kang, Jeong Seok; Fu, Michael
 Subject: Re: [Weston] More discussion about weston output relative motion
 algorithm
 
 On Wed, 7 May 2014 03:26:56 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Thanks Pq's comment. Before you continue my comment, I mentioned one
 point.
  This is output movement algorithm and it is completely different with pre-
 configuration of output in weston.ini.
  For output, no alive, no movement.
  If you want to keep pre-configuration in weston.ini when output change,
 just keep it in some place and follow it when pre-defined output is plugged
 or unplugged. But anyway, it is a static configuration.  It is conflict with
 dynamic output movement.
 
 
 Of course it is, but users expect both to work at the same time, not have an
 option to choose between dynamic and static configuration. They want to
 configure what the dynamic code will do on hotplug, hence these two cases
 are inseparable.
 
 The weston-randr protocol could be seen as a way to modify the static
 configuration in-memory and then trigger a re-layout.
 
 Btw. there is no reason to avoid negative global coordinates. If you have
 output A at 0,0 and add output B to the left of it, it is perfectly valid to 
 put B
 at -widthB,0. You *can* add to the left or above without moving all the
 existing outputs. Also nothing says that you have to have an output with
 origin at 0,0. (There may be bugs, but that is all.)
[Wang, Quanxian] no negative coordinate happens. Top left most is the (0,0), it 
is the primary output. 
When you move output left of or above primary output(0,0), other outputs will 
be moved consistently based on the change. Once you primary output is changed, 
for example in this case, primary output will be changed to another output.  
All others outputs' coordinate will be calculated again. But the relative 
position between outputs will not be changed. 
Top left most output is the root of tree. All other outputs must be descendants 
of it.(including clone link, hlink, vlink).  Output movement algorithm have 
implemented that. Negative coordinate is invalid.
 
 If you haven't, and I certainly have not, you really should study how existing
 monitor layout algorithms work. Do not set out to replicate them as is, but
 find out how they solve the problems and whether those solutions would be
 applicable here.
[Wang, Quanxian] You are right. Refer to others could get more valuable idea 
and be helpful to make algorithm stronger. My goal is to provide a reasonable 
algorithm for weston output movement. 
 
 For getting this work forward and upstream, I think it is much less
 controversial to first expand weston.ini to allow setting other than just
 horizontal line of outputs. That would be interesting to a lot more people
 than the dynamic configuration protocol, and you would still need to deal
 with hotplug.
 
 Btw. I don't understand the comment no alive, no movement.
[Wang, Quanxian] if output is not active, no movement should happens on this 
output. Any movement on such kind of output will be invalid.
The inactive status include two status.
 a)unplugged-hardware disable  
b) plugged but disabled - software disable
 
 
 Thanks,
 pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Wednesday, May 7, 2014 3:41 PM
 To: Wang, Quanxian
 Cc: Hardening (rdp.eff...@gmail.com); Bryce W. Harrington
 (b.harring...@samsung.com); wayland-devel@lists.freedesktop.org; Jason
 Ekstrand (ja...@jlekstrand.net); Kang, Jeong Seok; Fu, Michael
 Subject: Re: [Weston] More discussion about weston output relative motion
 algorithm
 
 On Wed, 7 May 2014 07:03:32 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
 
 
   -Original Message-
   From: Pekka Paalanen [mailto:ppaala...@gmail.com]
   Sent: Wednesday, May 7, 2014 2:17 PM
   To: Wang, Quanxian
   Cc: Hardening (rdp.eff...@gmail.com); Bryce W. Harrington
   (b.harring...@samsung.com); wayland-devel@lists.freedesktop.org;
   Jason Ekstrand (ja...@jlekstrand.net); Kang, Jeong Seok; Fu, Michael
   Subject: Re: [Weston] More discussion about weston output relative
   motion algorithm
  
   On Wed, 7 May 2014 03:26:56 +
   Wang, Quanxian quanxian.w...@intel.com wrote:
  
Thanks Pq's comment. Before you continue my comment, I mentioned
one
   point.
This is output movement algorithm and it is completely different
with pre-
   configuration of output in weston.ini.
For output, no alive, no movement.
If you want to keep pre-configuration in weston.ini when output
change,
   just keep it in some place and follow it when pre-defined output is
   plugged or unplugged. But anyway, it is a static configuration.  It
   is conflict with dynamic output movement.
   
  
   Of course it is, but users expect both to work at the same time, not
   have an option to choose between dynamic and static configuration.
   They want to configure what the dynamic code will do on hotplug,
   hence these two cases are inseparable.
  
   The weston-randr protocol could be seen as a way to modify the
   static configuration in-memory and then trigger a re-layout.
  
   Btw. there is no reason to avoid negative global coordinates. If you
   have output A at 0,0 and add output B to the left of it, it is
   perfectly valid to put B at -widthB,0. You *can* add to the left or
   above without moving all the existing outputs. Also nothing says
   that you have to have an output with origin at 0,0. (There may be
   bugs, but that is all.)
  [Wang, Quanxian] no negative coordinate happens. Top left most is the
  (0,0), it is the primary output.
 
 You misunderstood. I mean that sometimes it is *good* to use negative
 coordinates, exactly to avoid moving the world, and that is *perfectly ok*.
 
 Moving the world (all existing outputs) is usually not what a user might
 expect when adding a new output.
 
 The top-left most output does not need to be at 0,0.
 
  When you move output left of or
  above primary output(0,0), other outputs will be moved consistently
  based on the change. Once you primary output is changed, for example
  in this case, primary output will be changed to another output.  All
  others outputs' coordinate will be calculated again. But the relative
  position between outputs will not be changed. Top left most output is
  the root of tree. All other outputs must be descendants of
  it.(including clone link, hlink, vlink).  Output movement algorithm
  have implemented that. Negative coordinate is invalid.
 
 I'm not talking about your specific algorithm anymore. I am talking in 
 general.
 
 Existing outputs are usually expected to stay put, when adding outputs to
 the extremes. Literally that means that the position of the existing outputs
 must not change, but only the position of the new output is computed
 appropriately, even if it results in negative coordinates.
 
 If I have one output active, and I add another output to the left of it, I
 certainly do not expect all windows to jump from the existing output to the
 new output. IOW, the position of the existing output must not change, and
 the new output will get a negative position.
 
 This is about the absolute global coordinates, not the relative positioning
 between outputs.
[Wang, Quanxian] Hi, Pq
After getting your opinion, I called back my initial thought for this 
algorithm.  I ever thought it like you, but after draw them one by one, I found 
I was wrong.
The following information will tell you how to get the principle of algorithm.

How to get that principle?
Take a very simple example, it is a horizontal link.
Current output layout:
A-B-C-D
Next action:  Move new E output into left of C
A-B-E-C-D

Question: Can you keep un-changed on all output's coordinate? 
Absolutely not. C and D will be changed(right extension) or A and B are changed 
(left extension). Take common case to allow right extension.
E will take place of C, C and D's coordinate will be changed at the same time. 
It is the minor change.
The same thing for moving E leftof A. They are the same principle. Therefore in 
this case no negative happens. We implicitly support right extension.

Now you get the principle

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

Hi, Pq

 
 I meant this very simple example:
 
 There is only one output A.
 
 A
 
 Then I add output B to the left of A.
 
 B-A
 
 The position of A must not change. This means that B must get negative
 coordinates, and A's position stays at 0,0. In Weston, window positions are
 stored in global coordinates, and to keep windows that were on output A,
 still on output A, the position of A must not change.
[Wang, Quanxian] This case is a special case, from it, you could not find the 
principle. Because in this case, you have the choice 'A must not change' or 'A 
must change'.
But for C-B-A, it will be very clear why we follow the principle.
We must think about how to make the principle to be accepted anywhere instead 
of special case. If we randomly define the rule, will make everything complex. 
It is not what we want.
By the way, when moving output, the move event will be sent to windows in this 
output. The window will be moved at the same time. 
 
 Note, that you cannot infer this from the syntax B-A, but you can from
 put B left-of A: B is being set and A is the reference point.
 
 
 Another case:
 
 First we have
 A-B
 
 Then add C right-of A.
 
 Now you could get the chain A-C-B, or C could end up cloning B. I do not
 care too much on what happens there, since it is sort of ill-defined. But I do
 care about the first case where we are not adding in the middle.
 
  Now you get the principle from simple link. And then vlink is the same
  thing. And then combine vlink and hlink together. At last I got this
  algorithm. I know principle will exist in both complex and simple
  layout. The algorithm can be summarized from them. Therefore my first
  step is to study simple case and find what the rule is. And then turn
  to complex and enhance the algorithm. Make the algorithm to be
  complete.
 
  Here I list several questions for you.
  1) Why use tree to link all outputs together?
  If just use coordinate to calculated the x, y, you will be lost.
  Because when you moving output, you can't avoid affecting the position
  of other outputs. You have to review all coordinates of outputs and
  make decision who is left, right, above, below. It will be much more
  complex. But if you use tree to link all, everything is very clear.
  Who is left, right, above, below. It is very clear.
  Coordinate and relative motion could be easily figured out.
 
  It will be very easy to control the position and movement of outputs.
 
 My alternate proposition was to keep the ordered list of configuration
 directives instead of a tree structure. Not just x,y for each live output.
 
 Instead of going through the tree to assing new positions, you would
 execute the list of directives.
 
 You would also need some additional rule on where to add outputs that are
 not placed by any directive. Most likely this rule would just create 
 directives
 for the outputs.
 
 Whether that would work or not, I am not sure.
 
  2) Why one root
  One root will tell you, this is the top left most. No other outputs
  will be left or above it. From this, you can calculate and review all
  outputs. No one is lost.
 
 Your definition of top-left most causes you to have problems in realizing an
 arrangement where the top-left output does not exist, e.g.:
 
 - C
 B A
 
 OTOH with directives, there is no need to define a root output.
 
 Directives could also refer to inactive outputs. Inactive outputs would simply
 have zero size, and the directive would be executed as always.
[Wang, Quanxian] Let me think about that case. In current algorithm, this case 
will not happen. The algorithm design will avoid such condition. But in 
practice, this layout really exists.
The solution could be:
Root output should exist because it will link all outputs. We really need that. 
 But I like you suggestion to make root virtual (just as you said, zero size). 
At the same time we can define gap for every output. In this case, C'x 
coordinate will have this gap.
So the link could be 
Root-hlink = C (x's gap is x)
Root-vlink = B  (y's gap is y)
C-vlink = A or B-hlink = A.

Is it reasonable? :)

If so, pos(x,y) will take into consideration.
But you definitely can not define such case
B-hlink = A
C-vlink = A
If you has such case, it will crash all. A can't have two parent because it 
will cause conflict of A's coordinate.
 
  3) Why only one parent?
  If two parents, you will find more links will cross each other. It is
  absolutely a disaster. (This could cause conflict of coordinates, from
  this path, it will A, for that path, it will B. A and B is not
  same.)
 
  I am not sure if you understand my idea. Sorry for my poor language.
 
 No, I do understand the tree structure well. It is just the rationale behind
 that is unclear, it clearly has some reasonable cases it cannot represent, and
 some IMO surprising behaviour.
 
 
 Thanks,
 pq
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Tuesday, May 6, 2014 2:36 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org; Jason Ekstrand
 (ja...@jlekstrand.net); Bryce W. Harrington (b.harring...@samsung.com);
 Fu, Michael; Hardening (rdp.eff...@gmail.com); Kang, Jeong Seok
 Subject: Re: [Weston] More discussion about weston output relative motion
 algorithm
 
 On Wed, 16 Apr 2014 04:21:46 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Clear some definition for easy understanding.
  Supposed B is beside A. B maybe leftof, rightof, above, or below A.
  If leftof,  output(B) is the horizontal parent of A.
  If above, output(B) is the vertical parent of A.
  If rightof, output(B) is the horizontal child of A.
  If below, output(B) is the vertical child of A.
 
  With definition, you will be easy to set up a binary tree. But you may
  think some node may have two parent. The answer is no, you can have a
  try to move and will find it is right. Just let you know my mistakes I
  ever made. When you move output to be leftof or above of another, the
  output will take the place of another instead of really above or left
  of the position of another. :) After that, you will be know why one
  node has  one parent at most.
 
  Of course, the original layout will be one horizontal line for
  extended mode.
 
  If clone mode is supported, that is fine. Keep clone link for that in
  output structure. The algorithm will be updated with minimal. But it
  will be easy and based on clone mode design.
 
 Hi,
 
 I have nothing against an algorithm for placing outputs in Weston, and it
 would fit Weston core. I think we would like to support all kinds of layouts
 based on weston.ini alone, even without any dynamic configuration like your
 proposed protocol extension.
 
 However, from your algorithm description, I am not sure how it'll work for
 hotplug or hot-unplug.
[Wang, Quanxian] The algorithm will process all dynamic layout change. If you 
hotplug, just add one output into the tree, just call enable_node(A), supposed 
you hot-plug A. The initial design is inserted it into tail of master's hlink. 
(master is at (0,0)). 
When you hot-unplug, that means you delete this output from the tree. Following 
the algorithm, just call delete_node(A), supposed you hot-unplug A. the detail 
algorithm, you can refer to the disable_node algorithm in V4.  The order to 
take its place is clone link, horizontal link and vertical link. (If no clink, 
then hlink, still no, vlink)
In file module/wrandr/randr_layout_algorithm.h
115  * Disable Node Rules:
116  * 1) The node is deleted and the place is replaced by others.
117  * 2) The order to take the place of parent:
118  *clink, hlink and then vlink.
119  * Others are the same as clean_node.
 
 Let's imagine a physical setting of four monitors, arranged like this:
 
 ┌───┬───┐
 │ A │ B │
 ├───┼───┤
 │ C │ D │
 └───┴───┘
 
 That is a 2x2 grid.
 
 There are two ways to represent this in your tree structure, I believe:
 
 A.hlink - B
 A.vlink - C
 C.hlink - D
 
 or
 
 A.hlink - B
 A.vlink - C
 B.vlink - D
 
[Wang, Quanxian] Firstly, you imagine the grid. But from the very beginning,  C 
is not present. You can do like this.
a) Initial status
 ┌───┬───┐
 │ A │ B │
 ├───┼───┤
 │ │ D │
 └───┴───┘
 b) hot-plugged C and move C below of A
 ┌───┬───┐
 │ A │ B │
 ├───┼───┤
 │ C  │ D │
 └───┴───┘
Remember, since D is below of B, there will not any relation between C and D. 
Otherwise D will have two parents (horizontal C and vertical B).

 What happens, if C is not present originally and gets hotplugged?
[Wang, Quanxian] 
Original status: C is not present
 ┌───┬───┐
 │ A │ B │
 ├───┼───┤
 │ │ D │
 └───┴───┘
 A.hlink - B
 B.vlink - D

C is hot-plugged, it will go to tail of Master(A)'s hlink.
┌───┬───┐
 │ A │ B │C |
 ├───┼───┤
 │   │ D │
 └───┴───┘
A.hlink - B
B.hlink - C
B.vlink - D
 What happens, if C gets hot-unplugged? And then plugged back?
[Wang, Quanxian] 
C is hot-unplugged based on previous. In previous status, C has no any child 
link (clink, hlink, vlink)
┌───┬───┐
 │ A │ B │|
 ├───┼───┤
 │   │ D │
 └───┴───┘
C is hot-plugged, it will be as before.
┌───┬───┐
 │ A │ B │C |
 ├───┼───┤
 │   │ D │
 └───┴───┘
 What happens, if a user adds a new output and defines it to be left-of C? Will
 C end up below B or will it stay below A?
[Wang, Quanxian] 
If a new output called E is added and define it as left-of C. It will do two 
actions. One is add, another is move.
Adding:
┌───┬───┐
 │ A │ B │C | E
 ├───┼───┤
 │   │ D │
 └───┴───┘
Move: move E left of C (it will have the actions: remove E, and then E replace 
C'place, C is moving to right of E)
 ┌───┬───┐
 │ A │ B │E  | C
 ├───┼───┤
 │   │ D │
 └───┴───┘
 What if A is wider than C; will there be a gap between C and D?
[Wang, Quanxian] 
You imagine
 ┌───┬───┐
 │ A │ B   |
 ├───┼───┤
 │ C  │ D │
 └───┴───┘

A's hlink == B
A's vlink = C
B's vlink = D

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

Thanks Pq's comment. Before you continue my comment, I mentioned one point.
This is output movement algorithm and it is completely different with 
pre-configuration of output in weston.ini.
For output, no alive, no movement.
If you want to keep pre-configuration in weston.ini when output change, just 
keep it in some place and follow it when pre-defined output is plugged or 
unplugged. But anyway, it is a static configuration.  It is conflict with 
dynamic output movement.

Currently in my weston randr's patches has provided an interface to put your 
configure requests into one file. 

 -Original Message-
 From: wayland-devel [mailto:wayland-devel-boun...@lists.freedesktop.org]
 On Behalf Of Pekka Paalanen
 Sent: Tuesday, May 6, 2014 5:16 PM
 To: Wang, Quanxian
 Cc: Hardening (rdp.eff...@gmail.com); Bryce W. Harrington
 (b.harring...@samsung.com); wayland-devel@lists.freedesktop.org; Jason
 Ekstrand (ja...@jlekstrand.net); Kang, Jeong Seok; Fu, Michael
 Subject: Re: [Weston] More discussion about weston output relative motion
 algorithm
 
 On Tue, 6 May 2014 07:26:32 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
 
 
   -Original Message-
   From: Pekka Paalanen [mailto:ppaala...@gmail.com]
   Sent: Tuesday, May 6, 2014 2:36 PM
   To: Wang, Quanxian
   Cc: wayland-devel@lists.freedesktop.org; Jason Ekstrand
   (ja...@jlekstrand.net); Bryce W. Harrington
   (b.harring...@samsung.com); Fu, Michael; Hardening
   (rdp.eff...@gmail.com); Kang, Jeong Seok Subject: Re: [Weston] More
   discussion about weston output relative motion algorithm
  
   On Wed, 16 Apr 2014 04:21:46 +
   Wang, Quanxian quanxian.w...@intel.com wrote:
  
Clear some definition for easy understanding.
Supposed B is beside A. B maybe leftof, rightof, above, or below
A. If leftof,  output(B) is the horizontal parent of A.
If above, output(B) is the vertical parent of A.
If rightof, output(B) is the horizontal child of A.
If below, output(B) is the vertical child of A.
   
With definition, you will be easy to set up a binary tree. But you
may think some node may have two parent. The answer is no, you can
have a try to move and will find it is right. Just let you know my
mistakes I ever made. When you move output to be leftof or above
of another, the output will take the place of another instead of
really above or left of the position of another. :) After that,
you will be know why one node has  one parent at most.
   
Of course, the original layout will be one horizontal line for
extended mode.
   
If clone mode is supported, that is fine. Keep clone link for that
in output structure. The algorithm will be updated with minimal.
But it will be easy and based on clone mode design.
  
   Hi,
  
   I have nothing against an algorithm for placing outputs in Weston,
   and it would fit Weston core. I think we would like to support all
   kinds of layouts based on weston.ini alone, even without any dynamic
   configuration like your proposed protocol extension.
  
   However, from your algorithm description, I am not sure how it'll
   work for hotplug or hot-unplug.
  [Wang, Quanxian] The algorithm will process all dynamic layout change.
  If you hotplug, just add one output into the tree, just call
  enable_node(A), supposed you hot-plug A. The initial design is
  inserted it into tail of master's hlink. (master is at (0,0)). When
  you hot-unplug, that means you delete this output from the tree.
  Following the algorithm, just call delete_node(A), supposed you
  hot-unplug A. the detail algorithm, you can refer to the disable_node
  algorithm in V4.  The order to take its place is clone link,
  horizontal link and vertical link. (If no clink, then hlink, still no,
  vlink) In file module/wrandr/randr_layout_algorithm.h 115  *
  Disable Node Rules: 116  * 1) The node is deleted and the place
  is replaced by others. 117  * 2) The order to take the place of
  parent: 118  *clink, hlink and then vlink. 119  * Others
  are the same as clean_node.
  
   Let's imagine a physical setting of four monitors, arranged like
   this:
  
   ┌───┬───┐
   │ A │ B │
   ├───┼───┤
   │ C │ D │
   └───┴───┘
  
   That is a 2x2 grid.
 
 Hi,
 
 each question I made was independent, and supposed to start from this
 particulat setup. Not as a sequence of operations, but always starting with
 this.
[Wang, Quanxian] you expected it in pre-configuration. The algorithm is 
designed for output alive. For output, no alive, no movement.
 
  
   There are two ways to represent this in your tree structure, I
   believe:
  
   A.hlink - B
   A.vlink - C
   C.hlink - D
  
   or
  
   A.hlink - B
   A.vlink - C
   B.vlink - D
  
  [Wang, Quanxian] Firstly, you imagine the grid. But from the very
  beginning,  C is not present. You can do like this. a) Initial status
  ┌───┬───┐  │ A │ B │  ├───┼───┤
   │ │ D │
   └───┴───┘
   b) hot-plugged C and move C below

help: is there any way to use integer 64 type in protocol?

[Wang, Quanxian]

Hi,

Is there any way to use type of integer 64bit in protocol?

Currently I found I could not use 64bit. Any suggestion for that?

Only below basic data types are supported in wayland.

254 union wl_argument {
255 int32_t i; /** signed integer */
256 uint32_t u; /** unsigned integer */
257 wl_fixed_t f; /** fixed point */
258 const char *s; /** string */
259 struct wl_object *o; /** object */
260 uint32_t n; /** new_id */
261 struct wl_array *a; /** array */
262 int32_t h; /** file descriptor */
263 };

Regards

Thanks

Quanxian Wang


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: help: is there any way to use integer 64 type in protocol?

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Monday, April 21, 2014 6:32 AM
To: Pekka Paalanen
Cc: Wang, Quanxian; wayland-devel@lists.freedesktop.org
Subject: Re: help: is there any way to use integer 64 type in protocol?



On Sun, Apr 20, 2014 at 1:17 PM, Pekka Paalanen 
ppaala...@gmail.commailto:ppaala...@gmail.com wrote:
On Sun, 20 Apr 2014 11:10:15 +
Wang, Quanxian quanxian.w...@intel.commailto:quanxian.w...@intel.com 
wrote:

 Hi,

 Is there any way to use type of integer 64bit in protocol?
No. You will have to use two 32-bit arguments, or propose a patch
to add 64-bit types in a completely backwards-compatible way. I am
not sure what the latter option would entail.

Adding 64-bit types wouldn't be too hard.  It would involve adding a int64_t an 
uint64_t types to wl_argument (I'd call them U and I personally) and adding 
code throughout libwayland to parse them.  In terms of backwards compatibility, 
it should be fine as long as you make it 100% clear that your new protocol 
extension uses the new 64bit types and therefore requires the newer libwayland 
version.
The other option is that you could do what Pekka did in the presentation 
extension and split it into two 32-bit parts.  What do you want to represent 
that needs a 64-bit type?
[Wang, Quanxian] 32 is too short. Currently I use 32bit in Weston randr, it 
could only stand for 16 type of operations (every type use 2 bits, however 12 
of 16 have been used.). It will be fine to use 64 bit or more for future 
extension. I will try to add a patch for that. Thanks for your comment.
Thanks,
--Jason Ekstrand


The presentation extension could make use of a 64-bit type, too.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

Hi, All

Relative motion of output is needed in Weston compositor or others. For 
example, plugin a monitor, you want move it above or leftof or rightof or below 
others.

More details about this motion algorithm will be shown in the email. Before 
that, I mention one point, I don't refer any code from others including 
xserver. My idea is based on the current Weston compositor design and 
statistics. It is original for Weston compositor. At the same time, when I 
implemented it, I try to make parameters to be general. If possible, make it 
common instead of only in randr protocol.
Weston movement algorithm

If only support one line layout. For example, all vertical or all horizontal, 
it is very easy to implement. But if we want support both at the same time, it 
will be more complex instead of 1+1=2. Currently in Weston compositor, only 
horizontal is supported. Compositor uses output_list to make it happen.

Here are the summary of motion properties.


1)  It is similar with binary tree.  Here are some related features.  Every 
node is one output.

a.   Every node have one parent at most, also the parent maybe from 
vertical, maybe from horizontal(it is different with binary tree). But never 
has two parents. I will not explain why, you can use 4 outputs to do more 
movement, and you can find this.

b.  Every node have two children at most. One is for vertical, another is 
for horizontal.

c.   The tree will link all outputs, if you find some output is not in 
link, that means link is lost. So when you plugin or output, you need make the 
link complete. When scan Weston.ini configure, you need to organize the tree 
following this algorithm.

d.  Only one start point(root) exists whose coordinate is x=0 and y=0). 
From that, you could find all other outputs and position them from left up most 
to right down most.

e.  Every branch from the main branch will never cross each other.

2)  The tree is double linked.

3)  The only have-to interrupt is when one node is deleted, which child 
takes its place? Currently I force its horizontal child will take its place.

By the way,  I list some statistics. Maybe you could find some interesting why 
I post more effort on this algorithm. Layout means the final result of all 
outputs' position.

Output_number  Possible_Layout_numberOne_line_possbile_layout_number

   1  1 
   1

2 2x2=4   (2=2x1)2

35x6=30 (6=3x2x1)6

4  14x24=336  (24=4x3x2x1)   24

You could find that, if we have 4 outputs, there are 14x24=336 possible layout 
types. So the algorithm has to be designed as complete instead of only for 2, 
or 3, or less. It should be designed for more. For example, supposed 500, 1000 
outputs. Yes, it is in theory. But the algorithm must be that. I enumerate them 
based on the movement and get the possible layouts. For example, when you have 
4 outputs, even if they are in one line, there are 4x3x2x1=24 layouts.

Algorithm Implementation:
To implement this algorithm, more links have to be added for output. I add 
vlink and hlink into output structure. Currently I implemented it in randr 
protocol (will be posted in next version of Weston randr protocol)

With that features above, two links(vlink and hlink) should be added into 
current output structure and the tree should be set up. (This will not affect 
the usage of original list in output). More patches will be needed.

1)  Initialization (auto found outputs and link them together)

2)  Dynamic plugin and out.

3)  If Weston.ini supports the layout, it should work with the algorithm.

4)  Randr protocol

Important:
Currently the algorithm is implemented in randr protocol. But I want to move it 
into common code instead of only randr protocol. I am not sure if it is right 
and where to put algorithm code. I need agreement and discussion for that. If 
no permission, I will let it in randr protocol (create another structure for 
output, plus vlink and hlink).

Thanks

Regards

Quanxian Wang


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [Weston] More discussion about weston output relative motion algorithm

[Wang, Quanxian]

Clear some definition for easy understanding.
Supposed B is beside A. B maybe leftof, rightof, above, or below A.
If leftof,  output(B) is the horizontal parent of A.
If above, output(B) is the vertical parent of A.
If rightof, output(B) is the horizontal child of A.
If below, output(B) is the vertical child of A.

With definition, you will be easy to set up a binary tree. But you may think 
some node may have two parent. The answer is no, you can have a try to move and 
will find it is right. Just let you know my mistakes I ever made. When you move 
output to be leftof or above of another, the output will take the place of 
another instead of really above or left of the position of another. :) After 
that, you will be know why one node has  one parent at most.

Of course, the original layout will be one horizontal line for extended mode.

If clone mode is supported, that is fine. Keep clone link for that in output 
structure. The algorithm will be updated with minimal. But it will be easy and 
based on clone mode design.

Thanks

Regards

Quanxian Wang


From: Wang, Quanxian
Sent: Wednesday, April 16, 2014 11:26 AM
To: wayland-devel@lists.freedesktop.org
Cc: Wang, Quanxian; Pekka Paalanen (ppaala...@gmail.com); Jason Ekstrand 
(ja...@jlekstrand.net); Bryce W. Harrington (b.harring...@samsung.com); Fu, 
Michael; Hardening (rdp.eff...@gmail.com); Kang, Jeong Seok
Subject: [Weston] More discussion about weston output relative motion algorithm

Hi, All

Relative motion of output is needed in Weston compositor or others. For 
example, plugin a monitor, you want move it above or leftof or rightof or below 
others.

More details about this motion algorithm will be shown in the email. Before 
that, I mention one point, I don't refer any code from others including 
xserver. My idea is based on the current Weston compositor design and 
statistics. It is original for Weston compositor. At the same time, when I 
implemented it, I try to make parameters to be general. If possible, make it 
common instead of only in randr protocol.

Weston movement algorithm

If only support one line layout. For example, all vertical or all horizontal, 
it is very easy to implement. But if we want support both at the same time, it 
will be more complex instead of 1+1=2. Currently in Weston compositor, only 
horizontal is supported. Compositor uses output_list to make it happen.

Here are the summary of motion properties.


1)  It is similar with binary tree.  Here are some related features.  Every 
node is one output.

a.   Every node have one parent at most, also the parent maybe from 
vertical, maybe from horizontal(it is different with binary tree). But never 
has two parents. I will not explain why, you can use 4 outputs to do more 
movement, and you can find this.

b.  Every node have two children at most. One is for vertical, another is 
for horizontal.

c.   The tree will link all outputs, if you find some output is not in 
link, that means link is lost. So when you plugin or output, you need make the 
link complete. When scan Weston.ini configure, you need to organize the tree 
following this algorithm.

d.  Only one start point(root) exists whose coordinate is x=0 and y=0). 
From that, you could find all other outputs and position them from left up most 
to right down most.

e.  Every branch from the main branch will never cross each other.

2)  The tree is double linked.

3)  The only have-to interrupt is when one node is deleted, which child 
takes its place? Currently I force its horizontal child will take its place.

By the way,  I list some statistics. Maybe you could find some interesting why 
I post more effort on this algorithm. Layout means the final result of all 
outputs' position.

Output_number  Possible_Layout_numberOne_line_possbile_layout_number

   1  1 
   1

2 2x2=4   (2=2x1)2

35x6=30 (6=3x2x1)6

4  14x24=336  (24=4x3x2x1)   24

You could find that, if we have 4 outputs, there are 14x24=336 possible layout 
types. So the algorithm has to be designed as complete instead of only for 2, 
or 3, or less. It should be designed for more. For example, supposed 500, 1000 
outputs. Yes, it is in theory. But the algorithm must be that. I enumerate them 
based on the movement and get the possible layouts. For example, when you have 
4 outputs, even if they are in one line, there are 4x3x2x1=24 layouts.

Algorithm Implementation:
To implement this algorithm, more links have to be added for output. I add 
vlink and hlink into output structure. Currently I implemented it in randr 
protocol (will be posted in next version of Weston randr protocol)

With that features above, two links(vlink and hlink) should be added into 
current output structure and the tree

Re: [PATCH V3 5/7] weston:Add the weston randr protocol implementation

[Wang, Quanxian]

Sorry to be later. Thanks for your comment.
On Wed, 2014-04-09 at 09:59 +0200, Hardening wrote:
 Le 08/04/2014 07:03, Quanxian Wang a écrit :
  Signed-off-by: Quanxian Wang quanxian.w...@intel.com
  ---
module/Makefile.am|3 +
module/wrandr/Makefile.am |   32 ++
module/wrandr/wrandr.c| 1262 
  +
3 files changed, 1297 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 000..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 000..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
  +
 
 [...]
 
  +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 void
  +randr_switch_mode(struct randr_output_request *request,
  + uint32_t *results)
  +{
  +   time_t time = 0;
  +   int i = 0, found = 0;
  +   int result = WRANDR_TYPE_RET_NOACT;
  +   int timing_mode = 0;
  +   int move_bits = 0;
  +   struct weston_mode *mode;
  +   struct weston_output *output = request-output;
  +
  +   if (request-flags  1WRANDR_TYPE_OOP_MODENUM) {
  +   timing_mode = 1;
  +   time = request-modenum.modi_time;
  +   *results |= result
  +  (WRANDR_TYPE_OOP_MODENUM * 2);
  +   }
  +
  +   if (request-flags  1WRANDR_TYPE_OOP_MODE) {
  +   *results |= result
  +  (WRANDR_TYPE_OOP_MODE * 2);
  +   if (request-mode-modi_time  time) {
  +   time = request-mode-modi_time;
  +   timing_mode = 2;
  +   }
  +   }
 
 [Hardening] Perhaps the case where request-flags has not bits set 
 should be treated with an error returned ?
No need to return error. In this function, we just check these two methods. 
Others will be processed in other function. If no flag, that is fine, no 
operation is needed.
 
  +
  +   switch (timing_mode) {
  +   case 1:
  +   i = 0;
  +   wl_list_for_each(mode, output-mode_list, link) {
  +   i++;
  +   if (i == request-modenum.num) {
  +   found = 1;
  +   break;
  +   }
  +   }
  +
  +   if (found != 1)
  +   mode = NULL;
  +   break;
  +   case 2:
  +   mode = randr_find_mode(output,
  +  request-mode-width,
  +  request-mode-height,
  +  request-mode-refresh);
  +   break;
  +   default:
  +   return;
  +   }
 
 [Hardening] related to my remark above, *results should be set in the 
 default: handling, as it catches the case where timing_mode == 0 
 which means no flags where set.
If timing_mode is 0, that means, no operation need to be done for switch mode. 
Just return is enough.
 
  +
  +
  +   if (!mode) {
  +   result = WRANDR_TYPE_RET_FAIL;
  +   } else {
  +   if (!(mode-flags  WL_OUTPUT_MODE_CURRENT) 
  +   output-switch_mode) {
  +   result = output-switch_mode(output, 

Re: [PATCH V3 0/7] Add weston randr protocol

[Wang, Quanxian]

Sorry to be later. Thanks for your comment. 

On Wed, 2014-04-09 at 08:44 +0200, Hardening wrote:
 Le 08/04/2014 07:03, Quanxian Wang a écrit :
  Important Changes compared with V2:
 
  1) Provide 2 methods to mode match for mode setting and mode delete.
  a) Exact mode number match
  Client selects the mode number according to query information.
 
 
 [...]
 
 
  5) Add above and below move action of output.
  Currently it is the empty because compositor's related functions are 
  not ready.
 
  6) Newmode request is provided for RDP backend, but it should be 
  implemented in backend by RDP developer.
  Provide newmode option in apps(widthxheight or widthxheight@refresh)
  Provide newmode request in randr protocol to create a simple mode 
  without detail timing information.
 
 
 With RDP we know only width / height / bpp, so the timing callbacks 
 will be very easy to implement...
Right, it is easy for RDP guys to know how to do that. I like RDP guy could 
help to do that because they are familiar with RDP backend.  For others 
backend, it will be the same.
I could provide the patch for that if no one like to do that. At least, I think 
I should make these patches series to be accepted in upstream tree and then do 
the next. :)

Keep it.
 

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH V3 4/7] Add new mode function in drm backend

[Wang, Quanxian]

Sorry to be later. Thanks for your comment.

On Wed, 2014-04-09 at 08:36 +0200, Hardening wrote:
 Le 08/04/2014 07:03, Quanxian Wang a écrit :
  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.w...@intel.com
  ---
src/compositor-drm.c | 76 
  
1 file changed, 76 insertions(+)
 
  diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 
  154e15e..57e0585 100644
  --- a/src/compositor-drm.c
  +++ b/src/compositor-drm.c
  @@ -1390,6 +1390,80 @@ drm_output_add_mode(struct drm_output *output, 
  drmModeModeInfo *info)
}
 
 
 [...]
 
  +
  +static struct weston_mode *
  +drm_output_new_timing(struct weston_output *output,
  + uint32_t clock,
  + int hdisplay,
  + int hsync_start,
  + int hsync_end,
  + int htotal,
  + int vdisplay,
  + int vsync_start,
  + int vsync_end,
  + int vtotal,
  + int vscan,
  + uint32_t flags)
  +{
  +   drmModeModeInfo *modeinfo;
  +   struct drm_mode *mode = NULL;
  +
  +   modeinfo = malloc(sizeof(*modeinfo));
  +   if (modeinfo == NULL)
  +   return NULL;
  +   memset(modeinfo, 0x0, sizeof(*modeinfo));
 
 Hardening: you should use zmalloc here
Got that. Thanks
 
  +
  +   modeinfo-type = DRM_MODE_TYPE_USERDEF;
  +   modeinfo-hskew = 0;
  +   modeinfo-vrefresh = 0;
  +   modeinfo-hdisplay = hdisplay;
 
 [...]
 
 +
  +static int
drm_subpixel_to_wayland(int drm_value)
{
  switch (drm_value) {
  @@ -2046,6 +2120,8 @@ 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_timing = drm_output_new_timing;
  +   output-base.compare_timing = drm_output_compare_timing;
 
  output-base.gamma_size = output-original_crtc-gamma_size;
  output-base.set_gamma = drm_output_set_gamma;
 
 
 Regards.

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH V2 0/8] Add weston randr protocol

[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?
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.harring...@samsung.com]
Sent: Wednesday, April 02, 2014 10:53 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; ppaala...@gmail.com
Subject: Re: [PATCH V2 0/8] Add weston randr protocol

On Mon, Mar 24, 2014 at 07:39:12PM +0800, Quanxian Wang wrote:
 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

RE: [PATCH V2 1/8] wesston: Add weston randr protocol

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Friday, March 28, 2014 11:00 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH V2 1/8] wesston: Add weston randr protocol

On Mon, 24 Mar 2014 19:39:13 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 Weston protocol wrandr will provide interface to
 1) Mode set of output (scale, transform, mode)
 2) Position of output (currently support leftof, rightof)
 3) New a custom mode
 4) Delete mode

 This protocol is not expose public. It is only for QA testing and
 Admin configuration currently.

 Signed-off-by: Quanxian Wang quanxian.w...@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 000..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
 +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

RE: weston: weston randr protocol for testing and configuration

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Thursday, March 27, 2014 9:06 PM
To: Jasper St. Pierre
Cc: Wang, Quanxian; wayland-devel@lists.freedesktop.org
Subject: Re: weston: weston randr protocol for testing and configuration

On Thu, 27 Mar 2014 08:31:34 -0400
Jasper St. Pierre jstpie...@mecheye.net wrote:

 I don't think the user really knows what refresh is either.

 I'm actually curious: is there a reason to ever expose different modes
 to the user that have the same width/height but different timings?
 What's the rationale for choosing one instead of the other? I know
 nothing about display panels, keep in mind.

This is not for users, the whole weston-randr protocol is for developer and
administrator tools for testing different modes without having to restart 
Weston
and edit a config file in between.

I cannot see a reason to offer the same mode with different timings during 
normal
operations, but it would be useful for the very kind of experimentation that 
this
protocol extension is designed for.

At least, as far as I have understood. How far this interface should go is 
still unclear
to me. If Quanxian wants to stick with RandR 1.1 features, i.e. no dynamic 
custom
modelines, that's ok, though to me it seems like losing a good part of the 
possible
benefits.
[Wang, Quanxian] RandR protocol is just a reference. We take the ideas from it 
based on wayland/weston current mechanism. Custom should be supported. In my 
v2, I have provided newmode request, it provides the custom mode. But it is 
need to be enhanced in the future. I am planning to provide two ways for common 
and special case.

 I'm also assuming a modern system here with a modern display panel, so
 no saying the refresh means the CRT updates faster and it's brighter
 and less less flicker

My 1600x1200 monitor, connected via DVI-D, was unstable, going black once in a
while when ran with a normal timings mode at 60 Hz. Switching to reduced
blanking mode made it stable and got rid of the random green snow. Yes, you can
blame the hardware being on its edge.

The refresh rate was exactly the same. The reduced blanking mode just made the
pixel clock a bit lower, enough to make the picture stable.


Thanks,
pq

 On Thu, Mar 27, 2014 at 6:35 AM, Pekka Paalanen ppaala...@gmail.com
wrote:

  On Thu, 27 Mar 2014 04:08:15 +
  Wang, Quanxian quanxian.w...@intel.com wrote:
 
   Hi, Pq
  
   The information to identify the unique mode: width, height and
   refresh
  are enough? Not enough in theory. But is enough in real world. I
  have checked with xrandr. Read the following comment.
  
   Welcome any comment for that.
  
   Thanks
  
+
+request name=set_mode
+  description summary=set the mode of output  Set the
+ mode of output.
+  /description
+  arg name=output type=object interface=wl_output
+   summary=the output object/
+  arg name=width type=int summary=width of the mode
+ in
   hardware units/
+  arg name=height type=int summary=height of the
+ mode in
   hardware units/
+  arg name=refresh type=int summary=vertical refresh
+rate in mHz/
   
   So this is the simple mode set request.
   
   Do you think width/height/refresh is really enough to identify a mode?
  I don't
   think so. I think in the early days of X11 RandR, NVidia hit the
   same
  problem, and
   started to expose fake refresh values, only to be able to
   distinguish
  modes that
   were identical in width/height/refresh but still different in timings.
  Or actually I
   think it was much more complicated than that, but this is the
   point in
  simple terms.
   
   So we need something else here to identify a mode.
   
   Check what kind of protocol GNOME uses, and how current RandR
   protocol
  works.
   [Wang, Quanxian] Hi, Pq
   Your understanding are right in theory. But in reality, it is
   barely
  possible. Width and height could be easily same, not easy for
  refresh at the same time. Currently in xrandr, they use mode name to
  identify one mode (for example widthxheight_refresh). in xrandr
  process, I don't find they take mode information to compare. Maybe I
  missed, but from xrandr parameters, there is no such option. They
  just take width, height, refresh as mode name to identify one mode.
  Sometimes, only width and height. If we want to fully support one
  unique mode, all the mode information have to be compared. (clock,
  hdisplay, hsync_start, hsync_end, htotal, vdisplay, vsync_start,
  vsync_end, vtotal, flags). But it is not convenient. Sometime, user
  basically don't know what hdisplay is at all. They just know
  widthxheight_refresh. My idea is just take width, height, refresh as the 
  unique
id for mode.
  
 
  I assume the X server has the database of modes. I cannot think of
  any reason why the xrandr client would be comparing modes at all.
  Why did you expect that? Isn't the name of a mode in RandR just

RE: weston: weston randr protocol for testing and configuration

[Wang, Quanxian]

Hi, Pq

In weston randr v2, I don't include the following question. I am still in 
confused. Sorry about that.

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Saturday, March 22, 2014 8:20 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: weston: weston randr protocol for testing and configuration


Talking about threads here is misleading. Such threaded operations do not make
sense to begin with. The question is more about ambiguity between operations
and acknowledgements, and preferring explicit correspondence than relying on
protocol message ordering.
So the goal is good, but the rationale is slightly wrong.

[Wang, Quanxian] For this issue, I am still confused Supposed that case, the 
first client is doing the randr without commit, however the second commit his 
request. This will breakdown the first client if they works on the same output. 
My suggestion is that allocating a request id for every request set. (first 
client request set, second client request set, .. etc), everyone could not 
break down others operation before commit. Otherwise clients will not control 
his actions.
Even in callback, you must don't want to get the callback done event when you 
are doing the action without commit.


 For example
 In client:
 randr_resource =
 wl_randr_set_transform(randr.randr,wayland_output,argument.transform);
 wl_randr_add_listener(randr_resource, randr_transform_listener,
 randr);

 In compositor:
 randr_resource = wl_resource_create(client,wl_randr_interface,1,
 action); ...
 wl_randr_send_action_done(randr_resource,
 1WL_RANDR_ACTION_TRANSFORM, ret, action);
 wl_resource_destroy(randr_resource);

I do not want a reply for every single part of output state, I want a reply 
for the
final commit request that applies all the state updates gathered so far. That 
will
also solve the atomicity problem.

See how wl_surface works, and add a reply object only to the commit request.
[Wang, Quanxian] Yes, when surface get a commit, it will applies all the state 
updates gathered so far. And then inform user that (frame callback called after 
weston_output_repaint), If client is updating the surface and without commit. 
Suddenly when he got an reply event that surface has update his previous 
changes. But he didn't run surface_commit at that time. Here I don't understand 
if there are some conflict. (The reply callback is created by surface_frame to 
keep tuning of his update.)
That means compositor does the update when client don't run surface commit. 
Sorry, I am really confused here. Actually it is the same as above question to 
avoid the conflict.

Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: weston: weston randr protocol for testing and configuration

[Wang, Quanxian]

Thanks, Pq

-Original Message-
From: wayland-devel [mailto:wayland-devel-boun...@lists.freedesktop.org] On
Behalf Of Pekka Paalanen
Sent: Wednesday, March 26, 2014 3:54 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: weston: weston randr protocol for testing and configuration

On Wed, 26 Mar 2014 06:47:50 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Hi, Pq

 In weston randr v2, I don't include the following question. I am still in 
 confused.
Sorry about that.

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Saturday, March 22, 2014 8:20 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: weston: weston randr protocol for testing and
 configuration
 
 
 Talking about threads here is misleading. Such threaded operations do
 not make sense to begin with. The question is more about ambiguity
 between operations and acknowledgements, and preferring explicit
 correspondence than relying on protocol message ordering.
 So the goal is good, but the rationale is slightly wrong.
 
 [Wang, Quanxian] For this issue, I am still confused Supposed that case, the 
 first
client is doing the randr without commit, however the second commit his 
request.
This will breakdown the first client if they works on the same output.
 My suggestion is that allocating a request id for every request set. (first 
 client
request set, second client request set, .. etc), everyone could not break down
others operation before commit. Otherwise clients will not control his actions.
 Even in callback, you must don't want to get the callback done event when you
are doing the action without commit.


Hi,

clients cannot and must not ever interfere with each other. On the protocol 
level,
they simply cannot interfere, because all protocol objects are private to a 
client
(connection).

As long as you keep all uncommitted state as per protocol object (wl_resource 
in
the server), there cannot happen any mixups. In other words, the not yet
committed state should not be stored in struct weston_output, but in some new
struct that is created per wl_output protocol object. This seems to require
replacing weston_output::resource_list with a new list, maybe.

There is no reason to create request ids at all.

The only thing you need, is that the commit request creates a protocol object, 
that
will then deliver the result of the commit. This is very similar to how
wl_surface.frame creates a new object, that then delivers the 'done' event. The
difference to wl_surface is only that the commit creates the object, not a 
separate
request, and the delivered event is different.

Even that is not strictly necessary, but it makes a nicer API for clients than 
just
relying on the order of received events when the result event was just in the
global interface.

I don't really understand what your concerns are, so maybe I didn't reply to 
them?
[Wang, Quanxian] Actually, I am worry about the multiple threads and clients to 
avoid the conflict. They maybe work on the same output.
I think I should think about that since it is a protocol for every client. But 
your comment make me clear more and more. I will refer to wl_surface code to 
check how to make it happen.
Thanks, Pq.

 
  For example
  In client:
  randr_resource =
  wl_randr_set_transform(randr.randr,wayland_output,argument.transfor
  m); wl_randr_add_listener(randr_resource,
  randr_transform_listener, randr);
 
  In compositor:
  randr_resource = wl_resource_create(client,wl_randr_interface,1,
  action); ...
  wl_randr_send_action_done(randr_resource,
  1WL_RANDR_ACTION_TRANSFORM, ret, action);
  wl_resource_destroy(randr_resource);
 
 I do not want a reply for every single part of output state, I want a
 reply for the final commit request that applies all the state updates
 gathered so far. That will also solve the atomicity problem.
 
 See how wl_surface works, and add a reply object only to the commit request.
 [Wang, Quanxian] Yes, when surface get a commit, it will applies all
 the state updates gathered so far. And then inform user that (frame callback
called after weston_output_repaint), If client is updating the surface and 
without
commit. Suddenly when he got an reply event that surface has update his
previous changes. But he didn't run surface_commit at that time. Here I don't
understand if there are some conflict. (The reply callback is created by
surface_frame to keep tuning of his update.) That means compositor does the
update when client don't run surface commit. Sorry, I am really confused here.
Actually it is the same as above question to avoid the conflict.


The wl_callback from a wl_surface.frame request will never deliver an event, if
the client has not also sent wl_surface.commit following that particular 
request.

If there were earlier wl_surface.frame requests that were followed by
wl_surface.commits, then those events can be delivered, of course. The point 
is,
the client

RE: weston: weston randr protocol for testing and configuration

[Wang, Quanxian]

Hi, Pq

The information to identify the unique mode: width, height and refresh are 
enough? Not enough in theory. But is enough in real world. I have checked with 
xrandr. Read the following comment.

Welcome any comment for that.

Thanks 

 +
 +request name=set_mode
 +  description summary=set the mode of output
 +Set the mode of output.
 +  /description
 +  arg name=output type=object interface=wl_output
 +   summary=the output object/
 +  arg name=width type=int summary=width of the mode in
hardware units/
 +  arg name=height type=int summary=height of the mode in
hardware units/
 +  arg name=refresh type=int summary=vertical refresh rate
 +in mHz/

So this is the simple mode set request.

Do you think width/height/refresh is really enough to identify a mode? I don't
think so. I think in the early days of X11 RandR, NVidia hit the same problem, 
and
started to expose fake refresh values, only to be able to distinguish modes 
that
were identical in width/height/refresh but still different in timings. Or 
actually I
think it was much more complicated than that, but this is the point in simple 
terms.

So we need something else here to identify a mode.

Check what kind of protocol GNOME uses, and how current RandR protocol works.
[Wang, Quanxian] Hi, Pq
Your understanding are right in theory. But in reality, it is barely possible. 
Width and height could be easily same, not easy for refresh at the same time. 
Currently in xrandr, they use mode name to identify one mode (for example 
widthxheight_refresh). in xrandr process, I don't find they take mode 
information to compare. Maybe I missed, but from xrandr parameters, there is no 
such option. They just take width, height, refresh as mode name to identify one 
mode. Sometimes, only width and height. If we want to fully support one unique 
mode, all the mode information have to be compared. (clock, hdisplay, 
hsync_start, hsync_end, htotal, vdisplay, vsync_start, vsync_end, vtotal, 
flags). But it is not convenient. Sometime, user basically don't know what 
hdisplay is at all. They just know widthxheight_refresh. My idea is just take 
width, height, refresh as the unique id for mode. 





Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: weston: weston randr protocol for testing and configuration

[Wang, Quanxian]

Hi, Pq

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

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

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

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

Thanks for your support.

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Saturday, March 22, 2014 8:20 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: weston: weston randr protocol for testing and configuration

Hi

On Fri, 14 Mar 2014 11:18:48 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 Objective:
 With discussion in mail list, currently we have an agreement. Randr 
 interfaces
will not be exposed public.
 The objective will be only for testing and configuration. Thanks Pq, Jason, 
 Jasper,
Hardening, and other's comment.

 Before sending the email, the function list below is implementedi(tested)
except new mode creating(not testing).

 Thanks for your support.

 Updates:
 1) Change wl_randr to weston_randr for weston-specific.
 2) Unique operation issue
 Given that one client has two more threads to do mode setting on the same
output at the same time, how to identify what response (done event) is belong 
to
one or another request when they want to get response?


Talking about threads here is misleading. Such threaded operations do not make
sense to begin with. The question is more about ambiguity between operations
and acknowledgements, and preferring explicit correspondence than relying on
protocol message ordering.
So the goal is good, but the rationale is slightly wrong.

 This is a design flaw in the first version of randr protocol.

 The solution is to use the wayland/weston mechanism, every request will
generate a resource which be used to send done event to the owner who start it.
Owner could set the listener on that and keep tuning on the response event.

 For example
 In client:
 randr_resource =
 wl_randr_set_transform(randr.randr,wayland_output,argument.transform);
 wl_randr_add_listener(randr_resource, randr_transform_listener,
 randr);

 In compositor:
 randr_resource = wl_resource_create(client,wl_randr_interface,1,
 action); ...
 wl_randr_send_action_done(randr_resource,
 1WL_RANDR_ACTION_TRANSFORM, ret, action);
 wl_resource_destroy(randr_resource);

I do not want a reply for every single part of output state, I want a reply 
for the
final commit request that applies all the state updates gathered so far. That 
will
also solve the atomicity problem.

See how wl_surface works, and add a reply object only to the commit request.
[Wang, Quanxian] I am working on that. randr_commit will be added for this.


 3) Mess up issue
 Will take randr protocol implementation as a module. This will keep 
 compositor
clear and avoid messing up in compositor.c.

 You can enable it from command line
 'weston --tty=1 --enable-wrandr' or put wrandr.so in to module list in 
 weston.ini.

This kind of messing up was never a problem. Making it a module is a 
solution to
the security concerns. Again, the right goal, but not the right reason.

 4) Add group randr operations
 After talking with Pq, in order to avoid glitches, group operation is needed.
However, if operating on two outputs more at one time, it will bring more 
issues
which could not control. In this randr design, will not provide group 
operation on
multiple outputs. We provide atomic operation on one output, multi outputs
operation logic are left to designer/developers. Group operation on one output
will be designed. For example, you can set mode, scale, and transform at one 
time
with one randr request.


This seems to contradict your example above about the reply objects.
[Wang, Quanxian] yes.

 5) Mode setting parameters control
 Output name will be under the control, for mode setting, from suggestion of
Hardening and Jasper St. Pierre, it'd better to let user new mode. Defaultly I 
agree
that. If someone has other suggestion and reason, please let me know.


Sorry, what?

If this means you will support custom modes, that's good. It's not in your 
proposal
below, though.
[Wang, Quanxian] I will add newmode and delmode request for that.

 6) Disconnected outputs
 Currently I have disable that. But basically, when user query output 
 information,
showing connected and disconnected output as a whole will be fine for user and
QA people. Sometime, QA people or user like that information. It will be 
helpful
for them to identify

RE: [PATCH 0/3] Add wl_output name event

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Thursday, March 20, 2014 1:02 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; Bryce Harrington
Subject: Re: [PATCH 0/3] Add wl_output name event

Quaxian,
I looked over the latest versions, and I think they all look good now from a 
technical perspective.  I'm still not 100% sure that this is needed, but I 
think I'm ok with it.
--Jason Ekstrand
[Wang, Quanxian] That is fine. Thanks

On Wed, Mar 19, 2014 at 12:30 AM, Quanxian Wang 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
This event contains a human-readable name of output.
It may be sent after binding the output object.
It is intended that the client can use this output name
as a parameter or display it in logs.

For example, in weston randr application, output name can
be a parameter in command line to stand for an output.

Quanxian Wang (3):
  wayland: Add wl_output name event
  shell: Add wl_output name event
  weston:Add wl_output name event

Wayland:
 protocol/wayland.xml | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Shell:
 clients/desktop-shell.c | 12 ++--
 clients/window.c| 12 ++--
 2 files changed, 20 insertions(+), 4 deletions(-)

Weston:
 src/compositor.c|  7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

--
1.8.1.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/3] wayland: Add wl_output name event

[Wang, Quanxian]

-Original Message-
From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
Sent: Wednesday, March 19, 2014 3:09 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; ja...@jlekstrand.net
Subject: Re: [PATCH 1/3] wayland: Add wl_output name event

On Mon, Mar 17, 2014 at 11:34:50AM +0800, Quanxian Wang wrote:
 This event contains name of output. It may be sent after binding the
 output object. It is intended that client could input a character
 output name as a parameter or for log output.

 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  protocol/wayland.xml | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/protocol/wayland.xml b/protocol/wayland.xml index
 d47ee62..0332e1a 100644
 --- a/protocol/wayland.xml
 +++ b/protocol/wayland.xml
 @@ -1756,6 +1756,17 @@
/description
arg name=factor type=int summary=scaling factor of output/
  /event
 +
 +event name=name since=3
 +  description summary=output name properties
 +This event contains name of output. It may be sent after
 +binding the output object. It will provide useful name
 +for client when they generate the log for user.
 +Also it is intended that client could input a character
 +output name as a parameter or log.

Fixing up the grammar a bit:


This event contains the name of the output.  It may be sent after binding the
output object.  It will provide a useful name for the client when it generates 
the
log for the user.  Also, it is intended that the client could input a 
character output
name as a parameter or log.


If I understand the purpose of this patchset properly, this might be clearer
phrasing:


This event allows assigning a human-readable alias to an output; it may be sent
after binding the output object.  It is intended that the client can use this 
output
name as a parameter or display it in logs.


[Wang, Quanxian] Thanks Bryce.
I glanced at the other patches and they look technically correct, but I think 
what
might be missing here is a stronger justification about why this is needed?
[Wang, Quanxian] name is one property of output which is assigned detected by 
backend. In compositor, it is visible. Why is it assigned as a name in 
compositor? I think display it in compositor logs will be one of reason. 
Therefore the same thing in client.  At the same time, it provides a good 
communication with compositor as a choice. Client could take it as a parameter 
to communicate with compositor, instead of only by wl_output object. I like 
your word 'human-readable' especially for client. Give client a choice to 
communicate with compositor without wl_output object, and also give client a 
choice to display human-readable name in log which is visible to customer. 
Currently I am designing weston randr protocol, customer(admin or developer) 
could input a char name to stand for an output from command line. It is just 
one usage. Anyway, name of output should be visible for both of compositor and 
client. 

 +  /description
 +  arg name=name type=string summary=name of output/
 +/event
/interface

interface name=wl_region version=1
 --
 1.8.1.2

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Wednesday, March 19, 2014 8:55 AM
To: Wang, Quanxian
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event


On Mar 16, 2014 5:30 AM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:





 From: Jason Ekstrand 
 [mailto:ja...@jlekstrand.netmailto:ja...@jlekstrand.net]
 Sent: Saturday, March 15, 2014 9:10 PM

 To: Wang, Quanxian
 Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
 Subject: RE: [PATCH 2/3] weston:Add wl_output name event




 On Mar 15, 2014 4:57 AM, Wang, Quanxian 
 quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
 
 
 
 
 
  From: Jason Ekstrand 
  [mailto:ja...@jlekstrand.netmailto:ja...@jlekstrand.net]
  Sent: Saturday, March 15, 2014 3:54 AM
  To: Wang, Quanxian
  Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
  wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
 
  Subject: Re: [PATCH 2/3] weston:Add wl_output name event
 
 
 
 
  On Mar 13, 2014 9:12 PM, Quanxian Wang 
  quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
  
   Signed-off-by: Quanxian Wang 
   quanxian.w...@intel.commailto:quanxian.w...@intel.com
   ---
src/compositor.c | 3 +++
1 file changed, 3 insertions(+)
  
   diff --git a/src/compositor.c b/src/compositor.c
   index 98a4f6f..8e8964b 100644
   --- a/src/compositor.c
   +++ b/src/compositor.c
   @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
   }
  
   if (version = 2)
   +   wl_output_send_name(resource, output-name);
 
  As with my comment on the protocol, wl_output_send_name should be version 
  3, not version 2.  That should also clear up Bryce's comment.
 
  [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently 
  shell only support version 2. Right?

 We should still have patch 3 since weston should always support the latest 
 version of everything.

 [Wang, Quanxian]That is fine according to my testing. If no patch 3/3, 
 desktop-shell will crash. I am not sure if more updates are needed. For 
 example, in our shell code, we use such code  min(2, version) for wl_output 
 interface, should we change it to minal(3,version) if merge this patch?

Of anything is crashing, chances are that something is wrong. Perhaps this is 
tied to adding events without bumping the version number?

[Wang, Quanxian] maybe, I will have a try and check if it is the bump issue 
which cause client crash. Thanks for your reminder. If it is, patch 3 is not 
needed. If we want to support latest version, we should bump wl_output to 
version 3 and add name listener.

--Jason Ekstrand


 
  Thanks,
  --Jason Ekstrand
 
   +
   +   if (version = 2)
   wl_output_send_done(resource);
}
  
   --
   1.8.1.2
  
   ___
   wayland-devel mailing list
   wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/3] wayland: Add wl_output name event

[Wang, Quanxian]

-Original Message-
From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
Sent: Wednesday, March 19, 2014 10:01 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; ja...@jlekstrand.net
Subject: Re: [PATCH 1/3] wayland: Add wl_output name event

On Wed, Mar 19, 2014 at 01:54:13AM +, Wang, Quanxian wrote:
 [Wang, Quanxian] Thanks Bryce.
 I glanced at the other patches and they look technically correct, but
 I think what might be missing here is a stronger justification about why 
 this is
needed?

 [Wang, Quanxian] name is one property of output which is assigned
 detected by backend. In compositor, it is visible. Why is it assigned
 as a name in compositor? I think display it in compositor logs will be
 one of reason. Therefore the same thing in client.  At the same time,
 it provides a good communication with compositor as a choice. Client
 could take it as a parameter to communicate with compositor, instead
 of only by wl_output object. I like your word 'human-readable'
 especially for client. Give client a choice to communicate with
 compositor without wl_output object, and also give client a choice to
 display human-readable name in log which is visible to customer.
 Currently I am designing weston randr protocol, customer(admin or
 developer) could input a char name to stand for an output from command
 line. It is just one usage. Anyway, name of output

That's a good example to include for justification; maybe include mention of 
the
randr use case in your commit message.
[Wang, Quanxian] Got it. Thanks

 should be visible for both of compositor and client.


  +  /description
  +  arg name=name type=string summary=name of output/
  +/event
 /interface
 
 interface name=wl_region version=1
  --
  1.8.1.2
 
  ___
  wayland-devel mailing list
  wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Wang, Quanxian
Sent: Wednesday, March 19, 2014 10:03 AM
To: 'Jason Ekstrand'
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event



From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Wednesday, March 19, 2014 8:55 AM
To: Wang, Quanxian
Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event


On Mar 16, 2014 5:30 AM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:





 From: Jason Ekstrand 
 [mailto:ja...@jlekstrand.netmailto:ja...@jlekstrand.net]
 Sent: Saturday, March 15, 2014 9:10 PM

 To: Wang, Quanxian
 Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
 Subject: RE: [PATCH 2/3] weston:Add wl_output name event




 On Mar 15, 2014 4:57 AM, Wang, Quanxian 
 quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
 
 
 
 
 
  From: Jason Ekstrand 
  [mailto:ja...@jlekstrand.netmailto:ja...@jlekstrand.net]
  Sent: Saturday, March 15, 2014 3:54 AM
  To: Wang, Quanxian
  Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
  wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
 
  Subject: Re: [PATCH 2/3] weston:Add wl_output name event
 
 
 
 
  On Mar 13, 2014 9:12 PM, Quanxian Wang 
  quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
  
   Signed-off-by: Quanxian Wang 
   quanxian.w...@intel.commailto:quanxian.w...@intel.com
   ---
src/compositor.c | 3 +++
1 file changed, 3 insertions(+)
  
   diff --git a/src/compositor.c b/src/compositor.c
   index 98a4f6f..8e8964b 100644
   --- a/src/compositor.c
   +++ b/src/compositor.c
   @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
   }
  
   if (version = 2)
   +   wl_output_send_name(resource, output-name);
 
  As with my comment on the protocol, wl_output_send_name should be version 
  3, not version 2.  That should also clear up Bryce's comment.
 
  [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently 
  shell only support version 2. Right?

 We should still have patch 3 since weston should always support the latest 
 version of everything.

 [Wang, Quanxian]That is fine according to my testing. If no patch 3/3, 
 desktop-shell will crash. I am not sure if more updates are needed. For 
 example, in our shell code, we use such code  min(2, version) for wl_output 
 interface, should we change it to minal(3,version) if merge this patch?

Of anything is crashing, chances are that something is wrong. Perhaps this is 
tied to adding events without bumping the version number?

[Wang, Quanxian] maybe, I will have a try and check if it is the bump issue 
which cause client crash. Thanks for your reminder. If it is, patch 3 is not 
needed. If we want to support latest version, we should bump wl_output to 
version 3 and add name listener.

[Wang, Quanxian] I have tested it. It works as your said. The crash happens 
without bumping the version number. I will send the patch series later with 
updates in shell. Thanks

--Jason Ekstrand


 
  Thanks,
  --Jason Ekstrand
 
   +
   +   if (version = 2)
   wl_output_send_done(resource);
}
  
   --
   1.8.1.2
  
   ___
   wayland-devel mailing list
   wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Saturday, March 15, 2014 9:10 PM
To: Wang, Quanxian
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH 2/3] weston:Add wl_output name event


On Mar 15, 2014 4:57 AM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:





 From: Jason Ekstrand 
 [mailto:ja...@jlekstrand.netmailto:ja...@jlekstrand.net]
 Sent: Saturday, March 15, 2014 3:54 AM
 To: Wang, Quanxian
 Cc: ppaala...@gmail.commailto:ppaala...@gmail.com; 
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org

 Subject: Re: [PATCH 2/3] weston:Add wl_output name event




 On Mar 13, 2014 9:12 PM, Quanxian Wang 
 quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
 
  Signed-off-by: Quanxian Wang 
  quanxian.w...@intel.commailto:quanxian.w...@intel.com
  ---
   src/compositor.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/src/compositor.c b/src/compositor.c
  index 98a4f6f..8e8964b 100644
  --- a/src/compositor.c
  +++ b/src/compositor.c
  @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
  }
 
  if (version = 2)
  +   wl_output_send_name(resource, output-name);

 As with my comment on the protocol, wl_output_send_name should be version 3, 
 not version 2.  That should also clear up Bryce's comment.

 [Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently shell 
 only support version 2. Right?

We should still have patch 3 since weston should always support the latest 
version of everything.

[Wang, Quanxian]That is fine according to my testing. If no patch 3/3, 
desktop-shell will crash. I am not sure if more updates are needed. For 
example, in our shell code, we use such code  min(2, version) for wl_output 
interface, should we change it to minal(3,version) if merge this patch?


 Thanks,
 --Jason Ekstrand

  +
  +   if (version = 2)
  wl_output_send_done(resource);
   }
 
  --
  1.8.1.2
 
  ___
  wayland-devel mailing list
  wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Friday, March 14, 2014 6:29 PM
To: Wang, Quanxian
Cc: Jasper St. Pierre; Hardening; wayland-devel@lists.freedesktop.org; Zhang,
Xiong Y; Matthias Clasen; Jason Ekstrand
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Fri, 14 Mar 2014 09:34:28 +
Wang, Quanxian quanxian.w...@intel.com wrote:



 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Friday, March 14, 2014 4:13 PM
 To: Wang, Quanxian
 Cc: Jasper St. Pierre; Hardening;
 wayland-devel@lists.freedesktop.org; Zhang, Xiong Y; Matthias Clasen;
 Jason Ekstrand
 Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 On Fri, 14 Mar 2014 02:33:52 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  On Sun, Mar 9, 2014 at 10:34 PM, Wang, Quanxian
 quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
 
   5) mode setting parameters control Mode and output will be under
   the control. User could not randomly to set their
  mode. They have to select the available modes and outputs provided
  by compositor. Don't allow random mode setting. The mode and
  output information could be provided by weston-randr apps or wl_output
interface.
  
  I don't think that allowing to set only announced modes is a good idea.
  The RDP compositor is a good example where you can't know the
  supported modes (as nearly all modes can be supported).
  
  IIRC depending on the drivers, drm can also set arbitrary modes.
  [Wang, Quanxian] so, let user set the mode without limitation? Not
  sure if we
 should support that.
  Any comment for this requirement?
 
 Not a user, but an administrator or developer, remember?
 [Wang, Quanxian] got that.
 
 If we ignore all the no real hardware cases like RDP, VNC, and
 other stuff where the screen is actually a window on another system
 and so can be of any size, the following come to mind:
 
 - using the monitor hardware for scaling instead of the GPU
 
 - getting the native mode a display that has broken or missing EDID,
 as
   a workaround
 
 We have the support for arbitrary modelines already in weston's
 config file. Note that arbitrary size WxH is not enough, sometimes
 you need the whole timings to be custom. See the ModeLine directive
 for xorg.conf, and the arguments of xrandr --newmode.
 [Wang, Quanxian] Fine, drmModeSetCrtc will need more parameter which
 is from mode_info. like weston.ini or newmode in xrandr We create drm
modeinfo by these arguments. It need a new interface to do that. I will plan 
it in
new interface.
 
 While designing the interface, it would be nice to be able to easily
 reset to a known working mode, if the new mode doesn't work
 (modesetting succeeds, but the monitor does not like the signal).
 That means that if you first query/get the current mode, then switch
 to a new one, you can still switch back to the original exact mode
 even if it was a custom mode. There is probably a good reason why xrandr has
--newmode/addmode/mode instead of only --mode.
 [Wang, Quanxian] if you set the custom mode, basically we could not get the
result if it is right or not, it is based on the response from developer's 
view. But we
provide the interface to let them reset the mode.

Yes, I really mean the person looking at the monitor decides that it's not 
good,
need to set some known working mode before he can continue.
You've probably seen these click ok within 30 seconds if the picture looks ok 
now,
otherwise automatically reverts to the old mode dialogs.

 By the way, do we need provide a delete mode interface to delete the mode
which is not right determined by developer?

xrandr tool has it.

It's all up to you, really, if you see it as a feature that an administrator 
or developer
would use.

But do keep in mind, that protocol interface is not a user interface.
If someone makes GUI for weston-randr, and uses it to test a bunch of custom
modes where the last one actually works, it could be ugly if the GUI tool 
leaves a
bunch of non-working modelines in the server mode list. Then again, that list
probably gets wiped on server restart, and the admin is supposed to write the
good modeline into the server config file, so it becomes available by default.

And then there is the question, how far is it reasonable to go with this
weston-only tweaking protocol.

Like if you don't want to support custom modelines now, they can be added later
if needed, as long as the basic design here allows it. Do consider also your 
own
needs on IVI.

You should probably also look at the RandR X11 protocol, not what options 
xrandr
tool has. After all, you are designing a protocol here, not a user interface. 
xrandr is
just a user interface.
[Wang, Quanxian] Yes, I take randr X11 protocol as a basic reference and xrandr 
as a requirement to think about that. If we could provide randr function 
without protocol used, it will be the ideal case. However you know, we could 
not provide all without protocol

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Saturday, March 15, 2014 3:54 AM
To: Wang, Quanxian
Cc: ppaala...@gmail.com; wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/3] weston:Add wl_output name event


On Mar 13, 2014 9:12 PM, Quanxian Wang 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:

 Signed-off-by: Quanxian Wang 
 quanxian.w...@intel.commailto:quanxian.w...@intel.com
 ---
  src/compositor.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/src/compositor.c b/src/compositor.c
 index 98a4f6f..8e8964b 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
 }

 if (version = 2)
 +   wl_output_send_name(resource, output-name);

As with my comment on the protocol, wl_output_send_name should be version 3, 
not version 2.  That should also clear up Bryce's comment.

[Wang, Quanxian] if it is 2, patch 3/3 is not needed because  currently shell 
only support version 2. Right?

Thanks,
--Jason Ekstrand

 +
 +   if (version = 2)
 wl_output_send_done(resource);
  }

 --
 1.8.1.2

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] weston:Send done event with version 2 of wl_output

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Friday, March 14, 2014 10:58 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; b.harring...@samsung.com
Subject: Re: [PATCH] weston:Send done event with version 2 of wl_output

On Fri, 14 Mar 2014 12:16:05 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 With protocol of wl_output version 2, after the output change, it
 should send done event to all clients bound with it.

 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  src/compositor.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

 diff --git a/src/compositor.c b/src/compositor.c index
 7c29d51..98a4f6f 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -3245,7 +3245,7 @@ weston_output_move(struct weston_output *output,
int x, int y)
  wl_signal_emit(output-compositor-output_moved_signal, output);

  /* Notify clients of the change for output position. */
 -wl_resource_for_each(resource, output-resource_list)
 +wl_resource_for_each(resource, output-resource_list) {
  wl_output_send_geometry(resource,
  output-x,
  output-y,
 @@ -3255,6 +3255,10 @@ weston_output_move(struct weston_output
*output, int x, int y)
  output-make,
  output-model,
  output-transform);
 +
 +if (wl_resource_get_version(resource) = 2)
 +wl_output_send_done(resource);
 +}
  }

  WL_EXPORT void

Hi,

are you sure all the output state updates have been sent at this point?

I did a quick grep, and this seems to be fine. wl_output_send_*() functions are
only called from weston_output_switch_mode(), weston_output_move(), and
bind_output() which must be ok as is.

weston_output_move() is only called from
weston_compositor_remove_output(), so it seems very unlikely that
switch_mode and output_move could happen as a part of the same output
reconfiguration.
[Wang, Quanxian] currently weston_output_move() is only called by 
weston_compositor_remove_output(). But you could not make sure this function 
will not be called by others in the future.
For example in weston randr protocol, I will reuse this code. I suggest you 
make it complete instead of only send geometry. Whatever wl_output geometry, 
scale, transform change, they should be used with done event. It is determined 
by wl_output design logics. Any use familiar with wl_output interface will also 
follow this rule. They will wait for done event after version 2 of wl_output 
just as you said before. Right?

So, reviewed-by me.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Friday, March 14, 2014 4:13 PM
To: Wang, Quanxian
Cc: Jasper St. Pierre; Hardening; wayland-devel@lists.freedesktop.org; Zhang,
Xiong Y; Matthias Clasen; Jason Ekstrand
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Fri, 14 Mar 2014 02:33:52 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 On Sun, Mar 9, 2014 at 10:34 PM, Wang, Quanxian
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:

  5) mode setting parameters control
  Mode and output will be under the control. User could not randomly
  to set their
 mode. They have to select the available modes and outputs provided by
 compositor. Don't allow random mode setting. The mode and output
 information could be provided by weston-randr apps or wl_output interface.
 
 I don't think that allowing to set only announced modes is a good idea.
 The RDP compositor is a good example where you can't know the
 supported modes (as nearly all modes can be supported).
 
 IIRC depending on the drivers, drm can also set arbitrary modes.
 [Wang, Quanxian] so, let user set the mode without limitation? Not sure if we
should support that.
 Any comment for this requirement?

Not a user, but an administrator or developer, remember?
[Wang, Quanxian] got that.

If we ignore all the no real hardware cases like RDP, VNC, and other stuff 
where
the screen is actually a window on another system and so can be of any size, 
the
following come to mind:

- using the monitor hardware for scaling instead of the GPU

- getting the native mode a display that has broken or missing EDID, as
  a workaround

We have the support for arbitrary modelines already in weston's config file. 
Note
that arbitrary size WxH is not enough, sometimes you need the whole timings to
be custom. See the ModeLine directive for xorg.conf, and the arguments of 
xrandr
--newmode.
[Wang, Quanxian] Fine, drmModeSetCrtc will need more parameter which is from 
mode_info. like weston.ini or newmode in xrandr 
We create drm modeinfo by these arguments. It need a new interface to do that. 
I will plan it in new interface.

While designing the interface, it would be nice to be able to easily reset to a
known working mode, if the new mode doesn't work (modesetting succeeds, but
the monitor does not like the signal). That means that if you first query/get 
the
current mode, then switch to a new one, you can still switch back to the 
original
exact mode even if it was a custom mode. There is probably a good reason why
xrandr has --newmode/addmode/mode instead of only --mode.
[Wang, Quanxian] if you set the custom mode, basically we could not get the 
result if it is right or not, it is based on the response from developer's 
view. But we provide the interface to let them reset the mode. 
By the way, do we need provide a delete mode interface to delete the mode which 
is not right determined by developer?


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: magc...@gmail.com [mailto:magc...@gmail.com] On Behalf Of Jasper St. 
Pierre
Sent: Monday, March 10, 2014 10:42 AM
To: Wang, Quanxian
Cc: Hardening; wayland-devel@lists.freedesktop.org; Zhang, Xiong Y; Pekka 
Paalanen; Matthias Clasen; Jason Ekstrand
Subject: Re: [PATCH 1/6] Add weston randr protocol

There's two different things here. There's the protocol and the UI. (By UI I 
also mean a command-line tool like /usr/bin/xrandr, I just mean the way the 
user does a mode-set).
I think to build a good UI, we need a list of available modes that the user can 
choose from. Having a tool where the user enters two numbers, and then tells 
the user No, guess again if they entered the wrong numbers is bad user 
interface design.

But, as Hardening said, the protocol should allow setting modes that aren't in 
the advertised list.
[Wang, Quanxian] Sorry for response later. From command line tools, 
Weston-wrandr will provide the mode list instead of guessing what is in. you 
can firstly use 'weston-wrandr --output' to query all modes  of output. And 
then select one of them.
Of course, if you want to new a mode, that is fine. It is also reasonable for 
me.

On Sun, Mar 9, 2014 at 10:34 PM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:

 5) mode setting parameters control
 Mode and output will be under the control. User could not randomly to set 
 their
mode. They have to select the available modes and outputs provided by
compositor. Don't allow random mode setting. The mode and output information
could be provided by weston-randr apps or wl_output interface.

I don't think that allowing to set only announced modes is a good idea.
The RDP compositor is a good example where you can't know the supported
modes (as nearly all modes can be supported).

IIRC depending on the drivers, drm can also set arbitrary modes.
[Wang, Quanxian] so, let user set the mode without limitation? Not sure if we 
should support that.
Any comment for this requirement?


Regards

--
David FORT
website: http://www.hardening-consulting.com/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel



--
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] weston: Send done event with version 2 of wl_output

[Wang, Quanxian]

-Original Message-
From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
Sent: Friday, March 14, 2014 10:19 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] weston: Send done event with version 2 of wl_output

On Fri, Mar 14, 2014 at 09:16:25AM +0800, Quanxian Wang wrote:
 With protocol of wl_output version 2, after the output change, it
 should send done event to all clients bount with it.

bound?
[Wang, Quanxian] yes. Sorry.


 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  src/compositor.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

 diff --git a/src/compositor.c b/src/compositor.c index
 7c29d51..98a4f6f 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -3245,7 +3245,7 @@ weston_output_move(struct weston_output *output,
int x, int y)
  wl_signal_emit(output-compositor-output_moved_signal, output);

  /* Notify clients of the change for output position. */
 -wl_resource_for_each(resource, output-resource_list)
 +wl_resource_for_each(resource, output-resource_list) {
  wl_output_send_geometry(resource,
  output-x,
  output-y,
 @@ -3255,6 +3255,10 @@ weston_output_move(struct weston_output
*output, int x, int y)
  output-make,
  output-model,
  output-transform);
 +
 +if (wl_resource_get_version(resource) = 2)
 +wl_output_send_done(resource);
 +}
  }

  WL_EXPORT void
 --
 1.8.1.2

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 2/3] weston:Add wl_output name event

[Wang, Quanxian]

-Original Message-
From: Bryce W. Harrington [mailto:b.harring...@samsung.com]
Sent: Friday, March 14, 2014 10:44 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; ppaala...@gmail.com
Subject: Re: [PATCH 2/3] weston:Add wl_output name event

On Fri, Mar 14, 2014 at 10:27:05AM +0800, Quanxian Wang wrote:
 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  src/compositor.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/src/compositor.c b/src/compositor.c index
 98a4f6f..8e8964b 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -3045,6 +3045,9 @@ bind_output(struct wl_client *client,
  }

  if (version = 2)
 +wl_output_send_name(resource, output-name);
 +
 +if (version = 2)
  wl_output_send_done(resource);

Why not just put the two calls in a block?

   if (version = 2) {
   wl_output_send_name(resource, output-name);
   wl_output_send_done(resource);
   }
[Wang, Quanxian] I found scale and done are separately sent instead of in one 
block. I don't know why. Therefore I follow their original code style to do 
that.

  }

 --
 1.8.1.2

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

-Original Message-
From: wayland-devel-boun...@lists.freedesktop.org
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
Paalanen
Sent: Tuesday, March 11, 2014 4:26 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

On Tue, 11 Mar 2014 01:48:49 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Thanks.

 Quanxian

  binds with version 2 regardless of what the compositor advertised.
  
  You can compare to the desktop_shell global handling which seems
  correct.
  [Wang, Quanxian] I will check and update that. The change will be
  like that 1293 output-output =
  1294 display_bind(desktop-display, id,
  wl_output_interface, version); 1295 output-server_output_id
  = id; 1296 output-interface_version = (version  2) ?
  version : 2;
 
 Do not use the server advertized version in bind without checking it
 first. If the server version grows, the client still needs to bind
 with the old version, until the client code is ported to support the new 
 version.
 [Wang, Quanxian] Sorry for misunderstanding the interface version issue.
Currently my understanding is that client apps(desktop-shell) is based on
wl_output which version is under 2(including 2), so whatever wl_output upgrade
or not, we can use only the function provided by wl_output interface under the
version 2. Right?
 Another question is, since version is from server, why not keep it as 
 original
value? when you do some operation, you just check and choose what needed
based on the value(just like update_output). My suggestion is when you upgrade
client app to use wl_output, we don't need to continue maintain such code.(For
example from 2 to 3). The only update is the new code which want to use new
function provided by newly wl_output interface. What is your idea about that?
 

Let me put it more clearly. The client has been coded to support version 2. The
server advertises version N. The agreed interface version that both can deal 
with
is then min(2, N).

You have to tell the server, what the agreed version is: you have to call
display_bind() with version min(2, N). This way the server knows, that it must 
not
send events added in versions 3 or greater, because those would crash the 
client.

Also the client knows, that it cannot use requests added in versions
N+1 or greater.

It does not matter whether you set output-interface_version = N or min(2, N),
because the code in the client is anyway comparing that to what it actually is 
coded
for. It's just more logical to me to store the agreed version, not the server 
version.
[Wang, Quanxian] Thanks. Got that.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

-Original Message-
From: wayland-devel-boun...@lists.freedesktop.org
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
Paalanen
Sent: Monday, March 10, 2014 3:47 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

Hi,

overall this looks fine now, but I still haven't tested it. I'm moving on to 
nitpick on
the minor details. ;-)
[Wang, Quanxian] that is fine.

The patch title (topic line) is usually prefixed with the component the patch 
is
affecting. Looking at the clients/desktop-shell.c history, the appropriate 
prefix
would be desktop-shell:. Also, this patch does not affect any other clients.
[Wang, Quanxian] I will resend it again at last with title adding 
desktop-shell:. :)

On Thu,  6 Mar 2014 18:31:14 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 1)
 Width and height of Panel and Background depend on output's, therefore
 they should be bound with output changes including mode, transform and
 scale.

 2)
 Update the min_allocation before resize the panel and background
 window. Add window_set_min_allocation function because it is invisible
 outside window.c.

An alternative to adding the set_min_allocation would be to cause the
min_allocation to be something very small to begin with. The min_allocation is 
not
important for the panel and background surfaces, because they follow special
rules, and cannot be resized freely anyway.

I'm fine with either way.


 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  clients/desktop-shell.c | 80
 +++--
 clients/window.c|  7 + clients/window.h|  2 ++
  3 files changed, 87 insertions(+), 2 deletions(-)

 diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
 a0c6b6d..4373448 100644
 --- a/clients/desktop-shell.c
 +++ b/clients/desktop-shell.c
 @@ -103,6 +103,15 @@ struct output {

  struct panel *panel;
  struct background *background;
 +struct {
 +int height;
 +int width;
 +uint32_t refresh;
 +} mode;
 +
 +uint32_t interface_version;
 +uint32_t transform;
 +uint32_t scale;
  };

  struct panel_launcher {
 @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
 *desktop) }

  static void
 +update_output(struct output *output)
 +{
 +struct panel *panel = output-panel;
 +struct background *background = output-background;
 +int width, height;
 +
 +if (!output)

You already dereferenced 'output' above, checking for NULL here is useless.
'output' can never be NULL anyway, right?

 +return;
 +
 +width = output-mode.width;
 +height = output-mode.height;
 +
 +switch (output-transform) {
 +case WL_OUTPUT_TRANSFORM_90:
 +case WL_OUTPUT_TRANSFORM_270:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_90:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_270:
 +/* Swap width and height */
 +width = output-mode.height;
 +height = output-mode.width;
 +break;
 +default:
 +break;
 +}
 +
 +if (output-scale != 0) {

If scale was initialized to 1, there would be no need for 'if'. Right?

 +width /= output-scale;
 +height /= output-scale;
 +}
 +
 +/* Set min_allocation of panel */

Needless comment.

 +window_set_min_allocation(panel-window, width, 32);
 +window_set_min_allocation(background-window, width, height);
 +
 +window_schedule_resize(background-window, width, height);
 +window_schedule_resize(panel-window, width, 32); }
 +
 +static void
  output_handle_geometry(void *data,
 struct wl_output *wl_output,
 int x, int y,
 @@ -1157,6 +1205,11 @@ output_handle_geometry(void *data,  {
  struct output *output = data;

 +output-transform = transform;
 +
 +if (output-interface_version  2)
 +update_output(output);
 +
  window_set_buffer_transform(output-panel-window,
 transform); window_set_buffer_transform(output-background-window,
 transform); }
 @@ -1169,12 +1222,29 @@ output_handle_mode(void *data,
 int height,
 int refresh)
  {
 +struct output *output = data;
 +
 +if (flags  WL_OUTPUT_MODE_CURRENT) {
 +if (!output)
 +return;
 +
 +output-mode.width = width;
 +output-mode.height = height;
 +output-mode.refresh = refresh;
 +
 +if (output-interface_version  2)
 +update_output(output);
 +}
  }

  static void
  output_handle_done(void *data,
 struct wl_output *wl_output)  {
 +struct output *output = data;
 +
 +if (output-interface_version = 2)
 +update_output(output);

The only case when this can be called is when the interface version is at 
least 2, so
no need to check. The protocol specification guarantees

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

Thanks Pq. Comments below.

-Original Message-
From: wayland-devel-boun...@lists.freedesktop.org
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
Paalanen
Sent: Monday, March 10, 2014 3:47 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

Hi,

overall this looks fine now, but I still haven't tested it. I'm moving on to 
nitpick on
the minor details. ;-)

The patch title (topic line) is usually prefixed with the component the patch 
is
affecting. Looking at the clients/desktop-shell.c history, the appropriate 
prefix
would be desktop-shell:. Also, this patch does not affect any other clients.

On Thu,  6 Mar 2014 18:31:14 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 1)
 Width and height of Panel and Background depend on output's, therefore
 they should be bound with output changes including mode, transform and
 scale.

 2)
 Update the min_allocation before resize the panel and background
 window. Add window_set_min_allocation function because it is invisible
 outside window.c.

An alternative to adding the set_min_allocation would be to cause the
min_allocation to be something very small to begin with. The min_allocation is 
not
important for the panel and background surfaces, because they follow special
rules, and cannot be resized freely anyway.

I'm fine with either way.


 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  clients/desktop-shell.c | 80
 +++--
 clients/window.c|  7 + clients/window.h|  2 ++
  3 files changed, 87 insertions(+), 2 deletions(-)

 diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
 a0c6b6d..4373448 100644
 --- a/clients/desktop-shell.c
 +++ b/clients/desktop-shell.c
 @@ -103,6 +103,15 @@ struct output {

  struct panel *panel;
  struct background *background;
 +struct {
 +int height;
 +int width;
 +uint32_t refresh;
 +} mode;
 +
 +uint32_t interface_version;
 +uint32_t transform;
 +uint32_t scale;
  };

  struct panel_launcher {
 @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
 *desktop) }

  static void
 +update_output(struct output *output)
 +{
 +struct panel *panel = output-panel;
 +struct background *background = output-background;
 +int width, height;
 +
 +if (!output)

You already dereferenced 'output' above, checking for NULL here is useless.
'output' can never be NULL anyway, right?
[Wang, Quanxian] right.

 +return;
 +
 +width = output-mode.width;
 +height = output-mode.height;
 +
 +switch (output-transform) {
 +case WL_OUTPUT_TRANSFORM_90:
 +case WL_OUTPUT_TRANSFORM_270:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_90:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_270:
 +/* Swap width and height */
 +width = output-mode.height;
 +height = output-mode.width;
 +break;
 +default:
 +break;
 +}
 +
 +if (output-scale != 0) {

If scale was initialized to 1, there would be no need for 'if'. Right?
[Wang, Quanxian] in testing, this cause the error because scale is set to 0, it 
happens when weston is started at the very beginning. Here we need to do that.

 +width /= output-scale;
 +height /= output-scale;
 +}
 +
 +/* Set min_allocation of panel */

Needless comment.
[Wang, Quanxian] will fix.

 +window_set_min_allocation(panel-window, width, 32);
 +window_set_min_allocation(background-window, width, height);
 +
 +window_schedule_resize(background-window, width, height);
 +window_schedule_resize(panel-window, width, 32); }
 +
 +static void
  output_handle_geometry(void *data,
 struct wl_output *wl_output,
 int x, int y,
 @@ -1157,6 +1205,11 @@ output_handle_geometry(void *data,  {
  struct output *output = data;

 +output-transform = transform;
 +
 +if (output-interface_version  2)
 +update_output(output);
 +
  window_set_buffer_transform(output-panel-window,
 transform); window_set_buffer_transform(output-background-window,
 transform); }
 @@ -1169,12 +1222,29 @@ output_handle_mode(void *data,
 int height,
 int refresh)
  {
 +struct output *output = data;
 +
 +if (flags  WL_OUTPUT_MODE_CURRENT) {
 +if (!output)
 +return;
 +
 +output-mode.width = width;
 +output-mode.height = height;
 +output-mode.refresh = refresh;
 +
 +if (output-interface_version  2)
 +update_output(output);
 +}
  }

  static void
  output_handle_done(void *data,
 struct wl_output *wl_output)  {
 +struct output *output = data;
 +
 +if (output-interface_version = 2)
 +update_output(output);

The only case when

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Monday, March 10, 2014 5:58 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

On Mon, 10 Mar 2014 08:23:45 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Thanks Pq. Comments below.
...
  @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
  *desktop) }
 
   static void
  +update_output(struct output *output) {
  + struct panel *panel = output-panel;
  + struct background *background = output-background;
  + int width, height;
  +
  + if (!output)
 
 You already dereferenced 'output' above, checking for NULL here is
 useless. 'output' can never be NULL anyway, right?
 [Wang, Quanxian] right.
 
  + return;
  +
  + width = output-mode.width;
  + height = output-mode.height;
  +
  + switch (output-transform) {
  + case WL_OUTPUT_TRANSFORM_90:
  + case WL_OUTPUT_TRANSFORM_270:
  + case WL_OUTPUT_TRANSFORM_FLIPPED_90:
  + case WL_OUTPUT_TRANSFORM_FLIPPED_270:
  + /* Swap width and height */
  + width = output-mode.height;
  + height = output-mode.width;
  + break;
  + default:
  + break;
  + }
  +
  + if (output-scale != 0) {
 
 If scale was initialized to 1, there would be no need for 'if'.
 Right?
 [Wang, Quanxian] in testing, this cause the error because scale is set
 to 0, it happens when weston is started at the very beginning.
 Here we need to do that.

Why does that happen? Is Weston sending an event with scale=0? If so, then that
is a Weston bug and should be fixed. Until you get any event the scale is 1,
because that is the implicit scale in interface version 1.
[Wang, Quanxian] yes. Got geometry event with scale=0.

 
  + width /= output-scale;
  + height /= output-scale;
  + }
...
window_set_buffer_scale(output-panel-window, scale);
window_set_buffer_scale(output-background-window,
  scale);  } @@ -1212,7 +1287,7 @@ output_init(struct output *output,
  struct desktop *desktop) }
 
   static void
  -create_output(struct desktop *desktop, uint32_t id)
  +create_output(struct desktop *desktop, uint32_t id, uint32_t
  version) {
struct output *output;
 
  @@ -1223,6 +1298,7 @@ create_output(struct desktop *desktop,
  uint32_t id) output-output =
display_bind(desktop-display, id, wl_output_interface, 2);
  output-server_output_id = id;
  + output-interface_version = version;
 
 This is not completely correct. In the future, a compositor may
 advertise wl_output interface version 3 or greater, if the protocol
 has been augmented. However, the valid version is the minimum of what
 the compositor advertises and what the client is written to support.
 Therefore the interface version should be min(version, 2).
 
 Note that create_output() already has a small bug in this: it always
 binds with version 2 regardless of what the compositor advertised.
 
 You can compare to the desktop_shell global handling which seems
 correct.
 [Wang, Quanxian] I will check and update that. The change will be
 like that 1293 output-output =
 1294 display_bind(desktop-display, id,
 wl_output_interface, version); 1295 output-server_output_id
 = id; 1296 output-interface_version = (version  2) ?
 version : 2;

Do not use the server advertized version in bind without checking it first. If 
the
server version grows, the client still needs to bind with the old version, 
until the
client code is ported to support the new version.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Monday, March 10, 2014 7:05 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

On Mon, 10 Mar 2014 10:58:00 +
Wang, Quanxian quanxian.w...@intel.com wrote:



 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Monday, March 10, 2014 5:58 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: [PATCH] Bug fix client apps because of output change
 
 On Mon, 10 Mar 2014 08:23:45 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Thanks Pq. Comments below.
 ...
   @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
   *desktop) }
  
static void
   +update_output(struct output *output) {
   +  struct panel *panel = output-panel;
   +  struct background *background = output-background;
   +  int width, height;
   +
   +  if (!output)
  
  You already dereferenced 'output' above, checking for NULL here is
  useless. 'output' can never be NULL anyway, right?
  [Wang, Quanxian] right.
  
   +  return;
   +
   +  width = output-mode.width;
   +  height = output-mode.height;
   +
   +  switch (output-transform) {
   +  case WL_OUTPUT_TRANSFORM_90:
   +  case WL_OUTPUT_TRANSFORM_270:
   +  case WL_OUTPUT_TRANSFORM_FLIPPED_90:
   +  case WL_OUTPUT_TRANSFORM_FLIPPED_270:
   +  /* Swap width and height */
   +  width = output-mode.height;
   +  height = output-mode.width;
   +  break;
   +  default:
   +  break;
   +  }
   +
   +  if (output-scale != 0) {
  
  If scale was initialized to 1, there would be no need for 'if'.
  Right?
  [Wang, Quanxian] in testing, this cause the error because scale is
  set to 0, it happens when weston is started at the very beginning.
  Here we need to do that.
 
 Why does that happen? Is Weston sending an event with scale=0? If so,
 then that is a Weston bug and should be fixed. Until you get any
 event the scale is 1, because that is the implicit scale in interface 
 version 1.
 [Wang, Quanxian] yes. Got geometry event with scale=0.

What do you mean? wl_output.geometry does not carry scale at all, hence until
you receive the first wl_output.scale you should assume scale=1.
The easiest way to do that is to initialize output::scale to 1 in 
create_output(). You
should never have zero.
[Wang, Quanxian] sorry, is scale event instead of geometry. Yes, I can 
initialize scale to be 1, but at the same time, I need to get the scale event 
from server. Basically I still need to do some checking for that.
After that, I will check why scale(0) is sent by wl_output and push the patch 
to fix that. 


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

Thanks.

Quanxian

 binds with version 2 regardless of what the compositor advertised.
 
 You can compare to the desktop_shell global handling which seems
 correct.
 [Wang, Quanxian] I will check and update that. The change will be
 like that 1293 output-output =
 1294 display_bind(desktop-display, id,
 wl_output_interface, version); 1295 output-server_output_id
 = id; 1296 output-interface_version = (version  2) ?
 version : 2;

Do not use the server advertized version in bind without checking it first. If 
the
server version grows, the client still needs to bind with the old version, 
until the
client code is ported to support the new version.
[Wang, Quanxian] Sorry for misunderstanding the interface version issue. 
Currently my understanding is that client apps(desktop-shell) is based on 
wl_output which version is under 2(including 2), so whatever wl_output upgrade 
or not, we can use only the function provided by wl_output interface under the 
version 2. Right?
Another question is, since version is from server, why not keep it as original 
value? when you do some operation, you just check and choose what needed based 
on the value(just like update_output). My suggestion is when you upgrade client 
app to use wl_output, we don't need to continue maintain such code.(For example 
from 2 to 3). The only update is the new code which want to use new function 
provided by newly wl_output interface. What is your idea about that?


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

 5) mode setting parameters control
 Mode and output will be under the control. User could not randomly to set 
 their
mode. They have to select the available modes and outputs provided by
compositor. Don't allow random mode setting. The mode and output information
could be provided by weston-randr apps or wl_output interface.

I don't think that allowing to set only announced modes is a good idea.
The RDP compositor is a good example where you can't know the supported
modes (as nearly all modes can be supported).

IIRC depending on the drivers, drm can also set arbitrary modes.
[Wang, Quanxian] so, let user set the mode without limitation? Not sure if we 
should support that.
Any comment for this requirement?


Regards

--
David FORT
website: http://www.hardening-consulting.com/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

Thanks Pq's comment. 

By the way, if we make it as a configuration tools or testing protocol. Will it 
be fine or not? 

For example graphics QA testing, 
I just send a patch to bug fix the client apps because of output change. I use 
weston-wrandr to do the testing to make sure what I have done is right. If I 
have not this tool, I don't make sure how to test it. Because you need 
dynamically change rotate, transform, or mode set to make sure desktop shell 
could get the change and really works fine with new change.

Also this tool should be fine for wayland QA testing. 

Anyway, if you guys think it right for configuration or testing wayland 
graphics tools, that is fine. I can continue working on these patches instead 
of original idea to expose this to client to use.

Thanks for everyone's comment and help.

Regards

Quanxian Wang

-Original Message-
From: wayland-devel-boun...@lists.freedesktop.org
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
Paalanen
Sent: Thursday, March 06, 2014 4:25 PM
To: Wang, Quanxian
Cc: Hardening; Jasper St. Pierre; Matthias Clasen;
wayland-devel@lists.freedesktop.org; Jason Ekstrand; Zhang, Xiong Y
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Thu, 6 Mar 2014 03:01:11 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Except the comment below. I mention some points.

 1) My idea:
 My original idea is from xrandr of xserver. I just want to let xrandr
 could be implemented in wayland. If no protocol is used, that is fine.
 But no way to implement some function for example transform, scale,
 mode set output. I have to create a protocol to communicate with
 compositor and let compositor do that.

Usually such configuration changes should originate within the compositor 
itself. In
practice, it could be a special shell helper application (e.g. 
weston-desktop-shell)
using a private protocol to communicate the intent of the user.

The X server is different, because the X server is the same for every 
environment,
so it must have standard protocol to do configuration changes. On Wayland this 
is
not the case.

 This protocol basically live with compositor. It also provides the
 interface for library above to provide the randr function. For example
 efl, gtk, or ... If you think it is to build the standard protocol,
 that is fine. Anyway, my idea is that.

The point is, those toolkit libraries should not have access to 
wayland-randr at all.
It's not something a normal application should use.

Again in X things are different because the RandR protocol must exist, but 
there is
no easy way to make it restricted.

 2) About mode setting requirement:
 Most of case, it is for configuration of output as a whole. Generally
 it should be in setting panel for screen size, rotation, ...option
 setting. The user cases I mentioned are related with that setting. Of
 course you may prefer another way to implement.

Such use cases would be perfectly served by the special shell client, which 
already
needs a private, priviledged interface to the compositor anyway. This private
interface is almost always environment specific.

Wayland is not X; it is not intended that you could simply run a panel 
program at
any time and expect it to add a well-functioning panel to the desktop. Such
programs are special, so in the current design they are started directly by the
compositor.


 3) Security Issue:
 I found Pq, Jason, Japsper, ... don't want to expose the interface
 because of at will, arbitrary or disaster or any client. Actually it
 is security issue. That is fine. Yes, it really exists. We must be
 careful for that. Firstly I take it as a module, let owner to
 determine if he really need randr function. Secondly at the same time,
 it will be convenient for us to update randr when new wayland security
 control policy is ready.

Sure, we just need to be clear first on what the intended use cases are, 
because
they will affect the design a great deal, and even more on how well the 
proposal is
received.

 4) Here I can share an security idea for such protocol. I just want to
 show, if wayland provides such kind security checking, it will be
 easily adopted by randr interface. Previous interface could be default
 defined as general, other special could be identified as video or
 root. Please do not focus on the role definition and I just take an
 example.

 My security idea:
 Add two attributes separately to wl_client, wl_randr interface.
 wl_client has the user id and group id, wl_randr interface has an
 access attribute (general user, video user, root/admin). if you are
 afraid it is hacked, when you wl_closure_send, you can dynamically
 generate user id and group id.

 In client:
 wl_randr_set_mode(wl_wrandr_interface, ...) In compositor:
 Uid = get_uid(client)
 Gid = get_gid(client)
 If (It_video_user(uid, gid,..) || !is_root_user(uid,gid..))
 Wl_randr_send_permission(No permission to do that!\n);

 Continue...

Except that checking for the UID

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Thursday, March 06, 2014 4:42 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH] Bug fix client apps because of output change

On Thu,  6 Mar 2014 16:25:42 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

 1)
 Width and height of Panel and Background depend on output's, therefore
 they should be bound with output changes including mode, transform and
 scale.

 2)
 Update the min_allocation before resize the panel and background
 window. Add window_set_min_allocation function because it is invisible
 outside window.c.

 Signed-off-by: Quanxian Wang quanxian.w...@intel.com
 ---
  clients/desktop-shell.c | 70
 +
 clients/window.c|  7 + clients/window.h|  2 ++
  3 files changed, 79 insertions(+)

 diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
 a0c6b6d..9ccba34 100644
 --- a/clients/desktop-shell.c
 +++ b/clients/desktop-shell.c
 @@ -96,6 +96,12 @@ struct background {
  uint32_t color;
  };

 +struct mode {
 +int height;
 +int width;
 +uint32_t refresh;
 +};
 +
  struct output {
  struct wl_output *output;
  uint32_t server_output_id;
 @@ -103,6 +109,9 @@ struct output {

  struct panel *panel;
  struct background *background;
 +struct mode *mode;

Hi,

I think you could simplify this code quite a bit by just having the struct mode
embed in struct output, instead of dynamically allocating it. You will always 
have
exactly one struct mode for each struct output, right?
[Wang, Quanxian] fine. Got it.

 +uint32_t transform;
 +uint32_t scale;
  };

  struct panel_launcher {
 @@ -1128,6 +1137,10 @@ output_destroy(struct output *output)  {
  background_destroy(output-background);
  panel_destroy(output-panel);
 +
 +if (output-mode)
 +free(output-mode);
 +
  wl_output_destroy(output-output);
  wl_list_remove(output-link);

 @@ -1145,6 +1158,44 @@ desktop_destroy_outputs(struct desktop
 *desktop) }

  static void
 +update_output(struct output *output)
 +{
 +struct panel *panel = output-panel;
 +struct background *background = output-background;
 +int width, height;
 +
 +if (!output || !output-mode)
 +return;
 +
 +width = output-mode-width;
 +height = output-mode-height;
 +
 +switch (output-transform) {
 +case WL_OUTPUT_TRANSFORM_90:
 +case WL_OUTPUT_TRANSFORM_270:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_90:
 +case WL_OUTPUT_TRANSFORM_FLIPPED_270:
 +/* Swap width and height */
 +width = output-mode-height;
 +height = output-mode-width;
 +break;
 +default:
 +break;
 +}
 +
 +if (output-scale != 0) {
 +width /= output-scale;
 +height /= output-scale;
 +}
 +
 +/* Set min_allocation of panel */
 +window_set_min_allocation(panel-window, width, 32);

Is there a reason why the background window does not need the min_allocation
changed?
[Wang, Quanxian] With testing, this doesn't make error. Let me double check 
that. 

 +
 +window_schedule_resize(background-window, width, height);
 +window_schedule_resize(panel-window, width, 32); }
 +
 +static void
  output_handle_geometry(void *data,
 struct wl_output *wl_output,
 int x, int y,
 @@ -1157,6 +1208,8 @@ output_handle_geometry(void *data,  {
  struct output *output = data;

 +output-transform = transform;
 +update_output(output);
  window_set_buffer_transform(output-panel-window,
 transform); window_set_buffer_transform(output-background-window,
 transform); }
 @@ -1169,6 +1222,17 @@ output_handle_mode(void *data,
 int height,
 int refresh)
  {
 +struct output *output = data;
 +
 +if (flags  WL_OUTPUT_MODE_CURRENT) {
 +if (!output  !output-mode)
 +return;
 +
 +output-mode-width = width;
 +output-mode-height = height;
 +output-mode-refresh = refresh;
 +update_output(output);
 +}
  }

  static void
 @@ -1184,6 +1248,8 @@ output_handle_scale(void *data,  {
  struct output *output = data;

 +output-scale  = scale;
 +update_output(output);

I think calling update_output() would be better done from the wl_output.done
event handler, because there you are guaranteed to have all the relevant events
received. Otherwise you race the scheduled resize against receiving all the 
related
events.

However, the catch is that wl_output.done is in interface version = 2, but 
not in
version 1. For version 1 you'd need just call
update_output() directly like you already do.
[Wang, Quanxian] Ok

  window_set_buffer_scale(output-panel-window, scale);
  window_set_buffer_scale(output-background-window, scale);  } @@
 -1206,6 +1272,10 @@ output_init

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

Hi, Pq



Is there a reason why the background window does not need the
min_allocation changed?
[Wang, Quanxian] With testing, this doesn't make error. Let me double check 
that.

[Wang, Quanxian] According to the logic, we need set the min_allocation because 
of background should overwrite the whole output.
I have done with testing with set and not set. Nothing wrong happens in this 
condition. I add that in next updated patch.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

Ignore this, I have found a bug. After bug fixing, I will resend the patch. 
Sorry about that.

I have two monitors. 
VGA1(left) + HDMI3(right)

The bug is shown when you set the mode of HDMI3 to 800x600, and then move 
layout as HDMI3(left) + VGA1(right), and then change to mode 1920x1200.
It will have some issue.

Sorry about that.

Regards

Quanxian Wang

-Original Message-
From: Wang, Quanxian
Sent: Thursday, March 06, 2014 6:31 PM
To: wayland-devel@lists.freedesktop.org
Cc: ppaala...@gmail.com; Wang, Quanxian
Subject: [PATCH] Bug fix client apps because of output change

1)
Width and height of Panel and Background depend on output's, therefore they
should be bound with output changes including mode, transform and scale.

2)
Update the min_allocation before resize the panel and background window. Add
window_set_min_allocation function because it is invisible outside window.c.

Signed-off-by: Quanxian Wang quanxian.w...@intel.com
---
 clients/desktop-shell.c | 80
+++--
 clients/window.c|  7 +
 clients/window.h|  2 ++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
a0c6b6d..4373448 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -103,6 +103,15 @@ struct output {

   struct panel *panel;
   struct background *background;
+  struct {
+  int height;
+  int width;
+  uint32_t refresh;
+  } mode;
+
+  uint32_t interface_version;
+  uint32_t transform;
+  uint32_t scale;
 };

 struct panel_launcher {
@@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop *desktop)  }

 static void
+update_output(struct output *output)
+{
+  struct panel *panel = output-panel;
+  struct background *background = output-background;
+  int width, height;
+
+  if (!output)
+  return;
+
+  width = output-mode.width;
+  height = output-mode.height;
+
+  switch (output-transform) {
+  case WL_OUTPUT_TRANSFORM_90:
+  case WL_OUTPUT_TRANSFORM_270:
+  case WL_OUTPUT_TRANSFORM_FLIPPED_90:
+  case WL_OUTPUT_TRANSFORM_FLIPPED_270:
+  /* Swap width and height */
+  width = output-mode.height;
+  height = output-mode.width;
+  break;
+  default:
+  break;
+  }
+
+  if (output-scale != 0) {
+  width /= output-scale;
+  height /= output-scale;
+  }
+
+  /* Set min_allocation of panel */
+  window_set_min_allocation(panel-window, width, 32);
+  window_set_min_allocation(background-window, width, height);
+
+  window_schedule_resize(background-window, width, height);
+  window_schedule_resize(panel-window, width, 32); }
+
+static void
 output_handle_geometry(void *data,
struct wl_output *wl_output,
int x, int y,
@@ -1157,6 +1205,11 @@ output_handle_geometry(void *data,  {
   struct output *output = data;

+  output-transform = transform;
+
+  if (output-interface_version  2)
+  update_output(output);
+
   window_set_buffer_transform(output-panel-window, transform);
   window_set_buffer_transform(output-background-window,
transform);  } @@ -1169,12 +1222,29 @@ output_handle_mode(void *data,
  int height,
  int refresh)
 {
+  struct output *output = data;
+
+  if (flags  WL_OUTPUT_MODE_CURRENT) {
+  if (!output)
+  return;
+
+  output-mode.width = width;
+  output-mode.height = height;
+  output-mode.refresh = refresh;
+
+  if (output-interface_version  2)
+  update_output(output);
+  }
 }

 static void
 output_handle_done(void *data,
struct wl_output *wl_output)  {
+  struct output *output = data;
+
+  if (output-interface_version = 2)
+  update_output(output);
 }

 static void
@@ -1184,6 +1254,11 @@ output_handle_scale(void *data,  {
   struct output *output = data;

+  output-scale  = scale;
+
+  if (output-interface_version  2)
+  update_output(output);
+
   window_set_buffer_scale(output-panel-window, scale);
   window_set_buffer_scale(output-background-window, scale);  } @@
-1212,7 +1287,7 @@ output_init(struct output *output, struct desktop
*desktop)  }

 static void
-create_output(struct desktop *desktop, uint32_t id)
+create_output(struct desktop *desktop, uint32_t id, uint32_t version)
 {
   struct output *output;

@@ -1223,6 +1298,7 @@ create_output(struct desktop *desktop, uint32_t id)
   output-output =
   display_bind(desktop-display, id, wl_output_interface, 2);
   output-server_output_id = id;
+  output-interface_version = version;

   wl_output_add_listener(output-output, output_listener, output

RE: [PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

I have double checked the patch and retesting it.

The bug is caused by not setting background window size after output change.

This fix has been included in previous patch. Pq's comment is right. We should 
set background min allocation before resize. 

Why did I get this? Because I just use another patch which only comment 
min_allocation of background for testing. This caused the bug I mentioned.

Just use previous patch is fine.

Thanks

Regards

Quanxian Wang


-Original Message-
From: Wang, Quanxian
Sent: Friday, March 07, 2014 10:44 AM
To: wayland-devel@lists.freedesktop.org
Cc: ppaala...@gmail.com
Subject: RE: [PATCH] Bug fix client apps because of output change

Ignore this, I have found a bug. After bug fixing, I will resend the patch. 
Sorry
about that.

I have two monitors.
VGA1(left) + HDMI3(right)

The bug is shown when you set the mode of HDMI3 to 800x600, and then move
layout as HDMI3(left) + VGA1(right), and then change to mode 1920x1200.
It will have some issue.

Sorry about that.

Regards

Quanxian Wang

-Original Message-
From: Wang, Quanxian
Sent: Thursday, March 06, 2014 6:31 PM
To: wayland-devel@lists.freedesktop.org
Cc: ppaala...@gmail.com; Wang, Quanxian
Subject: [PATCH] Bug fix client apps because of output change

1)
Width and height of Panel and Background depend on output's, therefore
they should be bound with output changes including mode, transform and scale.

2)
Update the min_allocation before resize the panel and background
window. Add window_set_min_allocation function because it is invisible outside
window.c.

Signed-off-by: Quanxian Wang quanxian.w...@intel.com
---
 clients/desktop-shell.c | 80
+++--
 clients/window.c|  7 +
 clients/window.h|  2 ++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
a0c6b6d..4373448 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -103,6 +103,15 @@ struct output {

  struct panel *panel;
  struct background *background;
+ struct {
+ int height;
+ int width;
+ uint32_t refresh;
+ } mode;
+
+ uint32_t interface_version;
+ uint32_t transform;
+ uint32_t scale;
 };

 struct panel_launcher {
@@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop *desktop)
}

 static void
+update_output(struct output *output)
+{
+ struct panel *panel = output-panel;
+ struct background *background = output-background;
+ int width, height;
+
+ if (!output)
+ return;
+
+ width = output-mode.width;
+ height = output-mode.height;
+
+ switch (output-transform) {
+ case WL_OUTPUT_TRANSFORM_90:
+ case WL_OUTPUT_TRANSFORM_270:
+ case WL_OUTPUT_TRANSFORM_FLIPPED_90:
+ case WL_OUTPUT_TRANSFORM_FLIPPED_270:
+ /* Swap width and height */
+ width = output-mode.height;
+ height = output-mode.width;
+ break;
+ default:
+ break;
+ }
+
+ if (output-scale != 0) {
+ width /= output-scale;
+ height /= output-scale;
+ }
+
+ /* Set min_allocation of panel */
+ window_set_min_allocation(panel-window, width, 32);
+ window_set_min_allocation(background-window, width, height);
+
+ window_schedule_resize(background-window, width, height);
+ window_schedule_resize(panel-window, width, 32); }
+
+static void
 output_handle_geometry(void *data,
struct wl_output *wl_output,
int x, int y,
@@ -1157,6 +1205,11 @@ output_handle_geometry(void *data,  {
  struct output *output = data;

+ output-transform = transform;
+
+ if (output-interface_version  2)
+ update_output(output);
+
  window_set_buffer_transform(output-panel-window, transform);
  window_set_buffer_transform(output-background-window,
transform);  } @@ -1169,12 +1222,29 @@ output_handle_mode(void *data,
 int height,
 int refresh)
 {
+ struct output *output = data;
+
+ if (flags  WL_OUTPUT_MODE_CURRENT) {
+ if (!output)
+ return;
+
+ output-mode.width = width;
+ output-mode.height = height;
+ output-mode.refresh = refresh;
+
+ if (output-interface_version  2)
+ update_output(output);
+ }
 }

 static void
 output_handle_done(void *data,
struct wl_output *wl_output)  {
+ struct output *output = data;
+
+ if (output-interface_version = 2)
+ update_output(output);
 }

 static void
@@ -1184,6 +1254,11 @@ output_handle_scale(void *data,  {
  struct output *output = data;

+ output-scale  = scale;
+
+ if (output-interface_version  2)
+ update_output(output);
+
  window_set_buffer_scale(output-panel-window, scale);
  window_set_buffer_scale

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Friday, March 07, 2014 11:21 AM
To: Wang, Quanxian
Cc: Hardening; Matthias Clasen; wayland-devel@lists.freedesktop.org; Jasper St. 
Pierre; Pekka Paalanen
Subject: RE: [PATCH 1/6] Add weston randr protocol


On Mar 6, 2014 3:05 AM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:

 Thanks Pq's comment.

 By the way, if we make it as a configuration tools or testing protocol. Will 
 it be fine or not?

 For example graphics QA testing,
 I just send a patch to bug fix the client apps because of output change. I 
 use weston-wrandr to do the testing to make sure what I have done is right. 
 If I have not this tool, I don't make sure how to test it. Because you need 
 dynamically change rotate, transform, or mode set to make sure desktop shell 
 could get the change and really works fine with new change.

 Also this tool should be fine for wayland QA testing.

 Anyway, if you guys think it right for configuration or testing wayland 
 graphics tools, that is fine. I can continue working on these patches instead 
 of original idea to expose this to client to use.

Quanxian,
I think that for testing and configuration, this is fine.  In fact, when 
testing my fullscreen shell I really wish I had a utility like that.  Right 
now, with recent changes to toytoolkit, there's no good way to trigger a 
dynamic mode-switch for testing.  If you go ahead and add this, I have a couple 
suggestions:

1) Rename it weston_randr and make it weston-specific.

2) Make it a module and possibly require a command-line option to allow the 
command-line utility to connect.  It's great for testing, but we want the user 
to explicitly state that they want the command-line utility

3) It would be nice if we had a little GUI that gets launched priveledged to 
change the mode.  This can be launched from weston directly, so we can give it 
access to the private interface similar to the way the screenshooter or 
desktop-shell is launched.  This wouldn't need the security of a command-line 
option because it requires direct user interaction and can't be executed from a 
script.

[Wang, Quanxian] That is a good news. Thanks. I will continue working on that 
and provide the patches. Thanks for your response.

All that being said, I don't think you should expect GNOME, KDE, or the others 
to be interested in this as a standard.  They have their own mechanisms and 
GUI's for output configuration and I don't thin an output configuration 
standard would be of any real use.

Hope that helps,
--Jason Ekstrand


 Thanks for everyone's comment and help.

 Regards

 Quanxian Wang

 -Original Message-
 From: 
 wayland-devel-boun...@lists.freedesktop.orgmailto:wayland-devel-boun...@lists.freedesktop.org
 [mailto:wayland-devel-boun...@lists.freedesktop.orgmailto:wayland-devel-boun...@lists.freedesktop.org]
  On Behalf Of Pekka
 Paalanen
 Sent: Thursday, March 06, 2014 4:25 PM
 To: Wang, Quanxian
 Cc: Hardening; Jasper St. Pierre; Matthias Clasen;
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org;
  Jason Ekstrand; Zhang, Xiong Y
 Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 On Thu, 6 Mar 2014 03:01:11 +
 Wang, Quanxian quanxian.w...@intel.commailto:quanxian.w...@intel.com 
 wrote:
 
  Except the comment below. I mention some points.
 
  1) My idea:
  My original idea is from xrandr of xserver. I just want to let xrandr
  could be implemented in wayland. If no protocol is used, that is fine.
  But no way to implement some function for example transform, scale,
  mode set output. I have to create a protocol to communicate with
  compositor and let compositor do that.
 
 Usually such configuration changes should originate within the compositor 
 itself. In
 practice, it could be a special shell helper application (e.g. 
 weston-desktop-shell)
 using a private protocol to communicate the intent of the user.
 
 The X server is different, because the X server is the same for every 
 environment,
 so it must have standard protocol to do configuration changes. On Wayland 
 this is
 not the case.
 
  This protocol basically live with compositor. It also provides the
  interface for library above to provide the randr function. For example
  efl, gtk, or ... If you think it is to build the standard protocol,
  that is fine. Anyway, my idea is that.
 
 The point is, those toolkit libraries should not have access to 
 wayland-randr at all.
 It's not something a normal application should use.
 
 Again in X things are different because the RandR protocol must exist, but 
 there is
 no easy way to make it restricted.
 
  2) About mode setting requirement:
  Most of case, it is for configuration of output as a whole. Generally
  it should be in setting panel for screen size, rotation, ...option
  setting. The user cases I mentioned are related with that setting. Of
  course you may prefer another way to implement.
 
 Such use cases would

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

Just mention one thing

Pq:
But RandR is a disaster if random applications use it! Windows and icons 
squashed into top-left corner, dialogs too large to fit on the screen after the 
random application fails to restore the video mode, or the picture just looking 
horrible and an average user not even knowing why everything suddenly went ugly.
[Wang, Quanxian] For this, if you think it is disaster because of mode setting. 
It is a pity. If the apps designer is careful, layout should be consistent with 
width or height of output. In my testing for randr protocol, I found window is 
designed to use width and height of output. Because it uses width and height of 
output, but it doesn't care the change of output(wl_output provides the 
mechanism to listen mode, scale change). You can read my patch 6/6 for bug fix. 
It is just one fix. It is the apps design flaw instead of wayland issue. Also 
you also find 200 or 600 some hard code number is set.

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com] 
Sent: Wednesday, March 05, 2014 4:48 PM
To: Wang, Quanxian
Cc: Jasper St. Pierre; Jason Ekstrand; Hardening; Matthias Clasen; 
wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
Subject: Re: [PATCH 1/6] Add weston randr protocol

Hi,

first, could you please try to do proper quoting in emails so we can clearly 
see what you wrote and what is a quotation, for more levels than just the most 
recent email. See how I do it. Thanks.

I previously bypassed the question why, but in the below let's dig deeped 
into that.


On Wed, 5 Mar 2014 05:48:33 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 
 
 From: wayland-devel-boun...@lists.freedesktop.org
 [mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of 
 Jasper St. Pierre Sent: Wednesday, March 05, 2014 12:51 PM To: Jason 
 Ekstrand Cc: Hardening; Matthias Clasen; 
 wayland-devel@lists.freedesktop.org; Pekka Paalanen; Zhang, Xiong Y; 
 Wang, Quanxian Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 I'd also say that in the automotive case, you *don't* want arbitrary 
 modesetting. The user of the infotainment system in your Land Rover 
 will not want to change the display resolution from 800x600 to 
 1024x768; she won't choose it from a dropdown, and it's very likely 
 she doesn't know what such functionality is. [Wang, Quanxian] For 
 example, someone like screen to contain more icons(big resolution) and 
 someone like big icons in screen(small resolution). Resolution changed 
 will be one way. I just say one way. In randr protocol, I don't want 
 arbitrary. It is under the control. If security is fine, we could make 
 it. If you really need it at once, just make it happen as a module. 
 That is fine. Someone like 1024 or some one like 1920.
 It is different. I just provide one method for user or developer to 
 choose under  their requirement. Such UIs are also fairly closely 
 designed for various resolutions, with pixel-perfect graphics and so 
 forth. Letting any client change the mode would be disaster, as now 
 all the button sizes which were tested with various labels and font 
 sizes and fingers are all different, and the rest of everything is 
 chopped off! [Wang, Quanxian] I don't' see xrandr is a disaster for 
 xserver. It is a useful tool. Just like in window system, I will 
 change the resolution from 1024 to 1920. One thing we need to be done 
 is that it is must under the control. Basically we expected wayland 
 could do that. Actually we have the same goal, let right client do 
 right thing. But not means we should less some function. I expected 
 wayland security will be powerful.

But RandR is a disaster if random applications use it! Windows and icons 
squashed into top-left corner, dialogs too large to fit on the screen after the 
random application fails to restore the video mode, or the picture just looking 
horrible and an average user not even knowing why everything suddenly went ugly.

I must agree with Jasper and Jason here. What you are doing is a dynamic 
compositor configuration protocol. Configuration is for system administrators, 
not for the average Joe User. Furthermore, configuration changes made this way 
are not permanent, not with RandrR either (which for X is a blessing, a reboot 
will fix a messed up configuration). That means if Joe the User is lucky and 
finds a command line snippet to do what he wants, the setting will be gone 
after a reboot.

Only the technical users may want to change the resolution, others simply 
don't care as long as the picture is good. The non-technical users probably 
would not know they could do that, or cannot even imagine why they would ever 
want to do it.

If a graphical system wants to expose a setting like big icons vs.
small icons or whatever, they build that option into the window system stack, 
which for something like automotive would include at least the compositor, 
toolkits, and applications acting together to maintain the quality of the UI

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Wednesday, March 05, 2014 6:04 PM
To: Wang, Quanxian
Cc: Jasper St. Pierre; Jason Ekstrand; Hardening; Matthias Clasen;
wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Wed, 5 Mar 2014 09:40:32 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Just mention one thing

 Pq:
 But RandR is a disaster if random applications use it! Windows and
 icons squashed into top-left corner, dialogs too large to fit on the
 screen after the random application fails to restore the video mode,
 or the picture just looking horrible and an average user not even
 knowing why everything suddenly went ugly. [Wang, Quanxian] For this,
 if you think it is disaster because of mode setting. It is a pity. If
 the apps designer is careful, layout should be consistent with width
 or height of output. In my testing for randr protocol, I found window
 is designed to use width and height of output. Because it uses width
 and height of output, but it doesn't care the change of
 output(wl_output provides the mechanism to listen mode, scale change).
 You can read my patch 6/6 for bug fix. It is just one fix.
 It is the apps design flaw instead of wayland issue. Also you also
 find 200 or 600 some hard code number is set.

Yeah, it looks like the patch 6/6 would be a nice fix, but I think you should 
resend
that alone, so it won't have to wait until the protocol design is concluded. 
It's a
stand-alone patch, right?
[Wang, Quanxian] yes. It is found when I do testing. But not related with the 
weston randr protocol.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

Except the comment below. I mention some points.

1) My idea:
My original idea is from xrandr of xserver. I just want to let xrandr could be 
implemented in wayland. If no protocol is used, that is fine. But no way to 
implement some function for example transform, scale, mode set output. I have 
to create a protocol to communicate with compositor and let compositor do that.

This protocol basically live with compositor. It also provides the interface 
for library above to provide the randr function. For example efl, gtk, or ...
If you think it is to build the standard protocol, that is fine. Anyway, my 
idea is that.

2) About mode setting requirement:
Most of case, it is for configuration of output as a whole. Generally it should 
be in setting panel for screen size, rotation, ...option setting. The user 
cases I mentioned are related with that setting. Of course you may prefer 
another way to implement. 

3) Security Issue:
I found Pq, Jason, Japsper, ... don't want to expose the interface because of 
at will, arbitrary or disaster or any client. Actually it is security issue. 
That is fine.
Yes, it really exists. We must be careful for that. Firstly I take it as a 
module, let owner to determine if he really need randr function. Secondly at 
the same time, it will be convenient for us to update randr when new wayland 
security control policy is ready.

4) Here I can share an security idea for such protocol. I just want to show, if 
wayland provides such kind security checking, it will be easily adopted by 
randr interface. Previous interface could be default defined as general, other 
special could be identified as video or root. Please do not focus on the role 
definition and I just take an example. 

My security idea:
Add two attributes separately to wl_client, wl_randr interface.
wl_client has the user id and group id, wl_randr interface has an access 
attribute (general user, video user, root/admin).
if you are afraid it is hacked, when you wl_closure_send, you can dynamically 
generate user id and group id.

In client:
wl_randr_set_mode(wl_wrandr_interface, ...)
In compositor:
Uid = get_uid(client)
Gid = get_gid(client)
If (It_video_user(uid, gid,..) || !is_root_user(uid,gid..))
Wl_randr_send_permission(No permission to do that!\n);

Continue...


5) At last I answer the questions raised by Pq for me.

- Would you be happy with something that works for your specific use
  case only?
[Wang Quanxian]not happy, really not happy. I like what I do is helpful for 
everyone.

- Do you want to establish a universal standard, i.e. get this into
  Wayland core? If so, why?
[Wang Quanxian] No, it lives with compositor. Without compositor, randr could 
do nothing.

- Do you want a sample implementation and the protocol be included in
  Weston specifically? If so, why?
[Wang Quanxian] weston or other compositor, any compositor which wants that. I 
just provide a tool or protocol to implement randr function.

The foremost, what is the use case?
[Wang Quanxian] still, more cases are listed. In window, in linux(gnome, KDE), 
there is always some setting contains ration, leftof, rightof, primary, slave, 
scale, transform, mode set. Is it not use case? If not, why they are there? You 
know what I mean. Currently tablet, TV, automotive have not such option, it is 
not user don't want it. It is because no one provides that. Also it maybe some 
other reason, some apps don't like that flexible mode setting because it make 
it crashed or mess up.

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com]
Sent: Wednesday, March 05, 2014 6:28 PM
To: Wang, Quanxian
Cc: Jasper St. Pierre; Jason Ekstrand; Zhang, Xiong Y; Hardening; Matthias 
Clasen;
wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/6] Add weston randr protocol

On Wed, 5 Mar 2014 09:24:34 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Hi, Jasper  Jason

 In order to understand it more, I provide such cases.


 1)   One customer uses handset which OS using wayland. When he
 open the handset, there is the menu screen which contain icons list.
 Someone want to see 10 icons, someone wants to see 20 icons. (one
 requirement, it really happens, at least when use my handset, I like
 to see everything in one page or more). Surface mode set is one way,
 output mode set is another way, apps setting is also another way(font
 size or icon size).

Runtime video mode switching in a phone? Is that even a thing? I mean, does the
hardware even support anything but a single video mode for the panel?

As for the UI size, that is much better handled at the drawing phase in 
applications,
rather than by the scanout hardware doing scaling.
[Wang, Quanxian] Yes, I agree your point, in the drawing phase to do that based 
on your parent layout(the parent at last point to output/root window). if user 
open setting panel, and select screen size setting to some mode, what happens 
on application?
Mess up or disaster? I come across such thing

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: wayland-devel-boun...@lists.freedesktop.org 
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Jason Ekstrand
Sent: Wednesday, March 05, 2014 9:57 PM
To: Pekka Paalanen
Cc: Hardening; Jasper St. Pierre; Matthias Clasen; 
wayland-devel@lists.freedesktop.org; Zhang, Xiong Y; Wang, Quanxian
Subject: Re: [PATCH 1/6] Add weston randr protocol


On Mar 5, 2014 4:27 AM, Pekka Paalanen 
ppaala...@gmail.commailto:ppaala...@gmail.com wrote:

 On Wed, 5 Mar 2014 09:24:34 +
 Wang, Quanxian quanxian.w...@intel.commailto:quanxian.w...@intel.com 
 wrote:

  Hi, Jasper  Jason
 
  In order to understand it more, I provide such cases.
 
 
  1)   One customer uses handset which OS using wayland. When he
  open the handset, there is the menu screen which contain icons list.
  Someone want to see 10 icons, someone wants to see 20 icons. (one
  requirement, it really happens, at least when use my handset, I like
  to see everything in one page or more). Surface mode set is one way,
  output mode set is another way, apps setting is also another way(font
  size or icon size).

 Runtime video mode switching in a phone? Is that even a thing? I mean,
 does the hardware even support anything but a single video mode for
 the panel?

 As for the UI size, that is much better handled at the drawing phase in
 applications, rather than by the scanout hardware doing scaling.

  2)   Continue, customer click the web page apps, you could see
  the web contents. He can change the font size by setting web page(see
  clear or more contents). The same above, surface is one way, web
  setting another way, mode set for output is also a way.

 I would think adjusting what the browser renders is the only sane way.
 You definitely do not want a browser to control the video mode.

  3)   Ok, You can tell me, surface could do that, that is right.

 No, abusing the fullscreen surface semantics for all that would be
 wrong; the same as using video mode setting, in my opinion.

  You change menu screen surface, but at the same time you want to
  customer change the webpage surface with same action. Why do I say
  that? according to the custom, someone wants to see small or big,
  less or more, it will do the same thing in another apps. Generally
  when user have some sense for one apps to change the size of icon, it
  has the same feeling on other apps. Surface just update one surface,
  output will update all surfaces on the output. It is one shot thing.
  Surface mode set is one choice, why not provide output mode set to
  user? All done or part done, it is up to user. We just provide the
  choice.

 This is not a thing that should be set via output properties. I
 believe this should be done via the phone environment (cf. desktop
 environment) specific protocols, which already need to handle a lot
 more than that.

 Output properties are about the physical output features, like the
 size of a pixel. Those do not change with software usage.

Allow me to add just a bit to what Pekka said above.  10 or 15 years ago when 
people were using CRT monitors and drawing icons at multiple resolutions was 
expensive, mode-setting made sense.  It provided a simple way to physically 
scale the UI without making more work for the hardware.  However, in today's 
world of LCD's this is not the case.

First of all, this is because, on an LCD, there is no such thing as mode 
setting.  CRT monitors could actually be run at multiple different modes by 
adjusting how the ray scanned across the glass at the front of the monitor.  
With an LCD, all you can do is fake a different mode by scaling the output to 
more-or-less fill the monitor.  This is what your LCD does when you plug 
something in via VGA and it provides a smaller picture.  If, on the other hand, 
you plug it in via DVI there's a decent chance that it never gets sent a 
different mode at all but that the GPU siliently scales the picture.  The 
reason for this is that the *only* way to get a different mode is to scale the 
picture and the GPU will do a better job than the monitor.  In other words,  
there is nosuch thing as modesetting on an LCD, only scaling.

What it sounds like your user wants is not modesetting but a make everything 
bigger/smaller option.  Yes, one way to implement this would be some fake 
modesetting system where they set the screen resolution.  However, that is 
going to end with the applications drawing at one resolution, then the 
compositor or something scaling it to another resolution and everything looking 
fuzzy.  The user does *not* want fuzzy.  A far better option would be to 
provide a configuration interface that ties your options panel to your toolkit 
that allows them to set some sort of a universal size factor that affects icon 
resource sizes, font sizes, etc.  Then the clients will simply all render with 
bigger icons and text.  Since you are working on a controlled system, you 
should be able to ensure that this happens.  You will get

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

Hi, All

With the first version of randr protol, I got many useful idea and comments. 
Thanks.

Before I send the email, I have make it happen in real world but need more deep 
testing.

Here are new changes and idea for that protocol based on the idea and comments, 
welcome your input. 

1) Unique operation issue
Given that one client has two more threads to do mode setting on the same 
output at the same time, how to identify what response (done event) is belong 
to one or another request when they want to get response? 

This is a design flaw in the first version of randr protocol.

The solution is to use the wayland/weston mechanism, every request will 
generate a resource which be used to send done event to the owner who start it. 
Owner could set the listener on that and keep tuning on the response event.

For example
In client:
randr_transform = 
wl_randr_set_transform(randr.randr,wayland_output,argument.transform);

/* Here will will add listen callback to get the response for this unique 
request */
wl_randr_add_listener(randr_transform, randr_transform_listener, randr);

In compositor:
randr_resource = wl_resource_create(client,wl_randr_interface,1, action);
wl_randr_send_action_done(randr_resource, 1WL_RANDR_ACTION_TRANSFORM, ret, 
action);
wl_resource_destroy(randr_resource);

2) Security and mess up issue
Will take randr protocol implementation as a module or plugin. This will keep 
compositor clear and avoid messing up in compositor.c. Also it is will be fine 
for compositor when this could be public.
The security should be a general issue for wayland instead of for randr 
protocol only. Take randr protocol as a module for user to determine if they 
want to public randr interface for their clients. Once security mechanism is 
built up in wayland, randr could adopt the mechanism to enhance the security of 
randr protocol.

weston --tty=1 --modules=wrandr.so
or set it in weston.ini like that
[core]
modules=wrandr.so

3) Group randr operations
After talking with PQ, in order to avoid glitches, group operation is needed. 
However, if operate on two outputs more at one time, it will bring more issues 
which could not control. In this randr design, will not provide group operation 
on multiple outputs. We provide atomic operation on one output, multi outputs 
operation logic are left to designer/developers. Group operation on one output 
will be designed. For example, you can set mode, scale, and transform at one 
time with one randr request.

4) Configuration interface
Weston randr protocol will be taken as configuration interface for output mode 
setting in wayland. So the permission for that will be strictly under the 
control. Before security mechanism is ready, randr module will be designed for 
compositor as a choice. It is just a loadable module or plugin for special 
compositors. User will take the decision if start it when boot up compositor as 
a module.

5) mode setting parameters control
Mode and output will be under the control. User could not randomly to set their 
mode. They have to select the available modes and outputs provided by 
compositor. Don't allow random mode setting. The mode and output information 
could be provided by weston-randr apps or wl_output interface.

6) Disconnected outputs
When user query output information, showing connected and disconnected output 
as a whole will be fine for user and QA people. Sometime, QA people or user 
like that information. It will be helpful for them to identify how many outputs 
are provided by their machine. Which is connected and which is not connected.

7) wl_output property event
Delete get_output_name request and event in protocol. wl_output name event will 
be added into wl_output protocol. This event will send the output name to user 
when they bind wl_output. For width and height of wl_output, I am not sure if 
it is should be sent out at the same time. My idea is it should be the same 
event to send name, width, height after output is changed. But currently I will 
not add that.

8) adding set_scale request
Mode, scale, transform is the basic property of output, I will add them as a 
whole.

Thanks

Regards

Quanxian Wang

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

On Thu, 27 Feb 2014 09:15:55 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 
 
 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Thursday, February 27, 2014 4:36 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
 Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 On Thu, 27 Feb 2014 08:06:23 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  
  
  -Original Message-
  From: Pekka Paalanen [mailto:ppaala...@gmail.com]
  Sent: Thursday, February 27, 2014 3:28 PM
  To: Wang

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: wayland-devel-boun...@lists.freedesktop.org 
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Jasper St. 
Pierre
Sent: Wednesday, March 05, 2014 10:53 AM
To: Wang, Quanxian
Cc: Hardening; Matthias Clasen; wayland-devel@lists.freedesktop.org; Pekka 
Paalanen; Jason Ekstrand; Zhang, Xiong Y
Subject: Re: [PATCH 1/6] Add weston randr protocol

So, before we go further on this, I need to ask three basic questions: what are 
the goals of this interface? Who is supposed to use this interface? Why would 
they use this interface?
[Wang, Quanxian]
What are the goals of this interface?
The goal of this interface is to provide dynamic mode setting for output. Make 
wayland desktop configuration more flexible for scale, transform or mode 
setting.

Who is supposed to use this interface?
Any customer or user want to rotate, scale or mode the output when they needed. 
For example, Automotive, there are 2 screens, one is left another is on back, 
want to rotate the screen. You can sleep, stand, and more body postures, you 
want to rotate the screen to make you comfortable. For Automotive, mobile, TV 
or even if desktop. Just like xrandr for xserver.
You want to get another resolution (mode), for example 1440x900, or 1920x1080. 
Monitor producer provide such things in order to let user use this. This 
interface provides such function dynamically to meet the requirement.

Why would they use this interface? Dynamic mode setting for output will be 
provided to developer.

Here is a general case:
In window system or linux system, you want to change the resolution of your 
desktop at will. For example 1440x900 to 1920x1080, do you want to kill desktop 
server and then configure it and then restart desktop server. You will not, you 
just open the display configuration, and set it to 1920x1080 and then save it. 
The resolution of desktop will be changed. If you want another, same thing.

Dynamic mode setting for output is found everywhere in matured window or linux 
desktop system. Providing this interface to embedded system, multi screen 
supported system will be fine.


On Tue, Mar 4, 2014 at 9:07 PM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
Hi, All

With the first version of randr protol, I got many useful idea and comments. 
Thanks.

Before I send the email, I have make it happen in real world but need more deep 
testing.

Here are new changes and idea for that protocol based on the idea and comments, 
welcome your input.

1) Unique operation issue
Given that one client has two more threads to do mode setting on the same 
output at the same time, how to identify what response (done event) is belong 
to one or another request when they want to get response?

This is a design flaw in the first version of randr protocol.

The solution is to use the wayland/weston mechanism, every request will 
generate a resource which be used to send done event to the owner who start it. 
Owner could set the listener on that and keep tuning on the response event.

For example
In client:
randr_transform = 
wl_randr_set_transform(randr.randr,wayland_output,argument.transform);

/* Here will will add listen callback to get the response for this unique 
request */
wl_randr_add_listener(randr_transform, randr_transform_listener, randr);

In compositor:
randr_resource = wl_resource_create(client,wl_randr_interface,1, action);
wl_randr_send_action_done(randr_resource, 1WL_RANDR_ACTION_TRANSFORM, ret, 
action);
wl_resource_destroy(randr_resource);

2) Security and mess up issue
Will take randr protocol implementation as a module or plugin. This will keep 
compositor clear and avoid messing up in compositor.c. Also it is will be fine 
for compositor when this could be public.
The security should be a general issue for wayland instead of for randr 
protocol only. Take randr protocol as a module for user to determine if they 
want to public randr interface for their clients. Once security mechanism is 
built up in wayland, randr could adopt the mechanism to enhance the security of 
randr protocol.

weston --tty=1 --modules=wrandr.so
or set it in weston.ini like that
[core]
modules=wrandr.so

3) Group randr operations
After talking with PQ, in order to avoid glitches, group operation is needed. 
However, if operate on two outputs more at one time, it will bring more issues 
which could not control. In this randr design, will not provide group operation 
on multiple outputs. We provide atomic operation on one output, multi outputs 
operation logic are left to designer/developers. Group operation on one output 
will be designed. For example, you can set mode, scale, and transform at one 
time with one randr request.

4) Configuration interface
Weston randr protocol will be taken as configuration interface for output mode 
setting in wayland. So the permission for that will be strictly under the 
control. Before security mechanism is ready, randr module will be designed for 
compositor as a choice

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: wayland-devel-boun...@lists.freedesktop.org 
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Jasper St. 
Pierre
Sent: Wednesday, March 05, 2014 12:51 PM
To: Jason Ekstrand
Cc: Hardening; Matthias Clasen; wayland-devel@lists.freedesktop.org; Pekka 
Paalanen; Zhang, Xiong Y; Wang, Quanxian
Subject: Re: [PATCH 1/6] Add weston randr protocol

I'd also say that in the automotive case, you *don't* want arbitrary 
modesetting. The user of the infotainment system in your Land Rover will not 
want to change the display resolution from 800x600 to 1024x768; she won't 
choose it from a dropdown, and it's very likely she doesn't know what such 
functionality is.
[Wang, Quanxian] For example, someone like screen to contain more icons(big 
resolution) and someone like big icons in screen(small resolution). Resolution  
changed will be one way. I just say one way. In randr protocol, I don't want 
arbitrary. It is under the control. If security is fine, we could make it. If 
you really need it at once, just make it happen as a module. That is fine. 
Someone like 1024 or some one like 1920. It is different. I just provide one 
method for user or developer to choose under  their requirement.
Such UIs are also fairly closely designed for various resolutions, with 
pixel-perfect graphics and so forth. Letting any client change the mode would 
be disaster, as now all the button sizes which were tested with various labels 
and font sizes and fingers are all different, and the rest of everything is 
chopped off!
[Wang, Quanxian] I don't' see xrandr is a disaster for xserver. It is a useful 
tool. Just like in window system, I will change the resolution from 1024 to 
1920. One thing we need to be done is that it is must under the control. 
Basically we expected wayland could do that. Actually we have the same goal, 
let right client do right thing. But not means we should less some function. I 
expected wayland security will be powerful.

If some video player wants to go full-screen and all it has is a 800x600 
surface, then let the compositor set the mode based on seeing that a 
full-screen surface has size 800x600, and we can natively set the mode, without 
the client ever communicating that it wants to do a mode change.
[Wang, Quanxian] Yes, surface full screen mode set could do that. But it is 
only for one surface. How about others surface. It is really different thing. 
Output configuration is for all things happened on the output. Surface 
configuration is for all things happened on the surface.
One case, if it is pixel-perfect for graphics like you said, why monitor or 
screen producer provide more resolutions for that? Can you expect the reason? I 
think fix mode provided will be more cheap that more. Why producer like to do 
that? from my view, it is definitely the requirement of their customers.
On Tue, Mar 4, 2014 at 10:56 PM, Jason Ekstrand 
ja...@jlekstrand.netmailto:ja...@jlekstrand.net wrote:

Quanxian,
I think what Jasper is getting at is the difference between a configuration 
interface and a client-facing interface.  Unfortunately, in X the RandR 
interface was used in both capacities.  Allow me to try and clerify this 
distinction.

A client-facing interface is one that is detected and used by various 
clients.  The intention of most client-facing interfaces is to make them 
general enough for all compositors and eventually get them mainlined into the 
core wayland protocol.  If you are writing this kind of interface, you need to 
very specifically justify why clients need this kind of interface and not 
another.  In particular, most clients have no buiseness changing the screen 
resolution.  Some clients may cause a screen resolution change due to, for 
example, making their surface fullscreen.  However, that is a very different 
thing from making arbitrary resolution changes.  If you have a good reason for 
a client to make arbitrary RandR type changes other than dynamic configuration, 
you need to be very clear about why and we need to analize if there is a better 
way to do that than simply giving them access to RandR.

When I say configuration interface I mean something that is intended for the 
express purpose of changing Weston's confguration dynamically.  This could be a 
command-line or graphical utility that allows the user to select a new screen 
resolution, orientation, or whatever.  If this is your intention, then you 
should probably use the weston_ prefix rather than the wl_ prefix and it should 
be considered weston-specific.  Also, if possible, clients that use this 
interface should be launched from weston to ensure that they can only be used 
with the user's permission.  I'm not sure how to make this work properly for a 
command-line client, but a graphical one could be special-cased inside weston 
to be allowed the interface.

What I really don't think we need (and where I'm afraid this is headed) is a 
priviledged client-facing interface.  I can see very little

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Wednesday, March 05, 2014 11:56 AM
To: Wang, Quanxian
Cc: Hardening; Matthias Clasen; wayland-devel@lists.freedesktop.org; Zhang, 
Xiong Y; Pekka Paalanen; Jasper St. Pierre
Subject: RE: [PATCH 1/6] Add weston randr protocol


Quanxian,
I think what Jasper is getting at is the difference between a configuration 
interface and a client-facing interface.  Unfortunately, in X the RandR 
interface was used in both capacities.  Allow me to try and clerify this 
distinction.

A client-facing interface is one that is detected and used by various 
clients.  The intention of most client-facing interfaces is to make them 
general enough for all compositors and eventually get them mainlined into the 
core wayland protocol.  If you are writing this kind of interface, you need to 
very specifically justify why clients need this kind of interface and not 
another.  In particular, most clients have no buiseness changing the screen 
resolution.  Some clients may cause a screen resolution change due to, for 
example, making their surface fullscreen.  However, that is a very different 
thing from making arbitrary resolution changes.  If you have a good reason for 
a client to make arbitrary RandR type changes other than dynamic configuration, 
you need to be very clear about why and we need to analize if there is a better 
way to do that than simply giving them access to RandR.

When I say configuration interface I mean something that is intended for the 
express purpose of changing Weston's confguration dynamically.  This could be a 
command-line or graphical utility that allows the user to select a new screen 
resolution, orientation, or whatever.  If this is your intention, then you 
should probably use the weston_ prefix rather than the wl_ prefix and it should 
be considered weston-specific.  Also, if possible, clients that use this 
interface should be launched from weston to ensure that they can only be used 
with the user's permission.  I'm not sure how to make this work properly for a 
command-line client, but a graphical one could be special-cased inside weston 
to be allowed the interface.

[Wang, Quanxian] yes. Weston randr protocol live with compositor.  It will 
communicate with compositor to do that. weston_randr will be more reasonable. 
Thanks Jason

What I really don't think we need (and where I'm afraid this is headed) is a 
priviledged client-facing interface.  I can see very little use for general 
modesetting interface that all compositors support.  Output configuration GUI's 
aren't that hard to write and each compositor can have their own.  I highly 
doubt someone will write a particularly spectacular third-party output 
configuration GUI that someone will want to use with GNOME or KDE.  What about 
a standard command-line utility?  Frankly, I can't see that ending well.  The 
primary use for it would be by scripts and other clients completely breaking 
whatever security procedures we try to set up.

[Wang, Quanxian] Yes, permission control is the big stock on the way. As a 
script or application, you still need to communicate with compositor to do that.

So far, every use case you have given indicates that this is entirely a 
configuration interface.  I'm not saying that you don't have a good reason for 
wanting to be able to change the output configuration.  If what you're doing is 
something other than just on-the-fly weston configuration, I have a feeling 
that you have a more specific use in mind that would be better served by a more 
specific interface.

[Wang, Quanxian] not only for configuration. If configuration, just admin or 
root does that. It provides the mode set including transform, scale as a whole 
for output(screen) instead of for surface.  But I acknowledge, it should be 
under the control.

Thanks,
--Jason Ekstrand

On Mar 4, 2014 9:22 PM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:





 From: 
 wayland-devel-boun...@lists.freedesktop.orgmailto:wayland-devel-boun...@lists.freedesktop.org
  
 [mailto:wayland-devel-boun...@lists.freedesktop.orgmailto:wayland-devel-boun...@lists.freedesktop.org]
  On Behalf Of Jasper St. Pierre
 Sent: Wednesday, March 05, 2014 10:53 AM
 To: Wang, Quanxian
 Cc: Hardening; Matthias Clasen; 
 wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org;
  Pekka Paalanen; Jason Ekstrand; Zhang, Xiong Y

 Subject: Re: [PATCH 1/6] Add weston randr protocol



 So, before we go further on this, I need to ask three basic questions: what 
 are the goals of this interface? Who is supposed to use this interface? Why 
 would they use this interface?

 [Wang, Quanxian]

 What are the goals of this interface?

 The goal of this interface is to provide dynamic mode setting for output. 
 Make wayland desktop configuration more flexible for scale, transform or mode 
 setting.



 Who is supposed to use this interface?

 Any customer or user want to rotate

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

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

On Thu, 27 Feb 2014 11:28:00 +0800
Quanxian Wang quanxian.w...@intel.com wrote:

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

Hi,

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

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

Fix this.

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

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

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

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

On Thu, 27 Feb 2014 08:06:23 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 
 
 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Thursday, February 27, 2014 3:28 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
 Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 On Thu, 27 Feb 2014 11:28:00 +0800
 Quanxian Wang quanxian.w...@intel.com wrote:
 
  Weston protocol wrandr will provide interface to
  1) set output mode
  2) set output transform
  3) move output to relative position
  4) provide disconnected display port information
  
  *Dynamic* mode setting is the main objective of this protocol.
  Remember, it is one shot operation. For example, if setting the 
  mode, just call one request wl_randr_set_mode without any other 
  operation.
 
 Hi,
 
 like I said in my other reply, this is a configuration interface, not 
 something for all applications to use. Protocol comments below 
 assuming, that this will be a configuration interface and that the 
 generic idea is acceptable.
 
  Signed-off-by: Quanxian Wang quanxian.w...@intel.com
  Reviewed-by: Zhang, Xiong Y xiong.y.zh...@intel.com
  ---
   protocol/Makefile.am |   1 +
   protocol/randr.xml   | 151
  +++ 2 files
  changed, 152 insertions(+) create mode 100644 protocol/randr.xml
  
  diff --git a/protocol/Makefile.am b/protocol/Makefile.am index
  5e331a7..df2e070 100644
  --- a/protocol/Makefile.am
  +++ b/protocol/Makefile.am
  @@ -5,6 +5,7 @@ protocol_sources =  \
  text.xml\
  input-method.xml\
  workspaces.xml  \
  +   randr.xml   \
  text-cursor-position.xml\
  wayland-test.xml\
  xdg-shell.xml   \
  diff --git a/protocol/randr.xml b/protocol/randr.xml new file mode
  100644 index 000..f15e87a
  --- /dev/null
  +++ b/protocol/randr.xml
  @@ -0,0 +1,151 @@
  +?xml version=1.0 encoding=UTF-8? protocol name=randr
  +
  +  copyright
  +Copyright (c) 2012-2013 Collabora, Ltd.
 
 Fix this.
 
  +
  +Permission to use, copy, modify, distribute, and sell this
  +software and its documentation for any purpose is hereby
  granted
  +without fee, provided that the above copyright notice appear in
  +all copies and that both that copyright notice and this
  permission
  +notice appear in supporting documentation, and that the name of
  +the copyright holders not be used in advertising or publicity
  +pertaining to distribution of the software without specific,
  +written prior permission.  The copyright holders make no
  +representations about the suitability of this software for any
  +purpose.  It is provided as is without express or implied
  +warranty.
  +
  +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO
  THIS
  +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
  AND
  +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR
  ANY
  +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
  +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
  WHETHER IN
  +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
  +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
  +THIS SOFTWARE.
  +  /copyright
  +
  +  interface name=wl_randr version=1
  +description summary=randr
  +  The global interface exposing randr capabilities.
  +  As a wl_randr, that provides the interfaces for apps to more
  operations
  +  on output.
  +
  +  The aim of wl_randr is to get modes list, choose preferred
  mode,
  +  layout the output including position, rotate, and en/disable.
  +  The idea is from xrandr protocoal of xserver. It is very
  convenient for
  +  weston/wayland user to operates on mode setting of output.
  +/description
  +
  +enum name=error
  +  entry name=bad_randr value=0
  + summary=the to-be wl_randr is invalid/
  +/enum
  +
  +request name=destroy type=destructor
  +  description summary=unbind from the wl_randr interface
  +   Informs the server that the client will not be using this
  +   protocol object anymore. This does not affect any other
  +   objects, wl_randr objects included.
  +  /description
  +/request
  +
  +request name=set_mode
  +  description summary=set the mode of output
  +   set the mode of output
  +  /description
  +  arg name=output type=object interface=wl_output
  +   summary=the output object/
  +  arg

RE: [PATCH 1/6] Add weston randr protocol

[Wang, Quanxian]

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

On Thu, 27 Feb 2014 09:15:55 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 
 
 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Thursday, February 27, 2014 4:36 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
 Subject: Re: [PATCH 1/6] Add weston randr protocol
 
 On Thu, 27 Feb 2014 08:06:23 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  
  
  -Original Message-
  From: Pekka Paalanen [mailto:ppaala...@gmail.com]
  Sent: Thursday, February 27, 2014 3:28 PM
  To: Wang, Quanxian
  Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
  Subject: Re: [PATCH 1/6] Add weston randr protocol
  
  On Thu, 27 Feb 2014 11:28:00 +0800
  Quanxian Wang quanxian.w...@intel.com wrote:
  
   Weston protocol wrandr will provide interface to
   1) set output mode
   2) set output transform
   3) move output to relative position
   4) provide disconnected display port information
   
   *Dynamic* mode setting is the main objective of this protocol.
   Remember, it is one shot operation. For example, if setting the 
   mode, just call one request wl_randr_set_mode without any other 
   operation.
  
  Hi,
  
  like I said in my other reply, this is a configuration interface, 
  not something for all applications to use. Protocol comments below 
  assuming, that this will be a configuration interface and that the 
  generic idea is acceptable.
...
  If I change the mode on two different outputs, how do I know which 
  action_done corresponds to which request? [Wang, Quanxian] right, 
  add wl_output parameter.
 
 No. If I do two mode set operations on the same wl_output, then again 
 I would not know which answer was for which. [Wang, Quanxian] I know 
 you mean. Yes, if the same client has more threads which send mode 
 change at the same time, we have to use unique number to stand for 
 every operation.

That unique number could be a serial, but here more appropriate is a unique 
number for each request. You can let Wayland do all the unique number 
management for you by using the feedback object design I referred to below. 
After all, a Wayland protocol object is essentially just a unique number. They 
are very cheap.

That is why you almost never need to manually fiddle with unique numbers in 
the protocol.

 Instead, a generic pattern for this kind of return data is to let 
 the original request also create a feedback protocol object. This 
 object is unique to the request that was sent, and can deliver any 
 return data without any ambiguity. An example of a feedback object is 
 wl_callback, except it can only deliver done, not yes/no; not 
 delivering anything will cause problems. [Wang, Quanxian] Good, 
 thanks.
 
  
  What if move succeeds but mode change fails? Wouldn't that leave the 
  output in an unwanted state which is neither the original nor the 
  wanted setting? [Wang, Quanxian] one by one. Not support complex. If 
  you have such case, have to turn back. Call another move back. But 
  firstly make sure the previous is successful.
 
 That will require a lot of roundtrips, and it essentially forces the 
 compositor to show all the intermediate steps on the monitors. IOW, 
 that is designed to be both slow and glitchy. That's not how you 
 should do dynamic mode setting. I think you are going to need atomic 
 group operations. [Wang, Quanxian] Yes, we could provide a request 
 with more parameters setting(group operations). Just like many 
 parameters in weston_output_switch_mode.

Except you would need to let it cover an arbitrary number of outputs in one go. 
That means that you will need something like what wl_surface.commit does. 
Having a request with a huge number of arguments is not only ugly but 
inflexible, and cannot be extended in the future.
[Wang, Quanxian] Agree, I have found that when using this. Basically I want to 
add transform parameter, at last I give up. because it is complex, also ugly, 
and not readable. :)

  The solution to this would tie in with the solution to take changes 
  atomic. For instance, to prepare for a configuration change, one 
  might create a change object in the protocol, store all changes in 
  that object, and then commit that set of changes atomically. Then 
  have one return value: the whole set either succeeds or fails.
  
  I guess you could look for inspiration in the DRM atomic mode 
  setting API. I don't know how the RandR X11 protocol works, if that 
  would be a good example also.

Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 0/6] Add weston randr protocol

[Wang, Quanxian]

-Original Message-
From: wayland-devel-boun...@lists.freedesktop.org 
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Hardening
Sent: Thursday, February 27, 2014 5:15 PM
To: wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH 0/6] Add weston randr protocol

Le 27/02/2014 04:27, Quanxian Wang a écrit :
 These patches will provide weston randr protocol, its implementation 
 and randr application.

 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 without any other operation.

 With this protocol, weston-wrandr application is developped to help 
 user implement randr protocol in command line just like xrandr command 
 in xserver.


Hi Quanxian,

I have submitted such functionality one year ago:
http://lists.freedesktop.org/archives/wayland-devel/2013-March/007872.html. 
The scope was smaller (targeting essentially mode resolution) but it was 
working.

At the time there were no real enthusiasm, some people were even against. 
Perhaps some have changed their mind since.
[Wang, Quanxian] Got it, Thanks. Ahaha. Seems we really need the access control 
mechanism to make it happen. After all, it should be a good tool from my 
viewpoint.

Regards.

--
David FORT
website: http://www.hardening-consulting.com/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH 0/6] Add weston randr protocol

[Wang, Quanxian]

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Thursday, February 27, 2014 11:29 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
Subject: Re: [PATCH 0/6] Add weston randr protocol

Quanxian,
I haven't looked through the code line-by-line, but I do have a few general 
comments.
First, please double-check your copyright blocks.  One of your files is 
copyrighted Benjamin Franzke and another is copyrighted Collabora.  I don't 
think that was intended.
[Wang, Quanxian] Aha, Sorry about that. I will update that. Some header are 
copied from others file. I will check that. Thanks for your reminder.
Second, it doesn't appear that you have any security mechanism for this?
I don't think we want arbitrary clients to be allowed to rotate, change mode, 
and change output layout.
[Wang, Quanxian] The interface's security is not controlled by protocol.  As a 
whole, I think if weston want to control that, it should have an independent 
security to control that or use the system control. For example, if you want to 
open the server display from client, we should add the permission denied for 
that.
While I agree that it would be great to have a tool (possibly even a 
command-line one) to do these things dynamically, we shouldn't make it a public 
interface that just anyone can bind to.  I'm not saying that some sort of randr 
interface isn't needed. I'm simply that it should be privileged and we need a 
well-defined mechanism for keeping rogue applications from messing up the 
compositor for everyone else.  For standard applications, I believe the 
intention is that any modesetting they do will be done in a very controlled 
manner through the shell.
[Wang, Quanxian] Is there any plan for wayland/weston developer to add the 
security plan to control access? If there is some plan or interface, I can add 
it in the patches. Currently wayland/weston don't provide a dynamic mode 
setting. It limited the dynamic action of applications above wayland/weston 
level. You know, we have to define it in weston config at the very beginning.
Do you know anyone or program provide security control in weston code?
The objective of this patches provides a way how to do dynamic mode setting. It 
is really missed in wayland/weston.
Thanks,
--Jason Ekstrand

On Wed, Feb 26, 2014 at 9:27 PM, Quanxian Wang 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
These patches will provide weston randr protocol,
its implementation and randr application.

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 without any other operation.

With this protocol, weston-wrandr application is developped to help
user implement randr protocol in command line just like xrandr command
in xserver.

For wayland customer, this application could *DYNAMICALLY*
do mode setting in command line.

For wayland application developer, weston randr protocol provide
a external interface to *DYNAMICALLY* do mode setting instead of
static configuring weston at the very beginning.

Weston protocol wrandr will provide interface to
1) set output mode
2) set output transform
3) move output to relative position
4) provide disconnected output information

Currently randr has more functions to be defined and implemented.
Some other functions are planned. But at first we need support.
If this protocol is accepted, we could have the chance moving on.

The advantage is
randr architecture have been defined at this commit. New function will
be very easy to be added in the protocol.

One note:
Currently wl_output doesn't provide wl_output_send_name function,
I have to implemented it in randr protocol. Actually it will be very
easily implemented in wl_output interface. If this commit series is
accepted, I will provide the patch to add wl_output_send_name to make
issue simply resolved instead of in randr protocol.
'

Here are some test cases.

1. weston-randr -q # query all output mode info and disconnected output

HDMI3
1)1440x900@60 (current)
2)1920x1200@60
3)1680x1050@60
...

VGA1
1)1280x1024@60 (current)
2)1152x864@60
3)1024x768@60
...

HDMI1 disconnected
HDMI2 disconnected
DP1 disconnected
DP2 disconnected
...

2. weston-randr --output HDMI3 # query HDMI3 output mode info

HDMI3
1)1440x900@60 (current)
2)1920x1200@60
3)1680x1050@60

3. weston-randr --output HDMI3 -m 2 # which will set mode as 1920x1200

4. weston-randr --output HDMI3 -R 1 # rotate HDMI3 output 90 degree

5. weston-randr --output HDMI3 -leftof VGA1 # put HDMI3 output leftof VGA1

6. weston-randr --output HDMI3 -rightof VGA1 # put HDMI3 output rightof VGA1

Quanxian Wang (6):
  Add weston randr protocol
  Add weston_randr definition and randr_backend intreface
  Add the detailed implementation of randr protocol
  Initialize the randr interface in drm backend
  Add weston-randr application
  Change the size

RE: [PATCH 0/6] Add weston randr protocol

[Wang, Quanxian]

Hi, All

From Jason's comment, about the security issue, I am not sure if I should 
think about that in this protocol. For communication protocol between client 
and server, it is hard to control the permission by single protocol. It is not 
the same as directly call process that we can easily control the user id/group 
permission. Actually I am a little confused by that. :( Sorry  about that.

If you have some good idea of security process, it will be helpful for me to 
make some changes.

Another,
from my experience on Tizen IVI, I don't find a way to do mode setting from 
client. It is really missed. Whatever for EFL or other application or other 
libraries which use wayland, dynamic mode setting should be one important 
matrix. But until now, no interface or tools provides that. (provided in 
wayland protocol or library?)

The patches provide a way or idea for you to think about that. Maybe I miss 
something, but I just show my think for that.

Whatever randr protocol or others protocol, at least it should provide a public 
interface for user to mode setting.

By the way, I am not messing up compositor. It is the place I found to put code 
there. At least it should be in display server level.

Thanks

Regards

Quanxian Wang

From: Jason Ekstrand [mailto:ja...@jlekstrand.net]
Sent: Thursday, February 27, 2014 11:29 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org; Zhang, Xiong Y
Subject: Re: [PATCH 0/6] Add weston randr protocol

Quanxian,
I haven't looked through the code line-by-line, but I do have a few general 
comments.
First, please double-check your copyright blocks.  One of your files is 
copyrighted Benjamin Franzke and another is copyrighted Collabora.  I don't 
think that was intended.
Second, it doesn't appear that you have any security mechanism for this?  I 
don't think we want arbitrary clients to be allowed to rotate, change mode, and 
change output layout.  While I agree that it would be great to have a tool 
(possibly even a command-line one) to do these things dynamically, we shouldn't 
make it a public interface that just anyone can bind to.  I'm not saying that 
some sort of randr interface isn't needed. I'm simply that it should be 
privileged and we need a well-defined mechanism for keeping rogue applications 
from messing up the compositor for everyone else.  For standard applications, I 
believe the intention is that any modesetting they do will be done in a very 
controlled manner through the shell.
Thanks,
--Jason Ekstrand

On Wed, Feb 26, 2014 at 9:27 PM, Quanxian Wang 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
These patches will provide weston randr protocol,
its implementation and randr application.

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 without any other operation.

With this protocol, weston-wrandr application is developped to help
user implement randr protocol in command line just like xrandr command
in xserver.

For wayland customer, this application could *DYNAMICALLY*
do mode setting in command line.

For wayland application developer, weston randr protocol provide
a external interface to *DYNAMICALLY* do mode setting instead of
static configuring weston at the very beginning.

Weston protocol wrandr will provide interface to
1) set output mode
2) set output transform
3) move output to relative position
4) provide disconnected output information

Currently randr has more functions to be defined and implemented.
Some other functions are planned. But at first we need support.
If this protocol is accepted, we could have the chance moving on.

The advantage is
randr architecture have been defined at this commit. New function will
be very easy to be added in the protocol.

One note:
Currently wl_output doesn't provide wl_output_send_name function,
I have to implemented it in randr protocol. Actually it will be very
easily implemented in wl_output interface. If this commit series is
accepted, I will provide the patch to add wl_output_send_name to make
issue simply resolved instead of in randr protocol.
'

Here are some test cases.

1. weston-randr -q # query all output mode info and disconnected output

HDMI3
1)1440x900@60 (current)
2)1920x1200@60
3)1680x1050@60
...

VGA1
1)1280x1024@60 (current)
2)1152x864@60
3)1024x768@60
...

HDMI1 disconnected
HDMI2 disconnected
DP1 disconnected
DP2 disconnected
...

2. weston-randr --output HDMI3 # query HDMI3 output mode info

HDMI3
1)1440x900@60 (current)
2)1920x1200@60
3)1680x1050@60

3. weston-randr --output HDMI3 -m 2 # which will set mode as 1920x1200

4. weston-randr --output HDMI3 -R 1 # rotate HDMI3 output 90 degree

5. weston-randr --output HDMI3 -leftof VGA1 # put HDMI3 output leftof VGA1

6. weston-randr --output HDMI3 -rightof VGA1 # put HDMI3 output rightof VGA1

Quanxian Wang (6):
  Add weston randr protocol

RE: [PATCH 0/6] Add weston randr protocol

[Wang, Quanxian]

Thanks Pq and Jason's comment.

Regards

Quanxian Wang

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com] 
Sent: Thursday, February 27, 2014 3:08 PM
To: Wang, Quanxian
Cc: Jason Ekstrand; Kristensen, Kristian H; Zhang, Xiong Y; 
wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH 0/6] Add weston randr protocol

On Thu, 27 Feb 2014 05:30:21 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Hi, All
 
 From Jason's comment, about the security issue, I am not sure if I should 
 think about that in this protocol. For communication protocol between client 
 and server, it is hard to control the permission by single protocol. It is 
 not the same as directly call process that we can easily control the user 
 id/group permission. Actually I am a little confused by that. :( Sorry  about 
 that.
 

Then just simply do not advertise the global interface for this on usual 
desktop compositors. Advertise it only on special compositors where you control 
what clients can run to begin with. That would be the first step, and perhaps 
go nicely with your use case at Tizen IVI.

 If you have some good idea of security process, it will be helpful for me to 
 make some changes.
 
 Another,
 from my experience on Tizen IVI, I don't find a way to do mode setting from 
 client. It is really missed.

This is completely deliberate. We do NOT want all clients that can connect to 
the server to be able to force the video mode at will.

 Whatever for EFL or other application or other libraries which use
 wayland, dynamic mode setting should be one important matrix. But  until now, 
no interface or tools provides that. (provided in wayland  protocol or library?)
 

We have a shell mechanism for cooperative dynamic, temporary video mode 
changes. See wl_shell_surface.set_fullscreen. That request allows video mode 
setting in a user-friendly way. By user-friendly I mean in a way, that e.g. a 
crashing game cannot leave your display messed up.
Also, a temporary video mode change will not change the size of your desktop, 
which means that e.g. if you have icons on the desktop, they won't get squashed 
in the top-left corner, and your windows will stay put. That is what I as a 
user would expect, when I run a program that changes the video mode for its 
running duration, like a game.

Yes, the use cases between a temporary change and the permanent change done by 
your proposal are very different. I just want to point out, that we already 
have *dynamic* video mode changing for applications.
[Wang, Quanxian] Control individual surface mode in full screen status. Make 
sense.

What you propose is essentially a system configuration  management interface. 
Therefore it should be privileged, to avoid abuse.

 The patches provide a way or idea for you to think about that. Maybe I miss 
 something, but I just show my think for that.
 
 Whatever randr protocol or others protocol, at least it should provide a 
 public interface for user to mode setting.

No, I disagree. It should not be public on desktop compositors.

 By the way, I am not messing up compositor. It is the place I found to put 
 code there. At least it should be in display server level.
 

You could make it a plugin, which you load only in your special cases, where 
clients need to be in control of the video mode explicitly. That would probably 
be enough of security until there is a real solution to granting access to 
privileged interfaces.

To recap: my only fundamental objection here is about who is allowed to access 
this interface, and maybe what its purpose is.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: Is there any way to let compositor set the data in client space and then return back to client?

[Wang, Quanxian]

Thanks pq. It is helpful for me. I will have a try with your idea. :)

-Original Message-
From: Pekka Paalanen [mailto:ppaala...@gmail.com] 
Sent: Saturday, February 08, 2014 5:56 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: Is there any way to let compositor set the data in client space 
and then return back to client?

On Sat, 8 Feb 2014 08:19:06 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 Hi, All
 
 I want to allocate some space in client, and let compositor set some 
 data in this space and then return back to client.
 
 It seems like user data mechanism.
 
 Any way to implement that?

Not really, it's nothing like user data.

You have to make the client bind to a new protocol interface you designed, and 
then use events to pass the data to the client.

If you need to pass significant amounts of data, you can use a mechanism 
similar to passing XKB keymaps, see wl_keyboard.keymap event. There the 
compositor creates the shm file. If you want the shm file created by the 
client, you pass the file descriptor in the other direction, similar to 
wl_shm.create_pool, have the server fill out the memory, and then send an event 
to say it's done or rely on a wl_display.sync roundtrip.


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: Is there any way to let compositor set the data in client space and then return back to client?

[Wang, Quanxian]

From: magc...@gmail.com [mailto:magc...@gmail.com] On Behalf Of Jasper St. 
Pierre
Sent: Sunday, February 09, 2014 12:23 AM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: Is there any way to let compositor set the data in client space 
and then return back to client?

Well, there's not really anything that allows process A to arbitrarily modify 
process B's memory. The only thing that's close is memory-mapped files, and 
that's already what we use to share image data between the client and the 
compositor.
There's also memfd, which is more secure than a memory-mapped tmpfile, simply 
because it allows sealing the fd, so you can be sure it will never change as 
you try to read from it. But memfd isn't a public API yet.

What exactly are you trying to do here?
[Wang, Quanxian] I want to create a protocol to get the output data from 
compositor, I don't know where to put it for client.  So shared memory should 
be a good way to do that. Anyway,  I need to check it how to make it happen. 
Thanks for your idea.

On Sat, Feb 8, 2014 at 3:19 AM, Wang, Quanxian 
quanxian.w...@intel.commailto:quanxian.w...@intel.com wrote:
Hi, All

I want to allocate some space in client, and let compositor set some data in 
this space and then return back to client.

It seems like user data mechanism.

Any way to implement that?

Thanks

Regards

Quanxian Wang


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel



--
  Jasper
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Is there any way to let compositor set the data in client space and then return back to client?

[Wang, Quanxian]

Hi, All

I want to allocate some space in client, and let compositor set some data in 
this space and then return back to client.

It seems like user data mechanism.

Any way to implement that?

Thanks

Regards

Quanxian Wang

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: how to run subsurface-test?

[Wang, Quanxian]

Thanks for your information.

-Original Message-
From: wayland-devel-bounces+quanxian.wang=intel@lists.freedesktop.org 
[mailto:wayland-devel-bounces+quanxian.wang=intel@lists.freedesktop.org] On 
Behalf Of Pekka Paalanen
Sent: Monday, June 24, 2013 6:01 PM
To: Wang, Quanxian
Cc: wayland-devel@lists.freedesktop.org
Subject: Re: how to run subsurface-test?

On Mon, 24 Jun 2013 09:43:54 +
Wang, Quanxian quanxian.w...@intel.com wrote:

 
 
 -Original Message-
 From:
 wayland-devel-bounces+quanxian.wang=intel@lists.freedesktop.org
 [mailto:wayland-devel-bounces+quanxian.wang=intel.com@lists.freedeskto
 p.org] On Behalf Of Pekka Paalanen Sent: Monday, June 24, 2013 5:31 PM 
 To:
 Wang, Quanxian Cc: wayland-devel@lists.freedesktop.org Subject: Re:
 how to run subsurface-test?
 
 On Mon, 24 Jun 2013 07:10:53 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Hi, Pq
  
  Would you like to tell me how to run subsurface-test?
  
  I have found the source code in weston/tests is not built out.
  After checking with configure in Weston, there is no switch to 
  enable how to build subsurface-test object.
  
  Any hint?
 
 Hi,
 
 when you build Weston, you should configure it with
 --with-cairo-glesv2 regardless of whether your Cairo actually has
 GLESv2 or not. Otherwise the new subsurfaces demo might not be built.
 It does not matter how your Cairo is built, so you do not need to 
 rebuild Cairo. [Wang, Quanxian] Thanks.
 
 For why, see:
 http://cgit.freedesktop.org/wayland/weston/commit/clients/subsurfaces.
 c?id=7ff7a80007fd27b2082e5219f4b4cb35948e3ed4
 
 In short: this avoids a GL vs. GLES clash.
 
 [Wang, Quanxian] By the way, I have checked the tests.xml for wayland 
 test (http://wayland.freedesktop.org/testing.html), and there says I 
 need to run 'make check' to run tests. Is there any way to run 
 subsurface-test separately. For example, I built subsurface-test 
 binary file, and put it into weston environment and run it. (However 
 it failed, because it need wl_test interface, desktop shell doesn't 
 have). Do you have some suggestion for that? Do I need to register 
 wl_test interface into the shell(it is a little crazy for user to do
 that) or there is another shell manage to run tests?

Oh you really meant the tests, I thought you meant the subsurfaces demo.

You can do this:
$ cd tests
$ make check TESTS=subsurface-test

Indeed, the tests are only built for 'make check'.


Cheers,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: How to create pixmap or pixmap surface in wayland?

[Wang, Quanxian]

Pq, thanks so much. It is very nice to get great suggestion.

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Friday, February 08, 2013 6:34 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: How to create pixmap or pixmap surface in wayland?
 
 On Fri, 8 Feb 2013 02:16:39 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Sorry Pq. I confused you so much.
 
  Just one requirement not related with architecture. You can imagine write a
 simple program for example glxgears. No compositor manager, no architecture,
 no window manager. Just has x server, or compositor server(weston).
 
  I want to call eglCreatePixmapSurface to create a pixmap surface. If the
 xserver is backend, I need to connect with X server and create a pixmap and
 then create the pixmap surface using egl interface provided by mesa.
  If the backend is weston compositor, I need to connect with weston
 compositor and using egl to create the pixmap and its surface. Before that the
 context is created. Just like the code in simple-egl.c of weston.
  ===
  -- function init_egl()
  ...
  display-egl.dpy = eglGetDisplay(display-display);
  assert(display-egl.dpy);
 
  ret = eglInitialize(display-egl.dpy, major, minor);
  assert(ret == EGL_TRUE);
  ret = eglBindAPI(EGL_OPENGL_ES_API);
  assert(ret == EGL_TRUE);
 
  ret = eglChooseConfig(display-egl.dpy, config_attribs,
display-egl.conf, 1, n);
  assert(ret  n == 1);
 
  display-egl.ctx = eglCreateContext(display-egl.dpy,
  display-egl.conf,
  EGL_NO_CONTEXT,
 context_attribs);
  assert(display-egl.ctx);
  ...
  ===
 
  The current issue is pixmap of wayland is not supported in wayland protocol
 and mesa. I could not do this like X.
 
  Sorry my title and description make you confused, I change it to 'how to
 create pixmap or pixmap surface in wayland'.
 
 Alright, thanks. Let's see.
 
 First you need an EGL display. I would recommend to ask EFL or whatever is
 creating the main Wayland connection for its struct wl_display pointer, and
 pass that to eglGetDisplay(). This way you are not creating a new Wayland
 connection, and your program will behave as a single Wayland client in the
 compositor's perspective. This will let you take advantage of sub-surfaces[1] 
 in
 the future, once we get them in a usable form.
 
 I also assume there is no way to create an EGLDisplay without any window
 system connection. At least on a standard Linux system, I think you need to be
 authenticated in DRM, so ignoring the window system would not work.
 
 Next you have to choose how get a render target. There are couple of
 options:
 A. use the surfaceless EGL extension, and use a FBO for the rendering B. 
 create
 a dummy wl_surface, use it as the EGLSurface, and use FBO for
real rendering
 C. create a wl_subsurface, and just use it for rendering
 
 In case A, depending on your EGL stack, you might not have the extension
 available. I think Mesa always has it, but others might not.
 
 Both A and B require you to use FBOs. That may or may not be a problem,
 depending on how and what kind of an API you offer to e.g. webGL. If the
 webGL code does glBindFramebuffer(GL_FRAMEBUFFER, 0), can you intercept
 it and change it to activate your FBO?
 
 The immediate downside of case C is that its not available just yet.
 However, it would have considerable benefits:
 - You get a real EGLSurface for rendering, no need for tricks with FBOs.
 - No need to read back the rendering in the client, i.e. no
   glReadPixels or copying into the browser window image at client
   side. The rendering will go directly to the compositor into the
   sub-surface, and the compositor will combine it with the rest of your
   browser graphics.
 - The sub-surface can use a different renderer than the main surface,
   i.e. the sub-surface can be draw with GL and main surface in
   software, or vice versa.
 - Even the compositor may avoid copying the sub-surface contents, as
   the sub-surface might be assigned to a hardware overlay, that will
   scan it out as is, without compositing.
 
 With cases A and B, that is with FBO, you will render into a temporary buffer,
 and then you probably want to use that rendered image for something, most
 likely as a part of a web page your browser is rendering. So you either use
 glReadPixels (very slow) and draw the web page in software, or you can use the
 FBO render target as a texture, and draw the browser window in GL. After that,
 you send the image to the compositor for display.
 
 As you see, the context and architecture information is quite essential to 
 get a
 useful reply. You don't only want to create an off-screen rendering target, 
 you
 will also want to use the result for something.
 How you want to use the result matters

RE: How to create pixmap or pixmap surface in wayland?

[Wang, Quanxian]

Sorry Pq. I confused you so much.

Just one requirement not related with architecture. You can imagine write a 
simple program for example glxgears. No compositor manager, no architecture, no 
window manager. Just has x server, or compositor server(weston).

I want to call eglCreatePixmapSurface to create a pixmap surface. If the 
xserver is backend, I need to connect with X server and create a pixmap and 
then create the pixmap surface using egl interface provided by mesa.
If the backend is weston compositor, I need to connect with weston compositor 
and using egl to create the pixmap and its surface. Before that the context is 
created. Just like the code in simple-egl.c of weston.
===
-- function init_egl()
...
display-egl.dpy = eglGetDisplay(display-display);
assert(display-egl.dpy);

ret = eglInitialize(display-egl.dpy, major, minor);
assert(ret == EGL_TRUE);
ret = eglBindAPI(EGL_OPENGL_ES_API);
assert(ret == EGL_TRUE);

ret = eglChooseConfig(display-egl.dpy, config_attribs,
  display-egl.conf, 1, n);
assert(ret  n == 1); 

display-egl.ctx = eglCreateContext(display-egl.dpy,
display-egl.conf,
EGL_NO_CONTEXT, context_attribs);
assert(display-egl.ctx);
...
===

The current issue is pixmap of wayland is not supported in wayland protocol and 
mesa. I could not do this like X. 

Sorry my title and description make you confused, I change it to 'how to create 
pixmap or pixmap surface in wayland'.

Thanks

Regards

Quanxian Wang

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Thursday, February 07, 2013 6:05 PM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: How to enable Wayland when original software support X?
 
 On Thu, 7 Feb 2013 03:30:48 +
 Wang, Quanxian quanxian.w...@intel.com wrote:
 
  Thanks pq.
 
   -Original Message-
   From: Pekka Paalanen [mailto:ppaala...@gmail.com]
   Sent: Wednesday, February 06, 2013 7:34 PM
   To: Wang, Quanxian
   Subject: Re: How to enable Wayland when original software support X?
  
 I don't really see the replace X operations with Wayland
 operations as a feasible approach, in the way you have it
 below. I think it will become a nightmare to develop and
 maintain. The separation to
   X vs.
 Wayland paths should be done on a higher level, because usually
 there is no one-to-one correspondence at libwayland/xlib
 function level. At least you should use functions in different files 
 instead

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: How to enable Wayland when original software support X?

[Wang, Quanxian]

Thanks pq.

 -Original Message-
 From: Pekka Paalanen [mailto:ppaala...@gmail.com]
 Sent: Wednesday, February 06, 2013 7:34 PM
 To: Wang, Quanxian
 Subject: Re: How to enable Wayland when original software support X?
 
   I don't really see the replace X operations with Wayland
   operations as a feasible approach, in the way you have it below. I
   think it will become a nightmare to develop and maintain. The separation 
   to
 X vs.
   Wayland paths should be done on a higher level, because usually
   there is no one-to-one correspondence at libwayland/xlib function
   level. At least you should use functions in different files instead of 
   #ifdefs.
  
I think more and more developers will come across such issue when
they
   want to support Wayland.
   
Here are some code translation.
   
First part: open Display
   
+#if ENABLE(TIZEN_WAYLAND)
+g_nativeDisplay = wl_display_connect(NULL);
  
   Are you creating a new Wayland connection just for the WebGL widget,
   while the rest of the application is already using another connection?
  [Wang, Quanxian] Yes. Other apps will use another connection. If they start
 another web page for that.
   That simply won't work, as you cannot integrate display elements
   from different clients. Even if the originating process is the same,
   a new connection is seen as a new client in the compositor, and so they 
   are
 isolated.
  [Wang, Quanxian] They should be isolated. I just provide the surface for 
  user
 to do rendering. When client finished rendering, compositor will take the work
 and composite them. I will not care about the rest since app is just the 
 client.
 
 Okay, now I'm completely lost. It sounds like you are writing a compositor. I
 thought you were talking about one application, which is one Wayland client,
 and consists of webkit-efl with webGL capabilities.
[Wang, Quanxian] no, you are not lost. Low level compositor is weston. Window 
is created as a dummy to provide off-screen surface. The difference is the 
rendered surface will be located in web browser as a element in web page.
Webkit-efl is just a middleware which provide the interface for user to 
communicate with low level including libraries or device. For example, apps 
create egl surface through webkit-efl.
 
 What I was talking about is that you cannot have one connection for webkit,
 another connection for webGL implementation, and assume that you can stitch
 together a web page with a webGL element in the middle. That won't work
 with a generic Wayland compositor.
 
 Is your web browser actually an intermediate compositor?
[Wang, Quanxian] I think so.
 
 Like this:
 
 a Wayland compositor - the browser as client,
[Wang, Quanxian] yes.
 
 the browser as compositor - multiple processes, one per webGL instance or
 browser tab or whatever
 
 It seems I didn't understand your overall architecture. If it is something 
 else
 than browser+webGL+everything as a single Wayland client in one process, you
 should definitely explain that on the mailing list.
[Wang, Quanxian] Generally architecture is browser(webapi)+webkit(including 
webgl)+EFL(Enlightenment Foundation 
Libraries)+compositor(weston/enlightment-X)+kernel
 
 Or do you mean that your webGL thing will never display anything else than the
 webGL canvas: no other HTML elements, no window decorations?
 Do you even have any floating windows in your target system?
[Wang, Quanxian] All above are included.
 
 
 Thanks,
 pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

How to enable Wayland when original software support X?

[Wang, Quanxian]

Hi, All

I like to get some comment from you.

I am planning to enable wayland webgl support in webkit-efl. Basically they 
support X. With the update, I will not change the original mechanism and just 
translate X operation to wayland operation.

I think more and more developers will come across such issue when they want to 
support Wayland.

Here are some code translation.

First part: open Display

+#if ENABLE(TIZEN_WAYLAND)
+g_nativeDisplay = wl_display_connect(NULL);
+assert(g_nativeDisplay);
+struct wl_registry *registry = 
wl_display_get_registry(g_nativeDisplay);
+wl_registry_add_listener(registry, NULL, NULL);
+wl_display_dispatch(g_nativeDisplay);
+m_window = (void *)wl_egl_window_create(NULL, 1, 1);
+#else
+g_nativeDisplay = XOpenDisplay(0);
+g_nativeWindow = XCreateSimpleWindow(g_nativeDisplay, 
XDefaultRootWindow(g_nativeDisplay),
 0, 0, 1, 1, 0,
 BlackPixel(g_nativeDisplay, 0), 
WhitePixel(g_nativeDisplay, 0));
+XFlush(g_nativeDisplay);
+#endif

Second part: create surface

+#if ENABLE(TIZEN_WAYLAND)
+m_window = (void *)wl_egl_window_create(NULL, 1, 1);
+m_surface = eglCreateWindowSurface(m_display, surfaceConfig, (struct 
wl_egl_window *)m_window, NULL);
+#else
 m_pixmapID = XCreatePixmap(g_nativeDisplay, g_nativeWindow, 1, 1, 32);
 m_surface = eglCreatePixmapSurface(m_display, surfaceConfig, m_pixmapID, 
NULL);
+#endif

Wayland has no pixmap and wl_egl_pixmap has been moved out from mesa and 
wayland. Therefore I have to use window to replace them.

Currently with testing, it is fine. However I am afraid I miss something 
unexpected. Need Wayland expert like you to have a review of them.

Any suggestion for that?

Thanks

Regards

Quanxian Wang


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] compositor: fix overflow issue when calculate msecs of vblank

[Wang, Quanxian]

 -Original Message-
 From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
 Sent: Thursday, September 06, 2012 10:31 AM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org; Guo, Chaohong; Bradford, Robert
 Subject: Re: [PATCH] compositor: fix overflow issue when calculate msecs of
 vblank
 
 On Wed, Sep 5, 2012 at 9:28 PM, Wang, Quanxian quanxian.w...@intel.com
 wrote:
 
 
  -Original Message-
  From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
  Sent: Thursday, September 06, 2012 3:28 AM
  To: Wang, Quanxian
  Cc: wayland-devel@lists.freedesktop.org; Guo, Chaohong; Bradford,
  Robert
  Subject: Re: [PATCH] compositor: fix overflow issue when calculate
  msecs of vblank
 
  On Wed, Sep 05, 2012 at 03:35:32AM +, Wang, Quanxian wrote:
   From Quanxian Wang
   quanxian.w...@intel.commailto:quanxian.w...@intel.com
  
   commit f7b156cef1ed8081df6f25cc0e66c8d7fbf414fc
   Author: Wang Quanxian quanxian.w...@intel.com
   Date:   Wed Sep 5 11:30:38 2012 +0800
  
   Change int to long to avoid the overflow when calculation
   vblank msecs
  
   secs and nsecs are from vblank event, the number of secs is
   very
  big(134xxx),
   when turns secs to msecs(multiple 1000), it will exceed uint32
   max value.
  
   Signed-Off-By Quanxian Wang quanxian.w...@intel.com
 
  No, the overflow is itentional.  The timestamp is just a milisecond
  value with an unspecified epoch, only differences between timestamps
  are useful.  It may overflow and that's expected.  If you use
  uint32_t math, you can subtract timestamps before and after an
  overflow and still get the number of miliseconds elapsed.
  [Wang, Quanxian] I agree your point partly. My different point is for 
  example,
 take 100 as the limit, you have 101 and 99 number, the diff is 2. To 101, 
 after
 conversion, the number is 01 and to 99 is 99. The diff turns to be 98 and
 compared with previous 2 it turns to bigger. In the limit, it is the safe, 
 cross the
 border is not safe.
 
 No unsigned integer math in C is defined so that this actually works.
 Assume four bit integers (that is, our range is 0-15).  Suppose you have
 timestamps 14 and 17.  The difference is 17 - 14 = 3.  But with 4 bit unsigned
 integers, the values are truncated to 14 and 1.  However,
 1 - 14 still gives the right result.
[Wang, Quanxian] yes. It is minus. I see. Thanks
 
 Kristian
 
 
  Kristian
 
   diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
   df81aba..8b6285c 100644
   --- a/src/compositor-drm.c
   +++ b/src/compositor-drm.c
   @@ -447,7 +447,7 @@ vblank_handler(int fd, unsigned int frame,
   unsigned
  int sec, unsigned int usec,
   struct drm_sprite *s = (struct drm_sprite *)data;
   struct drm_compositor *c = s-compositor;
   struct drm_output *output = s-output;
   -   uint32_t msecs;
   +   uint64_t msecs = 0;
  
   output-vblank_pending = 0;
  
   @@ -480,7 +480,7 @@ page_flip_handler(int fd, unsigned int frame,
 unsigned int sec, unsigned int usec, void *data)  {
   struct drm_output *output = (struct drm_output *) data;
   -   uint32_t msecs;
   +   uint64_t msecs=0;
  
   output-page_flip_pending = 0;
  
   @@ -496,7 +496,7 @@ page_flip_handler(int fd, unsigned int frame,
   output-next = NULL;
  
   if (!output-vblank_pending) {
   -   msecs = sec * 1000 + usec / 1000;
   +   msecs = (uint64_t)sec * 1000 + (uint64_t)usec /
   + 1000;
   weston_output_finish_frame(output-base, msecs);
   }
}
   diff --git a/src/compositor.c b/src/compositor.c index
   f4263ac..37c52bc 100644
   --- a/src/compositor.c
   +++ b/src/compositor.c
   @@ -1026,7 +1026,7 @@ weston_output_damage(struct weston_output
   *output)
  
static void
fade_frame(struct weston_animation *animation,
   -  struct weston_output *output, uint32_t msecs)
   +  struct weston_output *output, uint64_t msecs)
{
   struct weston_compositor *compositor =
   container_of(animation, @@ -1129,7 +1129,7 @@
   surface_accumulate_damage(struct weston_surface *surface,  }
  
static void
   -weston_output_repaint(struct weston_output *output, uint32_t
   msecs)
   +weston_output_repaint(struct weston_output *output, uint64_t
   +msecs)
{
   struct weston_compositor *ec = output-compositor;
   struct weston_surface *es;
   @@ -1222,7 +1222,7 @@ weston_compositor_read_input(int fd,
 uint32_t
   mask, void *data)  }
  
WL_EXPORT void
   -weston_output_finish_frame(struct weston_output *output, uint32_t
   msecs)
   +weston_output_finish_frame(struct weston_output *output, uint64_t
   +msecs)
{
   struct weston_compositor *compositor = output-compositor;
   struct wl_event_loop *loop = diff --git a/src/compositor.h
   b/src/compositor.h index
   7a8058e..f49da84 100644
   --- a/src/compositor.h
   +++ b/src

[PATCH] compositor: fix overflow issue when calculate msecs of vblank

[Wang, Quanxian]

From Quanxian Wang quanxian.w...@intel.commailto:quanxian.w...@intel.com

commit f7b156cef1ed8081df6f25cc0e66c8d7fbf414fc
Author: Wang Quanxian quanxian.w...@intel.com
Date:   Wed Sep 5 11:30:38 2012 +0800

Change int to long to avoid the overflow when calculation vblank msecs

secs and nsecs are from vblank event, the number of secs is very 
big(134xxx),
when turns secs to msecs(multiple 1000), it will exceed uint32
max value.

Signed-Off-By Quanxian Wang quanxian.w...@intel.com

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index df81aba..8b6285c 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -447,7 +447,7 @@ vblank_handler(int fd, unsigned int frame, unsigned int 
sec, unsigned int usec,
struct drm_sprite *s = (struct drm_sprite *)data;
struct drm_compositor *c = s-compositor;
struct drm_output *output = s-output;
-   uint32_t msecs;
+   uint64_t msecs = 0;

output-vblank_pending = 0;

@@ -480,7 +480,7 @@ page_flip_handler(int fd, unsigned int frame,
  unsigned int sec, unsigned int usec, void *data)
 {
struct drm_output *output = (struct drm_output *) data;
-   uint32_t msecs;
+   uint64_t msecs=0;

output-page_flip_pending = 0;

@@ -496,7 +496,7 @@ page_flip_handler(int fd, unsigned int frame,
output-next = NULL;

if (!output-vblank_pending) {
-   msecs = sec * 1000 + usec / 1000;
+   msecs = (uint64_t)sec * 1000 + (uint64_t)usec / 1000;
weston_output_finish_frame(output-base, msecs);
}
 }
diff --git a/src/compositor.c b/src/compositor.c
index f4263ac..37c52bc 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -1026,7 +1026,7 @@ weston_output_damage(struct weston_output *output)

 static void
 fade_frame(struct weston_animation *animation,
-  struct weston_output *output, uint32_t msecs)
+  struct weston_output *output, uint64_t msecs)
 {
struct weston_compositor *compositor =
container_of(animation,
@@ -1129,7 +1129,7 @@ surface_accumulate_damage(struct weston_surface *surface,
 }

 static void
-weston_output_repaint(struct weston_output *output, uint32_t msecs)
+weston_output_repaint(struct weston_output *output, uint64_t msecs)
 {
struct weston_compositor *ec = output-compositor;
struct weston_surface *es;
@@ -1222,7 +1222,7 @@ weston_compositor_read_input(int fd, uint32_t mask, void 
*data)
 }

 WL_EXPORT void
-weston_output_finish_frame(struct weston_output *output, uint32_t msecs)
+weston_output_finish_frame(struct weston_output *output, uint64_t msecs)
 {
struct weston_compositor *compositor = output-compositor;
struct wl_event_loop *loop =
diff --git a/src/compositor.h b/src/compositor.h
index 7a8058e..f49da84 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -109,7 +109,7 @@ struct weston_spring {
double current;
double target;
double previous;
-   uint32_t timestamp;
+   uint64_t timestamp;
 };

 enum {
@@ -164,7 +164,7 @@ struct weston_output {
struct weston_output_zoom zoom;
int dirty;
struct wl_signal frame_signal;
-   uint32_t frame_time;
+   uint64_t frame_time;
int disable_planes;

char *make, *model;
@@ -493,7 +493,7 @@ void
 weston_spring_init(struct weston_spring *spring,
   double k, double current, double target);
 void
-weston_spring_update(struct weston_spring *spring, uint32_t msec);
+weston_spring_update(struct weston_spring *spring, uint64_t msec);
 int
 weston_spring_done(struct weston_spring *spring);

@@ -543,7 +543,7 @@ void
 weston_plane_release(struct weston_plane *plane);

 void
-weston_output_finish_frame(struct weston_output *output, uint32_t msecs);
+weston_output_finish_frame(struct weston_output *output, uint64_t msecs);
 void
 weston_output_schedule_repaint(struct weston_output *output);
 void
diff --git a/src/util.c b/src/util.c
index 4ff451a..34b140e 100644
--- a/src/util.c
+++ b/src/util.c
@@ -42,7 +42,7 @@ weston_spring_init(struct weston_spring *spring,
 }

 WL_EXPORT void
-weston_spring_update(struct weston_spring *spring, uint32_t msec)
+weston_spring_update(struct weston_spring *spring, uint64_t msec)
 {
double force, v, current, step;


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Do not set dpms to standby which cause system could not be resotred when system is locked at the second time.

[Wang, Quanxian]

This patch is invalid. It is special platform issue.

sorry

From: wayland-devel-bounces+quanxian.wang=intel@lists.freedesktop.org 
[mailto:wayland-devel-bounces+quanxian.wang=intel@lists.freedesktop.org] On 
Behalf Of Wang, Quanxian
Sent: Friday, August 24, 2012 7:00 PM
To: wayland-devel@lists.freedesktop.org
Subject: RE: [PATCH] Do not set dpms to standby which cause system could not be 
resotred when system is locked at the second time.

Could not resotred the system. Sorry

_
From: Wang, Quanxian
Sent: Friday, August 24, 2012 6:57 PM
To: 
wayland-devel@lists.freedesktop.orgmailto:wayland-devel@lists.freedesktop.org
Cc: Wang, Quanxian
Subject: [PATCH] Do not set dpms to standby which cause system could be 
resotred when system is locked at the second time.


Reproduce this:
1.   Weston -i1 and after 1 second, the system go into idle and wait a 
little while, go to lock status
2.   Press key and the screen is fine
3.   After 1 second, system go to idle and wait a little while, go to lock 
status again
4.   Whatever you press any key or mouse, you could not restore the system.

From Quanxian Wang quanxian.w...@intel.commailto:quanxian.w...@intel.com

Do not set dpms to standby which cause system could be restored.

diff --git a/src/shell.c b/src/shell.c
index 4d6bc4f..1df571b 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -2470,10 +2470,6 @@ lock(struct wl_listener *listener, void *data)
struct workspace *ws = get_current_workspace(shell);

if (shell-locked) {
-   wl_list_for_each(output, shell-compositor-output_list, link)
-   /* TODO: find a way to jump to other DPMS levels */
-   if (output-set_dpms)
-   output-set_dpms(output, WESTON_DPMS_STANDBY);
return;
}

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] Do not set dpms to standby which cause system could be resotred when system is locked at the second time.

[Wang, Quanxian]

Reproduce this:
1)  Weston -i1 and after 1 second, the system go into idle and wait a 
little while, go to lock status
2)  Press key and the screen is fine
3)  After 1 second, system go to idle and wait a little while, go to lock 
status again
4)  Whatever you press any key or mouse, you could restore the system.

From Quanxian Wang quanxian.w...@intel.com

Do not set dpms to standby which cause system could be restored.

diff --git a/src/shell.c b/src/shell.c
index 4d6bc4f..1df571b 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -2470,10 +2470,6 @@ lock(struct wl_listener *listener, void *data)
struct workspace *ws = get_current_workspace(shell);

if (shell-locked) {
-   wl_list_for_each(output, shell-compositor-output_list, link)
-   /* TODO: find a way to jump to other DPMS levels */
-   if (output-set_dpms)
-   output-set_dpms(output, WESTON_DPMS_STANDBY);
return;
}

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH] Do not set dpms to standby which cause system could not be resotred when system is locked at the second time.

[Wang, Quanxian]

Could not resotred the system. Sorry

   _
   From: Wang, Quanxian
   Sent: Friday, August 24, 2012 6:57 PM
   To: wayland-devel@lists.freedesktop.org
   Cc: Wang, Quanxian
   Subject: [PATCH] Do not set dpms to standby which cause system could be 
resotred when system is locked at the second time.


   Reproduce this:
1)  Weston -i1 and after 1 second, the system go into idle and wait a 
little while, go to lock status
2)  Press key and the screen is fine
3)  After 1 second, system go to idle and wait a little while, go to lock 
status again
4)  Whatever you press any key or mouse, you could not restore the system.

   From Quanxian Wang quanxian.w...@intel.commailto:quanxian.w...@intel.com

   Do not set dpms to standby which cause system could be restored.

   diff --git a/src/shell.c b/src/shell.c
   index 4d6bc4f..1df571b 100644
   --- a/src/shell.c
   +++ b/src/shell.c
   @@ -2470,10 +2470,6 @@ lock(struct wl_listener *listener, void *data)
   struct workspace *ws = get_current_workspace(shell);

   if (shell-locked) {
   -   wl_list_for_each(output, shell-compositor-output_list, 
link)
   -   /* TODO: find a way to jump to other DPMS levels */
   -   if (output-set_dpms)
   -   output-set_dpms(output, 
WESTON_DPMS_STANDBY);
   return;
   }

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: mode issue about no EDID and first mode when weston start

[Wang, Quanxian]

Got it.

Thanks

Quanxian Wang

 -Original Message-
 From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
 Sent: Wednesday, August 01, 2012 7:52 AM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: mode issue about no EDID and first mode when weston start
 
 On Tue, Jul 31, 2012 at 01:46:17AM +, Wang, Quanxian wrote:
 
  Ok, if we don't use builtin mode in weston. That is fine. In this
  case, I supposed to provide some useful error output to user instead
  of core dump weston.
 
 Yep, we should handle that better now.
 
  Another case is if we don't get active mode from current connector,
  and at the same time EDID mode information from KMS is available. In
  this case, I use the first mode we get from KMS as the default mode
  for connector. If not, we will still core dump weston.
 
 The EDID information should have a preferred mode.  I guess we can add a
 final fallback to pick the first mode if there is no preferred or current 
 mode.
 Also, there's a patch on the list now from Scott that lets you add a modeline 
 in
 the weston.ini config file.
 
 Kristian
 
   -Original Message-
   From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
   Sent: Tuesday, July 31, 2012 5:15 AM
   To: Wang, Quanxian
   Cc: wayland-devel@lists.freedesktop.org
   Subject: Re: mode issue about no EDID and first mode when weston
   start
  
   On Thu, Jul 26, 2012 at 07:54:45PM +, Wang, Quanxian wrote:
Hi, All
   
Help review.
   
Background
   
1) Some platforms has no EDID mode information. Therefore when KMS
initializes the driver, there will be no mode provided for user.
At that time, have to use the fixed mode defined by the
user.(builtin-800x480)
  
   What you want to do in this case is to either compile an EDID blob
   into the kernel or specify one through the firmware mechanism.  See
   Documentation/EDID/HOWTO.txt in the kernel and use
  
 video=HDMI-A-1:e drm_kms_helper.edid_firmware=HDMI-A-1:edid
  
   on the kernel command line.  This example assumes you're trying to
   bring up the HDMI-A-1 connector with an edid blob in
   /lib/firmware/edid.  That will bring up KMS early on and weston will
   be able to reuse the mode set from the custom EDID blob.  We will
   also add a feature to specify a modeline in weston.ini, so you'll be able 
   to
 use that instead.  Something like:
  
   [output]
   name=HDMI-A-1
   modeline=29.50  800 824 896 992  480 483 493 500 -hsync +vsync
  
   I don't want to build in a mode in weston.  KMS should give us a
   valid mode in all cases and if it doesn't it's probably something
   like this case where you have a custom board/driver/panel.
  
   Kristian
  
2) In the very beginning of weston restart, there exists a case,
no active mode is set to connector. We provide the first mode
gotten from KMS as default mode for connector.
   
We got weston core dump when weston start because drm will add
null mode when cases above happen.
   
commit 1a87302f288497ed8bbef3677286e27bb6931f72
Author: Wang Quanxian
   quanxian.w...@intel.commailto:quanxian.w...@intel.com
Date:   Wed Jul 25 11:21:00 2012 +0800
   
Bug fix for mode issue when weston start
   
Provide the default builtin(800x480) mode when platforms
have no EDID and KMS could not provide mode information.
In the very beginning of weston, the connector could not
be set active mode. At that time, take the first mode in
mode list from KMS as default mode.
   
Signed-Off-By Quanxian Wang
quanxian.w...@intel.commailto:quanxian.w...@intel.com
   
diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
4dffa1d..47cd512 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -143,6 +143,16 @@ struct drm_sprite {
uint32_t formats[];
 };
   
+static drmModeModeInfo builtin_800x480 = {
+   33750,  /* clock */
+   800, 864, 976, 1088, 0,
+   480, 486, 494, 517, 0,
+   59920,
+   DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
+   0,
+   800x480
+};
+
 static int
 surface_is_primary(struct weston_compositor *ec, struct
weston_surface *es)  { @@ -1344,13 +1354,30 @@
create_output_for_connector(struct drm_compositor *ec,
drmModeFreeEncoder(encoder);
if (crtc == NULL)
goto err_free;
-   crtc_mode = crtc-mode;
+
+   /* if don't get mode from drm driver, use default 800x480 */
+   if (crtc-mode.clock != 0)
+   {
+   crtc_mode = crtc-mode;
+   } else {
+   if (connector-count_modes == 0)
+   crtc_mode = builtin_800x480;
+   else
+   crtc_mode = connector-modes[0];
+   }
+
drmModeFreeCrtc(crtc);
   
-   for (i = 0; i

RE: mode issue about no EDID and first mode when weston start

[Wang, Quanxian]

Ok, if we don't use builtin mode in weston. That is fine. In this case, I 
supposed to provide some useful error output to user instead of core dump 
weston.

Another case is if we don't get active mode from current connector, and at the 
same time EDID mode information from KMS is available. In this case, I use the 
first mode we get from KMS as the default mode for connector. If not, we will 
still core dump weston.

 -Original Message-
 From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
 Sent: Tuesday, July 31, 2012 5:15 AM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: mode issue about no EDID and first mode when weston start
 
 On Thu, Jul 26, 2012 at 07:54:45PM +, Wang, Quanxian wrote:
  Hi, All
 
  Help review.
 
  Background
 
  1) Some platforms has no EDID mode information. Therefore when KMS
  initializes the driver, there will be no mode provided for user. At
  that time, have to use the fixed mode defined by the
  user.(builtin-800x480)
 
 What you want to do in this case is to either compile an EDID blob into the
 kernel or specify one through the firmware mechanism.  See
 Documentation/EDID/HOWTO.txt in the kernel and use
 
   video=HDMI-A-1:e drm_kms_helper.edid_firmware=HDMI-A-1:edid
 
 on the kernel command line.  This example assumes you're trying to bring up
 the HDMI-A-1 connector with an edid blob in /lib/firmware/edid.  That will
 bring up KMS early on and weston will be able to reuse the mode set from the
 custom EDID blob.  We will also add a feature to specify a modeline in
 weston.ini, so you'll be able to use that instead.  Something like:
 
 [output]
 name=HDMI-A-1
 modeline=29.50  800 824 896 992  480 483 493 500 -hsync +vsync
 
 I don't want to build in a mode in weston.  KMS should give us a valid mode in
 all cases and if it doesn't it's probably something like this case where you 
 have
 a custom board/driver/panel.
 
 Kristian
 
  2) In the very beginning of weston restart, there exists a case, no
  active mode is set to connector. We provide the first mode gotten from
  KMS as default mode for connector.
 
  We got weston core dump when weston start because drm will add null
  mode when cases above happen.
 
  commit 1a87302f288497ed8bbef3677286e27bb6931f72
  Author: Wang Quanxian
 quanxian.w...@intel.commailto:quanxian.w...@intel.com
  Date:   Wed Jul 25 11:21:00 2012 +0800
 
  Bug fix for mode issue when weston start
 
  Provide the default builtin(800x480) mode when platforms
  have no EDID and KMS could not provide mode information.
  In the very beginning of weston, the connector could not
  be set active mode. At that time, take the first mode in
  mode list from KMS as default mode.
 
  Signed-Off-By Quanxian Wang
  quanxian.w...@intel.commailto:quanxian.w...@intel.com
 
  diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
  4dffa1d..47cd512 100644
  --- a/src/compositor-drm.c
  +++ b/src/compositor-drm.c
  @@ -143,6 +143,16 @@ struct drm_sprite {
  uint32_t formats[];
   };
 
  +static drmModeModeInfo builtin_800x480 = {
  +   33750,  /* clock */
  +   800, 864, 976, 1088, 0,
  +   480, 486, 494, 517, 0,
  +   59920,
  +   DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
  +   0,
  +   800x480
  +};
  +
   static int
   surface_is_primary(struct weston_compositor *ec, struct
  weston_surface *es)  { @@ -1344,13 +1354,30 @@
  create_output_for_connector(struct drm_compositor *ec,
  drmModeFreeEncoder(encoder);
  if (crtc == NULL)
  goto err_free;
  -   crtc_mode = crtc-mode;
  +
  +   /* if don't get mode from drm driver, use default 800x480 */
  +   if (crtc-mode.clock != 0)
  +   {
  +   crtc_mode = crtc-mode;
  +   } else {
  +   if (connector-count_modes == 0)
  +   crtc_mode = builtin_800x480;
  +   else
  +   crtc_mode = connector-modes[0];
  +   }
  +
  drmModeFreeCrtc(crtc);
 
  -   for (i = 0; i  connector-count_modes; i++) {
  -   ret = drm_output_add_mode(output,
 connector-modes[i]);
  +   if (connector-count_modes == 0) {
  +   ret = drm_output_add_mode(output, crtc_mode);
  if (ret)
  goto err_free;
  +   }else{
  +   for (i = 0; i  connector-count_modes; i++) {
  +   ret = drm_output_add_mode(output,
 connector-modes[i]);
  +   if (ret)
  +   goto err_free;
  +   }
  }
 
  preferred = NULL;
 
 
  Thanks
 
  Quanxian Wang
 
 
 
 
  ___
  wayland-devel mailing list
  wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel

RE: bug fix inactive encoder when weston restart

[Wang, Quanxian]

Thanks for your review and your changes.

 -Original Message-
 From: Kristian Høgsberg [mailto:hoegsb...@gmail.com]
 Sent: Tuesday, July 31, 2012 6:15 AM
 To: Wang, Quanxian
 Cc: wayland-devel@lists.freedesktop.org
 Subject: Re: bug fix inactive encoder when weston restart
 
 On Thu, Jul 26, 2012 at 08:03:26PM +, Wang, Quanxian wrote:
  Hi, All
 
  Please help review the patch.
 
  Thanks
 
  Background:
  If we use 'pkill -9 weston', when restart weston, weston could not get the
 active encoder attached on connector and exit abnormally.
  This is because when weston is pkilled, drm cleanups the encoders attached
 to active connector in KMS.
  This patch will ignore the active encode checking, weston has the ability to
 re-initialize encoder and reattached it into connector.
 
 Good catch.  We don't handle connectors that aren't attached to a crtc, which
 happens in your case above, but also just when hotplugging a monitor.  I
 applied your patch with a few changes, mostly to deal with how we don't have
 the built-in mode.  We now just exit if we don't find any modes to the output.
 
 Kristian
 
  commit 7f991195c20d9dd57e14faf88cd36db0c77df424
  Author: Wang Quanxian quanxian.w...@intel.com
  Date:   Thu Jul 19 19:11:04 2012 +0800
  User sends SIGKILL to weston, KMS will clean up encoder
  attached with connector. When weston restarts again, it causes
  exit of weston because no active encoder is attached to connector.
  This patch will ignore the checking and weston will initialize
  encoder again and re-attach it to connector in the following process.
 
  Sign-Off-By Quanxian Wang quanxian.w...@intel.com
 
  diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
  47cd512..bb1b138 100644
  --- a/src/compositor-drm.c
  +++ b/src/compositor-drm.c
  @@ -1348,26 +1348,28 @@ create_output_for_connector(struct
 drm_compositor *ec,
  /* Get the current mode on the crtc that's currently driving
   * this connector. */
  encoder = drmModeGetEncoder(ec-drm.fd,
 connector-encoder_id);
  -   if (encoder == NULL)
  -   goto err_free;
  -   crtc = drmModeGetCrtc(ec-drm.fd, encoder-crtc_id);
  -   drmModeFreeEncoder(encoder);
  -   if (crtc == NULL)
  -   goto err_free;
  -
  -   /* if don't get mode from drm driver, use default 800x480 */
  -if (crtc-mode.clock != 0)
  -   {
  +   if (encoder != NULL)
  +{
  +   crtc = drmModeGetCrtc(ec-drm.fd, encoder-crtc_id);
  +   drmModeFreeEncoder(encoder);
  +   if (crtc == NULL)
  +   goto err_free;
  crtc_mode = crtc-mode;
  -   } else {
  +   drmModeFreeCrtc(crtc);
  +   }
  +   /*
  +* No actiave encoder is connected with the connector
  +* If connector has no mode available, takes detault mode
  +* Or takes the first mode from connector
  +*/
  +   if (encoder == NULL || crtc_mode.clock == 0)
  +{
  if (connector-count_modes == 0)
  crtc_mode = builtin_800x480;
  else
  crtc_mode = connector-modes[0];
  }
 
  -   drmModeFreeCrtc(crtc);
  -
  if (connector-count_modes == 0) {
  ret = drm_output_add_mode(output, crtc_mode);
  if (ret)
 
 
  Thanks
 
  Quanxian Wang
 
 
 
 
  ___
  wayland-devel mailing list
  wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

mode issue about no EDID and first mode when weston start

[Wang, Quanxian]

Hi, All

Help review.

Background
1)  Some platforms has no EDID mode information. Therefore when KMS 
initializes the driver, there will be no mode provided for user. At that time, 
have to use the fixed mode defined by the user.(builtin-800x480)
2)  In the very beginning of weston restart, there exists a case, no active 
mode is set to connector. We provide the first mode gotten from KMS as default 
mode for connector.

We got weston core dump when weston start because drm will add null mode when 
cases above happen.

commit 1a87302f288497ed8bbef3677286e27bb6931f72
Author: Wang Quanxian quanxian.w...@intel.commailto:quanxian.w...@intel.com
Date:   Wed Jul 25 11:21:00 2012 +0800

Bug fix for mode issue when weston start

Provide the default builtin(800x480) mode when platforms
have no EDID and KMS could not provide mode information.
In the very beginning of weston, the connector could not
be set active mode. At that time, take the first mode in
mode list from KMS as default mode.

Signed-Off-By Quanxian Wang 
quanxian.w...@intel.commailto:quanxian.w...@intel.com

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 4dffa1d..47cd512 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -143,6 +143,16 @@ struct drm_sprite {
uint32_t formats[];
 };

+static drmModeModeInfo builtin_800x480 = {
+   33750,  /* clock */
+   800, 864, 976, 1088, 0,
+   480, 486, 494, 517, 0,
+   59920,
+   DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_PVSYNC,
+   0,
+   800x480
+};
+
 static int
 surface_is_primary(struct weston_compositor *ec, struct weston_surface *es)
 {
@@ -1344,13 +1354,30 @@ create_output_for_connector(struct drm_compositor *ec,
drmModeFreeEncoder(encoder);
if (crtc == NULL)
goto err_free;
-   crtc_mode = crtc-mode;
+
+   /* if don't get mode from drm driver, use default 800x480 */
+   if (crtc-mode.clock != 0)
+   {
+   crtc_mode = crtc-mode;
+   } else {
+   if (connector-count_modes == 0)
+   crtc_mode = builtin_800x480;
+   else
+   crtc_mode = connector-modes[0];
+   }
+
drmModeFreeCrtc(crtc);

-   for (i = 0; i  connector-count_modes; i++) {
-   ret = drm_output_add_mode(output, connector-modes[i]);
+   if (connector-count_modes == 0) {
+   ret = drm_output_add_mode(output, crtc_mode);
if (ret)
goto err_free;
+   }else{
+   for (i = 0; i  connector-count_modes; i++) {
+   ret = drm_output_add_mode(output, connector-modes[i]);
+   if (ret)
+   goto err_free;
+   }
}

preferred = NULL;


Thanks

Quanxian Wang




mode_issue_with_weston_start.patch
Description: mode_issue_with_weston_start.patch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

bug fix inactive encoder when weston restart

[Wang, Quanxian]

Hi, All

Please help review the patch.

Thanks

Background:
If we use 'pkill -9 weston', when restart weston, weston could not get the 
active encoder attached on connector and exit abnormally.
This is because when weston is pkilled, drm cleanups the encoders attached to 
active connector in KMS.
This patch will ignore the active encode checking, weston has the ability to 
re-initialize encoder and reattached it into connector.

===
commit 7f991195c20d9dd57e14faf88cd36db0c77df424
Author: Wang Quanxian quanxian.w...@intel.com
Date:   Thu Jul 19 19:11:04 2012 +0800
User sends SIGKILL to weston, KMS will clean up encoder
attached with connector. When weston restarts again, it causes
exit of weston because no active encoder is attached to connector.
This patch will ignore the checking and weston will initialize
encoder again and re-attach it to connector in the following process.

Sign-Off-By Quanxian Wang quanxian.w...@intel.com

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 47cd512..bb1b138 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -1348,26 +1348,28 @@ create_output_for_connector(struct drm_compositor *ec,
/* Get the current mode on the crtc that's currently driving
 * this connector. */
encoder = drmModeGetEncoder(ec-drm.fd, connector-encoder_id);
-   if (encoder == NULL)
-   goto err_free;
-   crtc = drmModeGetCrtc(ec-drm.fd, encoder-crtc_id);
-   drmModeFreeEncoder(encoder);
-   if (crtc == NULL)
-   goto err_free;
-
-   /* if don't get mode from drm driver, use default 800x480 */
-if (crtc-mode.clock != 0)
-   {
+   if (encoder != NULL)
+{
+   crtc = drmModeGetCrtc(ec-drm.fd, encoder-crtc_id);
+   drmModeFreeEncoder(encoder);
+   if (crtc == NULL)
+   goto err_free;
crtc_mode = crtc-mode;
-   } else {
+   drmModeFreeCrtc(crtc);
+   }
+   /*
+* No actiave encoder is connected with the connector
+* If connector has no mode available, takes detault mode
+* Or takes the first mode from connector
+*/
+   if (encoder == NULL || crtc_mode.clock == 0)
+{
if (connector-count_modes == 0)
crtc_mode = builtin_800x480;
else
crtc_mode = connector-modes[0];
}

-   drmModeFreeCrtc(crtc);
-
if (connector-count_modes == 0) {
ret = drm_output_add_mode(output, crtc_mode);
if (ret)


Thanks

Quanxian Wang




bug_fix_for_previous_inactive_encoder.patch
Description: bug_fix_for_previous_inactive_encoder.patch
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel