Merge lp:~alan-griffiths/miral/fix-1602331 into lp:miral

[Alan Griffiths]

Don't assume that hidden windows don't move - i.e. "restore" them where they are.

Don't assume that hidden windows don't move - i.e. "restore" them where they are.

The most obvious issue is with menus that are hidden ans restored by toolkits (see linked bug report).

[Andreas Pokorny (andreas-pokorny)]

Hm is window and window_info diverging here?

This code looks strange. If miral updates the position of hidden windows, why would you have to restore anything here?

review: Needs Information

[Andreas Pokorny (andreas-pokorny)]

Ok this would work fine with the upcoming gtk+ ..

[Alan Griffiths (alan-griffiths)]

> Hm is window and window_info diverging here?

I don't understand the question. window_info is metadata needed by WM about the window being managed. They have different, but related, roles.

> This code looks strange. If miral updates the position of hidden windows, why
> would you have to restore anything here?

We have a state transition from "hidden" to (in the bug case) "restored".

It is simpler to set restore_rect when exiting "hidden" than add a special case when entering "restore" to see whether the old state was "hidden" and ignore restore_rect.

(IMO It also makes more sense if there's a transition to another state - although I suspect there's additional work required to get the right logic in all cases (e.g. where windows are resized when vertically/horizontally maximized).)

[Andreas Pokorny (andreas-pokorny)]

Ok understood now..

review: Approve

=== modified file 'miral/basic_window_manager.cpp'
--- miral/basic_window_manager.cpp	2016-07-11 15:57:49 +0000
+++ miral/basic_window_manager.cpp	2016-07-12 16:30:04 +0000
@@ -669,12 +669,18 @@
         return;
     }
 
-    if (window_info.state() == mir_surface_state_restored)
+    switch (window_info.state())
     {
+    case mir_surface_state_restored:
+    case mir_surface_state_hidden:
         window_info.restore_rect({window_info.window().top_left(), window_info.window().size()});
+        break;
+
+    default:
+        break;
     }
 
-    if (window_info.state() != mir_surface_state_fullscreen)
+    if (value != mir_surface_state_fullscreen)
     {
         window_info.output_id({});
         fullscreen_surfaces.erase(window_info.window());