Re: multiwindow support for _NET_WM_STATE_SKIP_TASKBAR

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

Jon TURNEY wrote:
> Yaakov (Cygwin/X) wrote:
> > Yaakov (Cygwin/X) wrote:
> > > I have noticed that normal windows don't appear in the taskbar until
> > > they are refocused upon.  Can you confirm?
> >
> > Ping?
>
> I'm not dead, just sleeping ;-)
>
> Yes, I see the same behaviour.  When I looked into this, it seems to be 
> because the taskbar doesn't re-evaluate if the window should be in it or 
> not when the style is changed, but only when a ShowWindow() occurs.

Attached is an updated version of this patch which fixes that problem, but the 
changes involved are rather larger than I'm happy with.

This patch re-arranges the sequence of events at window creation so we check 
all the styles before we first show the window (which is good)

However, it then exposes another problem: at the moment we only do this window 
styling at window creation time, if the hinting properties are updated after 
the window is shown, we don't use them.

This is accidentally demonstrated in the small test case posted previously as 
XMapWindow() occurs before XChangeProperty(): reverse the order and window is 
reliably styled correctly, but as it stands it is timing sensitive as to if 
the XChangeProperty() takes effect before or after we show the window...

So, I've added some more code to use the PropertyNotify event to watch for 
hint changes, but as written this is very inefficient, so needs more work...

Process _NET_WM_STATE_SKIP_TASKBAR hint in multiwindow mode.

To ensure that taskbar learns the correct state initially (since it
only updates it's idea of if it should show a button or not on a ShowWindow()),
we must be a bit more careful to do things in the right order and set all
the style flags before we show the window.

This is probably the right thing to do in any case as it means we can avoid
the messy appearance of the window changing style just after it is first shown

In this implementation, this is achieved by having WM_CREATE send a WM_WM_CREATE
message to our window manager thread, which then does all the work of discovering
the windows style; it's convenient to do that there as accessing X internals is
awkward in the wndproc; possibly it's more than convenient as there might be some
deadlock issue that thread avoids...

But note that this subtly changes the semantics of winCreateWindowsWindow():
previously the window was visible and drawn before that function returned, now
that happens asychronously; I'm not sure if that could cause problems or not...

---
 xserver/hw/xwin/winmultiwindowwm.c      |   43 ++
+++++++++++++++++++++++-------
 xserver/hw/xwin/winmultiwindowwindow.c  |    6 
 xserver/hw/xwin/winmultiwindowwm.c      |  202 +++++++++++++++++++-------------
 xserver/hw/xwin/winmultiwindowwndproc.c |   48 +++----
 xserver/hw/xwin/winwindow.h             |    1 
 4 files changed, 149 insertions(+), 108 deletions(-)

Index: xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwm.c
===================================================================
--- xorg-server-1.5.3.orig/xserver/hw/xwin/winmultiwindowwm.c	2009-01-06 02:41:28.901000000 +0000
+++ xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwm.c	2009-01-06 02:42:54.093750000 +0000
@@ -485,31 +485,24 @@
   return XSendEvent (pDisplay, iWin, False, NoEventMask, &e);
 }
 
-
 /*
- * Updates the name of a HWND according to its X WM_NAME property
+ * See if we can get the stored HWND for this window...
  */
-
-static void
-UpdateName (WMInfoPtr pWMInfo, Window iWindow)
+static HWND
+getHwnd (WMInfoPtr pWMInfo, Window iWindow)
 {
-  wchar_t		*pszName;
   Atom			atmType;
   int			fmtRet;
   unsigned long		items, remain;
-  HWND			*retHwnd, hWnd;
-  XWindowAttributes	attr;
+  HWND			*retHwnd, hWnd = NULL;
 
-  hWnd = 0;
-
-  /* See if we can get the cached HWND for this window... */
   if (XGetWindowProperty (pWMInfo->pDisplay,
 			  iWindow,
 			  pWMInfo->atmPrivMap,
 			  0,
 			  1,
 			  False,
-			  XA_INTEGER,//pWMInfo->atmPrivMap,
+			  XA_INTEGER,
 			  &atmType,
 			  &fmtRet,
 			  &items,
@@ -524,8 +517,25 @@
     }
   
   /* Some sanity checks */
+  if (!hWnd) return NULL;
+  if (!IsWindow (hWnd)) return NULL;
+
+  return hWnd;
+}
+
+/*
+ * Updates the name of a HWND according to its X WM_NAME property
+ */
+
+static void
+UpdateName (WMInfoPtr pWMInfo, Window iWindow)
+{
+  wchar_t		*pszName;
+  HWND			hWnd;
+  XWindowAttributes	attr;
+
+  hWnd = getHwnd (pWMInfo, iWindow);
   if (!hWnd) return;
-  if (!IsWindow (hWnd)) return;
 
   /* Set the Windows window name */
   GetWindowName (pWMInfo->pDisplay, iWindow, &pszName);
@@ -554,36 +564,12 @@
 static void
 PreserveWin32Stack(WMInfoPtr pWMInfo, Window iWindow, UINT direction)
 {
-  Atom                  atmType;
-  int                   fmtRet;
-  unsigned long         items, remain;
-  HWND                  hWnd, *retHwnd;
+  HWND                  hWnd;
   DWORD                 myWinProcID, winProcID;
   Window                xWindow;
   WINDOWPLACEMENT       wndPlace;
-  
-  hWnd = NULL;
-  /* See if we can get the cached HWND for this window... */
-  if (XGetWindowProperty (pWMInfo->pDisplay,
-			  iWindow,
-			  pWMInfo->atmPrivMap,
-			  0,
-			  1,
-			  False,
-			  XA_INTEGER,//pWMInfo->atmPrivMap,
-			  &atmType,
-			  &fmtRet,
-			  &items,
-			  &remain,
-			  (unsigned char **) &retHwnd) == Success)
-    {
-      if (retHwnd)
-	{
-	  hWnd = *retHwnd;
-	  XFree (retHwnd);
-	}
-    }
-  
+
+  hWnd = getHwnd (pWMInfo, iWindow);
   if (!hWnd) return;
   
   GetWindowThreadProcessId (hWnd, &myWinProcID);
@@ -660,6 +646,35 @@
       /* Branch on the message type */
       switch (pNode->msg.msg)
 	{
+	case WM_WM_CREATE:
+#if CYGMULTIWINDOW_DEBUG
+	  ErrorF ("\tWM_WM_CREATE\n");
+#endif
+	  /* Put a note as to the HWND associated with this Window */
+	  XChangeProperty (pWMInfo->pDisplay,
+			   pNode->msg.iWindow,
+			   pWMInfo->atmPrivMap,
+			   XA_INTEGER,
+			   32,
+			   PropModeReplace,
+			   (unsigned char *) &(pNode->msg.hwndWindow),
+			   1);
+
+	  /* Determine the Window style, which determines borders and clipping region... */
+	  {
+	    HWND zstyle = HWND_NOTOPMOST;
+	    winApplyHints (pWMInfo->pDisplay, pNode->msg.iWindow, pNode->msg.hwndWindow, &zstyle);
+	    winUpdateWindowPosition (pNode->msg.hwndWindow, TRUE, &zstyle);
+	  }
+
+	  /* Display the window without activating it */
+	  ShowWindow (pNode->msg.hwndWindow, SW_SHOWNOACTIVATE);
+
+	  /* Send first paint message */
+	  UpdateWindow (pNode->msg.hwndWindow);
+
+	  break;
+
 #if 0
 	case WM_WM_MOVE:
 	  ErrorF ("\tWM_WM_MOVE\n");
@@ -694,36 +709,16 @@
 #if CYGMULTIWINDOW_DEBUG
 	  ErrorF ("\tWM_WM_MAP\n");
 #endif
-	  /* Put a note as to the HWND associated with this Window */
-	  XChangeProperty (pWMInfo->pDisplay,
-			   pNode->msg.iWindow,
-			   pWMInfo->atmPrivMap,
-			   XA_INTEGER,//pWMInfo->atmPrivMap,
-			   32,
-			   PropModeReplace,
-			   (unsigned char *) &(pNode->msg.hwndWindow),
-			   1);
 	  UpdateName (pWMInfo, pNode->msg.iWindow);
 	  winUpdateIcon (pNode->msg.iWindow);
-	  {
-	    HWND zstyle = HWND_NOTOPMOST;
-	    winApplyHints (pWMInfo->pDisplay, pNode->msg.iWindow, pNode->msg.hwndWindow, &zstyle);
-	    winUpdateWindowPosition (pNode->msg.hwndWindow, TRUE, &zstyle);
-	  }
+
 	  break;
 
 	case WM_WM_MAP2:
 #if CYGMULTIWINDOW_DEBUG
 	  ErrorF ("\tWM_WM_MAP2\n");
 #endif
-	  XChangeProperty (pWMInfo->pDisplay,
-			   pNode->msg.iWindow,
-			   pWMInfo->atmPrivMap,
-			   XA_INTEGER,//pWMInfo->atmPrivMap,
-			   32,
-			   PropModeReplace,
-			   (unsigned char *) &(pNode->msg.hwndWindow),
-			   1);
+
 	  break;
 
 	case WM_WM_UNMAP:
@@ -786,6 +781,22 @@
 
 	case WM_WM_HINTS_EVENT:
 	  winUpdateIcon (pNode->msg.iWindow);
+
+	  pNode->msg.hwndWindow = getHwnd(pWMInfo, pNode->msg.iWindow);
+
+	  /* Hide the window whilst we change it */
+	  ShowWindow (pNode->msg.hwndWindow, SW_HIDE);
+
+	  /* Determine the Window style, which determines borders and clipping region... */
+	  {
+	    HWND zstyle = HWND_NOTOPMOST;
+	    winApplyHints (pWMInfo->pDisplay, pNode->msg.iWindow, pNode->msg.hwndWindow, &zstyle);
+	    winUpdateWindowPosition (pNode->msg.hwndWindow, TRUE, &zstyle);
+	  }
+
+	  /* Display the window without activating it */
+	  ShowWindow (pNode->msg.hwndWindow, SW_SHOWNOACTIVATE);
+
 	  break;
 
 	case WM_WM_CHANGE_STATE:
@@ -1040,8 +1051,13 @@
 				  event.xcreatewindow.window,
 				  0);
 	}
-      else if (event.type == PropertyNotify
-	       && event.xproperty.atom == atmWmName)
+      else if (event.type == PropertyNotify)
+	{
+	  char *atomName = XGetAtomName(pProcArg->pDisplay, event.xproperty.atom);
+	  winDebug("winMultiWindowXMsgProc: PropertyNotify %s\n", atomName);
+	  XFree(atomName);
+
+	  if (event.xproperty.atom == atmWmName)
 	{
 	  memset (&msg, 0, sizeof (msg));
 
@@ -1051,9 +1067,12 @@
 	  /* Other fields ignored */
 	  winSendMessageToWM (pProcArg->pWMInfo, &msg);
 	}
-      else if (event.type == PropertyNotify
-	       && event.xproperty.atom == atmWmHints)
+      else if (event.type == PropertyNotify)
 	{
+	  /*
+	    Lots of properties are considered for WM hints, rather than list them
+	     all here, we just check them all every time a property changes...
+	  */
 	  memset (&msg, 0, sizeof (msg));
 
 	  msg.msg = WM_WM_HINTS_EVENT;
@@ -1062,6 +1081,7 @@
 	  /* Other fields ignored */
 	  winSendMessageToWM (pProcArg->pWMInfo, &msg);
 	}
+	}
       else if (event.type == ClientMessage
 	       && event.xclient.message_type == atmWmChange
 	       && event.xclient.data.l[0] == IconicState)
@@ -1470,6 +1490,8 @@
 #define HINT_BORDER	(1L<<1)
 #define HINT_SIZEBOX	(1l<<2)
 #define HINT_CAPTION	(1l<<3)
+/* Windows extended window styles */
+#define HINT_SKIPTASKBAR	(1L<<0)
 /* These two are used on their own */
 #define HINT_MAX	(1L<<0)
 #define HINT_MIN	(1L<<1)
@@ -1480,7 +1502,7 @@
   static Atom	windowState, motif_wm_hints, windowType;
   Atom		type, *pAtom = NULL;
   int 		format;
-  unsigned long	hint = 0, maxmin = 0, rcStyle, nitems = 0 , left = 0;
+  unsigned long	hint = 0, exHint = 0, maxmin = 0, rcStyle, nitems = 0 , left = 0;
   WindowPtr	pWin = GetProp (hWnd, WIN_WINDOW_PROP);
 
   if (!hWnd) return;
@@ -1491,22 +1513,31 @@
   if (windowType == None) windowType = XInternAtom(pDisplay, "_NET_WM_WINDOW_TYPE", False);
 
   if (XGetWindowProperty(pDisplay, iWindow, windowState, 0L,
-			 1L, False, XA_ATOM, &type, &format,
+			 MAXINT, False, XA_ATOM, &type, &format,
 			 &nitems, &left, (unsigned char **)&pAtom) == Success)
   {
-    if (pAtom && nitems == 1)
+    if (pAtom)
     {
-      static Atom hiddenState, fullscreenState, belowState, aboveState;
+      static Atom skiptaskbarState, hiddenState, fullscreenState, belowState, aboveState;
+
+      if (skiptaskbarState == None) skiptaskbarState = XInternAtom(pDisplay, "_NET_WM_STATE_SKIP_TASKBAR", False);
       if (hiddenState == None) hiddenState = XInternAtom(pDisplay, "_NET_WM_STATE_HIDDEN", False);
       if (fullscreenState == None) fullscreenState = XInternAtom(pDisplay, "_NET_WM_STATE_FULLSCREEN", False);
       if (belowState == None) belowState = XInternAtom(pDisplay, "_NET_WM_STATE_BELOW", False);
       if (aboveState == None) aboveState = XInternAtom(pDisplay, "_NET_WM_STATE_ABOVE", False);
-      if (*pAtom == hiddenState) maxmin |= HINT_MIN;
-      else if (*pAtom == fullscreenState) maxmin |= HINT_MAX;
-      if (*pAtom == belowState) *zstyle = HWND_BOTTOM;
-      else if (*pAtom == aboveState) *zstyle = HWND_TOPMOST;
+
+      unsigned long i;
+      for (i = 0; i < nitems; i++)
+	{
+	  if (*pAtom == skiptaskbarState) exHint |= HINT_SKIPTASKBAR;
+	  if (*pAtom == hiddenState) maxmin |= HINT_MIN;
+	  else if (*pAtom == fullscreenState) maxmin |= HINT_MAX;
+	  if (*pAtom == belowState) *zstyle = HWND_BOTTOM;
+	  else if (*pAtom == aboveState) *zstyle = HWND_TOPMOST;
+	}
+
+      XFree(pAtom);
     }
-    if (pAtom) XFree(pAtom);
   }
 
   nitems = left = 0;
@@ -1564,7 +1595,23 @@
   else if (rcStyle & STYLE_NOFRAME)
 	hint = (hint & ~HINT_BORDER & ~HINT_CAPTION & ~HINT_SIZEBOX) | HINT_NOFRAME;
 
+  /*
+    Moved from WM_SHOWWINDOW now we are a bit more careful to do things in the right
+    order and set all the style flags before we show the window ...
+    but what exactly are we trying to do here?
+  */
+  if (GetParent(hWnd))
+    /* Set the transient style flags */
+    SetWindowLongPtr (hWnd, GWL_STYLE,
+		      WS_POPUP | WS_OVERLAPPED | WS_SYSMENU | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
+  else
+    /* Set the window standard style flags */
+    SetWindowLongPtr (hWnd, GWL_STYLE,
+		      (WS_POPUP | WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_CLIPSIBLINGS)
+		      & ~WS_CAPTION & ~WS_SIZEBOX);
+
   SetWindowLongPtr (hWnd, GWL_STYLE, GetWindowLongPtr(hWnd, GWL_STYLE) & ~WS_CAPTION & ~WS_SIZEBOX); /* Just in case */
+
   if (!hint) /* All on, but no resize of children is allowed */
     SetWindowLongPtr (hWnd, GWL_STYLE, GetWindowLongPtr(hWnd, GWL_STYLE) | WS_CAPTION | (GetParent(hWnd) ? 0 : WS_SIZEBOX));
   else if (hint & HINT_NOFRAME); /* All off, so do nothing */
@@ -1573,7 +1620,10 @@
 			((hint & HINT_SIZEBOX) ? (GetParent(hWnd) ? 0 : WS_SIZEBOX) : 0) |
 			((hint & HINT_CAPTION) ? WS_CAPTION : 0));
 
-  return;
+  if (exHint & HINT_SKIPTASKBAR)
+    SetWindowLongPtr(hWnd, GWL_EXSTYLE, WS_EX_TOOLWINDOW);
+  else
+    SetWindowLongPtr(hWnd, GWL_EXSTYLE, WS_EX_APPWINDOW);
 }
 
 void
Index: xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwndproc.c
===================================================================
--- xorg-server-1.5.3.orig/xserver/hw/xwin/winmultiwindowwndproc.c	2009-01-06 02:41:28.911000000 +0000
+++ xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwndproc.c	2009-01-06 02:42:54.093750000 +0000
@@ -332,6 +332,22 @@
 #if CYGDEBUG
   winDebugWin32Message("winTopLevelWindowProc", hwnd, message, wParam, lParam);
 #endif
+
+  /*
+     If this is WM_CREATE, set up the Windows window properties which point to X window information,
+     before we populate other local variables...
+  */
+  if (message == WM_CREATE)
+    {
+      /* */
+      SetProp (hwnd,
+	       WIN_WINDOW_PROP,
+	       (HANDLE)((LPCREATESTRUCT) lParam)->lpCreateParams);
+      /* */
+      SetProp (hwnd,
+	       WIN_WID_PROP,
+	       (HANDLE)winGetWindowID (((LPCREATESTRUCT) lParam)->lpCreateParams));
+    }
   
   /* Check if the Windows window property for our X window pointer is valid */
   if ((pWin = GetProp (hwnd, WIN_WINDOW_PROP)) != NULL)
@@ -395,17 +411,6 @@
   switch (message)
     {
     case WM_CREATE:
-
-      /* */
-      SetProp (hwnd,
-	       WIN_WINDOW_PROP,
-	       (HANDLE)((LPCREATESTRUCT) lParam)->lpCreateParams);
-      
-      /* */
-      SetProp (hwnd,
-	       WIN_WID_PROP,
-	       (HANDLE)winGetWindowID (((LPCREATESTRUCT) lParam)->lpCreateParams));
-
       /*
        * Make X windows' Z orders sync with Windows windows because
        * there can be AlwaysOnTop windows overlapped on the window
@@ -423,6 +428,11 @@
 
       SetWindowLongPtr(hwnd, GWLP_USERDATA, (LONG_PTR)XMING_SIGNATURE);
 
+      /* Tell our Window Manager thread to style the window */
+      wmMsg.msg = WM_WM_CREATE;
+      if (fWMMsgInitialized)
+	winSendMessageToWM (s_pScreenPriv->pWMInfo, &wmMsg);
+
       return 0;
 
     case WM_INIT_SYS_MENU:
@@ -866,24 +876,10 @@
 	  /* Flag that this window needs to be made active when clicked */
 	  SetProp (hwnd, WIN_NEEDMANAGE_PROP, (HANDLE) 1);
 
-	  if (!(GetWindowLongPtr (hwnd, GWL_EXSTYLE) & WS_EX_APPWINDOW))
-	    {
 	      HWND		zstyle = HWND_NOTOPMOST;
-
-	      /* Set the window extended style flags */
-	      SetWindowLongPtr (hwnd, GWL_EXSTYLE, WS_EX_APPWINDOW);
-
-	      /* Set the transient style flags */
-	      if (GetParent(hwnd)) SetWindowLongPtr (hwnd, GWL_STYLE,
-		   WS_POPUP | WS_OVERLAPPED | WS_SYSMENU | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
-	      /* Set the window standard style flags */
-	      else SetWindowLongPtr (hwnd, GWL_STYLE,
-		   (WS_POPUP | WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_CLIPSIBLINGS)
-		   & ~WS_CAPTION & ~WS_SIZEBOX);
-
 	      winUpdateWindowPosition (hwnd, FALSE, &zstyle);
 	      SetForegroundWindow (hwnd);
-	    }
+
 	  wmMsg.msg = WM_WM_MAP;
 	}
       else /* It is an overridden window so make it top of Z stack */
Index: xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwindow.c
===================================================================
--- xorg-server-1.5.3.orig/xserver/hw/xwin/winmultiwindowwindow.c	2009-01-06 02:41:28.885000000 +0000
+++ xorg-server-1.5.3/xserver/hw/xwin/winmultiwindowwindow.c	2009-01-06 02:42:54.093750000 +0000
@@ -734,12 +734,6 @@
 	  winCreateWindowsWindow (pWin);
 	  assert (pWinPriv->hWnd != NULL);
 	}
-
-      /* Display the window without activating it */
-      ShowWindow (pWinPriv->hWnd, SW_SHOWNOACTIVATE);
-
-      /* Send first paint message */
-      UpdateWindow (pWinPriv->hWnd);
     }
   else if (hWnd != NULL)
     {
Index: xorg-server-1.5.3/xserver/hw/xwin/winwindow.h
===================================================================
--- xorg-server-1.5.3.orig/xserver/hw/xwin/winwindow.h	2009-01-06 02:41:28.939000000 +0000
+++ xorg-server-1.5.3/xserver/hw/xwin/winwindow.h	2009-01-06 02:42:54.093750000 +0000
@@ -116,6 +116,7 @@
 #define		WM_WM_HINTS_EVENT	(WM_USER + 10)
 #define		WM_WM_CHANGE_STATE	(WM_USER + 11)
 #define		WM_WM_MAP2		(WM_USER + 12)
+#define		WM_WM_CREATE		(WM_USER + 13)
 #define		WM_MANAGE		(WM_USER + 100)
 #define		WM_UNMANAGE		(WM_USER + 102)
 

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://x.cygwin.com/docs/
FAQ:                   http://x.cygwin.com/docs/faq/