Re: [PATCH weston 3/4] evdev: Fix assertion error for unplugged output with paired touchscreen

[nerdopolis]

On Thursday, April 24, 2014 03:11:16 PM Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira 
> 
> If the output a touchscreen is paired to is unplugged, events coming
> from it should be ignored. Commit 17bccaed introduced logic for that
> in evdev_flush_pending_damage(). However, the break statements it
> introduced would cause the assertion after the switch statement to
> fail.
> 
> That function has the odd behavior that goto's are used to skip the
> assertion after the switch statement and jump to the hunk of code that
> marks the event as processed. Only in the case where the event type has
> an invalid value the assertion should trigger. So this patch fixes the
> problem by moving the assertion into the default case of the switch
> and replacing the goto statements with break ones.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=73950
> ---
>  src/evdev.c | 19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 9d97c87..ff951d3 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -100,7 +100,7 @@ evdev_flush_pending_event(struct evdev_device *device, 
> uint32_t time)
>   notify_motion(master, time, device->rel.dx, device->rel.dy);
>   device->rel.dx = 0;
>   device->rel.dy = 0;
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_MT_DOWN:
>   if (device->output == NULL)
>   break;
> @@ -113,7 +113,7 @@ evdev_flush_pending_event(struct evdev_device *device, 
> uint32_t time)
>   master->slot_map |= 1 << seat_slot;
>  
>   notify_touch(master, time, seat_slot, x, y, WL_TOUCH_DOWN);
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_MT_MOTION:
>   if (device->output == NULL)
>   break;
> @@ -123,12 +123,12 @@ evdev_flush_pending_event(struct evdev_device *device, 
> uint32_t time)
>  &x, &y);
>   seat_slot = device->mt.slots[slot].seat_slot;
>   notify_touch(master, time, seat_slot, x, y, WL_TOUCH_MOTION);
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_MT_UP:
>   seat_slot = device->mt.slots[slot].seat_slot;
>   master->slot_map &= ~(1 << seat_slot);
>   notify_touch(master, time, seat_slot, 0, 0, WL_TOUCH_UP);
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_TOUCH_DOWN:
>   if (device->output == NULL)
>   break;
> @@ -141,7 +141,7 @@ evdev_flush_pending_event(struct evdev_device *device, 
> uint32_t time)
>   device->abs.seat_slot = seat_slot;
>   master->slot_map |= 1 << seat_slot;
>   notify_touch(master, time, seat_slot, x, y, WL_TOUCH_DOWN);
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_MOTION:
>   if (device->output == NULL)
>   break;
> @@ -156,17 +156,16 @@ evdev_flush_pending_event(struct evdev_device *device, 
> uint32_t time)
>x, y, WL_TOUCH_MOTION);
>   else if (device->seat_caps & EVDEV_SEAT_POINTER)
>   notify_motion_absolute(master, time, x, y);
> - goto handled;
> + break;
>   case EVDEV_ABSOLUTE_TOUCH_UP:
>   seat_slot = device->abs.seat_slot;
>   master->slot_map &= ~(1 << seat_slot);
>   notify_touch(master, time, seat_slot, 0, 0, WL_TOUCH_UP);
> - goto handled;
> + break;
> + default:
> + assert(0 && "Unknown pending event type");
>   }
>  
> - assert(0 && "Unknown pending event type");
> -
> -handled:
>   device->pending_event = EVDEV_NONE;
>  }
>  
> 
These patches fixed the libinput crashing for me as well. 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: [PATCH weston v3 0/4] Input fixes

[Eoff, Ullysses A]

I tested these on Weston w/libinput and they seem to have fixed up
the touch and input issues nicely.  The only use-case I didn't test was
the one described in bug 73950... I didn't have the right hardware setup
for that.  We'll be sure to hit that case in beta testing, however.

Cheers,

U. Artie 

> -Original Message-
> From: wayland-devel [mailto:wayland-devel-boun...@lists.freedesktop.org] On 
> Behalf Of Ander Conselvan de Oliveira
> Sent: Thursday, April 24, 2014 5:11 AM
> To: wayland-devel@lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander
> Subject: [PATCH weston v3 0/4] Input fixes
> 
> From: Ander Conselvan de Oliveira 
> 
> Resend of the fixes update to cover libinput usage too.
> 
> Ander Conselvan de Oliveira (4):
>   libinput: Don't process touch events for devices without a valid
> output
>   evdev: Discard events from a touchscreen paired with an unplugged
> output
>   evdev: Fix assertion error for unplugged output with paired
> touchscreen
>   input: Fix errors due to initializing input before creating outputs
> 
>  src/evdev.c   | 21 ++---
>  src/libinput-device.c |  5 -
>  src/libinput-seat.c   | 10 ++
>  src/udev-seat.c   | 10 ++
>  4 files changed, 26 insertions(+), 20 deletions(-)
> 
> --
> 1.8.3.2
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] Use non-blocking timerfd to prevent blocking when updating timer event sources

[Kristian Høgsberg]

On Fri, Apr 25, 2014 at 03:00:54PM +0100, Andrew Wedgbury wrote:
> This implements a simple fix for the blocking problem that occurs when 
> updating a timer event source after the timer expires, but before its 
> callback is dispatched. This can happen when another event happens during the
> same epoll wakeup as the timer event, and causes the read() call in 
> wl_event_source_timer_dispatch() to block for the updated duration of the 
> timer.
> 
> We never want this read() call to block, so I believe it makes sense for the
> timerfd to be non-blocking, and we simply ignore the case where the read fails
> with EAGAIN. We still report all other errors as before, and still ignore the
> actual value read from the socket.
> 
> With this change, the event_loop_timer_updates unit test case I submitted
> previously now passes, and weston appears to work as before.

Thanks, Andrew, good test case and analysis and I agree with the fix.
Test case and fix committed.

Kristian

> 
> ---
>  src/event-loop.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/event-loop.c b/src/event-loop.c
> index d323601..1046600 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -173,7 +173,7 @@ wl_event_source_timer_dispatch(struct wl_event_source 
> *source,
>   int len;
>  
>   len = read(source->fd, &expires, sizeof expires);
> - if (len != sizeof expires)
> + if (!(len == -1 && errno == EAGAIN) && len != sizeof expires)
>   /* Is there anything we can do here?  Will this ever happen? */
>   fprintf(stderr, "timerfd read error: %m\n");
>  
> @@ -196,7 +196,8 @@ wl_event_loop_add_timer(struct wl_event_loop *loop,
>   return NULL;
>  
>   source->base.interface = &timer_source_interface;
> - source->base.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
> + source->base.fd = timerfd_create(CLOCK_MONOTONIC,
> +  
> TFD_CLOEXEC | TFD_NONBLOCK);
>   source->func = func;
>  
>   return add_source(loop, &source->base, WL_EVENT_READABLE, data);
> -- 
> 1.9.2
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] Simply the matrix calculation for zooming

[Jasper St. Pierre]

I fully support any patch that removes the phrase "modelview".


On Fri, Apr 25, 2014 at 5:07 PM, Kristian Høgsberg wrote:

> On Fri, Apr 25, 2014 at 01:19:37PM +0100, Neil Roberts wrote:
> > In order to apply the zoom transformation to the output matrix, Weston
> was
> > doing the following:
> >
> > • Create a temporary matrix to hold the translation
> > • Invert the translation matrix using weston_matrix_invert into
> >   another temporary matrix
> > • Scale that matrix by the scale factor
> > • Multiply the current matrix with the temporary matrix
> >
> > Using weston_matrix_invert to invert a translation matrix is over the
> top.
> > Instead we can just negate the values we pass to weston_matrix_translate.
> > Matrix multiplication is associative so creating a temporary matrix to
> hold the
> > scale and translation transform should be equivalent to just applying
> them
> > directly to the output matrix.
>
> Heh, nice clean up, that always looked like it was too complicated for its
> own good.  Patch applied.
>
> Kristian
>
> > ---
> >  src/compositor.c | 13 -
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index fd2decb..f836cf7 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -3186,8 +3186,6 @@ WL_EXPORT void
> >  weston_output_update_matrix(struct weston_output *output)
> >  {
> >   float magnification;
> > - struct weston_matrix camera;
> > - struct weston_matrix modelview;
> >
> >   weston_matrix_init(&output->matrix);
> >   weston_matrix_translate(&output->matrix,
> > @@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct weston_output
> *output)
> >
> >   if (output->zoom.active) {
> >   magnification = 1 / (1 - output->zoom.spring_z.current);
> > - weston_matrix_init(&camera);
> > - weston_matrix_init(&modelview);
> >   weston_output_update_zoom(output);
> > - weston_matrix_translate(&camera, output->zoom.trans_x,
> > - -output->zoom.trans_y, 0);
> > - weston_matrix_invert(&modelview, &camera);
> > - weston_matrix_scale(&modelview, magnification,
> magnification, 1.0);
> > - weston_matrix_multiply(&output->matrix, &modelview);
> > + weston_matrix_translate(&output->matrix,
> -output->zoom.trans_x,
> > + output->zoom.trans_y, 0);
> > + weston_matrix_scale(&output->matrix, magnification,
> > + magnification, 1.0);
> >   }
> >
> >   output->dirty = 0;
> > --
> > 1.9.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
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>



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

Re: [PATCH weston] Simply the matrix calculation for zooming

[Kristian Høgsberg]

On Fri, Apr 25, 2014 at 01:19:37PM +0100, Neil Roberts wrote:
> In order to apply the zoom transformation to the output matrix, Weston was
> doing the following:
> 
> • Create a temporary matrix to hold the translation
> • Invert the translation matrix using weston_matrix_invert into
>   another temporary matrix
> • Scale that matrix by the scale factor
> • Multiply the current matrix with the temporary matrix
> 
> Using weston_matrix_invert to invert a translation matrix is over the top.
> Instead we can just negate the values we pass to weston_matrix_translate.
> Matrix multiplication is associative so creating a temporary matrix to hold 
> the
> scale and translation transform should be equivalent to just applying them
> directly to the output matrix.

Heh, nice clean up, that always looked like it was too complicated for its
own good.  Patch applied.

Kristian

> ---
>  src/compositor.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index fd2decb..f836cf7 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3186,8 +3186,6 @@ WL_EXPORT void
>  weston_output_update_matrix(struct weston_output *output)
>  {
>   float magnification;
> - struct weston_matrix camera;
> - struct weston_matrix modelview;
>  
>   weston_matrix_init(&output->matrix);
>   weston_matrix_translate(&output->matrix,
> @@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct weston_output 
> *output)
>  
>   if (output->zoom.active) {
>   magnification = 1 / (1 - output->zoom.spring_z.current);
> - weston_matrix_init(&camera);
> - weston_matrix_init(&modelview);
>   weston_output_update_zoom(output);
> - weston_matrix_translate(&camera, output->zoom.trans_x,
> - -output->zoom.trans_y, 0);
> - weston_matrix_invert(&modelview, &camera);
> - weston_matrix_scale(&modelview, magnification, magnification, 
> 1.0);
> - weston_matrix_multiply(&output->matrix, &modelview);
> + weston_matrix_translate(&output->matrix, -output->zoom.trans_x,
> + output->zoom.trans_y, 0);
> + weston_matrix_scale(&output->matrix, magnification,
> + magnification, 1.0);
>   }
>  
>   output->dirty = 0;
> -- 
> 1.9.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
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] clients/window: Don't remove the touch listener on a frame event

[Kristian Høgsberg]

On Wed, Apr 23, 2014 at 06:02:47PM +0100, Neil Roberts wrote:
> It looks like the handler for frame events from the wl_touch interface for
> widgets may have been erroneously copied from the cancel handler so that it
> removes all handlers as they are processed. I don't think this makes much 
> sense
> for the frame event. This was stopping the panel icons from being pushable 
> with
> touch events when using libinput since commit 1679f232e541489e. All that 
> commit
> does it make it start sending the frame events.

Yeah, that doesn't look right, patch applied.

Kristian

> ---
>  clients/window.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/clients/window.c b/clients/window.c
> index e770a04..e2f7010 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -3065,9 +3065,6 @@ touch_handle_frame(void *data, struct wl_touch 
> *wl_touch)
>   if (tp->widget->touch_frame_handler)
>   (*tp->widget->touch_frame_handler)(tp->widget, input, 
>  
> tp->widget->user_data);
> -
> - wl_list_remove(&tp->link);
> - free(tp);
>   }
>  }
>  
> -- 
> 1.9.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
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Use the correct width/height when transforming surfaces with viewports.

[Kristian Høgsberg]

On Tue, Apr 22, 2014 at 09:51:53AM +0300, Pekka Paalanen wrote:
> On Mon, 21 Apr 2014 20:56:46 -0500
> Jason Ekstrand  wrote:
> 
> > Previously, because of the wrong width/height,
> > weston_surface_to_buffer_* would return the wrong values when
> > wl_viewport was used in combination with wl_surface.set_buffer_transform.
> > 
> > Signed-off-by: Jason Ekstrand 

Committed this one and the pixman-renderer, with Pekkas Reviewed-by added.

Kristian

> > ---
> >  src/compositor.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/compositor.c b/src/compositor.c
> > index a298fb8..342e5e4 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -675,7 +675,8 @@ weston_surface_to_buffer_float(struct weston_surface 
> > *surface,
> > /* first transform coordinates if the scaler is set */
> > scaler_surface_to_buffer(surface, sx, sy, bx, by);
> >  
> > -   weston_transformed_coord(surface->width, surface->height,
> > +   weston_transformed_coord(surface->width_from_buffer,
> > +surface->height_from_buffer,
> >  vp->buffer.transform, vp->buffer.scale,
> >  *bx, *by, bx, by);
> >  }
> > @@ -709,7 +710,8 @@ weston_surface_to_buffer_rect(struct weston_surface 
> > *surface,
> > rect.x2 = floorf(xf);
> > rect.y2 = floorf(yf);
> >  
> > -   return weston_transformed_rect(surface->width, surface->height,
> > +   return weston_transformed_rect(surface->width_from_buffer,
> > +  surface->height_from_buffer,
> >vp->buffer.transform, vp->buffer.scale,
> >rect);
> >  }
> 
> Hi Jason,
> 
> good catch, this looks correct.
> 
> 
> Thanks,
> pq
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 1/3] Check zalloc return for out of memory situation

[Kristian Høgsberg]

On Fri, Apr 25, 2014 at 1:18 PM, Kristian Høgsberg  wrote:
> On Mon, Apr 21, 2014 at 11:51:02PM +, Bryce W. Harrington wrote:
>> Most zalloc calls in weston are checked, this fixes a handful that were
>> being ignored.  As found by `grep -EIsr "[^x]zalloc\(" . -A1`
>
> Thanks Bryce, applied with once change as mentioned below.
>
> Kristian
>
>> Signed-off-by: Bryce Harrington 
>> ---
>>  src/compositor-wayland.c |6 ++
>>  src/libinput-seat.c  |2 +-
>>  src/screen-share.c   |8 +++-
>>  src/screenshooter.c  |   33 -
>>  src/udev-seat.c  |2 +-
>>  5 files changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index f35db9c..67f15be 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -256,6 +256,12 @@ wayland_output_get_shm_buffer(struct wayland_output 
>> *output)
>>   }
>>
>>   sb = zalloc(sizeof *sb);
>> + if (sb == NULL) {
>> + weston_log("could not zalloc %ld memory for sb: %m\n", sizeof 
>> *sb);
>> + close(fd);
>> + free(data);
>> + return NULL;
>> + }
>>
>>   sb->output = output;
>>   wl_list_init(&sb->free_link);
>> diff --git a/src/libinput-seat.c b/src/libinput-seat.c
>> index b6adc76..b2090fa 100644
>> --- a/src/libinput-seat.c
>> +++ b/src/libinput-seat.c
>> @@ -313,9 +313,9 @@ udev_seat_create(struct udev_input *input, const char 
>> *seat_name)
>>   struct udev_seat *seat;
>>
>>   seat = zalloc(sizeof *seat);
>> -
>>   if (!seat)
>>   return NULL;
>> +
>>   weston_seat_init(&seat->base, c, seat_name);
>>   seat->base.led_update = udev_seat_led_update;
>>
>> diff --git a/src/screen-share.c b/src/screen-share.c
>> index 5de20be..924672e 100644
>> --- a/src/screen-share.c
>> +++ b/src/screen-share.c
>> @@ -433,12 +433,18 @@ shared_output_get_shm_buffer(struct shared_output *so)
>>
>>   data = mmap(NULL, height * stride, PROT_READ | PROT_WRITE, MAP_SHARED, 
>> fd, 0);
>>   if (data == MAP_FAILED) {
>> - weston_log("mmap: %m");
>> + weston_log("could not mmap %d memory for data: %m\n", height * 
>> stride);
>>   close(fd);
>>   return NULL;
>>   }
>>
>>   sb = zalloc(sizeof *sb);
>> + if (sb == NULL) {
>> + weston_log("could not zalloc %d memory for sb: %m\n", sizeof 
>> *sb);
>> + close(fd);
>> + free(data);
>
> data was mmapped, I fixed this to use munmap instead.

And this was already fixed by Hardenings patch, so I left the
screen-share.c part out.

Kristian

>
>> + return NULL;
>> + }
>>
>>   sb->output = so;
>>   wl_list_init(&sb->free_link);
>> diff --git a/src/screenshooter.c b/src/screenshooter.c
>> index 02146c8..369e920 100644
>> --- a/src/screenshooter.c
>> +++ b/src/screenshooter.c
>> @@ -450,6 +450,17 @@ weston_recorder_frame_notify(struct wl_listener 
>> *listener, void *data)
>>  }
>>
>>  static void
>> +weston_recorder_free(struct weston_recorder *recorder)
>> +{
>> + if (recorder == NULL)
>> + return;
>> + free(recorder->rect);
>> + free(recorder->tmpbuf);
>> + free(recorder->frame);
>> + free(recorder);
>> +}
>> +
>> +static void
>>  weston_recorder_create(struct weston_output *output, const char *filename)
>>  {
>>   struct weston_compositor *compositor = output->compositor;
>> @@ -461,7 +472,6 @@ weston_recorder_create(struct weston_output *output, 
>> const char *filename)
>>   do_yflip = !!(compositor->capabilities & WESTON_CAP_CAPTURE_YFLIP);
>>
>>   recorder = malloc(sizeof *recorder);
>> -
>>   if (recorder == NULL) {
>>   weston_log("%s: out of memory\n", __func__);
>>   return;
>> @@ -476,6 +486,12 @@ weston_recorder_create(struct weston_output *output, 
>> const char *filename)
>>   recorder->destroying = 0;
>>   recorder->output = output;
>>
>> + if ((recorder->frame == NULL) || (recorder->rect == NULL)) {
>> + weston_log("%s: out of memory\n", __func__);
>> + weston_recorder_free(recorder);
>> + return;
>> + }
>> +
>>   if (do_yflip)
>>   recorder->tmpbuf = NULL;
>>   else
>> @@ -493,10 +509,7 @@ weston_recorder_create(struct weston_output *output, 
>> const char *filename)
>>   break;
>>   default:
>>   weston_log("unknown recorder format\n");
>> - free(recorder->rect);
>> - free(recorder->tmpbuf);
>> - free(recorder->frame);
>> - free(recorder);
>> + weston_recorder_free(recorder);
>>   return;
>>   }
>>
>> @@ -505,10 +518,7 @@ weston_recorder_create(struct weston_output *output, 
>> const char *filename)
>>
>>   if (recorder->fd < 0) {
>>   weston_log("problem opening output file %s: %m\n", filename);
>> -   

Re: [PATCH 2/3] xwayland: Check zalloc return for out of memory situation

[Kristian Høgsberg]

On Mon, Apr 21, 2014 at 11:51:03PM +, Bryce W. Harrington wrote:
> 
> Signed-off-by: Bryce Harrington 
> ---

Applied, thanks.

Kristian

>  xwayland/launcher.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xwayland/launcher.c b/xwayland/launcher.c
> index ac692de..70703a4 100644
> --- a/xwayland/launcher.c
> +++ b/xwayland/launcher.c
> @@ -348,6 +348,8 @@ module_init(struct weston_compositor *compositor,
>   char lockfile[256], display_name[8];
>  
>   wxs = zalloc(sizeof *wxs);
> + if (wxs == NULL)
> + return -1;
>   wxs->process.cleanup = weston_xserver_cleanup;
>   wxs->wl_display = display;
>   wxs->compositor = compositor;
> -- 
> 1.7.9.5
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 3/3] clients: Check zalloc return for out of memory situation

[Kristian Høgsberg]

On Mon, Apr 21, 2014 at 11:51:03PM +, Bryce W. Harrington wrote:
> Checking for these errors in the clients is perhaps a bit gratuitous but
> can't hurt.

In the clients we use the x*alloc functions which abort on allocation
failure.

Kristian

> Signed-off-by: Bryce Harrington 
> ---
>  clients/gears.c|3 +++
>  clients/terminal.c |   20 
>  2 files changed, 23 insertions(+)
> 
> diff --git a/clients/gears.c b/clients/gears.c
> index 93a86b4..22159be 100644
> --- a/clients/gears.c
> +++ b/clients/gears.c
> @@ -402,6 +402,9 @@ gears_create(struct display *display)
>   int i;
>  
>   gears = zalloc(sizeof *gears);
> + if (gears == NULL)
> + die("failed to zalloc memory for gears\n");
> +
>   gears->d = display;
>   gears->window = window_create(display);
>   gears->widget = window_frame_create(gears->window, gears);
> diff --git a/clients/terminal.c b/clients/terminal.c
> index 5931ce2..6bdb039 100644
> --- a/clients/terminal.c
> +++ b/clients/terminal.c
> @@ -773,9 +773,24 @@ terminal_resize_cells(struct terminal *terminal,
>   terminal->max_width = width;
>   data_pitch = width * sizeof(union utf8_char);
>   data = zalloc(data_pitch * terminal->buffer_height);
> + if (data == NULL) {
> + fprintf(stderr, "failed to zalloc data: %m\n");
> + return;
> + }
>   attr_pitch = width * sizeof(struct attr);
>   data_attr = malloc(attr_pitch * terminal->buffer_height);
> + if (data_attr == NULL) {
> + fprintf(stderr, "failed to zalloc data_attr: %m\n");
> + free(data);
> + return;
> + }
>   tab_ruler = zalloc(width);
> + if (tab_ruler == NULL) {
> + fprintf(stderr, "failed to zalloc tab_ruler: %m\n");
> + free(data);
> + free(data_attr);
> + return;
> + }
>   attr_init(data_attr, terminal->curr_attr,
> width * terminal->buffer_height);
>  
> @@ -2835,6 +2850,8 @@ terminal_create(struct display *display)
>   cairo_text_extents_t text_extents;
>  
>   terminal = xzalloc(sizeof *terminal);
> + if (terminal == NULL)
> + return NULL;
>   terminal->color_scheme = &DEFAULT_COLORS;
>   terminal_init(terminal);
>   terminal->margin_top = 0;
> @@ -2961,6 +2978,9 @@ terminal_run(struct terminal *terminal, const char 
> *path)
>   } else if (pid < 0) {
>   fprintf(stderr, "failed to fork and create pty (%m).\n");
>   return -1;
> + } else if (terminal == NULL) {
> + fprintf(stderr, "out of memory: %m\n");
> + return -1;
>   }
>  
>   terminal->master = master;
> -- 
> 1.7.9.5
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 1/3] Check zalloc return for out of memory situation

[Kristian Høgsberg]

On Mon, Apr 21, 2014 at 11:51:02PM +, Bryce W. Harrington wrote:
> Most zalloc calls in weston are checked, this fixes a handful that were
> being ignored.  As found by `grep -EIsr "[^x]zalloc\(" . -A1`

Thanks Bryce, applied with once change as mentioned below.

Kristian

> Signed-off-by: Bryce Harrington 
> ---
>  src/compositor-wayland.c |6 ++
>  src/libinput-seat.c  |2 +-
>  src/screen-share.c   |8 +++-
>  src/screenshooter.c  |   33 -
>  src/udev-seat.c  |2 +-
>  5 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index f35db9c..67f15be 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -256,6 +256,12 @@ wayland_output_get_shm_buffer(struct wayland_output 
> *output)
>   }
>  
>   sb = zalloc(sizeof *sb);
> + if (sb == NULL) {
> + weston_log("could not zalloc %ld memory for sb: %m\n", sizeof 
> *sb);
> + close(fd);
> + free(data);
> + return NULL;
> + }
>  
>   sb->output = output;
>   wl_list_init(&sb->free_link);
> diff --git a/src/libinput-seat.c b/src/libinput-seat.c
> index b6adc76..b2090fa 100644
> --- a/src/libinput-seat.c
> +++ b/src/libinput-seat.c
> @@ -313,9 +313,9 @@ udev_seat_create(struct udev_input *input, const char 
> *seat_name)
>   struct udev_seat *seat;
>  
>   seat = zalloc(sizeof *seat);
> -
>   if (!seat)
>   return NULL;
> +
>   weston_seat_init(&seat->base, c, seat_name);
>   seat->base.led_update = udev_seat_led_update;
>  
> diff --git a/src/screen-share.c b/src/screen-share.c
> index 5de20be..924672e 100644
> --- a/src/screen-share.c
> +++ b/src/screen-share.c
> @@ -433,12 +433,18 @@ shared_output_get_shm_buffer(struct shared_output *so)
>  
>   data = mmap(NULL, height * stride, PROT_READ | PROT_WRITE, MAP_SHARED, 
> fd, 0);
>   if (data == MAP_FAILED) {
> - weston_log("mmap: %m");
> + weston_log("could not mmap %d memory for data: %m\n", height * 
> stride);
>   close(fd);
>   return NULL;
>   }
>  
>   sb = zalloc(sizeof *sb);
> + if (sb == NULL) {
> + weston_log("could not zalloc %d memory for sb: %m\n", sizeof 
> *sb);
> + close(fd);
> + free(data);

data was mmapped, I fixed this to use munmap instead.

> + return NULL;
> + }
>  
>   sb->output = so;
>   wl_list_init(&sb->free_link);
> diff --git a/src/screenshooter.c b/src/screenshooter.c
> index 02146c8..369e920 100644
> --- a/src/screenshooter.c
> +++ b/src/screenshooter.c
> @@ -450,6 +450,17 @@ weston_recorder_frame_notify(struct wl_listener 
> *listener, void *data)
>  }
>  
>  static void
> +weston_recorder_free(struct weston_recorder *recorder)
> +{
> + if (recorder == NULL)
> + return;
> + free(recorder->rect);
> + free(recorder->tmpbuf);
> + free(recorder->frame);
> + free(recorder);
> +}
> +
> +static void
>  weston_recorder_create(struct weston_output *output, const char *filename)
>  {
>   struct weston_compositor *compositor = output->compositor;
> @@ -461,7 +472,6 @@ weston_recorder_create(struct weston_output *output, 
> const char *filename)
>   do_yflip = !!(compositor->capabilities & WESTON_CAP_CAPTURE_YFLIP);
>  
>   recorder = malloc(sizeof *recorder);
> -
>   if (recorder == NULL) {
>   weston_log("%s: out of memory\n", __func__);
>   return;
> @@ -476,6 +486,12 @@ weston_recorder_create(struct weston_output *output, 
> const char *filename)
>   recorder->destroying = 0;
>   recorder->output = output;
>  
> + if ((recorder->frame == NULL) || (recorder->rect == NULL)) {
> + weston_log("%s: out of memory\n", __func__);
> + weston_recorder_free(recorder);
> + return;
> + }
> +
>   if (do_yflip)
>   recorder->tmpbuf = NULL;
>   else
> @@ -493,10 +509,7 @@ weston_recorder_create(struct weston_output *output, 
> const char *filename)
>   break;
>   default:
>   weston_log("unknown recorder format\n");
> - free(recorder->rect);
> - free(recorder->tmpbuf);
> - free(recorder->frame);
> - free(recorder);
> + weston_recorder_free(recorder);
>   return;
>   }
>  
> @@ -505,10 +518,7 @@ weston_recorder_create(struct weston_output *output, 
> const char *filename)
>  
>   if (recorder->fd < 0) {
>   weston_log("problem opening output file %s: %m\n", filename);
> - free(recorder->rect);
> - free(recorder->tmpbuf);
> - free(recorder->frame);
> - free(recorder);
> + weston_recorder_free(recorder);
>   return;
>   }
>  
> @@ -527,11 +537,8 @@ weston_recorder_destroy(struct westo

Re: [PATCH weston-ivi-shell v4 1/9] ivi application protocol:

[Pekka Paalanen]

On Fri, 25 Apr 2014 22:34:27 +0900
Nobuhiko Tanibata  wrote:

> 2014-04-23 19:40 に Pekka Paalanen さんは書きました:
> > This is looking good, mostly just some details in the wording
> > to be tuned. :-)
> > 
> > I will see if I can review more of the patches, but I would also
> > suggest the following, in case you are still interested in
> > pushing this upstream.
> > 
> > Wait for Weston 1.5 to be released. We are currently in freeze,
> > so there is no point in re-sending until that is done. Check
> > that the 1.5 stable branch has been created, or at least the
> > release has been made, before you rebase and re-send this
> > series.
> > 
> Hi pq,
> 
> Yes, I am still interested in pushing them. I will rabase them
> and re-send them after weston 1.5 branch has been created.
> I am also confirming your comments for other patches as well.
> Btw, do you have a date when the branch is made?

Hi,

good to hear. :-)

I don't know of a date, but I suppose early May, perhaps.


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

Re: [PATCH weston] Simply the matrix calculation for zooming

[Jason Ekstrand]

On Apr 25, 2014 8:11 AM, "Pekka Paalanen"  wrote:
>
> On Fri, 25 Apr 2014 13:19:37 +0100
> Neil Roberts  wrote:
>
> > In order to apply the zoom transformation to the output matrix, Weston
was
> > doing the following:
> >
> > * Create a temporary matrix to hold the translation
> > * Invert the translation matrix using weston_matrix_invert into
> >   another temporary matrix
> > * Scale that matrix by the scale factor
> > * Multiply the current matrix with the temporary matrix
> >
> > Using weston_matrix_invert to invert a translation matrix is over the
top.
> > Instead we can just negate the values we pass to
weston_matrix_translate.
> > Matrix multiplication is associative so creating a temporary matrix to
hold the
> > scale and translation transform should be equivalent to just applying
them
> > directly to the output matrix.
> > ---
> >  src/compositor.c | 13 -
> >  1 file changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/compositor.c b/src/compositor.c
> > index fd2decb..f836cf7 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -3186,8 +3186,6 @@ WL_EXPORT void
> >  weston_output_update_matrix(struct weston_output *output)
> >  {
> >   float magnification;
> > - struct weston_matrix camera;
> > - struct weston_matrix modelview;
> >
> >   weston_matrix_init(&output->matrix);
> >   weston_matrix_translate(&output->matrix,
> > @@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct
weston_output *output)
> >
> >   if (output->zoom.active) {
> >   magnification = 1 / (1 - output->zoom.spring_z.current);
> > - weston_matrix_init(&camera);
> > - weston_matrix_init(&modelview);
> >   weston_output_update_zoom(output);
> > - weston_matrix_translate(&camera, output->zoom.trans_x,
> > - -output->zoom.trans_y, 0);
> > - weston_matrix_invert(&modelview, &camera);
> > - weston_matrix_scale(&modelview, magnification,
magnification, 1.0);
> > - weston_matrix_multiply(&output->matrix, &modelview);
> > + weston_matrix_translate(&output->matrix,
-output->zoom.trans_x,
> > + output->zoom.trans_y, 0);
> > + weston_matrix_scale(&output->matrix, magnification,
> > + magnification, 1.0);
> >   }
> >
> >   output->dirty = 0;
>
> About time someone did this! :-D
> Yes, please.

I'll second that.
--Jason Ekstrand

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

[PATCH wayland] Use non-blocking timerfd to prevent blocking when updating timer event sources

[Andrew Wedgbury]

This implements a simple fix for the blocking problem that occurs when 
updating a timer event source after the timer expires, but before its 
callback is dispatched. This can happen when another event happens during the
same epoll wakeup as the timer event, and causes the read() call in 
wl_event_source_timer_dispatch() to block for the updated duration of the 
timer.

We never want this read() call to block, so I believe it makes sense for the
timerfd to be non-blocking, and we simply ignore the case where the read fails
with EAGAIN. We still report all other errors as before, and still ignore the
actual value read from the socket.

With this change, the event_loop_timer_updates unit test case I submitted
previously now passes, and weston appears to work as before.

---
 src/event-loop.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/event-loop.c b/src/event-loop.c
index d323601..1046600 100644
--- a/src/event-loop.c
+++ b/src/event-loop.c
@@ -173,7 +173,7 @@ wl_event_source_timer_dispatch(struct wl_event_source 
*source,
int len;
 
len = read(source->fd, &expires, sizeof expires);
-   if (len != sizeof expires)
+   if (!(len == -1 && errno == EAGAIN) && len != sizeof expires)
/* Is there anything we can do here?  Will this ever happen? */
fprintf(stderr, "timerfd read error: %m\n");
 
@@ -196,7 +196,8 @@ wl_event_loop_add_timer(struct wl_event_loop *loop,
return NULL;
 
source->base.interface = &timer_source_interface;
-   source->base.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
+   source->base.fd = timerfd_create(CLOCK_MONOTONIC,
+
TFD_CLOEXEC | TFD_NONBLOCK);
source->func = func;
 
return add_source(loop, &source->base, WL_EVENT_READABLE, data);
-- 
1.9.2

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

Re: [PATCH weston-ivi-shell v4 1/9] ivi application protocol:

[Nobuhiko Tanibata]

2014-04-23 19:40 に Pekka Paalanen さんは書きました:

Hi,

it's been a long while since I have looked at this, but I got a bit of
time to come back. I hope you haven't abandoned this effort yet. :-)

I looked at the PDF from your post on March 6th, 2014, and some of my
own comments I gave at that time to recall what this was about, but I
probably still forgot something.

New comments below. I started by reading the global interface, and
moved on to ivi_surface after it, so the comments might seem
temporally strange if you read just top-down.


On Mon, 17 Mar 2014 15:23:22 +0900
Nobuhiko Tanibata  wrote:


Add interface ivi_application, which creates ivi_surface objects tied
to a given wl_surface with a given id. The given id can be used in a
shell to identify which application is assigned to a wl_surface and
layout the surface wherever the shell wants. ivi_surface objects can
be used to receive status of wl_surface in the scenegraph of the
compositor.

Signed-off-by: Nobuhiko Tanibata 
---

Changes for v2:
   - Rename "error" to "warning" because meaning of "error" in wayland 
is fatal.


Changes for v3:
   - Move "warning" from ivi_application to ivi_surface.
   - Squash Makefile.
   - Add description to ivi_surface:destroy.
   - Update description of ivi_application:surface_create.

Changes for v4:
   - Remove detail description of server side from 
ivi_surface::destroy
   - Add clear discripton what client shall do if it encounters 
warning in ivi_surface

 ::warning.
   - Add decription what happens when client tries to tie a wl_surface 
to multiple

 ivi_surfaces.

 protocol/Makefile.am |  3 +-
 protocol/ivi-application.xml | 99 


 2 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 protocol/ivi-application.xml

diff --git a/protocol/Makefile.am b/protocol/Makefile.am
index 5e331a7..9913f16 100644
--- a/protocol/Makefile.am
+++ b/protocol/Makefile.am
@@ -8,7 +8,8 @@ protocol_sources =  \
text-cursor-position.xml\
wayland-test.xml\
xdg-shell.xml   \
-   scaler.xml
+   scaler.xml  \
+   ivi-application.xml

 if HAVE_XMLLINT
 .PHONY: validate
diff --git a/protocol/ivi-application.xml 
b/protocol/ivi-application.xml

new file mode 100644
index 000..37ad489
--- /dev/null
+++ b/protocol/ivi-application.xml
@@ -0,0 +1,99 @@
+
+
+
+
+Copyright (C) 2013 DENSO CORPORATION
+Copyright (c) 2013 BMW Car IT GmbH
+
+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 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 removes link from surface_id to wl_surface and 
destroys ivi_surface.

+
+
+
+
+
+The new visibility state is provided in argument 
visibility.

+If visibility is 0, the surface has become invisible.
+If visibility is not 0, the surface has become 
visible.

+
+
+
+
+
+
+These warning codes define all possible warning codes 
returned by ivi compositor


Codes define codes? :-)


+on server-side warnings.
+invalid_wl_surface: invalid wl_surface is set. This 
happens if wl_surface is destroyed before this.


Ooh, does this mean that invalid_wl_surface is emitted if wl_surface is
destroyed before the ivi_surface? Try to use more nouns and less
pronouns in specification language to keep things explicit.

I was about ask what happens if the wl_surface gets destroyed first.

+surface_id_in_use: surface_id is already assigned by 
another application.

+
+summary="wl_surface is invalid"/>
+summary="surface_id is in use and can not be shared"/>

+
+
+
+

Re: [PATCH weston] Simply the matrix calculation for zooming

[Pekka Paalanen]

On Fri, 25 Apr 2014 13:19:37 +0100
Neil Roberts  wrote:

> In order to apply the zoom transformation to the output matrix, Weston was
> doing the following:
> 
> • Create a temporary matrix to hold the translation
> • Invert the translation matrix using weston_matrix_invert into
>   another temporary matrix
> • Scale that matrix by the scale factor
> • Multiply the current matrix with the temporary matrix
> 
> Using weston_matrix_invert to invert a translation matrix is over the top.
> Instead we can just negate the values we pass to weston_matrix_translate.
> Matrix multiplication is associative so creating a temporary matrix to hold 
> the
> scale and translation transform should be equivalent to just applying them
> directly to the output matrix.
> ---
>  src/compositor.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index fd2decb..f836cf7 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3186,8 +3186,6 @@ WL_EXPORT void
>  weston_output_update_matrix(struct weston_output *output)
>  {
>   float magnification;
> - struct weston_matrix camera;
> - struct weston_matrix modelview;
>  
>   weston_matrix_init(&output->matrix);
>   weston_matrix_translate(&output->matrix,
> @@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct weston_output 
> *output)
>  
>   if (output->zoom.active) {
>   magnification = 1 / (1 - output->zoom.spring_z.current);
> - weston_matrix_init(&camera);
> - weston_matrix_init(&modelview);
>   weston_output_update_zoom(output);
> - weston_matrix_translate(&camera, output->zoom.trans_x,
> - -output->zoom.trans_y, 0);
> - weston_matrix_invert(&modelview, &camera);
> - weston_matrix_scale(&modelview, magnification, magnification, 
> 1.0);
> - weston_matrix_multiply(&output->matrix, &modelview);
> + weston_matrix_translate(&output->matrix, -output->zoom.trans_x,
> + output->zoom.trans_y, 0);
> + weston_matrix_scale(&output->matrix, magnification,
> + magnification, 1.0);
>   }
>  
>   output->dirty = 0;

About time someone did this! :-D
Yes, please.


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

Re: [PATCH weston-ivi-shell v5 9/9] Modify example clients to support ivi-application.xml

[Pekka Paalanen]

On Tue, 18 Mar 2014 23:57:32 +0900
Nobuhiko Tanibata  wrote:

> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2, v3 and v4
>   - nothing. Version number aligned to the first patch
> 
> Changes for v5
>   - support weston-dnd-ivi to verify wl_pointer::set_cursor and 
> wl_data_device::start_drag
> 
>  clients/.gitignore   |  6 +
>  clients/Makefile.am  | 72 
> 
>  clients/simple-egl.c | 70 ++
>  clients/simple-shm.c | 53 +-
>  clients/window.c | 45 ++--
>  5 files changed, 228 insertions(+), 18 deletions(-)

I don't like the #ifdeffery, but any other solution to achieve the same
would probably copy a lot of code. There is also the controversial
question of keeping the simple clients simple while still being useful.
If they don't support ivi-shell, they won't be useful on ivi-shell
environment.

But I suppose we have kind of crossed that line already.

What I wonder is, if it would be better to just add ivi-shell support
to these two simple clients and toytoolkit as a first class feature.
Not by #ifdeffing and compiling another set of executables, but by
having the support always built in, and detect the shell at runtime.


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

[PATCH weston] Simply the matrix calculation for zooming

[Neil Roberts]

In order to apply the zoom transformation to the output matrix, Weston was
doing the following:

• Create a temporary matrix to hold the translation
• Invert the translation matrix using weston_matrix_invert into
  another temporary matrix
• Scale that matrix by the scale factor
• Multiply the current matrix with the temporary matrix

Using weston_matrix_invert to invert a translation matrix is over the top.
Instead we can just negate the values we pass to weston_matrix_translate.
Matrix multiplication is associative so creating a temporary matrix to hold the
scale and translation transform should be equivalent to just applying them
directly to the output matrix.
---
 src/compositor.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index fd2decb..f836cf7 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3186,8 +3186,6 @@ WL_EXPORT void
 weston_output_update_matrix(struct weston_output *output)
 {
float magnification;
-   struct weston_matrix camera;
-   struct weston_matrix modelview;
 
weston_matrix_init(&output->matrix);
weston_matrix_translate(&output->matrix,
@@ -3202,14 +3200,11 @@ weston_output_update_matrix(struct weston_output 
*output)
 
if (output->zoom.active) {
magnification = 1 / (1 - output->zoom.spring_z.current);
-   weston_matrix_init(&camera);
-   weston_matrix_init(&modelview);
weston_output_update_zoom(output);
-   weston_matrix_translate(&camera, output->zoom.trans_x,
-   -output->zoom.trans_y, 0);
-   weston_matrix_invert(&modelview, &camera);
-   weston_matrix_scale(&modelview, magnification, magnification, 
1.0);
-   weston_matrix_multiply(&output->matrix, &modelview);
+   weston_matrix_translate(&output->matrix, -output->zoom.trans_x,
+   output->zoom.trans_y, 0);
+   weston_matrix_scale(&output->matrix, magnification,
+   magnification, 1.0);
}
 
output->dirty = 0;
-- 
1.9.0

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

Re: [PATCH weston-ivi-shell v5 6/9] A reference implementation of UI client how to use ivi-hmi-controller.

[Pekka Paalanen]

On Thu, 20 Mar 2014 16:00:57 +0900
Nobuhiko Tanibata  wrote:

> This is launched from hmi-controller by using hmi_client_start and create a
> pthread.
> 
> The basic flow is as followed,
> 1/ create pthread
> 2/ read configuration from weston.ini.
> 3/ draw png file to surface according to configuration of weston.ini
> 4/ set up UI by using ivi-hmi-controller protocol
> 5/ Enter event loop
> 6/ If a surface receives touch/pointer event, followings are invoked according
>to type of event and surface
> 6-1/ If a surface to launch ivi_application receive touch up, it execs
>  ivi-application configured in weston.ini.
> 6-2/ If a surface to switch layout mode receive touch up, it sends a request,
>  ivi_hmi_controller_switch_mode, to hmi-controller.
> 6-3/ If a surface to show workspace having launchers, it sends a request,
>  ivi_hmi_controller_home, to hmi-controller.
> 6-4/ If touch down events happens in workspace,
>  ivi_hmi_controller_workspace_control is sent to slide workspace.
>  When control finished, event: ivi_hmi_controller_workspace_end_control
>  is received.
> 
> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4:
>  - nothing. Version number aligned to the first patch
> 
> Changes for v5:
>  - usleep with roundtrip uses CPU. replace them with wl_display_dispatch
> 
>  ivi-shell/Makefile.am |2 +
>  ivi-shell/hmi-controller-homescreen.c | 1369 
> +
>  ivi-shell/hmi-controller-homescreen.h |   36 +
>  ivi-shell/hmi-controller.c|3 +-
>  4 files changed, 1409 insertions(+), 1 deletion(-)
>  create mode 100644 ivi-shell/hmi-controller-homescreen.c
>  create mode 100644 ivi-shell/hmi-controller-homescreen.h

Would it not be simpler and more robust to make this an independent
program like e.g. clients/desktop-shell.c is, rather than running it in
a thread in the compositor?

I would certainly prefer it to be. We would avoid threads in the
compositor, and pulling in client side stuff. Now there is a huge risk
you might be calling compositor functions from client code, and a crash
in the client code would bring the whole compositor down.

If we look at weston-desktop-shell, if it crashes, Weston will respawn
it so fast that a user often does not even notice anything happened. ;-)

And when you attach gdb, it won't stop also the compositor.


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

Re: [PATCH weston-ivi-shell v4 5/9] A reference implementation how to use weston-layout library.

[Pekka Paalanen]

On Mon, 17 Mar 2014 15:28:22 +0900
Nobuhiko Tanibata  wrote:

> The library is used to manage layout of surfaces/layers. Layout change is
> triggered by ivi-hmi-controller protocol, ivi-hmi-controller.xml. A reference
> how to use the protocol, see hmi-controller-homescreen.
> 
> In-Vehicle Infotainment system usually manage properties of surfaces/layers
> by only a central component which decide where surfaces/layers shall be. This
> reference show examples to implement the central component as a module of
> weston.
> 
> Default Scene graph of UI is defined in hmi_controller_create. It consists of
> - In the bottom, a base layer to group surfaces of background, panel,
>   and buttons
> - Next, a application layer to show application surfaces.
> - Workspace background layer to show a surface of background image.
> - Workspace layer to show launcher to launch application with icons. Paths to
>   binary and icon are defined in weston.ini. The width of this layer is longer
>   than the size of screen because a workspace has several pages and is
>   controlled by motion of input.
> 
> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4
>  - nothing. Version number aligned to the first patch
> 
>  ivi-shell/.gitignore   |7 +
>  ivi-shell/Makefile.am  |   19 +-
>  ivi-shell/hmi-controller.c | 1775 
> 
>  3 files changed, 1799 insertions(+), 2 deletions(-)
>  create mode 100644 ivi-shell/.gitignore
>  create mode 100644 ivi-shell/hmi-controller.c

Hi,

so this is the hmi-controller. This is the part that IVI vendors will
be customizing, is it? Or replacing, actually?

The picture in your PDF has both ivi-controller.so and
hmi-controller.so, where the ivi-controller.so is exposing the
ivi-controller Wayland extension. These will be exclusive, right? Never
used at the same time.

hmi-controller.so exposes the ivi_hmi_controller private Wayland
protocol extension, right? So this patch series does not yet have the
ivi-controller part at all, and ivi_hmi_controller is just a part of
the demo that is hmi-controller et al.? And all that would be replaced
by a "real" IVI thing?

Oh yeah, you said the ivi-controller stuff is at
http://git.projects.genivi.org/?p=wayland-ivi-extension.git;a=summary

Okay, I'm actually happy that part is not in this patch series, the
protocol extension looks huge. ;-)

I'm not going through this patch too carefully, just making some general
observations.

There are lots of stuff using the weston-layout API, but I see also a
lot stuff using Weston core API like the grab handlers and seat stuff.
Since this is the part that vendors replace, would it not be better to
have the Weston core related stuff back in ivi-shell.so?

Or is the stuff used here such, that a real ivi-controller will not
need it?

Or is it just a work in progress to establish an abstraction and that
part is still to do?

How independent of a particular compositor implementation is the so
called weston-layout abstraction supposed to be? Will it evolve into a
completely compositor-agnostic API?

I am just trying to understand what exactly the API surface is, that
external modules like ivi-controller.so will be using. Are they allowed
to use Weston core API, ivi-shell.so exported API, only weston-layout
API?

I said it before, that I think you should pay attention to proper
versioning of the weston-layout API, or tie that to Weston core
versioning.

Again, I would suggest:
- document what APIs the external modules can use
- write a header for the API that ivi-shell.so will export for the
  external modules; this would replace the weston-layout.h and be
  smaller
- let ivi-shell.so export the above mentioned API, so you get rid of
  the library interfaces between libweston-layout and ivi-shell; just
  merge the code and remove everything that falls out by refactoring

That would mean external modules used libwayland-server, weston core(?),
and the ivi-shell.so exported APIs, and not additionally depending on
yet another library libweston-layout.so.

I don't think you currently export any API from ivi-shell.so, but
external modules still depend on it anyway, by relying on it using
libweston-layout.so properly. I think it would be good to make this
dependency explicit.

What do you think?
I believe it should work.


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

Re: [PATCH weston-ivi-shell v4 8/9] Add a reference of weston.ini for ivi-shell and ivi-hmi-controller.

[Pekka Paalanen]

On Mon, 17 Mar 2014 15:31:09 +0900
Nobuhiko Tanibata  wrote:

> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4
>   - nothing. Version number aligned to the first patch
> 
>  ivi-shell/Makefile.am   | 12 
>  ivi-shell/weston.ini.in | 79 
> +
>  2 files changed, 91 insertions(+)
>  create mode 100644 ivi-shell/weston.ini.in
> 
> diff --git a/ivi-shell/Makefile.am b/ivi-shell/Makefile.am
> index afaa5e3..333abb7 100644
> --- a/ivi-shell/Makefile.am
> +++ b/ivi-shell/Makefile.am
> @@ -67,3 +67,15 @@ CLEANFILES = $(BUILT_SOURCES)
>  
>  wayland_protocoldir = $(top_srcdir)/protocol
>  include $(top_srcdir)/wayland-scanner.mk
> +
> +
> +weston.ini : $(srcdir)/weston.ini.in
> + $(AM_V_GEN)$(SED) \
> + -e 's|@bindir[@]|$(bindir)|g' \
> + -e 's|@abs_top_builddir[@]|$(abs_top_builddir)|g' \
> + -e 's|@libexecdir[@]|$(libexecdir)|g' \
> + $< > $@
> +
> +all-local : weston.ini
> +
> +CLEANFILES += weston.ini
> diff --git a/ivi-shell/weston.ini.in b/ivi-shell/weston.ini.in
> new file mode 100644
> index 000..c9a6861
> --- /dev/null
> +++ b/ivi-shell/weston.ini.in
> @@ -0,0 +1,79 @@
> +[core]
> +shell=ivi-shell.so
> +modules=hmi-controller.so

Ooh, I see, this is how you get hmi-controller.so loaded.

Since both ivi-shell.so and hmi-controller.so both link to
libweston-layout.la at build time and both are dlopen'd at runtime, what
guarantees that both use the same instance of the global 'ivilayout'
defined in weston-layout.c?

My knowledge of runtime dynamic linking is a bit weak, but
personally I'd just avoid that global there. It is a bit much of magic.

AFAICS, you might be implicitly depending on the loading order of
ivi-shell.so first, hmi-controller.so next. Weston does do it that way,
but I'd hope the dependency was more obvious than both secretly using
the same global in a yet third library.

What happens, if hmi-controller.so is loaded, but shell is not
ivi-shell.so?

What happens, if ivi-shell.so is the shell, but hmi-controller.so is
not loaded?

Would it be better if ivi-shell.so loaded the whatever hmi-controller
plugin that is appropriate? weston_load_module() is exported, and that
would make the dependency explicit, allowing you to nicely exit weston
with an error if something doesn't load right.

After all, hmi-controller.so seems like a plugin to ivi-shell than
anything else.


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

[PATCH wayland] test: Add test showing blocking problem when updating timers

[Andrew Wedgbury]

I've noticed a blocking problem in Wayland's event-loop code when updating 
timer event sources. The problem occurs if you update the timer at a point 
after is has expired, but before it has been dispatched, i.e. from an event 
callback that happens during the same epoll wakeup.

When the timer is subsequently dispatched, wl_event_source_timer_dispatch 
blocks for the duration of the new timeout in its call to read() from the 
timer fd (which is the expected behaviour according to the man page for 
timerfd_settime).

This isn't too uncommon a scenario - for example, a socket with an associated
timeout timer. You'd typically want to update the timer when reading from the 
socket. This is how I noticed the issue, since I was setting a timeout of 
1 minute, and saw my server blocking for this duration!

The following patch adds a (currently failing) test case to Wayland's 
event-loop-test.c. It demonstrates the problem using two timers, which are
set to expire at the same time. The first timer to receive its expiry 
callback updates the other timer with a much larger timeout, which then 
causes the test to block for this timeout before calling the second timer's
callback.

As for a fix, I'm not so sure (which is why I thought I'd post the failing
test case first to show what I mean). I notice that it doesn't actually do
anything with the value read from the timerfd socket, which gives the number
of times the timer expired since the last read, or when the timer was last
updated (which blocks if the timer hasn't yet expired). I believe this value
should always read as 1 anyway, since we don't use periodic timers.

A simple fix would be to use the TFD_NONBLOCK option when creating the
timerfd, ensuring that the read call won't block. We'd then have to ignore
the case when the read returns EAGAIN.

---
 tests/event-loop-test.c | 74 +
 1 file changed, 74 insertions(+)

diff --git a/tests/event-loop-test.c b/tests/event-loop-test.c
index f250ed0..1218163 100644
--- a/tests/event-loop-test.c
+++ b/tests/event-loop-test.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "wayland-private.h"
 #include "wayland-server.h"
@@ -199,6 +200,79 @@ TEST(event_loop_timer)
wl_event_loop_destroy(loop);
 }
 
+struct timer_update_context {
+   struct wl_event_source *source1, *source2;
+   int count;
+};
+
+static int
+timer_update_callback_1(void *data)
+{
+   struct timer_update_context *context = data;
+
+   context->count++;
+   wl_event_source_timer_update(context->source2, 1000);
+   return 1;
+}
+
+static int
+timer_update_callback_2(void *data)
+{
+   struct timer_update_context *context = data;
+
+   context->count++;
+   wl_event_source_timer_update(context->source1, 1000);
+   return 1;
+}
+
+TEST(event_loop_timer_updates)
+{
+   struct wl_event_loop *loop = wl_event_loop_create();
+   struct wl_event_source *source;
+   struct timer_update_context context;
+   struct timeval start_time, end_time, interval;
+
+   /* Create two timers that should expire at the same time (after 10ms).
+* The first timer to receive its expiry callback updates the other 
timer
+* with a much larger timeout (1s). This highlights a bug where
+* wl_event_source_timer_dispatch would block for this larger timeout
+* when reading from the timer fd, before calling the second timer's
+* callback.
+*/
+
+   context.source1 = wl_event_loop_add_timer(loop, timer_update_callback_1,
+   
  &context);
+   assert(context.source1);
+   wl_event_source_timer_update(context.source1, 10);
+
+   context.source2 = wl_event_loop_add_timer(loop, timer_update_callback_2,
+   
  &context);
+   assert(context.source2);
+   wl_event_source_timer_update(context.source2, 10);
+
+   context.count = 0;
+
+   gettimeofday(&start_time, NULL);
+   wl_event_loop_dispatch(loop, 20);
+   gettimeofday(&end_time, NULL);
+
+   assert(context.count == 2);
+
+   /* Dispatching the events should not have taken much more than 20ms,
+* since this is the timeout passed to wl_event_loop_dispatch. If it
+* blocked, then it will have taken over 1s.
+* Of course, it could take over 1s anyway on a very slow or heavily
+* loaded system, so this test isn't 100% perfect.
+*/
+
+   timersub(&end_time, &start_time, &interval);
+   assert(interval.tv_sec < 1);
+
+   wl_event_source_remove(context.source1);
+   wl_event_source_remove(context.source2);
+   wl_event_loop_destroy(loop);
+}
+
 struct event_loop_destroy_listener {
struct wl_listener listener;
int done;
-- 
1.9.2

___
way

Re: [PATCH v3] shell: support window resizing using touchscreen

[Stanislav Vorobiov]

Ping

On 04/23/2014 05:41 PM, Jason Ekstrand wrote:
> 
> 
> 
> On Wed, Apr 23, 2014 at 1:02 AM, Stanislav Vorobiov  > wrote:
> 
> if the system doesn't have a pointer device
> common_surface_resize will crash on
> accessing seat->pointer->button_count. if the system
> does have a pointer device, but attempts to resize
> a window using touchscreen - nothing happens. here
> we implement separate window resizing path for
> seat->touch as it is done in common_surface_move
> ---
>  clients/window.c  |5 +-
>  desktop-shell/shell.c |  153 
> ++---
>  shared/cairo-util.h   |2 +-
>  shared/frame.c|   20 ---
>  4 files changed, 159 insertions(+), 21 deletions(-)
> 
> diff --git a/clients/window.c b/clients/window.c
> index e770a04..185fe23 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -2409,9 +2409,10 @@ frame_touch_down_handler(struct widget *widget, 
> struct input *input,
>  float x, float y, void *data)
>  {
> struct window_frame *frame = data;
> +   enum theme_location location;
> 
> -   frame_touch_down(frame->frame, input, id, x, y);
> -   frame_handle_status(frame, input, time, 
> THEME_LOCATION_CLIENT_AREA);
> +   location = frame_touch_down(frame->frame, input, id, x, y);
> +   frame_handle_status(frame, input, time, location);
>  }
> 
>  static void
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index bc4a258..23125af 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -1581,6 +1581,14 @@ struct weston_resize_grab {
> int32_t width, height;
>  };
> 
> +struct weston_touch_resize_grab {
> +   struct shell_touch_grab base;
> +   int active;
> +   uint32_t edges;
> +   int32_t width, height;
> +   wl_fixed_t dx, dy;
> +};
> +
>  static void
>  resize_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
>wl_fixed_t x, wl_fixed_t y)
> @@ -1668,6 +1676,84 @@ static const struct weston_pointer_grab_interface 
> resize_grab_interface = {
> resize_grab_cancel,
>  };
> 
> +static void
> +touch_resize_grab_down(struct weston_touch_grab *grab, uint32_t time,
> +int touch_id, wl_fixed_t sx, wl_fixed_t sy)
> +{
> +}
> +
> +static void
> +touch_resize_grab_up(struct weston_touch_grab *grab, uint32_t time, int 
> touch_id)
> +{
> +   struct weston_touch_resize_grab *resize =
> +   (struct weston_touch_resize_grab *) container_of(
> +   grab, struct shell_touch_grab, grab);
> +
> +   if (touch_id == 0)
> +   resize->active = 0;
> +
> +   if (grab->touch->num_tp == 0) {
> +   shell_touch_grab_end(&resize->base);
> +   free(resize);
> +   }
> +}
> +
> +static void
> +touch_resize_grab_motion(struct weston_touch_grab *grab, uint32_t time,
> +  int touch_id, wl_fixed_t sx, wl_fixed_t sy)
> +{
> +   struct weston_touch_resize_grab *resize = (struct 
> weston_touch_resize_grab *) grab;
> +   struct weston_touch *touch = grab->touch;
> +   struct shell_surface *shsurf = resize->base.shsurf;
> +   int32_t width, height;
> +   wl_fixed_t from_x, from_y;
> +   wl_fixed_t to_x, to_y;
> +
> +   if (!shsurf || !resize->active)
> +   return;
> +
> +   weston_view_from_global_fixed(shsurf->view,
> + resize->dx, resize->dy,
> + &from_x, &from_y);
> +   weston_view_from_global_fixed(shsurf->view,
> + touch->grab_x, touch->grab_y, 
> &to_x, &to_y);
> +
> +   width = resize->width;
> +   if (resize->edges & WL_SHELL_SURFACE_RESIZE_LEFT) {
> +   width += wl_fixed_to_int(from_x - to_x);
> +   } else if (resize->edges & WL_SHELL_SURFACE_RESIZE_RIGHT) {
> +   width += wl_fixed_to_int(to_x - from_x);
> +   }
> +
> +   height = resize->height;
> +   if (resize->edges & WL_SHELL_SURFACE_RESIZE_TOP) {
> +   height += wl_fixed_to_int(from_y - to_y);
> +   } else if (resize->edges & WL_SHELL_SURFACE_RESIZE_BOTTOM) {
> +   height += wl_fixed_to_int(to_y - from_y);
> +   }
> +
> +   shsurf->client->send_configure(shsurf->surface,
> +  resize->edges, width, height);
> +}
> +
> +static void
> +touch_resize_grab_cancel(struct weston_touch_grab *grab)
> +{
> 

Re: [PATCH weston-ivi-shell v4 7/9] Add refernce image files for ivi-hmi-controller

[Pekka Paalanen]

On Mon, 17 Mar 2014 15:29:56 +0900
Nobuhiko Tanibata  wrote:

> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4
>  - nothing. Version number aligned to the first patch
> 
>  data/Makefile.am |  14 +-
>  data/background.png  | Bin 0 -> 245579 bytes

You could use a simpler and smaller placeholder background, I guess.

>  data/fullscreen.png  | Bin 0 -> 3406 bytes
>  data/home.png| Bin 0 -> 4629 bytes
>  data/icon_ivi_clickdot.png   | Bin 0 -> 39523 bytes
>  data/icon_ivi_flower.png | Bin 0 -> 24475 bytes
>  data/icon_ivi_simple-egl.png | Bin 0 -> 29316 bytes
>  data/icon_ivi_simple-shm.png | Bin 0 -> 71120 bytes
>  data/icon_ivi_smoke.png  | Bin 0 -> 46577 bytes

These icons seems to be pretty big...

>  data/panel.png   | Bin 0 -> 41955 bytes
>  data/random.png  | Bin 0 -> 4891 bytes
>  data/sidebyside.png  | Bin 0 -> 3929 bytes
>  data/tiling.png  | Bin 0 -> 5620 bytes
>  13 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100644 data/background.png
>  create mode 100644 data/fullscreen.png
>  create mode 100644 data/home.png
>  create mode 100644 data/icon_ivi_clickdot.png
>  create mode 100644 data/icon_ivi_flower.png
>  create mode 100644 data/icon_ivi_simple-egl.png
>  create mode 100644 data/icon_ivi_simple-shm.png
>  create mode 100644 data/icon_ivi_smoke.png
>  create mode 100644 data/panel.png
>  create mode 100644 data/random.png
>  create mode 100644 data/sidebyside.png
>  create mode 100644 data/tiling.png
> 
> diff --git a/data/Makefile.am b/data/Makefile.am
> index a7cc944..2aa6e5c 100644
> --- a/data/Makefile.am
> +++ b/data/Makefile.am
> @@ -9,7 +9,19 @@ dist_westondata_DATA =   \
>   icon_window.png \
>   sign_close.png  \
>   sign_maximize.png   \
> - sign_minimize.png
> + sign_minimize.png   \
> + background.png  \
> + tiling.png  \
> + fullscreen.png  \
> + panel.png   \
> + random.png  \
> + sidebyside.png  \
> + home.png\
> + icon_ivi_clickdot.png   \
> + icon_ivi_flower.png \
> + icon_ivi_simple-egl.png \
> + icon_ivi_simple-shm.png \
> + icon_ivi_smoke.png
>  
>  if HAVE_RSVG_CONVERT
>  wayland_icon_png = wayland.png
> diff --git a/data/background.png b/data/background.png
> new file mode 100644
> index 
> ..60c317c945ae3a8c9f0875cc59a80fd248692fac
> GIT binary patch
> literal 245579

Hi,

posting binary patches to the mailing list is slightly useless, because
the content cannot really be reviewed by just reading the email. It is
very good, that you separated this into a patch of its own.

In the future, I would suggest you skip sending this patch to the
mailing list or remove all the binary stuff and leave just the plain
text part like I did in this reply, and publish a git branch with the
complete series.

Another benefit of a git branch is that we can easily see your
branching point, when coming back to it much later.


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

Re: [PATCH weston-ivi-shell v4 4/9] A reference protocol of ivi hmi controller to set up IVI style UI.

[Pekka Paalanen]

On Mon, 17 Mar 2014 15:27:46 +0900
Nobuhiko Tanibata  wrote:

> The reference protocol is used between hmi-controller and
> hmi-controller-homescreen.

A one paragraph explanation on what hmi-controller and
hmi-controller-homescreen each are would be nice, maybe added to the
XML itself.

I'm a bit lost here now.

> 
> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - squash Makefile to this patch
> 
> Changes for v3 and v4
>   - nothing. Version number aligned to the first patch
> 
>  protocol/Makefile.am|   3 +-
>  protocol/ivi-hmi-controller.xml | 105 
> 
>  2 files changed, 107 insertions(+), 1 deletion(-)
>  create mode 100644 protocol/ivi-hmi-controller.xml
> 
> diff --git a/protocol/Makefile.am b/protocol/Makefile.am
> index 9913f16..140aef5 100644
> --- a/protocol/Makefile.am
> +++ b/protocol/Makefile.am
> @@ -9,7 +9,8 @@ protocol_sources =\
>   wayland-test.xml\
>   xdg-shell.xml   \
>   scaler.xml  \
> - ivi-application.xml
> + ivi-application.xml \
> + ivi-hmi-controller.xml
>  
>  if HAVE_XMLLINT
>  .PHONY: validate
> diff --git a/protocol/ivi-hmi-controller.xml b/protocol/ivi-hmi-controller.xml
> new file mode 100644
> index 000..04e22f4
> --- /dev/null
> +++ b/protocol/ivi-hmi-controller.xml
> @@ -0,0 +1,105 @@
> +
> +
> +
> +
> +Copyright (C) 2013 DENSO CORPORATION
> +
> +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 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.
> +
> +
> +
> +
> +Currently it's possible to set up surface for background, panel,
> +buttons, and workspace.
> +
> +
> +
> +
> +

It is highly unusual for a Wayland extension work by ids like this. Is
the intention that one client creates the surface with a known
surface_id, and then another client using this interface says, that
that surface_id is actually a background?

Weren't the surface_ids determined in advance during the system design
phase, and then hardcoded constants in the software? Wouldn't
ivi-shell.so just read these constants from a file, or even hardcode
them?

That is to say, I do not understand the purpose of this interface just
by looking at the protocol specification. I feel the specification
should explain it.

If this is an attempt to replicate some features of the desktop_shell
extension, why not just use wl_surface instead of uint?

> +
> +
> +
> +
> +

What happens, if several set_* requests are made with the same srf_id?
What if they are different set_* requests with the same srf_id?
Does srf_id need to have assigned a surface already, or can it be done
later?

> +
> +
> +
> +
> +Several buttons are regitered on panel by using arg; number.
> +
> +
> +

What if the 'number' was already used?
What does 'number' mean, anyway?
What if a client uses an arbitrarily large or zero 'number'?

> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

How do you know what size each surface should be?
Would you ever want to have output-specific assignments, like a
different background on each output?

> +
> +
> +
> +
> +Per calling this request, a group of launchers are added.
> +
> +
> +

What is icon_size doing here?
Is it a request for whatever is providing the surfaces associated to
srf_ids, that they should be of this size? Or is it a request for the
compositor to scale all these surfaces to that size?

> +
> +
> +
> +
> +Give seat to the server to be controlled by server

Re: [PATCH weston-ivi-shell v4 3/9] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Pekka Paalanen]

Hi,

btw. pay attention to patch summaries (the email subject line). We use
a "component: description" format. Some examples:

"ivi application protocol:" ->
protocol: add ivi application extension

"The weston-layout library supports" ->
ivi-shell: add weston-layout library

"ivi-shell supports a type of shell for In-Vehicle Infotainment
system." ->
ivi-shell: add the shell plugin

etc.

Reading the weston git log should give an idea of how to write it.


On Mon, 17 Mar 2014 15:26:38 +0900
Nobuhiko Tanibata  wrote:

> In-Vehicle Infotainment system traditionally manages surfaces with global
> identification. A protocol, ivi_application, supports such a feature by
> implementing a request, ivi_application::surface_creation defined in
> ivi_application.xml. Additionally, it initialize a library, weston-layout,
> to manage properties of surfaces and group surfaces in layer. In detail,
> refer library, weston_layout.
> 
> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>  - apply review comments of mailing list.
>  - squash update of Makefile into this patch.
>  - move this patch after patch of weston-layout.
>  - support inherit propoerties of id_surface when client attaches another
>wl_surface with id_surface after destroying ivi_surface once.
> 
> Changes for v3:
>  - squash internal method, configure, to ivi_shell_surface_configure.
> 
> Changes for v4:
>  - nothing. Version number aligned to the first patch
>  
>  ivi-shell/Makefile.am |  24 +++-
>  ivi-shell/ivi-shell.c | 314 
> ++
>  2 files changed, 337 insertions(+), 1 deletion(-)
>  create mode 100644 ivi-shell/ivi-shell.c
> 
> diff --git a/ivi-shell/Makefile.am b/ivi-shell/Makefile.am
> index 4d54c2d..d0c0d62 100644
> --- a/ivi-shell/Makefile.am
> +++ b/ivi-shell/Makefile.am
> @@ -1,7 +1,8 @@
>  moduledir = $(libdir)/weston
>  
>  module_LTLIBRARIES = \
> - $(libweston_layout)
> + $(libweston_layout) \
> + $(ivi_shell)
>  
>  AM_CPPFLAGS =\
>   -I$(top_srcdir)/shared  \
> @@ -17,6 +18,7 @@ westoninclude_HEADERS =
>  
>  if ENABLE_IVI_SHELL
>  westoninclude_HEADERS += \
> + ivi-application-client-protocol.h   \

I'm not sure it makes sense to export the client protocol header,
because client programs do not get a library to link to for getting the
ivi-application-protocol.c part.

>   weston-layout.h
>  
>  libweston_layout = libweston-layout.la
> @@ -27,4 +29,24 @@ libweston_layout_la_SOURCES =  \
>   weston-layout.c \
>   weston-layout.h
>  
> +ivi_shell = ivi-shell.la
> +ivi_shell_la_LDFLAGS = -module -avoid-version
> +ivi_shell_la_LIBADD = $(COMPOSITOR_LIBS) $(IVI_SHELL_LIBS) 
> ./libweston-layout.la
> +ivi_shell_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(IVI_SHELL_CFLAGS)
> +ivi_shell_la_SOURCES =   \
> + ivi-shell.c \
> + weston-layout.h \

This is one more hint, that weston-layout.so should not be a library,
but a part of ivi-shell.so. You link the local build of
weston-layout.so into ivi-shell.so during the build.

Maybe you meant libweston-layout.la to be a convenience library instead?
Not sure that is needed either, nothing else will link to it I believe.
I think ivi-shell.so could export the weston-layout API just like the
weston executable exports the weston core API to plugins.

By all means do keep the code in weston-layout.c in a separate file if
you want, but weston-layout.h would be split into two files: one
private to ivi-shell.so, another the exported API for stuff like
ivi-controller.so. I think a sensible split between weston-layout.c and
ivi-shell.c would be based on ivi-shell.c handling mostly Weston core
callbacks, and weston-layout.c handling the public API.

There is the difference between static, non-static, and WL_EXPORT
symbols.

> + ivi-application-protocol.c  \
> + ivi-application-server-protocol.h
> +
>  endif
> +
> +BUILT_SOURCES =  \
> + ivi-application-protocol.c  \
> + ivi-application-server-protocol.h   \
> + ivi-application-client-protocol.h
> +
> +CLEANFILES = $(BUILT_SOURCES)
> +
> +wayland_protocoldir = $(top_srcdir)/protocol
> +include $(top_srcdir)/wayland-scanner.mk
> diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
> new file mode 100644
> index 000..31d39cc
> --- /dev/null
> +++ b/ivi-shell/ivi-shell.c
> @@ -0,0 +1,314 @@
> +/*
> + * Copyright (C) 2013 DENSO CORPORATION
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and
> + * its documentation for any purpose is hereby granted without fee, provided
> + * that the above copyright notice appear in all copies and that both that
> + * copyright notice and this permissio

Re: [PATCH weston-ivi-shell v5 2/9] The weston-layout library supports

[Pekka Paalanen]

On Thu, 20 Mar 2014 15:59:34 +0900
Nobuhiko Tanibata  wrote:

> API set of controlling properties of surface and layer which groups
> surfaces. An unique ID whose type is integer is required to create
> surface and layer. With the unique ID, surface and layer are identified
> to control them. The API set consists of APIs to control properties of
> surface and layers about followings,
> 
> - visibility.
> - opacity.
> - clipping (x,y,width,height).
> - position and size of it to be displayed.
> - orientation per 90 degree.
> - add or remove surfaces to a layer.
> - order of surfaces/layers in layer/screen to be displayed.
> - commit to apply property changes.
> - notifications of property change.
> 
> Signed-off-by: Nobuhiko Tanibata 
> ---
> 
> Changes for v2:
>   - move this patch in front of ivi-shell patch to be compiled successfully.
>   - unsupport weston_layout_takeSurfaceScreenshot because implementation 
> needs to
> be discussed more. It is pending.
>   - support inherit propoerties of id_surface when client attaches another
> wl_surface with id_surface after destroying ivi_surface once.
>   - bug fix of https://bugs.tizen.org/jira/browse/TIVI-2882
> 
> Changes for v3 and v4:
>   - nothing. Version number aligned to the first patch
> 
> Changes for v5:
>   - bug fix of https://bugs.tizen.org/jira/browse/TIVI-2881
> 
>  Makefile.am   |1 +
>  configure.ac  |   15 +-
>  ivi-shell/Makefile.am |   30 +
>  ivi-shell/weston-layout.c | 2639 
> +
>  ivi-shell/weston-layout.h |  934 
>  5 files changed, 3618 insertions(+), 1 deletion(-)
>  create mode 100644 ivi-shell/Makefile.am
>  create mode 100644 ivi-shell/weston-layout.c
>  create mode 100644 ivi-shell/weston-layout.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index f22c542..1bc35e2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -11,6 +11,7 @@ SUBDIRS =   \
>   src \
>   $(xwayland_subdir)  \
>   desktop-shell   \
> + ivi-shell   \
>   clients \
>   data\
>   protocol\

Weston recently moved to monolithic Makefile, so the automake stuff
will need porting.

> diff --git a/configure.ac b/configure.ac
> index cce1850..4c0a90f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -409,6 +409,16 @@ if test "x$enable_dbus" != "xno"; then
>  fi
>  AM_CONDITIONAL(ENABLE_DBUS, test "x$enable_dbus" = "xyes")
>  
> +# ivi-shell support
> +AC_ARG_ENABLE(ivi-shell,
> +  AS_HELP_STRING([--disable-ivi-shell],
> + [do not build ivi-shell server plugin and 
> client]),,
> +   enable_ivi_shell=yes)
> +AM_CONDITIONAL(ENABLE_IVI_SHELL, test "x$enable_ivi_shell" = "xyes")
> +if test x$enable_ivi_shell = xyes; then
> +  PKG_CHECK_MODULES(IVI_SHELL, [cairo])

Only cairo? Or maybe cairo-image?

Hmm, I didn't see this patch add any Cairo calls.

> +fi
> +
>  AC_ARG_ENABLE(wcap-tools, [  --disable-wcap-tools],, enable_wcap_tools=yes)
>  AM_CONDITIONAL(BUILD_WCAP_TOOLS, test x$enable_wcap_tools = xyes)
>  if test x$enable_wcap_tools = xyes; then
> @@ -505,7 +515,8 @@ AC_CONFIG_FILES([Makefile
>data/Makefile
>protocol/Makefile
>man/Makefile
> -  tests/Makefile])
> +  tests/Makefile
> +  ivi-shell/Makefile])
>  AC_OUTPUT
>  
>  AC_MSG_RESULT([
> @@ -519,6 +530,8 @@ AC_MSG_RESULT([
>   XWayland${enable_xwayland}
>   dbus${enable_dbus}
>  
> + ivi-shell   ${enable_ivi_shell}
> +
>   Build wcap utility  ${enable_wcap_tools}
>  
>   weston-launch utility   ${enable_weston_launch}
> diff --git a/ivi-shell/Makefile.am b/ivi-shell/Makefile.am
> new file mode 100644
> index 000..4d54c2d
> --- /dev/null
> +++ b/ivi-shell/Makefile.am
> @@ -0,0 +1,30 @@
> +moduledir = $(libdir)/weston
> +
> +module_LTLIBRARIES = \
> + $(libweston_layout)
> +
> +AM_CPPFLAGS =\
> + -I$(top_srcdir)/shared  \
> + -I$(top_srcdir)/src \
> + -I$(top_builddir)/src   \
> + -DDATADIR='"$(datadir)"'\
> + -DMODULEDIR='"$(moduledir)"'\
> + -DLIBEXECDIR='"$(libexecdir)"'  \
> + -DIN_WESTON
> +
> +westonincludedir = $(includedir)/weston
> +westoninclude_HEADERS =
> +
> +if ENABLE_IVI_SHELL
> +westoninclude_HEADERS += \
> + weston-layout.h
> +
> +libweston_layout = libweston-layout.la
> +libweston_layout_la_LDFLAGS = -avoid-version

This is not a Weston plugin, right?
You intend other stuff to link 

Re: [PATCH libinput 10/20] touchpad: Add tp_button_touch_active function

[Jonas Ådahl]

On Fri, Apr 25, 2014 at 08:27:17AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 04/24/2014 09:16 PM, Jonas Ådahl wrote:
> > On Tue, Apr 15, 2014 at 02:28:07PM +0200, Hans de Goede wrote:
> >> We don't want touches in the button area to cause the pointer to move, add
> >> a tp_button_touch_active function which the main code in evdev-mt-touchpad
> >> can call to see if a touch should be consider a candidate for being the
> >> pointer, should be taken into account for 2 finger scrolling, etc.
> >>
> >> The idea behind the main code polling for this is that in the future with
> >> ie edge scrolling we will have another independent state machine, which
> >> may also want to block a touch from being the pointer, so it is best for
> >> the main code to test all independent state machines, rather then having
> >> the state-machines poke the is_pointer variabel directly.
> >>
> >> Signed-off-by: Hans de Goede 
> >> Acked-by: Peter Hutterer 
> >> ---
> >>  src/evdev-mt-touchpad-buttons.c | 5 +
> >>  src/evdev-mt-touchpad.h | 3 +++
> >>  2 files changed, 8 insertions(+)
> >>
> >> diff --git a/src/evdev-mt-touchpad-buttons.c 
> >> b/src/evdev-mt-touchpad-buttons.c
> >> index f953cd1..e789a87 100644
> >> --- a/src/evdev-mt-touchpad-buttons.c
> >> +++ b/src/evdev-mt-touchpad-buttons.c
> >> @@ -602,3 +602,8 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t 
> >> time)
> >>return rc;
> >>  }
> >>  
> >> +int
> >> +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t)
> >> +{
> >> +  return t->button.state == BUTTON_STATE_AREA;
> >> +}
> >> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> >> index 8d8dd84..04da6a6 100644
> >> --- a/src/evdev-mt-touchpad.h
> >> +++ b/src/evdev-mt-touchpad.h
> >> @@ -229,4 +229,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t 
> >> time);
> >>  int
> >>  tp_button_handle_state(struct tp_dispatch *tp, uint32_t time);
> >>  
> >> +int
> >> +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t);
> >> +
> > 
> > I think the naming here is a bit inconsistent, as a function in the
> > following patch with the same naming convention (tp_touch_active()) is a
> > get:er, not a set:er.
> 
> This is a get-er too.

Ah. Thats interesting. Seems I misread the patch. Thanks for the
clarifications.

> 
> > IMO functions with side effects should have that
> > detail in their name some how, for example in this case _set_active() or
> > _activate().
> > 
> > Also this type of patch fits better as just a detail of the following
> > patch. That is in order to know how the intended use is without having
> > to jump between patches.
> 
> Ok, lets squash the 2 in the next version.
> 
> Regards,
> 
> Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel