Re: [PATCH] wl_shell: Add surface state changed event

2013-05-20 Thread Pekka Paalanen
On Wed, 15 May 2013 11:37:19 -0700
Mikko Levonmaa mikko.levon...@gmail.com wrote:

 On Wed, May 15, 2013 at 12:12:43PM -0500, Jason Ekstrand wrote:
  On Wed, May 15, 2013 at 9:39 AM, Mikko Levonmaa mikko.levon...@gmail.com
  wrote:
  
  This allows the shell to inform the surface that it has changed
  state, current supported states are default, minimized, maximized
  and fullscreen. The shell implementation is free to interpret the
  meaning for the state, i.e. the minimized might not always mean
  that the surface is fully hidden for example.
  
  
  We cannot simply have the shell telling clients it changed their state.  The
  clients need to be in control of the state of each surface.  This is because
  minimizing a client (for example) might not be as simple as hiding a 
  specific
  window.  Only the client can actually know how to minimize/maximize it.
 
 Hmm... not sure I fully understand nor agree (perhaps lack of
 understanding;). So to me it seems that the compositor should be the
 driver, not the passenger, i.e. it know how to animate the surface when
 it gets minimized and when maximied. How would the client know this?
 Also wouldn't this imply more knowledge on the toolkits side as well?

I think this got already clear at other points, but here's a recap:

1. the server suggests a state to the client
2. the client issues a new state with a new drawing etc.
3. the server performs the state change as the client issued

This has to work whether the server-client interaction starts in step 1
or 2. Ever starting at step 3 results almost certainly in some user
visible glitches.

A good example of this is surface resizing:

0. the client asks to start a shell resize operation with a pointer
(drag from a resize handle), and tells from which sides the
resize happens
1. the server suggests a new size, and tells from which sides the
resize happens
2. the client decides to use the new size, or something close to it,
sends a new drawing in the new size, and also tells in which direction
it resized (wl_surface.attach arguments x,y).
3. the server executes the resize and move (in case resized from
top/left) on screen

All that always works whether the interaction starts in steps 0, 1, or
2. It just cannot work without bad user visible effects if it starts in
step 3.

  Please read earlier min/max discussions or yesterday's IRC logs for more
  details.
 
 Neato, seems to be a hot topic, good to see someone else looking into
 this as well. I read through the email and pq's commmets about avoiding 
 flicker
 make sense, so having the compositor and the client be in sync about whats
 going on is needed. Also naturally the client can be the originator, so
 clearly a request is needed. However in some cases the request might not be
 honored by the compositor, especially in an embedded environment. And
 actually also the compositor might only show window only in certain
 state, i.e. fullscreen so having the client full to decline a request
 might not be good either.

Not honored by a compositor and the *compositor* misbehaving when a
client does not comply to a state suggestion are not an option. If a
client uses the defined protocol according to the spec, any
misbehaviour is the server's bug.

This may be a little side-step in the topic, but I want this out of my
system now. One thing is that Wayland protocol is very asynchronous, and
it has no return values. The only possibilities for failing a request
are to disconnect the client with a protocol error, or in some rare
cases just ignoring the request and guarantee that the following
protocol exchange corrects the situation without any user visible
effects. In other words, requests cannot really fail, unless the client
does something illegal, in which case it is fatal.

If we did have return fail or success in the protocol, all such
requests would become synchronous. Issuing such a request would always
require an immediate round-trip. Without a round-trip, the client and
the server will generally go out of sync on protocol state, when the
client sends more requests assuming the first request succeeded, and if
it instead fails, how does it track happens with the rest.

I think what you work on here is not something like the cursor change
or move/resize requests, which rely on other protocol to correct for
the state mismatch between the client and the server immediately. These
auto-correcting state mismatches are essentially due to races, and can
be recognized by the requests needing a specific serial number as an
argument.

So, you must design the protocol to pre-emptively avoid requests that
may fail or not be honored.

In this case, if we really wanted desktop shells that only ever support
just some of the window states, you have to advertise the supported
states beforehand. If a client uses an unsupported state, it will
trigger a protocol error and get disconnected. Not only does this make
the protocol clear, it allows the client to adapt 

Re: [PATCH 1/2] protocol: Allow output changes to be treated atomically

2013-05-20 Thread Pekka Paalanen
On Thu, 16 May 2013 15:49:35 +0200
al...@redhat.com wrote:

 From: Alexander Larsson al...@redhat.com
 
 This add a wl_output.done event which is send after every group
 of events caused by some property change. This allows clients to treat
 changes touching multiple events in an atomic fashion.

Hi,

why did you end up deciding that this is better than using the geometry
event as the end signal?


Thanks,
pq

 ---
  protocol/wayland.xml | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/protocol/wayland.xml b/protocol/wayland.xml
 index 3bce022..d3ae149 100644
 --- a/protocol/wayland.xml
 +++ b/protocol/wayland.xml
 @@ -1467,7 +1467,7 @@
  /event
/interface
  
 -  interface name=wl_output version=1
 +  interface name=wl_output version=2
  description summary=compositor output region
An output describes part of the compositor geometry.  The
compositor works in the 'compositor coordinate system' and an
 @@ -1565,6 +1565,16 @@
arg name=height type=int summary=height of the mode in pixels/
arg name=refresh type=int summary=vertical refresh rate in mHz/
  /event
 +
 +event name=done since=2
 +  description summary=sent all information about output
 +This event is sent after all other properties has been
 +sent after binding to the output object and after any
 +other property changes done after that. This allows
 +changes to the output properties to be seen as
 +atomic, even if they happen via multiple events.
 +  /description
 +/event
/interface
  
interface name=wl_region version=1

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


Re: [PATCH 2/2] protocol: Support scaled outputs and surfaces

2013-05-20 Thread Pekka Paalanen
Hi Alexander,

nice to see this going forward, and sorry for replying so rarely and
late.

On Thu, 16 May 2013 15:49:36 +0200
al...@redhat.com wrote:

 From: Alexander Larsson al...@redhat.com
 
 This adds the wl_surface.set_buffer_scale request, and a wl_output.scale
 event. These together lets us support automatic upscaling of old
 clients on very high resolution monitors, while allowing new clients
 to take advantage of this to render at the higher resolution when the
 surface is displayed on the scaled output.
 
 It is similar to set_buffer_transform in that the buffer is stored in
 a transformed pixels (in this case scaled). This means that if an output
 is scaled we can directly use the pre-scaled buffer with additional data,
 rather than having to scale it.
 
 Additionally this adds a scaled flag to the wl_output.mode flags
 so that clients know which resolutions are native and which are scaled.
 
 Also, in places where the documentation was previously not clear as to
 what coordinate system was used this was fleshed out.
 
 It also adds a scaling_factor event to wl_output that specifies the
 scaling of an output.
 
 This is meant to be used for outputs with a very high DPI to tell the
 client that this particular output has subpixel precision. Coordinates
 in other parts of the protocol, like input events, relative window
 positioning and output positioning are still in the compositor space

We don't have a single compositor space.

This needs some way of explaining that surface coordinates are always
the same, regardless of the attached buffer's scale. That is, surface
coordinates always correspond to the size of a buffer with scale 1.

 rather than the scaled space. However, input has subpixel precision
 so you can still get input at full resolution.
 
 This setup means global properties like mouse acceleration/speed,
 pointer size, monitor geometry, etc can be specified in a mostly
 similar resolution even on a multimonitor setup where some monitors
 are low dpi and some are e.g. retina-class outputs.
 ---
  protocol/wayland.xml | 107 
 ---
  1 file changed, 93 insertions(+), 14 deletions(-)
 
 diff --git a/protocol/wayland.xml b/protocol/wayland.xml
 index d3ae149..acfb140 100644
 --- a/protocol/wayland.xml
 +++ b/protocol/wayland.xml
 @@ -173,7 +173,7 @@
  /event
/interface
  
 -  interface name=wl_compositor version=2
 +  interface name=wl_compositor version=3

Ok.

  description summary=the compositor singleton
A compositor.  This object is a singleton global.  The
compositor is in charge of combining the contents of multiple
 @@ -709,7 +709,7 @@
  
   The x and y arguments specify the locations of the upper left
   corner of the surface relative to the upper left corner of the
 - parent surface.
 + parent surface, in surface local coordinates.
  
   The flags argument controls details of the transient behaviour.
/description
 @@ -777,6 +777,10 @@
   in any of the clients surfaces is reported as normal, however,
   clicks in other clients surfaces will be discarded and trigger
   the callback.
 +
 + The x and y arguments specify the locations of the upper left
 + corner of the surface relative to the upper left corner of the
 + parent surface, in surface local coordinates.

Surface local coordinates are defined to have their origin in the
surface top-left corner. If that is defined once and for all, you don't
have to repeat relative to upper left... everywhere.

Surface local coordinates relative to anything else do not exist.

When these were originally written in the spec, the term surface
coordinates had not settled yet.

/description
  
arg name=seat type=object interface=wl_seat summary=the 
 wl_seat whose pointer is used/
 @@ -860,6 +864,9 @@
  
   The client is free to dismiss all but the last configure
   event it received.
 +
 + The width and height arguments specify the size of the window
 + in surface local coordinates.

Yes, window is definitely the correct term here. Saying surface
would be incorrect, due to sub-surfaces, if I recall my discussion with
Giulio Camuffo right.

/description
  
arg name=edges type=uint/
 @@ -876,11 +883,16 @@
  /event
/interface
  
 -  interface name=wl_surface version=2
 +  interface name=wl_surface version=3
  description summary=an onscreen surface
A surface is a rectangular area that is displayed on the screen.
It has a location, size and pixel contents.
  
 +  The size of a surface (and relative positions on it) is described

The size of the surface and positions on the surface are described...?

 +  in surface local coordinates, which may differ from the buffer
 +  local coordinates of the pixel content, in case a buffer_transform
 +  or a buffer_scale is used.

I think we could additionally define here, that surface local

Re: [PATCH 2/2] protocol: Support scaled outputs and surfaces

2013-05-20 Thread Pekka Paalanen
On Thu, 16 May 2013 16:43:52 -0500
Jason Ekstrand ja...@jlekstrand.net wrote:

 The point of this soi is to allow surfaces to render the same size on
 different density outputs.

Are you serious? Really? Same size measured in meters?

I do not think that will ever work:
http://blogs.gnome.org/danni/2011/12/15/more-on-dpi/
and doing it via scaling is going to be worse.

Going for the same size is a very different problem than just trying to
get all apps readable by default. I'm not sure same size is a better
goal than same look.

And on a side note:
http://web.archive.org/web/20120102153021/http://www.fooishbar.org/blog

Which email was your detailed proposition?


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


Re: [PATCH 2/2] protocol: Support scaled outputs and surfaces

2013-05-20 Thread Pekka Paalanen
On Fri, 17 May 2013 12:06:35 -0700
Bill Spitzak spit...@gmail.com wrote:

 Alexander Larsson wrote:
 
  You can make a surface of any integer size (and it has to be integer due
  to existing APIs on surface coordinates/sizes), however the *buffer* has
  to be an integer multiple of the surface size. In other words, surface
  sizes and positions are described in the global compositor space, with
  integer sizes.
 
 This seems pretty limiting to me. What happens when *all* the outputs 
 are hi-res? You really think wayland clients should not be able to take 
 full advantage of this?

Then the individual pixels are so small that it won't matter.

If the pixels are not small enough to not matter, then you just set
scale=1 to everything, and the image it still legible.

  If nothing else it makes it so that subsurfaces are
  always positioned on integer positions on non-scaled displays, which
  makes things easier when monitor of differen scales are mixed.
 
 This is false if the subsurface is attached to a scaled parent surface.

Huh?

  I see it the other way. We currently have *two* coordinate spaces that
  the client has to think about. The buffer coordinates (it has to know
  this when rendering), and the surface coordinates (these are basically
  what all wayland APIs atm use, like in damage, positioning and input).
  The transform between two is currently the buffer_transform only. With
  the buffer_scale the transform is extended to also scale, but no
  additional coordinate space is added.
 
 The input rectangle to the scaler proposal is in the space between the 
 buffer transform and the scaling. Therefore there are *three* coordinate 
 spaces.

Where did you get this? Where is this defined or proposed?

 My proposal is that surface space be moved before the scaling. This 
 reduces the number of spaces back to two by using the same space for 
 input rectangle as for events and surface size, etc. It also means 
 integers always have a physical meaning for the client (ie buffer 
 pixels) and that odd-sized buffers are supported on the hi-res display.

On a quick thought, that seems only a different way of doing it,
without any benefits, and possibly having cons.

Actually, it means that the surface coordinate system can change
dramatically when a client sends a new buffer with a different scale,
which then raises a bucketful of races: is an incoming event using new
or old surface coordinates? That includes at least all input events
with a surface position, and the shell geometry event.

For the record, wl_surface.attach changes the surface coordinate system
by translating with x,y, but that is not a problem. The x,y do not
describe how the surface moves, they describe how pixel rows and
columns are added or removed on the edges. This means that the content
is presumed to stay put on screen. It's also hard to click a specific
point in a window whose size is changing, and the translation is not
dramatic. Even when one might claim that attach has the same problem as
your proposal, in practice it does not.

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


[PATCH weston] shell: End grab if the grabbed shell surface has been destroyed

2013-05-20 Thread Rob Bradford
From: Rob Bradford r...@linux.intel.com

The shell_grab_start function sets up a destroy notification on the
shell surface such that when the shell surface is destroyed the pointer
on the grab to the shell surface is set to NULL.

We must therefore check whether the shell surface is NULL and end the
grab if it is.

https://bugs.freedesktop.org/show_bug.cgi?id=64689
---
 src/shell.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shell.c b/src/shell.c
index f5d5bff..7261570 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -1296,7 +1296,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base)
 pointer-x, pointer-y,
 sx, sy);
 
-   if (grab-shsurf-surface != surface) {
+   if (!grab-shsurf || grab-shsurf-surface != surface) {
shell_grab_end(grab);
free(grab);
}
-- 
1.8.1.4

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


Re: [PATCH weston] shell: End grab if the grabbed shell surface has been destroyed

2013-05-20 Thread Rob Bradford
I should add that although I think that this patch fixes the bug, i've
written it by inspection of the code  backtraces only as I was unable
to reproduce the issue. Artie, perhaps you could try this and give me
a Tested-by if it resolves the problem.

Cheers,

Rob

On 20 May 2013 12:09, Rob Bradford robert.bradf...@intel.com wrote:
 From: Rob Bradford r...@linux.intel.com

 The shell_grab_start function sets up a destroy notification on the
 shell surface such that when the shell surface is destroyed the pointer
 on the grab to the shell surface is set to NULL.

 We must therefore check whether the shell surface is NULL and end the
 grab if it is.

 https://bugs.freedesktop.org/show_bug.cgi?id=64689
 ---
  src/shell.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/shell.c b/src/shell.c
 index f5d5bff..7261570 100644
 --- a/src/shell.c
 +++ b/src/shell.c
 @@ -1296,7 +1296,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base)
  pointer-x, pointer-y,
  sx, sy);

 -   if (grab-shsurf-surface != surface) {
 +   if (!grab-shsurf || grab-shsurf-surface != surface) {
 shell_grab_end(grab);
 free(grab);
 }
 --
 1.8.1.4

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


RE: [PATCH weston] shell: End grab if the grabbed shell surface has been destroyed

2013-05-20 Thread Eoff, Ullysses A
Rob, I tried this same local modification on Friday... it seems to fix
Weston from segfaulting. However, I have another test case that
triggered this problem, too; only it crashes on the client-side as well.
The client-side crash did not disappear with this Weston modification,
which indicates there may be some deeper, underlying issue.

I'll point you to the test case soon, real soon ;-)


U. Artie 


 -Original Message-
 From: Rob Bradford [mailto:robert.bradf...@intel.com]
 Sent: Monday, May 20, 2013 4:14 AM
 To: wayland-devel@lists.freedesktop.org; Eoff, Ullysses A
 Subject: Re: [PATCH weston] shell: End grab if the grabbed shell surface has 
 been
 destroyed
 
 I should add that although I think that this patch fixes the bug, i've
 written it by inspection of the code  backtraces only as I was unable
 to reproduce the issue. Artie, perhaps you could try this and give me
 a Tested-by if it resolves the problem.
 
 Cheers,
 
 Rob
 
 On 20 May 2013 12:09, Rob Bradford robert.bradf...@intel.com wrote:
  From: Rob Bradford r...@linux.intel.com
 
  The shell_grab_start function sets up a destroy notification on the
  shell surface such that when the shell surface is destroyed the pointer
  on the grab to the shell surface is set to NULL.
 
  We must therefore check whether the shell surface is NULL and end the
  grab if it is.
 
  https://bugs.freedesktop.org/show_bug.cgi?id=64689
  ---
   src/shell.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/src/shell.c b/src/shell.c
  index f5d5bff..7261570 100644
  --- a/src/shell.c
  +++ b/src/shell.c
  @@ -1296,7 +1296,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab
 *base)
   pointer-x, pointer-y,
   sx, sy);
 
  -   if (grab-shsurf-surface != surface) {
  +   if (!grab-shsurf || grab-shsurf-surface != surface) {
  shell_grab_end(grab);
  free(grab);
  }
  --
  1.8.1.4
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston] weston-launch: Print explanation of why we failed to open the device

2013-05-20 Thread Rob Bradford
From: Rob Bradford r...@linux.intel.com

---
 src/weston-launch.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/weston-launch.c b/src/weston-launch.c
index 89c3c5a..42b2506 100644
--- a/src/weston-launch.c
+++ b/src/weston-launch.c
@@ -320,12 +320,17 @@ handle_open(struct weston_launch *wl, struct msghdr *msg, 
ssize_t len)
goto err0;
 
fd = open(message-path, message-flags);
-   if (fd  0)
+   if (fd  0) {
+   fprintf(stderr, Error opening device %s: %m\n,
+   message-path);
goto err0;
+   }
 
if (major(s.st_rdev) != INPUT_MAJOR) {
close(fd);
fd = -1;
+   fprintf(stderr, Device %s is not an input device\n,
+   message-path);
goto err0;
}
 
-- 
1.8.1.4

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


RE: [PATCH weston] shell: End grab if the grabbed shell surface has been destroyed

2013-05-20 Thread Eoff, Ullysses A
Nonetheless, this is still a reasonable patch that can be applied to solve
part of the problem...  that is, it prevents Weston from crashing.

 -Original Message-
 From: wayland-devel-bounces+ullysses.a.eoff=intel@lists.freedesktop.org
 [mailto:wayland-devel-
 bounces+ullysses.a.eoff=intel@lists.freedesktop.org] On Behalf Of Eoff,
 Ullysses A
 Sent: Monday, May 20, 2013 5:59 AM
 To: Bradford, Robert; wayland-devel@lists.freedesktop.org
 Subject: RE: [PATCH weston] shell: End grab if the grabbed shell surface has 
 been
 destroyed
 
 Rob, I tried this same local modification on Friday... it seems to fix
 Weston from segfaulting. However, I have another test case that
 triggered this problem, too; only it crashes on the client-side as well.
 The client-side crash did not disappear with this Weston modification,
 which indicates there may be some deeper, underlying issue.
 
 I'll point you to the test case soon, real soon ;-)
 
 
 U. Artie
 
 
  -Original Message-
  From: Rob Bradford [mailto:robert.bradf...@intel.com]
  Sent: Monday, May 20, 2013 4:14 AM
  To: wayland-devel@lists.freedesktop.org; Eoff, Ullysses A
  Subject: Re: [PATCH weston] shell: End grab if the grabbed shell surface has
 been
  destroyed
 
  I should add that although I think that this patch fixes the bug, i've
  written it by inspection of the code  backtraces only as I was unable
  to reproduce the issue. Artie, perhaps you could try this and give me
  a Tested-by if it resolves the problem.
 
  Cheers,
 
  Rob
 
  On 20 May 2013 12:09, Rob Bradford robert.bradf...@intel.com wrote:
   From: Rob Bradford r...@linux.intel.com
  
   The shell_grab_start function sets up a destroy notification on the
   shell surface such that when the shell surface is destroyed the pointer
   on the grab to the shell surface is set to NULL.
  
   We must therefore check whether the shell surface is NULL and end the
   grab if it is.
  
   https://bugs.freedesktop.org/show_bug.cgi?id=64689
   ---
src/shell.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
  
   diff --git a/src/shell.c b/src/shell.c
   index f5d5bff..7261570 100644
   --- a/src/shell.c
   +++ b/src/shell.c
   @@ -1296,7 +1296,7 @@ busy_cursor_grab_focus(struct
 weston_pointer_grab
  *base)
pointer-x, pointer-y,
sx, sy);
  
   -   if (grab-shsurf-surface != surface) {
   +   if (!grab-shsurf || grab-shsurf-surface != surface) {
   shell_grab_end(grab);
   free(grab);
   }
   --
   1.8.1.4
  
 ___
 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


[PATCH weston] udev-seat: Fail seat setup only if the seat is incomplete

2013-05-20 Thread Rob Bradford
From: Rob Bradford r...@linux.intel.com

Rather than failing seat setup if we fail to open the input device
instead fail the seat setup if we don't have complete seat with both
keyboard and pointer or a touchscreen.

https://bugs.freedesktop.org/show_bug.cgi?id=64506
---
 src/udev-seat.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/udev-seat.c b/src/udev-seat.c
index 7e62429..3dd3438 100644
--- a/src/udev-seat.c
+++ b/src/udev-seat.c
@@ -58,7 +58,7 @@ device_added(struct udev_device *udev_device, struct 
udev_seat *master)
fd = weston_launcher_open(c, devnode, O_RDWR | O_NONBLOCK);
if (fd  0) {
weston_log(opening input device '%s' failed.\n, devnode);
-   return -1;
+   return 0;
}
 
device = evdev_device_create(master-base, devnode, fd);
@@ -69,7 +69,7 @@ device_added(struct udev_device *udev_device, struct 
udev_seat *master)
} else if (device == NULL) {
close(fd);
weston_log(failed to create input device '%s'.\n, devnode);
-   return -1;
+   return 0;
}
 
calibration_values =
@@ -142,6 +142,12 @@ udev_seat_add_devices(struct udev_seat *seat, struct udev 
*udev)
udev device property ID_SEAT)\n);
}
 
+   if (!(seat-base.touch || (seat-base.keyboard  seat-base.pointer))) 
{
+   weston_log (seat not complete: no touchscreen or 
+   no keyboard and pointer found.\n);
+   return -1;
+   }
+
return 0;
 }
 
-- 
1.8.1.4

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


Re: [PATCH 2/2] protocol: Support scaled outputs and surfaces

2013-05-20 Thread Jason Ekstrand
On Mon, May 20, 2013 at 4:00 AM, Pekka Paalanen ppaala...@gmail.com wrote:

 On Thu, 16 May 2013 16:43:52 -0500
 Jason Ekstrand ja...@jlekstrand.net wrote:

  The point of this soi is to allow surfaces to render the same size on
  different density outputs.

 Are you serious? Really? Same size measured in meters?


No, measured in inches. :-P

Seriously though.  While we can't make it *exactly* the same on all your
displays, we should be able to make it usably close.


 I do not think that will ever work:
 http://blogs.gnome.org/danni/2011/12/15/more-on-dpi/
 and doing it via scaling is going to be worse.


Yes, scaling looks bad.  I don't know that we can avoid it in all cases
(see also the 200DPI and 300 DPI case).


 Going for the same size is a very different problem than just trying to
 get all apps readable by default. I'm not sure same size is a better
 goal than same look.

 And on a side note:
 http://web.archive.org/web/20120102153021/http://www.fooishbar.org/blog


What I would like in the end is a per-output slider bar (or something of
that ilk) that let's the user select the interface size on that output.
Sure, they probably won't be able to select *any* resolution (the
compositor may limit it to multiples of 24 dpi or something).  And they can
certainly make an ugly set-up for themselves.  However, I want them to be
able to make something more-or-less reasonable and I see no reason why the
compositor shouldn't coordinate this and why this scale factor can't be
used for that.

My primary concern is that integer multiples of 96 DPI isn't going to be
enough granularity.  I don't know whether we can really accomplish a higher
granularity in a reasonable way.


 Which email was your detailed proposition?


Alexander already gave me a very good reason why my original idea won't
work (too restrictive on the protocol) and I now agree with him.  That
said, I'd like to find some way to accomplish the above.


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


Re: [PATCH weston] shell: End grab if the grabbed shell surface has been destroyed

2013-05-20 Thread Kristian Høgsberg
On Mon, May 20, 2013 at 12:09:20PM +0100, Rob Bradford wrote:
 From: Rob Bradford r...@linux.intel.com
 
 The shell_grab_start function sets up a destroy notification on the
 shell surface such that when the shell surface is destroyed the pointer
 on the grab to the shell surface is set to NULL.
 
 We must therefore check whether the shell surface is NULL and end the
 grab if it is.

Thanks Rob, I think that was indeed the problem I introduced with the
input rewrite.

Kristian

 https://bugs.freedesktop.org/show_bug.cgi?id=64689
 ---
  src/shell.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/shell.c b/src/shell.c
 index f5d5bff..7261570 100644
 --- a/src/shell.c
 +++ b/src/shell.c
 @@ -1296,7 +1296,7 @@ busy_cursor_grab_focus(struct weston_pointer_grab *base)
pointer-x, pointer-y,
sx, sy);
  
 - if (grab-shsurf-surface != surface) {
 + if (!grab-shsurf || grab-shsurf-surface != surface) {
   shell_grab_end(grab);
   free(grab);
   }
 -- 
 1.8.1.4
 
 ___
 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 weston] weston-launch: Print explanation of why we failed to open the device

2013-05-20 Thread Kristian Høgsberg
On Mon, May 20, 2013 at 04:55:10PM +0100, Rob Bradford wrote:
 From: Rob Bradford r...@linux.intel.com

That's better, though I wonder if we should instead let weston log the
error message using weston_log()... committed this for now.

Kristian

 ---
  src/weston-launch.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/src/weston-launch.c b/src/weston-launch.c
 index 89c3c5a..42b2506 100644
 --- a/src/weston-launch.c
 +++ b/src/weston-launch.c
 @@ -320,12 +320,17 @@ handle_open(struct weston_launch *wl, struct msghdr 
 *msg, ssize_t len)
   goto err0;
  
   fd = open(message-path, message-flags);
 - if (fd  0)
 + if (fd  0) {
 + fprintf(stderr, Error opening device %s: %m\n,
 + message-path);
   goto err0;
 + }
  
   if (major(s.st_rdev) != INPUT_MAJOR) {
   close(fd);
   fd = -1;
 + fprintf(stderr, Device %s is not an input device\n,
 + message-path);
   goto err0;
   }
  
 -- 
 1.8.1.4
 
 ___
 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 weston] udev-seat: Fail seat setup only if the seat is incomplete

2013-05-20 Thread Kristian Høgsberg
On Mon, May 20, 2013 at 05:55:03PM +0100, Rob Bradford wrote:
 From: Rob Bradford r...@linux.intel.com
 
 Rather than failing seat setup if we fail to open the input device
 instead fail the seat setup if we don't have complete seat with both
 keyboard and pointer or a touchscreen.

 https://bugs.freedesktop.org/show_bug.cgi?id=64506
 ---
  src/udev-seat.c | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/src/udev-seat.c b/src/udev-seat.c
 index 7e62429..3dd3438 100644
 --- a/src/udev-seat.c
 +++ b/src/udev-seat.c
 @@ -58,7 +58,7 @@ device_added(struct udev_device *udev_device, struct 
 udev_seat *master)
   fd = weston_launcher_open(c, devnode, O_RDWR | O_NONBLOCK);
   if (fd  0) {
   weston_log(opening input device '%s' failed.\n, devnode);
 - return -1;
 + return 0;
   }
  
   device = evdev_device_create(master-base, devnode, fd);
 @@ -69,7 +69,7 @@ device_added(struct udev_device *udev_device, struct 
 udev_seat *master)
   } else if (device == NULL) {
   close(fd);
   weston_log(failed to create input device '%s'.\n, devnode);
 - return -1;
 + return 0;
   }
  
   calibration_values =
 @@ -142,6 +142,12 @@ udev_seat_add_devices(struct udev_seat *seat, struct 
 udev *udev)
   udev device property ID_SEAT)\n);
   }
  
 + if (!(seat-base.touch || (seat-base.keyboard  seat-base.pointer))) 
 {
 + weston_log (seat not complete: no touchscreen or 
 + no keyboard and pointer found.\n);
 + return -1;
 + }
 +

I wonder if the previous check isn't good enough - I think requiring a
keyboard and a mouse is a little restrictive, there are many cases
where we only have a keyboard or only a mouse.  And if we do want this
more specific check, at least drop the check for an empty
devices_list.

   return 0;
  }
  
 -- 
 1.8.1.4
 
 ___
 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] configure.ac: colord version to 0.1.27

2013-05-20 Thread Kristian Høgsberg
On Sat, May 18, 2013 at 10:08:17PM +0900, Mun, Gwan-gyeong wrote:
 This patch fixes colord version check on confiure.ac file.
 
 Weston uses CD_PROFILE_METADATA_SCREEN_BRIGHTNESS,
 CD_DEVICE_PROPERTY_EMBEDDED colord macros.
 
 But weston checks colord old version. ( 0.1.8)
 
 CD_PROFILE_METADATA_SCREEN_BRIGHTNESS macro provided after colord 0.1.17.
 CD_DEVICE_PROPERTY_EMBEDDED macro provided after colord 0.1.27.
 
 so weston should check at least colord version: 0.1.27.

Thanks, patch applied.

Kristian

 ---
 From ea647f197b22d14f7828d511bc2443ba91001a30 Mon Sep 17 00:00:00 2001
 From: Mun Gwan-gyeong elong...@gmail.com
 Date: Sat, 18 May 2013 21:43:10 +0900
 Subject: [PATCH] configure.ac: colord version to 0.1.27
 
 ---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/configure.ac b/configure.ac
 index fd85b2c..93a6720 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -288,7 +288,7 @@ AC_ARG_ENABLE(colord,
 AM_CONDITIONAL(ENABLE_COLORD,
 test x$enable_colord = xyes)
 if test x$enable_colord = xyes; then
 - PKG_CHECK_MODULES(COLORD, colord = 0.1.8)
 + PKG_CHECK_MODULES(COLORD, colord = 0.1.27)
 fi
 
 AC_ARG_ENABLE(wcap-tools, [ --disable-wcap-tools],, enable_wcap_tools=yes)
 -- 
 1.7.9.5
 
 

 From ea647f197b22d14f7828d511bc2443ba91001a30 Mon Sep 17 00:00:00 2001
 From: Mun Gwan-gyeong elong...@gmail.com
 Date: Sat, 18 May 2013 21:43:10 +0900
 Subject: [PATCH] configure.ac: colord version to 0.1.27
 
 ---
  configure.ac |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/configure.ac b/configure.ac
 index fd85b2c..93a6720 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -288,7 +288,7 @@ AC_ARG_ENABLE(colord,
  AM_CONDITIONAL(ENABLE_COLORD,
  test x$enable_colord = xyes)
  if test x$enable_colord = xyes; then
 -  PKG_CHECK_MODULES(COLORD, colord = 0.1.8)
 +  PKG_CHECK_MODULES(COLORD, colord = 0.1.27)
  fi
  
  AC_ARG_ENABLE(wcap-tools, [  --disable-wcap-tools],, enable_wcap_tools=yes)
 -- 
 1.7.9.5
 

 ___
 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 weston 1/6] protocol: improve sub-surface spec wording

2013-05-20 Thread Kristian Høgsberg
On Mon, May 20, 2013 at 08:33:53AM +0300, Pekka Paalanen wrote:
 On Fri, 17 May 2013 16:20:48 -0400
 Kristian Høgsberg hoegsb...@gmail.com wrote:
 
  On Fri, May 17, 2013 at 04:46:03PM +0300, ppaala...@gmail.com wrote:
   From: Pekka Paalanen ppaala...@gmail.com
   
   Mention, that sub-surfaces are not clipped to the parent.
   Be more accurate on surface commit vs. apply state.
   Mention the initial stacking order.
  
  That looks good, applied.
 
 Hmm, I see the other patches, but not this particular patch in upstream
 master. What happened?

Yeah, good question... upstream now.

Kristian


 Thanks,
 pq
 
  
   Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk
   ---
protocol/subsurface.xml | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
   
   diff --git a/protocol/subsurface.xml b/protocol/subsurface.xml
   index 60b4002..71dc1e9 100644
   --- a/protocol/subsurface.xml
   +++ b/protocol/subsurface.xml
   @@ -87,7 +87,10 @@
  interface name=wl_subsurface version=1
description summary=sub-surface interface to a wl_surface
  An additional interface to a wl_surface object, which has been
   -  made a sub-surface. A sub-surface has one parent surface.
   +  made a sub-surface. A sub-surface has one parent surface. A
   +  sub-surface's size and position are not limited to that of the 
   parent.
   +  Particularly, a sub-surface is not automatically clipped to its
   +  parent's area.

  A sub-surface becomes mapped, when a non-NULL wl_buffer is applied
  and the parent surface is mapped. The order of which one happens
   @@ -99,8 +102,8 @@
  depends on the sub-surface's mode. The possible modes are
  synchronized and desynchronized, see methods
  wl_subsurface.set_sync and wl_subsurface.set_desync. Synchronized
   -  mode caches wl_surface state to be applied on the next parent
   -  surface's commit, and desynchronized mode applies the pending
   +  mode caches the wl_surface state to be applied when the parent's
   +  state gets applied, and desynchronized mode applies the pending
  wl_surface state directly. A sub-surface is initially in the
  synchronized mode.

   @@ -113,15 +116,15 @@
  wl_surface state is applied, regardless of the sub-surface's mode.
  As the exception, set_sync and set_desync are effective 
   immediately.

   -  The main surface can thought to be always in desynchronized mode,
   +  The main surface can be thought to be always in desynchronized 
   mode,
  since it does not have a parent in the sub-surfaces sense.

  Even if a sub-surface is in desynchronized mode, it will behave as
  in synchronized mode, if its parent surface behaves as in
  synchronized mode. This rule is applied recursively throughout the
  tree of surfaces. This means, that one can set a sub-surface into
   -  synchronized mode, and then assume that all its child sub-surfaces
   -  are synchronized, too, without explicitly setting them.
   +  synchronized mode, and then assume that all its child and 
   grand-child
   +  sub-surfaces are synchronized, too, without explicitly setting 
   them.

  If the wl_surface associated with the wl_subsurface is destroyed, 
   the
  wl_subsurface object becomes inert. Note, that destroying either 
   object
   @@ -153,7 +156,9 @@
  description summary=reposition the sub-surface
 This schedules a sub-surface position change.
 The sub-surface will be moved so, that its origin (top-left
   - corner pixel) will be at the location x, y of the parent surface.
   + corner pixel) will be at the location x, y of the parent surface
   + coordinate system. The coordinates are not restricted to the parent
   + surface area. Negative values are allowed.

 The next wl_surface.commit on the parent surface will reset
 the sub-surface's position to the scheduled coordinates.
   @@ -176,6 +181,9 @@
 The z-order is double-buffered state, and will be applied on the
 next commit of the parent surface.
 See wl_surface.commit and wl_subcompositor.get_subsurface.
   +
   + A new sub-surface is initially added as the top-most in the stack
   + of its siblings and parent.
  /description

  arg name=sibling type=object interface=wl_surface
   @@ -200,9 +208,8 @@
 In synchronized mode, wl_surface.commit on a sub-surface will
 accumulate the committed state in a cache, but the state will
 not be applied and hence will not change the compositor output.
   - The cached state is applied to the sub-surface when
   - wl_surface.commit is called on the parent surface, after the
   - parent surface's own state is applied. This ensures atomic
   + The cached state is applied to the sub-surface immediately after
   + the parent surface's state is applied. 

Re: [PATCH 2/2] protocol: Support scaled outputs and surfaces

2013-05-20 Thread Bill Spitzak

Pekka Paalanen wrote:

This seems pretty limiting to me. What happens when *all* the outputs 
are hi-res? You really think wayland clients should not be able to take 
full advantage of this?


Then the individual pixels are so small that it won't matter.


It does not matter how tiny the pixels are. The step between possible 
surface sizes and subsurface positions remains the size of a scale-1 
unit. Or else I am seriously mis-understanding the proposal:


Let's say the output is 10,000dpi and the compositor has set it's scale 
to 100. Can a client make a buffer that is 10,050 pixels wide appear 1:1 
on the pixels of this output? It looks to me like only multiples of 100 
are possible.



If nothing else it makes it so that subsurfaces are
always positioned on integer positions on non-scaled displays, which
makes things easier when monitor of differen scales are mixed.

This is false if the subsurface is attached to a scaled parent surface.


Huh?


Parent surface uses the scaler api to change a buffer width of 100 to 
150. The fullscreen and this hi-dpi interface can also produce similar 
scales. The subsurface has a width of 51. Either the left or right edge 
is going to land in the middle of an output pixel.


The input rectangle to the scaler proposal is in the space between the 
buffer transform and the scaling. Therefore there are *three* coordinate 
spaces.


Where did you get this? Where is this defined or proposed?


The input rectangle is in the same direction as the output rectangle 
even if the buffer is rotated 90 degrees by the buffer_transform.



On a quick thought, that seems only a different way of doing it,
without any benefits, and possibly having cons.


Benefits: the buffer can be any integer number of pixels in size, 
non-integer buffer sizes cannot be specified by the api, you can align 
subsurfaces with pixels in the buffer (which means a precomposite of 
subsurfaces into the main one before scaling is possible).



Actually, it means that the surface coordinate system can change
dramatically when a client sends a new buffer with a different scale,
which then raises a bucketful of races: is an incoming event using new
or old surface coordinates? That includes at least all input events
with a surface position,


This is a good point and the only counter argument that makes sense.

All solutions I can think of are equivalent to reporting events in the 
output space, the same as your proposal. However I still feel that the 
surface size, input area, and other communication from client to server 
should be specified in input space.



and the shell geometry event.


Geometry is in the space of the parent surface, not this surface. This 
is true in both proposals. Both would get exactly the same geometry events.



For the record, wl_surface.attach changes the surface coordinate system
by translating with x,y, but that is not a problem. The x,y do not
describe how the surface moves, they describe how pixel rows and
columns are added or removed on the edges.


If x,y is in buffer pixels then it matches my proposal. It can change 
the results of the scaler to non-integers then, so I was under the 
impression it would be ignored in this case. Assuming logical use of the 
hi-dpi I don't see a problem with it being in buffer pixels then.


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


Re: [RFC] libinputmapper: Input device configuration for graphic-servers

2013-05-20 Thread Peter Hutterer
On Thu, May 16, 2013 at 08:38:44AM -0400, Todd Showalter wrote:
 On Thu, May 16, 2013 at 1:37 AM, Peter Hutterer
 peter.hutte...@who-t.net wrote:
 
  why are gamepads and joysticks different? buttons, a few axes that may or
  may not map to x/y and the rest is device-specific.
  this may be in the thread, but I still haven't gone through all msgs here.
 
 Joysticks are designed for a different purpose (flight sims), and
 so have a different set of controls.  For example, on a lot of
 joysticks there is a throttle, which is a constrained axis you can
 set to any position and it will stay there until you move it again.
 Button placement on joysticks tends to be more arbitrary as well.
 
 In terms of raw functionality they're similar, but the differences
 are large enough (especially in the way they're used) that they are
 better treated separately.
 
what I am wondering is whether that difference matters to the outside
observer (i.e. the compositor). a gamepad and a joystick are both gaming
devices and with the exception of the odd need to control the pointer it
doesn't matter much which type they are.

as for a game that would access the device - does it matter if the device is
labelled gamepad or joystick? if it's a gaming device you have to look at
the pysical properties anyway and decide which you use in what matter.

Cheers,
   Peter


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


Re: [RFC] libinputmapper: Input device configuration for graphic-servers

2013-05-20 Thread Peter Hutterer
On Thu, May 16, 2013 at 03:16:11PM +0200, David Herrmann wrote:
 Hi Peter
 
 On Thu, May 16, 2013 at 7:37 AM, Peter Hutterer
 peter.hutte...@who-t.net wrote:
  On Sun, May 12, 2013 at 04:20:59PM +0200, David Herrmann wrote:
 [..]
  So what is the proposed solution?
  My recommendation is, that compositors still search for devices via
  udev and use device drivers like libxkbcommon. So linux evdev handling
  is still controlled by the compositor. However, I'd like to see
  something like my libinputmapper proposal being used for device
  detection and classification.
 
  libinputmapper provides an inmap_evdev object which reads device
  information from an evdev-fd or sysfs /sys/class/input/inputnum
  path, performs some heuristics to classify it and searches it's global
  database for known fixups for broken devices.
  It then provides capabilities to the caller, which allow them to see
  what drivers to load on the device. And it provides a very simple
  mapping table that allows to apply fixup mappings for broken devices.
  These mappings are simple 1-to-1 mappings that are supposed to be
  applied before drivers handle the input. This is to avoid
  device-specific fixup in the drivers and move all this to the
  inputmapper. An example would be a remapping for gamepads that report
  BTN_A instead of BTN_NORTH, but we cannot fix them in the kernel for
  backwards-compatibility reasons. The gamepad-driver can then assume
  that if it receives BTN_NORTH, it is guaranteed to be BTN_NORTH and
  doesn't need to special case xbox360/etc. controllers, because they're
  broken.
 
  I think evdev is exactly that interface and apparently it doesn't work.
 
  if you want a mapping table, you need a per-client table because sooner or
  later you have a client that needs BTN_FOO when the kernel gives you BTN_BAR
  and you can't change the client to fix it.
 
  i.e. the same issue evdev has now, having a global remapping table just
  moves the problem down by 2 years.
 
  a mapping table is good, but you probably want two stages of mapping: one
  that's used in the compositor for truly broken devices that for some reason
  can't be fixed in the kernel, and one that's used on a per-client basis. and
  you'll likely want to be able to overide the client-specific from outside
  the client too.
 
 IMHO, the problem with evdev is, that it doesn't provide device
 classes. The only class we have is this is an input device. All
 other event-flags can be combined in whatever way we want.
 
 So like 10 years ago when the first gamepad driver was introduced, we
 added some mapping that was unique to this device (the device was
 probably unique, too). Some time later, we added some other
 gamepad-like driver with a different mapping (as it was probably a
 very different device-type, back then, and we didn't see it coming
 that this will become a wide-spread device-type).
 However, today we notice that a GamePad is an established type of
 device (like a touchpad), but we have tons of different mappings in
 the kernel for backwards-compatibility reasons. I can see that this
 kind of development can happen again (and very likely it _will_ happen
 again) and it will happen for all kinds of devices.
 
 But that's why I designed the proposal from a compositor's view
 instead of from a kernel's view.
 
 A touchpad driver of the compositor needs to know exactly what kind of
 events it gets from the kernel. If it gets wrong events, it will
 misbehave. As we cannot guarantee that all kernel drivers behave the
 same way, the compositor's touchpad driver needs to work around all
 these little details on a per-device basis.
 To avoid this, I tried to abstract the touchpad-protocol and moved
 per-device handling into a separate library. It detects all devices
 that can serve as a touchpad and fixes trivial (1-to-1 mapping)
 incompatibilities. This removes all per-device handling from the
 touchpad driver and it can expect all input it gets to be conform with
 a touchpad protocol.
 And in fact, it removes this from all the compositor's input drivers.
 So I think of it more like a lib-detect-and-make-compat.
 
 All devices that do not fall into one of the categories (I called it
 capability), will be handled as custom devices. So if we want an input
 driver for a new fancy device, then we need a custom driver, anyway
 (or adjust a generic driver to handle both). If at some point it turns
 out, that this kind of device becomes more established, we can add a
 new capability for it. Or we try extending an existing capability in a
 backwards-compatible way. We can then remove the custom-device
 handling from the input-driver and instead extend/write a generic
 driver for the new capability.
 
 
 So I cannot follow how you think this will have the same problems as
 evdev? Or, let's ask the inverse question: How does this differ from
 the X11 model where we move the custom device handling into the
 drivers?

first of all - I do agree with all of the