Re: [compiz] [PATCH] Grab state & Last state remembering

[Danny Baumann]

Hi,

> > - Remembering of the last window state inside the state change notify
> > This is useful for properly tracking down the window state changes (e.g.
> > has the window been maximized in that state change?). In Beryl, we added
> > a variable lastState into CompWindow, which is updated inside core's
> > stateChangeNotify. The attached last-window-state.patch brings that
> > behaviour to Compiz.
> 
> I've applied the patch. However, all code that cause a change to the
> state should be modified so that a mask of changed bits can be passed to
> the state change notify function instead. This lastState variable
> shouldn't have to exist.

In the attached patch, I've modified your recently added function
changeWindowState to make the passing of a changed state bit mask
easier:
- Don't modify w->state outside of changeWindowState
- Issue the state change notify only if the state was changed

I think with these changes, only the actual bitmask passing is missing.
What do you think?

Regards,

Danny

diff --git a/src/event.c b/src/event.c
index b7f1d61..d2f1d22 100644
--- a/src/event.c
+++ b/src/event.c
@@ -1290,9 +1290,9 @@ handleEvent (CompDisplay *d,
 		if (w->state & CompWindowStateHiddenMask)
 		{
 		    w->minimized = FALSE;
-		    w->state &= ~CompWindowStateHiddenMask;
 
-		    changeWindowState (w, w->state);
+		    changeWindowState (w, w->state &
+			    ~CompWindowStateHiddenMask);
 
 		    updateClientListForScreen (w->screen);
 		}
@@ -1645,13 +1645,11 @@ handleEvent (CompDisplay *d,
 
 		if (wState != w->state)
 		{
-		    w->state = wState;
+		    changeWindowState (w, wState);
 
 		    recalcWindowType (w);
 		    recalcWindowActions (w);
 
-		    changeWindowState (w, w->state);
-
 		    updateWindowAttributes (w, FALSE);
 		}
 	    }
@@ -1927,8 +1925,6 @@ handleEvent (CompDisplay *d,
 	    w = findWindowAtDisplay (d, event->xfocus.window);
 	    if (w && w->managed)
 	    {
-		unsigned int state = w->state;
-
 		if (w->id != d->activeWindow)
 		{
 		    d->activeWindow = w->id;
@@ -1940,10 +1936,8 @@ handleEvent (CompDisplay *d,
 				     (unsigned char *) &w->id, 1);
 		}
 
-		state &= ~CompWindowStateDemandsAttentionMask;
-
-		if (w->state != state)
-		    changeWindowState (w, state);
+		changeWindowState (w, w->state &
+			~CompWindowStateDemandsAttentionMask);
 	    }
 	}
 	break;
diff --git a/src/window.c b/src/window.c
index 6fcfc21..11924ae 100644
--- a/src/window.c
+++ b/src/window.c
@@ -568,14 +568,17 @@ changeWindowState (CompWindow   *w,
 		   unsigned int newState)
 {
     CompDisplay *d = w->screen->display;
+    unsigned int lastState = w->state;
 
     w->state = newState;
 
     setWindowState (d, w->state, w->id);
 
-    (*w->screen->windowStateChangeNotify) (w);
-
-    (*d->matchPropertyChanged) (d, w);
+    if (w->state != lastState)
+    {
+       (*w->screen->windowStateChangeNotify) (w);
+       (*d->matchPropertyChanged) (d, w);
+    }
 }
 
 static void
@@ -3878,7 +3881,9 @@ activateWindow (CompWindow *w)
 
     if (w->state & CompWindowStateHiddenMask)
     {
-	w->state &= ~CompWindowStateShadedMask;
+	changeWindowState (w, w->state &
+		~CompWindowStateShadedMask);
+
 	if (w->shaded)
 	    showWindow (w);
     }
@@ -4098,6 +4103,7 @@ void
 hideWindow (CompWindow *w)
 {
     Bool onDesktop = onCurrentDesktop (w);
+    unsigned int state = w->state;
 
     if (!w->managed)
 	return;
@@ -4127,7 +4133,7 @@ hideWindow (CompWindow *w)
 	return;
 
     if (w->minimized || w->inShowDesktopMode || w->hidden || w->shaded)
-	w->state |= CompWindowStateHiddenMask;
+	state |= CompWindowStateHiddenMask;
 
     w->pendingUnmaps++;
 
@@ -4136,13 +4142,14 @@ hideWindow (CompWindow *w)
 
     XUnmapWindow (w->screen->display->display, w->id);
 
-    changeWindowState (w, w->state);
+    changeWindowState (w, state);
 }
 
 void
 showWindow (CompWindow *w)
 {
     Bool onDesktop = onCurrentDesktop (w);
+    unsigned int state = w->state;
 
     if (!w->managed)
 	return;
@@ -4154,9 +4161,9 @@ showWindow (CompWindow *w)
 	{
 	    if (w->state & CompWindowStateHiddenMask)
 	    {
-		w->state &= ~CompWindowStateHiddenMask;
+		state &= ~CompWindowStateHiddenMask;
 
-		changeWindowState (w, w->state);
+		changeWindowState (w, state);
 	    }
 	}
 
@@ -4186,13 +4193,13 @@ showWindow (CompWindow *w)
 	w->shaded = FALSE;
     }
 
-    w->state &= ~CompWindowStateHiddenMask;
+    state &= ~CompWindowStateHiddenMask;
 
     w->pendingMaps++;
 
     XMapWindow (w->screen->display->display, w->id);
 
-    changeWindowState (w, w->state);
+    changeWindowState (w, state);
 }
 
 static void
@@ -4259,6 +4266,8 @@ void
 maximizeWindow (CompWindow *w,
 		int	   state)
 {
+    unsigned int newState = w->state;
+
     if (w->attrib.override_redirect)
 	return;
 
@@ -4269,13 +4278,13 @@ maximizeWindow (CompWindow *w,
     if (state == (w->state & MAXIMIZE_STATE))
 	return;
 
-    w->state &= ~MAXIMIZE_STATE;
-    w->state |= state;
+    newState &= ~MAXIMIZE_STATE;
+    newState |= state;
 
     recalcWindowType (w);
     recalcWindowActions (w);
 
-    changeWindowState (w, w->state);
+    changeWindowState (w, newState);
 
     updateWindowAttributes (w, FALSE);
 }

_______________________________________________
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz