On Fri, 7 Aug 2015 09:47:02 +0900 Nobuhiko Tanibata <nobuhiko_tanib...@xddp.denso.co.jp> wrote:
> The api, ivi_layout_layer_remove_surface, shall remove a ivi_surface > from a list of ivi_layer. In previous code, there is no trigger to > refresh order of list, removing the ivi_surface, in commit_layer_list. > > To fix this bug, set a mask; IVI_NOTIFICATION_REMOVE in order to trigger > refresh list of surface in commit_layer_list. > > In commit_layer_list, this patch also removes duplicated code in two > conditions for IVI_NOTIFICATION_ADD/REMOVE. > > Signed-off-by: Nobuhiko Tanibata <nobuhiko_tanib...@xddp.denso.co.jp> > --- > v2 changes: > - fix 8 spaces to tab. > - clean up duplicate code in commit_layer_list. > - improve commit message. > > ivi-shell/ivi-layout.c | 28 +++++++++------------------- > 1 file changed, 9 insertions(+), 19 deletions(-) > > diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c > index 2974bb7..1b45003 100644 > --- a/ivi-shell/ivi-layout.c > +++ b/ivi-shell/ivi-layout.c > @@ -812,25 +812,7 @@ commit_layer_list(struct ivi_layout *layout) > if (!(ivilayer->event_mask & > (IVI_NOTIFICATION_ADD | IVI_NOTIFICATION_REMOVE)) ) { > continue; Hi Tanibata-san, using 'continue', there is no need to have an else-branch. This would save one level of indent in the remaining code. > - } > - > - if (ivilayer->event_mask & IVI_NOTIFICATION_REMOVE) { > - wl_list_for_each_safe(ivisurf, next, > - &ivilayer->order.surface_list, order.link) { > - remove_ordersurface_from_layer(ivisurf); > - > - if (!wl_list_empty(&ivisurf->order.link)) { > - wl_list_remove(&ivisurf->order.link); > - } > - > - wl_list_init(&ivisurf->order.link); > - ivisurf->event_mask |= IVI_NOTIFICATION_REMOVE; You are removing this setting of the flag IVI_NOTIFICATION_REMOVE, but in the code that remains I do not see that happening anymore in commit_layer_list(). However, I see it being set by ivi_layout_layer_remove_surface(). At which time should the flag be set? Should the ADD flag not work the same way? Would it be essential to flag ivi-surfaces that get removed from ivilayer->order.surface_list with IVI_NOTIFICATION_REMOVE, and surfaces that are added to flag with IVI_NOTIFICATION_ADD, and all remaining surfaces even if they are reordered not be flagged at all? Or is it intended that all surfaces that are originally in the ivilayer->order.surface_list are flagged as REMOVE, and all surfaces in the pending list are flagged as ADD? That would imply that surfaces that were and still remain in the order list are flagged as both REMOVE and ADD. > - } > - > - wl_list_init(&ivilayer->order.surface_list); > - } > - > - if (ivilayer->event_mask & IVI_NOTIFICATION_ADD) { > + } else { > wl_list_for_each_safe(ivisurf, next, > &ivilayer->order.surface_list, > order.link) { > remove_ordersurface_from_layer(ivisurf); > @@ -843,6 +825,13 @@ commit_layer_list(struct ivi_layout *layout) > } > > wl_list_init(&ivilayer->order.surface_list); > + > + /** > + * Following ivilayer->pending.surface_list must be > maintained by > + * a function who will set these masks. Order of > surfaces in a > + * layer is restructured here. if there is no surface in > + * surface_list, the following code is skipped. > + */ > wl_list_for_each(ivisurf, > &ivilayer->pending.surface_list, > pending.link) { > if(!wl_list_empty(&ivisurf->order.link)){ > @@ -2565,6 +2554,7 @@ ivi_layout_layer_remove_surface(struct ivi_layout_layer > *ivilayer, > } > > remsurf->event_mask |= IVI_NOTIFICATION_REMOVE; > + ivilayer->event_mask |= IVI_NOTIFICATION_REMOVE; > } > > static int32_t All the flagging issues aside, I see what this patch does. Previously removing a ivi-surface didn't work at all. With this patch, whether ivi-surfaces are added or removed from the ivi-layer, the code will always completely reconstruct the ivilayer->order.surface_list from the pending list. In that sense: Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> Do you want me to push this patch as is? Thanks, pq
pgp3VYI0xPmki.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel