From 4a37d2060248a148293143f46aef0febcf3efa9b Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 3 May 2019 15:38:37 +0300 Subject: [PATCH 1/4] ewmh: Introduce FOREACH_NONINTERNAL macro --- src/ewmh.c | 102 ++++++++++++++++++----------------------------------- 1 file changed, 34 insertions(+), 68 deletions(-) diff --git a/src/ewmh.c b/src/ewmh.c index e5dcafcb..bd8100b3 100644 --- a/src/ewmh.c +++ b/src/ewmh.c @@ -11,6 +11,11 @@ xcb_window_t ewmh_window; +#define FOREACH_NONINTERNAL \ + TAILQ_FOREACH(output, &(croot->nodes_head), nodes) \ + TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) \ + if (!con_is_internal(ws)) + /* * Updates _NET_CURRENT_DESKTOP with the current desktop number. * @@ -30,17 +35,12 @@ void ewmh_update_current_desktop(void) { * noninternal workspaces. */ void ewmh_update_number_of_desktops(void) { - Con *output; + Con *output, *ws; uint32_t idx = 0; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *ws; - TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) { - if (STARTS_WITH(ws->name, "__")) - continue; - ++idx; - } - } + FOREACH_NONINTERNAL { + idx++; + }; xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_NUMBER_OF_DESKTOPS, XCB_ATOM_CARDINAL, 32, 1, &idx); @@ -51,32 +51,21 @@ void ewmh_update_number_of_desktops(void) { * list of NULL-terminated strings in UTF-8 encoding" */ void ewmh_update_desktop_names(void) { - Con *output; + Con *output, *ws; int msg_length = 0; /* count the size of the property message to set */ - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *ws; - TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) { - if (STARTS_WITH(ws->name, "__")) - continue; - msg_length += strlen(ws->name) + 1; - } - } + FOREACH_NONINTERNAL { + msg_length += strlen(ws->name) + 1; + }; char desktop_names[msg_length]; int current_position = 0; /* fill the buffer with the names of the i3 workspaces */ - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *ws; - TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) { - if (STARTS_WITH(ws->name, "__")) - continue; - - for (size_t i = 0; i < strlen(ws->name) + 1; i++) { - desktop_names[current_position++] = ws->name[i]; - } + FOREACH_NONINTERNAL { + for (size_t i = 0; i < strlen(ws->name) + 1; i++) { + desktop_names[current_position++] = ws->name[i]; } } @@ -89,32 +78,20 @@ void ewmh_update_desktop_names(void) { * define the top left corner of each desktop's viewport. */ void ewmh_update_desktop_viewport(void) { - Con *output; + Con *output, *ws; int num_desktops = 0; /* count number of desktops */ - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *ws; - TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) { - if (STARTS_WITH(ws->name, "__")) - continue; - - num_desktops++; - } + FOREACH_NONINTERNAL { + num_desktops++; } uint32_t viewports[num_desktops * 2]; int current_position = 0; /* fill the viewport buffer */ - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *ws; - TAILQ_FOREACH(ws, &(output_get_content(output)->nodes_head), nodes) { - if (STARTS_WITH(ws->name, "__")) - continue; - - viewports[current_position++] = output->rect.x; - viewports[current_position++] = output->rect.y; - } + FOREACH_NONINTERNAL { + viewports[current_position++] = output->rect.x; + viewports[current_position++] = output->rect.y; } xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, @@ -354,18 +331,12 @@ Con *ewmh_get_workspace_by_index(uint32_t idx) { uint32_t current_index = 0; - Con *output; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *workspace; - TAILQ_FOREACH(workspace, &(output_get_content(output)->nodes_head), nodes) { - if (con_is_internal(workspace)) - continue; - - if (current_index == idx) - return workspace; - - ++current_index; + Con *output, *ws; + FOREACH_NONINTERNAL { + if (current_index == idx) { + return ws; } + current_index++; } return NULL; @@ -381,19 +352,14 @@ Con *ewmh_get_workspace_by_index(uint32_t idx) { uint32_t ewmh_get_workspace_index(Con *con) { uint32_t index = 0; - Con *workspace = con_get_workspace(con); - Con *output; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - Con *current; - TAILQ_FOREACH(current, &(output_get_content(output)->nodes_head), nodes) { - if (con_is_internal(current)) - continue; - - if (current == workspace) - return index; - - ++index; + Con *target_workspace = con_get_workspace(con); + Con *output, *ws; + FOREACH_NONINTERNAL { + if (ws == target_workspace) { + return index; } + + index++; } return NET_WM_DESKTOP_NONE; From 8c25bc1bd453535a21fb02e4cf5236fe4e3a7062 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 3 May 2019 15:39:04 +0300 Subject: [PATCH 2/4] tree_close_internal: Log workspace name in EWMH message --- src/tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tree.c b/src/tree.c index 5023e894..f93b2262 100644 --- a/src/tree.c +++ b/src/tree.c @@ -300,7 +300,7 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par x_con_kill(con); if (ws == con) { - DLOG("Closing a workspace container, updating EWMH atoms\n"); + DLOG("Closing workspace container %s, updating EWMH atoms\n", ws->name); ewmh_update_number_of_desktops(); ewmh_update_desktop_names(); ewmh_update_wm_desktop(); From 830465b39f60bd22bbb5053a1e664d30e03351ad Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 3 May 2019 16:04:32 +0300 Subject: [PATCH 3/4] ewmh: Cache idx to avoid xcb_change_property calls Updates ewmh_update_current_desktop, ewmh_update_number_of_desktops --- src/ewmh.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ewmh.c b/src/ewmh.c index bd8100b3..a26eef39 100644 --- a/src/ewmh.c +++ b/src/ewmh.c @@ -24,10 +24,15 @@ xcb_window_t ewmh_window; * */ void ewmh_update_current_desktop(void) { + static uint32_t old_idx = NET_WM_DESKTOP_NONE; const uint32_t idx = ewmh_get_workspace_index(focused); - if (idx != NET_WM_DESKTOP_NONE) { - xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_CURRENT_DESKTOP, XCB_ATOM_CARDINAL, 32, 1, &idx); + + if (idx == old_idx || idx == NET_WM_DESKTOP_NONE) { + return; } + old_idx = idx; + + xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_CURRENT_DESKTOP, XCB_ATOM_CARDINAL, 32, 1, &idx); } /* @@ -36,12 +41,18 @@ void ewmh_update_current_desktop(void) { */ void ewmh_update_number_of_desktops(void) { Con *output, *ws; + static uint32_t old_idx = 0; uint32_t idx = 0; FOREACH_NONINTERNAL { idx++; }; + if (idx == old_idx) { + return; + } + old_idx = idx; + xcb_change_property(conn, XCB_PROP_MODE_REPLACE, root, A__NET_NUMBER_OF_DESKTOPS, XCB_ATOM_CARDINAL, 32, 1, &idx); } From c9efa6dffe1dfda263d5ea25079168945ec52510 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Fri, 3 May 2019 15:46:38 +0300 Subject: [PATCH 4/4] Call all ewmh_update_* functions together when necessary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The testcase is changed because it was actually incorrect. Easy to verify because: > _NET_CURRENT_DESKTOP > … > The index of the current desktop. This is always an integer between 0 > and _NET_NUMBER_OF_DESKTOPS - 1. Fixes #3696. Also updates the viewports. Finally, fixes an issue with _NET_CURRENT_DESKTOP not being updated after a workspace rename. Example: - workspaces 1, 2, 3 - rename workspace 1 to 5 - All workspaces changed their index but _NET_CURRENT_DESKTOP was not updated --- include/ewmh.h | 24 ++++++------------------ src/commands.c | 4 +--- src/ewmh.c | 18 +++++++++++++++--- src/main.c | 5 +---- src/tree.c | 4 +--- src/workspace.c | 10 ++-------- testcases/t/294-update-ewmh-atoms.t | 10 +++++++++- 7 files changed, 35 insertions(+), 40 deletions(-) diff --git a/include/ewmh.h b/include/ewmh.h index 01ae67f9..f616eb84 100644 --- a/include/ewmh.h +++ b/include/ewmh.h @@ -11,6 +11,12 @@ #include +/** + * Updates all the EWMH desktop properties. + * + */ +void ewmh_update_desktop_properties(void); + /** * Updates _NET_CURRENT_DESKTOP with the current desktop number. * @@ -20,24 +26,6 @@ */ void ewmh_update_current_desktop(void); -/** - * Updates _NET_NUMBER_OF_DESKTOPS which we interpret as the number of - * noninternal workspaces. - */ -void ewmh_update_number_of_desktops(void); - -/** - * Updates _NET_DESKTOP_NAMES: "The names of all virtual desktops. This is a - * list of NULL-terminated strings in UTF-8 encoding" - */ -void ewmh_update_desktop_names(void); - -/** - * Updates _NET_DESKTOP_VIEWPORT, which is an array of pairs of cardinals that - * define the top left corner of each desktop's viewport. - */ -void ewmh_update_desktop_viewport(void); - /** * Updates _NET_WM_DESKTOP for all windows. * A request will only be made if the cached value differs from the calculated value. diff --git a/src/commands.c b/src/commands.c index 624c27db..db0b075d 100644 --- a/src/commands.c +++ b/src/commands.c @@ -2002,9 +2002,7 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { cmd_output->needs_tree_render = true; ysuccess(true); - ewmh_update_desktop_names(); - ewmh_update_desktop_viewport(); - ewmh_update_current_desktop(); + ewmh_update_desktop_properties(); startup_sequence_rename_workspace(old_name_copy, new_name); free(old_name_copy); diff --git a/src/ewmh.c b/src/ewmh.c index a26eef39..7bd23fb7 100644 --- a/src/ewmh.c +++ b/src/ewmh.c @@ -39,7 +39,7 @@ void ewmh_update_current_desktop(void) { * Updates _NET_NUMBER_OF_DESKTOPS which we interpret as the number of * noninternal workspaces. */ -void ewmh_update_number_of_desktops(void) { +static void ewmh_update_number_of_desktops(void) { Con *output, *ws; static uint32_t old_idx = 0; uint32_t idx = 0; @@ -61,7 +61,7 @@ void ewmh_update_number_of_desktops(void) { * Updates _NET_DESKTOP_NAMES: "The names of all virtual desktops. This is a * list of NULL-terminated strings in UTF-8 encoding" */ -void ewmh_update_desktop_names(void) { +static void ewmh_update_desktop_names(void) { Con *output, *ws; int msg_length = 0; @@ -88,7 +88,7 @@ void ewmh_update_desktop_names(void) { * Updates _NET_DESKTOP_VIEWPORT, which is an array of pairs of cardinals that * define the top left corner of each desktop's viewport. */ -void ewmh_update_desktop_viewport(void) { +static void ewmh_update_desktop_viewport(void) { Con *output, *ws; int num_desktops = 0; /* count number of desktops */ @@ -109,6 +109,18 @@ void ewmh_update_desktop_viewport(void) { A__NET_DESKTOP_VIEWPORT, XCB_ATOM_CARDINAL, 32, current_position, &viewports); } +/* + * Updates all the EWMH desktop properties. + * + */ +void ewmh_update_desktop_properties(void) { + ewmh_update_number_of_desktops(); + ewmh_update_desktop_viewport(); + ewmh_update_current_desktop(); + ewmh_update_desktop_names(); + ewmh_update_wm_desktop(); +} + static void ewmh_update_wm_desktop_recursively(Con *con, const uint32_t desktop) { Con *child; diff --git a/src/main.c b/src/main.c index 3ebdbf0c..9f69834f 100644 --- a/src/main.c +++ b/src/main.c @@ -852,10 +852,7 @@ int main(int argc, char *argv[]) { ewmh_update_workarea(); /* Set the ewmh desktop properties. */ - ewmh_update_current_desktop(); - ewmh_update_number_of_desktops(); - ewmh_update_desktop_names(); - ewmh_update_desktop_viewport(); + ewmh_update_desktop_properties(); struct ev_io *xcb_watcher = scalloc(1, sizeof(struct ev_io)); xcb_prepare = scalloc(1, sizeof(struct ev_prepare)); diff --git a/src/tree.c b/src/tree.c index f93b2262..4057d177 100644 --- a/src/tree.c +++ b/src/tree.c @@ -301,9 +301,7 @@ bool tree_close_internal(Con *con, kill_window_t kill_window, bool dont_kill_par if (ws == con) { DLOG("Closing workspace container %s, updating EWMH atoms\n", ws->name); - ewmh_update_number_of_desktops(); - ewmh_update_desktop_names(); - ewmh_update_wm_desktop(); + ewmh_update_desktop_properties(); } con_free(con); diff --git a/src/workspace.c b/src/workspace.c index 3cf74754..59705798 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -161,10 +161,7 @@ Con *workspace_get(const char *num, bool *created) { con_attach(workspace, content, false); ipc_send_workspace_event("init", workspace, NULL); - ewmh_update_number_of_desktops(); - ewmh_update_desktop_names(); - ewmh_update_desktop_viewport(); - ewmh_update_wm_desktop(); + ewmh_update_desktop_properties(); if (created != NULL) *created = true; } else if (created != NULL) { @@ -520,10 +517,7 @@ void workspace_show(Con *workspace) { old_focus = NULL; } - ewmh_update_number_of_desktops(); - ewmh_update_desktop_names(); - ewmh_update_desktop_viewport(); - ewmh_update_wm_desktop(); + ewmh_update_desktop_properties(); } } diff --git a/testcases/t/294-update-ewmh-atoms.t b/testcases/t/294-update-ewmh-atoms.t index 047cc119..f13b1764 100644 --- a/testcases/t/294-update-ewmh-atoms.t +++ b/testcases/t/294-update-ewmh-atoms.t @@ -101,11 +101,19 @@ is_deeply(\@actual_names, \@expected_names); # Kill first window to close a workspace. cmd '[id="' . $second->id . '"] kill'; -is(get_current_desktop, 2, '_NET_CURRENT_DESKTOP should be updated'); +is(get_current_desktop, 1, '_NET_CURRENT_DESKTOP should be updated'); is(get_num_of_desktops, 2, '_NET_NUMBER_OF_DESKTOPS should be updated'); my @actual_names = get_desktop_names; my @expected_names = ('0', '2'); is_deeply(\@actual_names, \@expected_names, '_NET_DESKTOP_NAMES should be updated'); +# Rename workspace to reorder them. +cmd 'rename workspace 0 to 5'; + +is(get_current_desktop, 0, '_NET_CURRENT_DESKTOP should be updated'); +is(get_num_of_desktops, 2, '_NET_NUMBER_OF_DESKTOPS should remain the same'); +my @actual_names = get_desktop_names; +my @expected_names = ('2', '5'); +is_deeply(\@actual_names, \@expected_names, '_NET_DESKTOP_NAMES should be updated'); done_testing;