On Thu, 31 Jul 2014 15:36:40 +0200 Manuel Bachmann <[email protected]> wrote:
> This fixes the following : > - if a surface was set fullscreen, and then minimized, > the fullscreen compositor state would stay on and display > a black screen ; > - if a surface was set fullscreen, and we would then > cycle between surfaces (with Mod+Tab e.g.), the fullscreen > compositor state would stay on, and the fullscreen layer > would sometimes hide surfaces positioned behind it ; > - style and functional polishing, remove dead code. Hi, sounds like this should really be three separate patches, as the commit message is a list of three things. ;-) Or would separating produce just useless churn? shell.c patches are hard to review to begin with, so I'd appreciate if each patch did one thing and nothing more. > - if a surface was set fullscreen, and we would then > cycle between surfaces (with Mod+Tab e.g.), the fullscreen > compositor state would stay on, and the fullscreen layer > would sometimes hide surfaces positioned behind it ; Hmm, are you sure this is a bug? What I see happening, when a fullscreen surface is on top and another window is below it, and I press Mod+tab, is that the fullscreen window does stay on top, but it becomes almost transparent, so I can still see which window would be raised if I released Mod+tab now. To me it looks like it is intentional to not change the stacking order until releasing Mod+tab. More comments below. > > Signed-off-by: Manuel Bachmann <[email protected]> > --- > desktop-shell/shell.c | 67 > ++++++++++++++++++++++++++++--------------------- > 1 file changed, 38 insertions(+), 29 deletions(-) > > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c > index 3c3649c..60639cc 100644 > --- a/desktop-shell/shell.c > +++ b/desktop-shell/shell.c > @@ -2510,13 +2510,14 @@ unset_maximized(struct shell_surface *shsurf) > } > > static void > -set_minimized(struct weston_surface *surface, uint32_t is_true) > +set_minimized(struct weston_surface *surface) > { > + struct weston_view *view; > struct shell_surface *shsurf; > - struct workspace *current_ws; > + struct workspace *ws; > struct weston_seat *seat; > struct weston_surface *focus; > - struct weston_view *view; > + float x,y; > > view = get_default_view(surface); > if (!view) > @@ -2525,34 +2526,31 @@ set_minimized(struct weston_surface *surface, > uint32_t is_true) > assert(weston_surface_get_main_surface(view->surface) == view->surface); > > shsurf = get_shell_surface(surface); > - current_ws = get_current_workspace(shsurf->shell); > + ws = get_current_workspace(shsurf->shell); > > - weston_layer_entry_remove(&view->layer_link); > - /* hide or show, depending on the state */ > - if (is_true) { > - > weston_layer_entry_insert(&shsurf->shell->minimized_layer.view_list, > &view->layer_link); > - > - drop_focus_state(shsurf->shell, current_ws, view->surface); > - wl_list_for_each(seat, &shsurf->shell->compositor->seat_list, > link) { > - if (!seat->keyboard) > - continue; > - focus = > weston_surface_get_main_surface(seat->keyboard->focus); > - if (focus == view->surface) > - weston_keyboard_set_focus(seat->keyboard, NULL); > - } > + /* if the surface is fullscreen, unset the global fullscreen state, > + * but keep the surface centered on its previous output. > + */ > + if (shsurf->state.fullscreen) { > + x = shsurf->view->geometry.x; > + y = shsurf->view->geometry.y; > + unset_fullscreen(shsurf); > + weston_view_set_position(shsurf->view, x, y); This seems slightly strange, doesn't unset_fullscreen() cause you to lose some bits of the fullscreen state, meaning you cannot restore it properly later? The framerate for instance? > } > - else { > - weston_layer_entry_insert(¤t_ws->layer.view_list, > &view->layer_link); > > - wl_list_for_each(seat, &shsurf->shell->compositor->seat_list, > link) { > - if (!seat->keyboard) > - continue; > - activate(shsurf->shell, view->surface, seat, true); > - } > + weston_layer_entry_remove(&view->layer_link); > + weston_layer_entry_insert(&shsurf->shell->minimized_layer.view_list, > &view->layer_link); > + > + drop_focus_state(shsurf->shell, ws, view->surface); > + wl_list_for_each(seat, &shsurf->shell->compositor->seat_list, link) { > + if (!seat->keyboard) > + continue; > + focus = weston_surface_get_main_surface(seat->keyboard->focus); > + if (focus == view->surface) > + weston_keyboard_set_focus(seat->keyboard, NULL); > } > > shell_surface_update_child_surface_layers(shsurf); > - > weston_view_damage_below(view); > } > > @@ -3534,8 +3532,7 @@ xdg_surface_set_minimized(struct wl_client *client, > if (shsurf->type != SHELL_SURFACE_TOPLEVEL) > return; > > - /* apply compositor's own minimization logic (hide) */ > - set_minimized(shsurf->surface, 1); > + set_minimized(shsurf->surface); > } > > static const struct xdg_surface_interface xdg_surface_implementation = { > @@ -5444,12 +5441,24 @@ struct switcher { > static void > switcher_next(struct switcher *switcher) > { > + struct focus_state *state; > + struct weston_surface *surface; > struct weston_view *view; > struct weston_surface *first = NULL, *prev = NULL, *next = NULL; > struct shell_surface *shsurf; > struct workspace *ws = get_current_workspace(switcher->shell); > > - /* temporary re-display minimized surfaces */ > + /* if the focused surface is fullscreen, minimize it */ > + wl_list_for_each(state, &ws->focus_list, link) { > + if (state->keyboard_focus) { > + surface = > weston_surface_get_main_surface(state->keyboard_focus); > + shsurf = get_shell_surface(surface); > + if (shsurf->state.fullscreen) > + set_minimized(surface); > + } > + } Err, can you explain what happens here and why? Do we have a concept of a fullscreen surface being shown while not being top-most or active? That is, is it expected to force-minimize a fullscreen window when it is not... top-most? Regardless whether it was minimized or not. > + > + /* temporarily display minimized surfaces again */ > struct weston_view *tmp; > struct weston_view **minimized; > wl_list_for_each_safe(view, tmp, > &switcher->shell->minimized_layer.view_list.link, layer_link.link) { > @@ -5495,7 +5504,7 @@ switcher_next(struct switcher *switcher) > view->alpha = 1.0; > > shsurf = get_shell_surface(switcher->current); > - if (shsurf && shsurf->state.fullscreen) > + if (shsurf && shsurf->state.fullscreen && shsurf->fullscreen.black_view) > shsurf->fullscreen.black_view->alpha = 1.0; > } > Sorry, I'm quite confused by these parts of shell.c. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
