On Tue, 17 Jan 2017 12:30:56 +0000 "Ucan, Emre (ADITG/SW1)" <[email protected]> wrote:
> If ivilayer or ivisurf of ivi_view is made invisible in the
> commit_changes call, we have to damage the weston_view below this
> ivi_view. Otherwise content of this ivi_view will stay visible.
Hi Emre,
from a quick check, I think it is indeed correct to call
weston_view_damage_below() when removing the view from the active
scenegraph. weston_view_geometry_dirty() would not be effective,
because the view would never hit weston_view_update_transform() via
weston_output_repaint().
At first sight, the commit message seems to have very little to do with
the full rewrite of commit_changes(). Could you explain and justify the
rewrite in the commit message?
It makes
assert(ivi_view->on_layer == ivilayer);
in update_prop() apparently useless, as we no longer traverse the
ivi-layer's list of ivi-views and so will not check for its consistency.
> Signed-off-by: Emre Ucan <[email protected]>
> ---
> ivi-shell/ivi-layout.c | 41 ++++++++++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index 60d05c4..cc01de3 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -666,28 +666,35 @@ commit_changes(struct ivi_layout *layout)
> {
> struct ivi_layout_screen *iviscrn = NULL;
> struct ivi_layout_layer *ivilayer = NULL;
> + struct ivi_layout_surface *ivisurf = NULL;
> struct ivi_layout_view *ivi_view = NULL;
>
> - wl_list_for_each(iviscrn, &layout->screen_list, link) {
> - wl_list_for_each(ivilayer, &iviscrn->order.layer_list,
> order.link) {
> - /*
> - * If ivilayer is invisible, weston_view of ivisurf
> doesn't
> - * need to be modified.
> - */
> - if (ivilayer->prop.visibility == false)
> - continue;
> + wl_list_for_each(ivi_view, &layout->view_list, link) {
> + ivilayer = ivi_view->on_layer;
> + ivisurf = ivi_view->ivisurf;
> + iviscrn = ivilayer->on_screen;
Does something guarantee that ivilayer cannot be NULL?
>
> - wl_list_for_each(ivi_view, &ivilayer->order.view_list,
> order_link) {
> - /*
> - * If ivilayer is invisible, weston_view of
> ivisurf doesn't
> - * need to be modified.
> - */
> - if (ivi_view->ivisurf->prop.visibility == false)
> - continue;
> + /*
> + * If ivi_view is not visible on a screen, weston_view of
> ivi_view
> + * doesn't need to be modified.
> + */
> + if (!wl_list_length(&ivi_view->order_link) || !iviscrn)
Use wl_list_empty() instead of wl_list_length().
> + continue;
>
> - update_prop(iviscrn, ivilayer, ivi_view);
> - }
> + /*
> + * If ivilayer or ivisurf of ivi_view is made invisible in this
> + * commit_changes call, we have to damage the weston_view below
> this
> + * ivi_view. Otherwise content of this ivi_view will stay
> visible.
> + */
> + if ((!ivilayer->prop.visibility &&
> + (ivilayer->prop.event_mask &
> IVI_NOTIFICATION_VISIBILITY)) ||
> + (!ivisurf->prop.visibility &&
> + (ivisurf->prop.event_mask &
> IVI_NOTIFICATION_VISIBILITY))) {
> + weston_view_damage_below(ivi_view->view);
> + continue;
Does this now skip update_prop() for things made invisible in this
cycle, while previously they went through update_prop()?
I think the commit message should explain why this change.
> }
> +
> + update_prop(iviscrn, ivilayer, ivi_view);
> }
> }
>
While I think the idea is good, the patch seems to be doing too much in
one go and without justification. But, I have also forgot most of how
ivi-layout works.
Thanks,
pq
pgpmsNfIh3eHK.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
