Re: [RFC] Common input device library
On Wed, Nov 13, 2013 at 01:22:26PM +0100, Jonas Ådahl wrote: > On Wed, Nov 13, 2013 at 7:22 AM, Peter Hutterer > wrote: > > Hi Jonas, > > > > On Tue, Nov 12, 2013 at 10:50:56PM +0100, Jonas Ådahl wrote: > >> Wayland compositors are expected to deal with input device processing > >> themselves providing input events according to the Wayland protocol to > >> the clients. So far only weston has had more than basic support for > >> processing raw input events from evdev. > >> > >> In order to avoid reimplementing support for various types of input > >> devices over and over again for every compositor, I extracted the input > >> device code from weston and put it in a new library called libinput. > > > > funny timing, I just started on something similar recently :) > > Well, it is needed, so no surprise there are more efforts going on :) > > > > > In my case it was more inspired by the xorg synaptics driver where I keep > > running into issues that can't be fixed without a rewrite. so I started > > with a library that should handle touchpads and can be used by X and > > wayland compositors. > > > > I just committed the current state, polished it a bit and pushed it to > > github: https://github.com/whot/libtouchpad > > > > The eventual goal was that Weston can use that for touchpads, but so far I > > only have a shell X driver for it. Please have a look at that and let me > > know what you think. There's something of an API, the basic design is > > inspired by Weston. > > > > fwiw, one reason I didn't just take evdev-touchpad.c is that I believe that > > it's not possible to write a good touchpad driver without the driver > > tracking and handling each touchpoint separately. weston currently doesn't > > do that. > > I think individual touch point tracking should not be something > required by a touchpad driver mostly because many touchpads don't > report it. As the main author of weston's evdev-touchpad.c I can say > that it does not track touch points individually simply because my > touchpad device doesn't report them as such (and it's not that old of > a laptop). It was not an active design choice not to track them. I did a quick check on the various big laptop makers and I can't find a new model that doesn't have a clickpad (dell gives me 404s but I don't think they do either). My x220 is now about 2 years old and it wasn't the first with a clickpad either. Laptops can last a lot longer of course, but imo we should be focusing on what's coming out now or in the immediate future and maybe tack support for older models on top, rather than the other way round. One reason the xorg synaptics driver is so bad is because it had MT support tacked on top (while trying to support multiple non-evdev backends), especially while MT support in touchpads was less-than-ideal (e.g. the Dell mini 10 touchpad from hell). > > also, I don't yet know if this will or can turn into a generic input > > library. I wanted to get the touchpad case sorted first since it's the most > > complicated. having said that, libtouchpad could be sitting underneath > > a more generic libinput (in design, not as separate library), similar to how > > evdev-touchpad is underneath evdev.c > > Regarding having a libtouchpad, I don't like the idea of separating > touchpad from other input device handling. In weston (and this PoC > libinput) parts of the input processing is designed to be shared > between mouse devices and touchpads for example pointer acceleration, I agree, and I wasn't planning on having pointer acceleration in the touchpad library. It still spits out everything in device coordinate, leaving the rest to the next level. > and possibly other things. I much more prefer the idea of a library > that handles touch devices, touchpads, pointer devices and possibly > others as well. But as you say, your libtouchpad could just be put > inside such a library, but preferably not as an isolated unit. Yeah, let me rephrase: I don't want libtouchpad end up as separate library if it's not required that way. Cheers, Peter > >> The API is very inspired by the internal weston API, but with some > >> simplifications and generalizations. The idea is to make it possible for > >> other compositors to be able to plug it in and receive post-processed > >> events such as pointer motion events, key events, touch events. > >> > >> This does not, however, mean that compositors shouldn't be able to > >> receive raw input events; it's just not there yet as there is no user of > >> such API. > >> > >> While making these changes, I removed some functionality, namely > >> configuring evdev-touchpad via weston.ini. While it should obviously be > >> possible to configure devices, I have yet to made API for doing so. Device > >> detection logging is more limited as well. > > > > fwiw, I don't do any device detection, the current entry point is > > touchpad_open_from_path(), with a open_from_fd() being trivial to add. > > configuration parsing is outside
Re: [PATCH weston 3/8] westoy: Use subsurfaces for tooltips instead of transient windows
On Tue, Nov 12, 2013 at 08:19:59PM -0500, Jasper St. Pierre wrote:
> Transient windows, at least not as they are today, don't exist in
> xdg_shell. Subsurfaces allow for specially placed surfaces relative
> to a window, so use these instead.
> ---
> clients/window.c | 47 ---
> 1 file changed, 8 insertions(+), 39 deletions(-)
Committed, thanks. Trivial merge conflict with the transient change I
made in previous patch.
Kristian
> diff --git a/clients/window.c b/clients/window.c
> index 46372d2..0e5fce2 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -139,7 +139,6 @@ enum {
> TYPE_TOPLEVEL,
> TYPE_FULLSCREEN,
> TYPE_MAXIMIZED,
> - TYPE_TRANSIENT,
> TYPE_MENU,
> TYPE_CUSTOM
> };
> @@ -371,7 +370,6 @@ struct menu {
>
> struct tooltip {
> struct widget *parent;
> - struct window *window;
> struct widget *widget;
> char *entry;
> struct task tooltip_task;
> @@ -1955,6 +1953,7 @@ tooltip_redraw_handler(struct widget *widget, void
> *data)
> int32_t width, height;
>
> cr = widget_cairo_create(widget);
> + cairo_translate(cr, widget->allocation.x, widget->allocation.y);
> cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 0.0);
> cairo_paint(cr);
> @@ -1974,7 +1973,7 @@ tooltip_redraw_handler(struct widget *widget, void
> *data)
> }
>
> static cairo_text_extents_t
> -get_text_extents(struct tooltip *tooltip)
> +get_text_extents(struct display *display, struct tooltip *tooltip)
> {
> cairo_t *cr;
> cairo_text_extents_t extents;
> @@ -1983,7 +1982,7 @@ get_text_extents(struct tooltip *tooltip)
>* created yet, and parent does not have a valid surface
>* outside repaint, either.
>*/
> - cr = cairo_create(tooltip->window->display->dummy_surface);
> + cr = cairo_create(display->dummy_surface);
> cairo_text_extents(cr, tooltip->entry, &extents);
> cairo_destroy(cr);
>
> @@ -1995,7 +1994,6 @@ window_create_tooltip(struct tooltip *tooltip)
> {
> struct widget *parent = tooltip->parent;
> struct display *display = parent->window->display;
> - struct window *window;
> const int offset_y = 27;
> const int margin = 3;
> cairo_text_extents_t extents;
> @@ -2003,18 +2001,13 @@ window_create_tooltip(struct tooltip *tooltip)
> if (tooltip->widget)
> return 0;
>
> - window = window_create_transient(display, parent->window, tooltip->x,
> - tooltip->y + offset_y,
> - WL_SHELL_SURFACE_TRANSIENT_INACTIVE);
> - if (!window)
> - return -1;
> -
> - tooltip->window = window;
> - tooltip->widget = window_add_widget(tooltip->window, tooltip);
> + tooltip->widget = window_add_subsurface(parent->window, tooltip,
> SUBSURFACE_DESYNCHRONIZED);
>
> - extents = get_text_extents(tooltip);
> + extents = get_text_extents(display, tooltip);
> widget_set_redraw_handler(tooltip->widget, tooltip_redraw_handler);
> - window_schedule_resize(window, extents.width + 20, 20 + margin * 2);
> + widget_set_allocation(tooltip->widget,
> + tooltip->x, tooltip->y + offset_y,
> + extents.width + 20, 20 + margin * 2);
>
> return 0;
> }
> @@ -2030,9 +2023,7 @@ widget_destroy_tooltip(struct widget *parent)
>
> if (tooltip->widget) {
> widget_destroy(tooltip->widget);
> - window_destroy(tooltip->window);
> tooltip->widget = NULL;
> - tooltip->window = NULL;
> }
>
> close(tooltip->tooltip_fd);
> @@ -2096,7 +2087,6 @@ widget_set_tooltip(struct widget *parent, char *entry,
> float x, float y)
> parent->tooltip = tooltip;
> tooltip->parent = parent;
> tooltip->widget = NULL;
> - tooltip->window = NULL;
> tooltip->x = x;
> tooltip->y = y;
> tooltip->entry = strdup(entry);
> @@ -4287,27 +4277,6 @@ window_create_custom(struct display *display)
> return window_create_internal(display, TYPE_CUSTOM);
> }
>
> -struct window *
> -window_create_transient(struct display *display, struct window *parent,
> - int32_t x, int32_t y, uint32_t flags)
> -{
> - struct window *window;
> -
> - window = window_create_internal(parent->display,
> - parent, TYPE_TRANSIENT);
> -
> - window->x = x;
> - window->y = y;
> -
> - if (display->shell)
> - wl_shell_surface_set_transient(
> - window->shell_surface,
> - window->parent->main_surface->surface,
> - window->x, window->y, flags);
> -
> - return window;
> -}
> -
> static void
> menu_set_item(struct menu *menu, int sy)
> {
> --
> 1.8.4.2
>
> __
Re: [PATCH weston 4/8] westoy: Remove some accessors for wl_shell / wl_shell_surface
On Tue, Nov 12, 2013 at 08:20:00PM -0500, Jasper St. Pierre wrote:
> We want to remove support for these deprecated interfaces. Since
> nothing is using them, this is a simple change.
Applied, thanks.
Kristian
> ---
> clients/window.c | 12
> clients/window.h | 6 --
> 2 files changed, 18 deletions(-)
>
> diff --git a/clients/window.c b/clients/window.c
> index 0e5fce2..29ca42d 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -1938,12 +1938,6 @@ window_get_wl_surface(struct window *window)
> return window->main_surface->surface;
> }
>
> -struct wl_shell_surface *
> -window_get_wl_shell_surface(struct window *window)
> -{
> - return window->shell_surface;
> -}
> -
> static void
> tooltip_redraw_handler(struct widget *widget, void *data)
> {
> @@ -5224,12 +5218,6 @@ display_get_argb_egl_config(struct display *d)
> return d->argb_config;
> }
>
> -struct wl_shell *
> -display_get_shell(struct display *display)
> -{
> - return display->shell;
> -}
> -
> int
> display_acquire_window_surface(struct display *display,
> struct window *window,
> diff --git a/clients/window.h b/clients/window.h
> index d5e40ed..838ea4e 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -87,9 +87,6 @@ display_get_cairo_device(struct display *display);
> struct wl_compositor *
> display_get_compositor(struct display *display);
>
> -struct wl_shell *
> -display_get_shell(struct display *display);
> -
> struct output *
> display_get_output(struct display *display);
>
> @@ -349,9 +346,6 @@ window_get_surface(struct window *window);
> struct wl_surface *
> window_get_wl_surface(struct window *window);
>
> -struct wl_shell_surface *
> -window_get_wl_shell_surface(struct window *window);
> -
> enum window_buffer_type {
> WINDOW_BUFFER_TYPE_EGL_WINDOW,
> WINDOW_BUFFER_TYPE_SHM,
> --
> 1.8.4.2
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/8] westoy: Remove unused support for window parents
On Tue, Nov 12, 2013 at 08:19:58PM -0500, Jasper St. Pierre wrote:
> It seems that this was only used by the popup menu infrastructure,
> which can handle this all on its own. Implementing e.g. transients
> in the future can be done with a simple xdg_shell_set_transient_for.
> ---
> clients/image.c| 2 +-
> clients/terminal.c | 2 +-
> clients/view.c | 2 +-
> clients/window.c | 28
> clients/window.h | 2 +-
> 5 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/clients/image.c b/clients/image.c
> index c73d0c0..3a52c22 100644
> --- a/clients/image.c
> +++ b/clients/image.c
> @@ -336,7 +336,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct image *image = data;
>
> diff --git a/clients/terminal.c b/clients/terminal.c
> index a321a1e..d09f94b 100644
> --- a/clients/terminal.c
> +++ b/clients/terminal.c
> @@ -2212,7 +2212,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct terminal *terminal = data;
>
> diff --git a/clients/view.c b/clients/view.c
> index cedef08..4ac9ca5 100644
> --- a/clients/view.c
> +++ b/clients/view.c
> @@ -168,7 +168,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct view *view = data;
>
> diff --git a/clients/window.c b/clients/window.c
> index 0afd46b..46372d2 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -221,7 +221,6 @@ struct surface {
>
> struct window {
> struct display *display;
> - struct window *parent;
> struct wl_list window_output_list;
> char *title;
> struct rectangle saved_allocation;
> @@ -358,6 +357,7 @@ struct window_frame {
>
> struct menu {
> struct window *window;
> + struct window *parent;
> struct widget *widget;
> struct input *input;
> struct frame *frame;
> @@ -2274,8 +2274,7 @@ frame_menu_func(struct window *window,
> switch (index) {
> case 0: /* close */
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> break;
> @@ -2392,8 +2391,7 @@ frame_handle_status(struct window_frame *frame, struct
> input *input,
>
> if (status & FRAME_STATUS_CLOSE) {
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> return;
> @@ -2894,8 +2892,7 @@ keyboard_handle_key(void *data, struct wl_keyboard
> *keyboard,
> input->modifiers == MOD_ALT_MASK &&
> state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> } else if (window->key_handler) {
> @@ -4230,8 +4227,7 @@ surface_create(struct window *window)
> }
>
> static struct window *
> -window_create_internal(struct display *display,
> -struct window *parent, int type)
> +window_create_internal(struct display *display, int type)
You didn't fix up window_create_transient() to work with this change,
the patch doesn't compile. I ammended the obvious fix.
Kristian
> {
> struct window *window;
> struct surface *surface;
> @@ -4239,7 +4235,6 @@ window_create_internal(struct display *display,
> window = xzalloc(sizeof *window);
> wl_list_init(&window->subsurface_list);
> window->display = display;
> - window->parent = parent;
>
> surface = surface_create(window);
> window->main_surface = surface;
> @@ -4283,13 +4278,13 @@ window_create_internal(struct display *display,
> struct window *
> window_create(struct display *display)
> {
> - return window_create_internal(display, NULL, TYPE_NONE);
> + return window_create_internal(display, TYPE_NONE);
> }
>
> struct window *
> window_create_custom(struct display *display)
> {
> - return window_create_internal(display, NULL, TYPE_CUSTOM);
> + return window_create_internal(display, TYPE_CUSTOM);
> }
>
> struct window *
> @@ -4374,8 +4369,8 @@ menu_button_handl
Re: [PATCH weston 2/8] westoy: Remove unused support for window parents
On Tue, Nov 12, 2013 at 08:19:58PM -0500, Jasper St. Pierre wrote:
> It seems that this was only used by the popup menu infrastructure,
> which can handle this all on its own. Implementing e.g. transients
> in the future can be done with a simple xdg_shell_set_transient_for.
Yeah, sounds good.
Kristian
> clients/image.c| 2 +-
> clients/terminal.c | 2 +-
> clients/view.c | 2 +-
> clients/window.c | 28
> clients/window.h | 2 +-
> 5 files changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/clients/image.c b/clients/image.c
> index c73d0c0..3a52c22 100644
> --- a/clients/image.c
> +++ b/clients/image.c
> @@ -336,7 +336,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct image *image = data;
>
> diff --git a/clients/terminal.c b/clients/terminal.c
> index a321a1e..d09f94b 100644
> --- a/clients/terminal.c
> +++ b/clients/terminal.c
> @@ -2212,7 +2212,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct terminal *terminal = data;
>
> diff --git a/clients/view.c b/clients/view.c
> index cedef08..4ac9ca5 100644
> --- a/clients/view.c
> +++ b/clients/view.c
> @@ -168,7 +168,7 @@ fullscreen_handler(struct window *window, void *data)
> }
>
> static void
> -close_handler(struct window *window, void *data)
> +close_handler(void *data)
> {
> struct view *view = data;
>
> diff --git a/clients/window.c b/clients/window.c
> index 0afd46b..46372d2 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -221,7 +221,6 @@ struct surface {
>
> struct window {
> struct display *display;
> - struct window *parent;
> struct wl_list window_output_list;
> char *title;
> struct rectangle saved_allocation;
> @@ -358,6 +357,7 @@ struct window_frame {
>
> struct menu {
> struct window *window;
> + struct window *parent;
> struct widget *widget;
> struct input *input;
> struct frame *frame;
> @@ -2274,8 +2274,7 @@ frame_menu_func(struct window *window,
> switch (index) {
> case 0: /* close */
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> break;
> @@ -2392,8 +2391,7 @@ frame_handle_status(struct window_frame *frame, struct
> input *input,
>
> if (status & FRAME_STATUS_CLOSE) {
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> return;
> @@ -2894,8 +2892,7 @@ keyboard_handle_key(void *data, struct wl_keyboard
> *keyboard,
> input->modifiers == MOD_ALT_MASK &&
> state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> if (window->close_handler)
> - window->close_handler(window->parent,
> - window->user_data);
> + window->close_handler(window->user_data);
> else
> display_exit(window->display);
> } else if (window->key_handler) {
> @@ -4230,8 +4227,7 @@ surface_create(struct window *window)
> }
>
> static struct window *
> -window_create_internal(struct display *display,
> -struct window *parent, int type)
> +window_create_internal(struct display *display, int type)
> {
> struct window *window;
> struct surface *surface;
> @@ -4239,7 +4235,6 @@ window_create_internal(struct display *display,
> window = xzalloc(sizeof *window);
> wl_list_init(&window->subsurface_list);
> window->display = display;
> - window->parent = parent;
>
> surface = surface_create(window);
> window->main_surface = surface;
> @@ -4283,13 +4278,13 @@ window_create_internal(struct display *display,
> struct window *
> window_create(struct display *display)
> {
> - return window_create_internal(display, NULL, TYPE_NONE);
> + return window_create_internal(display, TYPE_NONE);
> }
>
> struct window *
> window_create_custom(struct display *display)
> {
> - return window_create_internal(display, NULL, TYPE_CUSTOM);
> + return window_create_internal(display, TYPE_CUSTOM);
> }
>
> struct window *
> @@ -4374,8 +4369,8 @@ menu_button_handler(struct widget *widget,
> (menu->release_count > 0 || time - menu->time > 500)) {
> /*
Re: [PATCH weston 1/8] westoy: Remove window_touch_move
On Tue, Nov 12, 2013 at 08:19:57PM -0500, Jasper St. Pierre wrote:
> It seems to be the same as window_move, except it ignores the passed
> in serial (???) and instead just uses the one of the display.
Ok, hmm, yes, that's a little odd. Patch committed, thanks.
Kristian
> clients/flower.c | 3 +--
> clients/fullscreen.c | 3 +--
> clients/transformed.c | 3 +--
> clients/window.c | 10 --
> clients/window.h | 2 --
> 5 files changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/clients/flower.c b/clients/flower.c
> index 825c833..389f8be 100644
> --- a/clients/flower.c
> +++ b/clients/flower.c
> @@ -158,8 +158,7 @@ touch_down_handler(struct widget *widget, struct input
> *input,
> float x, float y, void *data)
> {
> struct flower *flower = data;
> - window_touch_move(flower->window, input,
> - display_get_serial(flower->display));
> + window_move(flower->window, input, display_get_serial(flower->display));
> }
>
> int main(int argc, char *argv[])
> diff --git a/clients/fullscreen.c b/clients/fullscreen.c
> index 72e2c81..034a352 100644
> --- a/clients/fullscreen.c
> +++ b/clients/fullscreen.c
> @@ -283,8 +283,7 @@ touch_handler(struct widget *widget, struct input *input,
> float x, float y, void *data)
> {
> struct fullscreen *fullscreen = data;
> - window_touch_move(fullscreen->window, input,
> - display_get_serial(fullscreen->display));
> + window_move(fullscreen->window, input,
> display_get_serial(fullscreen->display));
> }
>
> static void
> diff --git a/clients/transformed.c b/clients/transformed.c
> index 54212dd..bbc1dc0 100644
> --- a/clients/transformed.c
> +++ b/clients/transformed.c
> @@ -227,8 +227,7 @@ touch_handler(struct widget *widget, struct input *input,
> float x, float y, void *data)
> {
> struct transformed *transformed = data;
> - window_touch_move(transformed->window, input,
> - display_get_serial(transformed->display));
> + window_move(transformed->window, input,
> display_get_serial(transformed->display));
> }
>
> static void
> diff --git a/clients/window.c b/clients/window.c
> index b703656..0afd46b 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -3590,16 +3590,6 @@ window_move(struct window *window, struct input
> *input, uint32_t serial)
> wl_shell_surface_move(window->shell_surface, input->seat, serial);
> }
>
> -void
> -window_touch_move(struct window *window, struct input *input, uint32_t
> serial)
> -{
> - if (!window->shell_surface)
> - return;
> -
> - wl_shell_surface_move(window->shell_surface, input->seat,
> - window->display->serial);
> -}
> -
> static void
> surface_set_synchronized(struct surface *surface)
> {
> diff --git a/clients/window.h b/clients/window.h
> index ff7c87b..59e483e 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -333,8 +333,6 @@ window_get_display(struct window *window);
> void
> window_move(struct window *window, struct input *input, uint32_t time);
> void
> -window_touch_move(struct window *window, struct input *input, uint32_t time);
> -void
> window_get_allocation(struct window *window, struct rectangle *allocation);
> void
> window_schedule_redraw(struct window *window);
> --
> 1.8.4.2
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v2] server: Add API to protect access to an SHM buffer
On Wed, Nov 13, 2013 at 03:32:05PM +, Neil Roberts wrote:
> Ok, here is a second version of the patch which leaves the signal
> handler permanently installed and uses thread-local storage to keep
> track of the current pool.
Yeah, that looks good now, both patches pushed.
thanks,
Kristian
> Regards,
> - Neil
>
> --- >8 --- (use git am --scissors to automatically chop here)
>
> Linux will let you mmap a region of a file that is larger than the
> size of the file. If you then try to read from that region the process
> will get a SIGBUS signal. Currently the clients can use this to crash
> a compositor because it can create a pool and lie about the size of
> the file which will cause the compositor to try and read past the end
> of it. The compositor can't simply check the size of the file to
> verify that it is big enough because then there is a race condition
> where the client may truncate the file after the check is performed.
>
> This patch adds the following two public functions in the server API
> which can be used wrap access to an SHM buffer:
>
> void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
> void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
>
> The first time wl_shm_buffer_begin_access is called a signal handler
> for SIGBUS will be installed. If the signal is caught then the buffer
> for the current pool is remapped to an anonymous private buffer at the
> same address which allows the compositor to continue without crashing.
> The end_access function will then post an error to the buffer
> resource.
>
> The current pool is stored as part of some thread-local storage so
> that multiple threads can safely independently access separate
> buffers.
>
> Eventually we may want to add some more API so that compositors can
> hook into the signal handler or replace it entirely if they also want
> to do some SIGBUS handling.
> ---
> src/wayland-server.h | 6 +++
> src/wayland-shm.c| 131
> +++
> 2 files changed, 137 insertions(+)
>
> diff --git a/src/wayland-server.h b/src/wayland-server.h
> index 36c9a15..f5427fd 100644
> --- a/src/wayland-server.h
> +++ b/src/wayland-server.h
> @@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource
> *resource,
>
> struct wl_shm_buffer;
>
> +void
> +wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
> +
> +void
> +wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
> +
> struct wl_shm_buffer *
> wl_shm_buffer_get(struct wl_resource *resource);
>
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index eff29c3..28f52f4 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -32,10 +32,20 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
>
> #include "wayland-private.h"
> #include "wayland-server.h"
>
> +/* This once_t is used to synchronize installing the SIGBUS handler
> + * and creating the TLS key. This will be done in the first call
> + * wl_shm_buffer_begin_access which can happen from any thread */
> +static pthread_once_t wl_shm_sigbus_once = PTHREAD_ONCE_INIT;
> +static pthread_key_t wl_shm_sigbus_data_key;
> +static struct sigaction wl_shm_old_sigbus_action;
> +
> struct wl_shm_pool {
> struct wl_resource *resource;
> int refcount;
> @@ -52,6 +62,12 @@ struct wl_shm_buffer {
> struct wl_shm_pool *pool;
> };
>
> +struct wl_shm_sigbus_data {
> + struct wl_shm_pool *current_pool;
> + int access_count;
> + int fallback_mapping_used;
> +};
> +
> static void
> shm_pool_unref(struct wl_shm_pool *pool)
> {
> @@ -368,3 +384,118 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
> {
> return buffer->height;
> }
> +
> +static void
> +reraise_sigbus(void)
> +{
> + /* If SIGBUS is raised for some other reason than accessing
> + * the pool then we'll uninstall the signal handler so we can
> + * reraise it. This would presumably kill the process */
> + sigaction(SIGBUS, &wl_shm_old_sigbus_action, NULL);
> + raise(SIGBUS);
> +}
> +
> +static void
> +sigbus_handler(int signum, siginfo_t *info, void *context)
> +{
> + struct wl_shm_sigbus_data *sigbus_data =
> + pthread_getspecific(wl_shm_sigbus_data_key);
> + struct wl_shm_pool *pool;
> +
> + if (sigbus_data == NULL) {
> + reraise_sigbus();
> + return;
> + }
> +
> + pool = sigbus_data->current_pool;
> +
> + /* If the offending address is outside the mapped space for
> + * the pool then the error is a real problem so we'll reraise
> + * the signal */
> + if (pool == NULL ||
> + (char *) info->si_addr < pool->data ||
> + (char *) info->si_addr >= pool->data + pool->size) {
> + reraise_sigbus();
> + return;
> + }
> +
> + sigbus_data->fallback_mapping_used = 1;
> +
> + /* This should replace the previous mapping */
> + if (mmap(pool->da
Re: [PATCH weston 7/8] westoy: Schedule a full resize when we add a subsurface
On Wed, Nov 13, 2013 at 3:41 PM, Bill Spitzak wrote: > Is this to force the commit so that the subsurface appears? It might be > better to have a direct way to get the commit to happen without redrawing > the main surface. > It's to make sure that the transient's position and size is set, and that its redraw handler is called and that the subsurface is placed accordingly. I don't think we care too much about toytoolkit efficiently, so I'm fine with queueing a full redraw. Feel free to fix it if you feel strongly about making toytoolkit performant. > In any case this looks a lot like you are hiding a bug that perhaps should > be investigated, especially if it is in weston or your compositor, rather > than in the toytoolkit. > It is not a bug in the compositor. It's a bug in toytoolkit. Jasper St. Pierre wrote: > >> If a client adds a subsurface, we need to make sure it's allocated >> properly, so queue a resize and redraw on the parent window. >> --- >> clients/window.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/clients/window.c b/clients/window.c >> index 5a1c8ec..df56bd9 100644 >> --- a/clients/window.c >> +++ b/clients/window.c >> @@ -4462,6 +4462,9 @@ window_add_subsurface(struct window *window, void >> *data, >> assert(!"bad enum subsurface_mode"); >> } >> + window->resize_needed = 1; >> + window_schedule_redraw(window); >> + >> return widget; >> } >> >> > -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/8] westoy: Use subsurfaces for tooltips instead of transient windows
On Wed, Nov 13, 2013 at 3:36 PM, Bill Spitzak wrote: > Jasper St. Pierre wrote: > > Transient windows, at least not as they are today, don't exist in >> xdg_shell. Subsurfaces allow for specially placed surfaces relative >> to a window, so use these instead. >> > > So you noticed that subsurfaces and transient windows are very similer, > huh? :-) > > In all seriousness, I strongly suggest that these concepts be merged. > There is no difference except the compositor is not allowed to insert other > surfaces in the stacking between a subsurface and it's parent, while it is > allowed to do this for a transient. > Transients, as they existed in wl_shell_surface, have been removed from xdg_shell and aren't coming back. There is a new set_transient_for request, but it's more to provide something like the ICCCM WM_TRANSIENT_FOR property: associating modal dialogs with their "parent window". -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/8] westoy: Remove window_touch_move
On Wed, Nov 13, 2013 at 3:37 PM, Bill Spitzak wrote: > Did you see my suggestion that the move and resize requests also be merged? > Yes, but I don't see what that has to do with this patch. This is about toy toolkit cleanup... > Jasper St. Pierre wrote: > >> It seems to be the same as window_move, except it ignores the passed >> in serial (???) and instead just uses the one of the display. >> > -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 7/8] westoy: Schedule a full resize when we add a subsurface
Is this to force the commit so that the subsurface appears? It might be better to have a direct way to get the commit to happen without redrawing the main surface. In any case this looks a lot like you are hiding a bug that perhaps should be investigated, especially if it is in weston or your compositor, rather than in the toytoolkit. Jasper St. Pierre wrote: If a client adds a subsurface, we need to make sure it's allocated properly, so queue a resize and redraw on the parent window. --- clients/window.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clients/window.c b/clients/window.c index 5a1c8ec..df56bd9 100644 --- a/clients/window.c +++ b/clients/window.c @@ -4462,6 +4462,9 @@ window_add_subsurface(struct window *window, void *data, assert(!"bad enum subsurface_mode"); } + window->resize_needed = 1; + window_schedule_redraw(window); + return widget; } ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/8] westoy: Remove window_touch_move
Did you see my suggestion that the move and resize requests also be merged? Jasper St. Pierre wrote: It seems to be the same as window_move, except it ignores the passed in serial (???) and instead just uses the one of the display. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/8] westoy: Use subsurfaces for tooltips instead of transient windows
Jasper St. Pierre wrote: Transient windows, at least not as they are today, don't exist in xdg_shell. Subsurfaces allow for specially placed surfaces relative to a window, so use these instead. So you noticed that subsurfaces and transient windows are very similer, huh? :-) In all seriousness, I strongly suggest that these concepts be merged. There is no difference except the compositor is not allowed to insert other surfaces in the stacking between a subsurface and it's parent, while it is allowed to do this for a transient. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] surface crop & scale protocol extension
Pekka Paalanen wrote: Is your whole issue based on the premise, that the output resolution is not a multiple of output_scale? I think there is some serious confusion here. Everything here is a multiple of everything else. My client is attempting to obey the output_scale on the assumption that this advertised output_scale will produce the best image. It sees an output_scale of 3 and assumes the reason is that the pixels on that output are 3x smaller (1/9 the area) of "standard" pixels. The client says "If I want to produce the best hi-resolution image, I need to use a buffer that is 3x larger in each direction and use a buffer_scale of 1/3". Then the client says "I want to use a subsurface and the crop+scale api to blow this 512x512 image up to cover exactly 1024x1024 pixels in my main buffer" (on the assumption that this is also 1024x1024 pixels on the output). However instead of sending a 1024x1024 square to the compositor for the dst area, it has to send a 341.333 square using fixed-point. This requires everybody to agree on rounding rules, and is misleading because the crop+scale only will work for integer numbers of pixels. It also has to set the buffer_scale of the subsurface to 1, otherwise it is impossible to specify a 512x512 source rectangle because that is not using fixed point. Please explain if I have gotten this analysis wrong. My recommendation: buffer_scale is ignored for the crop rectangle (which should then be in the same space as the buffer width and height values, which I think is "after" the buffer_transform, not before it). That allows buffer_scale to be applied to the dst rectangle. The client would then set the buffer_scale of the subsurface to 1/3 and can set the dst rectangle to 1024. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] Add calls to wl_shm_buffer_begin/end_access
Thanks for the feedback. Here is a version two of the patch which
fixes some merge conflicts on master and adds support for the pixman
renderer.
Kristian wrote:
> As for the pixman renderer it should be a matter of just wrapping
> the calls to pixman_image_composite32() in
> pixman_renderer_read_pixels() and around the last if/else in
> draw_view() where we end up calling repaint_region() in either
> branch.
I think the pixman_renderer_read_pixels case doesn't need the wrapper
functions because it is reading from the output's buffer into another
malloc'd buffer. Both of those are allocated by the compositor and
shouldn't cause any problems.
I put the wrapper calls inside repaint_region in order to minimise the
length of time that the wrapper is enabled.
Regards,
- Neil
--- >8 --- (use git am --scissors to automatically chop here)
This wraps all accesses to an SHM buffer between wl_shm_buffer_begin
and end so that wayland-shm can install a handler for SIGBUS and catch
attempts to pass the compositor a buffer that is too small.
---
src/compositor-drm.c | 11 ---
src/gl-renderer.c | 6 ++
src/pixman-renderer.c | 6 ++
src/rpi-renderer.c| 4
src/screenshooter.c | 4
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index b929728..d4fc871 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -974,6 +974,7 @@ static void
drm_output_set_cursor(struct drm_output *output)
{
struct weston_view *ev = output->cursor_view;
+ struct weston_buffer *buffer;
struct drm_compositor *c =
(struct drm_compositor *) output->base.compositor;
EGLint handle, stride;
@@ -988,18 +989,22 @@ drm_output_set_cursor(struct drm_output *output)
return;
}
- if (ev->surface->buffer_ref.buffer &&
+ buffer = ev->surface->buffer_ref.buffer;
+
+ if (buffer &&
pixman_region32_not_empty(&output->cursor_plane.damage)) {
pixman_region32_fini(&output->cursor_plane.damage);
pixman_region32_init(&output->cursor_plane.damage);
output->current_cursor ^= 1;
bo = output->cursor_bo[output->current_cursor];
memset(buf, 0, sizeof buf);
- stride =
wl_shm_buffer_get_stride(ev->surface->buffer_ref.buffer->shm_buffer);
- s =
wl_shm_buffer_get_data(ev->surface->buffer_ref.buffer->shm_buffer);
+ stride = wl_shm_buffer_get_stride(buffer->shm_buffer);
+ s = wl_shm_buffer_get_data(buffer->shm_buffer);
+ wl_shm_buffer_begin_access(buffer->shm_buffer);
for (i = 0; i < ev->geometry.height; i++)
memcpy(buf + i * 64, s + i * stride,
ev->geometry.width * 4);
+ wl_shm_buffer_end_access(buffer->shm_buffer);
if (gbm_bo_write(bo, buf, sizeof buf) < 0)
weston_log("failed update cursor: %m\n");
diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index 68a071f..7a535c7 100644
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -899,10 +899,12 @@ gl_renderer_flush_damage(struct weston_surface *surface)
glBindTexture(GL_TEXTURE_2D, gs->textures[0]);
if (!gr->has_unpack_subimage) {
+ wl_shm_buffer_begin_access(buffer->shm_buffer);
glTexImage2D(GL_TEXTURE_2D, 0, format,
gs->pitch, buffer->height, 0,
format, pixel_type,
wl_shm_buffer_get_data(buffer->shm_buffer));
+ wl_shm_buffer_end_access(buffer->shm_buffer);
goto done;
}
@@ -914,13 +916,16 @@ gl_renderer_flush_damage(struct weston_surface *surface)
if (gs->needs_full_upload) {
glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
+ wl_shm_buffer_begin_access(buffer->shm_buffer);
glTexSubImage2D(GL_TEXTURE_2D, 0,
0, 0, gs->pitch, buffer->height,
format, pixel_type, data);
+ wl_shm_buffer_end_access(buffer->shm_buffer);
goto done;
}
rectangles = pixman_region32_rectangles(&gs->texture_damage, &n);
+ wl_shm_buffer_begin_access(buffer->shm_buffer);
for (i = 0; i < n; i++) {
pixman_box32_t r;
@@ -932,6 +937,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
r.x2 - r.x1, r.y2 - r.y1,
format, pixel_type, data);
}
+ wl_shm_buffer_end_access(buffer->shm_buffer);
#endif
done:
diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
index a80be5f..b719829 100644
--- a/src/pixman-renderer.c
+++ b/src/pixman-renderer.c
@@ -305,6 +305,
[PATCH v2] server: Add API to protect access to an SHM buffer
Ok, here is a second version of the patch which leaves the signal
handler permanently installed and uses thread-local storage to keep
track of the current pool.
Regards,
- Neil
--- >8 --- (use git am --scissors to automatically chop here)
Linux will let you mmap a region of a file that is larger than the
size of the file. If you then try to read from that region the process
will get a SIGBUS signal. Currently the clients can use this to crash
a compositor because it can create a pool and lie about the size of
the file which will cause the compositor to try and read past the end
of it. The compositor can't simply check the size of the file to
verify that it is big enough because then there is a race condition
where the client may truncate the file after the check is performed.
This patch adds the following two public functions in the server API
which can be used wrap access to an SHM buffer:
void wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
void wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
The first time wl_shm_buffer_begin_access is called a signal handler
for SIGBUS will be installed. If the signal is caught then the buffer
for the current pool is remapped to an anonymous private buffer at the
same address which allows the compositor to continue without crashing.
The end_access function will then post an error to the buffer
resource.
The current pool is stored as part of some thread-local storage so
that multiple threads can safely independently access separate
buffers.
Eventually we may want to add some more API so that compositors can
hook into the signal handler or replace it entirely if they also want
to do some SIGBUS handling.
---
src/wayland-server.h | 6 +++
src/wayland-shm.c| 131 +++
2 files changed, 137 insertions(+)
diff --git a/src/wayland-server.h b/src/wayland-server.h
index 36c9a15..f5427fd 100644
--- a/src/wayland-server.h
+++ b/src/wayland-server.h
@@ -412,6 +412,12 @@ wl_resource_get_destroy_listener(struct wl_resource
*resource,
struct wl_shm_buffer;
+void
+wl_shm_buffer_begin_access(struct wl_shm_buffer *buffer);
+
+void
+wl_shm_buffer_end_access(struct wl_shm_buffer *buffer);
+
struct wl_shm_buffer *
wl_shm_buffer_get(struct wl_resource *resource);
diff --git a/src/wayland-shm.c b/src/wayland-shm.c
index eff29c3..28f52f4 100644
--- a/src/wayland-shm.c
+++ b/src/wayland-shm.c
@@ -32,10 +32,20 @@
#include
#include
#include
+#include
+#include
+#include
#include "wayland-private.h"
#include "wayland-server.h"
+/* This once_t is used to synchronize installing the SIGBUS handler
+ * and creating the TLS key. This will be done in the first call
+ * wl_shm_buffer_begin_access which can happen from any thread */
+static pthread_once_t wl_shm_sigbus_once = PTHREAD_ONCE_INIT;
+static pthread_key_t wl_shm_sigbus_data_key;
+static struct sigaction wl_shm_old_sigbus_action;
+
struct wl_shm_pool {
struct wl_resource *resource;
int refcount;
@@ -52,6 +62,12 @@ struct wl_shm_buffer {
struct wl_shm_pool *pool;
};
+struct wl_shm_sigbus_data {
+ struct wl_shm_pool *current_pool;
+ int access_count;
+ int fallback_mapping_used;
+};
+
static void
shm_pool_unref(struct wl_shm_pool *pool)
{
@@ -368,3 +384,118 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
{
return buffer->height;
}
+
+static void
+reraise_sigbus(void)
+{
+ /* If SIGBUS is raised for some other reason than accessing
+* the pool then we'll uninstall the signal handler so we can
+* reraise it. This would presumably kill the process */
+ sigaction(SIGBUS, &wl_shm_old_sigbus_action, NULL);
+ raise(SIGBUS);
+}
+
+static void
+sigbus_handler(int signum, siginfo_t *info, void *context)
+{
+ struct wl_shm_sigbus_data *sigbus_data =
+ pthread_getspecific(wl_shm_sigbus_data_key);
+ struct wl_shm_pool *pool;
+
+ if (sigbus_data == NULL) {
+ reraise_sigbus();
+ return;
+ }
+
+ pool = sigbus_data->current_pool;
+
+ /* If the offending address is outside the mapped space for
+* the pool then the error is a real problem so we'll reraise
+* the signal */
+ if (pool == NULL ||
+ (char *) info->si_addr < pool->data ||
+ (char *) info->si_addr >= pool->data + pool->size) {
+ reraise_sigbus();
+ return;
+ }
+
+ sigbus_data->fallback_mapping_used = 1;
+
+ /* This should replace the previous mapping */
+ if (mmap(pool->data, pool->size,
+PROT_READ | PROT_WRITE,
+MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS,
+0, 0) == (void *) -1) {
+ reraise_sigbus();
+ return;
+ }
+}
+
+static void
+destroy_sigbus_data(void *data)
+{
+ struct wl_shm_sigbus_data *sigbus_data = data;
+
+ free(si
Re: [RFC] Common input device library
On Wed, Nov 13, 2013 at 7:22 AM, Peter Hutterer wrote: > Hi Jonas, > > On Tue, Nov 12, 2013 at 10:50:56PM +0100, Jonas Ådahl wrote: >> Wayland compositors are expected to deal with input device processing >> themselves providing input events according to the Wayland protocol to >> the clients. So far only weston has had more than basic support for >> processing raw input events from evdev. >> >> In order to avoid reimplementing support for various types of input >> devices over and over again for every compositor, I extracted the input >> device code from weston and put it in a new library called libinput. > > funny timing, I just started on something similar recently :) Well, it is needed, so no surprise there are more efforts going on :) > > In my case it was more inspired by the xorg synaptics driver where I keep > running into issues that can't be fixed without a rewrite. so I started > with a library that should handle touchpads and can be used by X and > wayland compositors. > > I just committed the current state, polished it a bit and pushed it to > github: https://github.com/whot/libtouchpad > > The eventual goal was that Weston can use that for touchpads, but so far I > only have a shell X driver for it. Please have a look at that and let me > know what you think. There's something of an API, the basic design is > inspired by Weston. > > fwiw, one reason I didn't just take evdev-touchpad.c is that I believe that > it's not possible to write a good touchpad driver without the driver > tracking and handling each touchpoint separately. weston currently doesn't > do that. I think individual touch point tracking should not be something required by a touchpad driver mostly because many touchpads don't report it. As the main author of weston's evdev-touchpad.c I can say that it does not track touch points individually simply because my touchpad device doesn't report them as such (and it's not that old of a laptop). It was not an active design choice not to track them. > > also, I don't yet know if this will or can turn into a generic input > library. I wanted to get the touchpad case sorted first since it's the most > complicated. having said that, libtouchpad could be sitting underneath > a more generic libinput (in design, not as separate library), similar to how > evdev-touchpad is underneath evdev.c Regarding having a libtouchpad, I don't like the idea of separating touchpad from other input device handling. In weston (and this PoC libinput) parts of the input processing is designed to be shared between mouse devices and touchpads for example pointer acceleration, and possibly other things. I much more prefer the idea of a library that handles touch devices, touchpads, pointer devices and possibly others as well. But as you say, your libtouchpad could just be put inside such a library, but preferably not as an isolated unit. > >> The API is very inspired by the internal weston API, but with some >> simplifications and generalizations. The idea is to make it possible for >> other compositors to be able to plug it in and receive post-processed >> events such as pointer motion events, key events, touch events. >> >> This does not, however, mean that compositors shouldn't be able to >> receive raw input events; it's just not there yet as there is no user of >> such API. >> >> While making these changes, I removed some functionality, namely >> configuring evdev-touchpad via weston.ini. While it should obviously be >> possible to configure devices, I have yet to made API for doing so. Device >> detection logging is more limited as well. > > fwiw, I don't do any device detection, the current entry point is > touchpad_open_from_path(), with a open_from_fd() being trivial to add. > configuration parsing is outside the library, but I have a few small hooks > to change tapping/scrolling parameters (not happy with the API here yet > though) I had the idea that maybe configuration could just be done via dbus, but then again, I wouldn't want random applications changing these parameters so might still be better with some API in the library. I too have not thought of a reasonable API for this. > >> >> The repository of libinput can currently be found here [0]. It is a >> history rewrite of the weston repository, so the history of related files >> are still intact. >> >> I have created patches for weston (that I will submit by replying to this >> E-mail) for consideration. I have tested this with a regular pointer >> device, keyboard, touchpad, but not with a touch device. >> >> This is more or less a RFC about this approach, and any input would be >> appreciated. > > IMO a separate library for the input handling is needed, thanks for > starting that effort. If we can sync up how to progress from here and if we > can find a common goal, that'd be great. Agreed. > > Cheers, >Peter > Cheers, Jonas ___ wayland-devel mailing list wayland-devel@lists.freedeskto
Re: [RFC] Common input device library
On Wed, Nov 13, 2013 at 4:00 AM, Christopher James Halse Rogers wrote: > On Tue, 2013-11-12 at 22:50 +0100, Jonas Ådahl wrote: >> Hi, >> >> Wayland compositors are expected to deal with input device processing >> themselves providing input events according to the Wayland protocol to >> the clients. So far only weston has had more than basic support for >> processing raw input events from evdev. >> >> In order to avoid reimplementing support for various types of input >> devices over and over again for every compositor, I extracted the input >> device code from weston and put it in a new library called libinput. >> >> The API is very inspired by the internal weston API, but with some >> simplifications and generalizations. The idea is to make it possible for >> other compositors to be able to plug it in and receive post-processed >> events such as pointer motion events, key events, touch events. >> >> This does not, however, mean that compositors shouldn't be able to >> receive raw input events; it's just not there yet as there is no user of >> such API. >> >> While making these changes, I removed some functionality, namely >> configuring evdev-touchpad via weston.ini. While it should obviously be >> possible to configure devices, I have yet to made API for doing so. Device >> detection logging is more limited as well. >> >> The repository of libinput can currently be found here [0]. It is a >> history rewrite of the weston repository, so the history of related files >> are still intact. >> >> I have created patches for weston (that I will submit by replying to this >> E-mail) for consideration. I have tested this with a regular pointer >> device, keyboard, touchpad, but not with a touch device. >> >> This is more or less a RFC about this approach, and any input would be >> appreciated. >> > > An idea whose time has come! See also > https://github.com/whot/libtouchpad > > Do you also intend it to be usable by the other consumers that > libtouchpad targets - ie: Xorg (and, hey, because I'm here, Mir)? Right now the only thing making this Wayland:y is the enum's which are so far made to be "compatible" (converting is just a type cast) with Wayland where possible. I don't see the reason to put too many assumptions about the user. Jonas ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC] Common input device library
On Wed, Nov 13, 2013 at 1:49 AM, Kristian Høgsberg wrote:
> On Tue, Nov 12, 2013 at 1:50 PM, Jonas Ådahl wrote:
>> Hi,
>>
>> Wayland compositors are expected to deal with input device processing
>> themselves providing input events according to the Wayland protocol to
>> the clients. So far only weston has had more than basic support for
>> processing raw input events from evdev.
>>
>> In order to avoid reimplementing support for various types of input
>> devices over and over again for every compositor, I extracted the input
>> device code from weston and put it in a new library called libinput.
>>
>> The API is very inspired by the internal weston API, but with some
>> simplifications and generalizations. The idea is to make it possible for
>> other compositors to be able to plug it in and receive post-processed
>> events such as pointer motion events, key events, touch events.
>>
>> This does not, however, mean that compositors shouldn't be able to
>> receive raw input events; it's just not there yet as there is no user of
>> such API.
>>
>> While making these changes, I removed some functionality, namely
>> configuring evdev-touchpad via weston.ini. While it should obviously be
>> possible to configure devices, I have yet to made API for doing so. Device
>> detection logging is more limited as well.
>>
>> The repository of libinput can currently be found here [0]. It is a
>> history rewrite of the weston repository, so the history of related files
>> are still intact.
>>
>> I have created patches for weston (that I will submit by replying to this
>> E-mail) for consideration. I have tested this with a regular pointer
>> device, keyboard, touchpad, but not with a touch device.
>>
>> This is more or less a RFC about this approach, and any input would be
>> appreciated.
>
> Looks promising... just few quick comments (I'll have a closer look later):
>
> - I was thinking we could also roll in the udev discovery support. A
> lot of the complexity is in the udev interaction, seat handling and
> setting up and shutting down the udev monitor etc. I'm not sure that
> we care about non-udev cases at all.
The question that comes to bind is how to open fd's, as udev-seat.c
uses weston_launch for opening input device files. I don't think we
should put weston_launch functionality in libinput, so some API
(similar to add_fd) for opening files would then probably be needed.
Anyhow, I don't have objections to putting device discovery in the
input library.
>
> - Instead of the add_fd API, could we just use an epoll loop
> internally and expose the epoll fd to the user?
I don't see why not. The add/remove_fd was put there because I needed
to use timer_fds, but if we'd put udev handling in libinput then I'd
think it makes sense to do something like this.
>
> - As for the API, I was thinking of something more like an event
> queue and a number of event types. We create the libinput object and
> it gives us an fd (the epoll fd). When that's readable, we call back
> into libinput to read events and we can then say libinput_get_event()
> to get a number of events: all the actual input events, of course
> (motion, buttons etc), but also a capability event when it grows or
> loses a pointer, for example.
You mean reading struct's as in something like:
struct event_base {
int type;
};
struct pointer_motion_event {
struct event_base base;
uint32_t time;
fixed_t dx, dy;
};
evdev_data(...)
{
libinput_dispatch(...);
while (event = libinput_get_event()) {
switch (event->type) {
...
}
}
}
This looks less callback:y, which is nice.
Jonas
>
> Kristian
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] surface crop & scale protocol extension
On ons, 2013-11-13 at 11:54 +0200, Pekka Paalanen wrote: > On Tue, 12 Nov 2013 16:14:36 -0800 > Bill Spitzak wrote: > > > Pekka Paalanen wrote: > > > > >> The source rectangle *must* be in buffer pixels. Putting it in > > >> buffer_scale units makes absolutely no sense, as the buffer_scale is in > > >> effect ignored for this surface, overridden entirely by the scaling. > > > > > > That means that dst_width and dst_height will be required to be in > > > multiples of buffer_scale. > > > > I am rather confused about this and possibly misunderstanding what > > Wayland is doing. Here is an example of what I think the current design is: > > > > Imagine there is an output_scale of 3 (chosen to not be a power of 2). A > > client is aware of this and wishes to draw a full-resolution display > > that is 2000x2000 pixels and make a subwindow that scales a 512x512 > > picture to fill a 1000x1000 pixel square. > > Is your whole issue based on the premise, that the output resolution is > not a multiple of output_scale? > > Just like you deduced, it won't work. It not working has nothing to do > with crop & scale state. > > Alexander, did you have any ideas for the case when someone sets > output_scale so that output resolution is not divisible by it? The output scale is a hint to the client what kind of pre-scaling it should apply to the buffer to best match the real output. There is no real guarantee that this is an exact match, because the compositor may be projecting it on a 3d-rotated spherical monitor or whatever. It is *commonly* the case that we have a regular rectangular output where the output scale is an integer divisor of the real output resolution such that the buffer can be used unscaled in the scanout framebuffer. I.e. the case of a 2560x1600 native display is typically run with a output scale of 2, which gives a maximized window a surface coordinate space of 1280x800. However, a compositor is also free to do something else, for instance like OSX it can allow using a 1440x900 global coordinate space, while saying output scale is 2. This would result in the client sending 2880x1800 buffers for fullscreen windows that the compositor will have to downscale (by a non-integer scaling factor) to draw to the framebuffer. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] surface crop & scale protocol extension
On Tue, 12 Nov 2013 16:14:36 -0800 Bill Spitzak wrote: > Pekka Paalanen wrote: > > >> The source rectangle *must* be in buffer pixels. Putting it in > >> buffer_scale units makes absolutely no sense, as the buffer_scale is in > >> effect ignored for this surface, overridden entirely by the scaling. > > > > That means that dst_width and dst_height will be required to be in > > multiples of buffer_scale. > > I am rather confused about this and possibly misunderstanding what > Wayland is doing. Here is an example of what I think the current design is: > > Imagine there is an output_scale of 3 (chosen to not be a power of 2). A > client is aware of this and wishes to draw a full-resolution display > that is 2000x2000 pixels and make a subwindow that scales a 512x512 > picture to fill a 1000x1000 pixel square. Is your whole issue based on the premise, that the output resolution is not a multiple of output_scale? Just like you deduced, it won't work. It not working has nothing to do with crop & scale state. Alexander, did you have any ideas for the case when someone sets output_scale so that output resolution is not divisible by it? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] input: Don't send leave events to destroyed views
On 12/11/13 19:28, Emilio Pozuelo Monfort wrote:
> If a view which has focus is destroyed, we would send a leave
> event while changing focus, causing a segfault. Prevent this
> by listening to the view's destroy signal and removing it from
> the pointer focus.
>
> Signed-off-by: Emilio Pozuelo Monfort
> ---
> src/compositor.h | 1 +
> src/input.c | 22 ++
> 2 files changed, 23 insertions(+)
>
> diff --git a/src/compositor.h b/src/compositor.h
> index 21d94d3..b69111f 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -311,6 +311,7 @@ struct weston_pointer {
> struct wl_list focus_resource_list;
> struct weston_view *focus;
> uint32_t focus_serial;
> + struct wl_listener focus_listener;
> struct wl_signal focus_signal;
>
> struct weston_view *sprite;
> diff --git a/src/input.c b/src/input.c
> index ed3e06f..64f0606 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -392,6 +392,7 @@ weston_pointer_create(void)
> pointer->default_grab.pointer = pointer;
> pointer->grab = &pointer->default_grab;
> wl_signal_init(&pointer->focus_signal);
> + wl_list_init(&pointer->focus_listener.link);
>
> pointer->sprite_destroy_listener.notify = pointer_handle_sprite_destroy;
>
> @@ -487,6 +488,23 @@ seat_send_updated_caps(struct weston_seat *seat)
> }
> }
>
> +static void
> +destroy_pointer_focus(struct wl_listener *listener, void *data)
> +{
> + struct weston_pointer *pointer;
> +
> + pointer = container_of(listener, struct weston_pointer,
> +focus_listener);
> +
> + pointer->focus = NULL;
> + move_resources(&pointer->resource_list, &pointer->focus_resource_list);
> +
> + wl_list_remove(&pointer->focus_listener.link);
> + wl_list_init(&pointer->focus_listener.link);
> +
> + wl_signal_emit(&pointer->focus_signal, pointer);
> +}
> +
> WL_EXPORT void
> weston_pointer_set_focus(struct weston_pointer *pointer,
>struct weston_view *view,
> @@ -543,7 +561,11 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
> pointer->focus_serial = serial;
> }
>
> + if (!wl_list_empty(&pointer->focus_listener.link))
> + wl_list_remove(&pointer->focus_listener.link);
> pointer->focus = view;
> + pointer->focus_listener.notify = destroy_pointer_focus;
> + wl_signal_add(&view->destroy_signal, &pointer->focus_listener);
There's a bug here if view is NULL (e.g. when unsetting focus). I'll send an
updated patch.
Emilio
> wl_signal_emit(&pointer->focus_signal, pointer);
> }
>
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
