Found it. Gratuitous calls to XReparentWindow(), due to "virtual
screens".

Can somebody please explain to me (and to the manual page!) how virtual
screens are supposed to work (conceptually)? What is their relation to
workspaces? The manpage uses them as synonyms. Can I test them with
captive windows? Can someone think about and reply to the comments I put
in the CHANGES file?

Apologies to the people who use/need them, but personally I'd rather rip
them out completely, since they mess up a lot of stuff. Really.

Proposed patch (including some cleanup of horrible layout; the meat is
around the calls to XReparentWindow()):

# 
# old_revision [c9d55f94ff451e3433418942745f3b423665914e]
# 
# patch "CHANGES"
#  from [1528a7a23da608e5ed0808513f0777fc83c3d6ab]
#    to [5ab433a15ec4c9254e10dce0d45cbc1b0c168ded]
# 
# patch "add_window.c"
#  from [ccf9dbbb842ae20d02cf3b4bdb1a32450dcd0781]
#    to [850b6c6be10c800e02729eaa04095b830b255936]
# 
# patch "twm.h"
#  from [3e6e0a4e490b895b1227c13aeb6b8341a2108e05]
#    to [cff6f00ee1666cfedf1c26090a31ab2984898704]
# 
# patch "workmgr.c"
#  from [7cfcd5ceb2faada1894fda33e1b558cd120537b8]
#    to [33f0ea1748e6dc148b4218d5fa4166f606377aef]
# 
============================================================
--- CHANGES     1528a7a23da608e5ed0808513f0777fc83c3d6ab
+++ CHANGES     5ab433a15ec4c9254e10dce0d45cbc1b0c168ded
@@ -1534,3 +1534,22 @@ VirtualScreens {
        
        This is basically change #7 above done right.
        [Olaf "Rhialto" Seibert]
+
+    9 - Re-introduced TwmWindow.oldvs, used to avoid calling
+       XReparentWindow() when possibe (it messed up the stacking order
+       of windows). However, maybe the use of .vs should be rethought a
+       bit: in Vanish() it is now set to NULL with the old value kept
+       in .oldvs.  However the window is still a child of the same vs.
+       Maybe it is better not to set it to NULL and then, when *really*
+       changing the virtual screen, .vs can be used instead of .oldvs.
+
+       This whole "virtual screen" thing is unexplained in the manual,
+       which even uses it as a synonym for "workspace" already in the
+       introduction paragraph. (There also does not seem to be a way
+       now to test virtual screens in captive windows) I suspect that
+       all this causes lots of confusion, and when cleared up, can
+       simplify the code a lot. 
+
+       I also fixed up the horrible indentation in the functions
+       where I changed something.
+       [Olaf "Rhialto" Seibert]
============================================================
--- add_window.c        ccf9dbbb842ae20d02cf3b4bdb1a32450dcd0781
+++ add_window.c        850b6c6be10c800e02729eaa04095b830b255936
@@ -266,6 +266,8 @@ TwmWindow *AddWindow(Window w, int iconm
     tmp_win->wspmgr = iswman;
     tmp_win->iswinbox = iswinbox;
     tmp_win->vs = NULL;
+    tmp_win->oldvs = NULL;
+    tmp_win->savevs = NULL;
     tmp_win->cmaps.number_cwins = 0;
     tmp_win->savegeometry.width = -1;
 
@@ -594,6 +596,7 @@ TwmWindow *AddWindow(Window w, int iconm
     } else
 #endif      
       SetupOccupation (tmp_win, 0);
+    tmp_win->oldvs = vs;
     /*=================================================================*/
 
     tmp_win->frame_width  = tmp_win->attr.width  + 2 * tmp_win->frame_bw3D;
============================================================
--- twm.h       3e6e0a4e490b895b1227c13aeb6b8341a2108e05
+++ twm.h       cff6f00ee1666cfedf1c26090a31ab2984898704
@@ -415,6 +415,7 @@ struct TwmWindow
        unsigned int width, height;
     } savegeometry;
     struct virtualScreen *vs;
+    struct virtualScreen *oldvs;
     struct virtualScreen *savevs;
 
 #ifdef X11R6
============================================================
--- workmgr.c   7cfcd5ceb2faada1894fda33e1b558cd120537b8
+++ workmgr.c   33f0ea1748e6dc148b4218d5fa4166f606377aef
@@ -962,9 +962,9 @@ void Occupy (TwmWindow *twm_win)
     if ((y + yoffset) > Scr->rooth) y = Scr->rooth - yoffset;
 
     Scr->workSpaceMgr.occupyWindow->twm_win->occupation = twm_win->occupation;
-    if (Scr->Root != Scr->CaptiveRoot)
+    if (Scr->Root != Scr->CaptiveRoot) {
       XReparentWindow  (dpy, Scr->workSpaceMgr.occupyWindow->twm_win->frame, 
Scr->Root, x, y);
-    else
+    } else
       XMoveWindow  (dpy, Scr->workSpaceMgr.occupyWindow->twm_win->frame, x, y);
 
     SetMapStateProp (Scr->workSpaceMgr.occupyWindow->twm_win, NormalState);
@@ -1262,11 +1262,11 @@ static void Vanish (virtualScreen *vs, T
     XWindowAttributes winattrs;
     unsigned long     eventMask;
 
-    if (vs && tmp_win->vs && (tmp_win->vs != vs)) return;
+    if (vs && tmp_win->vs && tmp_win->vs != vs)
+       return;
     if (tmp_win->UnmapByMovingFarAway) {
-        XMoveWindow (dpy, tmp_win->frame, Scr->rootw + 1, Scr->rooth + 1);
-    } else
-    if (tmp_win->mapped) {
+       XMoveWindow (dpy, tmp_win->frame, Scr->rootw + 1, Scr->rooth + 1);
+    } else if (tmp_win->mapped) {
        XGetWindowAttributes(dpy, tmp_win->w, &winattrs);
        eventMask = winattrs.your_event_mask;
        XSelectInput (dpy, tmp_win->w, eventMask & ~StructureNotifyMask);
@@ -1274,9 +1274,9 @@ static void Vanish (virtualScreen *vs, T
        XUnmapWindow (dpy, tmp_win->frame);
        XSelectInput (dpy, tmp_win->w, eventMask);
 
-       if (!tmp_win->DontSetInactive) SetMapStateProp (tmp_win, InactiveState);
-    } else
-    if (tmp_win->icon_on && tmp_win->icon && tmp_win->icon->w) {
+       if (!tmp_win->DontSetInactive)
+           SetMapStateProp (tmp_win, InactiveState);
+    } else if (tmp_win->icon_on && tmp_win->icon && tmp_win->icon->w) {
        XUnmapWindow (dpy, tmp_win->icon->w);
        IconDown (tmp_win);
     }
@@ -1287,23 +1287,30 @@ static void Vanish (virtualScreen *vs, T
      * may not.  The purpose of this is in the event of a ctwm death/restart,
      * geometries of windows that were on unmapped workspaces will show
      * up where they belong.
-     *
+     * XXX - XReparentWindow() messes up the stacking order of windows.
+     * It should be avoided as much as possible. This already affects
+     * switching away from and back to a workspace. Therefore do this only
+     * if there are at least 2 virtual screens AND the new one (firstvs)
+     * differs from where the window currently is. (Olaf Seibert).
      */
 
-    if(Scr->vScreenList) {
+    if (Scr->vScreenList && Scr->vScreenList->next) {
        int x, y;
        unsigned int junk;
        Window junkW, w = tmp_win->frame;
        virtualScreen *firstvs = NULL;
-       for(firstvs = Scr->vScreenList; firstvs; firstvs = firstvs->next)
-           if(firstvs->x == 0 && firstvs->y == 0)
-               break;
-       if(firstvs) {
+
+       for (firstvs = Scr->vScreenList; firstvs; firstvs = firstvs->next)
+           if (firstvs->x == 0 && firstvs->y == 0)
+               break;
+       if (firstvs && firstvs != vs) {
            XGetGeometry (dpy, w, &junkW, &x, &y, &junk, &junk, &junk, &junk);
            XReparentWindow(dpy, w, firstvs->window, x, y);
+           tmp_win->vs = firstvs;
        }
     }
 
+    tmp_win->oldvs = tmp_win->vs;
     tmp_win->vs = NULL;
 }
 
@@ -1312,31 +1319,37 @@ static void DisplayWin (virtualScreen *v
     XWindowAttributes  winattrs;
     unsigned long      eventMask;
 
-    if (vs && tmp_win->vs) return;
+    if (vs && tmp_win->vs)
+       return;
+
     tmp_win->vs = vs;
 
     if (!tmp_win->mapped) {
-      if (tmp_win->isicon) {
-       if (tmp_win->icon_on) {
-         if (tmp_win->icon && tmp_win->icon->w) {
-           int x, y;
-           unsigned int junk;
-           Window junkW, w = tmp_win->icon->w;
-           XGetGeometry (dpy, w, &junkW, &x, &y, &junk, &junk, &junk, &junk);
-           XReparentWindow (dpy, w, vs->window, x, y);
+       if (tmp_win->isicon) {
+           if (tmp_win->icon_on) {
+               if (tmp_win->icon && tmp_win->icon->w) {
+                   if (vs != tmp_win->oldvs) {
+                       int x, y;
+                       unsigned int junk;
+                       Window junkW, w = tmp_win->icon->w;
+                       XGetGeometry (dpy, w, &junkW, &x, &y, &junk, &junk, 
&junk, &junk);
+                       XReparentWindow (dpy, w, vs->window, x, y);
+                   }
 
-           IconUp (tmp_win);
-           XMapWindow (dpy, tmp_win->icon->w);
-           return;
-         }
+                   IconUp (tmp_win);
+                   XMapWindow (dpy, tmp_win->icon->w);
+                   return;
+               }
+           }
        }
-      }
-      return;
+       return;
     }
     if (tmp_win->UnmapByMovingFarAway) {
-        if (vs) XReparentWindow (dpy, tmp_win->frame, vs->window,
-                                tmp_win->frame_x, tmp_win->frame_y);
-       else XMoveWindow (dpy, tmp_win->frame, tmp_win->frame_x, 
tmp_win->frame_y);
+       if (vs)
+           XReparentWindow (dpy, tmp_win->frame, vs->window,
+               tmp_win->frame_x, tmp_win->frame_y);
+       else
+           XMoveWindow (dpy, tmp_win->frame, tmp_win->frame_x, 
tmp_win->frame_y);
     } else {
        if (!tmp_win->squeezed) {
            XGetWindowAttributes(dpy, tmp_win->w, &winattrs);
@@ -1345,7 +1358,9 @@ static void DisplayWin (virtualScreen *v
            XMapWindow   (dpy, tmp_win->w);
            XSelectInput (dpy, tmp_win->w, eventMask);
        }
-       XReparentWindow (dpy, tmp_win->frame, vs->window, tmp_win->frame_x, 
tmp_win->frame_y);
+       if (vs != tmp_win->oldvs) {
+           XReparentWindow (dpy, tmp_win->frame, vs->window, tmp_win->frame_x, 
tmp_win->frame_y);
+       }
        XMapWindow (dpy, tmp_win->frame);
        SetMapStateProp (tmp_win, NormalState);
     }

-Olaf.
-- 
___ Olaf 'Rhialto' Seibert      -- You author it, and I'll reader it.
\X/ rhialto/at/xs4all.nl        -- Cetero censeo "authored" delendum esse.

Reply via email to