[PATCH weston] hmi-controller: remove duplicate commit_changes in random mode

Wed Feb 10 08:54:47 UTC 2016

Hi Pekka,

My comments are below

> -----Original Message-----
> From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
> Sent: Montag, 8. Februar 2016 10:21
> To: Ucan, Emre (ADITG/SW1)
> Cc: Nobuhiko Tanibata; securitycheck at denso.co.jp; Natsume, Wataru
> (ADITJ/SWG); wayland-devel at lists.freedesktop.org; Friedrich, Eugen
> (ADITG/SW1)
> Subject: Re: [PATCH weston] hmi-controller: remove duplicate
> commit_changes in random mode
> 
> On Fri, 5 Feb 2016 21:42:02 +0000
> "Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:
> 
> > Hi Pekka,
> >
> > First Question:
> > An ivi-layer should have properties so that a group of surfaces can be
> > scaled and clipped according to destination and source rectangle of
> > the layer.
> >
> > Second Question:
> > We have requirements for compositing an ivi-surface on many layers
> > and/or screens. But I think it is reasonable to restrict an ivi-layer
> > to a single screen.
> 
> Hi Emre,
> 
> ok, and you are going to introduce ivi-view as a first-class concept in the
> ivi_layout API?
> 
> Below are some things that came to my mind and they are only food for
> thought, not requirements.
> 
> Do I understand the following right?
> 
> ivi-surface:
> - created by Wayland clients (ivi apps) with a specific ivi-id
> - has properties like size, but not position or src/dst rects
> - what about opacity etc.? here or in ivi-view?
> - may be associated with several ivi-views
> - backed by a weston_surface
> 

The surface will have buffer size as property. It will not have opacity and visibility.

> If opacity is not settable by the app, it would make more sense in ivi-view.
> 
> ivi-view:
> - is associated with exactly one ivi-surface
> - created by the window manager system as a Weston/ivi-shell internal
>   object
> - gets content from the ivi-surface
> - links the ivi-surface to an ivi-layer
> - property src rect given in ivi-surface coordinates
> - property dst rect given in ivi-layer coordinate space
> - backed by a weston_view
> 

View will have opacity and visibility. Src rectangle will be used for weston_view_set_mask.
Dst rect will be used to calculate the scaling factor of view. Dst rect is given in ivi-layer coordinate space.
The parts of the view are not shown, which are outside of the dest rect of its ivi-layer. 

> ivi-layer:
> - groups ivi-views into an ordered set
> - created by the window manager system as a Weston/ivi-shell internal
>   object
> - links the set of ivi-views to exactly zero or one ivi-screen
> - property src rect given in ivi-layer coordinate space
> - property dst rect given in ivi-screen coordinate space
> - IOW, maps content from ivi-layer space to ivi-screen space
> - other properties like opacity? (note that correctly implementing
>   opacity will have the same problem as sub-surfaces: you usually need
>   an intermediate composite, which Weston does not implement atm.)
> - may not be backed by any particular Weston core structure

Basically similar to my ideas. I have to think about visibility and opacity.
We have to support the legacy behavior of LayerManagerClient APIs.
Please check this: http://projects.genivi.org/wayland-ivi-extension/layer-manager-apis to understand how ivi-layers and ivi-surfaces should behave.
Most likely, we have to adjust some APIs. But it must be discussed in GENIVI community. We should not break the workflow of the users.
> 
> ivi-screen:
> - represents a single output (monitor)
> - contents specified by multiple ordered ivi-layers
> - no adjustable properties
> - created by the window manager system as a Weston/ivi-shell internal
>   object
> - 1:1 with a weston_output
> 
> In this scheme, you can place an ivi-surface on any ivi-layers at any number of
> times by creating an ivi-view for each. The ivi-view specifies the position, so
> each "clone" of the ivi-surface is completely independently mappable.
> 
> Putting an ivi-layer on multiple ivi-screens can be implemented by creating an
> ivi-layer per ivi-screen, and associating ivi-surfaces with each ivi-layer by
> creating the necessary ivi-views.
> 
> About the ivi_layout API and its implementation:
> 
> I would recommend to have the API use pointers to struct ivi-surface, ivi-
> view, etc. as the primary means of object reference, and leave the ivi-id
> usage just for getting the pointer from an ivi-id and functions creating
> objects. I think most of the API already is this way, but the implementations
> are still matching a lot by ivi-id, when they already have the pointer they are
> looking for.

You are right we do not need to use ivi-ids internally. Pointers will be easier to handle.
I want to get rid of most of ivi_layout_*_get* APIs, because most of the properties are accessible from the ivi_layout_surface/layer/screen_properties data structure.
It is enough to have APIs to get the prop data struct.

> 
> The transition framework seems to be using a timer to get a callback for
> advancing animations. Weston already has an animation framework, but it is
> quite specific to weston_views and weston_spring, so I'm not sure reusing
> that directly makes sense. However, what it does right is hooking up to the
> output repaint cycle instead of using timers or idle callbacks. This make sure
> that animation state is updated exactly the right time, and not too late or too
> often.
> 
> 
> Thanks,
> pq
> 
> > -----Original Message-----
> > From: wayland-devel
> > [mailto:wayland-devel-bounces at lists.freedesktop.org] On Behalf Of
> > Pekka Paalanen Sent: Freitag, 5. Februar 2016 15:08 To: Ucan, Emre
> > (ADITG/SW1) Cc: Nobuhiko Tanibata; securitycheck at denso.co.jp;
> Natsume,
> > Wataru (ADITJ/SWG); wayland-devel at lists.freedesktop.org
> > Subject: Re: [PATCH weston] hmi-controller: remove duplicate
> > commit_changes in random mode
> >
> > On Thu, 4 Feb 2016 16:31:50 +0000
> > "Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:
> >
> > > Hi Pekka,
> > >
> > > I am for removing the remaining bits of code which is wrongly
> > > assuming that a surface can be shown in multiple layers.
> >
> > Hi Emre,
> >
> > sounds good.
> >
> > >  Afterwards we have to redesign a big part of ivi_layout API to
> > > support this feauture.
> > >
> > > My ideas for the redesign so far:
> > > The most basic approach would be that an ivi_surface would have a
> > > weston_view for every layer/screen combination.
> >
> > Yes, I believe that is how it could/should work.
> >
> > > But ivi_layout API
> > > does not make much sense than. Because when controller calls
> > > ivi_layout_surface_set_destination_rectangle, should we scale all
> > > views ? I think it does not make sense to scale all views.
> >
> > Indeed, that was a bit weird, like a surface's position on any layer
> > being recorded with the surface, which means you cannot have per-layer
> > positions. The same with layer vs. screen, IIRC.
> >
> > > Therefore,
> > > the set APIs should be changed to get ivi_views and not
> > > ivi_surfaces. This would mean:
> > >
> > > - Modifying every ivi_layout_surface_set* API
> > > - Implementing new APIs to get layer/screen specific ivi_views for a
> > > given ivi_surface
> > > - ivi-layers should list ivi_views and not ivi_surfaces
> > > - The implementation of commit_changes should be updated
> > > accordingly.
> > >
> > > I am planning to do all these changes, but at first we can remove
> > > old code. Because it is very unlikely that we can reuse.
> >
> > Do you still think that ivi-layer should have a size and a position
> > and all the other attributes? Or should it be used only for grouping
> > and z-ordering ivi-surfaces/views, and linking them to a screen?
> >
> > Could an ivi-layer be on multiple screens? If you can restrict an
> > ivi-layer to a single ivi-screen, I think that would simplify things a
> > lot.
> >
> > It seems you are moving closer to weston's internal architecture with
> > surfaces, views, and layers. :-)
> >
> >
> > Thanks,
> > pq

Best regards

Emre Ucan
Software Group I (ADITG/SW1)

Tel. +49 5121 49 6937