Clean up maximization and un-maximization logic

We should not try to un-maximize the windows from inside
the function wMaximizeWindow(), as that makes no sense.

If the window is already maximized then we don't call
wMaximizeWindow() anymore and call wUnmaximizeWindow()
instead, which will use the old geometry regardless
of which maximization is active (horizontal, vertical,
maximus, etc). And the old geometry now is also saved
when we enter wMaximizeWindow().

So when we call wMaximizeWindow() or wUnmaximizeWindow()
we really mean "maximize to the specified state" or
"go back to whatever old geometry coordinates we had before".

src/actions.c

@@ -62,6 +62,7 @@ extern Atom _XA_WM_TAKE_FOCUS;
 extern void ProcessPendingEvents();
 extern int calcIntersectionLength(int p1, int l1, int p2, int l2);
 
+static void save_old_geometry(WWindow *wwin);
 static void find_Maximus_geometry(WWindow *wwin, WArea usableArea, int *new_x,
 				  int *new_y, int *new_width, int *new_height);
 
@@ -292,16 +293,27 @@ void wUnshadeWindow(WWindow *wwin)
 	WMPostNotificationName(WMNChangedState, wwin, "shade");
 }
 
+/* Set the old coordinates using the current values */
+static void save_old_geometry(WWindow *wwin)
+{
+	wwin->old_geometry.width = wwin->client.width;
+	wwin->old_geometry.height = wwin->client.height;
+	wwin->old_geometry.x = wwin->frame_x;
+	wwin->old_geometry.y = wwin->frame_y;
+}
+
 void wMaximizeWindow(WWindow * wwin, int directions)
 {
 	int new_x, new_y;
 	unsigned int new_width, new_height, half_scr_width;
-	int changed_h, changed_v, shrink_h, shrink_v;
 	WArea usableArea, totalArea;
 
 	if (!IS_RESIZABLE(wwin))
 		return;
 
+	/* save old coordinates before we change the current values */
+	save_old_geometry(wwin);
+
 	totalArea.x1 = 0;
 	totalArea.y1 = 0;
 	totalArea.x2 = wwin->screen_ptr->scr_width;
@@ -320,6 +332,9 @@ void wMaximizeWindow(WWindow * wwin, int directions)
 		usableArea = wGetUsableAreaForHead(scr, head, &totalArea, True);
 	}
 
+	/* Only save directions, not kbd or xinerama hints */
+	directions &= (MAX_HORIZONTAL | MAX_VERTICAL | MAX_LEFTHALF | MAX_RIGHTHALF | MAX_MAXIMUS);
+
 	if (WFLAGP(wwin, full_maximize)) {
 		usableArea = totalArea;
 	}
@@ -329,33 +344,6 @@ void wMaximizeWindow(WWindow * wwin, int directions)
 		wwin->flags.skip_next_animation = 1;
 		wUnshadeWindow(wwin);
 	}
-	/* Only save directions, not kbd or xinerama hints */
-	directions &= (MAX_HORIZONTAL | MAX_VERTICAL | MAX_LEFTHALF | MAX_RIGHTHALF | MAX_MAXIMUS);
-
-	changed_h = ((wwin->flags.maximized ^ directions) & MAX_HORIZONTAL);
-	changed_v = ((wwin->flags.maximized ^ directions) & MAX_VERTICAL);
-	shrink_h = (changed_h && (directions & MAX_HORIZONTAL) == 0);
-	shrink_v = (changed_v && (directions & MAX_VERTICAL) == 0);
-
-	if (wwin->flags.maximized) {
-		/* if already maximized in some direction, we only update the
-		 * appropriate old x, old y coordinates. This is necessary to
-		 * allow succesive maximizations in different directions without
-		 * the need to first do an un-maximize (to avoid flicker).
-		 */
-		if (!(wwin->flags.maximized & (MAX_HORIZONTAL|MAX_LEFTHALF|MAX_RIGHTHALF))) {
-			wwin->old_geometry.x = wwin->frame_x;
-		}
-		if (!(wwin->flags.maximized & MAX_VERTICAL)) {
-			wwin->old_geometry.y = wwin->frame_y;
-		}
-	} else {
-		wwin->old_geometry.width = wwin->client.width;
-		wwin->old_geometry.height = wwin->client.height;
-		wwin->old_geometry.x = wwin->frame_x;
-		wwin->old_geometry.y = wwin->frame_y;
-	}
-	wwin->flags.maximized = directions;
 
 	if (directions & MAX_HORIZONTAL) {
 		new_width = usableArea.x2 - usableArea.x1;
@@ -372,9 +360,6 @@ void wMaximizeWindow(WWindow * wwin, int directions)
 		if (HAS_BORDER(wwin))
 			new_width -= FRAME_BORDER_WIDTH * 2;
 		new_x = usableArea.x1 + half_scr_width;
-	} else if (shrink_h) {
-		new_x = wwin->old_geometry.x;
-		new_width = wwin->old_geometry.width;
 	} else {
 		new_x = wwin->frame_x;
 		new_width = wwin->frame->core->width;
@@ -389,9 +374,6 @@ void wMaximizeWindow(WWindow * wwin, int directions)
 			new_y -= wwin->frame->top_width;
 			new_height += wwin->frame->bottom_width - 1;
 		}
-	} else if (shrink_v) {
-		new_y = wwin->old_geometry.y;
-		new_height = wwin->old_geometry.height;
 	} else {
 		new_y = wwin->frame_y;
 		new_height = wwin->frame->core->height;
@@ -414,6 +396,8 @@ void wMaximizeWindow(WWindow * wwin, int directions)
 	WMPostNotificationName(WMNChangedState, wwin, "maximize");
 
 	wSoundPlay(WSOUND_MAXIMIZE);
+	/* set maximization state */
+	wwin->flags.maximized = directions;
 }
 
 /*
@@ -527,10 +511,9 @@ void wUnmaximizeWindow(WWindow * wwin)
 		wwin->flags.skip_next_animation = 1;
 		wUnshadeWindow(wwin);
 	}
-	x = ((wwin->flags.maximized & (MAX_HORIZONTAL|MAX_LEFTHALF|MAX_RIGHTHALF)) && wwin->old_geometry.x) ?
+	/* Use old coordinates if they are set, current values otherwise */
-	    wwin->old_geometry.x : wwin->frame_x;
+	x = wwin->old_geometry.x ? wwin->old_geometry.x : wwin->frame_x;
-	y = ((wwin->flags.maximized & MAX_VERTICAL) && wwin->old_geometry.y) ?
+	y = wwin->old_geometry.y ? wwin->old_geometry.y : wwin->frame_y;
-	    wwin->old_geometry.y : wwin->frame_y;
 	w = wwin->old_geometry.width ? wwin->old_geometry.width : wwin->client.width;
 	h = wwin->old_geometry.height ? wwin->old_geometry.height : wwin->client.height;
 

src/event.c

@@ -1434,80 +1434,62 @@ static void handleKeyPress(XEvent * event)
 		break;
 	case WKBD_MAXIMIZE:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = (MAX_VERTICAL | MAX_HORIZONTAL);
-
 			CloseWindowMenu(scr);
 
-			if (wwin->flags.maximized == newdir) {
+			if (wwin->flags.maximized == (MAX_VERTICAL | MAX_HORIZONTAL))
 				wUnmaximizeWindow(wwin);
-			} else {
+			else
-				wMaximizeWindow(wwin, newdir | MAX_KEYBOARD);
+				wMaximizeWindow(wwin, MAX_VERTICAL | MAX_HORIZONTAL | MAX_KEYBOARD);
-			}
 		}
 		break;
 	case WKBD_VMAXIMIZE:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = (MAX_VERTICAL ^ wwin->flags.maximized);
-
 			CloseWindowMenu(scr);
 
-			if (newdir) {
+			if (wwin->flags.maximized == MAX_VERTICAL)
-				wMaximizeWindow(wwin, newdir | MAX_KEYBOARD);
-			} else {
 				wUnmaximizeWindow(wwin);
-			}
+			else
+				wMaximizeWindow(wwin, MAX_VERTICAL | MAX_KEYBOARD);
 		}
 		break;
 	case WKBD_HMAXIMIZE:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = (MAX_HORIZONTAL ^ wwin->flags.maximized);
-
 			CloseWindowMenu(scr);
 
-			if (newdir) {
+			if (wwin->flags.maximized == MAX_HORIZONTAL)
-				wMaximizeWindow(wwin, newdir | MAX_KEYBOARD);
-			} else {
 				wUnmaximizeWindow(wwin);
-			}
+			else
+				wMaximizeWindow(wwin, MAX_HORIZONTAL | MAX_KEYBOARD);
 		}
 		break;
 	case WKBD_LHMAXIMIZE:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = (MAX_VERTICAL|MAX_LEFTHALF);
-
 			CloseWindowMenu(scr);
 
-			if (wwin->flags.maximized == newdir) {
+			if (wwin->flags.maximized == MAX_VERTICAL | MAX_LEFTHALF)
 				wUnmaximizeWindow(wwin);
-			} else {
+			else
-				wMaximizeWindow(wwin, newdir|MAX_KEYBOARD);
+				wMaximizeWindow(wwin, MAX_VERTICAL | MAX_LEFTHALF | MAX_KEYBOARD);
-			}
 		}
 		break;
 	case WKBD_RHMAXIMIZE:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = (MAX_VERTICAL|MAX_RIGHTHALF);
-
 			CloseWindowMenu(scr);
 
-			if (wwin->flags.maximized == newdir) {
+			if (wwin->flags.maximized == MAX_VERTICAL | MAX_RIGHTHALF)
 				wUnmaximizeWindow(wwin);
-			} else {
+			else
-				wMaximizeWindow(wwin, newdir|MAX_KEYBOARD);
+				wMaximizeWindow(wwin, MAX_VERTICAL | MAX_RIGHTHALF | MAX_KEYBOARD);
-			}
 		}
 		break;
 	case WKBD_MAXIMUS:
 		if (ISMAPPED(wwin) && ISFOCUSED(wwin) && IS_RESIZABLE(wwin)) {
-			int newdir = MAX_MAXIMUS;
-
 			CloseWindowMenu(scr);
 
-			if (wwin->flags.maximized == newdir) {
+			if (wwin->flags.maximized == MAX_MAXIMUS)
 				wUnmaximizeWindow(wwin);
-			} else {
+			else
-				wMaximizeWindow(wwin, newdir|MAX_KEYBOARD);
+				wMaximizeWindow(wwin, MAX_MAXIMUS | MAX_KEYBOARD);
-			}
 		}
 		break;
 	case WKBD_RAISE: