[PATCH wayland 3/5] server: don't proceed in posting no-memory error on client destruction
When a client is being destroyed, the display_resource is set to NULL. If then some destroy handler calls wl_client_post_no_memory() or wl_resource_post_no_memory() we crash. Signed-off-by: Marek Chalupa --- src/wayland-server.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/wayland-server.c b/src/wayland-server.c index c93a426..b26a48d 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -546,6 +546,11 @@ wl_client_get_object(struct wl_client *client, uint32_t id) WL_EXPORT void wl_client_post_no_memory(struct wl_client *client) { + /* don't send any other errors +* if we are destroying the client */ + if (!client->display_resource) + return; + wl_resource_post_error(client->display_resource, WL_DISPLAY_ERROR_NO_MEMORY, "no memory"); } @@ -553,6 +558,11 @@ wl_client_post_no_memory(struct wl_client *client) WL_EXPORT void wl_resource_post_no_memory(struct wl_resource *resource) { + /* don't send any other errors +* if we are destroying the client */ + if (!resource->client->display_resource) + return; + wl_resource_post_error(resource->client->display_resource, WL_DISPLAY_ERROR_NO_MEMORY, "no memory"); } -- 2.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 5/5] tests: test whether the destroyed object is set to NULL on client destruction
when the client is being destroyed, we can use wl_client_get_object() to get an object. Test whether it returns NULL when the object has been destroyed Signed-off-by: Marek Chalupa --- tests/client-test.c | 63 + 1 file changed, 63 insertions(+) diff --git a/tests/client-test.c b/tests/client-test.c index cb08e39..f171f19 100644 --- a/tests/client-test.c +++ b/tests/client-test.c @@ -126,3 +126,66 @@ TEST(client_post_nomem_on_destruction) wl_display_destroy(display); } +static void +seat_destroy_get_object(struct wl_listener *l, void *data) +{ + struct wl_resource *resource = data; + struct wl_client *client = wl_resource_get_client(resource); + /* This seat was created last, +* therefore the seat's id is the upper bound for all ids */ + uint32_t max_id = wl_resource_get_id(resource); + uint32_t i; + + for (i = WL_SERVER_ID_START; i < max_id; ++i) { + /* all these objects are destroyed now, since +* we destroy the objects in id order */ + assert(wl_client_get_object(client, i) == NULL); + } + + /* only our object is not still destroyed */ + assert(wl_client_get_object(client, max_id) != NULL); + assert(wl_client_get_object(client, max_id + 1) == NULL); +} + +TEST(client_get_object_on_destruction) +{ + struct wl_display *display; + struct wl_client *client; + struct wl_resource *resource; + int s[2]; + + assert(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, s) == 0); + display = wl_display_create(); + assert(display); + client = wl_client_create(display, s[0]); + assert(client); + + /* create few other objects +* -- it doesn't matter what is the interface */ + resource = wl_resource_create(client, &wl_seat_interface, + wl_seat_interface.version, 0); + assert(resource); + resource = wl_resource_create(client, &wl_seat_interface, + wl_seat_interface.version, 0); + assert(resource); + resource = wl_resource_create(client, &wl_seat_interface, + wl_seat_interface.version, 0); + assert(resource); + + + /* for the last one we set destroy listener */ + resource = wl_resource_create(client, &wl_seat_interface, + wl_seat_interface.version, 0); + assert(resource); + + seat_destroy_listener.notify = seat_destroy_get_object; + wl_resource_add_destroy_listener(resource, &seat_destroy_listener); + + wl_client_destroy(client); + + close(s[0]); + close(s[1]); + + wl_display_destroy(display); +} + -- 2.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 2/5] server: set map entry to NULL after resource is destroyed
We did it only for client entries for some reason, so when we used wl_client_get_object() for some server object that has been destroyed, we got dangling pointer. NOTE: this is basically an API change, since it changes the return value of wl_client_get_object() in some corner cases. However, now we return NULL insted of a pointer to invalid memory, which could be OK API break. Signed-off-by: Marek Chalupa --- src/wayland-server.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/wayland-server.c b/src/wayland-server.c index f745e62..c93a426 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -562,16 +562,20 @@ destroy_resource(void *element, void *data) { struct wl_resource *resource = element; struct wl_client *client = resource->client; + uint32_t id = resource->object.id; uint32_t flags; wl_signal_emit(&resource->destroy_signal, resource); - flags = wl_map_lookup_flags(&client->objects, resource->object.id); + flags = wl_map_lookup_flags(&client->objects, id); if (resource->destroy) resource->destroy(resource); if (!(flags & WL_MAP_ENTRY_LEGACY)) free(resource); + + /* replace the object with NULL since it is destroyed */ + wl_map_insert_at(&client->objects, 0, id, NULL); } WL_EXPORT void @@ -584,11 +588,9 @@ wl_resource_destroy(struct wl_resource *resource) destroy_resource(resource, NULL); if (id < WL_SERVER_ID_START) { - if (client->display_resource) { + if (client->display_resource) wl_resource_queue_event(client->display_resource, WL_DISPLAY_DELETE_ID, id); - } - wl_map_insert_at(&client->objects, 0, id, NULL); } else { wl_map_remove(&client->objects, id); } -- 2.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 4/5] client-test: test posting no-mem error on client destruction
while client is being destroyed, it destroys all its resources. Destroy handlers are called for the resources and if some destroy handler post no memory error, we get crash, because the display resource is NULL (it is destroyed as a first resource) Signed-off-by: Marek Chalupa --- tests/client-test.c | 37 + 1 file changed, 37 insertions(+) diff --git a/tests/client-test.c b/tests/client-test.c index 2e332f8..cb08e39 100644 --- a/tests/client-test.c +++ b/tests/client-test.c @@ -89,3 +89,40 @@ TEST(client_destroy_listener) wl_display_destroy(display); } +static struct wl_listener seat_destroy_listener; + +static void +seat_destroy_post_no_memory(struct wl_listener *l, void *data) +{ + struct wl_resource *resource = data; + wl_client_post_no_memory(wl_resource_get_client(resource)); +} + +TEST(client_post_nomem_on_destruction) +{ + struct wl_display *display; + struct wl_client *client; + struct wl_resource *resource; + int s[2]; + + assert(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, s) == 0); + display = wl_display_create(); + assert(display); + client = wl_client_create(display, s[0]); + assert(client); + + resource = wl_resource_create(client, &wl_seat_interface, + wl_seat_interface.version, 0); + assert(resource); + + seat_destroy_listener.notify = seat_destroy_post_no_memory; + wl_resource_add_destroy_listener(resource, &seat_destroy_listener); + + wl_client_destroy(client); + + close(s[0]); + close(s[1]); + + wl_display_destroy(display); +} + -- 2.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 1/5] display-test: move a misplaced comment
we split a function while refactoring in c643781 and now the comment makes no sense Signed-off-by: Marek Chalupa --- tests/display-test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/display-test.c b/tests/display-test.c index f9f8160..17956db 100644 --- a/tests/display-test.c +++ b/tests/display-test.c @@ -211,8 +211,6 @@ find_client_info(struct display *d, struct wl_client *client) { struct client_info *ci; - /* find the right client_info struct and save the -* resource as its data, so that we can use it later */ wl_list_for_each(ci, &d->clients, link) { if (ci->wl_client == client) return ci; @@ -235,6 +233,8 @@ bind_seat(struct wl_client *client, void *data, res = wl_resource_create(client, &wl_seat_interface, vers, id); assert(res); + /* save the resource as client's info data, +* so that we can use it later */ ci->data = res; } -- 2.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] eventdemo: use %u for uint32_t printing
Hello Pekka, Look sane. Reviewed-by: Benoit Gschwind On 13/05/2016 13:38, Pekka Paalanen wrote: > From: Pekka Paalanen > > I was confused why timestamp was printed negative. This fixes it, and > others while at it. > > Signed-off-by: Pekka Paalanen > --- > clients/eventdemo.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c > index 64b3d01..f04e39b 100644 > --- a/clients/eventdemo.c > +++ b/clients/eventdemo.c > @@ -206,7 +206,7 @@ key_handler(struct window *window, struct input *input, > uint32_t time, > if (!log_key) > return; > > - printf("key key: %d, unicode: %d, state: %s, modifiers: 0x%x\n", > + printf("key key: %u, unicode: %u, state: %s, modifiers: 0x%x\n", > key, unicode, > (state == WL_KEYBOARD_KEY_STATE_PRESSED) ? "pressed" : > "released", > @@ -235,7 +235,7 @@ button_handler(struct widget *widget, struct input > *input, uint32_t time, > e->print_pointer_frame = true; > > input_get_position(input, &x, &y); > - printf("button time: %d, button: %d, state: %s, x: %d, y: %d\n", > + printf("button time: %u, button: %u, state: %s, x: %d, y: %d\n", > time, button, > (state == WL_POINTER_BUTTON_STATE_PRESSED) ? "pressed" : > "released", > @@ -262,7 +262,7 @@ axis_handler(struct widget *widget, struct input *input, > uint32_t time, > > e->print_pointer_frame = true; > > - printf("axis time: %d, axis: %s, value: %f\n", > + printf("axis time: %u, axis: %s, value: %f\n", > time, > axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : >"horizontal", > @@ -322,7 +322,7 @@ axis_stop_handler(struct widget *widget, struct input > *input, > return; > > e->print_pointer_frame = true; > - printf("axis stop time: %d, axis: %s\n", > + printf("axis stop time: %u, axis: %s\n", > time, > axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : >"horizontal"); > @@ -338,7 +338,7 @@ axis_discrete_handler(struct widget *widget, struct input > *input, > return; > > e->print_pointer_frame = true; > - printf("axis discrete axis: %d value: %d\n", axis, discrete); > + printf("axis discrete axis: %u value: %d\n", axis, discrete); > } > > /** > @@ -361,7 +361,7 @@ motion_handler(struct widget *widget, struct input > *input, uint32_t time, > struct eventdemo *e = data; > > if (log_motion) { > - printf("motion time: %d, x: %f, y: %f\n", time, x, y); > + printf("motion time: %u, x: %f, y: %f\n", time, x, y); > e->print_pointer_frame = true; > } > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] eventdemo: use %u for uint32_t printing
From: Pekka Paalanen I was confused why timestamp was printed negative. This fixes it, and others while at it. Signed-off-by: Pekka Paalanen --- clients/eventdemo.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clients/eventdemo.c b/clients/eventdemo.c index 64b3d01..f04e39b 100644 --- a/clients/eventdemo.c +++ b/clients/eventdemo.c @@ -206,7 +206,7 @@ key_handler(struct window *window, struct input *input, uint32_t time, if (!log_key) return; - printf("key key: %d, unicode: %d, state: %s, modifiers: 0x%x\n", + printf("key key: %u, unicode: %u, state: %s, modifiers: 0x%x\n", key, unicode, (state == WL_KEYBOARD_KEY_STATE_PRESSED) ? "pressed" : "released", @@ -235,7 +235,7 @@ button_handler(struct widget *widget, struct input *input, uint32_t time, e->print_pointer_frame = true; input_get_position(input, &x, &y); - printf("button time: %d, button: %d, state: %s, x: %d, y: %d\n", + printf("button time: %u, button: %u, state: %s, x: %d, y: %d\n", time, button, (state == WL_POINTER_BUTTON_STATE_PRESSED) ? "pressed" : "released", @@ -262,7 +262,7 @@ axis_handler(struct widget *widget, struct input *input, uint32_t time, e->print_pointer_frame = true; - printf("axis time: %d, axis: %s, value: %f\n", + printf("axis time: %u, axis: %s, value: %f\n", time, axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : "horizontal", @@ -322,7 +322,7 @@ axis_stop_handler(struct widget *widget, struct input *input, return; e->print_pointer_frame = true; - printf("axis stop time: %d, axis: %s\n", + printf("axis stop time: %u, axis: %s\n", time, axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : "horizontal"); @@ -338,7 +338,7 @@ axis_discrete_handler(struct widget *widget, struct input *input, return; e->print_pointer_frame = true; - printf("axis discrete axis: %d value: %d\n", axis, discrete); + printf("axis discrete axis: %u value: %d\n", axis, discrete); } /** @@ -361,7 +361,7 @@ motion_handler(struct widget *widget, struct input *input, uint32_t time, struct eventdemo *e = data; if (log_motion) { - printf("motion time: %d, x: %f, y: %f\n", time, x, y); + printf("motion time: %u, x: %f, y: %f\n", time, x, y); e->print_pointer_frame = true; } -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] eventdemo: do not print pointer frames alone
On Fri, 13 May 2016 11:03:07 +0200 Benoit Gschwind wrote: > Hello Pekka, > > as-is you get my: > Reviewed-by: Benoit Gschwind > Tested-by: Benoit Gschwind > > But I have comments that may be worth to read :) > > On 10/05/2016 16:11, Pekka Paalanen wrote: > > From: Pekka Paalanen > > > > Print pointer frames only if any pointer related events are printed > > first. > > > > This avoids flooding the output with "pointer frame" just because of > > motion. > > I would use something like: > > This avoids flooding the output with "pointer frame" of hidden/filtered > events. > > But it's more about taste. Hi, I added: You can test this with e.g. $ ./weston-eventdemo --log-button > > > > Signed-off-by: Pekka Paalanen > > --- > > clients/eventdemo.c | 26 ++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c > > index e323aa5..f1558d2 100644 > > --- a/clients/eventdemo.c > > +++ b/clients/eventdemo.c > > @@ -35,6 +35,7 @@ > > > > #include > > #include > > +#include > > > > #include > > > > @@ -93,6 +94,8 @@ struct eventdemo { > > struct display *display; > > > > int x, y, w, h; > > + > > + bool print_pointer_frame; > > I would use a global static following the existing width, height, log_* > rather than smear the eventdemo structure. That also reduce the patch size. I hate both globals and statics used for no other reason than lazyness, so I try to avoid them. They are always somewhat surprising. Pushed all three: 6bc5254..0baffb0 master -> master Thanks, pq pgp3mpXUwf9NV.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v1 0/3] compositor-x11: improving handling of mouse events
v1: - add some sanity checking, - remove redundant viarable, - use a better X11 passive grab Benoit Gschwind (3): compositor-x11: add assert to avoid misuse of x11_backend_deliver_button_event compositor-x11: remove redundant state arg of x11_backend_deliver_button_event compositor-x11: remove manual mouse button grab/ungrab src/compositor-x11.c | 48 1 file changed, 24 insertions(+), 24 deletions(-) -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v1 3/3] compositor-x11: remove manual mouse button grab/ungrab
In current compositor-x11, the mouse buttons are grabbed and ungrabbed manually that may produce weird cases like starting a grab while the buttons are already released, due to asynchronous X11 events dispatching. The patch avoid the issue by using the better passive button grab, that automatically grab buttons as soon as they occur. The passive grab include an automatic ungrab when all mouse button are released. This probably fix some mysterious bugs. Signed-off-by: Benoit Gschwind --- src/compositor-x11.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/compositor-x11.c b/src/compositor-x11.c index 5b76dba..ae155d1 100644 --- a/src/compositor-x11.c +++ b/src/compositor-x11.c @@ -882,6 +882,19 @@ x11_backend_create_output(struct x11_backend *b, int x, int y, x11_output_set_wm_protocols(b, output); + /* Automatically grab and ungrab mouse buttons */ + xcb_grab_button(b->conn, 0, output->window, + XCB_EVENT_MASK_BUTTON_PRESS | + XCB_EVENT_MASK_BUTTON_RELEASE | + XCB_EVENT_MASK_POINTER_MOTION | + XCB_EVENT_MASK_ENTER_WINDOW | + XCB_EVENT_MASK_LEAVE_WINDOW, + XCB_GRAB_MODE_ASYNC, + XCB_GRAB_MODE_ASYNC, + output->window, XCB_CURSOR_NONE, + XCB_BUTTON_INDEX_ANY, + XCB_MOD_MASK_ANY); + xcb_map_window(b->conn, output->window); if (fullscreen) @@ -1052,20 +1065,6 @@ x11_backend_deliver_button_event(struct x11_backend *b, if (!output) return; - if (is_button_pressed) - xcb_grab_pointer(b->conn, 0, output->window, -XCB_EVENT_MASK_BUTTON_PRESS | -XCB_EVENT_MASK_BUTTON_RELEASE | -XCB_EVENT_MASK_POINTER_MOTION | -XCB_EVENT_MASK_ENTER_WINDOW | -XCB_EVENT_MASK_LEAVE_WINDOW, -XCB_GRAB_MODE_ASYNC, -XCB_GRAB_MODE_ASYNC, -output->window, XCB_CURSOR_NONE, -button_event->time); - else - xcb_ungrab_pointer(b->conn, button_event->time); - if (!b->has_xkb) update_xkb_state_from_core(b, button_event->state); -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v1 2/3] compositor-x11: remove redundant state arg of x11_backend_deliver_button_event
The "state" variable in x11_backend_deliver_button_event is basically the same as (event->response_type == XCB_BUTTON_PRESS), thus update the code to use the last one. Signed-off-by: Benoit Gschwind --- src/compositor-x11.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/compositor-x11.c b/src/compositor-x11.c index e7b09cf..5b76dba 100644 --- a/src/compositor-x11.c +++ b/src/compositor-x11.c @@ -1037,7 +1037,7 @@ update_xkb_state_from_core(struct x11_backend *b, uint16_t x11_mask) static void x11_backend_deliver_button_event(struct x11_backend *b, -xcb_generic_event_t *event, int state) +xcb_generic_event_t *event) { assert(event->response_type == XCB_BUTTON_PRESS || event->response_type == XCB_BUTTON_RELEASE); @@ -1046,12 +1046,13 @@ x11_backend_deliver_button_event(struct x11_backend *b, uint32_t button; struct x11_output *output; struct weston_pointer_axis_event weston_event; + bool is_button_pressed = event->response_type == XCB_BUTTON_PRESS; output = x11_backend_find_output(b, button_event->event); if (!output) return; - if (state) + if (is_button_pressed) xcb_grab_pointer(b->conn, 0, output->window, XCB_EVENT_MASK_BUTTON_PRESS | XCB_EVENT_MASK_BUTTON_RELEASE | @@ -1081,7 +1082,7 @@ x11_backend_deliver_button_event(struct x11_backend *b, case 4: /* Axis are measured in pixels, but the xcb events are discrete * steps. Therefore move the axis by some pixels every step. */ - if (state) { + if (is_button_pressed) { weston_event.value = -DEFAULT_AXIS_STEP_DISTANCE; weston_event.discrete = -1; weston_event.has_discrete = true; @@ -1094,7 +1095,7 @@ x11_backend_deliver_button_event(struct x11_backend *b, } return; case 5: - if (state) { + if (is_button_pressed) { weston_event.value = DEFAULT_AXIS_STEP_DISTANCE; weston_event.discrete = 1; weston_event.has_discrete = true; @@ -1107,7 +1108,7 @@ x11_backend_deliver_button_event(struct x11_backend *b, } return; case 6: - if (state) { + if (is_button_pressed) { weston_event.value = -DEFAULT_AXIS_STEP_DISTANCE; weston_event.discrete = -1; weston_event.has_discrete = true; @@ -1120,7 +1121,7 @@ x11_backend_deliver_button_event(struct x11_backend *b, } return; case 7: - if (state) { + if (is_button_pressed) { weston_event.value = DEFAULT_AXIS_STEP_DISTANCE; weston_event.discrete = 1; weston_event.has_discrete = true; @@ -1139,8 +1140,8 @@ x11_backend_deliver_button_event(struct x11_backend *b, notify_button(&b->core_seat, weston_compositor_get_time(), button, - state ? WL_POINTER_BUTTON_STATE_PRESSED : - WL_POINTER_BUTTON_STATE_RELEASED); + is_button_pressed ? WL_POINTER_BUTTON_STATE_PRESSED : + WL_POINTER_BUTTON_STATE_RELEASED); notify_pointer_frame(&b->core_seat); } @@ -1333,10 +1334,8 @@ x11_backend_handle_event(int fd, uint32_t mask, void *data) STATE_UPDATE_NONE); break; case XCB_BUTTON_PRESS: - x11_backend_deliver_button_event(b, event, 1); - break; case XCB_BUTTON_RELEASE: - x11_backend_deliver_button_event(b, event, 0); + x11_backend_deliver_button_event(b, event); break; case XCB_MOTION_NOTIFY: x11_backend_deliver_motion_event(b, event); -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v1 1/3] compositor-x11: add assert to avoid misuse of x11_backend_deliver_button_event
The x11_backend_deliver_button_event can be called with any xcb_generic_event. The assert check if the call is done with the expected events. Signed-off-by: Benoit Gschwind --- src/compositor-x11.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/compositor-x11.c b/src/compositor-x11.c index 629b5f3..e7b09cf 100644 --- a/src/compositor-x11.c +++ b/src/compositor-x11.c @@ -1039,6 +1039,8 @@ static void x11_backend_deliver_button_event(struct x11_backend *b, xcb_generic_event_t *event, int state) { + assert(event->response_type == XCB_BUTTON_PRESS || + event->response_type == XCB_BUTTON_RELEASE); xcb_button_press_event_t *button_event = (xcb_button_press_event_t *) event; uint32_t button; -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 2/3] eventdemo: use zalloc
Hello Pekka, Reviewed-by: Benoit Gschwind Tested-by: Benoit Gschwind just in case, If you apply my suggestion for the previous patch, this patch may be useless. Best regards, Benoit Gschwind On 10/05/2016 16:11, Pekka Paalanen wrote: > From: Pekka Paalanen > > Zero-initialize the struct, just in case. > > Signed-off-by: Pekka Paalanen > --- > clients/eventdemo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c > index f1558d2..38eb904 100644 > --- a/clients/eventdemo.c > +++ b/clients/eventdemo.c > @@ -373,7 +373,7 @@ eventdemo_create(struct display *d) > { > struct eventdemo *e; > > - e = malloc(sizeof (struct eventdemo)); > + e = zalloc(sizeof (struct eventdemo)); > if (e == NULL) > return NULL; > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] eventdemo: do not print axis events if not requested
Hello Pekka, Reviewed-by: Benoit Gschwind Tested-by: Benoit Gschwind That surprising that the log_axis was not used here :) Best regards, Benoit Gschwind On 10/05/2016 16:11, Pekka Paalanen wrote: > From: Pekka Paalanen > > Signed-off-by: Pekka Paalanen > --- > clients/eventdemo.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c > index 38eb904..64b3d01 100644 > --- a/clients/eventdemo.c > +++ b/clients/eventdemo.c > @@ -288,6 +288,9 @@ axis_source_handler(struct widget *widget, struct input > *input, > const char *axis_source; > struct eventdemo *e = data; > > + if (!log_axis) > + return; > + > e->print_pointer_frame = true; > > switch (source) { > @@ -315,6 +318,9 @@ axis_stop_handler(struct widget *widget, struct input > *input, > { > struct eventdemo *e = data; > > + if (!log_axis) > + return; > + > e->print_pointer_frame = true; > printf("axis stop time: %d, axis: %s\n", > time, > @@ -328,6 +334,9 @@ axis_discrete_handler(struct widget *widget, struct input > *input, > { > struct eventdemo *e = data; > > + if (!log_axis) > + return; > + > e->print_pointer_frame = true; > printf("axis discrete axis: %d value: %d\n", axis, discrete); > } > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/3] eventdemo: do not print pointer frames alone
Hello Pekka, as-is you get my: Reviewed-by: Benoit Gschwind Tested-by: Benoit Gschwind But I have comments that may be worth to read :) On 10/05/2016 16:11, Pekka Paalanen wrote: > From: Pekka Paalanen > > Print pointer frames only if any pointer related events are printed > first. > > This avoids flooding the output with "pointer frame" just because of > motion. I would use something like: This avoids flooding the output with "pointer frame" of hidden/filtered events. But it's more about taste. > > Signed-off-by: Pekka Paalanen > --- > clients/eventdemo.c | 26 ++ > 1 file changed, 26 insertions(+) > > diff --git a/clients/eventdemo.c b/clients/eventdemo.c > index e323aa5..f1558d2 100644 > --- a/clients/eventdemo.c > +++ b/clients/eventdemo.c > @@ -35,6 +35,7 @@ > > #include > #include > +#include > > #include > > @@ -93,6 +94,8 @@ struct eventdemo { > struct display *display; > > int x, y, w, h; > + > + bool print_pointer_frame; I would use a global static following the existing width, height, log_* rather than smear the eventdemo structure. That also reduce the patch size. > }; > > /** > @@ -223,11 +226,14 @@ static void > button_handler(struct widget *widget, struct input *input, uint32_t time, > uint32_t button, enum wl_pointer_button_state state, void *data) > { > + struct eventdemo *e = data; > int32_t x, y; > > if (!log_button) > return; > > + e->print_pointer_frame = true; > + > input_get_position(input, &x, &y); > printf("button time: %d, button: %d, state: %s, x: %d, y: %d\n", > time, button, > @@ -249,9 +255,13 @@ static void > axis_handler(struct widget *widget, struct input *input, uint32_t time, >uint32_t axis, wl_fixed_t value, void *data) > { > + struct eventdemo *e = data; > + > if (!log_axis) > return; > > + e->print_pointer_frame = true; > + > printf("axis time: %d, axis: %s, value: %f\n", > time, > axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : > @@ -262,7 +272,13 @@ axis_handler(struct widget *widget, struct input *input, > uint32_t time, > static void > pointer_frame_handler(struct widget *widget, struct input *input, void *data) > { > + struct eventdemo *e = data; > + > + if (!e->print_pointer_frame) > + return; > + > printf("pointer frame\n"); > + e->print_pointer_frame = false; > } > > static void > @@ -270,6 +286,9 @@ axis_source_handler(struct widget *widget, struct input > *input, > uint32_t source, void *data) > { > const char *axis_source; > + struct eventdemo *e = data; > + > + e->print_pointer_frame = true; > > switch (source) { > case WL_POINTER_AXIS_SOURCE_WHEEL: > @@ -294,6 +313,9 @@ axis_stop_handler(struct widget *widget, struct input > *input, > uint32_t time, uint32_t axis, > void *data) > { > + struct eventdemo *e = data; > + > + e->print_pointer_frame = true; > printf("axis stop time: %d, axis: %s\n", > time, > axis == WL_POINTER_AXIS_VERTICAL_SCROLL ? "vertical" : > @@ -304,6 +326,9 @@ static void > axis_discrete_handler(struct widget *widget, struct input *input, > uint32_t axis, int32_t discrete, void *data) > { > + struct eventdemo *e = data; > + > + e->print_pointer_frame = true; > printf("axis discrete axis: %d value: %d\n", axis, discrete); > } > > @@ -328,6 +353,7 @@ motion_handler(struct widget *widget, struct input > *input, uint32_t time, > > if (log_motion) { > printf("motion time: %d, x: %f, y: %f\n", time, x, y); > + e->print_pointer_frame = true; > } > > if (x > e->x && x < e->x + e->w) > Best regards, Benoit Gschwind. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Build a libweston library
On 11/05/2016 21:31, Giulio Camuffo wrote: This patch creates a libweston-N.so library, where N is the "libweston_abi_version" defined in configure.ac. Almost all the code that was previously built in the weston binary is now in libweston.sp, except main.c and log.c. Possibly other files will need to be moved to weston but it can be done when we identify which files are they. When the API/ABI of libweston changes in an incompatible way, the abi version will be bumped so that multiple versions can live together. Also a libweston-N.pc file is created and installed together with weston.pc. Signed-off-by: Giulio Camuffo --- Makefile.am | 39 --- configure.ac| 3 +++ src/input.c | 4 ++-- src/libweston.pc.in | 11 +++ 4 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 src/libweston.pc.in diff --git a/Makefile.am b/Makefile.am index 0b7aa35..fbc509d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3,7 +3,7 @@ ACLOCAL_AMFLAGS = -I m4 bin_PROGRAMS = noinst_PROGRAMS = libexec_PROGRAMS = -moduledir = $(libdir)/weston +moduledir = $(libdir)/weston-${LIBWESTON_ABI_VERSION} As Benoit said, you must split what is weston and what is libweston here. Let’s introduce libweston*dir. I prefer …/libweston-${abi}, because …/weston-${abi} is confusing and can look like a leftover or wrong rename. Also see an unrelated (to libweston) comment at the end of this email. module_LTLIBRARIES = noinst_LTLIBRARIES = BUILT_SOURCES = @@ -59,17 +59,15 @@ CLEANFILES = weston.ini \ internal-screenshot-00.png \ $(BUILT_SOURCES) -bin_PROGRAMS += weston +lib_LTLIBRARIES = libweston.la +libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON +libweston_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) You are missing $(AM_CFLAGS) here. If adding it breaks something, then fix AM_CFLAGS. +libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ +$(DLOPEN_LIBS) -lm libshared.la +libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION} -weston_LDFLAGS = -export-dynamic -weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON -weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) -weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ - $(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) libshared.la - -weston_SOURCES = \ +libweston_la_SOURCES = \ src/git-version.h \ - src/log.c \ src/compositor.c\ src/compositor.h\ src/compositor-drm.h\ @@ -92,7 +90,6 @@ weston_SOURCES = \ src/timeline.c \ src/timeline.h \ src/timeline-object.h \ - src/main.c \ src/linux-dmabuf.c \ src/linux-dmabuf.h \ shared/helpers.h\ @@ -119,7 +116,7 @@ systemd_notify_la_SOURCES = \ src/compositor.h endif -nodist_weston_SOURCES =\ +nodist_libweston_la_SOURCES = \ protocol/weston-screenshooter-protocol.c\ protocol/weston-screenshooter-server-protocol.h \ protocol/text-cursor-position-protocol.c\ @@ -135,7 +132,19 @@ nodist_weston_SOURCES = \ protocol/linux-dmabuf-unstable-v1-protocol.c\ protocol/linux-dmabuf-unstable-v1-server-protocol.h -BUILT_SOURCES += $(nodist_weston_SOURCES) +BUILT_SOURCES += $(nodist_libweston_la_SOURCES) + +bin_PROGRAMS += weston + +weston_LDFLAGS = -export-dynamic +weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON +weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) +weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \ + $(DLOPEN_LIBS) -lm libshared.la libweston.la + +weston_SOURCES = \ + src/main.c \ + src/log.c # Track this dependency explicitly instead of using BUILT_SOURCES. We # add BUILT_SOURCES to CLEANFILES, but we want to keep git-version.h @@ -205,12 +214,12 @@ endif endif # BUILD_WESTON_LAUNCH pkgconfigdir = $(libdir)/pkgconfig -pkgconfig_DATA = src/weston.pc +pkgconfig_DATA = src/weston.pc src/libweston-${LIBWESTON_ABI_VERSION}.pc wayland_sessiondir = $(datadir)/wayland-sessions dist_wayland_session_DATA = src/weston.desktop -westonincludedir = $(includedir)/weston +westonincludedir = $(includedir)/weston-${LIBWESTON_ABI_VERSION} Same as moduledir. westonincl
Re: [PATCH libinput] gestures: don't send swipe gestures when gestures are disabled
Hi, On 13-05-16 02:29, Peter Hutterer wrote: Introduced in 6ad303b as part of an code flow optimization, causing any 3+ finger gesture to be posted as swipe gesture, even when gestures are disabled. However, the event is filtered in the higher levels with a bug message printed to the log. Don't post swipe gestures for devices where gestures are disabled. https://bugs.freedesktop.org/show_bug.cgi?id=95314 Signed-off-by: Peter Hutterer Makes sense: Reviewed-by: Hans de Goede Regards, Hans --- src/evdev-mt-touchpad-gestures.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c index 7bbd3b8..2a804e7 100644 --- a/src/evdev-mt-touchpad-gestures.c +++ b/src/evdev-mt-touchpad-gestures.c @@ -248,7 +248,7 @@ tp_gesture_handle_state_none(struct tp_dispatch *tp, uint64_t time) if (ntouches == 2) return GESTURE_STATE_SCROLL; else - return GESTURE_STATE_SWIPE; + return GESTURE_STATE_NONE; } first = touches[0]; ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel