i3 - improved tiling WM

Reviews
Bugs

Fix keyboard and mouse resize in nested containers

Patch status: merged

Patch by jj

Long description:

fixes #1084
fixes #1085

To apply this patch, use:
curl http://cr.i3wm.org/patch/239/raw.patch | git am

b/include/resize.h

@@ -10,6 +10,8 @@
 #ifndef I3_RESIZE_H
 #define I3_RESIZE_H
 
+bool resize_find_tiling_participants(Con **current, Con **other, direction_t direction);
+
 int resize_graphical_handler(Con *first, Con *second, orientation_t orientation, const xcb_button_press_event_t *event);
 
 #endif

b/src/click.c

@@ -28,45 +28,43 @@ typedef enum { CLICK_BORDER = 0, CLICK_DECORATION = 1, CLICK_INSIDE = 2 } click_
  */
 static bool tiling_resize_for_border(Con *con, border_t border, xcb_button_press_event_t *event) {
     DLOG("border = %d, con = %p\n", border, con);
-    char way = (border == BORDER_TOP || border == BORDER_LEFT ? 'p' : 'n');
-    orientation_t orientation = (border == BORDER_TOP || border == BORDER_BOTTOM ? VERT : HORIZ);
-
-    /* look for a parent container with the right orientation */
-    Con *first = NULL, *second = NULL;
-    Con *resize_con = con;
-    while (resize_con->type != CT_WORKSPACE &&
-           resize_con->type != CT_FLOATING_CON &&
-           con_orientation(resize_con->parent) != orientation)
-        resize_con = resize_con->parent;
-
-    DLOG("resize_con = %p\n", resize_con);
-    if (resize_con->type != CT_WORKSPACE &&
-        resize_con->type != CT_FLOATING_CON &&
-        con_orientation(resize_con->parent) == orientation) {
-        first = resize_con;
-        second = (way == 'n') ? TAILQ_NEXT(first, nodes) : TAILQ_PREV(first, nodes_head, nodes);
-        if (second == TAILQ_END(&(first->nodes_head))) {
-            second = NULL;
-        }
-        else if (way == 'p') {
-            Con *tmp = first;
-            first = second;
-            second = tmp;
-        }
-        DLOG("first = %p, second = %p, resize_con = %p\n",
-                first, second, resize_con);
+    Con *second = NULL;
+    Con *first = con;
+    direction_t search_direction;
+    switch (border) {
+        case BORDER_LEFT:
+            search_direction = D_LEFT;
+            break;
+        case BORDER_RIGHT:
+            search_direction = D_RIGHT;
+            break;
+        case BORDER_TOP:
+            search_direction = D_UP;
+            break;
+        case BORDER_BOTTOM:
+            search_direction = D_DOWN;
+            break;
     }
 
-    if (first == NULL || second == NULL) {
-        DLOG("Resize not possible\n");
+    bool res = resize_find_tiling_participants(&first, &second, search_direction);
+    if (!res) {
+        LOG("No second container in this direction found.\n");
         return false;
     }
 
     assert(first != second);
     assert(first->parent == second->parent);
 
+    /* The first container should always be in front of the second container */
+    if (search_direction == D_UP || search_direction == D_LEFT) {
+        Con *tmp = first;
+        first = second;
+        second = tmp;
+    }
+
     /* We modify the X/Y position in the event so that the divider line is at
      * the actual position of the border, not at the position of the click. */
+    const orientation_t orientation = ((border == BORDER_LEFT || border == BORDER_RIGHT) ? HORIZ : VERT);
     if (orientation == HORIZ)
         event->root_x = second->rect.x;
     else event->root_y = second->rect.y;

b/src/commands.c

@@ -618,79 +618,50 @@ static void cmd_resize_floating(I3_CMD, char *way, char *direction, Con *floatin
 
 static bool cmd_resize_tiling_direction(I3_CMD, Con *current, char *way, char *direction, int ppt) {
     LOG("tiling resize\n");
-    /* get the appropriate current container (skip stacked/tabbed cons) */
-    Con *other = NULL;
-    double percentage = 0;
-    while (current->parent->layout == L_STACKED ||
-           current->parent->layout == L_TABBED)
-        current = current->parent;
-
-    /* Then further go up until we find one with the matching orientation. */
-    orientation_t search_orientation =
-        (strcmp(direction, "left") == 0 || strcmp(direction, "right") == 0 ? HORIZ : VERT);
-
-    do {
-        if (con_orientation(current->parent) != search_orientation) {
-            current = current->parent;
-            continue;
-        }
-
-        /* get the default percentage */
-        int children = con_num_children(current->parent);
-        LOG("ins. %d children\n", children);
-        percentage = 1.0 / children;
-        LOG("default percentage = %f\n", percentage);
-
-        orientation_t orientation = con_orientation(current->parent);
-
-        if ((orientation == HORIZ &&
-             (strcmp(direction, "up") == 0 || strcmp(direction, "down") == 0)) ||
-            (orientation == VERT &&
-             (strcmp(direction, "left") == 0 || strcmp(direction, "right") == 0))) {
-            LOG("You cannot resize in that direction. Your focus is in a %s split container currently.\n",
-                (orientation == HORIZ ? "horizontal" : "vertical"));
-            ysuccess(false);
-            return false;
-        }
-
-        if (strcmp(direction, "up") == 0 || strcmp(direction, "left") == 0) {
-            other = TAILQ_PREV(current, nodes_head, nodes);
-        } else {
-            other = TAILQ_NEXT(current, nodes);
-        }
-        if (other == TAILQ_END(workspaces)) {
-            LOG("No other container in this direction found, trying to look further up in the tree...\n");
-            current = current->parent;
-            continue;
-        }
-        break;
-    } while (current->type != CT_WORKSPACE &&
-             current->type != CT_FLOATING_CON);
+    Con *second = NULL;
+    Con *first = current;
+    direction_t search_direction;
+    if (!strcmp(direction, "left"))
+        search_direction = D_LEFT;
+    else if (!strcmp(direction, "right"))
+        search_direction = D_RIGHT;
+    else if (!strcmp(direction, "up"))
+        search_direction = D_UP;
+    else
+        search_direction = D_DOWN;
 
-    if (other == NULL) {
-        LOG("No other container in this direction found, trying to look further up in the tree...\n");
+    bool res = resize_find_tiling_participants(&first, &second, search_direction);
+    if (!res) {
+        LOG("No second container in this direction found.\n");
         ysuccess(false);
         return false;
     }
 
-    LOG("other->percent = %f\n", other->percent);
-    LOG("current->percent before = %f\n", current->percent);
-    if (current->percent == 0.0)
-        current->percent = percentage;
-    if (other->percent == 0.0)
-        other->percent = percentage;
-    double new_current_percent = current->percent + ((double)ppt / 100.0);
-    double new_other_percent = other->percent - ((double)ppt / 100.0);
-    LOG("new_current_percent = %f\n", new_current_percent);
-    LOG("new_other_percent = %f\n", new_other_percent);
+    /* get the default percentage */
+    int children = con_num_children(first->parent);
+    LOG("ins. %d children\n", children);
+    double percentage = 1.0 / children;
+    LOG("default percentage = %f\n", percentage);
+
+    /* resize */
+    LOG("second->percent = %f\n", second->percent);
+    LOG("first->percent before = %f\n", first->percent);
+    if (first->percent == 0.0)
+        first->percent = percentage;
+    if (second->percent == 0.0)
+        second->percent = percentage;
+    double new_first_percent = first->percent + ((double)ppt / 100.0);
+    double new_second_percent = second->percent - ((double)ppt / 100.0);
+    LOG("new_first_percent = %f\n", new_first_percent);
+    LOG("new_second_percent = %f\n", new_second_percent);
     /* Ensure that the new percentages are positive and greater than
      * 0.05 to have a reasonable minimum size. */
-    if (definitelyGreaterThan(new_current_percent, 0.05, DBL_EPSILON) &&
-        definitelyGreaterThan(new_other_percent, 0.05, DBL_EPSILON)) {
-        current->percent += ((double)ppt / 100.0);
-        other->percent -= ((double)ppt / 100.0);
-        LOG("current->percent after = %f\n", current->percent);
-        LOG("other->percent after = %f\n", other->percent);
+    if (definitelyGreaterThan(new_first_percent, 0.05, DBL_EPSILON) &&
+        definitelyGreaterThan(new_second_percent, 0.05, DBL_EPSILON)) {
+        first->percent += ((double)ppt / 100.0);
+        second->percent -= ((double)ppt / 100.0);
+        LOG("first->percent after = %f\n", first->percent);
+        LOG("second->percent after = %f\n", second->percent);
     } else {
         LOG("Not resizing, already at minimum size\n");
     }

b/src/resize.c

@@ -51,6 +51,54 @@ DRAGGING_CB(resize_callback) {
     xcb_flush(conn);
 }
 
+bool resize_find_tiling_participants(Con **current, Con **other, direction_t direction) {
+    DLOG("Find two participants for resizing container=%p in direction=%i\n", other, direction);
+    Con *first = *current;
+    Con *second = NULL;
+    if (first == NULL) {
+        DLOG("Current container is NULL, aborting.\n");
+        return false;
+    }
+
+    /* Go up in the tree and search for a container to resize */
+    const orientation_t search_orientation = ((direction == D_LEFT || direction == D_RIGHT) ? HORIZ : VERT);
+    const bool dir_backwards = (direction == D_UP || direction == D_LEFT);
+    while (first->type != CT_WORKSPACE &&
+           first->type != CT_FLOATING_CON &&
+           second == NULL) {
+        /* get the appropriate first container with the matching
+         * orientation (skip stacked/tabbed cons) */
+        if ((con_orientation(first->parent) != search_orientation) ||
+            (first->parent->layout == L_STACKED) ||
+            (first->parent->layout == L_TABBED)) {
+            first = first->parent;
+            continue;
+        }
+
+        /* get the counterpart for this resizement */
+        if (dir_backwards) {
+            second = TAILQ_PREV(first, nodes_head, nodes);
+        } else {
+            second = TAILQ_NEXT(first, nodes);
+        }
+
+        if (second == NULL) {
+            DLOG("No second container in this direction found, trying to look further up in the tree...\n");
+            first = first->parent;
+        }
+    }
+
+    DLOG("Found participants: first=%p and second=%p.", first, second);
+    *current = first;
+    *other = second;
+    if (first == NULL || second == NULL) {
+        DLOG("Could not find two participants for this resize request.\n");
+        return false;
+    }
+
+    return true;
+}
+
 int resize_graphical_handler(Con *first, Con *second, orientation_t orientation, const xcb_button_press_event_t *event) {
     DLOG("resize handler\n");
 

108	@@ -618,79 +618,50 @@ static void cmd_resize_floating(I3_CMD, char way, char direction, Con *floatin
109
110	static bool cmd_resize_tiling_direction(I3_CMD, Con current, char way, char *direction, int ppt) {
111	LOG("tiling resize\n");
112	- /* get the appropriate current container (skip stacked/tabbed cons) */
113	- Con *other = NULL;
114	- double percentage = 0;
115	- while (current->parent->layout == L_STACKED \|\|
116	- current->parent->layout == L_TABBED)
117	- current = current->parent;
118	-
119	- /* Then further go up until we find one with the matching orientation. */
120	- orientation_t search_orientation =
121	- (strcmp(direction, "left") == 0 \|\| strcmp(direction, "right") == 0 ? HORIZ : VERT);
122	-
123	- do {
124	- if (con_orientation(current->parent) != search_orientation) {
125	- current = current->parent;
126	- continue;
127	- }
128	-
129	- /* get the default percentage */
130	- int children = con_num_children(current->parent);
131	- LOG("ins. %d children\n", children);
132	- percentage = 1.0 / children;
133	- LOG("default percentage = %f\n", percentage);
134	-
135	- orientation_t orientation = con_orientation(current->parent);
136	-
137	- if ((orientation == HORIZ &&
138	- (strcmp(direction, "up") == 0 \|\| strcmp(direction, "down") == 0)) \|\|
139	- (orientation == VERT &&
140	- (strcmp(direction, "left") == 0 \|\| strcmp(direction, "right") == 0))) {
141	- LOG("You cannot resize in that direction. Your focus is in a %s split container currently.\n",
142	- (orientation == HORIZ ? "horizontal" : "vertical"));
143	- ysuccess(false);
144	- return false;
145	- }
146	-
147	- if (strcmp(direction, "up") == 0 \|\| strcmp(direction, "left") == 0) {
148	- other = TAILQ_PREV(current, nodes_head, nodes);
149	- } else {
150	- other = TAILQ_NEXT(current, nodes);
151	- }
152	- if (other == TAILQ_END(workspaces)) {
153	- LOG("No other container in this direction found, trying to look further up in the tree...\n");
154	- current = current->parent;
155	- continue;
156	- }
157	- break;
158	- } while (current->type != CT_WORKSPACE &&
159	- current->type != CT_FLOATING_CON);
160	+ Con *second = NULL;
161	+ Con *first = current;
162	+ direction_t search_direction;
163	+ if (!strcmp(direction, "left"))
164	+ search_direction = D_LEFT;
165	+ else if (!strcmp(direction, "right"))
166	+ search_direction = D_RIGHT;
167	+ else if (!strcmp(direction, "up"))
168	+ search_direction = D_UP;
169	+ else
170	+ search_direction = D_DOWN;
171
172	- if (other == NULL) {
173	- LOG("No other container in this direction found, trying to look further up in the tree...\n");
174	+ bool res = resize_find_tiling_participants(&first, &second, search_direction);
175	+ if (!res) {
176	+ LOG("No second container in this direction found.\n");
177	ysuccess(false);
178	return false;
179	}
180
181	- LOG("other->percent = %f\n", other->percent);
182	- LOG("current->percent before = %f\n", current->percent);
183	- if (current->percent == 0.0)
184	- current->percent = percentage;
185	- if (other->percent == 0.0)
186	- other->percent = percentage;
187	- double new_current_percent = current->percent + ((double)ppt / 100.0);
188	- double new_other_percent = other->percent - ((double)ppt / 100.0);
189	- LOG("new_current_percent = %f\n", new_current_percent);
190	- LOG("new_other_percent = %f\n", new_other_percent);
191	+ /* get the default percentage */
192	+ int children = con_num_children(first->parent);
193	+ LOG("ins. %d children\n", children);
194	+ double percentage = 1.0 / children;
195	+ LOG("default percentage = %f\n", percentage);
196	+
197	+ /* resize */
198	+ LOG("second->percent = %f\n", second->percent);
199	+ LOG("first->percent before = %f\n", first->percent);
200	+ if (first->percent == 0.0)
201	+ first->percent = percentage;
202	+ if (second->percent == 0.0)
203	+ second->percent = percentage;
204	+ double new_first_percent = first->percent + ((double)ppt / 100.0);
205	+ double new_second_percent = second->percent - ((double)ppt / 100.0);
206	+ LOG("new_first_percent = %f\n", new_first_percent);
207	+ LOG("new_second_percent = %f\n", new_second_percent);
208	/* Ensure that the new percentages are positive and greater than
209	* 0.05 to have a reasonable minimum size. */
210	- if (definitelyGreaterThan(new_current_percent, 0.05, DBL_EPSILON) &&
211	- definitelyGreaterThan(new_other_percent, 0.05, DBL_EPSILON)) {
212	- current->percent += ((double)ppt / 100.0);
213	- other->percent -= ((double)ppt / 100.0);
214	- LOG("current->percent after = %f\n", current->percent);
215	- LOG("other->percent after = %f\n", other->percent);
216	+ if (definitelyGreaterThan(new_first_percent, 0.05, DBL_EPSILON) &&
217	+ definitelyGreaterThan(new_second_percent, 0.05, DBL_EPSILON)) {
218	+ first->percent += ((double)ppt / 100.0);
219	+ second->percent -= ((double)ppt / 100.0);
220	+ LOG("first->percent after = %f\n", first->percent);
221	+ LOG("second->percent after = %f\n", second->percent);
222	} else {
223	LOG("Not resizing, already at minimum size\n");
224	}

229	@@ -51,6 +51,54 @@ DRAGGING_CB(resize_callback) {
230	xcb_flush(conn);
231	}
232
233	+bool resize_find_tiling_participants(Con current, Con other, direction_t direction) {
234	+ DLOG("Find two participants for resizing container=%p in direction=%i\n", other, direction);
235	+ Con first = current;
236	+ Con *second = NULL;
237	+ if (first == NULL) {
238	+ DLOG("Current container is NULL, aborting.\n");
239	+ return false;
240	+ }
241	+
242	+ /* Go up in the tree and search for a container to resize */
243	+ const orientation_t search_orientation = ((direction == D_LEFT \|\| direction == D_RIGHT) ? HORIZ : VERT);
244	+ const bool dir_backwards = (direction == D_UP \|\| direction == D_LEFT);
245	+ while (first->type != CT_WORKSPACE &&
246	+ first->type != CT_FLOATING_CON &&
247	+ second == NULL) {
248	+ /* get the appropriate first container with the matching
249	+ * orientation (skip stacked/tabbed cons) */
250	+ if ((con_orientation(first->parent) != search_orientation) \|\|
251	+ (first->parent->layout == L_STACKED) \|\|
252	+ (first->parent->layout == L_TABBED)) {
253	+ first = first->parent;
254	+ continue;
255	+ }
256	+
257	+ /* get the counterpart for this resizement */
258	+ if (dir_backwards) {
259	+ second = TAILQ_PREV(first, nodes_head, nodes);
260	+ } else {
261	+ second = TAILQ_NEXT(first, nodes);
262	+ }
263	+
264	+ if (second == NULL) {
265	+ DLOG("No second container in this direction found, trying to look further up in the tree...\n");
266	+ first = first->parent;
267	+ }
268	+ }
269	+
270	+ DLOG("Found participants: first=%p and second=%p.", first, second);
271	+ *current = first;
272	+ *other = second;
273	+ if (first == NULL \|\| second == NULL) {
274	+ DLOG("Could not find two participants for this resize request.\n");
275	+ return false;
276	+ }
277	+
278	+ return true;
279	+}
280	+
281	int resize_graphical_handler(Con first, Con second, orientation_t orientation, const xcb_button_press_event_t *event) {
282	DLOG("resize handler\n");
283

18	@@ -10,6 +10,8 @@
19	#ifndef I3_RESIZE_H
20	#define I3_RESIZE_H
21
22	+bool resize_find_tiling_participants(Con current, Con other, direction_t direction);
23	+
24	int resize_graphical_handler(Con first, Con second, orientation_t orientation, const xcb_button_press_event_t *event);
25
26	#endif

31	@@ -28,45 +28,43 @@ typedef enum { CLICK_BORDER = 0, CLICK_DECORATION = 1, CLICK_INSIDE = 2 } click_
32	*/
33	static bool tiling_resize_for_border(Con con, border_t border, xcb_button_press_event_t event) {
34	DLOG("border = %d, con = %p\n", border, con);
35	- char way = (border == BORDER_TOP \|\| border == BORDER_LEFT ? 'p' : 'n');
36	- orientation_t orientation = (border == BORDER_TOP \|\| border == BORDER_BOTTOM ? VERT : HORIZ);
37	-
38	- /* look for a parent container with the right orientation */
39	- Con first = NULL, second = NULL;
40	- Con *resize_con = con;
41	- while (resize_con->type != CT_WORKSPACE &&
42	- resize_con->type != CT_FLOATING_CON &&
43	- con_orientation(resize_con->parent) != orientation)
44	- resize_con = resize_con->parent;
45	-
46	- DLOG("resize_con = %p\n", resize_con);
47	- if (resize_con->type != CT_WORKSPACE &&
48	- resize_con->type != CT_FLOATING_CON &&
49	- con_orientation(resize_con->parent) == orientation) {
50	- first = resize_con;
51	- second = (way == 'n') ? TAILQ_NEXT(first, nodes) : TAILQ_PREV(first, nodes_head, nodes);
52	- if (second == TAILQ_END(&(first->nodes_head))) {
53	- second = NULL;
54	- }
55	- else if (way == 'p') {
56	- Con *tmp = first;
57	- first = second;
58	- second = tmp;
59	- }
60	- DLOG("first = %p, second = %p, resize_con = %p\n",
61	- first, second, resize_con);
62	+ Con *second = NULL;
63	+ Con *first = con;
64	+ direction_t search_direction;
65	+ switch (border) {
66	+ case BORDER_LEFT:
67	+ search_direction = D_LEFT;
68	+ break;
69	+ case BORDER_RIGHT:
70	+ search_direction = D_RIGHT;
71	+ break;
72	+ case BORDER_TOP:
73	+ search_direction = D_UP;
74	+ break;
75	+ case BORDER_BOTTOM:
76	+ search_direction = D_DOWN;
77	+ break;
78	}
79
80	- if (first == NULL \|\| second == NULL) {
81	- DLOG("Resize not possible\n");
82	+ bool res = resize_find_tiling_participants(&first, &second, search_direction);
83	+ if (!res) {
84	+ LOG("No second container in this direction found.\n");
85	return false;
86	}
87
88	assert(first != second);
89	assert(first->parent == second->parent);
90
91	+ /* The first container should always be in front of the second container */
92	+ if (search_direction == D_UP \|\| search_direction == D_LEFT) {
93	+ Con *tmp = first;
94	+ first = second;
95	+ second = tmp;
96	+ }
97	+
98	/* We modify the X/Y position in the event so that the divider line is at
99	* the actual position of the border, not at the position of the click. */
100	+ const orientation_t orientation = ((border == BORDER_LEFT \|\| border == BORDER_RIGHT) ? HORIZ : VERT);
101	if (orientation == HORIZ)
102	event->root_x = second->rect.x;
103	else event->root_y = second->rect.y;