From 8881068dc38f00fa3ca9e0d0340d5e70361ec47a Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <michael+git@stapelberg.de>
Date: Sat, 14 Feb 2009 02:19:04 +0100
Subject: [PATCH] Various code improvements

---
 include/data.h |  4 +--
 src/layout.c   | 69 ++++++++++++++++++++++++--------------------------
 src/mainx.c    | 36 +++++++++++++-------------
 3 files changed, 53 insertions(+), 56 deletions(-)

diff --git a/include/data.h b/include/data.h
index 203e7907..1fd856b3 100644
--- a/include/data.h
+++ b/include/data.h
@@ -101,8 +101,8 @@ struct Client {
 	/* Backpointer. A client is inside a container */
 	Container *container;
 
-	int x, y;
-	int width, height;
+	uint32_t x, y;
+	uint32_t width, height;
 
 	/* Name */
 	char *name;
diff --git a/src/layout.c b/src/layout.c
index cac32ff2..d98dc181 100644
--- a/src/layout.c
+++ b/src/layout.c
@@ -81,13 +81,8 @@ void decorate_window(xcb_connection_t *conn, Client *client) {
 	free(label);
 }
 
-void render_container(xcb_connection_t *connection, Container *container) {
+static void render_container(xcb_connection_t *connection, Container *container) {
 	Client *client;
-	uint32_t values[4];
-	uint32_t mask = XCB_CONFIG_WINDOW_X |
-			XCB_CONFIG_WINDOW_Y |
-			XCB_CONFIG_WINDOW_WIDTH |
-			XCB_CONFIG_WINDOW_HEIGHT;
 	i3Font *font = load_font(connection, pattern);
 
 	if (container->mode == MODE_DEFAULT) {
@@ -98,46 +93,48 @@ void render_container(xcb_connection_t *connection, Container *container) {
 
 		int current_client = 0;
 		CIRCLEQ_FOREACH(client, &(container->clients), clients) {
-			/* TODO: rewrite this block so that the need to puke vanishes :) */
-			/* TODO: at the moment, every column/row is 200px. This
+			/* TODO: at the moment, every column/row is screen / num_cols. This
 			 * needs to be changed to "percentage of the screen" by
 			 * default and adjustable by the user if necessary.
 			 */
-			values[0] = container->x + (container->col * container->width); /* x */
-			values[1] = container->y + (container->row * container->height +
-				(container->height / num_clients) * current_client); /* y */
 
-			if (client->x != values[0] || client->y != values[1]) {
-				printf("frame needs to be pushed to %dx%d\n",
-						values[0], values[1]);
-				client->x = values[0];
-				client->y = values[1];
+                        /* Check if we changed client->x or client->y by updating it…
+                         * Note the bitwise OR instead of logical OR to force evaluation of both statements */
+			if ((client->x != (client->x = container->x + (container->col * container->width))) |
+                            (client->y != (client->y = container->y + (container->row * container->height +
+                                          (container->height / num_clients) * current_client)))) {
+				printf("frame needs to be pushed to %dx%d\n", client->x, client->y);
+				/* Note: We can use a pointer to client->x like an array of uint32_ts
+				   because it is followed by client->y by definition */
 				xcb_configure_window(connection, client->frame,
-						XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y, values);
+						XCB_CONFIG_WINDOW_X | XCB_CONFIG_WINDOW_Y, &(client->x));
 			}
+
 			/* TODO: vertical default layout */
-			values[0] = container->width; /* width */
-			values[1] = container->height / num_clients; /* height */
-
-			if (client->width != values[0] || client->height != values[1]) {
-				client->width = values[0];
-				client->height = values[1];
+			if ((client->width != (client->width = container->width)) |
+                            (client->height != (client->height = container->height / num_clients))) {
 				xcb_configure_window(connection, client->frame,
-						XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT, values);
+						XCB_CONFIG_WINDOW_WIDTH | XCB_CONFIG_WINDOW_HEIGHT,
+                                                &(client->width));
+
+				/* Adjust the position of the child inside its frame.
+				 * The coordinates of the child are relative to its frame, we
+				 * add a border of 2 pixel to each value */
+                                uint32_t mask = XCB_CONFIG_WINDOW_X |
+                                                XCB_CONFIG_WINDOW_Y |
+                                                XCB_CONFIG_WINDOW_WIDTH |
+                                                XCB_CONFIG_WINDOW_HEIGHT;
+                                uint32_t values[4] = {2,                                              /* x */
+                                                      font->height + 2 + 2,                           /* y */
+                                                      client->width - (2 + 2),                        /* width */
+                                                      client->height - ((font->height + 2 + 2) + 2)}; /* height */
+
+				printf("child itself will be at %dx%d with size %dx%d\n",
+						values[0], values[1], values[2], values[3]);
+
+				xcb_configure_window(connection, client->child, mask, values);
 			}
 
-			/* TODO: hmm, only do this for new wins */
-			/* The coordinates of the child are relative to its frame, we
-			 * add a border of 2 pixel to each value */
-			values[0] = 2;
-			values[1] = font->height + 2 + 2;
-			values[2] = client->width - (values[0] + 2);
-			values[3] = client->height - (values[1] + 2);
-			printf("child itself will be at %dx%d with size %dx%d\n",
-					values[0], values[1], values[2], values[3]);
-
-			xcb_configure_window(connection, client->child, mask, values);
-
 			decorate_window(connection, client);
 			current_client++;
 		}
diff --git a/src/mainx.c b/src/mainx.c
index 2d2188e3..03be0a3d 100644
--- a/src/mainx.c
+++ b/src/mainx.c
@@ -37,11 +37,6 @@ Display *xkbdpy;
 TAILQ_HEAD(bindings_head, Binding) bindings;
 xcb_event_handlers_t evenths;
 
-static const int TOP = 20;
-static const int LEFT = 5;
-static const int BOTTOM = 5;
-static const int RIGHT = 5;
-
 /* hm, xcb_wm wants us to implement this. */
 table_t *byChild = 0;
 table_t *byParent = 0;
@@ -106,7 +101,10 @@ void manage_window(xcb_property_handlers_t *prophs, xcb_connection_t *c, xcb_win
 }
 
 /*
- * Let’s own this window…
+ * reparent_window() gets called when a new window was opened and becomes a child of the root
+ * window, or it gets called by us when we manage the already existing windows at startup.
+ *
+ * Essentially, this is the point, where we take over control.
  *
  */
 void reparent_window(xcb_connection_t *conn, xcb_window_t child,
@@ -115,8 +113,11 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 
 	Client *new = table_get(byChild, child);
 	if (new == NULL) {
+		/* TODO: When does this happen for existing clients? Is that a bug? */
 		printf("oh, it's new\n");
 		new = calloc(sizeof(Client), 1);
+		/* We initialize x and y with the invalid coordinates -1 so that they will
+		   get updated at the next render_layout() at any case */
 		new->x = -1;
 		new->y = -1;
 	}
@@ -126,7 +127,7 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 	/* Insert into the currently active container */
 	CIRCLEQ_INSERT_TAIL(&(CUR_CELL->clients), new, clients);
 
-	printf("currently_focused = %p\n", new);
+	/* Update the data structures */
 	CUR_CELL->currently_focused = new;
 	new->container = CUR_CELL;
 
@@ -141,13 +142,15 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 
 	/* We want to know when… */
 	mask |= XCB_CW_EVENT_MASK;
-	values[1] = 	XCB_EVENT_MASK_BUTTON_PRESS | /* …mouse is pressed/released */
+	values[1] = 	XCB_EVENT_MASK_BUTTON_PRESS | 	/* …mouse is pressed/released */
 			XCB_EVENT_MASK_BUTTON_RELEASE |
-			XCB_EVENT_MASK_EXPOSURE | /* …our window needs to be redrawn */
-			XCB_EVENT_MASK_ENTER_WINDOW /* …user moves cursor inside our window */;
+			XCB_EVENT_MASK_EXPOSURE | 	/* …our window needs to be redrawn */
+			XCB_EVENT_MASK_ENTER_WINDOW; 	/* …user moves cursor inside our window */
 
 	printf("Reparenting 0x%08x under 0x%08x.\n", child, new->frame);
 
+	i3Font *font = load_font(conn, pattern);
+
 	/* Yo dawg, I heard you like windows, so I create a window around your window… */
 	xcb_create_window(conn,
 			depth,
@@ -155,9 +158,9 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 			root,
 			x,
 			y,
-			width + LEFT + RIGHT,
-			height + TOP + BOTTOM,
-			/* border_width */ 0,
+			width + 2 + 2, 			/* 2 px border at each side */
+			height + 2 + 2 + font->height, 	/* 2 px border plus font’s height */
+			0, 				/* border_width = 0, we draw our own borders */
 			XCB_WINDOW_CLASS_INPUT_OUTPUT,
 			visual,
 			mask,
@@ -171,15 +174,11 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 	new->titlegc = xcb_generate_id(conn);
 	xcb_create_gc(conn, new->titlegc, new->frame, 0, 0);
 
-	/* Draw decorations */
-	decorate_window(conn, new);
-
 	/* Put our data structure (Client) into the table */
 	table_put(byParent, new->frame, new);
 	table_put(byChild, child, new);
 
 	/* Moves the original window into the new frame we've created for it */
-	i3Font *font = load_font(conn, pattern);
 	xcb_reparent_window(conn, child, new->frame, 0, font->height);
 
 	/* We are interested in property changes */
@@ -192,7 +191,8 @@ void reparent_window(xcb_connection_t *conn, xcb_window_t child,
 
 	/* We need to grab the mouse buttons for click to focus */
 	xcb_grab_button(conn, false, child, XCB_EVENT_MASK_BUTTON_PRESS,
-			XCB_GRAB_MODE_ASYNC, XCB_GRAB_MODE_ASYNC, root, XCB_NONE, 1 /* left mouse button */,
+			XCB_GRAB_MODE_ASYNC, XCB_GRAB_MODE_ASYNC, root, XCB_NONE,
+			1 /* left mouse button */,
 			XCB_BUTTON_MASK_ANY /* don’t filter for any modifiers */);
 
 	/* Focus the new window */