Re: [PATCH] Introduce set_size_hints xdg_surface request.

2014-08-07 Thread Jari Vetoniemi
Relying only on maximized/fullscreen states to do tiling or hiding
decorations feels bit nasty, as for client developers it's documented
that fullscreen and maximized are indeed what they are. For example
fullscreen application such as video player could hide controls in
fullscreen mode, which is not desirable. These all would be relying on
implementation detail that could break if client developer did things
bit differently, but still according to documentation.

As for maximum size, only use case I could come up with it is for
widget sort of applications that are supposed to be certain size, but
I would not bet my life on its usefulness.

>We are working things on one step at a time. Jari was working on a tiling WM 
>using the maximized state.

Yeah, I'll keep working on this more so we can pinpoint all the
problem points implementing tiling wms with the current interfaces.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] Introduce set_size_hints xdg_surface request.

2014-08-07 Thread Pekka Paalanen
On Tue,  5 Aug 2014 22:07:39 +0300
Jari Vetoniemi  wrote:

> Rationale for this patch is to provide way for applications that can't go 
> below/above
> certain bounds to inform compositor about it.
> 
> More feedback is needed. For example are maximimum size useful for any
> particular application?
> ---
>  protocol/xdg-shell.xml | 18 ++
>  1 file changed, 18 insertions(+)

Hi,

I have a few technical questions.

> 
> diff --git a/protocol/xdg-shell.xml b/protocol/xdg-shell.xml
> index bd36231..6e48ae6 100644
> --- a/protocol/xdg-shell.xml
> +++ b/protocol/xdg-shell.xml
> @@ -235,6 +235,24 @@
>
>  
>  
> +
> +  
> +Server should use these hints to figure out how much surface may be,
> +resized so that the content still remains sane. Hints will remain
> +the same until set_size_hints is requested again.
> +
> +Hints set this way are double buffered. They will get applied on
> +the next commit.
> +
> +These hints should take effect regardless of the surface state.
> +A size less than zero unsets the hint.

What is the default value of these hints?

When is the client expected to send this? Can this be sent before
the window gets mapped? After the window was unmapped?

How does this interact with the protocol sequence to decide the initial
size of a window before it gets mapped?

Making surface state double-buffered is desired for all state that
immediately affects the surface's presentation. However, these are
hints and do not change how the surface is presented; they change what
sizes the compositor may ask for in configure events. It that sense, it
is not necessary to make this state double-buffered, and I think not
double-buffering (not demanding a commit) will make it easier to use
and also to define how it works with the initial size negotiation.


Thanks,
pq

> +  
> +  
> +  
> +  
> +  
> +
> +
>  
>
>  The different state values used on the surface. This is designed for

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


Re: [PATCH wayland 6/6] server: Add a simple API to find a good default display

2014-08-07 Thread Pekka Paalanen
On Fri, 18 Jul 2014 11:05:03 +0200
Marek Chalupa  wrote:

> On 17 July 2014 19:54, Jasper St. Pierre  wrote:
> 
> > This allows compositors to easily select a good display to listen on.
> > ---
> >  src/wayland-server.c | 97
> > ++--
> >  src/wayland-server.h |  1 +
> >  2 files changed, 73 insertions(+), 25 deletions(-)

Hi,

the final series that actually got pushed was never on the mailing
list, so sorry for the crappy quote.

I have one observation.


   server: Add a simple API to find a good default display

This allows compositors to easily select a good display to listen on.

- src/wayland-server.c -
index b3929ed..39d29ec 100644
@@ -1093,19 +1093,90 @@ wl_socket_init_for_display_name(struct wl_socket *s, 
const char *name)
errno = ENAMETOOLONG;
return -1;
};
 
return 0;
 }
 
+static int
+_wl_display_add_socket(struct wl_display *display, struct wl_socket *s)
+{
+   socklen_t size;
+
+   s->fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
+   if (s->fd < 0) {
+   return -1;
+   }
+
+   size = offsetof (struct sockaddr_un, sun_path) + 
strlen(s->addr.sun_path);
+   if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0) {
+   wl_log("bind() failed with error: %m\n");
+   return -1;
+   }
+
+   if (listen(s->fd, 1) < 0) {
+   wl_log("listen() failed with error: %m\n");
+   return -1;
+   }
+
+   s->source = wl_event_loop_add_fd(display->loop, s->fd,
+WL_EVENT_READABLE,
+socket_data, display);
+   if (s->source == NULL) {
+   return -1;
+   }
+
+   wl_list_insert(display->socket_list.prev, &s->link);
+   return 0;
+}
+
+WL_EXPORT const char *
+wl_display_add_socket_auto(struct wl_display *display)
+{
+   struct wl_socket *s;
+   int displayno = 0;
+   char display_name[16] = "";
+
+   /* A reasonable number of maximum default sockets. If
+* you need more than this, use the explicit add_socket API. */
+   const int MAX_DISPLAYNO = 32;
+
+   s = malloc(sizeof *s);
+   memset(s, 0, sizeof *s);
+   if (s == NULL)
+   return NULL;
+
+   do {
+   snprintf(display_name, sizeof display_name, "wayland-%d", 
displayno);
+   if (wl_socket_init_for_display_name(s, display_name) < 0) {
+   wl_socket_destroy(s);
+   return NULL;
+   }
+
+   if (wl_socket_lock(s) < 0)
+   continue;
+
+   if (_wl_display_add_socket(display, s) < 0) {
+   wl_socket_destroy(s);
+   return NULL;
+   }

Let's imagine:

First iteration: socket already used => wl_socket_lock() fails.
Second iteration: wl_socket_init_for_display_name() happens to fail.
=> wl_socket_destroy() unlinks the existing socket's lock file.

The trick here is that reading what wl_socket_init_for_display_name()
does, it probably cannot fail on the second iteration. Would be nice to
have at least a comment here to that effect, otherwise some change in
the future may break that, as to me it seems fragile.

Or better, wl_socket_lock() should reset the lock file name before
returning an error.

+
+   return s->display_name;
+   } while (displayno++ < MAX_DISPLAYNO);
+
+   /* Ran out of display names. */
+   wl_socket_destroy(s);
+   errno = EINVAL;
+   return NULL;
+}
+
 WL_EXPORT int
 wl_display_add_socket(struct wl_display *display, const char *name)
 {
struct wl_socket *s;
-   socklen_t size;
 
s = malloc(sizeof *s);
memset(s, 0, sizeof *s);
if (s == NULL)
return -1;
 
if (name == NULL)
@@ -1119,42 +1190,19 @@ wl_display_add_socket(struct wl_display *display, const 
char *name)
}
 
if (wl_socket_lock(s) < 0) {
wl_socket_destroy(s);


Hmm, I think there is a similar problem here. If lock fails, the
existing lock file will be unlinked from under the existing compositor.


Thanks,
pq

return -1;
}
 
-   s->fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
-   if (s->fd < 0) {
+   if (_wl_display_add_socket(display, s) < 0) {
wl_socket_destroy(s);
return -1;
}
 
-   size = offsetof (struct sockaddr_un, sun_path) + 
strlen(s->addr.sun_path);
-   if (bind(s->fd, (struct sockaddr *) &s->addr, size) < 0) {
-   wl_log("bind() failed with error: %m\n");
-   wl_socket_destroy(s);
-   return -1;
-   }
-
-   if (listen(s->fd, 1) < 0) {
-   wl_log("listen() failed with error: %m\n");
-   wl_socket_destroy(s);
-

Re: [PATCH wayland 3/6] server: Split out code to initialize the socket address for a display name

2014-08-07 Thread Pekka Paalanen
On Thu, 17 Jul 2014 13:54:21 -0400
"Jasper St. Pierre"  wrote:

> We'll use this to autodetect a good socket to open on.
> ---
>  src/wayland-server.c | 41 ++---
>  1 file changed, 26 insertions(+), 15 deletions(-)

79b1d2039aeb77b712cf4e1bb4049ebf9c453b59 is the first bad commit
commit 79b1d2039aeb77b712cf4e1bb4049ebf9c453b59
Author: Jasper St. Pierre 
Date:   Thu May 8 10:25:13 2014 -0400

server: Split out code to initialize the socket address for a display name

We'll use this to autodetect a good socket to open on.


This patch breaks 'make check' on socket-test.

error: socket path 
"/home/pq/local/tmp/"
 plus null terminator exceeds 108 bytes
socket-test: tests/socket-test.c:79: socket_path_overflow_server_create: 
Assertion `(*__errno_location ()) == 36' failed.
test "socket_path_overflow_server_create":  signal 6, fail.


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


Re: [PATCH] server: fix error handling when adding socket

2014-08-07 Thread Pekka Paalanen
On Wed,  6 Aug 2014 11:21:59 +0200
Marek Chalupa  wrote:

> When some function during adding socket fails, it must clean
> everything it set or we can get funky errors.
> 
> This patch fixes:
> http://lists.freedesktop.org/archives/wayland-devel/2014-August/016331.html
> 
> Signed-off-by: Marek Chalupa 

Hi,

both patches pushed!

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


Re: [PATCH] server: move memset after check

2014-08-07 Thread Pekka Paalanen
On Wed,  6 Aug 2014 11:28:34 +0200
Marek Chalupa  wrote:

> If the malloc fails, memset would touch invalid memory.
> ---
>  src/wayland-server.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 75de313..3c162d4 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1150,10 +1150,11 @@ wl_display_add_socket_auto(struct wl_display *display)
>   const int MAX_DISPLAYNO = 32;
>  
>   s = malloc(sizeof *s);
> - memset(s, 0, sizeof *s);
>   if (s == NULL)
>   return NULL;
>  
> + memset(s, 0, sizeof *s);
> +
>   do {
>   snprintf(display_name, sizeof display_name, "wayland-%d", 
> displayno);
>   if (wl_socket_init_for_display_name(s, display_name) < 0) {


Hi,

patch pushed!


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


Re: [PATCH wayland 3/6] server: Split out code to initialize the socket address for a display name

2014-08-07 Thread Pekka Paalanen
On Thu, 7 Aug 2014 15:41:10 +0300
Pekka Paalanen  wrote:

> On Thu, 17 Jul 2014 13:54:21 -0400
> "Jasper St. Pierre"  wrote:
> 
> > We'll use this to autodetect a good socket to open on.
> > ---
> >  src/wayland-server.c | 41 ++---
> >  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> 79b1d2039aeb77b712cf4e1bb4049ebf9c453b59 is the first bad commit
> commit 79b1d2039aeb77b712cf4e1bb4049ebf9c453b59
> Author: Jasper St. Pierre 
> Date:   Thu May 8 10:25:13 2014 -0400
> 
> server: Split out code to initialize the socket address for a display name
> 
> We'll use this to autodetect a good socket to open on.
> 
> 
> This patch breaks 'make check' on socket-test.
> 
> error: socket path 
> "/home/pq/local/tmp/"
>  plus null terminator exceeds 108 bytes
> socket-test: tests/socket-test.c:79: socket_path_overflow_server_create: 
> Assertion `(*__errno_location ()) == 36' failed.
> test "socket_path_overflow_server_create":  signal 6, fail.
> 

And this is now fixed with Marek Chalupa's patch that was already on
the list. Thanks for pointing that out, Jasper.
- pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] desktop-shell: don't assume there's a pointer when mapping a popup

2014-08-07 Thread Pekka Paalanen
On Wed, 06 Aug 2014 11:51:26 +0200
Jonny Lamb  wrote:

> On mer, 2014-08-06 at 11:50 +0200, Jonny Lamb wrote:
> > -   if (shseat->seat->pointer->grab_serial == shsurf->popup.serial) {
> > +   if (shseat->seat->pointer &&
> > +   shseat->seat->pointer->grab_serial == shsurf->popup.serial) {
> 
> This should also be applied to the 1.5 branch (it applies cleanly).
> 

It indeed did apply. Pushed to both master and 1.5 branches.


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


Re: [PATCH 2/2] server: fix conditions for fds in wl_socket_destroy

2014-08-07 Thread Pekka Paalanen
On Wed,  6 Aug 2014 12:18:12 +0200
Marek Chalupa  wrote:

> The only value that is false with the former condition is 0.
> On error we set fd to -1.
> ---
>  src/wayland-server.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index ce7bbff..c8aecb8 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -848,11 +848,11 @@ wl_socket_destroy(struct wl_socket *s)
>   wl_event_source_remove(s->source);
>   if (s->addr.sun_path[0])
>   unlink(s->addr.sun_path);
> - if (s->fd)
> + if (s->fd > 0)
>   close(s->fd);
>   if (s->lock_addr[0])
>   unlink(s->lock_addr);
> - if (s->fd_lock)
> + if (s->fd_lock > 0)
>   close(s->fd_lock);
>  
>   free(s);

Hi,

also fd 0 is valid. Unfortunately, allowing to close fd 0 here leads to
'make check' failure, as the test accidentally closes stdout_fileno,
because the fds are not initialized to -1.

I will send a replacement patch.


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


[PATCH wayland] server: fix conditions for fds in wl_socket_destroy

2014-08-07 Thread Pekka Paalanen
From: Pekka Paalanen 

0 is also a valid fd, and needs to be closed.

On error we set fd to -1. We need to also initialize fds to -1, so we do
not accidentally close stdout on error.

While fixing this, also remove one use-before-NULL-check.

Based on the patch by Marek.

Cc: Marek Chalupa 
Signed-off-by: Pekka Paalanen 
---
 src/wayland-server.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/wayland-server.c b/src/wayland-server.c
index 3c162d4..3bd8e68 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -848,16 +848,32 @@ wl_socket_destroy(struct wl_socket *s)
wl_event_source_remove(s->source);
if (s->addr.sun_path[0])
unlink(s->addr.sun_path);
-   if (s->fd)
+   if (s->fd >= 0)
close(s->fd);
if (s->lock_addr[0])
unlink(s->lock_addr);
-   if (s->fd_lock)
+   if (s->fd_lock >= 0)
close(s->fd_lock);
 
free(s);
 }
 
+static struct wl_socket *
+wl_socket_alloc(void)
+{
+   struct wl_socket *s;
+
+   s = malloc(sizeof *s);
+   if (!s)
+   return NULL;
+
+   memset(s, 0, sizeof *s);
+   s->fd = -1;
+   s->fd_lock = -1;
+
+   return s;
+}
+
 WL_EXPORT void
 wl_display_destroy(struct wl_display *display)
 {
@@ -1149,12 +1165,10 @@ wl_display_add_socket_auto(struct wl_display *display)
 * you need more than this, use the explicit add_socket API. */
const int MAX_DISPLAYNO = 32;
 
-   s = malloc(sizeof *s);
+   s = wl_socket_alloc();
if (s == NULL)
return NULL;
 
-   memset(s, 0, sizeof *s);
-
do {
snprintf(display_name, sizeof display_name, "wayland-%d", 
displayno);
if (wl_socket_init_for_display_name(s, display_name) < 0) {
@@ -1184,8 +1198,7 @@ wl_display_add_socket(struct wl_display *display, const 
char *name)
 {
struct wl_socket *s;
 
-   s = malloc(sizeof *s);
-   memset(s, 0, sizeof *s);
+   s = wl_socket_alloc();
if (s == NULL)
return -1;
 
-- 
1.8.5.5

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


[PATCH wayland] server: Don't expose wl_display as a global

2014-08-07 Thread Jasper St. Pierre
The idea here was that once upon a time, clients could rebind wl_display
to a higher version, so we offered the ability to rebind it
here. However, this is particularly broken. The existing bind
implementation actually still hardcodes version numbers, and it leaks
previous resources, overwriting the existing one.

The newly bound resource *also* won't have any listeners attached by the
client, meaning that the error and delete_id events won't get delivered
correctly. Unless the client poked into libwayland internals, it also
can't possibly set up these handlers correctly either, so the client
will sustain errors and leak all deleted globals.

Since this never worked correctly in the first place, we can feel safe
removing it.
---
 src/wayland-server.c | 28 
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/src/wayland-server.c b/src/wayland-server.c
index 3c162d4..dc3f502 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -380,9 +380,8 @@ wl_client_get_display(struct wl_client *client)
return client->display;
 }
 
-static void
-bind_display(struct wl_client *client,
-void *data, uint32_t version, uint32_t id);
+static int
+bind_display(struct wl_client *client, struct wl_display *display);
 
 /** Create a client for the given file descriptor
  *
@@ -440,9 +439,7 @@ wl_client_create(struct wl_display *display, int fd)
goto err_map;
 
wl_signal_init(&client->destroy_signal);
-   bind_display(client, display, 1, 1);
-
-   if (!client->display_resource)
+   if (bind_display(client, display) < 0)
goto err_map;
 
wl_list_insert(display->client_list.prev, &client->link);
@@ -772,22 +769,20 @@ destroy_client_display_resource(struct wl_resource 
*resource)
resource->client->display_resource = NULL;
 }
 
-static void
-bind_display(struct wl_client *client,
-void *data, uint32_t version, uint32_t id)
+static int
+bind_display(struct wl_client *client, struct wl_display *display)
 {
-   struct wl_display *display = data;
-
client->display_resource =
-   wl_resource_create(client, &wl_display_interface, 1, id);
+   wl_resource_create(client, &wl_display_interface, 1, 1);
if (client->display_resource == NULL) {
wl_client_post_no_memory(client);
-   return;
+   return -1;
}
 
wl_resource_set_implementation(client->display_resource,
   &display_interface, display,
   destroy_client_display_resource);
+   return 0;
 }
 
 /** Create Wayland display object.
@@ -831,13 +826,6 @@ wl_display_create(void)
 
wl_array_init(&display->additional_shm_formats);
 
-   if (!wl_global_create(display, &wl_display_interface, 1,
- display, bind_display)) {
-   wl_event_loop_destroy(display->loop);
-   free(display);
-   return NULL;
-   }
-
return display;
 }
 
-- 
2.0.4

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


Re: [PATCH wayland] server: fix conditions for fds in wl_socket_destroy

2014-08-07 Thread Jasper St. Pierre
Reviewed-by: Jasper St. Pierre 


On Thu, Aug 7, 2014 at 9:51 AM, Pekka Paalanen  wrote:

> From: Pekka Paalanen 
>
> 0 is also a valid fd, and needs to be closed.
>
> On error we set fd to -1. We need to also initialize fds to -1, so we do
> not accidentally close stdout on error.
>
> While fixing this, also remove one use-before-NULL-check.
>
> Based on the patch by Marek.
>
> Cc: Marek Chalupa 
> Signed-off-by: Pekka Paalanen 
> ---
>  src/wayland-server.c | 27 ---
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 3c162d4..3bd8e68 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -848,16 +848,32 @@ wl_socket_destroy(struct wl_socket *s)
> wl_event_source_remove(s->source);
> if (s->addr.sun_path[0])
> unlink(s->addr.sun_path);
> -   if (s->fd)
> +   if (s->fd >= 0)
> close(s->fd);
> if (s->lock_addr[0])
> unlink(s->lock_addr);
> -   if (s->fd_lock)
> +   if (s->fd_lock >= 0)
> close(s->fd_lock);
>
> free(s);
>  }
>
> +static struct wl_socket *
> +wl_socket_alloc(void)
> +{
> +   struct wl_socket *s;
> +
> +   s = malloc(sizeof *s);
> +   if (!s)
> +   return NULL;
> +
> +   memset(s, 0, sizeof *s);
> +   s->fd = -1;
> +   s->fd_lock = -1;
> +
> +   return s;
> +}
> +
>  WL_EXPORT void
>  wl_display_destroy(struct wl_display *display)
>  {
> @@ -1149,12 +1165,10 @@ wl_display_add_socket_auto(struct wl_display
> *display)
>  * you need more than this, use the explicit add_socket API. */
> const int MAX_DISPLAYNO = 32;
>
> -   s = malloc(sizeof *s);
> +   s = wl_socket_alloc();
> if (s == NULL)
> return NULL;
>
> -   memset(s, 0, sizeof *s);
> -
> do {
> snprintf(display_name, sizeof display_name, "wayland-%d",
> displayno);
> if (wl_socket_init_for_display_name(s, display_name) < 0) {
> @@ -1184,8 +1198,7 @@ wl_display_add_socket(struct wl_display *display,
> const char *name)
>  {
> struct wl_socket *s;
>
> -   s = malloc(sizeof *s);
> -   memset(s, 0, sizeof *s);
> +   s = wl_socket_alloc();
> if (s == NULL)
> return -1;
>
> --
> 1.8.5.5
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>



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


Re: [PATCH] Introduce set_size_hints xdg_surface request.

2014-08-07 Thread Jari Vetoniemi
> What is the default value of these hints?

I would propose -1 as default, meaning unset.
Compositor has to deal with not knowing the minimum size, preferably
this would mean though that the client can be set to any valid size.

> When is the client expected to send this? Can this be sent before
> the window gets mapped? After the window was unmapped?

I would say this hint could be sent any time after surface object is created.

> How does this interact with the protocol sequence to decide the initial
> size of a window before it gets mapped?

If the maximum was less than initial size, this would need thinking.
For now it seems though that maximum might go away?

Needs further discussion.

> Making surface state double-buffered is desired for all state that
> immediately affects the surface's presentation. However, these are
> hints and do not change how the surface is presented; they change what
> sizes the compositor may ask for in configure events. It that sense, it
> is not necessary to make this state double-buffered, and I think not
> double-buffering (not demanding a commit) will make it easier to use
> and also to define how it works with the initial size negotiation.

I agree with this, I don't see any reason for this to be
double-buffered either to be honest.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 0/6] Introduce API to automatically find a good default display

2014-08-07 Thread Jason Ekstrand
Sorry for not posting to the ML, but this looked good so I pushed it.
--Jason


On Thu, Jul 17, 2014 at 10:54 AM, Jasper St. Pierre 
wrote:

> This patch series introduces a new API to find an open Wayland display
> so that compositors don't need to reimplement all this code. This way,
> the code searching for an appropriate socket can be shared between all
> compositors.
>
> Jasper St. Pierre (6):
>   server: Clean up socket destruction
>   server: Create the socket FD after taking the lock
>   server: Split out code to initialize the socket address for a display
> name
>   server: Make get_socket_lock operate directly on the socket's lock_fd
>   server: Save the display name in the wl_socket
>   server: Add a simple API to find a good default display
>
>  src/wayland-server.c | 170
> ++-
>  src/wayland-server.h |   1 +
>  2 files changed, 116 insertions(+), 55 deletions(-)
>
> --
> 2.0.1
>
> ___
> 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: wayland: new socket addition

2014-08-07 Thread Jason Ekstrand
On Wed, Aug 6, 2014 at 1:31 AM, Marek Chalupa  wrote:

> Hi,
>
> After the new patches for socket automatic naming the socket test, namely
> socket_path_overflow_server_create, fails. Probably introduced in
> 79b1d2039aeb77b712cf4e1bb4049ebf9c453b59. It does not set errno as
> expected:
>
> error: socket path "/run/user/1000/aa
> aa"
> plus null terminator exceeds 108 bytes
> lt-socket-test: tests/socket-test.c:83: socket_path_overflow_server_create:
> Assertion `(*__errno_location ()) == 36' failed.
> zsh: abort  ./socket-test socket_path_overflow_server_create
>

What exactly is going on here?


>
> Moreover, there's a bug. When you add a socket, then add the same socket
> again, the original socket is removed, i. e.:
>
> /* OK */
> wl_display_add_socket(display, "wayland-0");
>
> /* this call fails and will delete the original socket */
> wl_display_add_socket(display, "wayland-0");
>

Yes, we need to fix this.  We should be able to add multiple sockets.


>
> I wrote tests for it, sending them right away.
>
> And one more thing... well, rather idea. Maybe it'd be nice to print:
>
>unable to lock lockfile /run/user/1000/wayland-0.lock, maybe another
> compositor is running
>unable to lock lockfile /run/user/1000/wayland-1.lock, maybe another
> compositor is running
>unable to lock lockfile /run/user/1000/wayland-2.lock, maybe another
> compositor is running
>  etc. ...
>
> only when manually adding the socket. It's really annoying when the
> terminal gets flooded
> by 30 lines of this. But it's not a big deal either.
>

Sure, we could use wl_log for that.
--Jason Ekstrand


>
>
> Regards,
> Marek Chalupa
> ___
> 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] Introduce set_size_hints xdg_surface request.

2014-08-07 Thread Bill Spitzak

On 08/07/2014 07:18 AM, Jari Vetoniemi wrote:
>> What is the default value of these hints?

I think you could get away with defaulting the minimum to be about 
100x100. I think that is sufficient to get clients to work ok and it is 
small enough to encourage them to set this rather than relying on the 
default.


If the surface is currently smaller than the minimum it should act like 
the current size is the minimum.



How does this interact with the protocol sequence to decide the initial
size of a window before it gets mapped?


These should have no effect. The client is free to supply sizes outside 
the range it claims. And the compositor is free to request them. It is 
only a hint.


Under ICCCM and some X window managers it was literally impossible to 
change the minimum size because they refused to set it smaller than the 
window, and would also refuse to set the window smaller than the 
minimum. That was very frustrating. Please don't do things like that.

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


[PATCH weston 4/4] screenshooter: Add a missing return; in an error path

2014-08-07 Thread Jasper St. Pierre
---
 src/screenshooter.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/screenshooter.c b/src/screenshooter.c
index 9ae0d29..4403933 100644
--- a/src/screenshooter.c
+++ b/src/screenshooter.c
@@ -265,6 +265,7 @@ bind_shooter(struct wl_client *client,
if (client != shooter->client) {
wl_resource_post_error(resource, 
WL_DISPLAY_ERROR_INVALID_OBJECT,
   "screenshooter failed: permission 
denied");
+   return;
}
 
wl_resource_set_implementation(resource, &screenshooter_implementation,
-- 
2.0.4

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


[PATCH weston 2/4] Don't bother destroying resources after sending an error

2014-08-07 Thread Jasper St. Pierre
An error makes the client exit, which cleans up the resources anyway.
---
 desktop-shell/input-panel.c | 1 -
 desktop-shell/shell.c   | 2 --
 src/screenshooter.c | 1 -
 src/text-backend.c  | 2 --
 4 files changed, 6 deletions(-)

diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 47bd73c..435cd5d 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -356,7 +356,6 @@ bind_input_panel(struct wl_client *client,
 
wl_resource_post_error(resource, WL_DISPLAY_ERROR_INVALID_OBJECT,
   "interface object already bound");
-   wl_resource_destroy(resource);
 }
 
 void
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 5a613f1..250cf88 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -5347,7 +5347,6 @@ bind_desktop_shell(struct wl_client *client,
 
wl_resource_post_error(resource, WL_DISPLAY_ERROR_INVALID_OBJECT,
   "permission to bind desktop_shell denied");
-   wl_resource_destroy(resource);
 }
 
 static void
@@ -5431,7 +5430,6 @@ bind_screensaver(struct wl_client *client,
 
wl_resource_post_error(resource, WL_DISPLAY_ERROR_INVALID_OBJECT,
   "interface object already bound");
-   wl_resource_destroy(resource);
 }
 
 struct switcher {
diff --git a/src/screenshooter.c b/src/screenshooter.c
index 369e920..9ae0d29 100644
--- a/src/screenshooter.c
+++ b/src/screenshooter.c
@@ -265,7 +265,6 @@ bind_shooter(struct wl_client *client,
if (client != shooter->client) {
wl_resource_post_error(resource, 
WL_DISPLAY_ERROR_INVALID_OBJECT,
   "screenshooter failed: permission 
denied");
-   wl_resource_destroy(resource);
}
 
wl_resource_set_implementation(resource, &screenshooter_implementation,
diff --git a/src/text-backend.c b/src/text-backend.c
index d6a6f3b..1d549d4 100644
--- a/src/text-backend.c
+++ b/src/text-backend.c
@@ -790,14 +790,12 @@ bind_input_method(struct wl_client *client,
if (input_method->input_method_binding != NULL) {
wl_resource_post_error(resource, 
WL_DISPLAY_ERROR_INVALID_OBJECT,
   "interface object already bound");
-   wl_resource_destroy(resource);
return;
}
 
if (text_backend->input_method.client != client) {
wl_resource_post_error(resource, 
WL_DISPLAY_ERROR_INVALID_OBJECT,
   "permission to bind input_method 
denied");
-   wl_resource_destroy(resource);
return;
}
 
-- 
2.0.4

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


[PATCH weston 1/4] Use the named SINCE_VERSION defines for version checks

2014-08-07 Thread Jasper St. Pierre
To make our code more clear.
---
 src/compositor.c | 4 ++--
 src/input.c  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index 55c959e..4d6a02a 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3145,7 +3145,7 @@ bind_output(struct wl_client *client,
output->subpixel,
output->make, output->model,
output->transform);
-   if (version >= 2)
+   if (version >= WL_OUTPUT_SCALE_SINCE_VERSION)
wl_output_send_scale(resource,
 output->current_scale);
 
@@ -3157,7 +3157,7 @@ bind_output(struct wl_client *client,
mode->refresh);
}
 
-   if (version >= 2)
+   if (version >= WL_OUTPUT_DONE_SINCE_VERSION)
wl_output_send_done(resource);
 }
 
diff --git a/src/input.c b/src/input.c
index aaa2223..b6cd7df 100644
--- a/src/input.c
+++ b/src/input.c
@@ -1829,7 +1829,7 @@ bind_seat(struct wl_client *client, void *data, uint32_t 
version, uint32_t id)
caps |= WL_SEAT_CAPABILITY_TOUCH;
 
wl_seat_send_capabilities(resource, caps);
-   if (version >= 2)
+   if (version >= WL_SEAT_NAME_SINCE_VERSION)
wl_seat_send_name(resource, seat->seat_name);
 }
 
-- 
2.0.4

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


[PATCH weston 3/4] desktop-shell: Add a missing return; in an error path

2014-08-07 Thread Jasper St. Pierre
---
 desktop-shell/shell.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 250cf88..7370972 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3741,6 +3741,7 @@ xdg_get_xdg_popup(struct wl_client *client,
wl_resource_post_error(surface_resource,
   WL_DISPLAY_ERROR_INVALID_OBJECT,
   "xdg_shell::get_xdg_popup requires a 
parent shell surface");
+   return;
}
 
parent = wl_resource_get_user_data(parent_resource);
-- 
2.0.4

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


[PATCH libinput 2/2 v2] tablet: Add libinput_tool_has_axis() and tests

2014-08-07 Thread Stephen Chandler Paul
Because the axes that tool reports can change depending on the tool in use, we
want to be able to provide functionality to determine which axes each tool can
support.

Signed-off-by: Stephen Chandler Paul 
---

Changes:
- Added a description to the patch
- Fixed all of the memory leaks, passes make check with flying
  colors now

 src/evdev-tablet.c | 42 +
 src/evdev-tablet.h |  1 +
 src/libinput-private.h |  1 +
 src/libinput.c |  7 +
 src/libinput.h | 26 ++
 test/tablet.c  | 72 ++
 6 files changed, 149 insertions(+)

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 43da67a..5f8a466 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -298,6 +298,40 @@ tablet_get_tool(struct tablet_dispatch *tablet,
.refcount = 1,
};
 
+   /* Determine the axis capabilities of the tool. Here's a break
+* down of the heuristics used here:
+* - The Wacom art pen supports all of the extra axes, along
+*   with rotation
+* - All of normal pens and the airbrush support all of the
+*   extra axes if the tablet can report them
+* - All of the mouse like devices don't really report any of
+*   the extra axes except for rotation.
+* (as of writing this comment, rotation isn't supported, so you
+* won't see the mouse or art pen here)
+*/
+   switch (type) {
+   case LIBINPUT_TOOL_PEN:
+   case LIBINPUT_TOOL_ERASER:
+   case LIBINPUT_TOOL_PENCIL:
+   case LIBINPUT_TOOL_BRUSH:
+   case LIBINPUT_TOOL_AIRBRUSH:
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TOOL_AXIS_FLAG_PRESSURE))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TOOL_AXIS_FLAG_PRESSURE);
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TOOL_AXIS_FLAG_DISTANCE))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TOOL_AXIS_FLAG_DISTANCE);
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TOOL_AXIS_FLAG_TILT))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TOOL_AXIS_FLAG_TILT);
+   break;
+   default:
+   break;
+   }
+
list_insert(tool_list, &tool->link);
}
 
@@ -512,6 +546,14 @@ tablet_init(struct tablet_dispatch *tablet,
tablet->current_tool_type = LIBINPUT_TOOL_NONE;
list_init(&tablet->tool_list);
 
+   if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_PRESSURE))
+   set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_PRESSURE);
+   if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_DISTANCE))
+   set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_DISTANCE);
+   if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_TILT_X) &&
+   libevdev_has_event_code(device->evdev, EV_ABS, ABS_TILT_Y))
+   set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_TILT);
+
tablet_mark_all_axes_changed(tablet, device);
 
return 0;
diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
index d16aef3..be2d09a 100644
--- a/src/evdev-tablet.h
+++ b/src/evdev-tablet.h
@@ -49,6 +49,7 @@ struct tablet_dispatch {
unsigned char status;
unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
double axes[LIBINPUT_TABLET_AXIS_CNT];
+   unsigned char axis_caps[NCHARS(LIBINPUT_TOOL_AXIS_FLAG_CNT)];
 
/* Only used for tablets that don't report serial numbers */
struct list tool_list;
diff --git a/src/libinput-private.h b/src/libinput-private.h
index 8187564..bdef330 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -108,6 +108,7 @@ struct libinput_tool {
struct list link;
uint32_t serial;
enum libinput_tool_type type;
+   unsigned char axis_caps[NCHARS(LIBINPUT_TOOL_AXIS_FLAG_CNT)];
int refcount;
void *user_data;
 };
diff --git a/src/libinput.c b/src/libinput.c
index 68187d8..560fedd 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -599,6 +599,13 @@ libinput_tool_get_serial(struct libinput_tool *tool)
return tool->serial;
 }
 
+LIBINPUT_EXPORT int
+libinput_tool_has_axis(struct libinput_tool *tool,
+  enum libinput_tool_axis_flag axis)
+{
+   return bit_is_set(tool->axis_caps, axis);
+}
+
 LIBINPUT_EXPORT void
 libinput_tool_set_user_data(struct libinput_tool *tool,
void 

[PATCH libinput 1/2 v2] tablet: Use separate tool objects for tools without serials

2014-08-07 Thread Stephen Chandler Paul
With tablets that don't support serial numbers, we can't guarantee that the tool
objects are unique. Because of this, this can give clients the false impression
that a tool without a serial number is being shared between tablets when it very
well might not be. So we keep tools without serial numbers in a list that's
local to the tablet they belong to, and keep tools with serials in a list that's
local to the libinput context.

Signed-off-by: Stephen Chandler Paul 
---

Changes:
- Added a description to the patch
- Fixed all of the memory leaks, passes make check with flying colors
  now

 src/evdev-tablet.c | 37 ++-
 src/evdev-tablet.h |  4 +++
 test/tablet.c  | 87 ++
 3 files changed, 120 insertions(+), 8 deletions(-)

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 31dd8d7..43da67a 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -259,17 +259,32 @@ tablet_process_misc(struct tablet_dispatch *tablet,
 }
 
 static struct libinput_tool *
-tablet_get_tool(struct libinput *li,
+tablet_get_tool(struct tablet_dispatch *tablet,
enum libinput_tool_type type,
uint32_t serial)
 {
struct libinput_tool *tool = NULL, *t;
+   struct list *tool_list;
 
-   /* Check if we already have the tool in our list of tools */
-   list_for_each(t, &li->tool_list, link) {
-   if (type == t->type && serial == t->serial) {
-   tool = t;
-   break;
+   if (serial) {
+   tool_list = &tablet->device->base.seat->libinput->tool_list;
+
+   /* Check if we already have the tool in our list of tools */
+   list_for_each(t, tool_list, link) {
+   if (type == t->type && serial == t->serial) {
+   tool = t;
+   break;
+   }
+   }
+   } else {
+   tool_list = &tablet->tool_list;
+
+   /* Same as above, but don't bother checking the serial number */
+   list_for_each(t, tool_list, link) {
+   if (type == t->type) {
+   tool = t;
+   break;
+   }
}
}
 
@@ -283,7 +298,7 @@ tablet_get_tool(struct libinput *li,
.refcount = 1,
};
 
-   list_insert(&li->tool_list, &tool->link);
+   list_insert(tool_list, &tool->link);
}
 
return tool;
@@ -382,7 +397,7 @@ tablet_flush(struct tablet_dispatch *tablet,
 uint32_t time)
 {
struct libinput_tool *tool =
-   tablet_get_tool(device->base.seat->libinput,
+   tablet_get_tool(tablet,
tablet->current_tool_type,
tablet->current_tool_serial);
 
@@ -473,6 +488,11 @@ tablet_destroy(struct evdev_dispatch *dispatch)
 {
struct tablet_dispatch *tablet =
(struct tablet_dispatch*)dispatch;
+   struct libinput_tool *tool, *tmp;
+
+   list_for_each_safe(tool, tmp, &tablet->tool_list, link) {
+   libinput_tool_unref(tool);
+   }
 
free(tablet);
 }
@@ -490,6 +510,7 @@ tablet_init(struct tablet_dispatch *tablet,
tablet->device = device;
tablet->status = TABLET_NONE;
tablet->current_tool_type = LIBINPUT_TOOL_NONE;
+   list_init(&tablet->tool_list);
 
tablet_mark_all_axes_changed(tablet, device);
 
diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
index 1b53d20..d16aef3 100644
--- a/src/evdev-tablet.h
+++ b/src/evdev-tablet.h
@@ -26,6 +26,7 @@
 #define EVDEV_TABLET_H
 
 #include "evdev.h"
+#include 
 
 enum tablet_status {
TABLET_NONE = 0,
@@ -49,6 +50,9 @@ struct tablet_dispatch {
unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
double axes[LIBINPUT_TABLET_AXIS_CNT];
 
+   /* Only used for tablets that don't report serial numbers */
+   struct list tool_list;
+
struct button_state button_state;
struct button_state prev_button_state;
 
diff --git a/test/tablet.c b/test/tablet.c
index 0cd6357..2de0989 100644
--- a/test/tablet.c
+++ b/test/tablet.c
@@ -681,6 +681,91 @@ START_TEST(pad_buttons_ignored)
 }
 END_TEST
 
+START_TEST(tools_with_serials)
+{
+   struct libinput *li = litest_create_context();
+   struct litest_device *dev[2];
+   struct libinput_tool *tool[2];
+   struct libinput_event *event;
+   int i;
+
+   for (i = 0; i < 2; i++) {
+   dev[i] = litest_add_device_with_overrides(li,
+ LITEST_WACOM_INTUOS,
+ NULL,
+ NULL,
+   

[PATCH libinput v3] tablet: Use separate tool objects for tools without serials

2014-08-07 Thread Stephen Chandler Paul
With tablets that don't support serial numbers, we can't guarantee that the tool
objects are unique. Because of this, this can give clients the false impression
that a tool without a serial number is being shared between tablets when it very
well might not be. So we keep tools without serial numbers in a list that's
local to the tablet they belong to, and keep tools with serials in a list that's
local to the libinput context.

Signed-off-by: Stephen Chandler Paul 
---

Changes (forgot to make these with the last patch, sorry!):
- Add comment to explain why we use a separate tool list in the code
- Remove leftover include

 src/evdev-tablet.c | 41 -
 src/evdev-tablet.h |  3 ++
 test/tablet.c  | 87 ++
 3 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 31dd8d7..15f0663 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -259,17 +259,36 @@ tablet_process_misc(struct tablet_dispatch *tablet,
 }
 
 static struct libinput_tool *
-tablet_get_tool(struct libinput *li,
+tablet_get_tool(struct tablet_dispatch *tablet,
enum libinput_tool_type type,
uint32_t serial)
 {
struct libinput_tool *tool = NULL, *t;
+   struct list *tool_list;
 
-   /* Check if we already have the tool in our list of tools */
-   list_for_each(t, &li->tool_list, link) {
-   if (type == t->type && serial == t->serial) {
-   tool = t;
-   break;
+   if (serial) {
+   tool_list = &tablet->device->base.seat->libinput->tool_list;
+
+   /* Check if we already have the tool in our list of tools */
+   list_for_each(t, tool_list, link) {
+   if (type == t->type && serial == t->serial) {
+   tool = t;
+   break;
+   }
+   }
+   } else {
+   /* We can't guarantee that tools without serial numbers are
+* unique, so we keep them local to the tablet that they come
+* into proximity of instead of storing them in the global tool
+* list */
+   tool_list = &tablet->tool_list;
+
+   /* Same as above, but don't bother checking the serial number */
+   list_for_each(t, tool_list, link) {
+   if (type == t->type) {
+   tool = t;
+   break;
+   }
}
}
 
@@ -283,7 +302,7 @@ tablet_get_tool(struct libinput *li,
.refcount = 1,
};
 
-   list_insert(&li->tool_list, &tool->link);
+   list_insert(tool_list, &tool->link);
}
 
return tool;
@@ -382,7 +401,7 @@ tablet_flush(struct tablet_dispatch *tablet,
 uint32_t time)
 {
struct libinput_tool *tool =
-   tablet_get_tool(device->base.seat->libinput,
+   tablet_get_tool(tablet,
tablet->current_tool_type,
tablet->current_tool_serial);
 
@@ -473,6 +492,11 @@ tablet_destroy(struct evdev_dispatch *dispatch)
 {
struct tablet_dispatch *tablet =
(struct tablet_dispatch*)dispatch;
+   struct libinput_tool *tool, *tmp;
+
+   list_for_each_safe(tool, tmp, &tablet->tool_list, link) {
+   libinput_tool_unref(tool);
+   }
 
free(tablet);
 }
@@ -490,6 +514,7 @@ tablet_init(struct tablet_dispatch *tablet,
tablet->device = device;
tablet->status = TABLET_NONE;
tablet->current_tool_type = LIBINPUT_TOOL_NONE;
+   list_init(&tablet->tool_list);
 
tablet_mark_all_axes_changed(tablet, device);
 
diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
index 1b53d20..adc3aa4 100644
--- a/src/evdev-tablet.h
+++ b/src/evdev-tablet.h
@@ -49,6 +49,9 @@ struct tablet_dispatch {
unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
double axes[LIBINPUT_TABLET_AXIS_CNT];
 
+   /* Only used for tablets that don't report serial numbers */
+   struct list tool_list;
+
struct button_state button_state;
struct button_state prev_button_state;
 
diff --git a/test/tablet.c b/test/tablet.c
index 0cd6357..2de0989 100644
--- a/test/tablet.c
+++ b/test/tablet.c
@@ -681,6 +681,91 @@ START_TEST(pad_buttons_ignored)
 }
 END_TEST
 
+START_TEST(tools_with_serials)
+{
+   struct libinput *li = litest_create_context();
+   struct litest_device *dev[2];
+   struct libinput_tool *tool[2];
+   struct libinput_event *event;
+   int i;
+
+   for (i = 0; i < 2; i++) {
+   dev[i] = litest_add_device_with_overrides(li,
+ LITEST_WACOM_INTUOS,
+  

Re: [PATCH libinput v3] tablet: Use separate tool objects for tools without serials

2014-08-07 Thread Peter Hutterer
On Thu, Aug 07, 2014 at 07:00:52PM -0400, Stephen Chandler Paul wrote:
> With tablets that don't support serial numbers, we can't guarantee that the 
> tool
> objects are unique. Because of this, this can give clients the false 
> impression
> that a tool without a serial number is being shared between tablets when it 
> very
> well might not be. So we keep tools without serial numbers in a list that's
> local to the tablet they belong to, and keep tools with serials in a list 
> that's
> local to the libinput context.

the libinput context isn't really local, so I just replaced this to "global
within the libinput context" which I think is a bit easier to grasp.

> 
> Signed-off-by: Stephen Chandler Paul 
> ---
> 
> Changes (forgot to make these with the last patch, sorry!):
>   - Add comment to explain why we use a separate tool list in the code
>   - Remove leftover include
> 
>  src/evdev-tablet.c | 41 -
>  src/evdev-tablet.h |  3 ++
>  test/tablet.c  | 87 
> ++
>  3 files changed, 123 insertions(+), 8 deletions(-)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 31dd8d7..15f0663 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -259,17 +259,36 @@ tablet_process_misc(struct tablet_dispatch *tablet,
>  }
>  
>  static struct libinput_tool *
> -tablet_get_tool(struct libinput *li,
> +tablet_get_tool(struct tablet_dispatch *tablet,
>   enum libinput_tool_type type,
>   uint32_t serial)
>  {
>   struct libinput_tool *tool = NULL, *t;
> + struct list *tool_list;
>  
> - /* Check if we already have the tool in our list of tools */
> - list_for_each(t, &li->tool_list, link) {
> - if (type == t->type && serial == t->serial) {
> - tool = t;
> - break;
> + if (serial) {
> + tool_list = &tablet->device->base.seat->libinput->tool_list;
> +
> + /* Check if we already have the tool in our list of tools */
> + list_for_each(t, tool_list, link) {
> + if (type == t->type && serial == t->serial) {
> + tool = t;
> + break;
> + }
> + }
> + } else {
> + /* We can't guarantee that tools without serial numbers are
> +  * unique, so we keep them local to the tablet that they come
> +  * into proximity of instead of storing them in the global tool
> +  * list */
> + tool_list = &tablet->tool_list;
> +
> + /* Same as above, but don't bother checking the serial number */
> + list_for_each(t, tool_list, link) {
> + if (type == t->type) {
> + tool = t;
> + break;
> + }
>   }
>   }
>  
> @@ -283,7 +302,7 @@ tablet_get_tool(struct libinput *li,
>   .refcount = 1,
>   };
>  
> - list_insert(&li->tool_list, &tool->link);
> + list_insert(tool_list, &tool->link);
>   }
>  
>   return tool;
> @@ -382,7 +401,7 @@ tablet_flush(struct tablet_dispatch *tablet,
>uint32_t time)
>  {
>   struct libinput_tool *tool =
> - tablet_get_tool(device->base.seat->libinput,
> + tablet_get_tool(tablet,
>   tablet->current_tool_type,
>   tablet->current_tool_serial);
>  
> @@ -473,6 +492,11 @@ tablet_destroy(struct evdev_dispatch *dispatch)
>  {
>   struct tablet_dispatch *tablet =
>   (struct tablet_dispatch*)dispatch;
> + struct libinput_tool *tool, *tmp;
> +
> + list_for_each_safe(tool, tmp, &tablet->tool_list, link) {
> + libinput_tool_unref(tool);
> + }
>  
>   free(tablet);
>  }
> @@ -490,6 +514,7 @@ tablet_init(struct tablet_dispatch *tablet,
>   tablet->device = device;
>   tablet->status = TABLET_NONE;
>   tablet->current_tool_type = LIBINPUT_TOOL_NONE;
> + list_init(&tablet->tool_list);
>  
>   tablet_mark_all_axes_changed(tablet, device);
>  
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index 1b53d20..adc3aa4 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -49,6 +49,9 @@ struct tablet_dispatch {
>   unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>   double axes[LIBINPUT_TABLET_AXIS_CNT];
>  
> + /* Only used for tablets that don't report serial numbers */
> + struct list tool_list;
> +
>   struct button_state button_state;
>   struct button_state prev_button_state;
>  
> diff --git a/test/tablet.c b/test/tablet.c
> index 0cd6357..2de0989 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -681,6 +681,91 @@ START_TEST(pad_buttons_ignored)
>  }
>  END_TEST
>  
> +START_TEST(tools_with_serials)
> +{
> + 

Re: [PATCH libinput 2/2 v2] tablet: Add libinput_tool_has_axis() and tests

2014-08-07 Thread Peter Hutterer
On Thu, Aug 07, 2014 at 06:54:55PM -0400, Stephen Chandler Paul wrote:
> Because the axes that tool reports can change depending on the tool in use, we
> want to be able to provide functionality to determine which axes each tool can
> support.
> 
> Signed-off-by: Stephen Chandler Paul 
> ---
> 
> Changes:
>   - Added a description to the patch
>   - Fixed all of the memory leaks, passes make check with flying
> colors now
> 
>  src/evdev-tablet.c | 42 +
>  src/evdev-tablet.h |  1 +
>  src/libinput-private.h |  1 +
>  src/libinput.c |  7 +
>  src/libinput.h | 26 ++
>  test/tablet.c  | 72 
> ++
>  6 files changed, 149 insertions(+)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 43da67a..5f8a466 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -298,6 +298,40 @@ tablet_get_tool(struct tablet_dispatch *tablet,
>   .refcount = 1,
>   };
>  
> + /* Determine the axis capabilities of the tool. Here's a break
> +  * down of the heuristics used here:
> +  * - The Wacom art pen supports all of the extra axes, along
> +  *   with rotation
> +  * - All of normal pens and the airbrush support all of the
> +  *   extra axes if the tablet can report them
> +  * - All of the mouse like devices don't really report any of
> +  *   the extra axes except for rotation.
> +  * (as of writing this comment, rotation isn't supported, so you
> +  * won't see the mouse or art pen here)
> +  */
> + switch (type) {
> + case LIBINPUT_TOOL_PEN:
> + case LIBINPUT_TOOL_ERASER:
> + case LIBINPUT_TOOL_PENCIL:
> + case LIBINPUT_TOOL_BRUSH:
> + case LIBINPUT_TOOL_AIRBRUSH:
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TOOL_AXIS_FLAG_PRESSURE))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TOOL_AXIS_FLAG_PRESSURE);
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TOOL_AXIS_FLAG_DISTANCE))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TOOL_AXIS_FLAG_DISTANCE);
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TOOL_AXIS_FLAG_TILT))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TOOL_AXIS_FLAG_TILT);
> + break;
> + default:
> + break;
> + }
> +
>   list_insert(tool_list, &tool->link);
>   }
>  
> @@ -512,6 +546,14 @@ tablet_init(struct tablet_dispatch *tablet,
>   tablet->current_tool_type = LIBINPUT_TOOL_NONE;
>   list_init(&tablet->tool_list);
>  
> + if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_PRESSURE))
> + set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_PRESSURE);
> + if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_DISTANCE))
> + set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_DISTANCE);
> + if (libevdev_has_event_code(device->evdev, EV_ABS, ABS_TILT_X) &&
> + libevdev_has_event_code(device->evdev, EV_ABS, ABS_TILT_Y))
> + set_bit(tablet->axis_caps, LIBINPUT_TOOL_AXIS_FLAG_TILT);
> +
>   tablet_mark_all_axes_changed(tablet, device);
>  
>   return 0;
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index d16aef3..be2d09a 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -49,6 +49,7 @@ struct tablet_dispatch {
>   unsigned char status;
>   unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>   double axes[LIBINPUT_TABLET_AXIS_CNT];
> + unsigned char axis_caps[NCHARS(LIBINPUT_TOOL_AXIS_FLAG_CNT)];
>  
>   /* Only used for tablets that don't report serial numbers */
>   struct list tool_list;
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 8187564..bdef330 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -108,6 +108,7 @@ struct libinput_tool {
>   struct list link;
>   uint32_t serial;
>   enum libinput_tool_type type;
> + unsigned char axis_caps[NCHARS(LIBINPUT_TOOL_AXIS_FLAG_CNT)];
>   int refcount;
>   void *user_data;
>  };
> diff --git a/src/libinput.c b/src/libinput.c
> index 68187d8..560fedd 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -599,6 +599,13 @@ libinput_tool_get_serial(struct libinput_tool *tool)
>   return tool->serial;
>  }
>  
> +LIBINPUT_EXPORT int
> +libinput_tool_has_axis(struct libinput_tool *tool,
> +enum libinput_tool_axis_flag ax

[PATCH libinput v3] tablet: Add libinput_tool_has_axis() and tests

2014-08-07 Thread Stephen Chandler Paul
Because the axes that tool reports can change depending on the tool in use, we
want to be able to provide functionality to determine which axes each tool can
support.

Signed-off-by: Stephen Chandler Paul 
---

= Changes =
* Fixed the line width in test/tablet.c
* Removed libinput_tool_axis_flag and used the already existing axis enumerators
  instead

 src/evdev-tablet.c | 47 +++
 src/evdev-tablet.h |  1 +
 src/libinput-private.h |  1 +
 src/libinput.c |  7 +
 src/libinput.h | 13 +
 test/tablet.c  | 76 ++
 6 files changed, 145 insertions(+)

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 15f0663..a63b734 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -302,6 +302,44 @@ tablet_get_tool(struct tablet_dispatch *tablet,
.refcount = 1,
};
 
+   /* Determine the axis capabilities of the tool. Here's a break
+* down of the heuristics used here:
+* - The Wacom art pen supports all of the extra axes, along
+*   with rotation
+* - All of normal pens and the airbrush support all of the
+*   extra axes if the tablet can report them
+* - All of the mouse like devices don't really report any of
+*   the extra axes except for rotation.
+* (as of writing this comment, rotation isn't supported, so you
+* won't see the mouse or art pen here)
+*/
+   switch (type) {
+   case LIBINPUT_TOOL_PEN:
+   case LIBINPUT_TOOL_ERASER:
+   case LIBINPUT_TOOL_PENCIL:
+   case LIBINPUT_TOOL_BRUSH:
+   case LIBINPUT_TOOL_AIRBRUSH:
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TABLET_AXIS_PRESSURE))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TABLET_AXIS_PRESSURE);
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TABLET_AXIS_DISTANCE))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TABLET_AXIS_DISTANCE);
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TABLET_AXIS_TILT_X))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TABLET_AXIS_TILT_X);
+   if (bit_is_set(tablet->axis_caps,
+  LIBINPUT_TABLET_AXIS_TILT_Y))
+   set_bit(tool->axis_caps,
+   LIBINPUT_TABLET_AXIS_TILT_Y);
+   break;
+   default:
+   break;
+   }
+
list_insert(tool_list, &tool->link);
}
 
@@ -510,12 +548,21 @@ static int
 tablet_init(struct tablet_dispatch *tablet,
struct evdev_device *device)
 {
+   enum libinput_tablet_axis axis;
+
tablet->base.interface = &tablet_interface;
tablet->device = device;
tablet->status = TABLET_NONE;
tablet->current_tool_type = LIBINPUT_TOOL_NONE;
list_init(&tablet->tool_list);
 
+   for (axis = 0; axis < LIBINPUT_TABLET_AXIS_CNT; axis++) {
+   if (libevdev_has_event_code(device->evdev,
+   EV_ABS,
+   axis_to_evcode(axis)))
+   set_bit(tablet->axis_caps, axis);
+   }
+
tablet_mark_all_axes_changed(tablet, device);
 
return 0;
diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
index adc3aa4..cb37577 100644
--- a/src/evdev-tablet.h
+++ b/src/evdev-tablet.h
@@ -48,6 +48,7 @@ struct tablet_dispatch {
unsigned char status;
unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
double axes[LIBINPUT_TABLET_AXIS_CNT];
+   unsigned char axis_caps[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
 
/* Only used for tablets that don't report serial numbers */
struct list tool_list;
diff --git a/src/libinput-private.h b/src/libinput-private.h
index 8187564..369bb8f 100644
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -108,6 +108,7 @@ struct libinput_tool {
struct list link;
uint32_t serial;
enum libinput_tool_type type;
+   unsigned char axis_caps[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
int refcount;
void *user_data;
 };
diff --git a/src/libinput.c b/src/libinput.c
index 68187d8..e52527b 100644
--- a/src/libinput.c
+++ b/src/libinput.c
@@ -599,6 +599,13 @@ libinput_tool_get_serial(struct libinput_tool *tool)
return tool->serial;
 }
 
+LIBINPUT_EXPORT int
+libinput_tool_has_axis(struct libin

Re: [PATCH libinput v3] tablet: Add libinput_tool_has_axis() and tests

2014-08-07 Thread Peter Hutterer
On Thu, Aug 07, 2014 at 10:02:22PM -0400, Stephen Chandler Paul wrote:
> Because the axes that tool reports can change depending on the tool in use, we
> want to be able to provide functionality to determine which axes each tool can
> support.
> 
> Signed-off-by: Stephen Chandler Paul 
> ---

merged and pushed, thanks

Cheers,
   Peter


> 
> = Changes =
> * Fixed the line width in test/tablet.c
> * Removed libinput_tool_axis_flag and used the already existing axis 
> enumerators
>   instead
> 
>  src/evdev-tablet.c | 47 +++
>  src/evdev-tablet.h |  1 +
>  src/libinput-private.h |  1 +
>  src/libinput.c |  7 +
>  src/libinput.h | 13 +
>  test/tablet.c  | 76 
> ++
>  6 files changed, 145 insertions(+)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 15f0663..a63b734 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -302,6 +302,44 @@ tablet_get_tool(struct tablet_dispatch *tablet,
>   .refcount = 1,
>   };
>  
> + /* Determine the axis capabilities of the tool. Here's a break
> +  * down of the heuristics used here:
> +  * - The Wacom art pen supports all of the extra axes, along
> +  *   with rotation
> +  * - All of normal pens and the airbrush support all of the
> +  *   extra axes if the tablet can report them
> +  * - All of the mouse like devices don't really report any of
> +  *   the extra axes except for rotation.
> +  * (as of writing this comment, rotation isn't supported, so you
> +  * won't see the mouse or art pen here)
> +  */
> + switch (type) {
> + case LIBINPUT_TOOL_PEN:
> + case LIBINPUT_TOOL_ERASER:
> + case LIBINPUT_TOOL_PENCIL:
> + case LIBINPUT_TOOL_BRUSH:
> + case LIBINPUT_TOOL_AIRBRUSH:
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TABLET_AXIS_PRESSURE))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TABLET_AXIS_PRESSURE);
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TABLET_AXIS_DISTANCE))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TABLET_AXIS_DISTANCE);
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TABLET_AXIS_TILT_X))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TABLET_AXIS_TILT_X);
> + if (bit_is_set(tablet->axis_caps,
> +LIBINPUT_TABLET_AXIS_TILT_Y))
> + set_bit(tool->axis_caps,
> + LIBINPUT_TABLET_AXIS_TILT_Y);
> + break;
> + default:
> + break;
> + }
> +
>   list_insert(tool_list, &tool->link);
>   }
>  
> @@ -510,12 +548,21 @@ static int
>  tablet_init(struct tablet_dispatch *tablet,
>   struct evdev_device *device)
>  {
> + enum libinput_tablet_axis axis;
> +
>   tablet->base.interface = &tablet_interface;
>   tablet->device = device;
>   tablet->status = TABLET_NONE;
>   tablet->current_tool_type = LIBINPUT_TOOL_NONE;
>   list_init(&tablet->tool_list);
>  
> + for (axis = 0; axis < LIBINPUT_TABLET_AXIS_CNT; axis++) {
> + if (libevdev_has_event_code(device->evdev,
> + EV_ABS,
> + axis_to_evcode(axis)))
> + set_bit(tablet->axis_caps, axis);
> + }
> +
>   tablet_mark_all_axes_changed(tablet, device);
>  
>   return 0;
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index adc3aa4..cb37577 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -48,6 +48,7 @@ struct tablet_dispatch {
>   unsigned char status;
>   unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>   double axes[LIBINPUT_TABLET_AXIS_CNT];
> + unsigned char axis_caps[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>  
>   /* Only used for tablets that don't report serial numbers */
>   struct list tool_list;
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 8187564..369bb8f 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -108,6 +108,7 @@ struct libinput_tool {
>   struct list link;
>   uint32_t serial;
>   enum libinput_tool_type type;
> + unsigned char axis_caps[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>   int refcount;
>   void *user_data;
>  };
> diff --git a/src/libinput.c b/src/libinput.c
> index 68187d8..

[PATCH libinput] touchpad: increase top software button area to 15%

2014-08-07 Thread Peter Hutterer
We had reports that the top software button area is hard to hit for those
using the trackpoint and clicking the buttons with their thumb.

Analysis of event recordings (3 different people) for left, right and middle
clicks shows that there is a significant amount of events up to about 10mm
(with outliers up to 12mm) from the top of the touchpad. That maps to 15%.

Interestingly, the middle button does not seem to need this, presumably
the haptic feedback of the little dots sticking out from the surface
make hitting the button easier. Its size is increased to 15% anyway, for
simplicity and because a sample set of 3 is too small to be definitive about
this.

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad-buttons.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index fe33d0b..9262466 100644
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -542,12 +542,13 @@ tp_init_buttons(struct tp_dispatch *tp,
 
if (tp->buttons.has_topbuttons) {
/* T440s has the top button line 5mm from the top,
-  make the buttons 6mm high */
+  event analysis has shown events to start down to 
~10mm
+  from the top - which maps to 15% */
if (yres > 1) {
tp->buttons.top_area.bottom_edge =
-   yoffset + 6 * yres;
+   yoffset + 10 * yres;
} else {
-   tp->buttons.top_area.bottom_edge = height * .08 
+ yoffset;
+   tp->buttons.top_area.bottom_edge = height * .15 
+ yoffset;
}
tp->buttons.top_area.rightbutton_left_edge = width * 
.58 + xoffset;
tp->buttons.top_area.leftbutton_right_edge = width * 
.42 + xoffset;
-- 
1.9.3

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


Re: [PATCH wayland] server: fix conditions for fds in wl_socket_destroy

2014-08-07 Thread Pekka Paalanen
On Thu, 7 Aug 2014 09:56:59 -0400
"Jasper St. Pierre"  wrote:

> Reviewed-by: Jasper St. Pierre 

Thanks, pushed.
- pq


> On Thu, Aug 7, 2014 at 9:51 AM, Pekka Paalanen  wrote:
> 
> > From: Pekka Paalanen 
> >
> > 0 is also a valid fd, and needs to be closed.
> >
> > On error we set fd to -1. We need to also initialize fds to -1, so we do
> > not accidentally close stdout on error.
> >
> > While fixing this, also remove one use-before-NULL-check.
> >
> > Based on the patch by Marek.
> >
> > Cc: Marek Chalupa 
> > Signed-off-by: Pekka Paalanen 
> > ---
> >  src/wayland-server.c | 27 ---
> >  1 file changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > index 3c162d4..3bd8e68 100644
> > --- a/src/wayland-server.c
> > +++ b/src/wayland-server.c
> > @@ -848,16 +848,32 @@ wl_socket_destroy(struct wl_socket *s)
> > wl_event_source_remove(s->source);
> > if (s->addr.sun_path[0])
> > unlink(s->addr.sun_path);
> > -   if (s->fd)
> > +   if (s->fd >= 0)
> > close(s->fd);
> > if (s->lock_addr[0])
> > unlink(s->lock_addr);
> > -   if (s->fd_lock)
> > +   if (s->fd_lock >= 0)
> > close(s->fd_lock);
> >
> > free(s);
> >  }
> >
> > +static struct wl_socket *
> > +wl_socket_alloc(void)
> > +{
> > +   struct wl_socket *s;
> > +
> > +   s = malloc(sizeof *s);
> > +   if (!s)
> > +   return NULL;
> > +
> > +   memset(s, 0, sizeof *s);
> > +   s->fd = -1;
> > +   s->fd_lock = -1;
> > +
> > +   return s;
> > +}
> > +
> >  WL_EXPORT void
> >  wl_display_destroy(struct wl_display *display)
> >  {
> > @@ -1149,12 +1165,10 @@ wl_display_add_socket_auto(struct wl_display
> > *display)
> >  * you need more than this, use the explicit add_socket API. */
> > const int MAX_DISPLAYNO = 32;
> >
> > -   s = malloc(sizeof *s);
> > +   s = wl_socket_alloc();
> > if (s == NULL)
> > return NULL;
> >
> > -   memset(s, 0, sizeof *s);
> > -
> > do {
> > snprintf(display_name, sizeof display_name, "wayland-%d",
> > displayno);
> > if (wl_socket_init_for_display_name(s, display_name) < 0) {
> > @@ -1184,8 +1198,7 @@ wl_display_add_socket(struct wl_display *display,
> > const char *name)
> >  {
> > struct wl_socket *s;
> >
> > -   s = malloc(sizeof *s);
> > -   memset(s, 0, sizeof *s);
> > +   s = wl_socket_alloc();
> > if (s == NULL)
> > return -1;
> >
> > --
> > 1.8.5.5
> >
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> 
> 
> 

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


Re: [PATCH] Introduce set_size_hints xdg_surface request.

2014-08-07 Thread Pekka Paalanen
On Thu, 7 Aug 2014 17:18:54 +0300
Jari Vetoniemi  wrote:

> > What is the default value of these hints?
> 
> I would propose -1 as default, meaning unset.
> Compositor has to deal with not knowing the minimum size, preferably
> this would mean though that the client can be set to any valid size.
> 
> > When is the client expected to send this? Can this be sent before
> > the window gets mapped? After the window was unmapped?
> 
> I would say this hint could be sent any time after surface object is created.
> 
> > How does this interact with the protocol sequence to decide the initial
> > size of a window before it gets mapped?
> 
> If the maximum was less than initial size, this would need thinking.
> For now it seems though that maximum might go away?

My main question here was, how does the protocol sequence look like?

A client creates a wl_surface and a xdg_surface for it. What does the
following protocol sequence look like, when negotiating the initial
window size, leading to mapping the window the first time?

Does the server send a configure event immediately as a reply to
creating the xdg_surface? If yes, does the compositor send a new
configure event after the client has sent the size hint request?

If the client sends the request to create xdg_surface immediately
followed by the request to set size hints, how does the client know if
the next configure event it receives is before or after the
compositor saw the size hint? Does it even matter?

Or do you require a round-trip: the client must wait for the first
configure event before sending the size hints, so that it knows the
next configure event is with size hints applied?

(I would like to avoid round-trips whenever sanely possible.)

> Needs further discussion.
> 
> > Making surface state double-buffered is desired for all state that
> > immediately affects the surface's presentation. However, these are
> > hints and do not change how the surface is presented; they change what
> > sizes the compositor may ask for in configure events. It that sense, it
> > is not necessary to make this state double-buffered, and I think not
> > double-buffering (not demanding a commit) will make it easier to use
> > and also to define how it works with the initial size negotiation.
> 
> I agree with this, I don't see any reason for this to be
> double-buffered either to be honest.

Otherwise seems ok AFAIU.


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