[PATCH 3/3] shell: Implement "driver" method of fullsceen.

[wuzhiwen]

Hi,
Thanks for your review, comments inlined.

>-----Original Message-----
>From:
>wayland-devel-bounces+zhiwen.wu=linux.intel.com at lists.freedesktop.org
>[mailto:wayland-devel-bounces+zhiwen.wu=linux.intel.com at lists.freedesktop.o
>rg] On Behalf Of Pekka Paalanen
>Sent: Wednesday, March 07, 2012 9:13 PM
>To: zhiwen.wu at linux.intel.com
>Cc: krh at bitplanet.net; juan.j.zhao at linux.intel.com;
>wayland-devel at lists.freedesktop.org
>Subject: Re: [PATCH 3/3] shell: Implement "driver" method of fullsceen.
>
>On Wed,  7 Mar 2012 17:01:27 +0800
>zhiwen.wu at linux.intel.com wrote:
>
>> From: Alex Wu <zhiwen.wu at linux.intel.com>
>>
>> Using the switch_mode hook to change the display mode.
>> ---
>>  src/shell.c |   34 +++++++++++++++++++++++++++++++++-
>>  1 files changed, 33 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/shell.c b/src/shell.c index 5d2c239..c8cb41e 100644
>> --- a/src/shell.c
>> +++ b/src/shell.c
>> @@ -121,6 +121,7 @@ struct shell_surface {
>>  		struct weston_transform transform; /* matrix from x, y */
>>  		uint32_t framerate;
>>  		struct weston_surface *black_surface;
>> +		struct weston_mode *saved_mode;
>>  	} fullscreen;
>>
>>  	struct weston_output *fullscreen_output; @@ -372,6 +373,14 @@ static
>> void  shell_unset_fullscreen(struct shell_surface *shsurf)  {
>>  	/* undo all fullscreen things here */
>> +	if (shsurf->fullscreen.saved_mode) {
>> +
shsurf->fullscreen_output->switch_mode(shsurf->fullscreen_output,
>> +				shsurf->fullscreen.saved_mode->width,
>> +				shsurf->fullscreen.saved_mode->height,
>> +				shsurf->fullscreen.saved_mode->refresh);
>> +		shsurf->fullscreen.saved_mode = NULL;
>> +	}
>> +
>>  	shsurf->fullscreen.type =
>WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT;
>>  	shsurf->fullscreen.framerate = 0;
>>  	wl_list_remove(&shsurf->fullscreen.transform.link);
>> @@ -572,7 +581,23 @@ shell_configure_fullscreen(struct shell_surface
>*shsurf)
>>  		weston_surface_set_position(surface, output->x, output->y);
>>  		break;
>>  	case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DRIVER:
>> -		break;
>> +		if (output->current->width == surface->geometry.width &&
>> +	            output->current->height == surface->geometry.height)
>> +			break;
>> +
>> +		shsurf->fullscreen.saved_mode = output->current;
>
>I don't think you really want to remember "the previous video mode of an
output
>for this particular surface". Saving it in surface makes it difficult to
restore the
>original mode for e.g. when switcher activates another window. If you
happen to
>end up twice in configure, like a game switching to another video mode
while
>already in one, you lose the original. Also, raising & activating another
fullscreen
>app over the first fullscreen app.
>
>I think you want to save the default mode in the output struct, or use the
>output's preferred mode for it (whatever compositor uses to choose the
default
>or initial mode).
[Wu, Zhiwen] Good idea.
>
>Does your implementation allow the compositor/shell to switch between
>mode-set and fallback fullscreen presentation at will?
>That's for the use cases we already discussed somewhere, like doing
mode-set
>only after the fullscreen window has been raised on top of everything and
is
>active? And undo mode-set when something else gets focused?
[Wu, Zhiwen] No for this version, I will add this in next one.
>
>That is about being smart and avoiding unnecessary mode switches when a
user
>juggles between windows. If there is a regular top-level window on top of
the
>fullscreen mode-switched window, we probably want to (at least the option
to)
>switch to the default mode.
[Wu, Zhiwen]I have a question here: after the top-level window destroyed, do
we need
to automatically switch back to the mode the fullscreen window wanted? If
yes, how
can we do that? In the destroy_shell_surface()?
Another idea is, for the desktop shell, to minimize the mode-switched window
when a regular top-level window on top of it,
and switch mode to default. When the mode-switched window is top most again
(e.g. raised by switcher, or activate by clicking),
switch to the mode it wanted.

>
>All this mode switching should be encapsulated so that the shell can call
>functions for going to default mode for an output, and doing mode-set on an
>output for a particular surface.
>
>I hope that makes sense somehow, explaining it is a bit difficult for me
today.
>
>
>Thanks,
>pq
>_______________________________________________
>wayland-devel mailing list
>wayland-devel at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/wayland-devel