[x11] Rework map/unmap and resizing

The mapping and unmapping of the X11 stage implementation is
a bit bong. It's asynchronous, for starters, when it really
can avoid it by tracking the state internally.

The ordering of the map/unmap sequence is also broken with
respect to the resizing.

By tracking the state internally into StageX11 we can safely
remove the MapNotify and UnmapNotify X event handling.

In theory, we should use _NET_WM_STATE a lot more, and reuse
the X11 state flags for fullscreening as well.

clutter/clutter-actor.c

@@ -699,8 +699,9 @@ clutter_actor_update_map_state (ClutterActor  *self,
           !CLUTTER_ACTOR_IS_VISIBLE (self) &&
           !(CLUTTER_PRIVATE_FLAGS (self) & CLUTTER_ACTOR_IN_DESTRUCTION))
         {
-          g_warning ("Clutter toplevel is not visible, but is "
+          g_warning ("Clutter toplevel of type '%s' is not visible, but "
-                     "somehow still mapped");
+                     "it is somehow still mapped",
+                     G_OBJECT_TYPE_NAME (self));
         }
     }
   else

clutter/clutter-stage-window.h

@@ -27,6 +27,10 @@ struct _ClutterStageWindowIface
                                         gboolean            cursor_visible);
   void          (* set_user_resizable) (ClutterStageWindow *stage_window,
                                         gboolean            is_resizable);
+
+  void          (* show)               (ClutterStageWindow *stage_window,
+                                        gboolean            do_raise);
+  void          (* hide)               (ClutterStageWindow *stage_window);
 };
 
 GType clutter_stage_window_get_type (void) G_GNUC_CONST;

clutter/clutter-stage.c

@@ -316,23 +316,26 @@ static void
 clutter_stage_show (ClutterActor *self)
 {
   ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv;
+  ClutterStageWindow *impl;
 
   CLUTTER_ACTOR_CLASS (clutter_stage_parent_class)->show (self);
 
   g_assert (priv->impl != NULL);
-
+  impl = CLUTTER_STAGE_WINDOW (priv->impl);
-  clutter_actor_show (priv->impl);
+  CLUTTER_STAGE_WINDOW_GET_IFACE (impl)->show (impl, TRUE);
 }
 
 static void
 clutter_stage_hide (ClutterActor *self)
 {
   ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv;
-
+  ClutterStageWindow *impl;
-  CLUTTER_ACTOR_CLASS (clutter_stage_parent_class)->hide (self);
 
   g_assert (priv->impl != NULL);
-  clutter_actor_hide (priv->impl);
+  impl = CLUTTER_STAGE_WINDOW (priv->impl);
+  CLUTTER_STAGE_WINDOW_GET_IFACE (impl)->hide (impl);
+
+  CLUTTER_ACTOR_CLASS (clutter_stage_parent_class)->hide (self);
 }
 
 static void

clutter/glx/clutter-stage-glx.c

@@ -199,7 +199,7 @@ clutter_stage_glx_realize (ClutterActor *actor)
         }
 
       /* no user resize.. */
-      clutter_stage_x11_fix_window_size (stage_x11);
+      clutter_stage_x11_fix_window_size (stage_x11, -1, -1);
       clutter_stage_x11_set_wm_protocols (stage_x11);
 
       /* ask for a context; a no-op, if a context already exists */

clutter/x11/clutter-event-x11.c

@@ -479,15 +479,16 @@ event_translate (ClutterBackend *backend,
                         xevent->xconfigure.width,
                         xevent->xconfigure.height);
 
-	  clutter_actor_set_size (CLUTTER_ACTOR (stage),
+          stage_x11->xwin_width = xevent->xconfigure.width;
-				  xevent->xconfigure.width,
+          stage_x11->xwin_height = xevent->xconfigure.height;
-				  xevent->xconfigure.height);
 
           /* the resize process is complete, so we can ask the stage
            * to set up the GL viewport with the new size
            */
           CLUTTER_SET_PRIVATE_FLAGS (CLUTTER_ACTOR (stage_x11->wrapper),
                                      CLUTTER_ACTOR_SYNC_MATRICES);
+
+          clutter_actor_queue_relayout (CLUTTER_ACTOR (stage_x11->wrapper));
         }
       res = FALSE;
       break;
@@ -552,12 +553,10 @@ event_translate (ClutterBackend *backend,
       break;
 
     case MapNotify:
-      clutter_stage_x11_map (stage_x11);
       res = FALSE;
       break;
 
     case UnmapNotify:
-      clutter_stage_x11_unmap (stage_x11);
       res = FALSE;
       break;
 

clutter/x11/clutter-stage-x11.c

@@ -85,7 +85,9 @@ send_wmspec_change_state (ClutterBackendX11 *backend_x11,
 }
 
 void
-clutter_stage_x11_fix_window_size (ClutterStageX11 *stage_x11)
+clutter_stage_x11_fix_window_size (ClutterStageX11 *stage_x11,
+                                   gint             new_width,
+                                   gint             new_height)
 {
   gboolean resize;
 
@@ -98,12 +100,19 @@ clutter_stage_x11_fix_window_size (ClutterStageX11 *stage_x11)
 
       size_hints = XAllocSizeHints();
 
-      clutter_actor_get_preferred_width (CLUTTER_ACTOR (stage_x11),
+      if (new_width < 0)
-                                         -1,
+        clutter_actor_get_preferred_width (CLUTTER_ACTOR (stage_x11),
-                                         &min_width, NULL);
+                                           -1,
-      clutter_actor_get_preferred_height (CLUTTER_ACTOR (stage_x11),
+                                           &min_width, NULL);
-                                          min_width,
+      else
-                                          &min_height, NULL);
+        min_width = new_width;
+
+      if (new_height < 0)
+        clutter_actor_get_preferred_height (CLUTTER_ACTOR (stage_x11),
+                                            min_width,
+                                            &min_height, NULL);
+      else
+        min_height = new_height;
 
       size_hints->flags = 0;
 
@@ -129,34 +138,6 @@ clutter_stage_x11_fix_window_size (ClutterStageX11 *stage_x11)
     }
 }
 
-static void
-clutter_stage_x11_show (ClutterActor *actor)
-{
-  ClutterStageX11 *stage_x11 = CLUTTER_STAGE_X11 (actor);
-
-  CLUTTER_ACTOR_CLASS (clutter_stage_x11_parent_class)->show (actor);
-
-  if (stage_x11->xwin)
-    {
-      /* Fire off a redraw to avoid flicker on first map.
-       * Appears not to work perfectly on intel drivers at least.
-       */
-      clutter_redraw (stage_x11->wrapper);
-
-      XSync (stage_x11->xdpy, FALSE);
-      XMapWindow (stage_x11->xdpy, stage_x11->xwin);
-    }
-}
-
-static void
-clutter_stage_x11_hide (ClutterActor *actor)
-{
-  ClutterStageX11 *stage_x11 = CLUTTER_STAGE_X11 (actor);
-
-  if (stage_x11->xwin)
-    XWithdrawWindow (stage_x11->xdpy, stage_x11->xwin, stage_x11->xscreen);
-}
-
 void
 clutter_stage_x11_set_wm_protocols (ClutterStageX11 *stage_x11)
 {
@@ -268,6 +249,10 @@ clutter_stage_x11_allocate (ClutterActor           *self,
       new_height = 1;
     }
 
+  CLUTTER_NOTE (BACKEND, "New allocation received: (%d, %d)",
+                new_width,
+                new_height);
+
   if (new_width != stage_x11->xwin_width ||
       new_height != stage_x11->xwin_height)
     {
@@ -289,7 +274,7 @@ clutter_stage_x11_allocate (ClutterActor           *self,
                          stage_x11->xwin_height);
         }
 
-      clutter_stage_x11_fix_window_size (stage_x11);
+      clutter_stage_x11_fix_window_size (stage_x11, new_width, new_height);
 
       if (stage_x11->xpixmap != None)
         {
@@ -440,7 +425,7 @@ clutter_stage_x11_set_fullscreen (ClutterStageWindow *stage_window,
                  the maximum and minimum window hints. Otherwise
                  metacity will honour the restrictions and not
                  fullscreen correctly. */
-              clutter_stage_x11_fix_window_size (stage_x11);
+              clutter_stage_x11_fix_window_size (stage_x11, -1, -1);
 
               send_wmspec_change_state (backend_x11, stage_x11->xwin,
                                         backend_x11->atom_NET_WM_STATE_FULLSCREEN,
@@ -470,7 +455,7 @@ clutter_stage_x11_set_fullscreen (ClutterStageWindow *stage_window,
 
               /* Fix the window size to restore the minimum/maximum
                  restriction */
-              clutter_stage_x11_fix_window_size (stage_x11);
+              clutter_stage_x11_fix_window_size (stage_x11, -1, -1);
             }
         }
     }
@@ -503,7 +488,104 @@ clutter_stage_x11_set_user_resizable (ClutterStageWindow *stage_window,
 {
   ClutterStageX11 *stage_x11 = CLUTTER_STAGE_X11 (stage_window);
 
-  clutter_stage_x11_fix_window_size (stage_x11);
+  clutter_stage_x11_fix_window_size (stage_x11, -1, -1);
+}
+
+#define STAGE_X11_IS_MAPPED(s)  ((((ClutterStageX11 *) (s))->wm_state & STAGE_X11_WITHDRAWN) == 0)
+
+static void
+update_wm_hints (ClutterStageX11 *stage_x11)
+{
+  XWMHints wm_hints;
+
+  if (stage_x11->wm_state & STAGE_X11_WITHDRAWN)
+    return;
+
+  wm_hints.flags = StateHint;
+  wm_hints.initial_state = NormalState;
+
+  XSetWMHints (stage_x11->xdpy, stage_x11->xwin, &wm_hints);
+}
+
+static void
+set_stage_state (ClutterStageX11      *stage_x11,
+                 ClutterStageX11State  unset_flags,
+                 ClutterStageX11State  set_flags)
+{
+  ClutterStageX11State new_stage_state, old_stage_state;
+
+  old_stage_state = stage_x11->wm_state;
+
+  new_stage_state = old_stage_state;
+  new_stage_state |= set_flags;
+  new_stage_state &= ~unset_flags;
+
+  if (new_stage_state == old_stage_state)
+    return;
+
+  stage_x11->wm_state = new_stage_state;
+}
+
+static void
+clutter_stage_x11_show (ClutterStageWindow *stage_window,
+                        gboolean            do_raise)
+{
+  ClutterStageX11 *stage_x11 = CLUTTER_STAGE_X11 (stage_window);
+
+  if (stage_x11->xwin != None)
+    {
+      if (do_raise)
+        XRaiseWindow (stage_x11->xdpy, stage_x11->xwin);
+
+      if (!STAGE_X11_IS_MAPPED (stage_x11))
+        {
+          CLUTTER_NOTE (BACKEND, "Mapping stage[%lu] (%d, %d)",
+                        (unsigned long) stage_x11->xwin,
+                        stage_x11->xwin_width,
+                        stage_x11->xwin_height);
+
+          update_wm_hints (stage_x11);
+
+          if (stage_x11->fullscreen_on_map)
+            clutter_stage_x11_set_fullscreen (stage_window, TRUE);
+          else
+            clutter_stage_x11_set_fullscreen (stage_window, FALSE);
+
+          set_stage_state (stage_x11, STAGE_X11_WITHDRAWN, 0);
+
+          clutter_stage_ensure_viewport (CLUTTER_STAGE (stage_x11->wrapper));
+        }
+
+      g_assert (STAGE_X11_IS_MAPPED (stage_x11));
+
+      XMapWindow (stage_x11->xdpy, stage_x11->xwin);
+
+      clutter_actor_map (CLUTTER_ACTOR (stage_x11));
+      clutter_actor_map (CLUTTER_ACTOR (stage_x11->wrapper));
+
+      clutter_actor_queue_relayout (CLUTTER_ACTOR (stage_x11->wrapper));
+    }
+}
+
+static void
+clutter_stage_x11_hide (ClutterStageWindow *stage_window)
+{
+  ClutterStageX11 *stage_x11 = CLUTTER_STAGE_X11 (stage_window);
+
+  if (stage_x11->xwin != None)
+    {
+      if (STAGE_X11_IS_MAPPED (stage_x11))
+        set_stage_state (stage_x11, 0, STAGE_X11_WITHDRAWN);
+
+      g_assert (!STAGE_X11_IS_MAPPED (stage_x11));
+
+      clutter_actor_unmap (CLUTTER_ACTOR (stage_x11));
+      clutter_actor_unmap (CLUTTER_ACTOR (stage_x11->wrapper));
+
+      XWithdrawWindow (stage_x11->xdpy,
+                       stage_x11->xwin,
+                       0);
+    }
 }
 
 static ClutterActor *
@@ -538,8 +620,6 @@ clutter_stage_x11_class_init (ClutterStageX11Class *klass)
   gobject_class->dispose = clutter_stage_x11_dispose;
 
   actor_class->realize = clutter_stage_x11_realize;
-  actor_class->show = clutter_stage_x11_show;
-  actor_class->hide = clutter_stage_x11_hide;
 
   actor_class->get_preferred_width = clutter_stage_x11_get_preferred_width;
   actor_class->get_preferred_height = clutter_stage_x11_get_preferred_height;
@@ -558,6 +638,8 @@ clutter_stage_x11_init (ClutterStageX11 *stage)
   stage->xwin_height = 480;
   stage->xvisinfo = None;
 
+  stage->wm_state = STAGE_X11_WITHDRAWN;
+
   stage->is_foreign_xwin = FALSE;
   stage->fullscreen_on_map = FALSE;
   stage->is_cursor_visible = TRUE;
@@ -577,6 +659,8 @@ clutter_stage_window_iface_init (ClutterStageWindowIface *iface)
   iface->set_fullscreen = clutter_stage_x11_set_fullscreen;
   iface->set_cursor_visible = clutter_stage_x11_set_cursor_visible;
   iface->set_user_resizable = clutter_stage_x11_set_user_resizable;
+  iface->show = clutter_stage_x11_show;
+  iface->hide = clutter_stage_x11_hide;
 }
 
 /**
@@ -770,32 +854,3 @@ clutter_x11_set_stage_foreign (ClutterStage *stage,
 
   return TRUE;
 }
-
-void
-clutter_stage_x11_map (ClutterStageX11 *stage_x11)
-{
-  CLUTTER_NOTE (BACKEND, "Mapping stage '%s' [%p]",
-                G_OBJECT_TYPE_NAME (stage_x11),
-                stage_x11);
-
-  clutter_actor_map (CLUTTER_ACTOR (stage_x11));
-  clutter_actor_map (CLUTTER_ACTOR (stage_x11->wrapper));
-
-  if (stage_x11->fullscreen_on_map)
-    clutter_stage_fullscreen (CLUTTER_STAGE (stage_x11->wrapper));
-  else
-    clutter_stage_unfullscreen (CLUTTER_STAGE (stage_x11->wrapper));
-
-  clutter_actor_queue_relayout (CLUTTER_ACTOR (stage_x11->wrapper));
-}
-
-void
-clutter_stage_x11_unmap (ClutterStageX11 *stage_x11)
-{
-  CLUTTER_NOTE (BACKEND, "Unmapping stage '%s' [%p]",
-                G_OBJECT_TYPE_NAME (stage_x11),
-                stage_x11);
-
-  clutter_actor_unmap (CLUTTER_ACTOR (stage_x11));
-  clutter_actor_unmap (CLUTTER_ACTOR (stage_x11->wrapper));
-}

clutter/x11/clutter-stage-x11.h

@@ -41,6 +41,11 @@ G_BEGIN_DECLS
 typedef struct _ClutterStageX11         ClutterStageX11;
 typedef struct _ClutterStageX11Class    ClutterStageX11Class;
 
+typedef enum
+{
+  STAGE_X11_WITHDRAWN = 1 << 1
+} ClutterStageX11State;
+
 struct _ClutterStageX11
 {
   ClutterGroup parent_instance;
@@ -62,6 +67,8 @@ struct _ClutterStageX11
   ClutterBackendX11 *backend;
   ClutterStageState  state;
 
+  ClutterStageX11State wm_state;
+
   int event_types[CLUTTER_X11_XINPUT_LAST_EVENT];
   GList *devices;
 
@@ -76,7 +83,9 @@ struct _ClutterStageX11Class
 GType clutter_stage_x11_get_type (void) G_GNUC_CONST;
 
 /* Private to subclasses */
-void clutter_stage_x11_fix_window_size  (ClutterStageX11 *stage_x11);
+void clutter_stage_x11_fix_window_size  (ClutterStageX11 *stage_x11,
+                                         gint             new_width,
+                                         gint             new_height);
 void clutter_stage_x11_set_wm_protocols (ClutterStageX11 *stage_x11);
 void clutter_stage_x11_map              (ClutterStageX11 *stage_x11);
 void clutter_stage_x11_unmap            (ClutterStageX11 *stage_x11);