Re: [RFC wayland 13/18] connections: Add remote sockets

[Pekka Paalanen]

On Wed, 10 Feb 2016 11:49:05 -0600
Derek Foreman  wrote:

> Please forgive my mailer for what it's done to the content below. :/
> 
> On 10/02/16 07:33 AM, Pekka Paalanen wrote:
> > On Tue,  9 Feb 2016 10:56:00 -0600 Derek Foreman
> >  wrote:
> >   
> >> This allows adding a tcp/ip socket and adds tracking of the
> >> remote status for clients and proxies

> >> +void +wl_connection_set_remote(struct wl_connection
> >> *connection) +{ +  connection->remote = true; +} + int 
> >> wl_connection_destroy(struct wl_connection *connection) { @@
> >> -278,6 +286,7 @@ wl_connection_flush(struct wl_connection
> >> *connection) char cmsg[CLEN]; int len = 0, count, clen; uint32_t
> >> tail; +int wait = connection->remote ? 0 : MSG_DONTWAIT;
> >> 
> >> if (!connection->want_flush) return 0; @@ -298,7 +307,7 @@
> >> wl_connection_flush(struct wl_connection *connection)
> >> 
> >> do { len = sendmsg(connection->fd, , -   
> >> MSG_NOSIGNAL
> >> | MSG_DONTWAIT); +   MSG_NOSIGNAL | wait);  
> > 
> > Hi,
> > 
> > why do you want to block for remote?
> > 
> > Could this be because something isn't polling for writable
> > properly? Or don't we support that somehow?  
> 
> This was failing with EAGAIN so I just dropped the dontwait without
> thinking about it too much, but now that you're forcing me to...
> 
> We can technically fill up the send buffer and have this fail with
> EAGAIN even on local connections, but it takes an unreasonably massive
> amount of spew to do it - I made weston repeat mouse motion events
> something like 1 times as a test. :)
> 
> I don't think epoll telling us it's writable actually promises there's
> enough room for a potentially 4k wayland packet.
> 
> With a remote network connection things backlog much more quickly...

Hi,

hrm, does this mean that we try to send the whole send-buffer's worth
of data in one go, and if it doesn't fit, we give up completely on this
event loop cycle rather than trying to send just some of the messages?

If that's true, a slow client would cause the server to busy-spin with
the socket polling readable until there is room for the whole mass, and
that doesn't sound too good.

I fear fixing this would be quite hard. Ok, I see blocking is the easy
way out.

> Oh, and as you mentioned to me on irc, pushing "bulk data" through the
> pipe with write(2) directly certainly doesn't make this any less
> likely to happen.  (And neither does sending piles of 4k packets worth
> of buffer contents, I guess)
> 
> I think it's been safe so far to assume that a full send buffer means
> the app on the other end isn't responding at all, but with a tcp/ip
> connection it isn't?

That seems like a hasty assumption to me. OTOH, libwayland-client tries
to read as much as possible, doesn't it? I mean, it will usually read
everything there is coming, which on a local connection means that
there will be room for the "whole mass" the next time. And tcp/ip
doesn't work like that because the server's send queue may drain little
by little?

I suppose this is one of the reasons why libwayland as is, being
optimized for local transport, is not a good fit for the network.


Thanks,
pq


pgpGIx9ZdzApf.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] connection: Don't add uninitialized memory as 4 byte alignment padding

[Pekka Paalanen]

On Thu, 11 Feb 2016 09:47:29 +0800
Jonas Ådahl  wrote:

> On Wed, Feb 10, 2016 at 12:37:43PM -0600, Derek Foreman wrote:
> > On 10/02/16 09:35 AM, Jonas Ådahl wrote:  
> > > When we are adding padding bytes making our wl_buffer buffer content 4
> > > byte aligned, we are just moving the pointer. Since the buffer is
> > > allocated using plain malloc(), this means our padding bytes are
> > > effectively uninitialized data, which could be anything previously
> > > allocated in the server process. As we'll be sharing this buffer
> > > content with arbitrary clients, we are effectively sharing private
> > > memory with every client, and even though a well behaving client will
> > > discard any such memory, a malicious client may not.
> > > 
> > > Therefor, to avoid any potential missuse of the uninitialized padding
> > > memory shared between the server and client, initialize the buffer
> > > content to 0, making the padding bytes always 0.  
> > 
> > Cool - is this the cause of the valgrind complaints I've never bothered
> > to track down? :)  
> 
> Yes. Should have mentioned that in the commit message, as well as the
> bug (https://bugs.freedesktop.org/show_bug.cgi?id=94071 ) that made me
> actually dig out what memory was uninitialized. I'll add those two
> things to the commit message.
> 
> >   
> > > Signed-off-by: Jonas Ådahl 
> > > ---
> > >  src/connection.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/src/connection.c b/src/connection.c
> > > index 65b64e9..c0e322f 100644
> > > --- a/src/connection.c
> > > +++ b/src/connection.c
> > > @@ -1137,7 +1137,7 @@ wl_closure_send(struct wl_closure *closure, struct 
> > > wl_connection *connection)
> > >   return -1;
> > >  
> > >   buffer_size = buffer_size_for_closure(closure);
> > > - buffer = malloc(buffer_size * sizeof buffer[0]);
> > > + buffer = zalloc(buffer_size * sizeof buffer[0]);  
> > 
> > Oh, took me a couple of reads to realize the padding is between certain
> > potential members in the closure. (and not trivially at the start or end
> > of the buffer)

Hi,

just to see I understood right too, you mean the string and array types
in serialize_closure() which use DIV_ROUNDUP() and nothing else?

If that is the case, then have my
Reviewed-by: Pekka Paalanen 

and with that I also nominate this patch to be landed for 1.10: it is
trivial and obviously correct, and if it in the unlikely event causes
any problems, those problems have existed before. It might even be
considered as a security fix by some far stretch.

> > I guess we could zero just the pad bytes in serialize_closure, I don't
> > know whether that falls under "premature optimization" or not, though.
> > Looks pretty easy to do...  
> 
> We could do that, but is calloc() really any noticably slower than
> malloc() in the way we are using it?

I wouldn't think there is any observable performance penalty here. And
I like the simplicity of the patch and the catch-all nature of the fix.


Thanks,
pq

> It is fairly easy (I did it at first just to make sure it was those
> bytes only that triggered the warning), but s/malloc/zalloc/ seemed like
> the cleaner solution to me.
> 
> 
> Jonas
> 
> > 
> > In any case,
> > Reviewed-by: Derek Foreman 
> > 
> > Thanks,
> > Derek
> >   
> > >   if (buffer == NULL)
> > >   return -1;
> > >  
> > >   


pgpnMOJKjPqKr.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC wayland 01/18] shm: add getters for the fd and offset

[Pekka Paalanen]

On Wed, 10 Feb 2016 12:00:48 -0600
Derek Foreman  wrote:

> On 10/02/16 07:08 AM, Pekka Paalanen wrote:
> > On Tue,  9 Feb 2016 10:55:48 -0600
> > Derek Foreman  wrote:
> >   
> >> From: Giulio Camuffo 
> >>
> >> This allows to share the buffer data by mmapping the fd again.
> >> Reviewed-by: David FORT 
> >>
> >> Signed-off-by: Derek Foreman 
> >> ---
> >>  src/wayland-server-core.h |  6 ++
> >>  src/wayland-shm.c | 16 +++-
> >>  2 files changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> >> index e8e1e9c..3316022 100644
> >> --- a/src/wayland-server-core.h
> >> +++ b/src/wayland-server-core.h
> >> @@ -480,6 +480,12 @@ wl_shm_buffer_create(struct wl_client *client,
> >> uint32_t id, int32_t width, int32_t height,
> >> int32_t stride, uint32_t format) WL_DEPRECATED;
> >>  
> >> +int
> >> +wl_shm_buffer_get_offset(struct wl_shm_buffer *buffer);
> >> +
> >> +int
> >> +wl_shm_buffer_get_fd(struct wl_shm_buffer *buffer);
> >> +
> >>  void wl_log_set_handler_server(wl_log_func_t handler);
> >>  
> >>  #ifdef  __cplusplus
> >> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> >> index a4343a4..911165d 100644
> >> --- a/src/wayland-shm.c
> >> +++ b/src/wayland-shm.c
> >> @@ -55,6 +55,7 @@ struct wl_shm_pool {
> >>int refcount;
> >>char *data;
> >>int32_t size;
> >> +  int fd;
> >>  };
> >>  
> >>  struct wl_shm_buffer {
> >> @@ -80,6 +81,7 @@ shm_pool_unref(struct wl_shm_pool *pool)
> >>return;
> >>  
> >>munmap(pool->data, pool->size);
> >> +  close(pool->fd);
> >>free(pool);
> >>  }
> >>  
> >> @@ -253,7 +255,7 @@ shm_create_pool(struct wl_client *client, struct 
> >> wl_resource *resource,
> >>   "failed mmap fd %d", fd);
> >>goto err_close;
> >>}
> >> -  close(fd);
> >> +  pool->fd = fd;
> >>  
> >>pool->resource =
> >>wl_resource_create(client, _shm_pool_interface, 1, id);
> >> @@ -421,6 +423,18 @@ wl_shm_pool_unref(struct wl_shm_pool *pool)
> >>shm_pool_unref(pool);
> >>  }
> >>  
> >> +WL_EXPORT int
> >> +wl_shm_buffer_get_offset(struct wl_shm_buffer *buffer)
> >> +{
> >> +  return buffer->offset;
> >> +}
> >> +
> >> +WL_EXPORT int
> >> +wl_shm_buffer_get_fd(struct wl_shm_buffer *buffer)
> >> +{
> >> +  return buffer->pool->fd;
> >> +}
> >> +
> >>  static void
> >>  reraise_sigbus(void)
> >>  {  
> > 
> > Hi,
> > 
> > Derek did wonder about this the last time with this patch, and I want
> > to reiterate, that leaving the pool fd open causes the server process
> > to have much much more open fds.  
> 
> I don't think Giulio is carrying this patch anymore - I just needed the
> functionality and he'd already done the work.  I'm quite happy to take
> any beatings related to the implementation.
> 
> And yeah, *piles* of fds.  Shortly after I complained last time I
> checked what my local limits are, and they're pretty huge.
> 
> Yesterday I learned that lots of other distributions are still happy
> with 4096 as a hard limit.
> 
> > The server-side fds must be kept open also after the client has
> > destroyed the wl_shm_pool (and can no longer resize it), as the usual
> > usage pattern is to create a pool, create a wl_buffer, destroy the
> > pool. No fd is left open on the client, so the client is not in risk of
> > running out of open file slots.
> > 
> > Not only that, but a single server process is now keeping the shm fds
> > open from all clients.  
> 
> Also, no matter how high the fd limit is raised, a client would be able
> to create an unbounded number of open fds just by allocating a bunch of
> small shm pools?

I don't think we've actually reviewed protocols against fd-flooding the
compositor... but OTOH, is it even possible? Don't we get fds open
already when libwayland-server buffers the incoming requests, which
means the actual protocol semantics are irrelevant?

So if we wanted to limit open fds per client, it needs to be done as
early as right after the recvmsg() call, I think.


Thanks,
pq

> I don't think we currently have to do anything to limit the number of
> fds a single client consumes, but if we did something like this we'd
> need to.
> 
> > We should carefully consider that before accepting this patch.  
> 
> Indeed!
> 
> > Maybe it's time to start lobbying for raising the open fd limits? We
> > use fds for so many things.  
> 
> Couldn't hurt. ;)
> 


pgpqCbnNO4Xhu.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 1/2] simple-touch: Use damage_buffer if available

[Giulio Camuffo]

2016-02-12 6:26 GMT+02:00 Bryce Harrington :
> On Fri, Jan 08, 2016 at 03:00:55PM -0600, Derek Foreman wrote:
>> Signed-off-by: Derek Foreman 
>> ---
>>  clients/simple-touch.c | 18 +++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> Given that the demo clients exist to show the functionality of the
> weston they're included with, damage_buffer is always going to be
> available, isn't it?

But it can be used to test other compositors too. I think demo clients
should strive to be compatible with as many compositors as possible.


>
> Maybe just keep the client concise and just use the new API?
>
> If we do want to demonstrate both routines, then IMHO the code should
> either show or explain why one would prefer one over the other.  I
> imagine a new wayland coder would be looking at simple-touch as a simple
> example of how to do touch, and might get confused seeing two ways to do
> it.
>
> Bryce
>
>> diff --git a/clients/simple-touch.c b/clients/simple-touch.c
>> index 446f2ca..383d802 100644
>> --- a/clients/simple-touch.c
>> +++ b/clients/simple-touch.c
>> @@ -56,6 +56,7 @@ struct touch {
>>   int has_argb;
>>   int width, height;
>>   void *data;
>> + bool use_damage_buffer;
>>  };
>>
>>  static void
>> @@ -148,7 +149,10 @@ touch_paint(struct touch *touch, int32_t x, int32_t y, 
>> int32_t id)
>>   p[2] = c;
>>
>>   wl_surface_attach(touch->surface, touch->buffer, 0, 0);
>> - wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
>> + if (touch->use_damage_buffer)
>> + wl_surface_damage_buffer(touch->surface, x - 2, y - 2, 5, 5);
>> + else
>> + wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
>>   /* todo: We could queue up more damage before committing, if there
>>* are more input events to handle.
>>*/
>> @@ -269,9 +273,13 @@ handle_global(void *data, struct wl_registry *registry,
>>   struct touch *touch = data;
>>
>>   if (strcmp(interface, "wl_compositor") == 0) {
>> + int cv = MIN(WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION, version);
>> +
>>   touch->compositor =
>>   wl_registry_bind(registry, name,
>> -  _compositor_interface, 1);
>> +  _compositor_interface, cv);
>> + if (cv >= WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION)
>> + touch->use_damage_buffer = true;
>>   } else if (strcmp(interface, "wl_shell") == 0) {
>>   touch->shell =
>>   wl_registry_bind(registry, name,
>> @@ -308,6 +316,7 @@ touch_create(int width, int height)
>>   touch->display = wl_display_connect(NULL);
>>   assert(touch->display);
>>
>> + touch->use_damage_buffer = false;
>>   touch->has_argb = 0;
>>   touch->registry = wl_display_get_registry(touch->display);
>>   wl_registry_add_listener(touch->registry, _listener, touch);
>> @@ -337,7 +346,10 @@ touch_create(int width, int height)
>>
>>   memset(touch->data, 64, width * height * 4);
>>   wl_surface_attach(touch->surface, touch->buffer, 0, 0);
>> - wl_surface_damage(touch->surface, 0, 0, width, height);
>> + if (touch->use_damage_buffer)
>> + wl_surface_damage_buffer(touch->surface, 0, 0, width, height);
>> + else
>> + wl_surface_damage(touch->surface, 0, 0, width, height);
>>   wl_surface_commit(touch->surface);
>>
>>   return touch;
>> --
>> 2.6.4
>>
>> ___
>> wayland-devel mailing list
>> wayland-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> 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

Re: [PATCH] server: Add get/set user data for wl_client

[Pekka Paalanen]

Hi,

I think we should think this a little more.

There is no absolute requirement to add a user_data member to
wl_client, because you can use a destroy listener to look up your user
data struct. When you attach user data, you also want to always be
notified about wl_client destruction, which means you always have a
destroy listener.

The downside of using a destroy listener to look up your user data
struct is that it is an O(number of destroy listeners) operation rather
than O(1), but usually that number should be 1 or 2 I think, your user
data destructor included.

So the first question is, is this really worthwhile?

For comparison to another related patch, adding the client created
signal is obviously worthwhile, since there is no other sure way to be
notified about new wl_clients.

Then let's assume user_data for wl_client is worthwhile. The closest
example of a server-side structure with user data is wl_resource.
wl_resource also has an explicit wl_resource_destroy_func_t member, and
the rationale is clear: if you add user data, you want to be able to
tear down the user data at the right time, so you always need a way to
be notified about destruction.

The second question is, should also wl_client have an explicit user
data destructor function? If yes, you have to add API for it.

The third question is: do all wl_client objects have the same single
owner?

The owner problem is particularly visible in the client side with
wl_proxy, which also has a single user data member of type void
pointer. This creates two problems:

- If wl_proxy should have more than one user data attached, only one of
  them can use the user data member and the others must use the
  destructor trick anyway - oops, wl_proxy does not have a destroy
  signal, but at least wl_client does.

- It is not always obvious who owns the user data. If there are more
  than one component that assume they own the wl_proxy and its user
  data, then passing the wl_proxy to another component that also
  assumes the same causes the other component to access not the data it
  thinks it gets.

The latter problem is especially dangerous when an application is
sharing the same connection with multiple toolkits. Both toolkits may
create wl_surface objects and naturally put their own pointer in the
wl_proxy user data field. They also listen for input events, and input
events carry a reference to a wl_surface. Therefore, a toolkit may
receive a wl_proxy with the other toolkit's user data, and crash or
misbehave due to assuming it is its own.

In the wl_client case, I think the owner issue should be negligible,
there is always one obvious owner for the wl_client and its user data -
or that is what I assume, as I haven't read any other compositor code
than Weston's.

In summary, while there is technically nothing wrong with this idea per
se, I do wonder if it is a net win.

I believe that if there is a dedicated user data slot, there should
also be a dedicated destructor function slot for it.

What do others think? I could go either way.


On Wed, 27 Jan 2016 19:34:56 +0900
Sung-Jin Park  wrote:

> Signed-off-by: Sung-Jin Park 
> ---
>  src/wayland-server-core.h |  6 ++
>  src/wayland-server.c  | 13 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> index e8e1e9c..6990423 100644
> --- a/src/wayland-server-core.h
> +++ b/src/wayland-server-core.h
> @@ -186,6 +186,12 @@ int
>  wl_client_get_fd(struct wl_client *client);
>  
>  void
> +wl_client_set_user_data(struct wl_client *client, void *data);
> +
> +void *
> +wl_client_get_user_data(struct wl_client *client);
> +
> +void
>  wl_client_add_destroy_listener(struct wl_client *client,
>  struct wl_listener *listener);
>  
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 6654cd7..e7a6eae 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;
> + void *data;

Please name this user_data.

>  };
>  
>  struct wl_display {
> @@ -526,6 +527,18 @@ wl_client_get_fd(struct wl_client *client)
>   return wl_connection_get_fd(client->connection);
>  }
>  
> +WL_EXPORT void
> +wl_client_set_user_data(struct wl_client *client, void *data)
> +{
> + client->data = data;
> +}
> +
> +WL_EXPORT void *
> +wl_client_get_user_data(struct wl_client *client)
> +{
> + return client->data;
> +}

Documentation is missing.

> +
>  /** Look up an object in the client name space
>   *
>   * \param client The client object

Thanks
pq


pgpjFvMZWzOwz.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland-protocols] xdg-shell: Add startup notification

[Carlos Garnacho]

The xdg_launcher interface is added for the launcher, it's used
to notify of the startup ID to be transmitted to the launchee,
plus notifications about the startup success/failure.

On the launchee side, we now have xdg_shell.set_startup_id,
which will notify the compositor of startup finalization.

This has been made to be compatible with the XDG Startup
Notification spec available for X11, the startup ID is
transmitted from the launcher to the launchee in the same
ways, so we can launch x11 from wayland applications and
viceversa. The notable difference is that wayland launchers
receive startup IDs that are guaranteed to be unique, whereas
in X11 this is a best effort of the launcher client.

Some notes have also been added about focus stealing prevention,
although that's mostly up for compositors to implement.

Signed-off-by: Carlos Garnacho 
---

I've got no full implementations yet, so this is mostly an RFC at the
moment. I mainly wonder, should we add a "serial" argument to the
create_launcher request? that'd at least ensure the launcher application
has some sort of focus, although nothing prevents an application from
being a fork bomb otherwise.

 unstable/xdg-shell/xdg-shell-unstable-v5.xml | 71 +++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v5.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v5.xml
index 542491f..1c4ef54 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v5.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v5.xml
@@ -27,7 +27,7 @@
 DEALINGS IN THE SOFTWARE.
   
 
-  
+  
 
   xdg_shell allows clients to turn a wl_surface into a "real window"
   which can be dragged, resized, stacked, and moved around by the
@@ -135,6 +135,35 @@
   
   
 
+
+
+
+  
+Creates a new launcher context.
+
+The surface argument is the toplevel where the application
+was launched from, compositors may want to place the launched
+application relative to the launcher surface.
+
+Compositors that desire to implement focus stealing prevention
+can mark the time this request is received as the "startup" time.
+  
+  
+  
+
+
+
+  
+Notifies the compositor of the startup ID of this launched application.
+Applications will typically receive this through the DESKTOP_STARTUP_ID
+environment variable as set by its launcher, and should unset the
+environment variable right after this request, in order to avoid
+propagating it to child processes.
+
+Compositors will ignore unknown startup IDs.
+  
+  
+
   
 
   
@@ -622,4 +651,44 @@
 
 
   
+
+  
+
+  xdg_launcher allows clients to get the necessary context to launch
+  applications, so the compositor can provide feedback about the
+  application being launched.
+
+
+
+  
+Destroys this xdg_launcher object.
+  
+
+
+
+  
+Notifies of an unique startup_id (eg. UUIDs) to be used for the
+application about to be launched.
+
+In order to guarantee interoperation with the XDG Startup Notification
+spec, this startup_id is recommended to be transmitted to the launched
+application through the DESKTOP_STARTUP_ID environment variable.
+  
+  
+
+
+
+  
+Notifies that the compositor is no longer watching this launched
+application.
+  
+
+
+
+  
+Notifies that the launched application successfully called
+xdg_shell.set_startup_id after startup.
+  
+
+  
 
-- 
2.5.0

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

[PATCH wayland] server: add listener API for new clients

[nicesj]

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 

diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
index cb72981..1bc4d6b 100644
--- a/src/wayland-server-core.h
+++ b/src/wayland-server-core.h
@@ -156,19 +156,8 @@ void
 wl_display_add_destroy_listener(struct wl_display *display,
struct wl_listener *listener);
 
-/** Add a listener for getting a notification of creation of clients.
- *  If you add a listener, server will emits a signal if a new client
- *  is created.
- *
- *  \ref wl_client_create
- *  \ref wl_display
- *  \ref wl_listener
- *
- * \param display The display object
- * \param listener Signal handler object
- */
 void
-wl_display_add_create_client_listener(struct wl_display *display,
+wl_display_add_client_created_listener(struct wl_display *display,
struct wl_listener *listener);
 
 struct wl_listener *
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 0eff8f6..2857b1d 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1357,8 +1357,19 @@ wl_display_add_destroy_listener(struct wl_display 
*display,
wl_signal_add(>destroy_signal, listener);
 }
 
+/** Registers a listener for the client connection signal.
+ *  When a new client object is created, \a listener will be notified, carring
+ *  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_create_client_listener(struct wl_display *display,
+wl_display_add_client_created_listener(struct wl_display *display,
struct wl_listener *listener)
 {
wl_signal_add(>create_client_signal, listener);
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC wayland 01/18] shm: add getters for the fd and offset

[Derek Foreman]

On 11/02/16 02:52 AM, Pekka Paalanen wrote:
> On Wed, 10 Feb 2016 12:00:48 -0600 Derek Foreman
>  wrote:
> 
>> On 10/02/16 07:08 AM, Pekka Paalanen wrote:
>>> On Tue,  9 Feb 2016 10:55:48 -0600 Derek Foreman
>>>  wrote:
>>> 
 From: Giulio Camuffo 
 
 This allows to share the buffer data by mmapping the fd
 again. Reviewed-by: David FORT
 
 
 Signed-off-by: Derek Foreman  --- 
 src/wayland-server-core.h |  6 ++ src/wayland-shm.c
 | 16 +++- 2 files changed, 21 insertions(+), 1
 deletion(-)
 
 diff --git a/src/wayland-server-core.h
 b/src/wayland-server-core.h index e8e1e9c..3316022 100644 ---
 a/src/wayland-server-core.h +++ b/src/wayland-server-core.h 
 @@ -480,6 +480,12 @@ wl_shm_buffer_create(struct wl_client
 *client, uint32_t id, int32_t width, int32_t height, int32_t
 stride, uint32_t format) WL_DEPRECATED;
 
 +int +wl_shm_buffer_get_offset(struct wl_shm_buffer
 *buffer); + +int +wl_shm_buffer_get_fd(struct wl_shm_buffer
 *buffer); + void wl_log_set_handler_server(wl_log_func_t
 handler);
 
 #ifdef  __cplusplus diff --git a/src/wayland-shm.c
 b/src/wayland-shm.c index a4343a4..911165d 100644 ---
 a/src/wayland-shm.c +++ b/src/wayland-shm.c @@ -55,6 +55,7 @@
 struct wl_shm_pool { int refcount; char *data; int32_t size; 
 +  int fd; };
 
 struct wl_shm_buffer { @@ -80,6 +81,7 @@
 shm_pool_unref(struct wl_shm_pool *pool) return;
 
 munmap(pool->data, pool->size); +  close(pool->fd); 
 free(pool); }
 
 @@ -253,7 +255,7 @@ shm_create_pool(struct wl_client *client,
 struct wl_resource *resource, "failed mmap fd %d", fd); goto
 err_close; } - close(fd); +pool->fd = fd;
 
 pool->resource = wl_resource_create(client,
 _shm_pool_interface, 1, id); @@ -421,6 +423,18 @@
 wl_shm_pool_unref(struct wl_shm_pool *pool) 
 shm_pool_unref(pool); }
 
 +WL_EXPORT int +wl_shm_buffer_get_offset(struct wl_shm_buffer
 *buffer) +{ +  return buffer->offset; +} + +WL_EXPORT int 
 +wl_shm_buffer_get_fd(struct wl_shm_buffer *buffer) +{ +
 return buffer->pool->fd; +} + static void 
 reraise_sigbus(void) {
>>> 
>>> Hi,
>>> 
>>> Derek did wonder about this the last time with this patch, and
>>> I want to reiterate, that leaving the pool fd open causes the
>>> server process to have much much more open fds.
>> 
>> I don't think Giulio is carrying this patch anymore - I just
>> needed the functionality and he'd already done the work.  I'm
>> quite happy to take any beatings related to the implementation.
>> 
>> And yeah, *piles* of fds.  Shortly after I complained last time
>> I checked what my local limits are, and they're pretty huge.
>> 
>> Yesterday I learned that lots of other distributions are still
>> happy with 4096 as a hard limit.
>> 
>>> The server-side fds must be kept open also after the client
>>> has destroyed the wl_shm_pool (and can no longer resize it), as
>>> the usual usage pattern is to create a pool, create a
>>> wl_buffer, destroy the pool. No fd is left open on the client,
>>> so the client is not in risk of running out of open file
>>> slots.
>>> 
>>> Not only that, but a single server process is now keeping the
>>> shm fds open from all clients.
>> 
>> Also, no matter how high the fd limit is raised, a client would
>> be able to create an unbounded number of open fds just by
>> allocating a bunch of small shm pools?
> 
> I don't think we've actually reviewed protocols against fd-flooding
> the compositor... but OTOH, is it even possible? Don't we get fds
> open already when libwayland-server buffers the incoming requests,
> which means the actual protocol semantics are irrelevant?

Yeah, they're already open when received.  For shm pools we currently
close them before we move on to the next closure, if I understand
things correctly.

What I'm wondering about now is drag and drop/selection behaviour...
I think we're ok, and we divest ourselves of the open fd before
processing any additional closures...

Oh, dmabuf looks like fun, I just made weston hold open 5000 fds for a
single client which has closed all its own fds.  This seems
potentially problematic. :)

> So if we wanted to limit open fds per client, it needs to be done
> as early as right after the recvmsg() call, I think.

I can't off the top of my head think of a good way to check how many
files the current process has open.  libwayland will know how often
we're passed open file descriptors, but won't know when the compositor
has closed them.

How would we limit the number of open file descriptors anyway? :)

> 
> Thanks, pq
> 
>> I don't think we currently have to do anything to limit the
>> number of fds a single client consumes, but if we did something
>> like this we'd need to.
>> 
>>> We should 

[PATCH] clients: Use zalloc

[Bryce Harrington]

Signed-off-by: Bryce Harrington 
---
 clients/desktop-shell.c| 3 ++-
 clients/fullscreen.c   | 7 ++-
 clients/ivi-shell-user-interface.c | 3 ++-
 clients/multi-resource.c   | 3 ++-
 clients/presentation-shm.c | 7 ---
 clients/simple-damage.c| 3 ++-
 clients/simple-dmabuf-intel.c  | 3 ++-
 clients/simple-dmabuf-v4l.c| 3 ++-
 clients/simple-shm.c   | 3 ++-
 clients/subsurfaces.c  | 3 ++-
 clients/weston-info.c  | 3 ++-
 clients/window.c   | 3 ++-
 12 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 6ab76dc..49a3077 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -46,6 +46,7 @@
 #include "shared/cairo-util.h"
 #include "shared/config-parser.h"
 #include "shared/helpers.h"
+#include "shared/zalloc.h"
 
 #include "weston-desktop-shell-client-protocol.h"
 
@@ -1203,7 +1204,7 @@ create_output(struct desktop *desktop, uint32_t id)
 {
struct output *output;
 
-   output = calloc(1, sizeof *output);
+   output = zalloc(sizeof *output);
if (!output)
return;
 
diff --git a/clients/fullscreen.c b/clients/fullscreen.c
index 653372a..e2e6477 100644
--- a/clients/fullscreen.c
+++ b/clients/fullscreen.c
@@ -36,6 +36,7 @@
 #include 
 #include "window.h"
 #include "fullscreen-shell-unstable-v1-client-protocol.h"
+#include "shared/zalloc.h"
 
 struct fs_output {
struct wl_list link;
@@ -460,7 +461,11 @@ output_handler(struct output *output, void *data)
if (fsout->output == output)
return;
 
-   fsout = calloc(1, sizeof *fsout);
+   fsout = zalloc(sizeof *fsout);
+   if (fsout == NULL) {
+   fprintf(stderr, "out of memory in output_handler\n");
+   return;
+   }
fsout->output = output;
wl_list_insert(>output_list, >link);
 }
diff --git a/clients/ivi-shell-user-interface.c 
b/clients/ivi-shell-user-interface.c
index 2e0622b..1349c73 100644
--- a/clients/ivi-shell-user-interface.c
+++ b/clients/ivi-shell-user-interface.c
@@ -40,6 +40,7 @@
 #include "shared/config-parser.h"
 #include "shared/helpers.h"
 #include "shared/os-compatibility.h"
+#include "shared/zalloc.h"
 #include "ivi-application-client-protocol.h"
 #include "ivi-hmi-controller-client-protocol.h"
 
@@ -177,7 +178,7 @@ fail_on_null(void *p, size_t size, char *file, int32_t line)
 static void *
 mem_alloc(size_t size, char *file, int32_t line)
 {
-   return fail_on_null(calloc(1, size), size, file, line);
+   return fail_on_null(zalloc(size), size, file, line);
 }
 
 #define MEM_ALLOC(s) mem_alloc((s),__FILE__,__LINE__)
diff --git a/clients/multi-resource.c b/clients/multi-resource.c
index c30d38a..4ed4d50 100644
--- a/clients/multi-resource.c
+++ b/clients/multi-resource.c
@@ -40,6 +40,7 @@
 
 #include 
 #include "shared/os-compatibility.h"
+#include "shared/zalloc.h"
 
 struct device {
enum { KEYBOARD, POINTER } type;
@@ -87,7 +88,7 @@ xzalloc(size_t s)
 {
void *p;
 
-   p = calloc(1, s);
+   p = zalloc(s);
if (p == NULL) {
fprintf(stderr, "%s: out of memory\n", 
program_invocation_short_name);
exit(EXIT_FAILURE);
diff --git a/clients/presentation-shm.c b/clients/presentation-shm.c
index 120c40c..ee662d6 100644
--- a/clients/presentation-shm.c
+++ b/clients/presentation-shm.c
@@ -37,6 +37,7 @@
 
 #include 
 #include "shared/helpers.h"
+#include "shared/zalloc.h"
 #include "shared/os-compatibility.h"
 #include "presentation_timing-client-protocol.h"
 
@@ -212,7 +213,7 @@ create_window(struct display *display, int width, int 
height,
 "presentation-shm: %s [Delay %i msecs]", run_mode_name[mode],
 commit_delay_msecs);
 
-   window = calloc(1, sizeof *window);
+   window = zalloc(sizeof *window);
if (!window)
return NULL;
 
@@ -500,7 +501,7 @@ window_create_feedback(struct window *window, uint32_t 
frame_stamp)
if (!pres)
return;
 
-   feedback = calloc(1, sizeof *feedback);
+   feedback = zalloc(sizeof *feedback);
if (!feedback)
return;
 
@@ -677,7 +678,7 @@ display_add_output(struct display *d, uint32_t name, 
uint32_t version)
 {
struct output *o;
 
-   o = calloc(1, sizeof(*o));
+   o = zalloc(sizeof(*o));
assert(o);
 
o->output = wl_registry_bind(d->registry, name,
diff --git a/clients/simple-damage.c b/clients/simple-damage.c
index 0353ccc..321b90f 100644
--- a/clients/simple-damage.c
+++ b/clients/simple-damage.c
@@ -37,6 +37,7 @@
 
 #include 
 #include "shared/os-compatibility.h"
+#include "shared/zalloc.h"
 #include "xdg-shell-unstable-v5-client-protocol.h"
 #include "fullscreen-shell-unstable-v1-client-protocol.h"
 #include "scaler-client-protocol.h"
@@ -271,7 

[PATCH] option-parser: Handle short double-arg options

[Bryce Harrington]

weston allows both short and long style options to take arguments.  In
the case of short options, allow an optional space between the option
name and value.  E.g., previously you could launch weston this way:

  weston -i2 -cmyconfig.ini

now you can also launch it like this:

  weston -i 2 -c myconfig.ini

Signed-off-by: Bryce Harrington 
---
 shared/option-parser.c | 41 ++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/shared/option-parser.c b/shared/option-parser.c
index f1cc529..d5fee8e 100644
--- a/shared/option-parser.c
+++ b/shared/option-parser.c
@@ -98,14 +98,37 @@ short_option(const struct weston_option *options, int 
count, char *arg)
 
return 1;
}
-   } else {
+   } else if (arg[2]) {
return handle_option(options + k, arg + 2);
+   } else {
+   return 0;
}
}
 
return 0;
 }
 
+static int
+short_option_with_arg(const struct weston_option *options, int count, char 
*arg, char *param)
+{
+   int k;
+
+   if (!arg[1])
+   return 0;
+
+   for (k = 0; k < count; k++) {
+   if (options[k].short_name != arg[1])
+   continue;
+
+   if (options[k].type == WESTON_OPTION_BOOLEAN)
+   continue;
+
+   return handle_option(options + k, param);
+   }
+
+   return 0;
+}
+
 int
 parse_options(const struct weston_option *options,
  int count, int *argc, char *argv[])
@@ -115,10 +138,22 @@ parse_options(const struct weston_option *options,
for (i = 1, j = 1; i < *argc; i++) {
if (argv[i][0] == '-') {
if (argv[i][1] == '-') {
+   /* Long option, e.g. --foo or --foo=bar */
if (long_option(options, count, argv[i]))
continue;
-   } else if (short_option(options, count, argv[i]))
-   continue;
+
+   } else {
+   /* Short option, e.g -f or -f42 */
+   if (short_option(options, count, argv[i]))
+   continue;
+
+   /* ...also handle -f 42 */
+   if (i+1 < *argc &&
+   short_option_with_arg(options, count, 
argv[i], argv[i+1])) {
+   i++;
+   continue;
+   }
+   }
}
argv[j++] = argv[i];
}
-- 
1.9.1

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

Re: [PATCH] connection: Don't add uninitialized memory as 4 byte alignment padding

[Bryce Harrington]

On Thu, Feb 11, 2016 at 11:34:15AM +0200, Pekka Paalanen wrote:
> On Thu, 11 Feb 2016 09:47:29 +0800
> Jonas Ådahl  wrote:
> 
> > On Wed, Feb 10, 2016 at 12:37:43PM -0600, Derek Foreman wrote:
> > > On 10/02/16 09:35 AM, Jonas Ådahl wrote:  
> > > > When we are adding padding bytes making our wl_buffer buffer content 4
> > > > byte aligned, we are just moving the pointer. Since the buffer is
> > > > allocated using plain malloc(), this means our padding bytes are
> > > > effectively uninitialized data, which could be anything previously
> > > > allocated in the server process. As we'll be sharing this buffer
> > > > content with arbitrary clients, we are effectively sharing private
> > > > memory with every client, and even though a well behaving client will
> > > > discard any such memory, a malicious client may not.
> > > > 
> > > > Therefor, to avoid any potential missuse of the uninitialized padding
> > > > memory shared between the server and client, initialize the buffer
> > > > content to 0, making the padding bytes always 0.  
> > > 
> > > Cool - is this the cause of the valgrind complaints I've never bothered
> > > to track down? :)  
> > 
> > Yes. Should have mentioned that in the commit message, as well as the
> > bug (https://bugs.freedesktop.org/show_bug.cgi?id=94071 ) that made me
> > actually dig out what memory was uninitialized. I'll add those two
> > things to the commit message.
> > 
> > >   
> > > > Signed-off-by: Jonas Ådahl 
> > > > ---
> > > >  src/connection.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/connection.c b/src/connection.c
> > > > index 65b64e9..c0e322f 100644
> > > > --- a/src/connection.c
> > > > +++ b/src/connection.c
> > > > @@ -1137,7 +1137,7 @@ wl_closure_send(struct wl_closure *closure, 
> > > > struct wl_connection *connection)
> > > > return -1;
> > > >  
> > > > buffer_size = buffer_size_for_closure(closure);
> > > > -   buffer = malloc(buffer_size * sizeof buffer[0]);
> > > > +   buffer = zalloc(buffer_size * sizeof buffer[0]);  
> > > 
> > > Oh, took me a couple of reads to realize the padding is between certain
> > > potential members in the closure. (and not trivially at the start or end
> > > of the buffer)
> 
> Hi,
> 
> just to see I understood right too, you mean the string and array types
> in serialize_closure() which use DIV_ROUNDUP() and nothing else?
> 
> If that is the case, then have my
> Reviewed-by: Pekka Paalanen 
> 
> and with that I also nominate this patch to be landed for 1.10: it is
> trivial and obviously correct, and if it in the unlikely event causes
> any problems, those problems have existed before. It might even be
> considered as a security fix by some far stretch.

Yep, good simple fix:

Reviewed-by: Bryce Harrington 

and landed for 1.10:
To ssh://git.freedesktop.org/git/wayland/wayland
   1906a90..bf34ac7  master -> master

Bryce
 
> > > I guess we could zero just the pad bytes in serialize_closure, I don't
> > > know whether that falls under "premature optimization" or not, though.
> > > Looks pretty easy to do...  
> > 
> > We could do that, but is calloc() really any noticably slower than
> > malloc() in the way we are using it?
> 
> I wouldn't think there is any observable performance penalty here. And
> I like the simplicity of the patch and the catch-all nature of the fix.
>
> Thanks,
> pq
> 
> > It is fairly easy (I did it at first just to make sure it was those
> > bytes only that triggered the warning), but s/malloc/zalloc/ seemed like
> > the cleaner solution to me.
> > 
> > 
> > Jonas
> > 
> > > 
> > > In any case,
> > > Reviewed-by: Derek Foreman 
> > > 
> > > Thanks,
> > > Derek
> > >   
> > > > if (buffer == NULL)
> > > > return -1;
> > > >  
> > > >   

> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2
> 
> iQIVAwUBVrxVlyNf5bQRqqqnAQgpyhAAkBMiboYyTiE7MRPuNc3LfDxnfx9NXJdB
> SY4miIA4VUu4XsZWSaXE6/48HFnFqlDaNhKuPpHCap8aInMrG0T1ZDw2McYGLM8Y
> g3U48ZyQNvzyHE+PzOWcdPfmGsKGkwsmA+Epo16DEAqbbzWV+pu66Ny0gILWHPmX
> g7i206GJ3eetYdQPtj0HdxPZe7zFTNMI9dDLhEFPNFDwSm+I+RdJICXoMCFYyL7z
> OSXj0X7OWAJk0/oFqfV3DH2xXi6elW629GJmUgNVfF5GqP6sSzaGerccr6sYjXI5
> kTyL6CP1C6kNt+ylL0FG/80Oy3ET7/7+FnUQ/auic9UWGAwRz5hbpdj9rKcmpqf7
> zcz2SmGPGH74O/ZFK5NEkfU9TkDrwFUt/jtZ9Bwak4WyrmG0XhyuTz3vvP6fEBsj
> dCzZy/W9LE8Fj5Yp1QM9tLsaITpgMjeHyE3kdcOBMq1Dg5rcKsTCn++YarQ7NY6X
> zMF9Dw469GHmJP1+HdDxmDvH5tdWPm0nCXozx/7sYS/So4DElPHALj0QkY0tNI7U
> X47SxYrBklJIglxvwdnh8MfmBQN8n5gMK3KjIxHST+iIgK0W9jUnjma5XoKXdpVm
> SmK3AsJmCegyXmpUsQjCnS+wlI8Lv6NLI2chxpztVL1uBLKN69Bs7155/LLtQE8x
> BX38/Od7YBc=
> =YXcL
> -END PGP SIGNATURE-

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

[PATCH libinput] touchpad: if the second finger is within 20x5mm, bias towards 2fg scrolling

[Peter Hutterer]

Scrolling is much more common than a 2fg spread gesture, so if the finger
position indicates that the fingers are next to each other, switch to
scrolling immediately.

https://bugs.freedesktop.org/show_bug.cgi?id=93504

Signed-off-by: Peter Hutterer 
---
 src/evdev-mt-touchpad-gestures.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c
index dc8d606..a9d7fa1 100644
--- a/src/evdev-mt-touchpad-gestures.c
+++ b/src/evdev-mt-touchpad-gestures.c
@@ -325,8 +325,9 @@ tp_gesture_handle_state_unknown(struct tp_dispatch *tp, 
uint64_t time)
struct tp_touch *first = tp->gesture.touches[0],
*second = tp->gesture.touches[1];
int dir1, dir2;
-   int yres = tp->device->abs.absinfo_y->resolution;
-   int vert_distance;
+   int yres = tp->device->abs.absinfo_y->resolution,
+   xres = tp->device->abs.absinfo_x->resolution;
+   int vert_distance, horiz_distance;
 
/* for two-finger gestures, if the fingers stay unmoving for a
 * while, assume (slow) scroll */
@@ -338,10 +339,16 @@ tp_gesture_handle_state_unknown(struct tp_dispatch *tp, 
uint64_t time)
 
/* Else check if one finger is > 20mm below the others */
vert_distance = abs(first->point.y - second->point.y);
+   horiz_distance = abs(first->point.x - second->point.x);
if (vert_distance > 20 * yres &&
tp->gesture.enabled) {
tp_gesture_init_pinch(tp);
return GESTURE_STATE_PINCH;
+   /* Else if the fingers are within 20x5mm of each other */
+   } else if (vert_distance < 5 * yres &&
+  horiz_distance < 20 * xres) {
+   tp_gesture_set_scroll_buildup(tp);
+   return GESTURE_STATE_SCROLL;
}
 
/* Else wait for both fingers to have moved */
-- 
2.5.0

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

Re: [PATCH weston 1/2] simple-touch: Use damage_buffer if available

[Bryce Harrington]

On Fri, Jan 08, 2016 at 03:00:55PM -0600, Derek Foreman wrote:
> Signed-off-by: Derek Foreman 
> ---
>  clients/simple-touch.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Given that the demo clients exist to show the functionality of the
weston they're included with, damage_buffer is always going to be
available, isn't it?

Maybe just keep the client concise and just use the new API?

If we do want to demonstrate both routines, then IMHO the code should
either show or explain why one would prefer one over the other.  I
imagine a new wayland coder would be looking at simple-touch as a simple
example of how to do touch, and might get confused seeing two ways to do
it.

Bryce
 
> diff --git a/clients/simple-touch.c b/clients/simple-touch.c
> index 446f2ca..383d802 100644
> --- a/clients/simple-touch.c
> +++ b/clients/simple-touch.c
> @@ -56,6 +56,7 @@ struct touch {
>   int has_argb;
>   int width, height;
>   void *data;
> + bool use_damage_buffer;
>  };
>  
>  static void
> @@ -148,7 +149,10 @@ touch_paint(struct touch *touch, int32_t x, int32_t y, 
> int32_t id)
>   p[2] = c;
>  
>   wl_surface_attach(touch->surface, touch->buffer, 0, 0);
> - wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
> + if (touch->use_damage_buffer)
> + wl_surface_damage_buffer(touch->surface, x - 2, y - 2, 5, 5);
> + else
> + wl_surface_damage(touch->surface, x - 2, y - 2, 5, 5);
>   /* todo: We could queue up more damage before committing, if there
>* are more input events to handle.
>*/
> @@ -269,9 +273,13 @@ handle_global(void *data, struct wl_registry *registry,
>   struct touch *touch = data;
>  
>   if (strcmp(interface, "wl_compositor") == 0) {
> + int cv = MIN(WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION, version);
> +
>   touch->compositor =
>   wl_registry_bind(registry, name,
> -  _compositor_interface, 1);
> +  _compositor_interface, cv);
> + if (cv >= WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION)
> + touch->use_damage_buffer = true;
>   } else if (strcmp(interface, "wl_shell") == 0) {
>   touch->shell =
>   wl_registry_bind(registry, name,
> @@ -308,6 +316,7 @@ touch_create(int width, int height)
>   touch->display = wl_display_connect(NULL);
>   assert(touch->display);
>  
> + touch->use_damage_buffer = false;
>   touch->has_argb = 0;
>   touch->registry = wl_display_get_registry(touch->display);
>   wl_registry_add_listener(touch->registry, _listener, touch);
> @@ -337,7 +346,10 @@ touch_create(int width, int height)
>  
>   memset(touch->data, 64, width * height * 4);
>   wl_surface_attach(touch->surface, touch->buffer, 0, 0);
> - wl_surface_damage(touch->surface, 0, 0, width, height);
> + if (touch->use_damage_buffer)
> + wl_surface_damage_buffer(touch->surface, 0, 0, width, height);
> + else
> + wl_surface_damage(touch->surface, 0, 0, width, height);
>   wl_surface_commit(touch->surface);
>  
>   return touch;
> -- 
> 2.6.4
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] compositor-wayland: Use damage_buffer if available

[Bryce Harrington]

On Fri, Jan 08, 2016 at 03:00:56PM -0600, Derek Foreman wrote:
> Try to replace most usage of wl_surface.damage with
> wl_surface.damage_buffer.  Some calls are beyond our control,
> such as the gl_renderer's damage handling.
> 
> Signed-off-by: Derek Foreman 

Reviewed-by: Bryce Harrington 

> ---
>  src/compositor-wayland.c | 52 
> ++--
>  1 file changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 4ea0b7d..77ed1e4 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -79,6 +79,8 @@ struct wayland_backend {
>   struct wl_cursor *cursor;
>  
>   struct wl_list input_list;
> +
> + bool use_damage_buffer;
>  };
>  
>  struct wayland_output {
> @@ -342,6 +344,8 @@ static const struct wl_callback_listener frame_listener = 
> {
>  static void
>  draw_initial_frame(struct wayland_output *output)
>  {
> + struct wayland_backend *b =
> + (struct wayland_backend *) output->base.compositor->backend;
>   struct wayland_shm_buffer *sb;
>  
>   sb = wayland_output_get_shm_buffer(output);
> @@ -352,9 +356,15 @@ draw_initial_frame(struct wayland_output *output)
>   sb->output = NULL;
>  
>   wl_surface_attach(output->parent.surface, sb->buffer, 0, 0);
> - wl_surface_damage(output->parent.surface, 0, 0,
> -   output->base.current_mode->width,
> -   output->base.current_mode->height);
> +
> + if (b->use_damage_buffer)
> + wl_surface_damage(output->parent.surface, 0, 0,
> +   output->base.current_mode->width,
> +   output->base.current_mode->height);
> + else
> + wl_surface_damage_buffer(output->parent.surface, 0, 0,
> +  output->base.current_mode->width,
> +  output->base.current_mode->height);
>  }
>  
>  static void
> @@ -515,6 +525,8 @@ wayland_output_update_shm_border(struct 
> wayland_shm_buffer *buffer)
>  static void
>  wayland_shm_buffer_attach(struct wayland_shm_buffer *sb)
>  {
> + struct wayland_backend *b =
> + (struct wayland_backend *) sb->output->base.compositor->backend;
>   pixman_region32_t damage;
>   pixman_box32_t *rects;
>   int32_t ix, iy, iwidth, iheight, fwidth, fheight;
> @@ -550,10 +562,20 @@ wayland_shm_buffer_attach(struct wayland_shm_buffer *sb)
>  
>   rects = pixman_region32_rectangles(, );
>   wl_surface_attach(sb->output->parent.surface, sb->buffer, 0, 0);
> - for (i = 0; i < n; ++i)
> - wl_surface_damage(sb->output->parent.surface, rects[i].x1,
> -   rects[i].y1, rects[i].x2 - rects[i].x1,
> -   rects[i].y2 - rects[i].y1);
> + for (i = 0; i < n; ++i) {
> + if (b->use_damage_buffer)
> + wl_surface_damage_buffer(sb->output->parent.surface,
> +  rects[i].x1,
> +  rects[i].y1,
> +  rects[i].x2 - rects[i].x1,
> +  rects[i].y2 - rects[i].y1);
> + else
> + wl_surface_damage(sb->output->parent.surface,
> +   rects[i].x1,
> +   rects[i].y1,
> +   rects[i].x2 - rects[i].x1,
> +   rects[i].y2 - rects[i].y1);
> + }
>  
>   if (sb->output->frame)
>   pixman_region32_fini();
> @@ -1265,8 +1287,13 @@ input_set_cursor(struct wayland_input *input)
> image->hotspot_x, image->hotspot_y);
>  
>   wl_surface_attach(input->parent.cursor.surface, buffer, 0, 0);
> - wl_surface_damage(input->parent.cursor.surface, 0, 0,
> -   image->width, image->height);
> + if (input->backend->use_damage_buffer)
> + wl_surface_damage_buffer(input->parent.cursor.surface, 0, 0,
> +  image->width, image->height);
> + else
> + wl_surface_damage(input->parent.cursor.surface, 0, 0,
> +   image->width, image->height);
> +
>   wl_surface_commit(input->parent.cursor.surface);
>  }
>  
> @@ -1983,9 +2010,14 @@ registry_handle_global(void *data, struct wl_registry 
> *registry, uint32_t name,
>   struct wayland_backend *b = data;
>  
>   if (strcmp(interface, "wl_compositor") == 0) {
> + int cv = MIN(WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION, version);
> +
>   b->parent.compositor =
>   wl_registry_bind(registry, name,
> -  _compositor_interface, 1);
> +   

Re: [PATCH libinput] test: add tablet test for out-of-bounds motion coordinates

[Carlos Garnacho]

Hey :),

On Thu, Feb 11, 2016 at 8:31 AM, Peter Hutterer
 wrote:
> The newer Cintiqs have a minimum value of 400/400 advertised by the kernel but
> the actual sensor goes past the 0/0 origin. Test this, make sure that a value
> outside the boundaries generates negative mm values.
>
> Signed-off-by: Peter Hutterer 
> ---
> Mostly just documenting this and adding a test. The behaviour is that any
> out-of-bounds motion may provide negative coordinates, when transformed may
> provide something outside the [0, max-range].
>
> Now, this is documenting the API behaviour. For the Cintiq I think it might
> be better to put a device-specific quirk in to simply supress those events
> since we don't have anything outside of these boundaries. Jason, Carlos, any
> comments?

I agree, reporting those would mean that such tablets could move
"outside" the crtc they're mapped to, even if not too far around the
borders. Although that same thought comes from any other tablet that
happens to behave the same. Not that we can preemptively add quirks
for these, but we probably should too when we know of these.

Cheers,
  Carlos

>
>  doc/svg/tablet-out-of-bounds.svg | 271 
> +++
>  doc/tablet-support.dox   |  22 
>  src/libinput.h   |  12 ++
>  test/tablet.c|  58 +
>  4 files changed, 363 insertions(+)
>  create mode 100644 doc/svg/tablet-out-of-bounds.svg
>
> diff --git a/doc/svg/tablet-out-of-bounds.svg 
> b/doc/svg/tablet-out-of-bounds.svg
> new file mode 100644
> index 000..83e5021
> --- /dev/null
> +++ b/doc/svg/tablet-out-of-bounds.svg
> @@ -0,0 +1,271 @@
> +
> +
> +
> + +   xmlns:dc="http://purl.org/dc/elements/1.1/;
> +   xmlns:cc="http://creativecommons.org/ns#;
> +   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#;
> +   xmlns:svg="http://www.w3.org/2000/svg;
> +   xmlns="http://www.w3.org/2000/svg;
> +   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd;
> +   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape;
> +   width="134.12477mm"
> +   height="73.121971mm"
> +   viewBox="0 0 475.24525 259.0936"
> +   id="svg2"
> +   version="1.1"
> +   inkscape:version="0.91 r13725"
> +   sodipodi:docname="tablet-out-of-bounds.svg">
> +   + id="defs4" />
> +   + id="base"
> + pagecolor="#ff"
> + bordercolor="#66"
> + borderopacity="1.0"
> + inkscape:pageopacity="0.0"
> + inkscape:pageshadow="2"
> + inkscape:zoom="5.6"
> + inkscape:cx="105.43109"
> + inkscape:cy="265.46934"
> + inkscape:document-units="px"
> + inkscape:current-layer="layer1"
> + showgrid="false"
> + showguides="true"
> + inkscape:guide-bbox="true"
> + inkscape:window-width="1920"
> + inkscape:window-height="1136"
> + inkscape:window-x="0"
> + inkscape:window-y="27"
> + inkscape:window-maximized="1"
> + fit-margin-top="0"
> + fit-margin-left="0"
> + fit-margin-right="0"
> + fit-margin-bottom="0">
> + +   position="63.6133,240.91614"
> +   orientation="0,1"
> +   id="guide4164" />
> + +   position="61.08792,13.126743"
> +   orientation="0,1"
> +   id="guide4166" />
> +  
> +   + id="metadata7">
> +
> +   + rdf:about="">
> +image/svg+xml
> + +   rdf:resource="http://purl.org/dc/dcmitype/StillImage; />
> +
> +  
> +
> +  
> +   + inkscape:label="tablet"
> + inkscape:groupmode="layer"
> + id="layer1"
> + style="display:inline"
> + transform="translate(-139.42736,-156.36219)">
> + +   
> style="opacity:0.9202;fill:#4d4d4d;fill-opacity:1;stroke:#4d4d4d;stroke-width:0.9201867;stroke-linecap:round;stroke-miterlimit:4;stroke-dasharray:3.68074643,
>  0.92018661;stroke-dashoffset:0;stroke-opacity:1"
> +   id="rect4136"
> +   width="474.32504"
> +   height="258.1734"
> +   x="139.88745"
> +   y="156.82228"
> +   rx="5" />
> + +   y="175.42407"
> +   x="199.33878"
> +   height="226.52563"
> +   width="357.34042"
> +   id="rect4140"
> +   
> style="opacity:0.9202;fill:#a44d4d;fill-opacity:1;stroke:#4d4d4d;stroke-width:0.7483;stroke-linecap:round;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:1"
>  />
> + +   id="g7041"
> +   transform="translate(12,0)">
> +   + transform="matrix(0.53265351,0,0,0.53265351,92.308091,96.440418)"
> + id="g7023">
> + +   
> style="opacity:0.9202;fill:#4d4d4d;fill-opacity:1;stroke:#4d4d4d;stroke-width:0.9892;stroke-linecap:round;stroke-miterlimit:4;stroke-dasharray:3.956,
>  0.989;stroke-dashoffset:0;stroke-opacity:1"
> +   id="path4158"
> +   cx="135.61298"
> +   cy="287.06125"
> +   r="22.98097" />
> + +   cy="287.06125"
> +   cx="135.61298"
> +