[PATCH weston 00/14] IVI Layout API Cleanup

Tue Mar 1 12:47:40 UTC 2016

Hello Bryce-san, Pekka-san,

My understanding is that cleanup is required, if matured. The point is 
anyone using ivi-shell can know the cleanup and replace APIs/ABIs.
However due to the discussion below of situation around ivi-shell, this 
removal seems not to confuse most users. So Emre-san's patch looks fine 
with me.

Thanks,
Wataru Natsume

On 2016-03-01 08:38, Bryce Harrington wrote:
> On Mon, Feb 29, 2016 at 03:31:46PM +0000, Ucan, Emre (ADITG/SW1) wrote:
>> Hi Pekka,
>> 
>> Best regards
>> 
>> > -----Original Message-----
>> > From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
>> > Sent: Montag, 29. Februar 2016 16:15
>> > To: Ucan, Emre (ADITG/SW1); Wataru Natsume
>> > Cc: Bryce Harrington; wayland-devel at lists.freedesktop.org
>> > Subject: Re: [PATCH weston 00/14] IVI Layout API Cleanup
>> >
>> > On Mon, 29 Feb 2016 08:04:05 +0000
>> > "Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:
>> >
>> > > Hello Bryce,
>> > >
>> > > As far as I know, there are two controller plugins which are using ivi
>> > > layout interface. 1. HMI controller
>> > > 	2. IVI controller in Genivi Wayland IVI Extension
>> > >
>> > > I updated the hmi controller for these changes, and ivi controller
>> > > does not use these APIs.
>> > >
>> > > Furthermore, IVI Layout APIs are internal. It is quite often that
>> > > weston plugins gets break after a major release because of either API
>> > > changes or data struct changes.
>> >
>> > Hi,
>> >
>> > Weston plugins can break on every minor bump, yes, but here we are talking
>> > about controller plugins which might follow different rules set by the
>> > ivi_layout ABI.
>> >
>> > However, as long as controller plugins also use Weston ABI in addition to
>> > ivi_layout ABI, they are susceptible to break as often as Weston plugins.
>> >
>> > > For example, we have an input plugin in Wayland IVI Extension which
>> > > replaces the default grab interfaces. The plugin does not compile with
>> > > 1.10 weston because weston_pointer data struct is changed after 1.9.
>> >
>> > I presume that is a Weston plugin.
>> 
>> Yes
>> 
>> >
>> > > > -----Original Message-----
>> > > > From: Bryce Harrington [mailto:bryce at osg.samsung.com]
>> > > > Sent: Freitag, 26. Februar 2016 19:03
>> > > > To: Ucan, Emre (ADITG/SW1)
>> > > > Cc: wayland-devel at lists.freedesktop.org
>> > > > Subject: Re: [PATCH weston 00/14] IVI Layout API Cleanup
>> > > >
>> > > > On Fri, Feb 26, 2016 at 03:57:56PM +0000, Ucan, Emre (ADITG/SW1)
>> > > > wrote:
>> > > > > I removed the get APIs, because the same information can be get
>> > > > > from ivi_layout_get_properties_of_surface/layer APIs. Therefore,
>> > > > > these APIs are redundant.
>> > > >
>> > > > Looks like a good cleanup, but do we have any concerns about API
>> > > > stability in dropping these getter/setters?
>> >
>> > Even though I called for it several times, it seems the ABI stability guarantees
>> > were never documented anywhere, so I suppose the standard Weston
>> > plugin rules apply. At least on a quick glance I cannot find any comment to
>> > that effect.
>> 
>> I think the stability rules for IVI layout should be the same as 
>> standart weston plugins.
>> We cannot realize some usecases with ivi-shell, which we want to have, 
>> e.g: same ivi-surface on many layers/screens.
>> We have to most likely break ABI again to realize these usecases.
>> 
>> Where is a good place to write down the ABI stability rules, do you 
>> think ? ivi-shell/README, maybe ?
> 
> I don't know if there is a convention for where ABI stability rules are
> indicated, but that would probably be the first place I'd check.
> 
>> > The controller plugin API does however use the size of struct
>> > ivi_layout_interface to detect API compatiblity. Now that the struct grows
>> > smaller, this check does not work, yet the ABI does break, and requires at
>> > least rebuilding all controller plugins. If we want to maintain the ABI, the
>> > fields in struct ivi_layout_interface should be set to NULL or a generic
>> > function that complains and aborts, not removed.
>> >
>> > Natsume-san, do you have an opinion?
>> >
>> > From my behalf this is a welcome simplification, so consider the whole series
>> > Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk> anyway.
> 
> Thanks for the clarifications; as I mentioned the changes themselves
> look technically fine, so given the clarification on the ABI situation,
> I can add my:
> 
> Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
> 
> Holding off on landing until we hear confirmation from Natsume-san.
> 
> Bryce
> 
>> > Thanks,
>> > pq
>> >
>> > > > > Furthermore, I removed *_set_position/dimension APIs, because
>> > > > > position and dimension can be set by
>> > > > > ivi_layout_surface/layer_set_destination_rectangle APIs.
>> > > > >
>> > > > > I adjusted ivi-layout-transition.c, ivi shell test code and
>> > > > > hmi-controller.c for these changes.
>> > > > >
>> > > > > Emre Ucan (14):
>> > > > >   ivi-shell: remove ivi_layout_surface_get_visibility API
>> > > > >   ivi-shell: remove ivi_layout_layer_get_visibility API
>> > > > >   ivi-shell: remove ivi_layout_surface_get_opacity API
>> > > > >   ivi-shell: remove ivi_layout_layer_get_opacity API
>> > > > >   ivi-shell: remove ivi_layout_surface_get_position API
>> > > > >   ivi-shell: remove ivi_layout_layer_get_position API
>> > > > >   ivi-shell: remove ivi_layout_surface_get_dimension API
>> > > > >   ivi-shell: remove ivi_layout_layer_get_dimension API
>> > > > >   ivi-shell: remove ivi_layout_surface_get_orientation API
>> > > > >   ivi-shell: remove ivi_layout_layer_get_orientation API
>> > > > >   ivi-shell: remove ivi_layout_surface_set_position API
>> > > > >   ivi-shell: remove ivi_layout_layer_set_position API
>> > > > >   ivi-shell: remove ivi_layout_surface_set_dimension API
>> > > > >   ivi-shell: remove ivi_layout_layer_set_dimension API
>> > > > >
>> > > > >  ivi-shell/hmi-controller.c        |   17 ++-
>> > > > >  ivi-shell/ivi-layout-export.h     |  127 --------------------
>> > > > >  ivi-shell/ivi-layout-private.h    |   17 +--
>> > > > >  ivi-shell/ivi-layout-transition.c |   19 ++-
>> > > > >  ivi-shell/ivi-layout.c            |  237
>> > > > > +------------------------------------
>> > > > > ivi-shell/ivi-shell.c             |    7 +-
>> > > > > tests/ivi_layout-internal-test.c  |  220
>> > > > > +++++-----------------------------
>> > > > > tests/ivi_layout-test-plugin.c    |  126 +++++--------------- 8
>> > > > > files changed, 85 insertions(+), 685 deletions(-)
>> > > > >
>> > > > > --
>> > > > > 1.7.9.5
>> 
>> Emre Ucan
>> Software Group I (ADITG/SW1)
>> 
>> Tel. +49 5121 49 6937