Bug 146040

Summary: Restarting KWin messes up active border order
Product: [Plasma] kwin Reporter: Thomas McGuire <mcguire>
Component: generalAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Ubuntu   
OS: Linux   
Latest Commit: Version Fixed In:

Description Thomas McGuire 2007-05-27 23:07:31 UTC
Version:           KDE 3.5.7 (using KDE KDE 3.5.7)
Installed from:    Ubuntu Packages
OS:                Linux

When restarting KWin with "kwin --replace", the order of the active borders is no longer the same as the order of the desktops in the pager.

Example:
I have 8 virtual desktops arranged in two rows. If I am on desktop 2 and move my mouse to the right border, it jumps to desktop 4 instead of desktop 3.

To reproduce:
0. Enable active borders (Advanced->Active Desktop Borders). They need to be "always enabled" with a low switch delay, for example 100 msec.
1. Restart KWin by typing "kwin --replace" in a terminal window.
2. Try the scenario described in the above example


Additionally, restarting KWin places the KMail systray icon onto the top left corner of the screen. But this is probably a KMail bug.
Comment 1 Lubos Lunak 2007-05-30 20:36:53 UTC
SVN commit 669899 by lunakl:

Backport _NET_DESKTOP_LAYOUT support from trunk. This will allow
mixing kde3/kde4 KWin and also fix #146040.
BUG: 146040



 M  +21 -18    kdebase/kicker/applets/minipager/pagerapplet.cpp  
 M  +2 -0      kdebase/kicker/applets/minipager/pagerapplet.h  
 M  +7 -17     kdebase/kwin/events.cpp  
 M  +17 -9     kdebase/kwin/workspace.cpp  
 M  +2 -2      kdebase/kwin/workspace.h  
 M  +89 -2     kdelibs/kdecore/netwm.cpp  
 M  +23 -0     kdelibs/kdecore/netwm.h  
 M  +21 -1     kdelibs/kdecore/netwm_def.h  
 M  +3 -0      kdelibs/kdecore/netwm_p.h  


--- branches/KDE/3.5/kdebase/kicker/applets/minipager/pagerapplet.cpp #669898:669899
@@ -42,6 +42,7 @@
 #include <kiconloader.h>
 #include <dcopclient.h>
 #include <netwm.h>
+#include <kmanagerselection.h>
 
 #include "global.h"
 #include "kickertip.h"
@@ -83,6 +84,7 @@
                        QWidget *parent, const char *name)
     : KPanelApplet( configFile, type, actions, parent, name ),
       m_layout(0),
+      m_desktopLayoutOwner( NULL ),
       m_shadowEngine(0),
       m_contextMenu(0),
       m_settings( new PagerSettings(sharedConfig()) )
@@ -350,27 +352,28 @@
       return;
     }
 
-    QCString kwin_name;
-    int screen_number = DefaultScreen(qt_xdisplay());
-    if (screen_number == 0)
-        kwin_name = "kwin";
-    else
-        kwin_name.sprintf("kwin-screen-%d", screen_number);
-
-    QCString replyType;
-    QByteArray data, replyData;
-    QDataStream arg(data, IO_WriteOnly);
-    arg << o << x << y;
-    if ( !(kapp->dcopClient()->call( kwin_name, "KWinInterface", "setDesktopLayout(int, int, int)",
-           data, replyType, replyData)))
-    {
-        kdDebug() << "KMiniPager: Call to KWinInterface::setDesktopLayout(int, int, int) failed" << endl;
-        return;
-    }
-
     desktopLayoutOrientation = o;
     desktopLayoutX = x;
     desktopLayoutY = y;
+    if( x == -1 ) // do-the-maths-yourself is encoded as 0 in the wm spec
+        x = 0;
+    if( y == -1 )
+        y = 0;
+    if( m_desktopLayoutOwner == NULL )
+    { // must own manager selection before setting global desktop layout
+        int screen = DefaultScreen( qt_xdisplay());
+        m_desktopLayoutOwner = new KSelectionOwner( QString( "_NET_DESKTOP_LAYOUT_S%1" ).arg( screen ).latin1(),
+            screen, this );
+        if( !m_desktopLayoutOwner->claim( false ))
+        {
+            delete m_desktopLayoutOwner;
+            m_desktopLayoutOwner = NULL;
+            return;
+        }
+    }
+    NET::Orientation orient = o == Qt::Horizontal ? NET::OrientationHorizontal : NET::OrientationVertical;
+    NETRootInfo i( qt_xdisplay(), 0 );
+    i.setDesktopLayout( orient, x, y, NET::DesktopLayoutCornerTopLeft );
 }
 
 void KMiniPager::resizeEvent(QResizeEvent*)
--- branches/KDE/3.5/kdebase/kicker/applets/minipager/pagerapplet.h #669898:669899
@@ -40,6 +40,7 @@
 class KProcess;
 class KWinModule;
 class KShadowEngine;
+class KSelectionOwner;
 
 class PagerSettings;
 
@@ -124,6 +125,7 @@
     int desktopLayoutOrientation;
     int desktopLayoutX;
     int desktopLayoutY;
+    KSelectionOwner* m_desktopLayoutOwner;
 
     KWinModule *m_kwin;
     KShadowEngine* m_shadowEngine;
--- branches/KDE/3.5/kdebase/kwin/events.cpp #669898:669899
@@ -200,10 +200,14 @@
         XUngrabKeyboard( qt_xdisplay(), qt_x_time );
         }
 
-    if ( e->type == PropertyNotify || e->type == ClientMessage ) 
+    if( e->type == PropertyNotify || e->type == ClientMessage )
         {
-        if ( netCheck( e ) )
-            return TRUE;
+        unsigned long dirty[ NETRootInfo::PROPERTIES_SIZE ];
+        rootInfo->event( e, dirty, NETRootInfo::PROPERTIES_SIZE );
+        if( dirty[ NETRootInfo::PROTOCOLS ] & NET::DesktopNames )
+            saveDesktopSettings();
+        if( dirty[ NETRootInfo::PROTOCOLS2 ] & NET::WM2DesktopLayout )
+            updateDesktopLayout();
         }
 
     // events that should be handled before Clients can get them
@@ -491,20 +495,6 @@
         };
     }
 
-/*!
-  Handles client messages sent to the workspace
- */
-bool Workspace::netCheck( XEvent* e )
-    {
-    unsigned int dirty = rootInfo->event( e );
-
-    if ( dirty & NET::DesktopNames )
-        saveDesktopSettings();
-
-    return dirty != 0;
-    }
-
-
 // ****************************************
 // Client
 // ****************************************
--- branches/KDE/3.5/kdebase/kwin/workspace.cpp #669898:669899
@@ -290,6 +290,7 @@
         NET::WM2ExtendedStrut |
         NET::WM2KDETemporaryRules |
         NET::WM2ShowingDesktop |
+        NET::WM2DesktopLayout |
         0
         ,
         NET::ActionMove |
@@ -310,6 +311,7 @@
         protocols, 5, qt_xscreen() );
 
     loadDesktopSettings();
+    updateDesktopLayout();
     // extra NETRootInfo instance in Client mode is needed to get the values of the properties
     NETRootInfo client_info( qt_xdisplay(), NET::ActiveWindow | NET::CurrentDesktop );
     int initial_desktop;
@@ -1521,25 +1523,31 @@
     updateClientArea();
     }
 
-void Workspace::setDesktopLayout(int o, int x, int y)
+void Workspace::setDesktopLayout( int, int, int )
+    { // DCOP-only, unused
+    }
+
+void Workspace::updateDesktopLayout()
     {
-    layoutOrientation = (Qt::Orientation) o;
-    layoutX = x;
-    layoutY = y;
+    // rootInfo->desktopLayoutCorner(); // I don't find this worth bothering, feel free to
+    layoutOrientation = ( rootInfo->desktopLayoutOrientation() == NET::OrientationHorizontal
+        ? Qt::Horizontal : Qt::Vertical );
+    layoutX = rootInfo->desktopLayoutColumnsRows().width();
+    layoutY = rootInfo->desktopLayoutColumnsRows().height();
     }
 
 void Workspace::calcDesktopLayout(int &x, int &y) const
     {
-    x = layoutX;
+    x = layoutX; // <= 0 means compute it from the other and total number of desktops
     y = layoutY;
-    if ((x == -1) && (y > 0))
+    if((x <= 0) && (y > 0))
        x = (numberOfDesktops()+y-1) / y;
-    else if ((y == -1) && (x > 0))
+    else if((y <=0) && (x > 0))
        y = (numberOfDesktops()+x-1) / x;
 
-    if (x == -1)
+    if(x <=0)
        x = 1;
-    if (y == -1)
+    if (y <= 0)
        y = 1;
     }
 
--- branches/KDE/3.5/kdebase/kwin/workspace.h #669898:669899
@@ -226,7 +226,8 @@
         void circulateDesktopApplications();
 
         QString desktopName( int desk ) const;
-        void setDesktopLayout(int o, int x, int y);
+        virtual void setDesktopLayout(int , int , int );
+        void updateDesktopLayout();
         void setShowingDesktop( bool showing );
         void resetShowingDesktop( bool keep_hidden );
         bool showingDesktop() const;
@@ -388,7 +389,6 @@
 
     protected:
         bool keyPressMouseEmulation( XKeyEvent& ev );
-        bool netCheck( XEvent* e );
 
     private:
         void init();
--- branches/KDE/3.5/kdelibs/kdecore/netwm.cpp #669898:669899
@@ -56,6 +56,7 @@
 static Atom net_supporting_wm_check  = 0;
 static Atom net_virtual_roots        = 0;
 static Atom net_showing_desktop      = 0;
+static Atom net_desktop_layout       = 0;
 
 // root window messages
 static Atom net_close_window         = 0;
@@ -235,7 +236,7 @@
 }
 
 
-static const int netAtomCount = 83;
+static const int netAtomCount = 84;
 static void create_atoms(Display *d) {
     static const char * const names[netAtomCount] =
     {
@@ -252,6 +253,7 @@
 	    "_NET_ACTIVE_WINDOW",
 	    "_NET_WORKAREA",
 	    "_NET_VIRTUAL_ROOTS",
+            "_NET_DESKTOP_LAYOUT",
             "_NET_SHOWING_DESKTOP",
 	    "_NET_CLOSE_WINDOW",
             "_NET_RESTACK_WINDOW",
@@ -346,6 +348,7 @@
 	    &net_active_window,
 	    &net_workarea,
 	    &net_virtual_roots,
+            &net_desktop_layout,
             &net_showing_desktop,
 	    &net_close_window,
             &net_restack_window,
@@ -613,6 +616,9 @@
     p->kde_system_tray_windows = 0;
     p->kde_system_tray_windows_count = 0;
     p->showing_desktop = false;
+    p->desktop_layout_orientation = OrientationHorizontal;
+    p->desktop_layout_corner = DesktopLayoutCornerTopLeft;
+    p->desktop_layout_columns = p->desktop_layout_rows = 0;
     setDefaultProperties();
     if( properties_size > PROPERTIES_SIZE ) {
         fprintf( stderr, "NETRootInfo::NETRootInfo(): properties array too large\n");
@@ -624,7 +630,7 @@
     p->properties[ PROTOCOLS ] |= ( Supported | SupportingWMCheck );
     p->client_properties[ PROTOCOLS ] = DesktopNames // the only thing that can be changed by clients
 			                | WMPing; // or they can reply to this
-    p->client_properties[ PROTOCOLS2 ] = WM2TakeActivity;
+    p->client_properties[ PROTOCOLS2 ] = WM2TakeActivity | WM2DesktopLayout;
 
     role = WindowManager;
 
@@ -711,6 +717,9 @@
     p->kde_system_tray_windows = 0;
     p->kde_system_tray_windows_count = 0;
     p->showing_desktop = false;
+    p->desktop_layout_orientation = OrientationHorizontal;
+    p->desktop_layout_corner = DesktopLayoutCornerTopLeft;
+    p->desktop_layout_columns = p->desktop_layout_rows = 0;
     setDefaultProperties();
     if( properties_size > 2 ) {
         fprintf( stderr, "NETWinInfo::NETWinInfo(): properties array too large\n");
@@ -769,6 +778,9 @@
     p->kde_system_tray_windows = 0;
     p->kde_system_tray_windows_count = 0;
     p->showing_desktop = false;
+    p->desktop_layout_orientation = OrientationHorizontal;
+    p->desktop_layout_corner = DesktopLayoutCornerTopLeft;
+    p->desktop_layout_columns = p->desktop_layout_rows = 0;
     setDefaultProperties();
     p->client_properties[ PROTOCOLS ] = properties;
     for( int i = 0; i < PROPERTIES_SIZE; ++i )
@@ -873,6 +885,7 @@
 #endif
 
 	setSupported();
+	update(p->client_properties);
     } else {
 
 #ifdef    NETWMDEBUG
@@ -1166,6 +1179,9 @@
     if (p->properties[ PROTOCOLS ] & VirtualRoots)
 	atoms[pnum++] = net_virtual_roots;
 
+    if (p->properties[ PROTOCOLS2 ] & WM2DesktopLayout)
+	atoms[pnum++] = net_desktop_layout;
+
     if (p->properties[ PROTOCOLS ] & CloseWindow)
 	atoms[pnum++] = net_close_window;
 
@@ -1398,6 +1414,9 @@
     else if( atom == net_virtual_roots )
         p->properties[ PROTOCOLS ] |= VirtualRoots;
 
+    else if( atom == net_desktop_layout )
+        p->properties[ PROTOCOLS2 ] |= WM2DesktopLayout;
+
     else if( atom == net_close_window )
         p->properties[ PROTOCOLS ] |= CloseWindow;
 
@@ -1652,6 +1671,29 @@
 }
 
 
+void NETRootInfo::setDesktopLayout(NET::Orientation orientation, int columns, int rows,
+    NET::DesktopLayoutCorner corner)
+{
+    p->desktop_layout_orientation = orientation;
+    p->desktop_layout_columns = columns;
+    p->desktop_layout_rows = rows;
+    p->desktop_layout_corner = corner;
+
+#ifdef   NETWMDEBUG
+    fprintf(stderr, "NETRootInfo::setDesktopLayout: %d %d %d %d\n",
+	    orientation, columns, rows, corner);
+#endif
+
+    long data[ 4 ];
+    data[ 0 ] = orientation;
+    data[ 1 ] = columns;
+    data[ 2 ] = rows;
+    data[ 3 ] = corner;
+    XChangeProperty(p->display, p->root, net_desktop_layout, XA_CARDINAL, 32,
+		    PropModeReplace, (unsigned char *) &data, 4);
+}
+
+
 void NETRootInfo::setShowingDesktop( bool showing ) {
     if (role == WindowManager) {
 	long d = p->showing_desktop = showing;
@@ -2547,6 +2589,36 @@
 	}
     }
 
+    if (dirty2 & WM2DesktopLayout) {
+        p->desktop_layout_orientation = OrientationHorizontal;
+        p->desktop_layout_corner = DesktopLayoutCornerTopLeft;
+        p->desktop_layout_columns = p->desktop_layout_rows = 0;
+	if (XGetWindowProperty(p->display, p->root, net_desktop_layout,
+			       0, MAX_PROP_SIZE, False, XA_CARDINAL, &type_ret,
+			       &format_ret, &nitems_ret, &unused, &data_ret)
+	    == Success) {
+	    if (type_ret == XA_CARDINAL && format_ret == 32) {
+                long* data = (long*) data_ret;
+                if( nitems_ret >= 4 && data[ 3 ] >= 0 && data[ 3 ] <= 3 )
+                    p->desktop_layout_corner = (NET::DesktopLayoutCorner)data[ 3 ];
+                if( nitems_ret >= 3 ) {
+                    if( data[ 0 ] >= 0 && data[ 0 ] <= 1 )
+                        p->desktop_layout_orientation = (NET::Orientation)data[ 0 ];
+                    p->desktop_layout_columns = data[ 1 ];
+                    p->desktop_layout_rows = data[ 2 ];
+                }
+	    }
+
+#ifdef    NETWMDEBUG
+	    fprintf(stderr, "NETRootInfo::updated: desktop layout updated (%d %d %d %d)\n",
+                p->desktop_layout_orientation, p->desktop_layout_columns,
+                p->desktop_layout_rows, p->desktop_layout_corner );
+#endif
+	    if ( data_ret )
+		XFree(data_ret);
+	}
+    }
+
     if (dirty2 & WM2ShowingDesktop) {
         p->showing_desktop = false;
 	if (XGetWindowProperty(p->display, p->root, net_showing_desktop,
@@ -2702,6 +2774,21 @@
 }
 
 
+NET::Orientation NETRootInfo::desktopLayoutOrientation() const {
+    return p->desktop_layout_orientation;
+}
+
+
+QSize NETRootInfo::desktopLayoutColumnsRows() const {
+    return QSize( p->desktop_layout_columns, p->desktop_layout_rows );
+}
+
+
+NET::DesktopLayoutCorner NETRootInfo::desktopLayoutCorner() const {
+    return p->desktop_layout_corner;
+}
+
+
 int NETRootInfo::numberOfDesktops() const {
     return p->number_of_desktops == 0 ? 1 : p->number_of_desktops;
 }
--- branches/KDE/3.5/kdelibs/kdecore/netwm.h #669898:669899
@@ -373,6 +373,22 @@
     int virtualRootsCount() const;
 
     /**
+       Returns the desktop layout orientation.
+    **/
+    NET::Orientation desktopLayoutOrientation() const;
+
+    /**
+       Returns the desktop layout number of columns and rows. Note that
+       either may be 0 (see _NET_DESKTOP_LAYOUT).
+    **/
+    QSize desktopLayoutColumnsRows() const;
+
+    /**
+       Returns the desktop layout starting corner.
+    **/
+    NET::DesktopLayoutCorner desktopLayoutCorner() const;
+
+    /**
        Returns the number of desktops.
 
        @return the number of desktops
@@ -518,6 +534,13 @@
     void setVirtualRoots(Window *windows, unsigned int count);
     
     /**
+       Sets the desktop layout. This is set by the pager. When setting, the pager must
+       own the _NET_DESKTOP_LAYOUT_Sn manager selection. See _NET_DESKTOP_LAYOUT for details.
+    **/
+    void setDesktopLayout(NET::Orientation orientation, int columns, int rows,
+        NET::DesktopLayoutCorner corner);
+
+    /**
      * Sets the _NET_SHOWING_DESKTOP status (whether desktop is being shown).
      * @since 3.5
      */
--- branches/KDE/3.5/kdelibs/kdecore/netwm_def.h #669898:669899
@@ -590,6 +590,7 @@
         @li WM2WindowClass  WM_CLASS
         @li WM2WindowRole   WM_WINDOW_ROLE
         @li WM2ClientMachine WM_CLIENT_MACHINE
+        @li WM2DesktopLayout _NET_DESKTOP_LAYOUT
         
         @since 3.2
 
@@ -608,7 +609,8 @@
         WM2WindowClass         = 1<<10, ///< @since 3.3
         WM2WindowRole          = 1<<11, ///< @since 3.3
         WM2ClientMachine       = 1<<12, ///< @since 3.3
-        WM2ShowingDesktop      = 1<<13  ///< @since 3.5
+        WM2ShowingDesktop      = 1<<13, ///< @since 3.5
+        WM2DesktopLayout       = 1<<15  ///< @since 3.5.8
     };
 
     /**
@@ -632,6 +634,24 @@
     };
     
     /**
+      Orientation.
+    **/
+    enum Orientation {
+        OrientationHorizontal = 0,
+        OrientationVertical = 1
+    };
+    
+    /**
+     Starting corner for desktop layout.
+    **/
+    enum DesktopLayoutCorner {
+        DesktopLayoutCornerTopLeft = 0,
+        DesktopLayoutCornerTopRight = 1,
+        DesktopLayoutCornerBottomLeft = 2,
+        DesktopLayoutCornerBottomRight = 3
+    };
+    
+    /**
      Compares two X timestamps, taking into account wrapping and 64bit architectures.
      Return value is like with strcmp(), 0 for equal, -1 for time1 < time2, 1 for time1 > time2.
      @since 3.5.3
--- branches/KDE/3.5/kdelibs/kdecore/netwm_p.h #669898:669899
@@ -103,6 +103,9 @@
     unsigned long clients_count, stacking_count, virtual_roots_count,
 	kde_system_tray_windows_count;
     bool showing_desktop;
+    NET::Orientation desktop_layout_orientation;
+    NET::DesktopLayoutCorner desktop_layout_corner;
+    int desktop_layout_columns, desktop_layout_rows;
 
     unsigned long properties[ 5 ];
     unsigned long client_properties[ 5 ]; // properties the client is interested in