[Spice-devel] [PATCH spice 3/4] spicec-x11: Change WmSizeHints in fullscreen mode

[Hans de Goede]

Hi,

On 10/17/2010 03:50 PM, Arnon Gilboa wrote:
> Ack, with 2 notes below
>

Thanks for the reviews!

> Hans de Goede wrote:
>> Some window managers will ignore the fullscreen hint, unless WmSizeHints
>> allow them to resize the window so that they can give it the size of
>> the roo-window. This fixes fullscreen mode in compiz.
>> ---
>> client/x11/red_window.cpp | 50 ++++++++++++++++++++++++++++----------------
>> client/x11/red_window_p.h | 9 +++++--
>> 2 files changed, 38 insertions(+), 21 deletions(-)
>>
>> diff --git a/client/x11/red_window.cpp b/client/x11/red_window.cpp
>> index e7b6815..8f6535b 100644
>> --- a/client/x11/red_window.cpp
>> +++ b/client/x11/red_window.cpp
>> @@ -1063,18 +1063,27 @@ void RedWindow_p::set_glx(int width, int height)
>> }
>> #endif // USE_OGL
>>
>> -void RedWindow_p::set_minmax(PixelsSource_p& pix_source, int width, int height)
>> +void RedWindow_p::set_minmax(PixelsSource_p& pix_source)
>> {
>> //todo: auto res
>> XSizeHints* size_hints = XAllocSizeHints();
>> ASSERT(size_hints);
>> size_hints->flags = PMinSize | PMaxSize;
>> - size_hints->min_width = size_hints->max_width = width;
>> - size_hints->min_height = size_hints->max_height = height;
>> + if (_red_window->_type == RedWindow::TYPE_FULLSCREEN) {
>> + /* Some window managers won't allow full screen mode with a fixed
>> + width / height */
> so u need to give them the complete theoretical range?

Well I also tried with:
size_hints->min_width = _width;
size_hints->max_width = _width + 1;
size_hints->min_height = _height;
size_hints->max_height = _height + 1;

And that works fine for compiz too (I checked the compiz
code to see why fullscreen was not working). The point is that
(some) window managers want to be able to resize the window
to fill the entire root window. Given that the resolution
we are running at can change while we stay fullscreen, and
that some window managers may actually check if the window
dimensions fit the resolution X is running at (*) this seems
the safest option.

*) compiz only checks that min_width != max_width, iow
that the window is resizable not if it the given limits
will actually allow it to resize to screen filling size.

>> + size_hints->min_width = 1;
>> + size_hints->max_width = 65535;
>> + size_hints->min_height = 1;
>> + size_hints->max_height = 65535;
>> + } else {
>> + size_hints->min_width = size_hints->max_width = _width;
>> + size_hints->min_height = size_hints->max_height = _height;
>> + }
>> XSetWMNormalHints(x_display, _win, size_hints);
>> XFree(size_hints);
>> - pix_source.x_drawable.height = height;
>> - pix_source.x_drawable.width = width;
>> + pix_source.x_drawable.height = _height;
>> + pix_source.x_drawable.width = _width;
>> }
>>
>> Cursor RedWindow_p::create_invisible_cursor(Window window)
>> @@ -1100,6 +1109,8 @@ RedWindow_p::RedWindow_p()
>> , _ignore_foucs (false)
>> , _pointer_in_window (false)
>> , _ignore_pointer (false)
>> + ,_width (200)
>> + ,_height (200)
> i guess these are just random values?

Yes, before this patch they were in the create call in RedWindow::RedWindow
no idea why 200x200 was chosen.

<snip>

Regards,

Hans