Re: [PATCH wayland-protocols v3 7/7] xdg-shell: Introduce xdg_positioner
On Tue, Jun 14, 2016 at 01:01:15PM -0400, Jonas Ådahl wrote: > On Thu, Jun 09, 2016 at 12:05:28AM +0200, Benoit Gschwind wrote: > > Hello Jonas, > > > > The proposal of xdg_positioner is a good idea. I still need to review > > your proposal deeper, but at the moment I have few question to help my > > understanding. > > ... snip ... > > > + > > > + > > > + > > > + > > > + Don't alter the surface position even if it is constrained on some > > > + axis, for example partially outside the edge of a monitor. > > > + > > > + > > > + > > > + > > > + Slide the surface along the x axis until it is no longer constrained. > > > + > > > + First try to slide towards the direction of the gravity on the x axis > > > + until either the edge in the opposite direction of the gravity is > > > + unconstrained or the edge in the direction of the gravity is > > > + constrained. > > > + > > > + Then try to slide towards the opposite direction of the gravity on the > > > + x axis until either the edge in the direction of the gravity is > > > + unconstrained or the edge in the opposite direction of the gravity is > > > + constrained. > > > + > > > + > > > + > > > + > > > + Slide the surface along the y axis until it is no longer constrained. > > > + > > > + First try to slide towards the direction of the gravity on the y axis > > > + until either the edge in the opposite direction of the gravity is > > > + unconstrained or the edge in the direction of the gravity is > > > + constrained. > > > + > > > + Then try to slide towards the opposite direction of the gravity on the > > > + y axis until either the edge in the direction of the gravity is > > > + unconstrained or the edge in the opposite direction of the gravity is > > > + constrained. > > > + > > > + > > > + > > > + > > > + Invert the anchor and gravity on the x axis if the surface is > > > + constrained on the x axis. For example, if the left edge of the > > > + surface is constrained, the gravity is 'left' and the anchor is > > > + 'left', change the gravity to 'right' and the anchor to 'right'. If > > > + the resulting position after inverting ends up also being constrained, > > > + the behaviour is undefined. > > > + > > > + > > > + > > > + > > > + Invert the anchor and gravity on the y axis if the surface is > > > + constrained on the y axis. For example, if the bottom edge of the > > > + surface is constrained, the gravity is 'bottom' and the anchor is > > > + 'bottom', change the gravity to 'top' and the anchor to 'top'. If > > > + the resulting position after inverting ends up also being constrained, > > > + the behaviour is undefined. > > > + > > > + > > > + > > > + > > > + > > > + > > > + Specify how the window should be positioned if the originally intended > > > + position caused the surface to be constrained, meaning at least > > > + partially outside positioning boundaries set by the compositor. The > > > + adjustment is set by constructing a bitmask with one bit per axis set > > > + describing the adjustment to be made when the surface is constrained on > > > + that axis. If no bit for one axis is set, the compositor will assume > > > + that the child surface should not change its position on that axis when > > > + constrained. The default adjustment is none. > > > + > > > + > > +summary="bit mask of constrain adjustments"/> > > > > Does setting all the bitmask mean do what you prefer to the compositor ? > > No, as written, its "one bit per axis". > > > > > Would it be better to give a priority list like: try flip_x first, then > > try slice_x then try flip_y for example ? > > That would complicate the semantics of the constraints we have. For > example, you could not try flipping after having tried sliding, since > sliding is defined in a moving mechanism, not an boolean "on-off". > > I think it might be more sane to just leave that case (flip and fallback > to slide) out right now, and if we actually end up needing it, adding > more rules to the enum is easy, and we can clearly specify the semantics > without having to deal with inter-rule semantics. > I have changed my mind about this part, I think it makes sense to be able to combine them, kind of in a way that is what you described. But rather than having a array, which would allow to create invalid combinations, we should just allow combining them in a bitmask, documenting the meaning of the combination. For example, starting with "sliding" then falling back to "flipping" is tricky, because "sliding" is defined as a set of actions; while "flipping" and then falling back to "sliding" does make sense, because flipping is more of a "boolean" transition. So in the next version I'll change it to make it posible to allow more than one bit set per axis, while what can be combined and the meaning of it. Jonas
Re: [PATCH weston] weston-simple-im: Make capitalization consistent in error messages
On Thu, Jun 16, 2016 at 09:24:56PM -0600, Yong Bakos wrote:
>
> > On Jun 16, 2016, at 12:11 PM, Bryce Harrington
> > wrote:
> >
> > On Thu, Jun 16, 2016 at 11:14:01AM -0500, Derek Foreman wrote:
> >> On 15/06/16 07:59 PM, Bryce Harrington wrote:
> >>> Signed-off-by: Bryce Harrington
> >>
> >> Super important work, proudly
> >> Reviewed-by: Derek Foreman
> >>
> >> Though maybe we should put . on the end if they're capitalized, make
> >> them complete sentences?
>
> You will think I'm crazy, but I recently sent a patch to the git crew
> about adding a period to some specific git output and, long story short,
> one of their conventions is to not use them at the end of error messages.
>
> Makes me wonder, as mundane as it may sound, if this is a greater (posix/
> unix/linux) convention.
I've seen polices both ways in other projects. I personally don't have
an opinion one way or the other if we're consistent.
One thing to consider is the policy of libraries we use, that might be
issuing warnings and error messages. If we adopt a differing policy
then those will show up as inconsistent.
> Anyway, just wanted to share that silly story, not debate the point, er,
> period.
>
> yong
>
>
> > Maybe, yeah, but I'll leave that to someone else. Looks like there's a
> > lot more formatting/wording inconsistencies in the client error messages.
> >
> > Thanks for the reviews, I've pushed the three weston ones.
> >
> > b01c31b..f013ce9 master -> master
> >
> > Bryce
> >
> >>> ---
> >>> clients/weston-simple-im.c | 6 +++---
> >>> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/clients/weston-simple-im.c b/clients/weston-simple-im.c
> >>> index 4c1d7cf..0ee505a 100644
> >>> --- a/clients/weston-simple-im.c
> >>> +++ b/clients/weston-simple-im.c
> >>> @@ -197,13 +197,13 @@ input_method_keyboard_keymap(void *data,
> >>> close(fd);
> >>>
> >>> if (!keyboard->keymap) {
> >>> - fprintf(stderr, "failed to compile keymap\n");
> >>> + fprintf(stderr, "Failed to compile keymap\n");
> >>> return;
> >>> }
> >>>
> >>> keyboard->state = xkb_state_new(keyboard->keymap);
> >>> if (!keyboard->state) {
> >>> - fprintf(stderr, "failed to create XKB state\n");
> >>> + fprintf(stderr, "Failed to create XKB state\n");
> >>> xkb_keymap_unref(keyboard->keymap);
> >>> return;
> >>> }
> >>> @@ -489,7 +489,7 @@ main(int argc, char *argv[])
> >>>
> >>> simple_im.display = wl_display_connect(NULL);
> >>> if (simple_im.display == NULL) {
> >>> - fprintf(stderr, "failed to connect to server: %m\n");
> >>> + fprintf(stderr, "Failed to connect to server: %m\n");
> >>> return -1;
> >>> }
> >>>
> >>>
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] ivi-shell: add surface_created listener after launchers
Add surface_created listener after the initialization of launchers. Otherwise, surfaces of the launchers will be added to the application layer too. Signed-off-by: Emre Ucan --- ivi-shell/hmi-controller.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c index 77278ee..df99183 100644 --- a/ivi-shell/hmi-controller.c +++ b/ivi-shell/hmi-controller.c @@ -845,9 +845,6 @@ hmi_controller_create(struct weston_compositor *ec) wl_list_insert(&hmi_ctrl->workspace_fade.layer_list, &tmp_link_layer->link); - hmi_ctrl->surface_created.notify = set_notification_create_surface; - ivi_layout_interface->add_listener_create_surface(&hmi_ctrl->surface_created); - hmi_ctrl->surface_removed.notify = set_notification_remove_surface; ivi_layout_interface->add_listener_remove_surface(&hmi_ctrl->surface_removed); @@ -1277,6 +1274,13 @@ ivi_hmi_controller_UI_ready(struct wl_client *client, ivi_layout_interface->commit_changes(); ivi_hmi_controller_add_launchers(hmi_ctrl, 256); + + /* Add surface_created listener after the initialization of launchers. +* Otherwise, surfaces of the launchers will be added to application +* layer too.*/ + hmi_ctrl->surface_created.notify = set_notification_create_surface; + ivi_layout_interface->add_listener_create_surface(&hmi_ctrl->surface_created); + hmi_ctrl->is_initialized = 1; } -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2] Add API to install protocol loggers on the server wl_display
On Thu, 7 Apr 2016 14:37:44 +0300
Giulio Camuffo wrote:
> The new wl_display_add_protocol_logger allows to set a function as
> a logger, which will get called when a new request is received or an
> event is sent.
> This is akin to setting WAYLAND_DEBUG=1, but more powerful because it
> can be enabled at run time and allows to show the log e.g. in a UI view.
>
> Signed-off-by: Giulio Camuffo
> ---
>
> v2: changed the logger function to pass a struct wl_log_message* with all
> the relevant data instead of a string.
>
> src/wayland-server-core.h | 21 ++
> src/wayland-server.c | 100
> +++---
> 2 files changed, 115 insertions(+), 6 deletions(-)
Hi,
I tried a few points in the git history around the date this was
posted, but I couldn't find a revision where this patch would apply.
This is an interesting idea indeed. A major point in understanding how
this feature can be used is the ptrace hookup you mentioned. That could
be worth adding in the commit message.
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index 9980c29..07a42fc 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -514,6 +514,27 @@ wl_shm_buffer_create(struct wl_client *client,
>
> void wl_log_set_handler_server(wl_log_func_t handler);
>
> +enum wl_protocol_logger_direction {
> +WL_PROTOCOL_LOGGER_INCOMING,
> +WL_PROTOCOL_LOGGER_OUTGOING,
How about calling these EVENT and REQUEST, to match our protocol
vocabulary?
> +};
> +
> +struct wl_log_message {
> + struct wl_resource *resource;
> + int message_opcode;
> + const struct wl_message *message;
> + int arguments_count;
> + const union wl_argument *arguments;
> +};
This is as expected. You can get the client and resource version from
the resource.
> +
> +typedef void (*wl_protocol_logger_func_t)(void *, enum
> wl_protocol_logger_direction,
> + const struct wl_log_message *);
Are the arguments of this callback documented anywhere? I couldn't find
them and they don't even have names to give a hint.
> +void wl_add_protocol_logger(struct wl_display *display,
> +wl_protocol_logger_func_t, void *user_data);
> +
> +void wl_remove_protocol_logger(struct wl_display *display,
> +wl_protocol_logger_func_t, void *user_data);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index e47ccec..18bbec8 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -95,6 +95,7 @@ struct wl_display {
> struct wl_list global_list;
> struct wl_list socket_list;
> struct wl_list client_list;
> + struct wl_list protocol_loggers;
>
> struct wl_signal destroy_signal;
> struct wl_signal create_client_signal;
> @@ -121,10 +122,45 @@ struct wl_resource {
> void *data;
> int version;
> wl_dispatcher_func_t dispatcher;
> + struct wl_resource *parent;
Wait, what is this doing here?
> +};
> +
> +struct wl_protocol_logger {
> + struct wl_list link;
> + wl_protocol_logger_func_t func;
> + void *user_data;
> };
>
> static int debug_server = 0;
>
I am very glad to see the function-local static variables gone in this
version. Those would have been pain for someone running two different
wl_displays in threads. No, I do not know of such a user, thankfully.
> +static void
> +closure_log(struct wl_resource *resource,
> + struct wl_closure *closure, int send)
How about instead of a boolean, use the enum you defined?
That would read much more obvious in the callers.
Oh but wl_closure_print() does the bool, and it's used in both server
and client code. Bummer.
> +{
> + struct wl_object *object = &resource->object;
> + struct wl_display *display = resource->client->display;
> + struct wl_protocol_logger *protocol_logger;
> + struct wl_log_message message;
> +
> + if (debug_server)
> + wl_closure_print(closure, object, send);
> +
> + if (!wl_list_empty(&display->protocol_loggers)) {
> + message.resource = resource;
> + message.message_opcode = closure->opcode;
> + message.message = closure->message;
> + message.arguments_count = closure->count;
> + message.arguments = closure->args;
> + wl_list_for_each(protocol_logger,
> + &display->protocol_loggers, link) {
> + protocol_logger->func(protocol_logger->user_data,
> + send ?
> WL_PROTOCOL_LOGGER_OUTGOING :
> +
> WL_PROTOCOL_LOGGER_INCOMING,
> + &message);
> + }
> + }
> +}
> +
> WL_EXPORT void
> wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode,
>
Re: [PATCH wayland 5/5] Add API to retrieve and iterate over the resources list of a client
On Mon, 7 Mar 2016 18:31:35 +0100
Giulio Camuffo wrote:
> To complement on the new resource created signal, this allows to
> iterate over the existing resources of a client.
> ---
> src/wayland-server-core.h | 8
> src/wayland-server.c | 23 +++
> 2 files changed, 31 insertions(+)
>
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index 9af2481..9980c29 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -221,6 +221,14 @@ void
> wl_client_add_resource_created_listener(struct wl_client *client,
> struct wl_listener *listener);
>
> +typedef void (*wl_client_for_each_resource_iterator_func_t)(struct
> wl_resource *,
> +void *user_data);
> +
> +void
> +wl_client_for_each_resource(struct wl_client *client,
> +wl_client_for_each_resource_iterator_func_t
> iterator,
> +void *user_data);
Hi,
I wrote some pondering for patch 3 about the style of the API. Not much
choice here because of the wl_map.
Would you ever want to stop iterating in the middle? Should we return a
bool or int from the func_t to signal early return?
> +
> /** \class wl_listener
> *
> * \brief A single listener for Wayland signals
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 56e17c3..e47ccec 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1584,6 +1584,29 @@ wl_client_add_resource_created_listener(struct
> wl_client *client,
> wl_signal_add(&client->resource_created_signal, listener);
> }
>
> +/** Iterate over all the resources of a client
> + *
> + * \param client The client object
> + * \param iterator The iterator function
> + * \param user_data The user data pointer
> + *
> + * The function pointed by \a iterator will be called for each
> + * resource owned by the client. The \a user_data will be passed
> + * as the second argument of the iterator function.
> + *
> + * \memberof wl_client
Is it safe to create or destroy wl_resources while iterating, or is it
forbidden?
Please, document that.
> + */
> +WL_EXPORT void
> +wl_client_for_each_resource(struct wl_client *client,
> + wl_client_for_each_resource_iterator_func_t
> iterator,
> + void *user_data)
> +{
> + /* wl_iterator_func_t passes wl_object as the first argument, and
> + * wl_resource is a wl_object, so we can safely cast it. */
> + wl_iterator_func_t it = (wl_iterator_func_t)iterator;
This function-pointer-casting is a little odd, since it will never be
cast back to the correct one before calling it. I'm sure it works as
is, but... this could really use a test in 'make check', if not wrapped
in a correct type.
> + wl_map_for_each(&client->objects, it, user_data);
> +}
> +
> /** \cond */ /* Deprecated functions below. */
>
> uint32_t
Looks good otherwise.
Thanks,
pq
pgpyuBqUwwoHa.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 4/5] Add a resource creation signal
On Mon, 7 Mar 2016 18:31:34 +0100
Giulio Camuffo wrote:
> ---
> src/wayland-server-core.h | 4
> src/wayland-server.c | 20
> 2 files changed, 24 insertions(+)
>
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index 4201b2c..9af2481 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -217,6 +217,10 @@ wl_client_get_object(struct wl_client *client, uint32_t
> id);
> void
> wl_client_post_no_memory(struct wl_client *client);
>
> +void
> +wl_client_add_resource_created_listener(struct wl_client *client,
> +struct wl_listener *listener);
> +
> /** \class wl_listener
> *
> * \brief A single listener for Wayland signals
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index a5527eb..56e17c3 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -81,6 +81,7 @@ struct wl_client {
> struct wl_signal destroy_signal;
> struct ucred ucred;
> int error;
> + struct wl_signal resource_created_signal;
> };
>
> struct wl_display {
> @@ -421,6 +422,7 @@ wl_client_create(struct wl_display *display, int fd)
> if (client == NULL)
> return NULL;
>
> + wl_signal_init(&client->resource_created_signal);
> client->display = display;
> client->source = wl_event_loop_add_fd(display->loop, fd,
> WL_EVENT_READABLE,
> @@ -1451,6 +1453,7 @@ wl_resource_create(struct wl_client *client,
> return NULL;
> }
>
> + wl_signal_emit(&client->resource_created_signal, resource);
Again, the documentation could mention the relationship to
wl_client_add_resource_created_listener().
> return resource;
> }
>
> @@ -1564,6 +1567,23 @@ wl_client_from_link(struct wl_list *link)
> return wl_container_of(link, (struct wl_client *)0, link);
> }
>
> +/** Add a listener for the client's resource creation signal
> + *
> + * \param client The client object
> + * \param listener The listener to be added
> + *
> + * When a new resource is created for this client the listener
> + * will be notified, carrying the new resource as the data argument.
> + *
> + * \memberof wl_client
> + */
> +WL_EXPORT void
> +wl_client_add_resource_created_listener(struct wl_client *client,
> + struct wl_listener *listener)
> +{
> + wl_signal_add(&client->resource_created_signal, listener);
> +}
> +
> /** \cond */ /* Deprecated functions below. */
>
> uint32_t
Could you say something about why this is better as per-client
signal/listener than a per-wl_display in the commit message?
I'm not saying it isn't, I'd just like to see some justification, as
compositor-wide instrumentation would usually be interested in all
clients. Would it not?
Once more, an almost perfect patch.
Thanks,
pq
pgpYUb3OB8fee.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 3/5] Add API to get the list of connected clients
On Mon, 7 Mar 2016 18:31:33 +0100
Giulio Camuffo wrote:
> ---
> src/wayland-server-core.h | 14 +
> src/wayland-server.c | 52
> +++
> 2 files changed, 66 insertions(+)
>
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index a4ca350..4201b2c 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -176,6 +176,20 @@ wl_global_destroy(struct wl_global *global);
> struct wl_client *
> wl_client_create(struct wl_display *display, int fd);
>
> +struct wl_list *
> +wl_display_get_client_list(struct wl_display *display);
> +
> +struct wl_list *
> +wl_client_get_link(struct wl_client *client);
> +
> +struct wl_client *
> +wl_client_from_link(struct wl_list *link);
> +
> +#define wl_client_for_each(client, list) \
> + for (client = 0, client = wl_client_from_link((list)->next);\
> + wl_client_get_link(client) != (list); \
> + client = wl_client_from_link(wl_client_get_link(client)->next))
> +
Hi,
ok, there is this style of iterating over opaque objects. Then there is
the other style you add for iterating over resources. Why are they
different? Shouldn't we use the same style for both?
That would mean we use the funcptr callback style, because wl_resources
are kept in a wl_map, not a wl_list. But should that be the exception,
is it so much harder to use?
We already have wl_resource_for_each{,_safe}() macros. One could have
written a wl_list_for_each_resource(struct wl_list *resource_list, func
iterator, void *user_data). I do not know why it wasn't made that way.
Ease of use at the cost of more function calls into the library?
Do you know why there is the 'client = 0'? It seems to be present
already in wl_resource_for_each() and I can't figure out why.
I suppose the style you chose here is the one you prefer the most?
I'm fine with both ways, though I would hope the commit message
explained the reason why go this way and not the other way.
> void
> wl_client_destroy(struct wl_client *client);
>
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 5099614..a5527eb 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1512,6 +1512,58 @@ wl_display_get_additional_shm_formats(struct
> wl_display *display)
> return &display->additional_shm_formats;
> }
>
> +/** Get the list of currently connected clients
> + *
> + * \param display The display object
> + *
> + * This function returns a pointer to the list of clients currently
> + * connected to the display. You can iterate on the list by using
> + * the \a wl_client_for_each macro.
> + *
> + * \sa wl_client_for_each()
> + * \sa wl_client_get_link()
> + * \sa wl_client_from_link()
> + *
> + * \memberof wl_display
> + */
> +WL_EXPORT struct wl_list *
> +wl_display_get_client_list(struct wl_display *display)
> +{
> + return &display->client_list;
> +}
This should document how long the returned list head is valid. First I
thought it would become invalid the moment the list is manipulated
again, but it doesn't. Shouldn't we also say one must not change the
list?
> +
> +/** Get the link by which a client is inserted in the client list
> + *
> + * \param client The client object
> + *
> + * \sa wl_client_for_each()
> + * \sa wl_display_get_client_list()
> + * \sa wl_client_from_link()
> + *
> + * \memberof wl_client
> + */
> +WL_EXPORT struct wl_list *
> +wl_client_get_link(struct wl_client *client)
> +{
> + return &client->link;
> +}
> +
> +/** Get a wl_client by its link
> + *
> + * \param link The link of a wl_client
> + *
> + * \sa wl_client_for_each()
> + * \sa wl_display_get_client_list()
> + * \sa wl_client_get_link()
> + *
> + * \memberof wl_client
> + */
> +WL_EXPORT struct wl_client *
> +wl_client_from_link(struct wl_list *link)
> +{
> + return wl_container_of(link, (struct wl_client *)0, link);
How about using container_of() instead?
wl_resource_from_link() could use the same treatment...
> +}
> +
> /** \cond */ /* Deprecated functions below. */
>
> uint32_t
Looks pretty good to me in general.
Thanks,
pq
pgpuNLUU4OGVE.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/5] Add API to retrieve the interface of a wl_resource
2016-06-17 14:09 GMT+03:00 Pekka Paalanen :
> On Mon, 7 Mar 2016 18:31:32 +0100
> Giulio Camuffo wrote:
>
>> ---
>> src/wayland-server-core.h | 2 ++
>> src/wayland-server.c | 12
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
>> index 1bc4d6b..a4ca350 100644
>> --- a/src/wayland-server-core.h
>> +++ b/src/wayland-server-core.h
>> @@ -419,6 +419,8 @@ int
>> wl_resource_instance_of(struct wl_resource *resource,
>> const struct wl_interface *interface,
>> const void *implementation);
>> +const struct wl_interface *
>> +wl_resource_get_interface(struct wl_resource *resource);
>>
>> void
>> wl_resource_add_destroy_listener(struct wl_resource *resource,
>> diff --git a/src/wayland-server.c b/src/wayland-server.c
>> index 2857b1d..5099614 100644
>> --- a/src/wayland-server.c
>> +++ b/src/wayland-server.c
>> @@ -687,6 +687,18 @@ wl_resource_get_destroy_listener(struct wl_resource
>> *resource,
>> return wl_signal_get(&resource->destroy_signal, notify);
>> }
>>
>> +/** Retrieve the interface of a resource.
>> + *
>> + * \param resource The resource object
>> + *
>> + * \memberof wl_resource
>> + */
>> +WL_EXPORT const struct wl_interface *
>> +wl_resource_get_interface(struct wl_resource *resource)
>> +{
>> + return resource->object.interface;
>> +}
>> +
>> WL_EXPORT void
>> wl_client_add_destroy_listener(struct wl_client *client,
>> struct wl_listener *listener)
>
> Hi,
>
> is it necessary to get the wl_interface pointer, or would it be enough
> to get just the interface name, similar to wl_proxy_get_class()?
The name would be enough for me, i guess for simmetry purposes it may be better.
Thanks,
Giulio
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 2/5] Add API to retrieve the interface of a wl_resource
On Mon, 7 Mar 2016 18:31:32 +0100
Giulio Camuffo wrote:
> ---
> src/wayland-server-core.h | 2 ++
> src/wayland-server.c | 12
> 2 files changed, 14 insertions(+)
>
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index 1bc4d6b..a4ca350 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -419,6 +419,8 @@ int
> wl_resource_instance_of(struct wl_resource *resource,
> const struct wl_interface *interface,
> const void *implementation);
> +const struct wl_interface *
> +wl_resource_get_interface(struct wl_resource *resource);
>
> void
> wl_resource_add_destroy_listener(struct wl_resource *resource,
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 2857b1d..5099614 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -687,6 +687,18 @@ wl_resource_get_destroy_listener(struct wl_resource
> *resource,
> return wl_signal_get(&resource->destroy_signal, notify);
> }
>
> +/** Retrieve the interface of a resource.
> + *
> + * \param resource The resource object
> + *
> + * \memberof wl_resource
> + */
> +WL_EXPORT const struct wl_interface *
> +wl_resource_get_interface(struct wl_resource *resource)
> +{
> + return resource->object.interface;
> +}
> +
> WL_EXPORT void
> wl_client_add_destroy_listener(struct wl_client *client,
> struct wl_listener *listener)
Hi,
is it necessary to get the wl_interface pointer, or would it be enough
to get just the interface name, similar to wl_proxy_get_class()?
Thanks,
pq
pgpD1Me_vGIu1.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 1/5] server: add listener API for new clients
On Mon, 7 Mar 2016 18:31:31 +0100
Giulio Camuffo wrote:
> From: Sungjae Park
>
> Using display object, Emit a signal if a new client is created.
>
> In the server-side, we can get the destroy event of a client,
> But there is no way to get the created event of it.
> Of course, we can get the client object from the global registry
> binding callbacks.
> But it can be called several times with same client object.
> And even if A client creates display object,
> (so there is a connection), The server could not know that.
> There could be more use-cases not only for this.
>
> Signed-off-by: Sung-jae Park
> ---
>
> This is the v2 of the patch by Sung-jae, i applied the incremental diff
> on the first version.
Hi Giulio,
you forgot your S-o-b. This patch accounts as Reviewed-by you, right?
>
> src/wayland-server-core.h | 4
> src/wayland-server.c | 22 ++
> 2 files changed, 26 insertions(+)
>
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index e8e1e9c..1bc4d6b 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -156,6 +156,10 @@ void
> wl_display_add_destroy_listener(struct wl_display *display,
> struct wl_listener *listener);
>
> +void
> +wl_display_add_client_created_listener(struct wl_display *display,
> + struct wl_listener *listener);
> +
> struct wl_listener *
> wl_display_get_destroy_listener(struct wl_display *display,
> wl_notify_func_t notify);
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index ae9365f..2857b1d 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -96,6 +96,7 @@ struct wl_display {
> struct wl_list client_list;
>
> struct wl_signal destroy_signal;
> + struct wl_signal create_client_signal;
>
> struct wl_array additional_shm_formats;
> };
> @@ -448,6 +449,8 @@ wl_client_create(struct wl_display *display, int fd)
>
> wl_list_insert(display->client_list.prev, &client->link);
>
> + wl_signal_emit(&display->create_client_signal, client);
wl_client_create() documentation needs to mention that listeners added
with wl_display_add_client_created_listener() will be called after the
wl_client is fully initialized, before returning from the function.
This may not be obvious otherwise.
> +
> return client;
>
> err_map:
> @@ -864,6 +867,7 @@ wl_display_create(void)
> wl_list_init(&display->registry_resource_list);
>
> wl_signal_init(&display->destroy_signal);
> + wl_signal_init(&display->create_client_signal);
>
> display->id = 1;
> display->serial = 0;
> @@ -1353,6 +1357,24 @@ wl_display_add_destroy_listener(struct wl_display
> *display,
> wl_signal_add(&display->destroy_signal, listener);
> }
>
> +/** Registers a listener for the client connection signal.
> + * When a new client object is created, \a listener will be notified,
> carring
"carrying"
> + * a pointer to the new wl_client object.
> + *
> + * \ref wl_client_create
> + * \ref wl_display
> + * \ref wl_listener
> + *
> + * \param display The display object
> + * \param listener Signal handler object
> + */
> +WL_EXPORT void
> +wl_display_add_client_created_listener(struct wl_display *display,
> + struct wl_listener *listener)
> +{
> + wl_signal_add(&display->create_client_signal, listener);
> +}
> +
> WL_EXPORT struct wl_listener *
> wl_display_get_destroy_listener(struct wl_display *display,
> wl_notify_func_t notify)
The new added API should have at least one test in 'make check'. I would
prefer that test to be about a client connecting to a listening socket,
rather than a direct call wl_client_create(). I don't think I'll
consider that a blocker for this patch, though. The same goes for all
the patches adding new API in this series.
With all those fixed I can give my R-b.
Thanks,
pq
pgp7JbFKI2S0x.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Add a new wl_proxy_set_listener function
On Sat, 19 Mar 2016 18:14:35 +0200
Giulio Camuffo wrote:
> The wl_proxy_add_listener function is ill-named, because it suggests
> that a proxy can have many listeners. Instead it can only have one so
> deprecate the old function and add a new wl_proxy_set_listener. The
> implementation of the new function is exactly the same implementation
> wl_proxy_add_listener had, and that is now implemented by calling
> wl_proxy_set_listener.
> Also make the scanner output new _set_listener functions.
Hi,
a good idea.
Missing S-o-b. I suppose this is the latest patch? It doesn't say v2.
I marked the other one as superseded.
This patch has an easy to solve conflict with the proxy wrappers patch,
and now there is also this:
tests/queue-test.c: In function ‘client_test_queue_proxy_wrapper’:
tests/queue-test.c:242:2: warning: ‘wl_callback_add_listener’ is deprecated
(declared at ./protocol/wayland-client-protocol.h:1123)
[-Wdeprecated-declarations]
wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
^
tests/queue-test.c: In function ‘client_test_queue_set_queue_race’:
tests/queue-test.c:289:2: warning: ‘wl_callback_add_listener’ is deprecated
(declared at ./protocol/wayland-client-protocol.h:1123)
[-Wdeprecated-declarations]
wl_callback_add_listener(callback, &sync_listener_roundtrip, &done);
^
> ---
> src/scanner.c | 20 ++--
> src/wayland-client-core.h | 12 +++-
> src/wayland-client.c | 23 +--
> tests/display-test.c | 2 +-
> tests/queue-test.c| 12 ++--
> tests/test-compositor.c | 4 ++--
> 6 files changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/src/scanner.c b/src/scanner.c
> index 04747e3..441ab1f 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -1267,10 +1267,10 @@ emit_structs(struct wl_list *message_list, struct
> interface *interface, enum sid
>
> if (side == CLIENT) {
> printf("static inline int\n"
> -"%s_add_listener(struct %s *%s,\n"
> +"%s_set_listener(struct %s *%s,\n"
> "%sconst struct %s_listener *listener, void *data)\n"
> "{\n"
> -"\treturn wl_proxy_add_listener((struct wl_proxy *) %s,\n"
> +"\treturn wl_proxy_set_listener((struct wl_proxy *) %s,\n"
> "%s(void (**)(void)) listener, data);\n"
> "}\n\n",
> interface->name, interface->name, interface->name,
> @@ -1278,6 +1278,22 @@ emit_structs(struct wl_list *message_list, struct
> interface *interface, enum sid
> interface->name,
> interface->name,
> indent(37));
> + printf("#ifndef WL_HIDE_DEPRECATED\n");
We cannot use WL_HIDE_DEPRECATED, because people may already be using
it, and then this change would break their code. I think we should look
into some other projects on how to deal with deprecation properly.
WL_HIDE_DEPRECATED worked only once, and no more functions can be moved
behind it.
> + printf("static inline int\n"
> +"%s_add_listener(struct %s *%s,\n"
> +"%sconst struct %s_listener *listener, void *data)
> WL_DEPRECATED;\n",
> +interface->name, interface->name, interface->name,
> +indent(14 + strlen(interface->name)), interface->name);
We can't use function attributes on definitions, only on declarations?
> + printf("static inline int\n"
> +"%s_add_listener(struct %s *%s,\n"
> +"%sconst struct %s_listener *listener, void *data)\n"
> +"{\n"
> +"\treturn %s_set_listener(%s, listener, data);\n"
> +"}\n",
> +interface->name, interface->name, interface->name,
> +indent(14 + strlen(interface->name)), interface->name,
> +interface->name, interface->name);
> + printf("#endif //WL_HIDE_DEPRECATED\n\n");
> }
> }
>
Otherwise the change to scanner looks fine.
We should probably think of a standard procedure in making deprecations
to the API. Who would like to take that task?
It would be three parts: compile time warnings, hiding API from
libwayland headers, and removing API from scanner-generated headers. I
think we might need some kind of a running version form of
WL_HIDE_DEPRECATED, one for each release, plus an option for scanner to
"stop emitting wrappers for things deprecated up to and including
release 1.x". Or should we just forget about hiding/removing deprecated
API?
Please start a new thread about this.
> diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h
> index 91f7e7b..f748a42 100644
> --- a/src/wayland-client-core.h
> +++ b/src/wayland-client-core.h
> @@ -159,7 +159,7 @@ void
> wl_proxy_destroy(struct wl_proxy *proxy);
>
> int
> -wl_proxy_add_listener(struct wl_proxy *proxy,
> +wl_proxy_set_listener(struct wl_proxy *proxy,
>
Re: [PATCH] Add a new wl_proxy_set_listener function
On Thu, 16 Jun 2016 14:33:44 -0400 Jonas Ådahl wrote: > On Thu, Jun 16, 2016 at 11:27:31AM -0500, Derek Foreman wrote: > > On 19/03/16 11:14 AM, Giulio Camuffo wrote: > > > The wl_proxy_add_listener function is ill-named, because it suggests > > > that a proxy can have many listeners. Instead it can only have one so > > > deprecate the old function and add a new wl_proxy_set_listener. The > > > implementation of the new function is exactly the same implementation > > > wl_proxy_add_listener had, and that is now implemented by calling > > > wl_proxy_set_listener. > > > Also make the scanner output new _set_listener functions. > > > > I like this, and it's > > Reviewed-by: Derek Foreman > > > > However, since set_listener is new, this is a chance to make > > proxy_set_listener() bail more aggressively (perhaps abort) on a second > > set (while still letting add_listener continue to work as it always has) > > > > Do we want to do that now before it's too late? I have no strong > > opinion either way, so I'll defer to your judgment here. > > Do we really want to? What if you want to switch listener for whatever > reason? Is there any reason we wouldn't want to allow it? > > Given that we change the name to "set", and if we make it clear that > setting a listener replaces the previous one, it should be obvious > enough that it is indeed simply replaced, I don't see any reason for not > allowing it. That's one way to go with it, sure. If we look at user_data, add/set_listener is not the only one setting it, there is also set_user_data. It seems to be a common confusion to call both set_user_data and add_listener. If you pass different pointers to each without realizing they both set the same member, you would be in for a surprise. Do we really want to hold the users' hand so much to try to detect that? I think good API design principles say we have already lost that case. It is not unthinkable to want to re-set user_data, a little less so to re-set the listener, but still no solid reason to forbid either. What is the path of least surprise? I believe that is to not forbid re-setting, so I'm with Jonas on this. My actual patch review will be in another email. Thanks, pq pgpw8r3AAKgQd.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] tests: fix the cursor race in internal-screenshot
On Thu, 16 Jun 2016 10:59:21 -0500
Derek Foreman wrote:
> On 16/06/16 05:36 AM, Pekka Paalanen wrote:
> > From: Pekka Paalanen
> >
> > This fix also depends on "compositor-headless: do not create a seat".
> >
> > If we lose the race against weston-desktop-shell setting cursors, which
> > is very rare, we get a cursor image in the screenshot, causing the test
> > to fail. This is now fixed by moving the (remaining) cursor out of the
> > way.
> >
> > Arguably we should have better solutions for this, but that is another
> > story. This is a stop-gap measure we can copy also in new
> > screenshooting tests.
> >
> > Code and explanation on how to lose the race for sure are included.
> >
> > Signed-off-by: Pekka Paalanen
> > ---
> > tests/internal-screenshot-test.c | 27 +++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/tests/internal-screenshot-test.c
> > b/tests/internal-screenshot-test.c
> > index 563aa3d..e022d20 100644
> > --- a/tests/internal-screenshot-test.c
> > +++ b/tests/internal-screenshot-test.c
> > @@ -26,6 +26,7 @@
> > #include "config.h"
> >
> > #include
> > +#include
> >
> > #include "weston-test-client-helper.h"
> >
> > @@ -48,6 +49,27 @@ draw_stuff(void *pixels, int w, int h)
> > }
> > }
> >
> > +/*
> > + * We are racing our screenshooting against weston-desktop-shell setting
> > the
> > + * cursor. If w-d-s wins, our screenshot will have a cursor shown, which
> > makes
> > + * the image comparison fail. Our window and the default pointer position
> > are
> > + * accidentally causing an overlap that intersects our test clip rectangle.
> > + *
> > + * This delay, if enabled, ensures this test loses that race.
> > + *
> > + * To actually see the failure, comment out the workaround call to
> > + * weston_test_move_pointer() below.
> > + */
> > +static void
> > +lose_the_race(void)
> > +{
> > +#if 0
> > + struct timespec delay = { .tv_sec = 0, .tv_nsec = 100 * 1000 * 1000 };
> > +
> > + nanosleep(&delay, NULL);
>
> BAAArrf. :)
>
> I don't get the play here, this is still if 0 so does nothing, so the
> race is still present.
Hi,
this is not what fixes the race. The fix is the move_pointer call
below. This is here just to remind a casual reader on how you can
ensure to lose the race and see the result.
> So we're going to propagate this "fix" to other new tests, and "just"
> set all these #if 0 to #if 1 before running make distcheck, or what?
No. lose_the_race() should not be copied anywhere. It's just something
you can enable if you start seeing random failures here, or you want to
inspect what the failure looks like without having to wait a hundred
attempts to get lucky.
I could have just left the delay be there, it's only 100 ms, and it
would make it much much more likely that the race is lost, and if the
bug fixed by this patch comes back it will be noticed soon. However,
the nanonsleep is not a guarantee, and I hate tests that include delays
because they arbitrarily make the check take longer, so I'd rather not
have it on by default. Delays are never a good solution for races.
OTOH, actually guaranteeing that w-d-s is ready before the test begins
would be useful, but quite a lot more effort for just this case. I also
did consider moving all seat and input device control interfaces from
weston-test.so into the headless-backend, so that we could just remove
all pointer devices (or rather not create them in the first place) in
tests that do not need them, which is what I could consider the proper
fix for the cursor issue. However, as my current goal is to fix some of
Weston's sub-surface issues, which lead me to wanting screenshot-based
tests, which lead to refactor the whole pixel buffer stuff in tests
(see the 11 patch series), I thought it's time to skip one rabbit hole
to get forward. So pardon my lazyness. :-P
(Oh, and there is the issue of ensuring that weston tests will *only*
use the fallback cursors from libwayland-cursor, so that custom cursor
themes won't cause false failures in tests.)
In essence, lose_the_race() function is *purely* documentation.
> I'm not really sure this is all that good even as a stop-gap measure...
Well, the real fix is the move_pointer, not the delay.
> > +#endif
> > +}
> > +
> > TEST(internal_screenshot)
> > {
> > struct wl_buffer *buf;
> > @@ -62,12 +84,17 @@ TEST(internal_screenshot)
> > bool dump_all_images = true;
> > void *pixels;
> >
> > + lose_the_race();
> > +
> > /* Create the client */
> > printf("Creating client for test\n");
> > client = create_client_and_test_surface(100, 100, 100, 100);
> > assert(client);
> > surface = client->surface->wl_surface;
> >
> > + /* Move the pointer away from the screenshot area. */
> > + weston_test_move_pointer(client->test->weston_test, 0, 0);
This line ^ here is the actual fix. I suppose I should underline that
in the commit message?
Certainly something was off in this pa
Re: [PATCH weston] clients/dmabuf-intel: use three buffers
On Mon, 13 Jun 2016 08:14:12 -0500 Yong Bakos wrote: > On Jun 13, 2016, at 6:17 AM, Pekka Paalanen wrote: > > > > From: Pekka Paalanen > > > > Use three buffers like simple-dmabuf-v4l instead of just two. > > > > This is required, because when a frame callback arrives, the just > > committed buffer is only on its way to the screen, while the previous > > buffer is still being scanned out. It will take for the page flip to > > complete, before the previous buffer is release. However, we want to be > > able to repaint already at the frame callback, so three buffers can be > > necessary. > > > > This patch fixes weston-simple-dmabuf-intel to not abort with "Both > > buffers busy at redraw()." when hardware overlays are used and the > > surface gets directly scanned out. > > > > Signed-off-by: Pekka Paalanen > > While I could not test this, it's a straightforward change, and it > does touch all cases in simple-dmabuf-intel. > > Reviewed-by: Yong Bakos Thanks for the review, pushed: be112d4..d56b94a master -> master Thanks, pq pgpb43Gr0ttRI.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] main: report presentation clock resolution
On Wed, 15 Jun 2016 14:28:09 +0200
Hardening wrote:
> Le 13/06/2016 13:34, Pekka Paalanen a écrit :
> > From: Pekka Paalanen
> >
> > For debugging weird timing issues. If your clock resolution is
> > unexpectedly e.g. 10 ms, you can be sure you will have strange timing
> > issues. This is almost certainly caused by kernel misconfiguration.
> >
> > We rely on clock_getres() being available by the same thing that gets us
> > clock_gettime(), so that no new configure.ac check is needed.
> >
> > Signed-off-by: Pekka Paalanen
> > ---
> > src/main.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/src/main.c b/src/main.c
> > index 193a845..28f6969 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -647,6 +647,7 @@ weston_compositor_log_capabilities(struct
> > weston_compositor *compositor)
> > {
> > unsigned i;
> > int yes;
> > + struct timespec res;
> >
> > weston_log("Compositor capabilities:\n");
> > for (i = 0; i < ARRAY_LENGTH(capability_strings); i++) {
> > @@ -659,6 +660,14 @@ weston_compositor_log_capabilities(struct
> > weston_compositor *compositor)
> > weston_log_continue(STAMP_SPACE "presentation clock: %s, id %d\n",
> > clock_name(compositor->presentation_clock),
> > compositor->presentation_clock);
> > +
> > + if (clock_getres(compositor->presentation_clock, &res) == 0)
> > + weston_log_continue(STAMP_SPACE
> > + "presentation clock resolution: %d.%09ld s\n",
> > + (int)res.tv_sec, res.tv_nsec);
> > + else
> > + weston_log_continue(STAMP_SPACE
> > + "presentation clock resolution: N/A\n");
> > }
> >
> > static void
> >
> Reviewed-By: David Fort
>
pushed:
e77f8ad..be112d4 master -> master
Thanks,
pq
pgpPzJtQxy2hr.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] compositor-fbdev: drop EGL support
On Thu, 16 Jun 2016 10:06:29 -0500 Derek Foreman wrote: > On 13/06/16 06:24 AM, Pekka Paalanen wrote: > > From: Pekka Paalanen > > > > EGL code was added to the fbdev backend in > > 4aa756dc7a8d3cf3b4c6f018c3e2a053fff424b0 in 2013, apparently for running > > Weston on libhybris with Android hardware drivers. > > > > This actually had nothing to do with the fbdev backend, really. Fbdev > > was just a convenient platform to plug in the EGL init code and load > > GL-renderer. Fbdev itself was not used at all in this case. > > > > Fbdev should be forgotten and die, as we have better interfaces for > > accelerated rendering and for controlling displays. It may be a bit too > > harsh to remove the whole fbdev backend just yet, but let us at least > > simplify it this much. > > > > Fbdev+EGL has been the unholy union used by proprietary driver stacks of > > hardware vendors in the non-PC world as a quick and dirty way to get > > something out on the screen. In these cases it is actually the EGL > > implementation that does everything internally, fbdev is not needed. > > Fbdev was never meant for the sort anyway. > > > > If anyone still needs this use case, I recommend sticking with a > > outdated Weston to match your outdated platform. Or if you really > > insist, write a new backend that does not pretend to use fbdev and just > > initializes EGL and GL-renderer. > > > > Cc: Adrian Negreanu > > Signed-off-by: Pekka Paalanen > > This looks good to me (modulo Quentin's suggestion, which I missed) > > Reviewed-by: Derek Foreman > > ... You have tested it, right? ;) I quickly tested it runs on inteldrmfb, yeah. I fixed the issue pointed out by Quentin and pushed: f013ce9..e77f8ad master -> master Thanks, pq > > --- > > src/compositor-fbdev.c | 97 > > -- > > src/compositor-fbdev.h | 1 - > > src/main.c | 4 +-- > > 3 files changed, 16 insertions(+), 86 deletions(-) pgpF_rMdFMsNT.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 0/4] Restore hysteresis for all touchpads
Hi, On 17-06-16 05:17, Peter Hutterer wrote: Already enabled for all touchpads but the ones labelled as precise (which is just the apples and the lenovo *40 and later series anyway). Even with those we still see some unexpected pointer movement (e.g. #94379). The small benefit we get from removing the hysteresis isn't worth those drawbacks, so just bring it back for all touchpads. The first three patches are just reverts for additions caused by the hysteresis removal, they are not needed anymore now. Series looks good to me: Reviewed-by: Hans de Goede Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 1/2] test: when creating an abs test device, force the abs->value to the mimimum
Hi,
On 16-06-16 06:36, Peter Hutterer wrote:
Otherwise the abs->value could lie outside the [min, max] range of the axis.
This isn't much of an issue for actual axes but in the case of ABS_MT_SLOT
(value 47) it causes errors when libevdev sanitises the event into the allowed
slot range.
Signed-off-by: Peter Hutterer
Series looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
---
test/litest.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/test/litest.c b/test/litest.c
index 66f08f2..7ccfbf7 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -2250,8 +2250,12 @@ litest_create_uinput(const char *name,
abs = abs_info;
while (abs && abs->value != -1) {
- rc = libevdev_enable_event_code(dev, EV_ABS,
- abs->value, abs);
+ struct input_absinfo a = *abs;
+
+ /* abs_info->value is used for the code and may be outside
+ of [min, max] */
+ a.value = abs->minimum;
+ rc = libevdev_enable_event_code(dev, EV_ABS, abs->value, &a);
litest_assert_int_eq(rc, 0);
abs++;
}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Override redirect windows with keyboard grabs on Xwayland
Hi all, First of all, sorry for cross posting, but I am not really sure where the issue is to be addressed... I am looking into https://bugs.freedesktop.org/show_bug.cgi?id=96547 and concluded this is actually the same as https://bugzilla.gnome.org/show_bug.cgi?id=752956 Basically, long story short, we have clients in X that map a full-screen override redirect window with a keyboard/mouse active grab for the user to enter a password. Because override redirect windows are not managed by X11 window managers, these won't focus the O-R window by themselves but that's not a problem on X11 thanks to the active grab. Things get more complicated with those clients on Xwayland, because the X11 active grab cannot be honoured in Wayland, and therefore the window may or may not get focused automatically, depending on the compositor/WM. In the worst case, the user might type his/her password in some other (focused but hidden underneath) window without even knowing it! I am not sure how or even where to address such cases... Suggestions welcome. PS: weston and gnome-shell differ here, weston will give focus to the O-R window but will obviously allow for alt-tab and can sometimes segfault afterwards - that's a bug in weston, but off topic for now - gnome-shell will *not* focus the O-R window and seems right in doing to, but that obviously breaks those apps that map an O-R window with a grab on the keyboard. Cheers, Olivier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
