[PATCH weston 00/16] IVI Layout Notification APIs Rework

[Pekka Paalanen]

On Thu, 31 Mar 2016 11:08:46 +0000
"Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:

> I changed the interface of the notification APIs to get a wl_listener
> instead of a ivi-shell specific notification callback function.
> 
> This change has several advantages:
> 1. Code cleanup
> 2. No dynamic memory allocation. Listeners are allocated
>    by controller plugins
> 3. Remove API is not needed. Controller plugins can easily
>    remove the listener link.
> 
> The first patch is a preparation for the notification API changes.
> It moves the event_mask to ivi_layout_surface/layer_properties struct,
> so that it is accessible by get_properties_of_layer/surface APIs. Then,
> it is not needed to send event_mask with an ivi-shell callback function.
> 
> The patches 2-15 modifies and renames IVI Layout notification APIs, and
> removes their remove APIs.
> 
> The 16th patch removes content_observer callback function which is a
> leftover of the patch: 193e301c740b582f20307e3e08f8373d26ea968f
> 
> Emre Ucan (16):
>   ivi-shell: move event_mask to properties struct
>   ivi-shell: rework surface_add_notification API
>   ivi-shell: remove surface_remove_notification APIs
>   ivi-shell: rework layer_add_notification API
>   ivi-shell: remove layer_remove_notification APIs
>   ivi-shell: rework create_surface notification
>   ivi-shell: remove remove_notification_create_surface
>   ivi-shell: rework create_layer_notification
>   ivi-shell: remove remove_notification_create_layer
>   ivi-shell: rework remove_layer notification
>   ivi-shell: remove remove_notification_remove_layer
>   ivi-shell: rework remove_surface notification
>   ivi-shell: remove remove_notification_remove_surface
>   ivi-shell: rework configure_surface notification
>   ivi-shell: remove remove_notification_configure_surface
>   ivi-shell: remove content_observer leftover
> 
>  ivi-shell/hmi-controller.c       |   42 +--
>  ivi-shell/ivi-layout-export.h    |  113 +-------
>  ivi-shell/ivi-layout-private.h   |    2 -
>  ivi-shell/ivi-layout.c           |  545 +++++---------------------------------
>  tests/ivi_layout-internal-test.c |   76 +++---
>  tests/ivi_layout-test-plugin.c   |   93 ++++---
>  6 files changed, 209 insertions(+), 662 deletions(-)
> 

Hi,

I had separate comments on patch 2, which probably needs some bits of
patch 3.

Somehow I would assume that patch 4 would have a similar issue with
ivi_layout_layer_remove_notification() call being removed only in patch
5, but it didn't trigger for me.

Considering the add and remove notifier/listener are two sides of the
same thing, maybe it would be logical to do both in the same patch?
That is, squash 2 and 3, 4 and 5, etc.?

For all the add_listener public API functions you should document what
the void *data argument to the wl_listener::notify callback is.

In the great assignment block of static struct ivi_layout_interface
ivi_layout_interface it is ok to keep a comma also after the last item.
It helps keeping diffs one change smaller.

It is not needed to add a cast to/from void*.

Patches 1 and 16 are R-b me and pushed:
   87953b7..0bd29b6  master -> master

Patches 2 - 15 I hope to see revised.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160401/25486d94/attachment-0001.sig>