[PATCH 02/16] shell: Remove SHELL_SURFACE_FULLSCREEN and SHELL_SURFACE_MAXIMIZED.
Rafael Antognolli
antognolli at gmail.com
Tue Dec 3 09:43:09 PST 2013
All done.
On Fri, Nov 29, 2013 at 9:00 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> On Wed, Nov 27, 2013 at 03:50:18PM -0200, Rafael Antognolli wrote:
>> These surface types don't exist anymore inside weston desktop shell
>> implementation. They are just exposed as wl_shell surface types, but
>> internally the implementation is done with surface states.
>>
>> The previous
>> behavior (setting a surface type unsets another one) still happens when
>> using wl_shell. This change is mainly done as a refactory to allow
>> xdg-shell to use the same code.
>> ---
>> src/shell.c | 224 +++++++++++++++++++++++++++++++++---------------------------
>> 1 file changed, 123 insertions(+), 101 deletions(-)
>
> This patch is good, it's a nice, self-contained step towards
> xdg-shell. I just have a couple of nit-picks below.
>
>> diff --git a/src/shell.c b/src/shell.c
>> index f102e9a..cf89a84 100644
>> --- a/src/shell.c
>> +++ b/src/shell.c
>> @@ -237,8 +237,6 @@ enum shell_surface_type {
>> SHELL_SURFACE_NONE,
>> SHELL_SURFACE_TOPLEVEL,
>> SHELL_SURFACE_TRANSIENT,
>> - SHELL_SURFACE_FULLSCREEN,
>> - SHELL_SURFACE_MAXIMIZED,
>> SHELL_SURFACE_POPUP,
>> SHELL_SURFACE_XWAYLAND
>> };
>> @@ -298,6 +296,12 @@ struct shell_surface {
>> struct wl_list link;
>>
>> const struct weston_shell_client *client;
>> +
>> + struct {
>> + bool maximized;
>> + bool fullscreen;
>> + } cur, next; /* surface states */
>
> We try to avoid abbreviations like cur - can we just call it state and
> next_state?
>
>> + bool state_changed;
>> };
>>
>> struct shell_grab {
>> @@ -1450,7 +1454,7 @@ surface_touch_move(struct shell_surface *shsurf, struct weston_seat *seat)
>> if (!shsurf)
>> return -1;
>>
>> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
>> + if (shsurf->cur.fullscreen)
>> return 0;
>> if (shsurf->grabbed)
>> return 0;
>> @@ -1541,7 +1545,7 @@ surface_move(struct shell_surface *shsurf, struct weston_seat *seat)
>>
>> if (shsurf->grabbed)
>> return 0;
>> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
>> + if (shsurf->cur.fullscreen)
>> return 0;
>>
>> move = malloc(sizeof *move);
>> @@ -1715,8 +1719,7 @@ surface_resize(struct shell_surface *shsurf,
>> {
>> struct weston_resize_grab *resize;
>>
>> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN ||
>> - shsurf->type == SHELL_SURFACE_MAXIMIZED)
>> + if (shsurf->cur.fullscreen || shsurf->cur.maximized)
>> return 0;
>>
>> if (edges == 0 || edges > 15 ||
>> @@ -1746,7 +1749,7 @@ shell_surface_resize(struct wl_client *client, struct wl_resource *resource,
>> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>> struct weston_surface *surface;
>>
>> - if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
>> + if (shsurf->cur.fullscreen)
>> return;
>>
>> surface = weston_surface_get_main_surface(seat->pointer->focus->surface);
>> @@ -2061,26 +2064,31 @@ shell_unset_maximized(struct shell_surface *shsurf)
>> static int
>> reset_shell_surface_type(struct shell_surface *surface)
>> {
>> - switch (surface->type) {
>> - case SHELL_SURFACE_FULLSCREEN:
>> + if (surface->cur.fullscreen)
>> shell_unset_fullscreen(surface);
>> - break;
>> - case SHELL_SURFACE_MAXIMIZED:
>> + if (surface->cur.maximized)
>> shell_unset_maximized(surface);
>> - break;
>> - case SHELL_SURFACE_NONE:
>> - case SHELL_SURFACE_TOPLEVEL:
>> - case SHELL_SURFACE_TRANSIENT:
>> - case SHELL_SURFACE_POPUP:
>> - case SHELL_SURFACE_XWAYLAND:
>> - break;
>> - }
>>
>> surface->type = SHELL_SURFACE_NONE;
>> return 0;
>> }
>>
>> static void
>> +set_full_output(struct shell_surface *shsurf)
>> +{
>> + shsurf->saved_x = shsurf->view->geometry.x;
>> + shsurf->saved_y = shsurf->view->geometry.y;
>> + shsurf->saved_position_valid = true;
>> +
>> + if (!wl_list_empty(&shsurf->rotation.transform.link)) {
>> + wl_list_remove(&shsurf->rotation.transform.link);
>> + wl_list_init(&shsurf->rotation.transform.link);
>> + weston_view_geometry_dirty(shsurf->view);
>> + shsurf->saved_rotation_valid = true;
>> + }
>> +}
>> +
>> +static void
>> set_surface_type(struct shell_surface *shsurf)
>> {
>> struct weston_surface *pes = shsurf->parent;
>> @@ -2089,10 +2097,14 @@ set_surface_type(struct shell_surface *shsurf)
>> reset_shell_surface_type(shsurf);
>>
>> shsurf->type = shsurf->next_type;
>> + shsurf->cur = shsurf->next;
>> shsurf->next_type = SHELL_SURFACE_NONE;
>> + shsurf->state_changed = false;
>>
>> switch (shsurf->type) {
>> case SHELL_SURFACE_TOPLEVEL:
>> + if (shsurf->cur.maximized || shsurf->cur.fullscreen)
>> + set_full_output(shsurf);
>> break;
>> case SHELL_SURFACE_TRANSIENT:
>> if (pev)
>> @@ -2101,20 +2113,6 @@ set_surface_type(struct shell_surface *shsurf)
>> pev->geometry.y + shsurf->transient.y);
>> break;
>>
>> - case SHELL_SURFACE_MAXIMIZED:
>> - case SHELL_SURFACE_FULLSCREEN:
>> - shsurf->saved_x = shsurf->view->geometry.x;
>> - shsurf->saved_y = shsurf->view->geometry.y;
>> - shsurf->saved_position_valid = true;
>> -
>> - if (!wl_list_empty(&shsurf->rotation.transform.link)) {
>> - wl_list_remove(&shsurf->rotation.transform.link);
>> - wl_list_init(&shsurf->rotation.transform.link);
>> - weston_view_geometry_dirty(shsurf->view);
>> - shsurf->saved_rotation_valid = true;
>> - }
>> - break;
>> -
>> case SHELL_SURFACE_XWAYLAND:
>> weston_view_set_position(shsurf->view, shsurf->transient.x,
>> shsurf->transient.y);
>> @@ -2126,9 +2124,20 @@ set_surface_type(struct shell_surface *shsurf)
>> }
>>
>> static void
>> +surface_clear_next_states(struct shell_surface *shsurf)
>> +{
>> + shsurf->next.maximized = false;
>> + shsurf->next.fullscreen = false;
>> +
>> + if ((shsurf->next.maximized != shsurf->cur.maximized) ||
>> + (shsurf->next.fullscreen != shsurf->cur.fullscreen))
>> + shsurf->state_changed = true;
>> +}
>> +
>> +static void
>> set_toplevel(struct shell_surface *shsurf)
>> {
>> - shsurf->next_type = SHELL_SURFACE_TOPLEVEL;
>> + shsurf->next_type = SHELL_SURFACE_TOPLEVEL;
>> }
>>
>> static void
>> @@ -2137,6 +2146,7 @@ shell_surface_set_toplevel(struct wl_client *client,
>> {
>> struct shell_surface *surface = wl_resource_get_user_data(resource);
>>
>> + surface_clear_next_states(surface);
>> set_toplevel(surface);
>> }
>>
>> @@ -2150,6 +2160,7 @@ set_transient(struct shell_surface *shsurf,
>> shsurf->transient.y = y;
>> shsurf->transient.flags = flags;
>> shsurf->next_type = SHELL_SURFACE_TRANSIENT;
>> + surface_clear_next_states(shsurf);
>> }
>>
>> static void
>> @@ -2162,6 +2173,7 @@ shell_surface_set_transient(struct wl_client *client,
>> struct weston_surface *parent =
>> wl_resource_get_user_data(parent_resource);
>>
>> + surface_clear_next_states(shsurf);
>> set_transient(shsurf, parent, x, y, flags);
>> }
>>
>> @@ -2192,9 +2204,9 @@ get_output_panel_height(struct desktop_shell *shell,
>> }
>>
>> static void
>> -shell_surface_set_maximized(struct wl_client *client,
>> - struct wl_resource *resource,
>> - struct wl_resource *output_resource )
>> +set_maximized(struct wl_client *client,
>> + struct wl_resource *resource,
>> + struct wl_resource *output_resource)
>> {
>> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>> struct weston_surface *es = shsurf->surface;
>> @@ -2218,7 +2230,19 @@ shell_surface_set_maximized(struct wl_client *client,
>> shsurf->output->width,
>> shsurf->output->height - panel_height);
>>
>> - shsurf->next_type = SHELL_SURFACE_MAXIMIZED;
>> + shsurf->next_type = shsurf->type;
>> +}
>> +
>> +static void
>> +shell_surface_set_maximized(struct wl_client *client,
>> + struct wl_resource *resource,
>> + struct wl_resource *output_resource)
>> +{
>> + struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>> + surface_clear_next_states(shsurf);
>> + set_maximized(client, resource, output_resource);
>> + shsurf->next.maximized = true;
>> + shsurf->state_changed = true;
>> }
>>
>> static void
>> @@ -2410,7 +2434,7 @@ set_fullscreen(struct shell_surface *shsurf,
>> shsurf->fullscreen_output = shsurf->output;
>> shsurf->fullscreen.type = method;
>> shsurf->fullscreen.framerate = framerate;
>> - shsurf->next_type = SHELL_SURFACE_FULLSCREEN;
>> + shsurf->next_type = shsurf->type;
>>
>> shsurf->client->send_configure(shsurf->surface, 0,
>> shsurf->output->width,
>> @@ -2432,13 +2456,17 @@ shell_surface_set_fullscreen(struct wl_client *client,
>> else
>> output = NULL;
>>
>> + surface_clear_next_states(shsurf);
>> set_fullscreen(shsurf, method, framerate, output);
>> + shsurf->next.fullscreen = true;
>> + shsurf->state_changed = true;
>> }
>>
>> static void
>> set_xwayland(struct shell_surface *shsurf, int x, int y, uint32_t flags)
>> {
>> /* XXX: using the same fields for transient type */
>> + surface_clear_next_states(shsurf);
>> shsurf->transient.x = x;
>> shsurf->transient.y = y;
>> shsurf->transient.flags = flags;
>> @@ -2684,6 +2712,7 @@ shell_surface_set_popup(struct wl_client *client,
>> {
>> struct shell_surface *shsurf = wl_resource_get_user_data(resource);
>>
>> + surface_clear_next_states(shsurf);
>> shsurf->type = SHELL_SURFACE_POPUP;
>> shsurf->parent = wl_resource_get_user_data(parent_resource);
>> shsurf->popup.shseat = get_shell_seat(wl_resource_get_user_data(seat_resource));
>> @@ -3190,8 +3219,7 @@ move_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void *dat
>> return;
>>
>> shsurf = get_shell_surface(surface);
>> - if (shsurf == NULL || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
>> - shsurf->type == SHELL_SURFACE_MAXIMIZED)
>> + if (shsurf == NULL || shsurf->cur.fullscreen || shsurf->cur.maximized)
>> return;
>>
>> surface_move(shsurf, (struct weston_seat *) seat);
>> @@ -3209,8 +3237,7 @@ touch_move_binding(struct weston_seat *seat, uint32_t time, void *data)
>> return;
>>
>> shsurf = get_shell_surface(surface);
>> - if (shsurf == NULL || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
>> - shsurf->type == SHELL_SURFACE_MAXIMIZED)
>> + if (shsurf == NULL || shsurf->cur.fullscreen || shsurf->cur.maximized)
>> return;
>>
>> surface_touch_move(shsurf, (struct weston_seat *) seat);
>> @@ -3230,8 +3257,7 @@ resize_binding(struct weston_seat *seat, uint32_t time, uint32_t button, void *d
>> return;
>>
>> shsurf = get_shell_surface(surface);
>> - if (!shsurf || shsurf->type == SHELL_SURFACE_FULLSCREEN ||
>> - shsurf->type == SHELL_SURFACE_MAXIMIZED)
>> + if (!shsurf || shsurf->cur.fullscreen || shsurf->cur.maximized)
>> return;
>>
>> weston_view_from_global(shsurf->view,
>> @@ -3747,8 +3773,7 @@ rotate_binding(struct weston_seat *seat, uint32_t time, uint32_t button,
>> return;
>>
>> surface = get_shell_surface(base_surface);
>> - if (!surface || surface->type == SHELL_SURFACE_FULLSCREEN ||
>> - surface->type == SHELL_SURFACE_MAXIMIZED)
>> + if (!surface || surface->cur.fullscreen || surface->cur.maximized)
>> return;
>>
>> surface_rotate(surface, seat);
>> @@ -3776,6 +3801,7 @@ activate(struct desktop_shell *shell, struct weston_surface *es,
>> struct focus_state *state;
>> struct workspace *ws;
>> struct weston_surface *old_es;
>> + struct shell_surface *shsurf;
>>
>> main_surface = weston_surface_get_main_surface(es);
>>
>> @@ -3790,21 +3816,20 @@ activate(struct desktop_shell *shell, struct weston_surface *es,
>> wl_list_remove(&state->surface_destroy_listener.link);
>> wl_signal_add(&es->destroy_signal, &state->surface_destroy_listener);
>>
>> - switch (get_shell_surface_type(main_surface)) {
>> - case SHELL_SURFACE_FULLSCREEN:
>> + shsurf = get_shell_surface(main_surface);
>> + if (shsurf->cur.fullscreen) {
>> /* should on top of panels */
>> shell_stack_fullscreen(get_shell_surface(main_surface));
>> shell_configure_fullscreen(get_shell_surface(main_surface));
>> return;
>> - default:
>> - restore_all_output_modes(shell->compositor);
>> - ws = get_current_workspace(shell);
>> - main_view = get_default_view(main_surface);
>> - if (main_view)
>> - weston_view_restack(main_view, &ws->layer.view_list);
>> - break;
>> }
>>
>> + restore_all_output_modes(shell->compositor);
>> + ws = get_current_workspace(shell);
>> + main_view = get_default_view(main_surface);
>> + if (main_view)
>> + weston_view_restack(main_view, &ws->layer.view_list);
>> +
>> if (shell->focus_animation_type != ANIMATION_NONE)
>> animate_focus_change(shell, ws, get_default_view(old_es), get_default_view(es));
>> }
>> @@ -4241,20 +4266,23 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>> /* initial positioning, see also configure() */
>> switch (shsurf->type) {
>> case SHELL_SURFACE_TOPLEVEL:
>> - weston_view_set_initial_position(shsurf->view, shell);
>> - break;
>> - case SHELL_SURFACE_FULLSCREEN:
>> - center_on_output(shsurf->view, shsurf->fullscreen_output);
>> - shell_map_fullscreen(shsurf);
>> - break;
>> - case SHELL_SURFACE_MAXIMIZED:
>> - /* use surface configure to set the geometry */
>> - panel_height = get_output_panel_height(shell, shsurf->output);
>> - surface_subsurfaces_boundingbox(shsurf->surface,
>> - &surf_x, &surf_y, NULL, NULL);
>> - weston_view_set_position(shsurf->view,
>> - shsurf->output->x - surf_x,
>> - shsurf->output->y + panel_height - surf_y);
>> + if (shsurf->cur.fullscreen) {
>> + center_on_output(shsurf->view, shsurf->fullscreen_output);
>> + shell_map_fullscreen(shsurf);
>> + } else if (shsurf->cur.maximized) {
>> + /* use surface configure to set the geometry */
>> + panel_height = get_output_panel_height(shell,
>> + shsurf->output);
>> + surface_subsurfaces_boundingbox(shsurf->surface,
>> + &surf_x, &surf_y, NULL,
>> + NULL);
>> + weston_view_set_position(shsurf->view,
>> + shsurf->output->x - surf_x,
>> + shsurf->output->y +
>> + panel_height - surf_y);
>> + } else {
>> + weston_view_set_initial_position(shsurf->view, shell);
>> + }
>> break;
>> case SHELL_SURFACE_POPUP:
>> shell_map_popup(shsurf);
>> @@ -4280,11 +4308,12 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>> &shsurf->view->layer_link);
>> }
>> break;
>> - case SHELL_SURFACE_FULLSCREEN:
>> case SHELL_SURFACE_NONE:
>> break;
>> case SHELL_SURFACE_XWAYLAND:
>> default:
>> + if (shsurf->cur.fullscreen)
>> + break;
>> ws = get_current_workspace(shell);
>> wl_list_remove(&shsurf->view->layer_link);
>> wl_list_insert(&ws->layer.view_list, &shsurf->view->layer_link);
>> @@ -4293,7 +4322,7 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>>
>> if (shsurf->type != SHELL_SURFACE_NONE) {
>> weston_view_update_transform(shsurf->view);
>> - if (shsurf->type == SHELL_SURFACE_MAXIMIZED) {
>> + if (shsurf->cur.maximized) {
>> shsurf->surface->output = shsurf->output;
>> shsurf->view->output = shsurf->output;
>> }
>> @@ -4307,8 +4336,6 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>> WL_SHELL_SURFACE_TRANSIENT_INACTIVE)
>> break;
>> case SHELL_SURFACE_TOPLEVEL:
>> - case SHELL_SURFACE_FULLSCREEN:
>> - case SHELL_SURFACE_MAXIMIZED:
>> if (!shell->locked) {
>> wl_list_for_each(seat, &compositor->seat_list, link)
>> activate(shell, shsurf->surface, seat);
>> @@ -4318,7 +4345,8 @@ map(struct desktop_shell *shell, struct shell_surface *shsurf,
>> break;
>> }
>>
>> - if (shsurf->type == SHELL_SURFACE_TOPLEVEL)
>> + if ((shsurf->type == SHELL_SURFACE_TOPLEVEL) &&
>> + !(shsurf->cur.fullscreen || shsurf->cur.maximized))
>> {
>> switch (shell->win_animation_type) {
>> case ANIMATION_FADE:
>> @@ -4337,14 +4365,11 @@ static void
>> configure(struct desktop_shell *shell, struct weston_surface *surface,
>> float x, float y, int32_t width, int32_t height)
>> {
>> - enum shell_surface_type surface_type = SHELL_SURFACE_NONE;
>> struct shell_surface *shsurf;
>> struct weston_view *view;
>> int32_t surf_x, surf_y;
>>
>> shsurf = get_shell_surface(surface);
>> - if (shsurf)
>> - surface_type = shsurf->type;
>>
>> /* TODO:
>> * This should probably be changed to be more shell_surface
>> @@ -4353,31 +4378,29 @@ configure(struct desktop_shell *shell, struct weston_surface *surface,
>> wl_list_for_each(view, &surface->views, surface_link)
>> weston_view_configure(view, x, y, width, height);
>>
>> - switch (surface_type) {
>> - case SHELL_SURFACE_FULLSCREEN:
>> - shell_stack_fullscreen(shsurf);
>> - shell_configure_fullscreen(shsurf);
>> - break;
>> - case SHELL_SURFACE_MAXIMIZED:
>> - /* setting x, y and using configure to change that geometry */
>> - surface_subsurfaces_boundingbox(shsurf->surface, &surf_x, &surf_y,
>> - NULL, NULL);
>> - shsurf->view->geometry.x = shsurf->output->x - surf_x;
>> - shsurf->view->geometry.y = shsurf->output->y +
>> - get_output_panel_height(shell,shsurf->output) - surf_y;
>> - break;
>> - case SHELL_SURFACE_TOPLEVEL:
>> - break;
>> - default:
>> - break;
>> + if (shsurf) {
>
> The "what if shsurf is NULL" case is a left-over. It can never happen
> today, so just drop the if statement here.
>
>> + if (shsurf->cur.fullscreen) {
>> + shell_stack_fullscreen(shsurf);
>> + shell_configure_fullscreen(shsurf);
>> + } else if (shsurf->cur.maximized) {
>> + /* setting x, y and using configure to change that geometry */
>> + surface_subsurfaces_boundingbox(shsurf->surface,
>> + &surf_x, &surf_y,
>> + NULL, NULL);
>> + shsurf->view->geometry.x = shsurf->output->x - surf_x;
>> + shsurf->view->geometry.y = shsurf->output->y +
>> + get_output_panel_height(shell,shsurf->output) -
>> + surf_y;
>> + }
>> }
>>
>> +
>> /* XXX: would a fullscreen surface need the same handling? */
>> if (surface->output) {
>> wl_list_for_each(view, &surface->views, surface_link)
>> weston_view_update_transform(view);
>>
>> - if (surface_type == SHELL_SURFACE_MAXIMIZED)
>> + if (shsurf->cur.maximized)
>> surface->output = shsurf->output;
>> }
>> }
>> @@ -4398,8 +4421,9 @@ shell_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy, int32
>> if (width == 0)
>> return;
>>
>> - if (shsurf->next_type != SHELL_SURFACE_NONE &&
>> - shsurf->type != shsurf->next_type) {
>> + if ((shsurf->next_type != SHELL_SURFACE_NONE &&
>> + shsurf->type != shsurf->next_type) ||
>> + shsurf->state_changed) {
>> set_surface_type(shsurf);
>> type_changed = 1;
>> }
>> @@ -4836,8 +4860,6 @@ switcher_next(struct switcher *switcher)
>> wl_list_for_each(view, &ws->layer.view_list, layer_link) {
>> switch (get_shell_surface_type(view->surface)) {
>> case SHELL_SURFACE_TOPLEVEL:
>> - case SHELL_SURFACE_FULLSCREEN:
>> - case SHELL_SURFACE_MAXIMIZED:
>> if (first == NULL)
>> first = view->surface;
>> if (prev == switcher->current)
>> @@ -4872,7 +4894,7 @@ switcher_next(struct switcher *switcher)
>> view->alpha = 1.0;
>>
>> shsurf = get_shell_surface(switcher->current);
>> - if (shsurf && shsurf->type ==SHELL_SURFACE_FULLSCREEN)
>> + if (shsurf && shsurf->cur.fullscreen)
>> shsurf->fullscreen.black_view->alpha = 1.0;
>> }
>>
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
--
Rafael Antognolli
More information about the wayland-devel
mailing list