Re: [PATCH] wl_shell: Add surface state changed event
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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