[PATCH 1/3] shell: restore app on non default and unplugged output
Ander Conselvan de Oliveira
conselvan2 at gmail.com
Tue Dec 3 07:05:47 PST 2013
Hi Xiong,
It seems that we need different logic for moving the views depending on
the surface role, so I'm thinking this code should actually be part of a
surface role implementation. For instance, instead of implementing the
logic of moving the pointer surface in the shell, that would go in
src/input.c together with the implementation of
pointer_cursor_surface_configure().
To achieve that, we could add an output_destroyed() function pointer in
weston_surface, that would be called from an output_destroy_listener in
weston_view. That should simplify the code below quite a bit since
traversing the list of views would be done by the wl_signal_emit() code.
The output_destroyed() entry point could receive the new output as a
parameter, so the role-specific logic would only have to determine where
in that output the new surface should be and move it. And of course
handle any role-specific needs, such as warping the pointer.
I have some comments about the coding style below.
On 12/03/2013 05:25 AM, Xiong Zhang wrote:
> if the unplugged output isn't the default output, move cursor
> and APP widow to default output
The commit message should be properly capitalized and punctuated.
>
> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
> src/shell.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 186 insertions(+), 4 deletions(-)
>
> diff --git a/src/shell.c b/src/shell.c
> index dfcb525..16d22f1 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -6061,15 +6061,197 @@ workspace_move_surface_down_binding(struct weston_seat *seat, uint32_t time,
> take_surface_to_workspace_by_seat(shell, seat, new_index);
> }
>
> +/* if (*x, *y) will be moved beyong target_output, clamp it*/
Beyond is misspelled and there's a space missing in the end. I'd also
recommend properly capitalizing and punctuating the sentences.
/* If (*x, *y) will be removed beyond target_output, clamp it. */
> +static int
> +clamp_coordinate(struct weston_output *target_output,
> + struct weston_output *original_output,
> + float *x, float *y)
The parameters should be aligned with the parenthesis, i.e.,
clamp_coordinate(struct weston_output *target_output,
struct weston_output *original_output,
> +{
> + int32_t rel_x, rel_y;
> + int result = 0;
> +
> + rel_x = *x - original_output->x;
> + rel_y = *y - original_output->y;
> +
> + if (rel_x > target_output->width) {
> + *x = original_output->x + target_output->width / 4;
> + result = 1;
> + }
> + if (rel_y > target_output->height) {
> + *y = original_output->y + target_output->height / 4;
> + result = 1;
> + }
> + return result;
> +}
> +
> +static void
> +handle_view_on_destroyed_output(struct weston_view *view,
> + struct weston_output *target_output)
> +{
> + float new_x, new_y, original_x, original_y;
> +
> + /* the child window's view->geometry is a relative coordinate to */
> + /* parent view, no need to move child_view */
We format multi-line comments like this:
/* First line.
* Second line. */
> + if (view->geometry.parent)
> + return;
> +
> + original_x = view->geometry.x;
> + original_y = view->geometry.y;
> +
> + clamp_coordinate(target_output, view->output,
> + &original_x, &original_y);
> +
> + new_x = target_output->x + original_x - view->output->x;
> + new_y = target_output->y + original_y - view->output->y;
> + weston_view_set_position(view, new_x, new_y);
> +}
> +
> +static void
> +normal_view_on_destroyed_output(struct shell_output *shell_output,
> + struct weston_output *target_output)
Parameter alignment.
> +{
> + struct weston_output *del_output = shell_output->output;
> + struct desktop_shell *shell = shell_output->shell;
> + struct weston_view *view;
> + unsigned int i;
> + struct workspace *ws;
> + struct shell_surface *shsurf;
> + enum shell_surface_type surface_type = SHELL_SURFACE_NONE;
> + struct wl_client *client;
> +
> + /*if view on destroyed output, move it to target output*/
Spaces around the comment.
> + for (i = 0; i < shell->workspaces.num; i++) {
> + ws = get_workspace(shell, i);
> + wl_list_for_each(view, &ws->layer.view_list, layer_link) {
> + if (view->output == del_output) {
> + handle_view_on_destroyed_output(view, target_output);
> + surface_type = get_shell_surface_type(view->surface);
A blank line here would make the whole block easier to read.
> + /*if surface is maximized, resize it to target output*/
> + if (surface_type == SHELL_SURFACE_MAXIMIZED) {
> + shsurf = get_shell_surface(view->surface);
> + shsurf->type = SHELL_SURFACE_NONE;
> + client = wl_resource_get_client(shsurf->resource);
> + shell_surface_set_maximized(client,
> + shsurf->resource,
> + wl_resource_find_for_client(&target_output->resource_list, client));
We also align parameters for multi-line function calls with the
parenthesis. This also breaks the 80 character limit. You can avoid this
by having an extra variable for the client_resource, i.e.:
resource = wl_resource_find_for_client();
shell_surface_set_maximized(client, shsurf->resource,
resource);
> + }
> + }
> + }
> + }
> +}
> +
> +static void
> +full_screen_view_on_destroyed_output(struct shell_output *shell_output,
> + struct weston_output *target_output)
Parameter alignement.
> +{
> + struct weston_output *del_output = shell_output->output;
> + struct desktop_shell *shell = shell_output->shell;
> + struct weston_view *view;
> + struct shell_surface *shsurf;
> +
> + /*if view on destroyed output is fullscreen, resize it and associated black_view */
> + /*to target_output */
Comment format.
> + shsurf = NULL;
> + wl_list_for_each(view, &shell->fullscreen_layer.view_list, layer_link) {
> + /* fullscreen.black_view followed fullscreen.view */
Comment format.
> + if (shsurf && shsurf->fullscreen.black_view == view) {
> + handle_view_on_destroyed_output(view, target_output);
> + weston_view_configure(view, target_output->x, target_output->y,
> + target_output->width, target_output->height);
> + pixman_region32_fini(&view->surface->opaque);
> + pixman_region32_init_rect(&view->surface->opaque, 0, 0,
> + target_output->width, target_output->height);
> + pixman_region32_fini(&view->surface->input);
> + pixman_region32_init_rect(&view->surface->input, 0, 0,
> + target_output->width, target_output->height);
> + continue;
> + }
> + shsurf = get_shell_surface(view->surface);
> + if (shsurf->fullscreen_output == del_output) {
> + handle_view_on_destroyed_output(view, target_output);
> + shsurf->type = SHELL_SURFACE_NONE;
> + set_fullscreen(shsurf,
> + shsurf->fullscreen.type,
> + shsurf->fullscreen.framerate,
> + target_output);
> + }
> + }
> +}
> +
> +static void
> +pointer_on_destroyed_output(struct shell_output *shell_output,
> + struct weston_output *target_output)
Parameter alignment.
> +{
> + struct weston_output *del_output = shell_output->output;
> + struct weston_compositor *wc = del_output->compositor;
> + struct weston_seat *seat;
> + struct weston_pointer *pointer;
> + wl_fixed_t dx, dy;
> + float pointer_x, pointer_y;
> + struct wl_list *resource_list;
> + struct wl_resource *resource;
> +
> + /*if cursor exist in destroyed output, issue a fake motion to move cursor to target_output*/
Comment format.
> + wl_list_for_each(seat, &wc->seat_list, link) {
> + if (!seat->pointer)
> + continue;
This whole block could use some blank lines to make it easier to read.
For instance, one here, and also ...
> + pointer = seat->pointer;
> + pointer_x = wl_fixed_to_double(pointer->x);
> + pointer_y = wl_fixed_to_double(pointer->y);
here,
> + if (!pixman_region32_contains_point(&del_output->region,
> + pointer_x, pointer_y, NULL))
> + continue;
here,
> + if (clamp_coordinate(target_output, del_output, &pointer_x, &pointer_y)) {
> + pointer->x = wl_fixed_from_double(pointer_x);
> + pointer->y = wl_fixed_from_double(pointer_y);
> + }
here,
> + dx = wl_fixed_from_int(target_output->x - del_output->x);
> + dy = wl_fixed_from_int(target_output->y - del_output->y);
> + pointer->x += dx;
> + pointer->y += dy;
here,
> + if (pointer->sprite)
> + handle_view_on_destroyed_output(pointer->sprite, target_output);
here
> + pointer->grab->interface->focus(pointer->grab);
> + resource_list = &pointer->focus_resource_list;
and here.
> + wl_resource_for_each(resource, resource_list) {
> + weston_view_from_global_fixed(pointer->focus,
> + pointer->x, pointer->y,
> + &dx, &dy);
> + wl_pointer_send_motion(resource,
> + weston_compositor_get_time(),
> + dx, dy);
> + }
> + }
> +}
> +
> static void
> handle_output_destroy(struct wl_listener *listener, void *data)
> {
> - struct shell_output *output_listener =
> + struct shell_output *shell_output =
> container_of(listener, struct shell_output, destroy_listener);
> + struct weston_output *output = (struct weston_output *)data;
> + struct weston_compositor *wc = output->compositor;
> + struct weston_output *default_output, *target_output = NULL;
> +
> + default_output = get_default_output(wc);
> + /* if default output is the only output in output_list, or destroyed
> + output is default output, return*/
Comment format.
> + if ((default_output->link.prev == default_output->link.next) ||
> + (default_output == output))
Align "(default_output) ..." with the parenthesis. Also, the goto below
is really unnecessary. You could just revert the condition and put the
code until the exit_handler label inside the if.
> + goto exit_handler;
> +
> + target_output = default_output;
> +
> + normal_view_on_destroyed_output(shell_output, target_output);
> + full_screen_view_on_destroyed_output(shell_output, target_output);
> + pointer_on_destroyed_output(shell_output, target_output);
> +
> + weston_output_schedule_repaint(target_output);
>
> - wl_list_remove(&output_listener->destroy_listener.link);
> - wl_list_remove(&output_listener->link);
> - free(output_listener);
> +exit_handler:
> + wl_list_remove(&shell_output->destroy_listener.link);
> + wl_list_remove(&shell_output->link);
> + free(shell_output);
> }
>
> static void
>
More information about the wayland-devel
mailing list