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

[Pekka Paalanen]

On Fri, 26 Feb 2016 09:28:14 +0900
wataru_natsume <wataru_natsume at xddp.denso.co.jp> wrote:

> Hello Pekka-san,
> 
> 
> Thank you so much for your review and proposal.
> I will send another patch of the warning removal and memory leak which 
> you kindly found during this topic.
> 
> Thanks,
> Wataru Natsume
> 
> On 2016-02-19 20:20, Pekka Paalanen wrote:
> > On Thu, 18 Feb 2016 08:48:40 +0900
> > Wataru Natsume <wataru_natsume at xddp.denso.co.jp> wrote:
> >   
> >> From: Wataru Natsume <WATARU_NATSUME at xddp.denso.co.jp>
> >> 
> >> Previous code cleaned up surfaces in layer once and then added
> >> surfaces to a layer in random. In this flow, two commitchanges are
> >> required.
> >> 
> >> Signed-off-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
> >> [WATARU_NATSUME at xddp.denso.co.jp: Removes unnecessary check]
> >> Signed-off-by: Wataru Natsume <WATARU_NATSUME at xddp.denso.co.jp>
> >> 
> >> ---
> >> Changes from v1 - Removes unnecessary check if the surface is on a 
> >> layer.
> >> 
> >>  ivi-shell/hmi-controller.c |    9 ---------
> >>  1 file changed, 9 deletions(-)
> >> 
> >> diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
> >> index 8da3d3c..ace6555 100644
> >> --- a/ivi-shell/hmi-controller.c
> >> +++ b/ivi-shell/hmi-controller.c
> >> @@ -424,18 +424,9 @@ mode_random_replace(struct hmi_controller 
> >> *hmi_ctrl,
> >> 
> >>  	wl_list_for_each(application_layer, layer_list, link) {
> >>  		layers[layer_idx] = application_layer;
> >> -		ivi_layout_interface->layer_set_render_order(layers[layer_idx]->ivilayer,
> >> -							NULL, 0);
> >>  		layer_idx++;
> >>  	}
> >> 
> >> -	/*
> >> -	 * This commit change is needed because ivisurface can not belongs 
> >> to several layers
> >> -	 * at the same time. So ivisurfaces shall be removed from layers 
> >> once and then set them
> >> -	 * to layers randomly.
> >> -	 */
> >> -	ivi_layout_interface->commit_changes();
> >> -
> >>  	for (i = 0; i < surface_length; i++) {
> >>  		ivisurf = pp_surface[i];
> >>   
> > 
> > Hi Natsume-san,
> > 
> > this looks fine at first, but when testing it, mode_random_replace()
> > will trigger one "ivi_layout_layer_add_surface: addsurf is already
> > available" warning per existing surface.
> > 
> > ivi_layout_layer_add_surface() is checking if the surface is already
> > (current, not the pending state) on the given layer. This is likely
> > because in a previously intended future a surface might be in multiple
> > layers, and adding it multiple times to the same layer is considered a
> > mistake (given how surface positioning works in this ivi-layout API
> > design, that is understandable).
> > 
> > Maybe we should also just remove that check from
> > ivi_layout_layer_add_surface()? I don't see any value from it in the
> > current code base. If Emre adds views as a tying object in the
> > ivi-layout API, this code will get rewritten anyway.
> > 
> > Apart from the harmless log spew, this patch is:
> > Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > If you want to make a patch to remove the warning, I can push the both
> > patches at the same time.

Pushed:
   7e7f793..9d8b441  master -> master


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/20160304/3157532f/attachment-0001.sig>