Re: [PATCH v2 wayland 3/3] doc: generate doxygen html output from the scanner

[Bryce Harrington]

On Mon, Dec 07, 2015 at 02:41:58PM -0800, Bryce Harrington wrote:
> On Tue, Nov 17, 2015 at 10:17:23AM +1000, Peter Hutterer wrote:
> > This switches the scanner to generate doxygen-compatible tags for the
> > generated protocol headers, and hooks up the doxygen build to generate 
> > server
> > and client-side API documentation.
> > 
> > Each protocol is a separate doxygen @page, with each interface a @subpage.
> > Wayland only has one protocol, wayland-protocols will have these nested.
> > Each protocol page has a list of interfaces and the copyright and 
> > description
> > where available.
> > All interfaces are grouped by doxygen @defgroup and @ingroups and appear in
> > "Modules" in the generated output. Each interface subpage has the 
> > description
> > and a link to the actual API doc.
> > Function, struct and #defines are documented in doxygen style and associated
> > with the matching interface.
> > 
> > Note that pages and groups have fixed HTML file names and are directly
> > linkable/bookmark-able.
> > 
> > The @mainpage is a separate file that's included at build time. It doesn't
> > contain much other than links to where the interesting bits are. It's a 
> > static
> > file though that supports markdown, so we can extend it easily in the 
> > future.
> > 
> > For doxygen we need the new options EXTRACT_ALL and OPTIMIZE_OUTPUT_FOR_C so
> > it scans C code properly. EXTRACT_STATIC is needed since most of the 
> > protocol
> > hooks are static. WARN_IF_DOC_ERROR is required to silence the warnings when
> > some parameters are documented but others aren't.
> > 
> > Signed-off-by: Peter Hutterer 
> 
> LGTM,
> Reviewed-by: Bryce Harrington 
> 
> Anyone else care to comment on this?
> Else, this seems pretty low risk for me to go ahead and land.

Hi Peter,

Looks like we overlooked this one for the release, but it seems the
codebase has moved along a bit.  Would you mind respinning this against
the current tree, and I'll go ahead and land it.

Thanks,
Bryce

> > ---
> > Changes to v1:
> > - rebase to master (c7bada036db1b)
> > - add EXTRACT_STATIC doxygen command, otherwise we'll miss most of the
> >   actual functions in the protocol API.
> > 
> >  doc/doxygen/Makefile.am|  27 +-
> >  doc/doxygen/mainpage.dox   |  13 +++
> >  doc/doxygen/wayland.doxygen.in |   5 +-
> >  src/scanner.c  | 207 
> > +++--
> >  4 files changed, 177 insertions(+), 75 deletions(-)
> >  create mode 100644 doc/doxygen/mainpage.dox
> > 
> > diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am
> > index a8bb95f..e80c401 100644
> > --- a/doc/doxygen/Makefile.am
> > +++ b/doc/doxygen/Makefile.am
> > @@ -1,7 +1,11 @@
> >  
> >  .SUFFIXES = .gv .png .map
> >  
> > -noinst_DATA = xml/Client/index.xml xml/Server/index.xml
> > +noinst_DATA = \
> > +  xml/Client/index.xml \
> > +  xml/Server/index.xml \
> > +  html/Client/index.html \
> > +  html/Server/index.html
> >  dist_noinst_DATA = wayland.doxygen.in
> >  
> >  scanned_src_files_shared = \
> > @@ -27,6 +31,17 @@ scanned_src_files_man =  
> > \
> > $(top_srcdir)/src/wayland-client.h  \
> > $(top_srcdir)/src/wayland-client-core.h
> >  
> > +extra_doxygen = \
> > +   mainpage.dox
> > +
> > +extra_doxygen_Server = \
> > +   $(top_builddir)/protocol/wayland-server-protocol.h \
> > +   $(extra_doxygen)
> > +
> > +extra_doxygen_Client = \
> > +   $(top_builddir)/protocol/wayland-client-protocol.h \
> > +   $(extra_doxygen)
> > +
> >  diagramsdir := $(srcdir)/dot
> >  diagramssrc := $(wildcard $(diagramsdir)/*.gv)
> >  diagrams := $(patsubst $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.png))
> > @@ -38,7 +53,7 @@ diagram_maps := $(patsubst 
> > $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map))
> >  dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3" 
> > -printf "man/man3/%P\n")
> >  
> >  # Listing various directories that might need to be created.
> > -alldirs := xml xml/Client xml/Server man/man3
> > +alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server
> >  
> >  $(diagrams): $(diagramssrc)
> >  
> > @@ -51,6 +66,13 @@ xml/%/index.xml: $(top_srcdir)/src/scanner.c 
> > $(scanned_src_files_%) wayland.doxy
> >echo "INPUT= $(scanned_src_files_$*)"; \
> >) | $(DOXYGEN) -
> >  
> > +html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) 
> > $(diagram_maps) | html/%
> > +   $(AM_V_GEN)(cat wayland.doxygen; \
> > +  echo "GENERATE_HTML=YES"; \
> > +  echo "HTML_OUTPUT=html/$*"; \
> > +  echo "INPUT= $(scanned_src_files_$*) $(extra_doxygen_$*)"; \
> > +  ) | $(DOXYGEN) -
> > +
> >  man/man3/wl_display.3: $(top_srcdir)/src/scanner.c 
> > $(scanned_src_files_man) wayland.doxygen | man/man3
> > $(AM_V_GEN)(cat wayland.doxygen; \
> >  

Re: [PATCH 2/2] xdg-shell: clarify xdg_surface creation semantics regarding buffers

[Bryce Harrington]

On Wed, Dec 02, 2015 at 08:06:46PM -0500, Mike Blumenkrantz wrote:
> this change ensures that the client will set its initial state
> before performing any drawing, ensuring that there is no mismatch
> when creating a surface with a non-default state
> (eg. maximize, fullscreen, ...)
> 
> looking at the following event flows:
> 1) wl_surface.attach, wl_surface.commit, xdg_shell.get_xdg_surface
> 
> 2) wl_surface.attach, xdg_shell.get_xdg_surface, wl_surface.commit
> 
> 3) xdg_shell.get_xdg_surface, wl_surface.commit, xdg_surface.configure,
>wl_surface.attach, wl_surface.commit
> 
> only 3) is now valid, while 1) and 2) will trigger errors as a result
> of handling buffers prior to creating the xdg surface
> 
> Reviewed-by: Jasper St. Pierre 
> Signed-off-by: Mike Blumenkrantz 
> Signed-off-by: Jonas Ådahl 

Reviewed-by: Bryce Harrington 

> ---
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index 196c332..a03a615 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -147,14 +147,12 @@
>them, and associate metadata like title and app id.
>  
>The client must call wl_surface.commit on the corresponding wl_surface
> -  for the xdg_surface state to take effect. Prior to committing the new
> -  state, it can set up initial configuration, such as maximizing or 
> setting
> -  a window geometry.
> -
> -  Even without attaching a buffer the compositor must respond to initial
> -  committed configuration, for instance sending a configure event with
> -  expected window geometry if the client maximized its surface during
> -  initialization.
> +  for the xdg_surface state to take effect.
> +
> +  Creating an xdg_surface from a wl_surface which has a buffer attached 
> or
> +  committed is a client error, and any attempts by a client to attach or
> +  manipulate a buffer prior to the first xdg_surface.configure call must
> +  also be treated as errors.
>  
>For a surface to be mapped by the compositor the client must have
>committed both an xdg_surface state and a buffer.
> -- 
> 2.4.3
> 
> ___
> 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 1/2] simple-touch: Use damage_buffer if available

[Bryce Harrington]

On Fri, Feb 12, 2016 at 09:03:16AM +0200, Giulio Camuffo wrote:
> 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.

Yes, they can be used to test other compositors, but I would argue their
ultimate purpose is more educational oriented, and in service of that
aim, keeping the code concise is beneficial.

On the topic of testing, if you were in fact using this to test your
compositor implementation, and your compositor lacks the damage_buffer
feature, the error generated in that case would be a valid testing
result, I should think.

Bryce

> > 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] platform: explicitly cast the return value of weston_platform_get_egl_proc_address

[Bryce Harrington]

On Fri, Jan 29, 2016 at 05:02:15PM +0100, Matthias Treydte wrote:
> This allows the header to be consumed by C++ compilers, because C++ does
> away with C's implicit cast from (void*).
> ---
>  shared/platform.h | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

Tabbing looks off maybe but otherwise just adding a cast seems fine.

Reviewed-by: Bryce Harrington 
 
> diff --git a/shared/platform.h b/shared/platform.h
> index cf4ecc0..dd55008 100644
> --- a/shared/platform.h
> +++ b/shared/platform.h
> @@ -75,8 +75,9 @@ weston_platform_get_egl_display(EGLenum platform, void 
> *native_display,
>   static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
>  
>   if (!get_platform_display) {
> - get_platform_display = weston_platform_get_egl_proc_address(
> - "eglGetPlatformDisplayEXT");
> + get_platform_display = (PFNEGLGETPLATFORMDISPLAYEXTPROC)
> +weston_platform_get_egl_proc_address(
> +"eglGetPlatformDisplayEXT");
>   }
>  
>   if (get_platform_display)
> @@ -95,8 +96,9 @@ weston_platform_create_egl_surface(EGLDisplay dpy, 
> EGLConfig config,
>   create_platform_window = NULL;
>  
>   if (!create_platform_window) {
> - create_platform_window = weston_platform_get_egl_proc_address(
> - "eglCreatePlatformWindowSurfaceEXT");
> + create_platform_window = 
> (PFNEGLCREATEPLATFORMWINDOWSURFACEEXTPROC)
> +weston_platform_get_egl_proc_address(
> +"eglCreatePlatformWindowSurfaceEXT");
>   }
>  
>   if (create_platform_window)
> -- 
> 2.7.0
> 
> ___
> 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 wayland 2/2] shm: Defer wl_shm_pool_resize if a pool has external references

[Bryce Harrington]

On Tue, Feb 09, 2016 at 04:03:48PM -0600, Derek Foreman wrote:
> If a compositor is rendering in one thread while dispatching wayland
> events in another, a wl_shm_pool_resize() could change the memory
> mappings it's rendering from and cause a crash.
> 
> Now we defer wl_shm_pool_resize() if the compositor has references on a
> pool, and perform the actual resize when it drops those references.
> 
> Signed-off-by: Derek Foreman 

Reviewed-by: Bryce Harrington 

> ---
>  src/wayland-shm.c | 47 +++
>  1 file changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index 6fbf34b..6351259 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -56,6 +56,7 @@ struct wl_shm_pool {
>   int external_refcount;
>   char *data;
>   int32_t size;
> + int32_t new_size;
>  };
>  
>  struct wl_shm_buffer {
> @@ -74,12 +75,35 @@ struct wl_shm_sigbus_data {
>  };
>  
>  static void
> +shm_pool_finish_resize(struct wl_shm_pool *pool)
> +{
> + void *data;
> +
> + if (pool->size == pool->new_size)
> + return;
> +
> + data = mremap(pool->data, pool->size, pool->new_size, MREMAP_MAYMOVE);
> + if (data == MAP_FAILED) {
> + wl_resource_post_error(pool->resource,
> +WL_SHM_ERROR_INVALID_FD,
> +"failed mremap");
> + return;
> + }
> +
> + pool->data = data;
> + pool->size = pool->new_size;
> +}
> +
> +static void
>  shm_pool_unref(struct wl_shm_pool *pool, bool external)
>  {
> - if (external)
> + if (external) {
>   pool->external_refcount--;
> - else
> + if (pool->external_refcount == 0)
> + shm_pool_finish_resize(pool);
> + } else {
>   pool->internal_refcount--;
> + }
>  
>   if (pool->internal_refcount + pool->external_refcount)
>   return;
> @@ -202,7 +226,6 @@ shm_pool_resize(struct wl_client *client, struct 
> wl_resource *resource,
>   int32_t size)
>  {
>   struct wl_shm_pool *pool = wl_resource_get_user_data(resource);
> - void *data;
>  
>   if (size < pool->size) {
>   wl_resource_post_error(resource,
> @@ -211,16 +234,15 @@ shm_pool_resize(struct wl_client *client, struct 
> wl_resource *resource,
>   return;
>   }
>  
> - data = mremap(pool->data, pool->size, size, MREMAP_MAYMOVE);
> - if (data == MAP_FAILED) {
> - wl_resource_post_error(resource,
> -WL_SHM_ERROR_INVALID_FD,
> -"failed mremap");
> - return;
> - }
> + pool->new_size = size;
>  
> - pool->data = data;
> - pool->size = size;
> + /* If the compositor has taken references on this pool it
> +  * may be caching pointers into it. In that case we
> +  * defer the resize (which may move the entire mapping)
> +  * until the compositor finishes dereferencing the pool.
> +  */
> + if (pool->external_refcount == 0)
> + shm_pool_finish_resize(pool);
>  }
>  
>  struct wl_shm_pool_interface shm_pool_interface = {
> @@ -251,6 +273,7 @@ shm_create_pool(struct wl_client *client, struct 
> wl_resource *resource,
>   pool->internal_refcount = 1;
>   pool->external_refcount = 0;
>   pool->size = size;
> + pool->new_size = size;
>   pool->data = mmap(NULL, size,
> PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>   if (pool->data == MAP_FAILED) {
> -- 
> 2.7.0
> 
> ___
> 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 wayland 1/2] shm: Split pool reference counting into external and internal references

[Bryce Harrington]

On Tue, Feb 09, 2016 at 04:03:47PM -0600, Derek Foreman wrote:
> This is a preliminary step towards deferring shm resize operations until
> after the compositor has released all external references on a pool.
> 
> Signed-off-by: Derek Foreman 

Yep looks good.
Reviewed-by: Bryce Harrington 
> ---
>  src/wayland-shm.c | 31 +++
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index a4343a4..6fbf34b 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -52,7 +52,8 @@ static struct sigaction wl_shm_old_sigbus_action;
>  
>  struct wl_shm_pool {
>   struct wl_resource *resource;
> - int refcount;
> + int internal_refcount;
> + int external_refcount;
>   char *data;
>   int32_t size;
>  };
> @@ -73,10 +74,14 @@ struct wl_shm_sigbus_data {
>  };
>  
>  static void
> -shm_pool_unref(struct wl_shm_pool *pool)
> +shm_pool_unref(struct wl_shm_pool *pool, bool external)
>  {
> - pool->refcount--;
> - if (pool->refcount)
> + if (external)
> + pool->external_refcount--;
> + else
> + pool->internal_refcount--;
> +
> + if (pool->internal_refcount + pool->external_refcount)
>   return;
>  
>   munmap(pool->data, pool->size);
> @@ -89,7 +94,7 @@ destroy_buffer(struct wl_resource *resource)
>   struct wl_shm_buffer *buffer = wl_resource_get_user_data(resource);
>  
>   if (buffer->pool)
> - shm_pool_unref(buffer->pool);
> + shm_pool_unref(buffer->pool, false);
>   free(buffer);
>  }
>  
> @@ -162,13 +167,13 @@ shm_pool_create_buffer(struct wl_client *client, struct 
> wl_resource *resource,
>   buffer->stride = stride;
>   buffer->offset = offset;
>   buffer->pool = pool;
> - pool->refcount++;
> + pool->internal_refcount++;
>  
>   buffer->resource =
>   wl_resource_create(client, _buffer_interface, 1, id);
>   if (buffer->resource == NULL) {
>   wl_client_post_no_memory(client);
> - shm_pool_unref(pool);
> + shm_pool_unref(pool, false);
>   free(buffer);
>   return;
>   }
> @@ -183,7 +188,7 @@ destroy_pool(struct wl_resource *resource)
>  {
>   struct wl_shm_pool *pool = wl_resource_get_user_data(resource);
>  
> - shm_pool_unref(pool);
> + shm_pool_unref(pool, false);
>  }
>  
>  static void
> @@ -243,7 +248,8 @@ shm_create_pool(struct wl_client *client, struct 
> wl_resource *resource,
>   goto err_free;
>   }
>  
> - pool->refcount = 1;
> + pool->internal_refcount = 1;
> + pool->external_refcount = 0;
>   pool->size = size;
>   pool->data = mmap(NULL, size,
> PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> @@ -396,9 +402,10 @@ wl_shm_buffer_get_height(struct wl_shm_buffer *buffer)
>  WL_EXPORT struct wl_shm_pool *
>  wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer)
>  {
> - assert(buffer->pool->refcount);
> + assert(buffer->pool->internal_refcount +
> +buffer->pool->external_refcount);
>  
> - buffer->pool->refcount++;
> + buffer->pool->external_refcount++;
>   return buffer->pool;
>  }
>  
> @@ -418,7 +425,7 @@ wl_shm_buffer_ref_pool(struct wl_shm_buffer *buffer)
>  WL_EXPORT void
>  wl_shm_pool_unref(struct wl_shm_pool *pool)
>  {
> - shm_pool_unref(pool);
> + shm_pool_unref(pool, true);
>  }
>  
>  static void
> -- 
> 2.7.0
> 
> ___
> 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 wayland] shm: Log a warning if a shm buffer address is requested when it may be invalid

[Bryce Harrington]

On Tue, Feb 09, 2016 at 04:29:49PM -0600, Derek Foreman wrote:
> If wl_shm_buffer_get_data() is called on a shm pool that has an external
> reference and a pending resize, then the buffer may be outside the pool's
> current mapping.
> 
> Log a warning if this happens.
> 
> Signed-off-by: Derek Foreman 

Part of me wonders if this should be treated as an error situation, but
adding a log message doesn't prevent that and may help in diagnosing
issues, so:

Reviewed-by: Bryce Harrington 

> ---
>  src/wayland-shm.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> index 6351259..7c67575 100644
> --- a/src/wayland-shm.c
> +++ b/src/wayland-shm.c
> @@ -387,6 +387,11 @@ wl_shm_buffer_get_data(struct wl_shm_buffer *buffer)
>   if (!buffer->pool)
>   return NULL;
>  
> + if (buffer->pool->external_refcount &&
> + (buffer->pool->size != buffer->pool->new_size))
> + wl_log("Buffer address requested when its parent pool "
> +"has an external reference and a deferred resize "
> +"pending.\n");
>   return buffer->pool->data + buffer->offset;
>  }
>  
> -- 
> 2.7.0
> 
> ___
> 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 wayland] server: add listener API for new clients

[Bryce Harrington]

On Thu, Feb 11, 2016 at 09:20:00PM +0900, nic...@nicesj.com wrote:
> 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 

Hi Sung-jae,

I'm a bit confused here; this patch appears to simply rename an existing
API call and move its docs from the .h to the .c.  Is there more code
that's missing, or am I misunderstanding the purpose of this patch?

Bryce
 
> 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
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 1/2] configure: Make WebP support togglable, and improve its error message.

[Bryce Harrington]

On Tue, Feb 16, 2016 at 10:26:25PM -0800, Bryce Harrington wrote:
> On Tue, Feb 16, 2016 at 01:57:51AM +, Emmanuel Gil Peyrot wrote:
> > The current way was enabling WebP support whenever libwebp was found,
> > giving no way to the user to disable it if they had the library
> > installed but didn’t want to link against it.  This adds a
> > --without-webp configure option to never link against it, and a
> > --with-webp one to fail the build if it isn’t found, the default being
> > to use it if it is present.
> > 
> > Additionally, we now tell the user when WebP support has been disabled
> > and they try to load a WebP file.
> > 
> > Signed-off-by: Emmanuel Gil Peyrot 
> 
> Reviewed-by: Bryce Harrington 
> 
> This looks fine, only functionality that is lost for webp is reading of
> riff files.

Thanks, pushed:
To ssh://git.freedesktop.org/git/wayland/weston
   e1719c7..58b7a15  master -> master

> > ---
> >  configure.ac  | 12 ++--
> >  shared/image-loader.c | 11 +--
> >  2 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index c32ddc9..878b30a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -293,9 +293,17 @@ fi
> >  
> >  PKG_CHECK_MODULES(PIXMAN, [pixman-1])
> >  PKG_CHECK_MODULES(PNG, [libpng])
> > -PKG_CHECK_MODULES(WEBP, [libwebp], [have_webp=yes], [have_webp=no])
> > +
> > +AC_ARG_WITH([webp],
> > +AS_HELP_STRING([--without-webp],
> > +   [Use libwebp for WebP decoding support 
> > [default=auto]]))
> > +AS_IF([test "x$with_webp" != "xno"],
> > +  [PKG_CHECK_MODULES(WEBP, [libwebp], [have_webp=yes], 
> > [have_webp=no])],
> > +  [have_webp=no])
> >  AS_IF([test "x$have_webp" = "xyes"],
> > -  [AC_DEFINE([HAVE_WEBP], [1], [Have webp])])
> > +  [AC_DEFINE([HAVE_WEBP], [1], [Have webp])],
> > +  [AS_IF([test "x$with_webp" = "xyes"],
> > + [AC_MSG_ERROR([WebP support explicitly requested, but libwebp 
> > couldn't be found])])])
> >  
> >  AC_ARG_ENABLE(vaapi-recorder, [  --enable-vaapi-recorder],,
> >   enable_vaapi_recorder=auto)
> > diff --git a/shared/image-loader.c b/shared/image-loader.c
> > index ec75bd4..050f067 100644
> > --- a/shared/image-loader.c
> > +++ b/shared/image-loader.c
> > @@ -352,6 +352,15 @@ load_webp(FILE *fp)
> > config.output.u.RGBA.stride);
> >  }
> >  
> > +#else
> > +
> > +static pixman_image_t *
> > +load_webp(FILE *fp)
> > +{
> > +   fprintf(stderr, "WebP support disabled at compile-time\n");
> > +   return NULL;
> > +}
> > +
> >  #endif
> >  
> >  
> > @@ -364,9 +373,7 @@ struct image_loader {
> >  static const struct image_loader loaders[] = {
> > { { 0x89, 'P', 'N', 'G' }, 4, load_png },
> > { { 0xff, 0xd8 }, 2, load_jpeg },
> > -#ifdef HAVE_WEBP
> > { { 'R', 'I', 'F', 'F' }, 4, load_webp }
> > -#endif
> >  };
> >  
> >  pixman_image_t *
> > -- 
> > 2.7.1
> > 
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] libinput-device: Remove unnecessary function call

[Bryce Harrington]

On Fri, Feb 19, 2016 at 11:07:00AM -0500, Chris Michael wrote:
> When we handle pointer button events, we already retrieve the button
> state at the top of this function, so there is no real need to call
> the same function again as we can just reuse the 'button_state'
> variable that we have above.
> 
> Signed-off-by: Chris Michael 

Reviewed-by: Bryce Harrington 

pushed:
To ssh://git.freedesktop.org/git/wayland/weston
   f989c38..e1719c7  master -> master

> ---
>  src/libinput-device.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libinput-device.c b/src/libinput-device.c
> index 99b2916..78b0ac9 100644
> --- a/src/libinput-device.c
> +++ b/src/libinput-device.c
> @@ -154,7 +154,8 @@ handle_pointer_button(struct libinput_device 
> *libinput_device,
>   notify_button(device->seat,
> libinput_event_pointer_get_time(pointer_event),
> libinput_event_pointer_get_button(pointer_event),
> -   libinput_event_pointer_get_button_state(pointer_event));
> +  button_state);
> +
>   return true;
>  }
>  
> -- 
> 2.7.0
> 
> ___
> 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] libinput-device: Remove unnecessary function call

[Bryce Harrington]

On Fri, Feb 19, 2016 at 11:07:00AM -0500, Chris Michael wrote:
> When we handle pointer button events, we already retrieve the button
> state at the top of this function, so there is no real need to call
> the same function again as we can just reuse the 'button_state'
> variable that we have above.
> 
> Signed-off-by: Chris Michael 
Reviewed-by: Bryce Harrington 

> ---
>  src/libinput-device.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libinput-device.c b/src/libinput-device.c
> index 99b2916..78b0ac9 100644
> --- a/src/libinput-device.c
> +++ b/src/libinput-device.c
> @@ -154,7 +154,8 @@ handle_pointer_button(struct libinput_device 
> *libinput_device,
>   notify_button(device->seat,
> libinput_event_pointer_get_time(pointer_event),
> libinput_event_pointer_get_button(pointer_event),
> -   libinput_event_pointer_get_button_state(pointer_event));
> +  button_state);
> +
>   return true;
>  }
>  
> -- 
> 2.7.0
> 
> ___
> 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

[RFC wayland-protocols v4] Add Primary Selection Protocol Version 1

[Carlos Garnacho]

From: Lyude 

This primary selection is similar in spirit to the eponimous
in X11, allowing a quick "select text + middle click" shortcut
to copying and pasting.

It's otherwise very similar to it wayland counterpart, and
explicitly made consistent with it.

Signed-off-by: Lyude 
Signed-off-by: Carlos Garnacho 
---
After having talked with Lyude, I'll be trying to move this ahead.

Changes since v3:
- Added a rather verbose protocol description, including a
  high-level overview of the workings.
- Made event emission 1:1 with wayland core protocol selections,
  wp_primary_selection_offer.offer events are now expected to be
  emitted between wp_primary_data_device.data_offer and 
  wp_primary_data_device.selection
- Improved wording here and there.
- Added serial argument to wp_primary_data_offer.receive that can be
  used to match against recent events.


 Makefile.am|   1 +
 unstable/primary-selection/README  |   4 +
 .../primary-selection-unstable-v1.xml  | 229 +
 3 files changed, 234 insertions(+)
 create mode 100644 unstable/primary-selection/README
 create mode 100644 unstable/primary-selection/primary-selection-unstable-v1.xml

diff --git a/Makefile.am b/Makefile.am
index 57d0023..eefa20f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,6 +7,7 @@ unstable_protocols =
\
unstable/xdg-shell/xdg-shell-unstable-v5.xml
\
unstable/relative-pointer/relative-pointer-unstable-v1.xml  
\
unstable/pointer-constraints/pointer-constraints-unstable-v1.xml
\
+   unstable/primary-selection/primary-selection-unstable-v1.xml
\
$(NULL)
 
 stable_protocols = 
\
diff --git a/unstable/primary-selection/README 
b/unstable/primary-selection/README
new file mode 100644
index 000..6d8c0c6
--- /dev/null
+++ b/unstable/primary-selection/README
@@ -0,0 +1,4 @@
+Primary selection protocol
+
+Maintainers:
+Lyude 
diff --git a/unstable/primary-selection/primary-selection-unstable-v1.xml 
b/unstable/primary-selection/primary-selection-unstable-v1.xml
new file mode 100644
index 000..a3618d5
--- /dev/null
+++ b/unstable/primary-selection/primary-selection-unstable-v1.xml
@@ -0,0 +1,229 @@
+
+
+  
+Copyright © 2015 Red Hat
+
+Permission is hereby granted, free of charge, to any person obtaining a
+copy of this software and associated documentation files (the "Software"),
+to deal in the Software without restriction, including without limitation
+the rights to use, copy, modify, merge, publish, distribute, sublicense,
+and/or sell copies of the Software, and to permit persons to whom the
+Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice (including the next
+paragraph) shall be included in all copies or substantial portions of the
+Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+DEALINGS IN THE SOFTWARE.
+  
+
+  
+This protocol provides the ability to have a primary selection device to
+match that of the X server. This primary selection is a shortcut to the
+common clipboard selection, where text just needs to be selected in order
+to allow copying it elsewhere. The de facto way to perform this action
+is the middle mouse button, although it is not limited to this one.
+
+Clients wishing to honor primary selection should create a primary
+selection source and set it as the selection through
+wp_primary_selection_device.set_selection whenever the text selection
+changes. In order to minimize calls in pointer-driven text selection,
+it should happen only once after the operation finished. Similarly,
+a NULL source should be set when text is unselected.
+
+wp_primary_selection_offer objects are first announced through the
+wp_primary_selection_device.data_offer event. Immediately after this event,
+the primary data offer will emit wp_primary_selection_offer.offer events
+to let know of the mime types being offered.
+
+When the primary selection changes, the client with the keyboard focus
+will receive wp_primary_selection_device.selection events. Only the client
+with the keyboard focus will receive such events with a non-NULL
+wp_primary_selection_offer. Across 

Re: [RFC libinput] Add dial input device support

[Peter Hutterer]

On Fri, Feb 19, 2016 at 08:03:52PM +0530, PrasannaKumar Muralidharan wrote:
> > right. the approach we used for this was to have an API that exports the
> > value in degrees and an API for get_value_discrete() or similarly named.
> > Look up the libinput_event_pointer*axis* API to get an idea.
> 
> For several devices (example dimmers used for controlling lamps) value
> in degrees is not useful. Exported value depends on device.

remember that libinput is a low-level library and doesn't know about how a
specific device is being used by the layers above it. Hence a value in
degrees isn't any less useful than a value in miscellaneous units which is
what the _discrete() API bit is for. If the caller cares about units, it
uses discrete steps, if it cares about number of rotations, it uses
degrees.

The only requirement in libinput is that you have a units -> physical
degrees mapping which is something we used the hwdb for (in the case of
mouse wheels anyway).

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

[PATCH] refactor x11-backend configuration API

[Benoit Gschwind]

Use a "well" defined structure to configure x11-backend and move configuration
file parsing inside the weston's compositor code.
---
 Makefile.am  |   1 +
 src/compositor-x11.c | 160 ++---
 src/compositor-x11.h |  61 +++
 src/compositor.h |   1 +
 src/main.c   | 164 ++-
 5 files changed, 283 insertions(+), 104 deletions(-)
 create mode 100644 src/compositor-x11.h

diff --git a/Makefile.am b/Makefile.am
index 623621d..7acef6a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -72,6 +72,7 @@ weston_SOURCES =  \
src/log.c   \
src/compositor.c\
src/compositor.h\
+   src/compositor-x11.h\
src/input.c \
src/data-device.c   \
src/screenshooter.c \
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 13a5d73..3966faf 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -50,6 +50,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor-x11.h"
 #include "gl-renderer.h"
 #include "pixman-renderer.h"
 #include "shared/config-parser.h"
@@ -60,11 +61,6 @@
 
 #define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
 
-static int option_width;
-static int option_height;
-static int option_scale;
-static int option_count;
-
 struct x11_backend {
struct weston_backendbase;
struct weston_compositor *compositor;
@@ -105,6 +101,7 @@ struct x11_backend {
xcb_atom_t   cardinal;
xcb_atom_t   xkb_names;
} atom;
+
 };
 
 struct x11_output {
@@ -130,6 +127,25 @@ struct window_delete_data {
 
 struct gl_renderer_interface *gl_renderer;
 
+static void
+weston_x11_backend_config_outputs_clear(
+   struct weston_x11_backend_config * ths) {
+   struct weston_x11_backend_output_config * pos;
+   struct weston_x11_backend_output_config * tmp;
+   wl_list_for_each_safe(pos, tmp, >outputs, link) {
+   wl_list_remove(>link);
+   free(pos->name);
+   free(pos);
+   }
+}
+
+static void
+weston_x11_backend_config_destroy(
+   struct weston_x11_backend_config * ths) {
+   weston_x11_backend_config_outputs_clear(ths);
+   free(ths);
+}
+
 static xcb_screen_t *
 x11_compositor_get_default_screen(struct x11_backend *b)
 {
@@ -1548,6 +1564,7 @@ x11_destroy(struct weston_compositor *ec)
weston_compositor_shutdown(ec); /* destroys outputs, too */
 
XCloseDisplay(backend->dpy);
+
free(backend);
 }
 
@@ -1568,20 +1585,11 @@ init_gl_renderer(struct x11_backend *b)
 }
 static struct x11_backend *
 x11_backend_create(struct weston_compositor *compositor,
-  int fullscreen,
-  int no_input,
-  int use_pixman,
-  int *argc, char *argv[],
-  struct weston_config *config)
+  struct weston_x11_backend_config *config)
 {
struct x11_backend *b;
struct x11_output *output;
-   struct weston_config_section *section;
-   int i, x = 0, output_count = 0;
-   int width, height, scale, count;
-   const char *section_name;
-   char *name, *t, *mode;
-   uint32_t transform;
+   int x = 0;
 
weston_log("initializing x11 backend\n");
 
@@ -1609,13 +1617,13 @@ x11_backend_create(struct weston_compositor *compositor,
x11_backend_get_resources(b);
x11_backend_get_wm_info(b);
 
-   if (!b->has_net_wm_state_fullscreen && fullscreen) {
+   if (!b->has_net_wm_state_fullscreen && config->fullscreen) {
weston_log("Can not fullscreen without window manager support"
   "(need _NET_WM_STATE_FULLSCREEN)\n");
-   fullscreen = 0;
+   config->fullscreen = 0;
}
 
-   b->use_pixman = use_pixman;
+   b->use_pixman = config->use_pixman;
if (b->use_pixman) {
if (pixman_renderer_init(compositor) < 0) {
weston_log("Failed to initialize pixman renderer for 
X11 backend\n");
@@ -1625,84 +1633,53 @@ x11_backend_create(struct weston_compositor *compositor,
else if (init_gl_renderer(b) < 0) {
goto err_xdisplay;
}
-   weston_log("Using %s renderer\n", use_pixman ? "pixman" : "gl");
+   weston_log("Using %s renderer\n", config->use_pixman ? "pixman" : "gl");
 
b->base.destroy = x11_destroy;
b->base.restore = x11_restore;
 
-   if (x11_input_create(b, no_input) < 0) {
+   if (x11_input_create(b, config->no_input) < 0) {
weston_log("Failed to create X11 input\n");
goto 

Re: [PATCH v4 wayland-protocols] text: Create second version of text input protocol

[Jan Arne Petersen]

Hi,

On 18/02/16 20:36, Rui Tiago Cação Matos wrote:
> On Wed, Feb 17, 2016 at 6:13 AM, Jan Arne Petersen  wrote:
 +
> ...
 +  
 +  
 +  
 +  
>>>
>>> These arguments could, and perhaps should, all be type="uint"
>>
>> Yes I guess for width/height.
> 
> As Jonas pointed out, just leave them as "int". One thing I now
> noticed is that the text says the coordinates are relative to the
> surface but I think it's better to be explicit and use the main
> wayland protocol terms "surface local coordinates" i.e. the
> coordinates should take the surface scale into account.

Ok I will do.

 +
> ...
>> It is used to be able to interact with pre-edit text (move the cursor
>> within pre-edit text or touch-and-hold to get some more options).
> 
> Aren't those client side operations? This request just doesn't seem
> right in this protocol. Perhaps it belongs in a different protocol for
> specific environments/devices? The text protocol should be strictly
> about text input IMO.

Yes we can just leave that out for now. I will remove it from the protocol.

 +
> ...
>> Yes we have the text focus in cases there is no keyboard available
>> (currently there is no way to have a focus then).
>>
>> It does not make sense to have text and keyboard focus on different
>> surfaces at least not for the same seat. I do not see any case where you
>> would want hardware keyboard input and virtual keyboard input going into
>> different widgets/surfaces at the same time.
> 
> Perhaps it doesn't make sense but I don't see why we need to tie them
> up here, just like wl_pointer's focus isn't necessarily tied to
> wl_keyboard's focus in the main protocol.

Well pointer focus and keyboard focus are usually not in sync (pointer
focus is on the surface where the pointer currently is over, keyboard
focus where the keyboard input happens).

I do not know any toolkit supporting an extra text input focus beside
the keyboard focus. The current problem is the case when there is no
keyboard/wl_keyboard present in a wl_seat and then they do not have a
way to get a keyboard/text-input focus at all.

When we specify it is always the same as the keyboard focus, the
toolkits can easily use it for there text input/keyboard focus.

If you really want an extra independent text input focus that should
probably just be in an extra seat, which is of course also allowed from
this protocol.

> In practice, sure, compositors will likely keep them all synced. In
> any case, the sentence
> 
> "When the seat has one or more keyboards the text-input focus follows
> the keyboard focus."
> 
> can't be worded like that because, again, even if a seat doesn't have
> a hardware keyboard, the wl_keyboard object (and its focus) may exist
> anyway. At least, it needs to be changed to

That is coming from wl_seat documentation of the keyboard capability:
"The seat has one or more keyboards", I can of course also write "When
the seat has the keyboard capability the seat's text-input ..."

> "The seat's text-input focus follows the keyboard focus."

That is even better.

> if we indeed want to specify that they must be tied.

Yes I really want to specify that, so clients/toolkits can rely on that

 +
 +
> ...
>> wl_keyboard currently does not allow to send synthetic key events when
>> you want to just send keysyms.
> 
> Why do we need to send keysyms though? Isn't it enough to commit
> (single character) strings?

There is also things like backspace, cursor keys and escape (or
something like ctrl-c for a terminal) which you want to input in a
virtual keyboard and which can not be represented as text commits.

Thanks for the feedback. I will prepare an updated v5 soon.
Jan Arne

-- 
Jan Arne Petersen | jan.peter...@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] libinput-device: Remove unnecessary function call

[Chris Michael]

When we handle pointer button events, we already retrieve the button
state at the top of this function, so there is no real need to call
the same function again as we can just reuse the 'button_state'
variable that we have above.

Signed-off-by: Chris Michael 
---
 src/libinput-device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libinput-device.c b/src/libinput-device.c
index 99b2916..78b0ac9 100644
--- a/src/libinput-device.c
+++ b/src/libinput-device.c
@@ -154,7 +154,8 @@ handle_pointer_button(struct libinput_device 
*libinput_device,
notify_button(device->seat,
  libinput_event_pointer_get_time(pointer_event),
  libinput_event_pointer_get_button(pointer_event),
- libinput_event_pointer_get_button_state(pointer_event));
+  button_state);
+
return true;
 }
 
-- 
2.7.0

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

Re: [RFC libinput] Add dial input device support

[PrasannaKumar Muralidharan]

> right. the approach we used for this was to have an API that exports the
> value in degrees and an API for get_value_discrete() or similarly named.
> Look up the libinput_event_pointer*axis* API to get an idea.

For several devices (example dimmers used for controlling lamps) value
in degrees is not useful. Exported value depends on device.

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

Re: [RFC libinput] Add dial input device support

[PrasannaKumar Muralidharan]

Hi Andreas,

> Instead of car steering wheels I would rather mention rotaty knobs that are
> common in car infotainment  systems to navigate through menues. Those are
> relative - you never get absolute angles just rotation ticks.
> But is that the type of device you had in mind? Those devices usually have a
> bunch of further buttons.. so buttonset sounds like a good generalization
> for those too.

Yes buttonset interface suits well for dial.

As of now kernel provides REL_DIAL. As of now I am thinking about
rotatory knobs or something like PowerMate device. Other use case is
dimmers used for light / lamps which return absolute value, has some
maximum value. I did not look much to find absolute dial interface
from kernel, so skipping it for now. I guess ABS_WHEEL Is not suitable
for this type of devices.

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

Re: [PATCH weston v2] hmi-controller: remove duplicate commit_changes in random mode

[Pekka Paalanen]

On Thu, 18 Feb 2016 08:48:40 +0900
Wataru Natsume  wrote:

> From: Wataru Natsume 
> 
> Previous code cleaned up surfaces in layer once and then added
> surfaces to a layer in random. In this flow, two commitchanges are
> required.
> 
> Signed-off-by: Nobuhiko Tanibata 
> [wataru_nats...@xddp.denso.co.jp: Removes unnecessary check]
> Signed-off-by: Wataru Natsume 
> 
> ---
> Changes from v1 - Removes unnecessary check if the surface is on a layer.
> 
>  ivi-shell/hmi-controller.c |9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
> index 8da3d3c..ace6555 100644
> --- a/ivi-shell/hmi-controller.c
> +++ b/ivi-shell/hmi-controller.c
> @@ -424,18 +424,9 @@ mode_random_replace(struct hmi_controller *hmi_ctrl,
>  
>   wl_list_for_each(application_layer, layer_list, link) {
>   layers[layer_idx] = application_layer;
> - 
> ivi_layout_interface->layer_set_render_order(layers[layer_idx]->ivilayer,
> - NULL, 0);
>   layer_idx++;
>   }
>  
> - /*
> -  * This commit change is needed because ivisurface can not belongs to 
> several layers
> -  * at the same time. So ivisurfaces shall be removed from layers once 
> and then set them
> -  * to layers randomly.
> -  */
> - ivi_layout_interface->commit_changes();
> -
>   for (i = 0; i < surface_length; i++) {
>   ivisurf = pp_surface[i];
>  

Hi Natsume-san,

this looks fine at first, but when testing it, mode_random_replace()
will trigger one "ivi_layout_layer_add_surface: addsurf is already
available" warning per existing surface.

ivi_layout_layer_add_surface() is checking if the surface is already
(current, not the pending state) on the given layer. This is likely
because in a previously intended future a surface might be in multiple
layers, and adding it multiple times to the same layer is considered a
mistake (given how surface positioning works in this ivi-layout API
design, that is understandable).

Maybe we should also just remove that check from
ivi_layout_layer_add_surface()? I don't see any value from it in the
current code base. If Emre adds views as a tying object in the
ivi-layout API, this code will get rewritten anyway.

Apart from the harmless log spew, this patch is:
Reviewed-by: Pekka Paalanen 

If you want to make a patch to remove the warning, I can push the both
patches at the same time.


Thanks,
pq


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

Re: [PATCH] server: Fix shm_create_pool size fail path fd leak

[xerpi]

I was just reading the source when I found it (no valgrind involved). So
as wl_connection_destroy() already takes care of that, my patch is
pointless.

My final idea would be to add support to fds that have the F_SEAL_SHRINK
seal set (see memfd_create(2) and fcntl(2)), so if a client creates
shm_pool with the shrink seal set, and the fd size (fstat(2)) is already >=
size arg passed to wl_shm_create_pool, we no longer would need to set
SIGBUS handlers when the compositor needs to access that shm memory.
Does this make any sense/would be a good idea?

2016-02-19 2:49 GMT+01:00 Jonas Ådahl :

> On Thu, Feb 18, 2016 at 11:59:29PM +0100, Sergi Granell wrote:
> > If the client passed a size <= 0 to shm_create_pool, it would
> > go to err_free, which wouldn't close the fd, and thus leave it opened.
> >
> > We can also move the size check before the struct wl_shm_pool
> > malloc, so in case the client passes a wrong size, it won't
> > do an unnecessary malloc and then free.
>
> Did you detect this using valgrind or something like that? Because IIRC,
> we collect all transmitted file descriptors and close them when the
> client is destroyed (see wl_connection_destory()). Since we post an
> error, the client will eventually be destroyed, so we shouldn't leak
> the fd here already.
>
>
> Jonas
>
> > ---
> >  src/wayland-shm.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> > index a4343a4..81bf657 100644
> > --- a/src/wayland-shm.c
> > +++ b/src/wayland-shm.c
> > @@ -230,17 +230,17 @@ shm_create_pool(struct wl_client *client, struct
> wl_resource *resource,
> >  {
> >   struct wl_shm_pool *pool;
> >
> > - pool = malloc(sizeof *pool);
> > - if (pool == NULL) {
> > - wl_client_post_no_memory(client);
> > - goto err_close;
> > - }
> > -
> >   if (size <= 0) {
> >   wl_resource_post_error(resource,
> >  WL_SHM_ERROR_INVALID_STRIDE,
> >  "invalid size (%d)", size);
> > - goto err_free;
> > + goto err_close;
> > + }
> > +
> > + pool = malloc(sizeof *pool);
> > + if (pool == NULL) {
> > + wl_client_post_no_memory(client);
> > + goto err_close;
> >   }
> >
> >   pool->refcount = 1;
> > @@ -251,7 +251,7 @@ shm_create_pool(struct wl_client *client, struct
> wl_resource *resource,
> >   wl_resource_post_error(resource,
> >  WL_SHM_ERROR_INVALID_FD,
> >  "failed mmap fd %d", fd);
> > - goto err_close;
> > + goto err_free;
> >   }
> >   close(fd);
> >
> > @@ -270,10 +270,10 @@ shm_create_pool(struct wl_client *client, struct
> wl_resource *resource,
> >
> >   return;
> >
> > -err_close:
> > - close(fd);
> >  err_free:
> >   free(pool);
> > +err_close:
> > + close(fd);
> >  }
> >
> >  static const struct wl_shm_interface shm_interface = {
> > --
> > 2.7.1
> >
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v2] server: validate resource versions at creation time

[Pekka Paalanen]

On Tue, 16 Feb 2016 22:08:50 -0800
Bryce Harrington  wrote:

> On Fri, Jan 15, 2016 at 10:06:48AM -0600, Derek Foreman wrote:
> > We shouldn't ever create a resource with version less than 1 or
> > greater than the interface version.
> > 
> > Reviewed-by: Marek Chalupa 
> > Signed-off-by: Derek Foreman 
> > ---
> > Changes since v1:
> > Fix silly typo - [0, %d] is now [1, %d]  
> 
> Reviewed-by: Bryce Harrington 
> 
> And pushed:
> To ssh://git.freedesktop.org/git/wayland/wayland
>6801d2d..88ff135  master -> master
> 
> 
> >  src/wayland-server.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > index 3a7d79d..08864eb 100644
> > --- a/src/wayland-server.c
> > +++ b/src/wayland-server.c
> > @@ -1349,6 +1349,13 @@ wl_resource_create(struct wl_client *client,
> >  {
> > struct wl_resource *resource;
> >  
> > +   if (version < 1 || version > interface->version) {
> > +   wl_log("wl_resource_create: invalid resource version %d "
> > +  "for interface '%s' - must be in range [1, %d]\n",
> > +  version, interface->name, interface->version);
> > +   return NULL;
> > +   }
> > +
> > resource = malloc(sizeof *resource);
> > if (resource == NULL)
> > return NULL;
> > -- 
> > 2.7.0.rc3

Hi,

I'm notifying the list that this patch got rightly reverted:
https://cgit.freedesktop.org/wayland/wayland/commit/?id=3a2553ff013a3661ec044917ccde4a74fe152dc9

However, I don't think creating a wl_resource with version < 1 is legal
anyway, so that could come back.


Thanks,
pq


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