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

[wataru_natsume]

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.
> 
> 
> Thanks,
> pq
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel