sync maximization and quick tiling state in tabs

BUG: 296097
REVIEW: 104293
FIXED-IN: 4.9

client.cpp

@@ -1000,7 +1000,7 @@ void Client::minimize(bool avoid_animation)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Minimized);
     emit minimizedChanged();
 }
 
@@ -1026,7 +1026,7 @@ void Client::unminimize(bool avoid_animation)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Minimized);
     emit minimizedChanged();
 }
 
@@ -1148,7 +1148,7 @@ void Client::setShade(ShadeMode mode)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Shaded);
     emit shadeChanged();
 }
 
@@ -1598,7 +1598,7 @@ void Client::setDesktop(int desktop)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Desktop);
     emit desktopChanged();
 }
 
@@ -1661,7 +1661,7 @@ void Client::updateActivities(bool includeTransients)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Activity);
 }
 
 /**
@@ -1703,7 +1703,7 @@ void Client::setOnAllDesktops(bool b)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Desktop);
 }
 
 /**

client.h

@@ -368,6 +368,7 @@ public:
     bool isMaximizable() const;
     QRect geometryRestore() const;
     MaximizeMode maximizeMode() const;
+    QuickTileMode quickTileMode() const;
     bool isMinimizable() const;
     void setMaximize(bool vertically, bool horizontally);
     QRect iconGeometry() const;
@@ -1117,6 +1118,11 @@ inline Client::MaximizeMode Client::maximizeMode() const
     return max_mode;
 }
 
+inline KWin::QuickTileMode Client::quickTileMode() const
+{
+    return (KWin::QuickTileMode)quick_tile_mode;
+}
+
 inline bool Client::skipTaskbar(bool from_outside) const
 {
     return from_outside ? original_skip_taskbar : skip_taskbar;

geometry.cpp

@@ -1928,7 +1928,7 @@ void Client::setGeometry(int x, int y, int w, int h, ForceGeometry_t force)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Geometry);
 
     // TODO: this signal is emitted too often
     emit geometryChanged();
@@ -1995,7 +1995,7 @@ void Client::plainResize(int w, int h, ForceGeometry_t force)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Geometry);
     // TODO: this signal is emitted too often
     emit geometryChanged();
 }
@@ -2042,7 +2042,7 @@ void Client::move(int x, int y, ForceGeometry_t force)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Geometry);
 }
 
 void Client::blockGeometryUpdates(bool block)
@@ -2082,11 +2082,36 @@ void Client::setMaximize(bool vertically, bool horizontally)
     emit clientMaximizedStateChanged(this, max_mode);
     emit clientMaximizedStateChanged(this, vertically, horizontally);
 
-    // Update states of all other windows in this group
-    if (tabGroup())
-        tabGroup()->updateStates(this);
 }
 
+// Update states of all other windows in this group
+class TabSynchronizer
+{
+public:
+    TabSynchronizer(Client *client, TabGroup::States syncStates) :
+    m_client(client) , m_states(syncStates)
+    {
+        if (client->tabGroup())
+            client->tabGroup()->blockStateUpdates(true);
+    }
+    ~TabSynchronizer()
+    {
+        syncNow();
+    }
+    void syncNow()
+    {
+        if (m_client && m_client->tabGroup()) {
+            m_client->tabGroup()->blockStateUpdates(false);
+            m_client->tabGroup()->updateStates(m_client, m_states);
+        }
+        m_client = 0;
+    }
+private:
+    Client *m_client;
+    TabGroup::States m_states;
+};
+
+
 static bool changeMaximizeRecursion = false;
 void Client::changeMaximize(bool vertical, bool horizontal, bool adjust)
 {
@@ -2116,6 +2141,8 @@ void Client::changeMaximize(bool vertical, bool horizontal, bool adjust)
         return;
 
     GeometryUpdatesBlocker blocker(this);
+    // QT synchronizing required because we eventually change from QT to Maximized
+    TabSynchronizer syncer(this, TabGroup::Maximized|TabGroup::QuickTile);
 
     // maximing one way and unmaximizing the other way shouldn't happen,
     // so restore first and then maximize the other way
@@ -2272,6 +2299,8 @@ void Client::changeMaximize(bool vertical, bool horizontal, bool adjust)
         break;
     }
 
+    syncer.syncNow(); // important because of window rule updates!
+
     updateAllowedActions();
     if (decoration != NULL)
         decoration->maximizeChange();
@@ -3101,6 +3130,7 @@ void Client::setQuickTileMode(QuickTileMode mode, bool keyboard)
 
     if (mode == QuickTileMaximize)
     {
+        TabSynchronizer syncer(this, TabGroup::QuickTile|TabGroup::Geometry|TabGroup::Maximized);
         quick_tile_mode = QuickTileNone;
         if (maximizeMode() == MaximizeFull)
             setMaximize(false, false);
@@ -3122,6 +3152,9 @@ void Client::setQuickTileMode(QuickTileMode mode, bool keyboard)
 
     // restore from maximized so that it is possible to tile maximized windows with one hit or by dragging
     if (maximizeMode() == MaximizeFull) {
+
+        TabSynchronizer syncer(this, TabGroup::QuickTile|TabGroup::Geometry|TabGroup::Maximized);
+
         setMaximize(false, false);
 
         // Temporary, so the maximize code doesn't get all confused
@@ -3137,14 +3170,18 @@ void Client::setQuickTileMode(QuickTileMode mode, bool keyboard)
     // First, check if the requested tile negates the tile we're in now: move right when left or left when right
     // is the same as explicitly untiling this window, so allow it.
     if (mode == QuickTileNone || ((quick_tile_mode & QuickTileHorizontal) && (mode & QuickTileHorizontal))) {
+        TabSynchronizer syncer(this, TabGroup::QuickTile|TabGroup::Geometry);
+
+        quick_tile_mode = QuickTileNone;
         // Untiling, so just restore geometry, and we're done.
         if (!geom_restore.isValid()) // invalid if we started maximized and wait for placement
             geom_restore = geometry();
         setGeometry(geom_restore);
-        quick_tile_mode = QuickTileNone;
         checkWorkspacePosition(); // Just in case it's a different screen
         return;
     } else {
+        TabSynchronizer syncer(this, TabGroup::QuickTile|TabGroup::Geometry);
+
         QPoint whichScreen = keyboard ? geometry().center() : cursorPos();
 
         // If trying to tile to the side that the window is already tiled to move the window to the next
@@ -3182,18 +3219,19 @@ void Client::setQuickTileMode(QuickTileMode mode, bool keyboard)
                 mode = QuickTileRight;
             else
                 mode = QuickTileLeft;
-        } else
+        } else {
             // Not coming out of an existing tile, not shifting monitors, we're setting a brand new tile.
             // Store geometry first, so we can go out of this tile later.
             geom_restore = geometry();
+        }
 
         // Temporary, so the maximize code doesn't get all confused
         quick_tile_mode = QuickTileNone;
         if (mode != QuickTileNone)
             setGeometry(electricBorderMaximizeGeometry(whichScreen, desktop()));
+
         // Store the mode change
         quick_tile_mode = mode;
-
     }
 }
 

layers.cpp

@@ -812,7 +812,7 @@ void Client::setKeepAbove(bool b)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Layer);
     emit keepAboveChanged();
 }
 
@@ -836,7 +836,7 @@ void Client::setKeepBelow(bool b)
 
     // Update states of all other windows in this group
     if (tabGroup())
-        tabGroup()->updateStates(this);
+        tabGroup()->updateStates(this, TabGroup::Layer);
     emit keepBelowChanged();
 }
 

tabgroup.cpp

@@ -31,6 +31,7 @@ TabGroup::TabGroup(Client *c)
     , m_current(c)
     , m_minSize(c->minSize())
     , m_maxSize(c->maxSize())
+    , m_stateUpdatesBlocked(0)
 {
     QIcon icon(c->icon());
     icon.addPixmap(c->miniIcon());
@@ -112,7 +113,7 @@ bool TabGroup::add(Client* c, Client *other, bool after, bool becomeVisible)
     c->setTabGroup(this);   // Let the client know which group it belongs to
 
     updateMinMaxSize();
-    updateStates(m_current, c);
+    updateStates(m_current, All, c);
 
     if (!becomeVisible)
         c->setClientShown(false);
@@ -156,9 +157,12 @@ bool TabGroup::remove(Client* c, const QRect& newGeom)
             static_cast<EffectsHandlerImpl*>(effects)->slotCurrentTabAboutToChange(c->effectWindow(), m_current->effectWindow());
     }
 
-    if (newGeom.isValid()) {
+    if (c->quickTileMode() != QuickTileNone)
+        c->setQuickTileMode(QuickTileNone); // if we leave a quicktiled group, assume that the user wants to untile
+    else if (newGeom.isValid()) {
         c->maximize(Client::MaximizeRestore); // explicitly calling for a geometry -> unmaximize - in doubt
         c->setGeometry(newGeom);
+        c->checkWorkspacePosition(); // oxygen has now twice kicked me a window out of the screen - better be safe then sorry
     }
 
     // Notify effects of removal
@@ -268,32 +272,56 @@ void TabGroup::updateMinMaxSize()
     }
 }
 
-void TabGroup::updateStates(Client* main, Client* only)
+
+void TabGroup::blockStateUpdates(bool more) {
+    more ? ++m_stateUpdatesBlocked : --m_stateUpdatesBlocked;
+    if (m_stateUpdatesBlocked < 0) {
+        m_stateUpdatesBlocked = 0;
+        qWarning("TabGroup: Something is messed up with TabGroup::blockStateUpdates() invokation\nReleased more than blocked!");
+    }
+}
+
+void TabGroup::updateStates(Client* main, States states, Client* only)
 {
+    if (m_stateUpdatesBlocked > 0)
+        return;
+
     ClientList toBeRemoved;
     for (ClientList::const_iterator i = m_clients.constBegin(), end = m_clients.constEnd(); i != end; ++i) {
         Client *c = (*i);
         if (c != main && (!only || c == only)) {
-            if (c->isMinimized() != main->isMinimized()) {
+            if ((states & Minimized) && c->isMinimized() != main->isMinimized()) {
                 if (main->isMinimized())
                     c->minimize(true);
                 else
                     c->unminimize(true);
             }
-            if (c->isShade() != main->isShade())
+
+            // the order QuickTile -> Maximized -> Geometry is somewhat important because one will change the other
+            // don't change w/o good reason and care
+            if ((states & QuickTile) && c->quickTileMode() != main->quickTileMode())
+                c->setQuickTileMode(main->quickTileMode());
+            if ((states & Maximized) && c->maximizeMode() != main->maximizeMode())
+                c->maximize(main->maximizeMode());
+            // the order Shaded -> Geometry is somewhat important because one will change the other
+            if ((states & Shaded) && c->isShade() != main->isShade())
                 c->setShade(main->isShade() ? ShadeNormal : ShadeNone);
-            if (c->geometry() != main->geometry())
+            if ((states & Geometry) && c->geometry() != main->geometry())
                 c->setGeometry(main->geometry());
-            if (c->isOnAllDesktops() != main->isOnAllDesktops())
+            if (states & Desktop) {
-                c->setOnAllDesktops(main->isOnAllDesktops());
+                if (c->isOnAllDesktops() != main->isOnAllDesktops())
-            if (c->desktop() != main->desktop())
+                    c->setOnAllDesktops(main->isOnAllDesktops());
-                c->setDesktop(main->desktop());
+                if (c->desktop() != main->desktop())
-            if (c->activities() != main->activities())
+                    c->setDesktop(main->desktop());
+            }
+            if ((states & Activity) && c->activities() != main->activities())
                 c->setOnActivities(main->activities());
-            if (c->keepAbove() != main->keepAbove())
+            if (states & Layer) {
-                c->setKeepAbove(main->keepAbove());
+                if (c->keepAbove() != main->keepAbove())
-            if (c->keepBelow() != main->keepBelow())
+                    c->setKeepAbove(main->keepAbove());
-                c->setKeepBelow(main->keepBelow());
+                if (c->keepBelow() != main->keepBelow())
+                    c->setKeepBelow(main->keepBelow());
+            }
 
             // If it's not possible to have the same states then ungroup them, TODO: Check all states
             if (c->geometry() != main->geometry() || c->desktop() != main->desktop())

tabgroup.h

@@ -56,6 +56,13 @@ public:
     TabGroup(Client* c);
     ~TabGroup();
 
+    enum State {
+        Minimized = 1<<0, Maximized = 1<<1, Shaded = 1<<2,
+        Geometry = 1<<3, Desktop = 1<<4, Activity = 1<<5,
+        Layer = 1<<6, QuickTile = 1<<7, All = 0xffffffff
+    };
+    Q_DECLARE_FLAGS(States, State)
+
     /**
      * Activate next tab (flips)
      */
@@ -66,6 +73,13 @@ public:
      */
     void activatePrev();
 
+    /**
+     * Allows to alter several attributes in random order and trigger a general update at the end
+     * (must still be explicitly called)
+     * this is to prevent side effects, mostly for geometry adjustments during maximization and QuickTiling
+     */
+    void blockStateUpdates(bool);
+
     /**
      * Close all clients in this group.
      */
@@ -125,7 +139,7 @@ public:
      * \p main as the primary client to copy the settings off. If \p only is set then only
      * that client is updated to match \p main.
      */
-    void updateStates(Client* main, Client* only = NULL);
+    void updateStates(Client* main, States states, Client* only = NULL);
 
     /**
      * updates geometry restrictions of this group, basically called from Client::getWmNormalHints(), otherwise rather private
@@ -149,6 +163,7 @@ private:
     Client *m_current;
     QSize m_minSize;
     QSize m_maxSize;
+    int m_stateUpdatesBlocked;
 };
 
 inline bool TabGroup::contains(Client* c) const
@@ -188,4 +203,6 @@ inline QSize TabGroup::maxSize() const
 
 }
 
+Q_DECLARE_OPERATORS_FOR_FLAGS(KWin::TabGroup::States)
+
 #endif