[PATCH 2/2] Notify clients on mode_switch()
Jason Ekstrand
jason at jlekstrand.net
Fri Sep 13 09:21:50 PDT 2013
I'll try to recap IRC.
There is an error in the way you implemented weston_switch_mode in the
following scenario: A client sets a fullscreen surface at the same
resolution and refresh rate as the "native" mode. After this, a native
mode switch occurs for some reason. Because origin_mode == native_mode,
your implementation will not detect the fullscreen surface and will
mode-switch out from under the client. My recommendation would be to use
origin_mode != NULL to indicate that the current mode is temporary.
Also, I think it would be better to call it MODE_SWITCH_SET_TEMPORARY
rather than MODE_SWITCH_SET_FULLSCREEN. The former is more descriptive of
what is actually happening to the output and the latter is highly wl_shell
dependent (there may be temporary switches that are not caused by a
fullscreen surface).
--Jason Ekstrand
On Tue, Sep 10, 2013 at 5:28 PM, Hardening <rdp.effort at gmail.com> wrote:
> This patch implements the notification of clients during mode_switch.
> As discussed on IRC, clients are notified of mode_switch only when the
> "native" mode is changed and activated. That means that if the native
> mode is changed and the compositor had activated a temporary mode for
> a fullscreen surface, the clients will be notified only when the native
> mode is restored.
> The scaling factor is treated the same way as modes.
> ---
> src/compositor-rdp.c | 28 +++++++------
> src/compositor.c | 109
> +++++++++++++++++++++++++++++++++++++++++++++++----
> src/compositor.h | 13 +++++-
> src/shell.c | 12 +++---
> 4 files changed, 135 insertions(+), 27 deletions(-)
>
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 642a6b7..80b340d 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -372,9 +372,10 @@ rdp_switch_mode(struct weston_output *output, struct
> weston_mode *target_mode) {
> if(local_mode == output->current_mode)
> return 0;
>
> - output->current_mode->flags = 0;
> + output->current_mode->flags &= ~WL_OUTPUT_MODE_CURRENT;
> +
> output->current_mode = local_mode;
> - output->current_mode->flags = WL_OUTPUT_MODE_CURRENT |
> WL_OUTPUT_MODE_PREFERRED;
> + output->current_mode->flags |= WL_OUTPUT_MODE_CURRENT;
>
> pixman_renderer_output_destroy(output);
> pixman_renderer_output_create(output);
> @@ -466,7 +467,7 @@ rdp_compositor_create_output(struct rdp_compositor *c,
> int width, int height,
> goto out_free_output_and_modes;
> }
>
> - output->base.current_mode = currentMode;
> + output->base.current_mode = output->base.native_mode = currentMode;
> weston_output_init(&output->base, &c->base, 0, 0, width, height,
> WL_OUTPUT_TRANSFORM_NORMAL, 1);
>
> @@ -679,23 +680,26 @@ xf_peer_post_connect(freerdp_peer* client)
> output = c->output;
> settings = client->settings;
>
> - if(!settings->SurfaceCommandsEnabled) {
> + if (!settings->SurfaceCommandsEnabled) {
> weston_log("client doesn't support required
> SurfaceCommands\n");
> return FALSE;
> }
>
> - if(output->base.width != (int)settings->DesktopWidth ||
> + if (output->base.width != (int)settings->DesktopWidth ||
> output->base.height !=
> (int)settings->DesktopHeight)
> {
> - if(!settings->DesktopResize) {
> - weston_log("client don't support
> desktopResize()\n");
> + struct weston_mode new_mode;
> + struct weston_mode *target_mode;
> + new_mode.width = (int)settings->DesktopWidth;
> + new_mode.height = (int)settings->DesktopHeight;
> + target_mode = find_matching_mode(&output->base, &new_mode);
> + if (!target_mode) {
> + weston_log("client mode not found\n");
> return FALSE;
> }
> -
> - /* force the client size */
> - settings->DesktopWidth = output->base.width;
> - settings->DesktopHeight = output->base.height;
> - client->update->DesktopResize(client->context);
> + weston_output_switch_mode(&output->base, target_mode, 1,
> WESTON_MODE_SWITCH_SET_NATIVE);
> + output->base.width = new_mode.width;
> + output->base.height = new_mode.height;
> }
>
> weston_log("kbd_layout:%x kbd_type:%x kbd_subType:%x
> kbd_functionKeys:%x\n",
> diff --git a/src/compositor.c b/src/compositor.c
> index 4787857..d6ec316 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -96,20 +96,88 @@ static void
> weston_compositor_build_surface_list(struct weston_compositor
> *compositor);
>
> WL_EXPORT int
> -weston_output_switch_mode(struct weston_output *output, struct
> weston_mode *mode, int32_t scale)
> +weston_output_switch_mode(struct weston_output *output, struct
> weston_mode *mode,
> + int32_t scale, enum weston_mode_switch_op op)
> {
> struct weston_seat *seat;
> + struct wl_resource *resource;
> pixman_region32_t old_output_region;
> - int ret;
> + struct weston_mode old_mode;
> + int ret, notify_mode_changed, notify_scale_changed;
> + int fullscreen_mode, fullscreen_scale;
>
> if (!output->switch_mode)
> return -1;
>
> - ret = output->switch_mode(output, mode);
> - if (ret < 0)
> - return ret;
> + fullscreen_mode = (output->current_mode != output->original_mode);
> + fullscreen_scale = (output->current_scale !=
> output->original_scale);
> + ret = 0;
> +
> + notify_mode_changed = 0;
> + notify_scale_changed = 0;
> + switch(op) {
> + case WESTON_MODE_SWITCH_SET_NATIVE:
> + old_mode.width = output->native_mode->width;
> + old_mode.height = output->native_mode->height;
> + old_mode.refresh = output->native_mode->refresh;
> + old_mode.flags = output->native_mode->flags;
> +
> + output->native_mode = mode;
> + if (!fullscreen_mode) {
> + notify_mode_changed = 1;
> + ret = output->switch_mode(output, mode);
> + if (ret < 0)
> + return ret;
> + }
> +
> + output->native_scale = scale;
> + if(!fullscreen_scale) {
> + notify_scale_changed = 1;
> + }
> + break;
> + case WESTON_MODE_SWITCH_SET_FULLSCREEN:
> + if (!fullscreen_mode)
> + output->original_mode = output->native_mode;
> + if (!fullscreen_scale)
> + output->original_scale = output->native_scale;
> +
> + ret = output->switch_mode(output, mode);
> + if (ret < 0)
> + return ret;
> +
> + output->current_mode = mode;
> + output->current_scale = scale;
> + break;
> + case WESTON_MODE_SWITCH_RESTORE_NATIVE:
> + if (!fullscreen_mode) {
> + weston_log("already in the native mode\n");
> + return -1;
> + }
>
> - output->current_scale = scale;
> + if (output->original_mode != output->native_mode) {
> + notify_mode_changed = 1;
> + old_mode.width = output->original_mode->width;
> + old_mode.height = output->original_mode->height;
> + old_mode.refresh = output->original_mode->refresh;
> + old_mode.flags = output->original_mode->flags;
> + output->original_mode = output->native_mode;
> + }
> +
> + ret = output->switch_mode(output, mode);
> + if (ret < 0)
> + return ret;
> +
> + if (output->original_scale != output->native_scale) {
> + notify_scale_changed = 1;
> + scale = output->native_scale;
> + }
> +
> + output->current_scale = output->native_scale;
> + break;
> + default:
> + weston_log("unknown weston_switch_mode_op %d\n", op);
> + break;
> + }
>
> pixman_region32_init(&old_output_region);
> pixman_region32_copy(&old_output_region, &output->region);
> @@ -152,6 +220,31 @@ weston_output_switch_mode(struct weston_output
> *output, struct weston_mode *mode
>
> pixman_region32_fini(&old_output_region);
>
> + /* notify clients of the changes */
> + if (notify_mode_changed || notify_scale_changed) {
> + wl_resource_for_each(resource, &output->resource_list) {
> + if(notify_mode_changed) {
> + wl_output_send_mode(resource,
> + old_mode.flags &
> ~WL_OUTPUT_MODE_CURRENT,
> + old_mode.width,
> + old_mode.height,
> + old_mode.refresh);
> +
> + wl_output_send_mode(resource,
> + mode->flags |
> WL_OUTPUT_MODE_CURRENT,
> + mode->width,
> + mode->height,
> + mode->refresh);
> + }
> +
> + if (notify_scale_changed)
> + wl_output_send_scale(resource, scale);
> +
> + if (wl_resource_get_version(resource) >= 2)
> + wl_output_send_done(resource);
> + }
> + }
> +
> return ret;
> }
>
> @@ -1873,7 +1966,7 @@ weston_subsurface_commit_to_cache(struct
> weston_subsurface *sub)
> * If this commit would cause the surface to move by the
> * attach(dx, dy) parameters, the old damage region must be
> * translated to correspond to the new surface coordinate system
> - * origin.
> + * original_mode.
> */
> pixman_region32_translate(&sub->cached.damage,
> -surface->pending.sx, -surface->
> pending.sy);
> @@ -2727,7 +2820,7 @@ weston_output_transform_scale_init(struct
> weston_output *output, uint32_t transf
> break;
> }
>
> - output->current_scale = scale;
> + output->native_scale = output->current_scale = scale;
> output->width /= scale;
> output->height /= scale;
> }
> diff --git a/src/compositor.h b/src/compositor.h
> index 49365fd..7d6b059 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -172,6 +172,12 @@ enum dpms_enum {
> WESTON_DPMS_OFF
> };
>
> +enum weston_mode_switch_op {
> + WESTON_MODE_SWITCH_SET_NATIVE,
> + WESTON_MODE_SWITCH_SET_FULLSCREEN,
> + WESTON_MODE_SWITCH_RESTORE_NATIVE
> +};
> +
> struct weston_output {
> uint32_t id;
> char *name;
> @@ -201,11 +207,13 @@ struct weston_output {
> char *make, *model, *serial_number;
> uint32_t subpixel;
> uint32_t transform;
> + int32_t native_scale;
> int32_t current_scale;
> + int32_t original_scale;
>
> + struct weston_mode *native_mode;
> struct weston_mode *current_mode;
> struct weston_mode *original_mode;
> - int32_t original_scale;
> struct wl_list mode_list;
>
> void (*start_repaint_loop)(struct weston_output *output);
> @@ -1207,7 +1215,8 @@ void
> weston_surface_destroy(struct weston_surface *surface);
>
> int
> -weston_output_switch_mode(struct weston_output *output, struct
> weston_mode *mode, int32_t scale);
> +weston_output_switch_mode(struct weston_output *output, struct
> weston_mode *mode,
> + int32_t scale, enum weston_mode_switch_op op);
>
> int
> noop_renderer_init(struct weston_compositor *ec);
> diff --git a/src/shell.c b/src/shell.c
> index cd94aa5..fca4f3c 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -1624,11 +1624,12 @@ get_default_output(struct weston_compositor
> *compositor)
> static void
> restore_output_mode(struct weston_output *output)
> {
> - if (output->current != output->origin ||
> - (int32_t)output->scale != output->origin_scale)
> + if (output->current_mode != output->original_mode ||
> + (int32_t)output->current_scale != output->original_scale)
> weston_output_switch_mode(output,
> - output->origin,
> - output->origin_scale);
> + output->original_mode,
> + output->original_scale,
> +
> WESTON_MODE_SWITCH_RESTORE_NATIVE);
> }
>
> static void
> @@ -1955,7 +1956,8 @@ shell_configure_fullscreen(struct shell_surface
> *shsurf)
> surf_height * surface->buffer_scale,
> shsurf->fullscreen.framerate};
>
> - if (weston_output_switch_mode(output, &mode,
> surface->buffer_scale) == 0) {
> + if (weston_output_switch_mode(output, &mode,
> surface->buffer_scale,
> + WESTON_MODE_SWITCH_SET_FULLSCREEN)
> == 0) {
> weston_surface_set_position(surface,
> output->x -
> surf_x,
> output->y -
> surf_y);
> --
> 1.8.1.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130913/fb1e2155/attachment-0001.html>
More information about the wayland-devel
mailing list