[PATCH 2/2] Notify clients on mode_switch()

[Pekka Paalanen]

On Wed, 11 Sep 2013 00:28:50 +0200
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(-)

...

> 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);

Hi,

is it really necessary to send the old mode again without the "current"
flag? Are clients not supposed to handle it with just the new mode,
since having more than one mode current does not make sense?

The protocol is slightly awkward here to begin with, because for each
mode event a client needs to iterate over all old modes, check if there
is a matching one: if yes, update flags; if not, add mode.


Thanks,
pq

> +
> +				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;
>  }