On Tue, Feb 28, 2012 at 06:08:38PM +0800, zhiwen...@linux.intel.com wrote:
> From: Alex Wu <zhiwen...@linux.intel.com>
>
> All the fullscreen things (black surface, raise atop panels, transform,
> positioning)
> are handled in map() or configure().
Ok, looking good, we're almost there now :) A few comments below (and
please try to keep the commit message less than 78 chars wide).
thanks,
Kristian
> Signed-off-by: Alex Wu <zhiwen...@linux.intel.com>
> Signed-off-by: Juan Zhao <juan.j.z...@linux.intel.com>
> ---
> src/shell.c | 231
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 214 insertions(+), 17 deletions(-)
>
> diff --git a/src/shell.c b/src/shell.c
> index d949d0c..add9547 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -38,6 +38,8 @@
>
> struct shell_surface;
>
> +#define INVALID_X (-1)
> +#define INVALID_Y (-1)
> struct wl_shell {
> struct weston_compositor *compositor;
> struct weston_shell shell;
> @@ -108,6 +110,13 @@ struct shell_surface {
> int32_t initial_up;
> } popup;
>
> + struct {
> + enum wl_shell_surface_fullscreen_method type;
> + struct weston_transform transform; /* matrix from x, y */
> + uint32_t framerate;
> + struct weston_surface *black_surface;
> + } fullscreen;
> +
> struct weston_output *fullscreen_output;
> struct weston_output *output;
> struct wl_list link;
> @@ -134,6 +143,14 @@ struct rotate_grab {
> };
>
> static void
> +activate(struct weston_shell *base, struct weston_surface *es,
> + struct weston_input_device *device, uint32_t time);
> +
> +static void
> +center_on_output(struct weston_surface *surface,
> + struct weston_output *output);
> +
> +static void
> shell_configuration(struct wl_shell *shell)
> {
> char *config_file;
> @@ -303,7 +320,8 @@ weston_surface_resize(struct shell_surface *shsurf,
> {
> struct weston_resize_grab *resize;
>
> - /* FIXME: Reject if fullscreen */
> + if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> + return 0;
>
> if (edges == 0 || edges > 15 ||
> (edges & 3) == 3 || (edges & 12) == 12)
> @@ -334,7 +352,8 @@ shell_surface_resize(struct wl_client *client, struct
> wl_resource *resource,
> struct weston_input_device *wd = input_resource->data;
> struct shell_surface *shsurf = resource->data;
>
> - /* FIXME: Reject if fullscreen */
> + if (shsurf->type == SHELL_SURFACE_FULLSCREEN)
> + return;
>
> if (wd->input_device.button_count == 0 ||
> wd->input_device.grab_time != time ||
> @@ -352,15 +371,22 @@ get_default_output(struct weston_compositor *compositor)
> struct weston_output, link);
> }
>
> +static void
> +shell_unset_fullscreen(struct shell_surface *shsurf)
> +{
> + shsurf->fullscreen_output = NULL;
> + shsurf->fullscreen.type = WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT;
> + shsurf->surface->force_configure = 1;
> + wl_list_remove(&shsurf->fullscreen.transform.link);
> + wl_list_init(&shsurf->fullscreen.transform.link);
> +}
> +
> static int
> reset_shell_surface_type(struct shell_surface *surface)
> {
> switch (surface->type) {
> case SHELL_SURFACE_FULLSCREEN:
> - weston_surface_set_position(surface->surface,
> - surface->saved_x,
> - surface->saved_y);
> - surface->fullscreen_output = NULL;
> + shell_unset_fullscreen(surface);
> break;
> case SHELL_SURFACE_MAXIMIZED:
> surface->output =
> get_default_output(surface->surface->compositor);
> @@ -487,6 +513,109 @@ shell_surface_set_maximized(struct wl_client *client,
> shsurf->type = SHELL_SURFACE_MAXIMIZED;
> }
>
> +static struct weston_surface *
> +create_black_surface(struct weston_compositor *ec)
> +{
> + struct weston_surface *surface = NULL;
> +
> + surface = weston_surface_create(ec);
> + if (surface == NULL) {
> + fprintf(stderr, "no memory\n");
> + return NULL;
> + }
> +
> + weston_surface_configure(surface, 0, 0, 8192, 8192);
> + weston_surface_set_color(surface, 0.0, 0.0, 0.0, 1);
This surface (8192x8192 at 0,0) will cover all outputs. We need to
make a surface for a specific output such that the size and position
matches the output.
> + return surface;
> +}
> +
> +/*
> + * Handle size dismatch and positioning according to the method.
> + */
> +static void
> +shell_configure_fullscreen(struct shell_surface *shsurf)
> +{
> + struct weston_output *output = shsurf->fullscreen_output;
> + struct weston_surface *surface = shsurf->surface;
> + struct weston_matrix *matrix;
> + float scale;
> +
> + center_on_output(surface, output);
> +
> + switch (shsurf->fullscreen.type) {
> + case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT:
> + break;
> + case WL_SHELL_SURFACE_FULLSCREEN_METHOD_SCALE:
> + matrix = &shsurf->fullscreen.transform.matrix;
> + weston_matrix_init(matrix);
> + scale =
> (float)output->current->width/(float)surface->geometry.width;
> + weston_matrix_scale(matrix, scale, scale, 1);
> + wl_list_remove(&shsurf->fullscreen.transform.link);
> + wl_list_insert(surface->geometry.transformation_list.prev,
> + &shsurf->fullscreen.transform.link);
> + weston_surface_set_position(surface, output->x, output->y);
> + break;
> + case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER:
> + break;
> + case WL_SHELL_SURFACE_FULLSCREEN_METHOD_FILL:
> + break;
> + default:
> + break;
> + }
> +}
> +
> +/*
> + * Handle the stacking order of the fullscreen surface and the associated
> black surface.
> + */
> +static void
> +shell_stack_fullscreen(struct shell_surface *shsurf, bool mapped)
> +{
> + struct weston_surface *surface = shsurf->surface;
> + struct wl_shell *shell = shell_surface_get_shell(shsurf);
> + struct wl_list *list;
> +
> + if (mapped) {
> + wl_list_remove(&surface->link);
> + wl_list_remove(&shsurf->fullscreen.black_surface->link);
> + }
> +
> + if (shell->locked) {
> + wl_list_insert(&shell->hidden_surface_list, &surface->link);
> + wl_list_insert(&surface->link,
> &shsurf->fullscreen.black_surface->link);
> + } else {
> + list = weston_compositor_top(surface->compositor);
> + wl_list_insert(list, &surface->link);
> + wl_list_insert(&surface->link,
> &shsurf->fullscreen.black_surface->link);
> +
> + /*assign output*/
> + surface->output = shsurf->fullscreen_output;
> + shsurf->fullscreen.black_surface->output =
> shsurf->fullscreen_output;
> + if (!wl_list_empty(&surface->frame_callback_list)) {
> +
> wl_list_insert_list(surface->output->frame_callback_list.prev,
> + &surface->frame_callback_list);
> + wl_list_init(&surface->frame_callback_list);
> + }
> +
> + weston_compositor_repick(surface->compositor);
> + weston_surface_damage(surface);
> + weston_surface_damage(shsurf->fullscreen.black_surface);
> + }
> +}
> +
> +static void
> +shell_map_fullscreen(struct shell_surface *shsurf)
> +{
> + shell_configure_fullscreen(shsurf);
> + shell_stack_fullscreen(shsurf, false);
> +}
> +
> +static void
> +shell_raise_fullscreen(struct shell_surface *shsurf)
> +{
> + shell_configure_fullscreen(shsurf);
> + shell_stack_fullscreen(shsurf, true);
> +}
> +
> static void
> shell_surface_set_fullscreen(struct wl_client *client,
> struct wl_resource *resource,
> @@ -496,21 +625,28 @@ shell_surface_set_fullscreen(struct wl_client *client,
> {
> struct shell_surface *shsurf = resource->data;
> struct weston_surface *es = shsurf->surface;
> - struct weston_output *output;
> +
> + if (output_resource)
> + shsurf->output = output_resource->data;
> + else
> + shsurf->output = get_default_output(es->compositor);
>
> if (reset_shell_surface_type(shsurf))
> return;
>
> - /* FIXME: Fullscreen on first output */
> - /* FIXME: Handle output going away */
> - output = get_default_output(es->compositor);
> -
> shsurf->saved_x = es->geometry.x;
> shsurf->saved_y = es->geometry.y;
> - shsurf->output = output;
> - shsurf->fullscreen_output = output;
> + shsurf->fullscreen_output = shsurf->output;
> + shsurf->fullscreen.type = method;
> + shsurf->fullscreen.framerate = framerate;
> shsurf->type = SHELL_SURFACE_FULLSCREEN;
>
> + if (shsurf->fullscreen.black_surface == NULL)
> + shsurf->fullscreen.black_surface =
> create_black_surface(es->compositor);
> +
> + if (es->output)
> + shsurf->surface->force_configure = 1;
> +
> wl_resource_post_event(resource,
> WL_SHELL_SURFACE_CONFIGURE,
> weston_compositor_get_time(), 0,
> @@ -653,6 +789,9 @@ destroy_shell_surface(struct wl_resource *resource)
> if (shsurf->surface)
> wl_list_remove(&shsurf->surface_destroy_listener.link);
>
> + if (shsurf->fullscreen.black_surface)
> + weston_surface_destroy(shsurf->fullscreen.black_surface);
> +
I think we should just destroy the black surface in configure() when
we switch away from fullscreen. It's cheap to create and destroy and
there's no reason to keep it around.
> wl_list_remove(&shsurf->link);
> free(shsurf);
> }
> @@ -715,7 +854,15 @@ shell_get_shell_surface(struct wl_client *client,
> (void (**)(void)) &shell_surface_implementation;
> shsurf->resource.data = shsurf;
>
> + shsurf->saved_x = INVALID_X;
> + shsurf->saved_y = INVALID_Y;
-1,-1 is a valid surface position and we can't use that to indicate
that the surface never received an initial top-level position. We
could use INT32_MIN, I guess, or just add a saved_position_valid flag
to the struct.
> shsurf->surface = surface;
> + shsurf->fullscreen.type = WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT;
> + shsurf->fullscreen.framerate = 0;
> + shsurf->fullscreen.black_surface = NULL;
> + wl_list_init(&shsurf->fullscreen.transform.link);
> + weston_matrix_init(&shsurf->fullscreen.transform.matrix);
Shouldn't need to init the matrix here.
> +
> shsurf->surface_destroy_listener.func = shell_handle_surface_destroy;
> wl_list_insert(surface->surface.resource.destroy_listener_list.prev,
> &shsurf->surface_destroy_listener.link);
> @@ -1274,6 +1421,9 @@ activate(struct weston_shell *base, struct
> weston_surface *es,
> &es->link);
> }
> break;
> + case SHELL_SURFACE_FULLSCREEN:
> + /* should on top of panels*/
> + break;
> default:
> if (!shell->locked) {
> list = weston_compositor_top(compositor);
> @@ -1290,14 +1440,22 @@ activate(struct weston_shell *base, struct
> weston_surface *es,
>
> static void
> click_to_activate_binding(struct wl_input_device *device,
> - uint32_t time, uint32_t key,
> + uint32_t time, uint32_t key,
> uint32_t button, uint32_t state, void *data)
> {
> struct weston_input_device *wd = (struct weston_input_device *) device;
> struct weston_compositor *compositor = data;
> struct weston_surface *focus;
> + struct weston_surface *upper;
>
> focus = (struct weston_surface *) device->pointer_focus;
> + upper = container_of(focus->link.prev, struct weston_surface, link);
> + if (get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN) {
> + printf("%s: focus is black surface, raise its fullscreen
> surf\n", __func__);
> + shell_raise_fullscreen (get_shell_surface(upper));
> + focus = upper;
> + }
> +
This causes a crash when I click in a just started compositor (no
clients, except desktop-shell). You can't just follow the prev
pointer like that, it may point to compositor->surface_list. Instead
you have to
if (focus->link.prev != &compositor->surface_list &&
get_shell_surface_type(upper) == SHELL_SURFACE_FULLSCREEN) {
> if (state && focus && device->pointer_grab ==
> &device->default_pointer_grab)
> activate(compositor->shell, focus, wd, time);
> }
> @@ -1438,9 +1596,11 @@ map(struct weston_shell *base, struct weston_surface
> *surface,
> 10 + random() % 400);
> break;
> case SHELL_SURFACE_SCREENSAVER:
> - case SHELL_SURFACE_FULLSCREEN:
> center_on_output(surface, shsurf->fullscreen_output);
> break;
> + case SHELL_SURFACE_FULLSCREEN:
> + shell_map_fullscreen(shsurf);
> + break;
> case SHELL_SURFACE_MAXIMIZED:
> /*use surface configure to set the geometry*/
> panel_height = get_output_panel_height(shell,surface->output);
> @@ -1490,6 +1650,9 @@ map(struct weston_shell *base, struct weston_surface
> *surface,
> }
> do_configure = 0;
> break;
> + case SHELL_SURFACE_FULLSCREEN:
> + do_configure = 0;
> + break;
> case SHELL_SURFACE_NONE:
> do_configure = 0;
> break;
> @@ -1520,7 +1683,7 @@ map(struct weston_shell *base, struct weston_surface
> *surface,
> if (!shell->locked)
> activate(base, surface,
> (struct weston_input_device *)
> - compositor->input_device,
> + compositor->input_device,
> weston_compositor_get_time());
> break;
> default:
> @@ -1538,6 +1701,7 @@ configure(struct weston_shell *base, struct
> weston_surface *surface,
> struct wl_shell *shell = container_of(base, struct wl_shell, shell);
> enum shell_surface_type surface_type = SHELL_SURFACE_NONE;
> struct shell_surface *shsurf;
> + bool do_activate = false;
>
> shsurf = get_shell_surface(surface);
> if (shsurf)
> @@ -1551,15 +1715,42 @@ configure(struct weston_shell *base, struct
> weston_surface *surface,
>
> switch (surface_type) {
> case SHELL_SURFACE_SCREENSAVER:
> - case SHELL_SURFACE_FULLSCREEN:
> center_on_output(surface, shsurf->fullscreen_output);
> break;
> + case SHELL_SURFACE_FULLSCREEN:
> + if (wl_list_empty(&shsurf->fullscreen.black_surface->link)) {
> + shell_raise_fullscreen(shsurf);
> + do_activate = true;
> + } else {
> + /*just do the positioning*/
> + shell_configure_fullscreen(shsurf);
> + }
> + break;
> case SHELL_SURFACE_MAXIMIZED:
> /*setting x, y and using configure to change that geometry*/
> surface->geometry.x = surface->output->x;
> surface->geometry.y = surface->output->y +
> get_output_panel_height(shell,surface->output);
> break;
> + case SHELL_SURFACE_TOPLEVEL:
> + if (!wl_list_empty(&shsurf->fullscreen.black_surface->link)) {
I think we want to save the previous surface type, and then do this in
a separate switch on shsurf->previous_type above this one. That
switch will undo/tear down whatever the previous surface type was, and
the second switch will be responsible for setting up for the new
surface type.
> + wl_list_remove(&surface->link);
> + wl_list_init(&surface->link);
> +
> weston_surface_damage_below(shsurf->fullscreen.black_surface);
> + wl_list_remove(&shsurf->fullscreen.black_surface->link);
> + wl_list_init(&shsurf->fullscreen.black_surface->link);
> + shsurf->fullscreen.black_surface->output = NULL;
> + if (shsurf->saved_x != INVALID_X &&
> + shsurf->saved_y != INVALID_Y &&
> + shsurf->saved_x != surface->geometry.x
> &&
> + shsurf->saved_y != surface->geometry.y)
> {
> + weston_surface_set_position(surface,
> + shsurf->saved_x,
> + shsurf->saved_y);
> + }
If saved_x/y wasn't valid we need to pick an initial position in the else
clause of this if statement.
> + do_activate = true;
Why do we activate when we go from fullscreen to toplevel?
> + }
> + break;
> default:
> break;
> }
> @@ -1573,6 +1764,12 @@ configure(struct weston_shell *base, struct
> weston_surface *surface,
> else if (surface_type == SHELL_SURFACE_MAXIMIZED)
> surface->output = shsurf->output;
> }
> +
> + if (do_activate && !shell->locked)
> + activate(surface->compositor->shell, surface,
> + (struct weston_input_device *)
> + surface->compositor->input_device,
> + weston_compositor_get_time());
> }
>
> static int launch_desktop_shell_process(struct wl_shell *shell);
> --
> 1.7.5.4
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
