RE: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not removed from list of ivi_layer when the ivi_surface is removed from the compositor.
Hi Tanibata-san,
Tel. +49 5121 49 6937
-Original Message-
From: wayland-devel [mailto:wayland-devel-
boun...@lists.freedesktop.org] On Behalf Of Tanibata, Nobuhiko
(ADITJ/SWG)
Sent: Montag, 17. August 2015 08:01
To: Pekka Paalanen; Nobuhiko Tanibata
Cc: Ishikawa, Tetsuri (ADITJ/SWG); securitych...@denso.co.jp; wayland-
de...@lists.freedesktop.org
Subject: RE: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not
removed from list of ivi_layer when the ivi_surface is removed from the
compositor.
-Original Message-
From: wayland-devel
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of
Pekka Paalanen
Sent: Thursday, August 13, 2015 10:21 PM
To: Nobuhiko Tanibata
Cc: securitych...@denso.co.jp; wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is
not removed from list of ivi_layer when the ivi_surface is removed
from the compositor.
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
ivilayer-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
ivilayer-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 @@
RE: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not removed from list of ivi_layer when the ivi_surface is removed from the compositor.
-Original Message-
From: wayland-devel
[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
Paalanen
Sent: Thursday, August 13, 2015 10:21 PM
To: Nobuhiko Tanibata
Cc: securitych...@denso.co.jp; wayland-devel@lists.freedesktop.org
Subject: Re: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not
removed from list of ivi_layer when the ivi_surface is removed from the
compositor.
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?
Re: [PATCH weston v2] ivi-shell: bugfix, an ivi_surface is not removed from list of ivi_layer when the ivi_surface is removed from the compositor.
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
