[1/5] ivi-shell: remove struct link_screen

Submitted by Ucan, Emre (ADITG/ESB) on Aug. 28, 2015, 12:58 p.m.

Details

Message ID AF835182DB40564F805CF05008AAFCA1A0EF70@HI2EXCH01.adit-jv.com
State Accepted
Commit 8a223673ad464516b99f4322577ad93748349ed3
Delegated to: Nobuhiko Tanibata
Headers show

Not browsing as part of any series.

Commit Message

Ucan, Emre (ADITG/ESB) Aug. 28, 2015, 12:58 p.m.
link_screen's sole purpose is to link a layer to multiple screens, if the layer should be shown in multiple screens.
This can be only achieved, if surfaces of the layer have multiple weston_views dedicated to the different screens.

Current implementation assumes in many places that a ivi_surface has only one weston_view.
Therefore, a layer can be only shown on one screen.

Although this (a layer on multiple screens) is a nice to have feature for ivi-shell, it is not very crucial.
In any case, it is not an easy task to implement this feature, because it has lot of corner cases.

I removed with this patch the link_screen data structure, because it does not have any purpose in current implementation.

Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
---
 ivi-shell/ivi-layout-private.h |    2 +-
 ivi-shell/ivi-layout.c         |   67 +++-------------------------------------
 2 files changed, 6 insertions(+), 63 deletions(-)

Patch hide | download patch | download mbox

diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 074d598..a9dbdde 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -65,11 +65,11 @@  struct ivi_layout_surface {
 struct ivi_layout_layer {
 	struct wl_list link;
 	struct wl_signal property_changed;
-	struct wl_list screen_list;
 	struct wl_list link_to_surface;
 	uint32_t id_layer;
 
 	struct ivi_layout *layout;
+	struct ivi_layout_screen *on_screen;
 
 	struct ivi_layout_layer_properties prop;
 	uint32_t event_mask;
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 24bd8dd..087f94c 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -75,12 +75,6 @@  struct link_layer {
 	struct wl_list link_to_layer;
 };
 
-struct link_screen {
-	struct ivi_layout_screen *iviscrn;
-	struct wl_list link;
-	struct wl_list link_to_screen;
-};
-
 struct listener_layout_notification {
 	void *userdata;
 	struct wl_listener listener;
@@ -90,7 +84,6 @@  struct ivi_layout;
 
 struct ivi_layout_screen {
 	struct wl_list link;
-	struct wl_list link_to_layer;
 	uint32_t id_screen;
 
 	struct ivi_layout *layout;
@@ -165,16 +158,6 @@  remove_link_to_surface(struct ivi_layout_layer *ivilayer)
 }
 
 /**
- * Internal API to add a link to ivi_layer from ivi_screen.
- */
-static void
-add_link_to_layer(struct ivi_layout_screen *iviscrn,
-		  struct link_screen *link_screen)
-{
-	wl_list_insert(&iviscrn->link_to_layer, &link_screen->link_to_screen);
-}
-
-/**
  * Internal API to add/remove a ivi_surface from ivi_layer.
  */
 static void
@@ -211,40 +194,6 @@  remove_ordersurface_from_layer(struct ivi_layout_surface *ivisurf)
 /**
  * Internal API to add/remove a ivi_layer to/from ivi_screen.
  */
-static void
-add_orderlayer_to_screen(struct ivi_layout_layer *ivilayer,
-			 struct ivi_layout_screen *iviscrn)
-{
-	struct link_screen *link_scrn = NULL;
-
-	link_scrn = malloc(sizeof *link_scrn);
-	if (link_scrn == NULL) {
-		weston_log("fails to allocate memory\n");
-		return;
-	}
-
-	link_scrn->iviscrn = iviscrn;
-	wl_list_insert(&ivilayer->screen_list, &link_scrn->link);
-	add_link_to_layer(iviscrn, link_scrn);
-}
-
-static void
-remove_orderlayer_from_screen(struct ivi_layout_layer *ivilayer)
-{
-	struct link_screen *link_scrn = NULL;
-	struct link_screen *next = NULL;
-
-	wl_list_for_each_safe(link_scrn, next, &ivilayer->screen_list, link) {
-		wl_list_remove(&link_scrn->link);
-		wl_list_remove(&link_scrn->link_to_screen);
-		free(link_scrn);
-	}
-	wl_list_init(&ivilayer->screen_list);
-}
-
-/**
- * Internal API to add/remove a ivi_layer to/from ivi_screen.
- */
 static struct ivi_layout_surface *
 get_surface(struct wl_list *surf_list, uint32_t id_surface)
 {
@@ -422,8 +371,6 @@  create_screen(struct weston_compositor *ec)
 		wl_list_init(&iviscrn->order.layer_list);
 		wl_list_init(&iviscrn->order.link);
 
-		wl_list_init(&iviscrn->link_to_layer);
-
 		wl_list_insert(&layout->screen_list, &iviscrn->link);
 	}
 }
@@ -964,7 +911,7 @@  commit_screen_list(struct ivi_layout *layout)
 		if (iviscrn->order.dirty) {
 			wl_list_for_each_safe(ivilayer, next,
 					      &iviscrn->order.layer_list, order.link) {
-				remove_orderlayer_from_screen(ivilayer);
+				ivilayer->on_screen = NULL;
 				wl_list_remove(&ivilayer->order.link);
 				wl_list_init(&ivilayer->order.link);
 				ivilayer->event_mask |= IVI_NOTIFICATION_REMOVE;
@@ -976,7 +923,7 @@  commit_screen_list(struct ivi_layout *layout)
 					 pending.link) {
 				wl_list_insert(&iviscrn->order.layer_list,
 					       &ivilayer->order.link);
-				add_orderlayer_to_screen(ivilayer, iviscrn);
+				ivilayer->on_screen = iviscrn;
 				ivilayer->event_mask |= IVI_NOTIFICATION_ADD;
 			}
 
@@ -1595,7 +1542,6 @@  ivi_layout_get_screens_under_layer(struct ivi_layout_layer *ivilayer,
 				   int32_t *pLength,
 				   struct ivi_layout_screen ***ppArray)
 {
-	struct link_screen *link_scrn = NULL;
 	int32_t length = 0;
 	int32_t n = 0;
 
@@ -1604,7 +1550,8 @@  ivi_layout_get_screens_under_layer(struct ivi_layout_layer *ivilayer,
 		return IVI_FAILED;
 	}
 
-	length = wl_list_length(&ivilayer->screen_list);
+	if (ivilayer->on_screen != NULL)
+		length = 1;
 
 	if (length != 0) {
 		/* the Array must be free by module which called this function */
@@ -1614,9 +1561,7 @@  ivi_layout_get_screens_under_layer(struct ivi_layout_layer *ivilayer,
 			return IVI_FAILED;
 		}
 
-		wl_list_for_each(link_scrn, &ivilayer->screen_list, link) {
-			(*ppArray)[n++] = link_scrn->iviscrn;
-		}
+		(*ppArray)[n++] = ivilayer->on_screen;
 	}
 
 	*pLength = length;
@@ -1815,7 +1760,6 @@  ivi_layout_layer_create_with_dimension(uint32_t id_layer,
 
 	ivilayer->ref_count = 1;
 	wl_signal_init(&ivilayer->property_changed);
-	wl_list_init(&ivilayer->screen_list);
 	wl_list_init(&ivilayer->link_to_surface);
 	ivilayer->layout = layout;
 	ivilayer->id_layer = id_layer;
@@ -1883,7 +1827,6 @@  ivi_layout_layer_destroy(struct ivi_layout_layer *ivilayer)
 	wl_list_remove(&ivilayer->order.link);
 	wl_list_remove(&ivilayer->link);
 
-	remove_orderlayer_from_screen(ivilayer);
 	remove_link_to_surface(ivilayer);
 	ivi_layout_layer_remove_notification(ivilayer);
 

Comments

Hi,

Tested-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA@xddp.denso.co.jp>
Reviewed-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA@xddp.denso.co.jp>

These constructors are reserved for a feature of 'a ivi-surface to 
several layers' and 'a ivi-layer to several screens'. This feature is 
required for
- for example, car navigation application prepares junction guide on a 
surface, which shall be displayed in its own UI and cloned on 
instrumental cluster in another screen as well. In this case, a 
ivi-surface can be located in both.

However, I have to modify weston core to realize it. So, for the time 
being, we can remove them at this time to simplify codes.

BR,
Nobuhiko Tanibata

2015-08-28 21:58 に Ucan, Emre (ADITG/SW1) さんは書きました:
> link_screen's sole purpose is to link a layer to multiple screens, if
> the layer should be shown in multiple screens.
> This can be only achieved, if surfaces of the layer have multiple
> weston_views dedicated to the different screens.
> 
> Current implementation assumes in many places that a ivi_surface has
> only one weston_view.
> Therefore, a layer can be only shown on one screen.
> 
> Although this (a layer on multiple screens) is a nice to have feature
> for ivi-shell, it is not very crucial.
> In any case, it is not an easy task to implement this feature, because
> it has lot of corner cases.
> 
> I removed with this patch the link_screen data structure, because it
> does not have any purpose in current implementation.
> 
> Signed-off-by: Emre Ucan <eucan@de.adit-jv.com>
> ---
>  ivi-shell/ivi-layout-private.h |    2 +-
>  ivi-shell/ivi-layout.c         |   67 
> +++-------------------------------------
>  2 files changed, 6 insertions(+), 63 deletions(-)
> 
> diff --git a/ivi-shell/ivi-layout-private.h 
> b/ivi-shell/ivi-layout-private.h
> index 074d598..a9dbdde 100644
> --- a/ivi-shell/ivi-layout-private.h
> +++ b/ivi-shell/ivi-layout-private.h
> @@ -65,11 +65,11 @@ struct ivi_layout_surface {
>  struct ivi_layout_layer {
>  	struct wl_list link;
>  	struct wl_signal property_changed;
> -	struct wl_list screen_list;
>  	struct wl_list link_to_surface;
>  	uint32_t id_layer;
> 
>  	struct ivi_layout *layout;
> +	struct ivi_layout_screen *on_screen;
> 
>  	struct ivi_layout_layer_properties prop;
>  	uint32_t event_mask;
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index 24bd8dd..087f94c 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -75,12 +75,6 @@ struct link_layer {
>  	struct wl_list link_to_layer;
>  };
> 
> -struct link_screen {
> -	struct ivi_layout_screen *iviscrn;
> -	struct wl_list link;
> -	struct wl_list link_to_screen;
> -};
> -
>  struct listener_layout_notification {
>  	void *userdata;
>  	struct wl_listener listener;
> @@ -90,7 +84,6 @@ struct ivi_layout;
> 
>  struct ivi_layout_screen {
>  	struct wl_list link;
> -	struct wl_list link_to_layer;
>  	uint32_t id_screen;
> 
>  	struct ivi_layout *layout;
> @@ -165,16 +158,6 @@ remove_link_to_surface(struct ivi_layout_layer 
> *ivilayer)
>  }
> 
>  /**
> - * Internal API to add a link to ivi_layer from ivi_screen.
> - */
> -static void
> -add_link_to_layer(struct ivi_layout_screen *iviscrn,
> -		  struct link_screen *link_screen)
> -{
> -	wl_list_insert(&iviscrn->link_to_layer, 
> &link_screen->link_to_screen);
> -}
> -
> -/**
>   * Internal API to add/remove a ivi_surface from ivi_layer.
>   */
>  static void
> @@ -211,40 +194,6 @@ remove_ordersurface_from_layer(struct
> ivi_layout_surface *ivisurf)
>  /**
>   * Internal API to add/remove a ivi_layer to/from ivi_screen.
>   */
> -static void
> -add_orderlayer_to_screen(struct ivi_layout_layer *ivilayer,
> -			 struct ivi_layout_screen *iviscrn)
> -{
> -	struct link_screen *link_scrn = NULL;
> -
> -	link_scrn = malloc(sizeof *link_scrn);
> -	if (link_scrn == NULL) {
> -		weston_log("fails to allocate memory\n");
> -		return;
> -	}
> -
> -	link_scrn->iviscrn = iviscrn;
> -	wl_list_insert(&ivilayer->screen_list, &link_scrn->link);
> -	add_link_to_layer(iviscrn, link_scrn);
> -}
> -
> -static void
> -remove_orderlayer_from_screen(struct ivi_layout_layer *ivilayer)
> -{
> -	struct link_screen *link_scrn = NULL;
> -	struct link_screen *next = NULL;
> -
> -	wl_list_for_each_safe(link_scrn, next, &ivilayer->screen_list, link) 
> {
> -		wl_list_remove(&link_scrn->link);
> -		wl_list_remove(&link_scrn->link_to_screen);
> -		free(link_scrn);
> -	}
> -	wl_list_init(&ivilayer->screen_list);
> -}
> -
> -/**
> - * Internal API to add/remove a ivi_layer to/from ivi_screen.
> - */
>  static struct ivi_layout_surface *
>  get_surface(struct wl_list *surf_list, uint32_t id_surface)
>  {
> @@ -422,8 +371,6 @@ create_screen(struct weston_compositor *ec)
>  		wl_list_init(&iviscrn->order.layer_list);
>  		wl_list_init(&iviscrn->order.link);
> 
> -		wl_list_init(&iviscrn->link_to_layer);
> -
>  		wl_list_insert(&layout->screen_list, &iviscrn->link);
>  	}
>  }
> @@ -964,7 +911,7 @@ commit_screen_list(struct ivi_layout *layout)
>  		if (iviscrn->order.dirty) {
>  			wl_list_for_each_safe(ivilayer, next,
>  					      &iviscrn->order.layer_list, order.link) {
> -				remove_orderlayer_from_screen(ivilayer);
> +				ivilayer->on_screen = NULL;
>  				wl_list_remove(&ivilayer->order.link);
>  				wl_list_init(&ivilayer->order.link);
>  				ivilayer->event_mask |= IVI_NOTIFICATION_REMOVE;
> @@ -976,7 +923,7 @@ commit_screen_list(struct ivi_layout *layout)
>  					 pending.link) {
>  				wl_list_insert(&iviscrn->order.layer_list,
>  					       &ivilayer->order.link);
> -				add_orderlayer_to_screen(ivilayer, iviscrn);
> +				ivilayer->on_screen = iviscrn;
>  				ivilayer->event_mask |= IVI_NOTIFICATION_ADD;
>  			}
> 
> @@ -1595,7 +1542,6 @@ ivi_layout_get_screens_under_layer(struct
> ivi_layout_layer *ivilayer,
>  				   int32_t *pLength,
>  				   struct ivi_layout_screen ***ppArray)
>  {
> -	struct link_screen *link_scrn = NULL;
>  	int32_t length = 0;
>  	int32_t n = 0;
> 
> @@ -1604,7 +1550,8 @@ ivi_layout_get_screens_under_layer(struct
> ivi_layout_layer *ivilayer,
>  		return IVI_FAILED;
>  	}
> 
> -	length = wl_list_length(&ivilayer->screen_list);
> +	if (ivilayer->on_screen != NULL)
> +		length = 1;
> 
>  	if (length != 0) {
>  		/* the Array must be free by module which called this function */
> @@ -1614,9 +1561,7 @@ ivi_layout_get_screens_under_layer(struct
> ivi_layout_layer *ivilayer,
>  			return IVI_FAILED;
>  		}
> 
> -		wl_list_for_each(link_scrn, &ivilayer->screen_list, link) {
> -			(*ppArray)[n++] = link_scrn->iviscrn;
> -		}
> +		(*ppArray)[n++] = ivilayer->on_screen;
>  	}
> 
>  	*pLength = length;
> @@ -1815,7 +1760,6 @@ ivi_layout_layer_create_with_dimension(uint32_t 
> id_layer,
> 
>  	ivilayer->ref_count = 1;
>  	wl_signal_init(&ivilayer->property_changed);
> -	wl_list_init(&ivilayer->screen_list);
>  	wl_list_init(&ivilayer->link_to_surface);
>  	ivilayer->layout = layout;
>  	ivilayer->id_layer = id_layer;
> @@ -1883,7 +1827,6 @@ ivi_layout_layer_destroy(struct ivi_layout_layer
> *ivilayer)
>  	wl_list_remove(&ivilayer->order.link);
>  	wl_list_remove(&ivilayer->link);
> 
> -	remove_orderlayer_from_screen(ivilayer);
>  	remove_link_to_surface(ivilayer);
>  	ivi_layout_layer_remove_notification(ivilayer);
On Thu, 01 Oct 2015 17:13:47 +0900
Nobuhiko Tanibata <nobuhiko_tanibata@xddp.denso.co.jp> wrote:

> Hi,
> 
> Tested-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA@xddp.denso.co.jp>
> Reviewed-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA@xddp.denso.co.jp>
> 
> These constructors are reserved for a feature of 'a ivi-surface to 
> several layers' and 'a ivi-layer to several screens'. This feature is 
> required for
> - for example, car navigation application prepares junction guide on a 
> surface, which shall be displayed in its own UI and cloned on 
> instrumental cluster in another screen as well. In this case, a 
> ivi-surface can be located in both.
> 
> However, I have to modify weston core to realize it. So, for the time 
> being, we can remove them at this time to simplify codes.

Hi,

I'm not sure you need to modify Weston core. You need to create
one weston_view for each ivi-screen/ivi-layer/ivi-surface combination.

Emre, these are nice clean-ups. I have a couple of comments for the
future:
- mind the line length in the commit message, I re-wrapped all the
  patches
- use a space after a keyword like 'if', I think there were a few without

Pushed all 5 patches:
   dcba1a1..64635ee  master -> master


Thanks,
pq