[PATCH weston 00/14] IVI Layout API Cleanup

[Pekka Paalanen]

On Mon, 29 Feb 2016 15:38:41 -0800
Bryce Harrington <bryce at osg.samsung.com> 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.

That, and we also have ivi-layout-export.h which defines the
ivi-layout API we are talking about here. So I'd suggest documenting it
in either place, and adding a comment to the other place referring to
the first.

> > > 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.

Cool, thanks Bryce.


Cheers,
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/20160301/8f566a50/attachment.sig>