On Mon, 11 Jul 2016 11:29:40 +0200 Quentin Glidic <[email protected]> wrote:
> From: Quentin Glidic <[email protected]> > > Currently, layers’ order depends on the module loading order and it does > not survive runtime modifications (like shell locking/unlocking). > With this patch, modules can safely add their own layer at the expected > position in the stack, with runtime persistence. > > Signed-off-by: Quentin Glidic <[email protected]> > Acked-by: Pekka Paalanen <[email protected]> > --- > > v3: > - Added weston_layer_unset_position > - HIDDEN is now still rendered > (Feature needed by Giulio Camuffo) > - weston_layer now stores the weston_compositor pointer to avoid its > need in set_position > - Reordered weston_layer members (we break the ABI anyway) > - weston_layer_init now only initialize the struct, you must > call set_position to add it to the layer list > - BACKGROUND is now 2 instead of 1, as these values are mainly meant > for libweston modules. The compositor should simply use > (BACKGROUND - 1) if it wants to support such modules with > a fallback surface > (Suggestion by Bill Spitzak) > > v2: > - Tiny commit message addition: added runtime behaviour comment. > - Reworked (a lot) the enum comment to explain further the position > values and their actual expected scope. I hope their are clear enough. > Here are the biggest changes: > - Added a BOTTOM_UI value for widgets and conky-like applications. > - Changed BACKGROUND to be 1, as nothing should be under it > - Added a comment about mandatory background > > desktop-shell/input-panel.c | 6 ++-- > desktop-shell/shell.c | 64 +++++++++++++++++---------------- > fullscreen-shell/fullscreen-shell.c | 4 ++- > ivi-shell/input-panel-ivi.c | 6 ++-- > ivi-shell/ivi-layout.c | 4 ++- > ivi-shell/ivi-shell.c | 2 +- > libweston/compositor.c | 52 ++++++++++++++++++++++++--- > libweston/compositor.h | 70 > +++++++++++++++++++++++++++++++++++-- > tests/weston-test.c | 3 +- > 9 files changed, 163 insertions(+), 48 deletions(-) > > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c > index c72f801..bdaad87 100644 > --- a/desktop-shell/shell.c > +++ b/desktop-shell/shell.c I looks like these items still remain: desktop-shell/shell.c=1048=finish_workspace_change_animation(struct desktop_shell *shell, desktop-shell/shell.c:1068: wl_list_remove(&shell->workspaces.anim_from->layer.link); desktop-shell/shell.c=1123=animate_workspace_change(struct desktop_shell *shell, desktop-shell/shell.c:1150: wl_list_insert(from->layer.link.prev, &to->layer.link); desktop-shell/shell.c=1160=update_workspace(struct desktop_shell *shell, unsigned int index, desktop-shell/shell.c:1164: wl_list_insert(&from->layer.link, &to->layer.link); desktop-shell/shell.c:1165: wl_list_remove(&from->layer.link); desktop-shell/shell.c=1255=take_surface_to_workspace_by_seat(struct desktop_shell *shell, desktop-shell/shell.c:1289: wl_list_remove(&to->layer.link); desktop-shell/shell.c:1290: wl_list_insert(from->layer.link.prev, &to->layer.link); They are all manipulating the weston_layer::link, so it seems to me they should be fixed. > diff --git a/libweston/compositor.c b/libweston/compositor.c > index 771f1c9..9d6bbf6 100644 > --- a/libweston/compositor.c > +++ b/libweston/compositor.c > @@ -2402,14 +2402,52 @@ weston_layer_entry_remove(struct weston_layer_entry > *entry) > entry->layer = NULL; > } > > + > +/** Initialize the weston_layer struct. > + * > + * \param compositor The compositor instance > + * \param layer The layer to initialize > + */ > WL_EXPORT void > -weston_layer_init(struct weston_layer *layer, struct wl_list *below) > +weston_layer_init(struct weston_compositor *compositor, > + struct weston_layer *layer) > { > + layer->compositor = compositor; > + wl_list_init(&layer->link); > wl_list_init(&layer->view_list.link); > layer->view_list.layer = layer; > weston_layer_set_mask_infinite(layer); > - if (below != NULL) > - wl_list_insert(below, &layer->link); > +} > + > +/** Sets the position of the layer in the layer list. > + * > + * \param layer The layer to modify > + * \param position The position the layer will be placed at This should document that calling set_position twice without an unset in between is not allowed. > + */ > +WL_EXPORT void > +weston_layer_set_position(struct weston_layer *layer, > + enum weston_layer_position position) > +{ > + struct weston_layer *below; > + > + layer->position = position; > + wl_list_for_each_reverse(below, &layer->compositor->layer_list, link) { > + if (below->position >= layer->position) { > + wl_list_insert(&below->link, &layer->link); > + return; > + } > + } > + wl_list_insert(layer->compositor->layer_list.next, &layer->link); I think this should not have .next, should it? Now it's adding the new layer below the top-most layer when it should become the top-most layer, no? > +} > + > +/** Hide a layer by taking it off the layer list. > + * > + * \param layer The layer to hide This should document that calling this on a layer that is "not on the list" is not allowed. However, since init() does init the link, I wonder if you meant these to be safe instead of disallow repeated calls? > + */ > +WL_EXPORT void > +weston_layer_unset_position(struct weston_layer *layer) > +{ > + wl_list_remove(&layer->link); > } > > WL_EXPORT void > @@ -4725,8 +4763,12 @@ weston_compositor_create(struct wl_display *display, > void *user_data) > loop = wl_display_get_event_loop(ec->wl_display); > ec->idle_source = wl_event_loop_add_timer(loop, idle_handler, ec); > > - weston_layer_init(&ec->fade_layer, &ec->layer_list); > - weston_layer_init(&ec->cursor_layer, &ec->fade_layer.link); > + weston_layer_init(ec, &ec->fade_layer); > + weston_layer_init(ec, &ec->cursor_layer); > + > + weston_layer_set_position(&ec->fade_layer, WESTON_LAYER_POSITION_FADE); > + weston_layer_set_position(&ec->cursor_layer, > + WESTON_LAYER_POSITION_CURSOR); > > weston_compositor_add_debug_binding(ec, KEY_T, > timeline_key_binding_handler, ec); > diff --git a/libweston/compositor.h b/libweston/compositor.h > index 557d2f5..d6e35e0 100644 > --- a/libweston/compositor.h > +++ b/libweston/compositor.h > @@ -605,10 +605,68 @@ struct weston_layer_entry { > struct weston_layer *layer; > }; > > +/** > + * Higher value means higher in the stack. > + * > + * These values are based on well-known concepts in a classic desktop > + * environment. Third-party modules based on libweston are encouraged to use > + * them to integrate better with other projects. > + * > + * A fully integrated environment can use any value, based on these or not, > + * at their discretion. > + */ > +enum weston_layer_position { > + /* > + * Special value to make the layer invisible and still rendered. > + * This is used by compositors wanting e.g. minimized surfaces to still > + * receive frame callbacks. > + */ > + WESTON_LAYER_POSITION_HIDDEN = 0x00000000, > + > + /* > + * There should always be a background layer with a surface covering > + * the visible area. > + * > + * If the compositor handles the background itself, it should use > + * BACKGROUND. > + * > + * If the compositor supports runtime-loadable modules to set the > + * background, it should put a solid color surface at (BACKGROUND - 1) > + * and modules must use BACKGROUND. > + */ > + WESTON_LAYER_POSITION_BACKGROUND = 0x00000002, > + > + /* For "desktop widgets" and applications like conky. */ > + WESTON_LAYER_POSITION_BOTTOM_UI = 0x30000000, > + > + /* For regular applications, only one layer should have this value > + * to ensure proper stacking control. */ > + WESTON_LAYER_POSITION_NORMAL = 0x50000000, > + > + /* For desktop UI, like panels. */ > + WESTON_LAYER_POSITION_UI = 0x80000000, > + > + /* For fullscreen applications that should cover UI. */ > + WESTON_LAYER_POSITION_FULLSCREEN = 0xb0000000, > + > + /* For special UI like on-screen keyboard that fullscreen applications > + * will need. */ > + WESTON_LAYER_POSITION_TOP_UI = 0xe0000000, > + > + /* For the lock surface. */ > + WESTON_LAYER_POSITION_LOCK = 0xffff0000, > + > + /* Values reserved for libweston internal usage */ > + WESTON_LAYER_POSITION_CURSOR = 0xfffffffe, > + WESTON_LAYER_POSITION_FADE = 0xffffffff, > +}; > + > struct weston_layer { > - struct weston_layer_entry view_list; > - struct wl_list link; > + struct weston_compositor *compositor; > + struct wl_list link; /* weston_compositor::layer_list */ > + enum weston_layer_position position; I'm not sure this should be 'enum' because the enumeration does not actually list all possible values. > pixman_box32_t mask; > + struct weston_layer_entry view_list; > }; > > struct weston_plane { > @@ -1222,7 +1280,13 @@ weston_layer_entry_insert(struct weston_layer_entry > *list, > void > weston_layer_entry_remove(struct weston_layer_entry *entry); > void > -weston_layer_init(struct weston_layer *layer, struct wl_list *below); > +weston_layer_init(struct weston_compositor *compositor, > + struct weston_layer *layer); > +void > +weston_layer_set_position(struct weston_layer *layer, > + enum weston_layer_position position); > +void > +weston_layer_unset_position(struct weston_layer *layer); > > void > weston_layer_set_mask(struct weston_layer *layer, int x, int y, int width, > int height); > diff --git a/tests/weston-test.c b/tests/weston-test.c > index 09d8b5e..4de42fc 100644 > --- a/tests/weston-test.c > +++ b/tests/weston-test.c > @@ -595,7 +595,8 @@ module_init(struct weston_compositor *ec, > return -1; > > test->compositor = ec; > - weston_layer_init(&test->layer, &ec->cursor_layer.link); > + weston_layer_init(ec, &test->layer); > + weston_layer_set_position(&test->layer, WESTON_LAYER_POSITION_NORMAL); Looks like this was intended to go immediately below the cursor layer, so on top of everything else. Since it's a special layer for tests only, how about using WESTON_LAYER_POSITION_CURSOR - 1? > > if (wl_global_create(ec->wl_display, &weston_test_interface, 1, > test, bind_test) == NULL) Very good work indeed. Thanks, pq
pgpY2khePVvI0.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
