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:[email protected]]
> Sent: Montag, 29. Februar 2016 16:15
> To: Ucan, Emre (ADITG/SW1); Wataru Natsume
> Cc: Bryce Harrington; [email protected]
> Subject: Re: [PATCH weston 00/14] IVI Layout API Cleanup
>
> On Mon, 29 Feb 2016 08:04:05 +0000
> "Ucan, Emre (ADITG/SW1)" <[email protected]> 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:[email protected]]
> > > Sent: Freitag, 26. Februar 2016 19:03
> > > To: Ucan, Emre (ADITG/SW1)
> > > Cc: [email protected]
> > > 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 <[email protected]> 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 <[email protected]>
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
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
