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

Wed Feb 10 15:56:21 UTC 2016

On Wed, 10 Feb 2016 08:54:47 +0000
"Ucan, Emre (ADITG/SW1)" <eucan at de.adit-jv.com> wrote:

> Hi Pekka,
> 
> My comments are below

Hi Emre

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

Ok, buffer size is a matter of opinion. In my view, buffer size is a
property of the buffer. Then wl_surface has things like
buffer_transform, buffer_scale, and wl_viewport settings, and all these
together produce the surface size. This is all specified by the Wayland
protocol and controlled by the client, so they are read-only in
the compositor.

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

Agreed.

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

Yes, I am familiar with that page. It is mostly doable, just the need
for an intermediate composite could be an issue.

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

Thank you! :-)

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

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/20160210/8c3c586e/attachment.sig>