Hi, Thanks for your review, comments inlined. >-----Original Message----- >From: >wayland-devel-bounces+zhiwen.wu=linux.intel....@lists.freedesktop.org >[mailto:wayland-devel-bounces+zhiwen.wu=linux.intel.com@lists.freedesktop.o >rg] On Behalf Of Pekka Paalanen >Sent: Wednesday, March 07, 2012 9:13 PM >To: [email protected] >Cc: [email protected]; [email protected]; >[email protected] >Subject: Re: [PATCH 3/3] shell: Implement "driver" method of fullsceen. > >On Wed, 7 Mar 2012 17:01:27 +0800 >[email protected] wrote: > >> From: Alex Wu <[email protected]> >> >> 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 >[email protected] >http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
