Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window going fullscreen issue #354

Closed
choco opened this issue Dec 26, 2019 · 7 comments
Closed

Window going fullscreen issue #354

choco opened this issue Dec 26, 2019 · 7 comments
Labels
bug Something isn't working

Comments

@choco
Copy link

choco commented Dec 26, 2019

I have made a video to better show what's happening.

https://www.youtube.com/watch?v=DFceUGk5MtA

@choco
Copy link
Author

choco commented Dec 26, 2019

And here's the log from yabai

EVENT_HANDLER_APPLICATION_ACTIVATED: Google Chrome
EVENT_HANDLER_WINDOW_TITLE_CHANGED: Google Chrome 23174
EVENT_HANDLER_WINDOW_TITLE_CHANGED: Google Chrome 23174
EVENT_HANDLER_WINDOW_TITLE_CHANGED: Google Chrome 23174
EVENT_HANDLER_WINDOW_TITLE_CHANGED: Google Chrome 23174
EVENT_HANDLER_MOUSE_DOWN: 963.11, 102.93
EVENT_HANDLER_MOUSE_UP: 963.11, 102.93
EVENT_HANDLER_WINDOW_CREATED: ignoring window Google Chrome 23180
EVENT_HANDLER_WINDOW_RESIZED: Google Chrome 23174
EVENT_HANDLER_SPACE_CHANGED: 305
window_manager_add_application_windows: ignoring window Google Chrome 23180
EVENT_HANDLER_WINDOW_TITLE_CHANGED: iTerm2 23119
EVENT_HANDLER_WINDOW_RESIZED: iTerm2 23119
EVENT_HANDLER_WINDOW_TITLE_CHANGED: iTerm2 23119

which shows chrome activation, me pressing the fullscreen button, the switch to the fullscreen space

@koekeishiya
Copy link
Owner

I assume the issue you are talking about is the transition into fullscreen? I'm unable to reproduce that issue on my end - I am still running macOS Mojave though.

@koekeishiya koekeishiya added bug Something isn't working help wanted Community help appreciated obscure Weird behaviour that is difficult to debug labels Dec 26, 2019
@choco
Copy link
Author

choco commented Dec 26, 2019

Yep, but the transition taking longer isn't really an issue. The real issue is that the window title bar is messed up in the process. If you see at the end when the window is fullscreen, when trying to show the titlebar the toolbar buttons aren't there anymore.

I remember a visually similar issue with chunkwm or kwm that was happening because resizing events were sent to the window during the transition, not sure if the same applies here.

@choco
Copy link
Author

choco commented Dec 26, 2019

If I disable the mouse event tap I'm unable to reproduce the issue and I can't reproduce it either if I enable fullscreen by using the keyboard shortcut instead of clicking on the button, so something wrong must be going on there.

@choco
Copy link
Author

choco commented Dec 26, 2019

As suspected the issue seems to be caused by resize event being sent during the transition in particular:

https://github.com/koekeishiya/yabai/blob/master/src/event.c#L1098

and

https://github.com/koekeishiya/yabai/blob/master/src/event.c#L1105

If I comment those out I can't reproduce the issue. Not sure what would be the best way to gate those resizes in this situation.

@choco
Copy link
Author

choco commented Dec 27, 2019

For reference, similar issues in kwm https://github.com/koekeishiya/kwm/pull/527 and chunkwm https://github.com/koekeishiya/chunkwm/pull/63/files#diff-57487569d98240026c2cbfa7c271f7e9 (in the case both resizing were caused by the lock to container feature).
Right now I'm using this diff and it seems to solve the issue, but I'm not confortable with yabai codebase so I'm not sure if there's a more elegant approach.

diff --git a/src/event.c b/src/event.c
index c75a92a..c70a841 100644
--- a/src/event.c
+++ b/src/event.c
@@ -942,169 +942,173 @@ static EVENT_CALLBACK(EVENT_HANDLER_MOUSE_UP)
     CGPoint point = CGEventGetLocation(context);
     debug("%s: %.2f, %.2f\n", __FUNCTION__, point.x, point.y);
 
-    struct view *src_view = window_manager_find_managed_window(&g_window_manager, g_mouse_state.window);
-    if (src_view && src_view->layout == VIEW_BSP) {
-        CGRect frame = window_ax_frame(g_mouse_state.window);
-        float dx = frame.origin.x - g_mouse_state.window_frame.origin.x;
-        float dy = frame.origin.y - g_mouse_state.window_frame.origin.y;
-        float dw = frame.size.width - g_mouse_state.window_frame.size.width;
-        float dh = frame.size.height - g_mouse_state.window_frame.size.height;
+    if (!window_is_fullscreen(g_mouse_state.window)) {
+        struct view *src_view = window_manager_find_managed_window(&g_window_manager, g_mouse_state.window);
+        if (src_view && src_view->layout == VIEW_BSP) {
+            CGRect frame = window_ax_frame(g_mouse_state.window);
+            float dx = frame.origin.x - g_mouse_state.window_frame.origin.x;
+            float dy = frame.origin.y - g_mouse_state.window_frame.origin.y;
+            float dw = frame.size.width - g_mouse_state.window_frame.size.width;
+            float dh = frame.size.height - g_mouse_state.window_frame.size.height;
 
-        bool did_change_x = dx != 0.0f;
-        bool did_change_y = dy != 0.0f;
-        bool did_change_w = dw != 0.0f;
-        bool did_change_h = dh != 0.0f;
-        bool did_change_p = did_change_x || did_change_y;
-        bool did_change_s = did_change_w || did_change_h;
+            bool did_change_x = dx != 0.0f;
+            bool did_change_y = dy != 0.0f;
+            bool did_change_w = dw != 0.0f;
+            bool did_change_h = dh != 0.0f;
+            bool did_change_p = did_change_x || did_change_y;
+            bool did_change_s = did_change_w || did_change_h;
 
-        if (did_change_p && !did_change_s) {
-            uint64_t cursor_sid = display_space_id(display_manager_cursor_display_id());
-            struct view *dst_view = space_manager_find_view(&g_space_manager, cursor_sid);
+            if (did_change_p && !did_change_s) {
+                uint64_t cursor_sid = display_space_id(display_manager_cursor_display_id());
+                struct view *dst_view = space_manager_find_view(&g_space_manager, cursor_sid);
 
-            struct window *window = window_manager_find_window_at_point_filtering_window(&g_window_manager, point, g_mouse_state.window->id);
-            if (!window) window = window_manager_find_window_at_point(&g_window_manager, point);
-            if (window == g_mouse_state.window) window = NULL;
+                struct window *window = window_manager_find_window_at_point_filtering_window(&g_window_manager, point, g_mouse_state.window->id);
+                if (!window) window = window_manager_find_window_at_point(&g_window_manager, point);
+                if (window == g_mouse_state.window) window = NULL;
 
-            struct window_node *a_node = view_find_window_node(src_view, g_mouse_state.window->id);
-            struct window_node *b_node = window ? view_find_window_node(dst_view, window->id) : NULL;
+                struct window_node *a_node = view_find_window_node(src_view, g_mouse_state.window->id);
+                struct window_node *b_node = window ? view_find_window_node(dst_view, window->id) : NULL;
 
-            /*
-             * @cleanup
-             *
-             * NOTE(koekeishiya): The following section of code contains duplicated code,
-             * but is left inline as-is for readability purposes. Consider replacing with
-             * macros, because C does not allow for locally-scoped functions.
-             * */
+                /*
+                * @cleanup
+                *
+                * NOTE(koekeishiya): The following section of code contains duplicated code,
+                * but is left inline as-is for readability purposes. Consider replacing with
+                * macros, because C does not allow for locally-scoped functions.
+                * */
 
-            if (a_node && b_node) {
-                CGRect frame = window_ax_frame(window);
-                CGPoint point_in_window = {
-                    .x = point.x - frame.origin.x,
-                    .y = point.y - frame.origin.y
-                };
+                if (a_node && b_node) {
+                    CGRect frame = window_ax_frame(window);
+                    CGPoint point_in_window = {
+                        .x = point.x - frame.origin.x,
+                        .y = point.y - frame.origin.y
+                    };
 
-                CGRect window_center;
-                window_center.origin.x    = 0.25f * frame.size.width;
-                window_center.origin.y    = 0.25f * frame.size.height;
-                window_center.size.width  = 0.50f * frame.size.width;
-                window_center.size.height = 0.50f * frame.size.height;
+                    CGRect window_center;
+                    window_center.origin.x    = 0.25f * frame.size.width;
+                    window_center.origin.y    = 0.25f * frame.size.height;
+                    window_center.size.width  = 0.50f * frame.size.width;
+                    window_center.size.height = 0.50f * frame.size.height;
 
-                CGPoint top_triangle[3];
-                top_triangle[0].x = 0.0f;
-                top_triangle[0].y = 0.0f;
-                top_triangle[1].x = 0.5f * frame.size.width;
-                top_triangle[1].y = 0.5f * frame.size.height;
-                top_triangle[2].x = frame.size.width;
-                top_triangle[2].y = 0.0f;
+                    CGPoint top_triangle[3];
+                    top_triangle[0].x = 0.0f;
+                    top_triangle[0].y = 0.0f;
+                    top_triangle[1].x = 0.5f * frame.size.width;
+                    top_triangle[1].y = 0.5f * frame.size.height;
+                    top_triangle[2].x = frame.size.width;
+                    top_triangle[2].y = 0.0f;
 
-                CGPoint right_triangle[3];
-                right_triangle[0].x = frame.size.width;
-                right_triangle[0].y = 0.0f;
-                right_triangle[1].x = 0.5f * frame.size.width;
-                right_triangle[1].y = 0.5f * frame.size.height;
-                right_triangle[2].x = frame.size.width;
-                right_triangle[2].y = frame.size.height;
+                    CGPoint right_triangle[3];
+                    right_triangle[0].x = frame.size.width;
+                    right_triangle[0].y = 0.0f;
+                    right_triangle[1].x = 0.5f * frame.size.width;
+                    right_triangle[1].y = 0.5f * frame.size.height;
+                    right_triangle[2].x = frame.size.width;
+                    right_triangle[2].y = frame.size.height;
 
-                CGPoint bottom_triangle[3];
-                bottom_triangle[0].x = frame.size.width;
-                bottom_triangle[0].y = frame.size.height;
-                bottom_triangle[1].x = 0.5f * frame.size.width;
-                bottom_triangle[1].y = 0.5f * frame.size.height;
-                bottom_triangle[2].x = 0.0f;
-                bottom_triangle[2].y = frame.size.height;
+                    CGPoint bottom_triangle[3];
+                    bottom_triangle[0].x = frame.size.width;
+                    bottom_triangle[0].y = frame.size.height;
+                    bottom_triangle[1].x = 0.5f * frame.size.width;
+                    bottom_triangle[1].y = 0.5f * frame.size.height;
+                    bottom_triangle[2].x = 0.0f;
+                    bottom_triangle[2].y = frame.size.height;
 
-                CGPoint left_triangle[3];
-                left_triangle[0].x = 0.0f;
-                left_triangle[0].y = frame.size.height;
-                left_triangle[1].x = 0.5f * frame.size.width;
-                left_triangle[1].y = 0.5f * frame.size.height;
-                left_triangle[2].x = 0.0f;
-                left_triangle[2].y = 0.0f;
+                    CGPoint left_triangle[3];
+                    left_triangle[0].x = 0.0f;
+                    left_triangle[0].y = frame.size.height;
+                    left_triangle[1].x = 0.5f * frame.size.width;
+                    left_triangle[1].y = 0.5f * frame.size.height;
+                    left_triangle[2].x = 0.0f;
+                    left_triangle[2].y = 0.0f;
 
-                enum window_node_split new_split;
-                enum window_node_child new_child;
+                    enum window_node_split new_split;
+                    enum window_node_child new_child;
 
-                if (CGRectContainsPoint(window_center, point_in_window)) {
-do_swap:
-                    a_node->window_id = window->id;
-                    a_node->zoom = NULL;
-                    b_node->window_id = g_mouse_state.window->id;
-                    b_node->zoom = NULL;
+                    if (CGRectContainsPoint(window_center, point_in_window)) {
+    do_swap:
+                        a_node->window_id = window->id;
+                        a_node->zoom = NULL;
+                        b_node->window_id = g_mouse_state.window->id;
+                        b_node->zoom = NULL;
 
-                    if (src_view->sid != dst_view->sid) {
-                        window_manager_remove_managed_window(&g_window_manager, a_node->window_id);
-                        window_manager_remove_managed_window(&g_window_manager, b_node->window_id);
-                        window_manager_add_managed_window(&g_window_manager, window, src_view);
-                        window_manager_add_managed_window(&g_window_manager, g_mouse_state.window, dst_view);
-                    }
+                        if (src_view->sid != dst_view->sid) {
+                            window_manager_remove_managed_window(&g_window_manager, a_node->window_id);
+                            window_manager_remove_managed_window(&g_window_manager, b_node->window_id);
+                            window_manager_add_managed_window(&g_window_manager, window, src_view);
+                            window_manager_add_managed_window(&g_window_manager, g_mouse_state.window, dst_view);
+                        }
 
-                    window_node_flush(a_node);
-                    window_node_flush(b_node);
-                    goto end;
-                } else if (triangle_contains_point(top_triangle, point_in_window)) {
-                    new_split = SPLIT_X;
-                    new_child = CHILD_FIRST;
-                } else if (triangle_contains_point(right_triangle, point_in_window)) {
-                    new_split = SPLIT_Y;
-                    new_child = CHILD_SECOND;
-                } else if (triangle_contains_point(bottom_triangle, point_in_window)) {
-                    new_split = SPLIT_X;
-                    new_child = CHILD_SECOND;
-                } else if (triangle_contains_point(left_triangle, point_in_window)) {
-                    new_split = SPLIT_Y;
-                    new_child = CHILD_FIRST;
-                } else {
-                    goto end;
-                }
-
-                if ((a_node->parent && b_node->parent) &&
-                    (a_node->parent == b_node->parent)) {
-                    if (b_node->parent->split == new_split) {
-                        goto do_swap;
+                        window_node_flush(a_node);
+                        window_node_flush(b_node);
+                        goto end;
+                    } else if (triangle_contains_point(top_triangle, point_in_window)) {
+                        new_split = SPLIT_X;
+                        new_child = CHILD_FIRST;
+                    } else if (triangle_contains_point(right_triangle, point_in_window)) {
+                        new_split = SPLIT_Y;
+                        new_child = CHILD_SECOND;
+                    } else if (triangle_contains_point(bottom_triangle, point_in_window)) {
+                        new_split = SPLIT_X;
+                        new_child = CHILD_SECOND;
+                    } else if (triangle_contains_point(left_triangle, point_in_window)) {
+                        new_split = SPLIT_Y;
+                        new_child = CHILD_FIRST;
                     } else {
-                        b_node->parent->split = new_split;
-                        b_node->parent->child = new_child;
+                        goto end;
                     }
-                } else {
-                    b_node->split = new_split;
-                    b_node->child = new_child;
-                }
 
-                space_manager_untile_window(&g_space_manager, src_view, g_mouse_state.window);
-                window_manager_remove_managed_window(&g_window_manager, g_mouse_state.window->id);
-                window_manager_purify_window(&g_window_manager, g_mouse_state.window);
+                    if ((a_node->parent && b_node->parent) &&
+                        (a_node->parent == b_node->parent)) {
+                        if (b_node->parent->split == new_split) {
+                            goto do_swap;
+                        } else {
+                            b_node->parent->split = new_split;
+                            b_node->parent->child = new_child;
+                        }
+                    } else {
+                        b_node->split = new_split;
+                        b_node->child = new_child;
+                    }
 
-                struct view *view = space_manager_tile_window_on_space_with_insertion_point(&g_space_manager, g_mouse_state.window, dst_view->sid, window->id);
-                window_manager_add_managed_window(&g_window_manager, g_mouse_state.window, view);
-end:;
-            } else if (a_node) {
-                if (src_view->sid == dst_view->sid) {
-                    a_node->zoom = NULL;
-                    window_node_flush(a_node);
-                } else {
                     space_manager_untile_window(&g_space_manager, src_view, g_mouse_state.window);
                     window_manager_remove_managed_window(&g_window_manager, g_mouse_state.window->id);
                     window_manager_purify_window(&g_window_manager, g_mouse_state.window);
 
-                    struct view *view = space_manager_tile_window_on_space(&g_space_manager, g_mouse_state.window, dst_view->sid);
+                    struct view *view = space_manager_tile_window_on_space_with_insertion_point(&g_space_manager, g_mouse_state.window, dst_view->sid, window->id);
                     window_manager_add_managed_window(&g_window_manager, g_mouse_state.window, view);
+    end:;
+                } else if (a_node) {
+                    if (src_view->sid == dst_view->sid) {
+                        a_node->zoom = NULL;
+                        window_node_flush(a_node);
+                    } else {
+                        space_manager_untile_window(&g_space_manager, src_view, g_mouse_state.window);
+                        window_manager_remove_managed_window(&g_window_manager, g_mouse_state.window->id);
+                        window_manager_purify_window(&g_window_manager, g_mouse_state.window);
+
+                        struct view *view = space_manager_tile_window_on_space(&g_space_manager, g_mouse_state.window, dst_view->sid);
+                        window_manager_add_managed_window(&g_window_manager, g_mouse_state.window, view);
+                    }
+                }
+            } else {
+                if (did_change_p) {
+                    uint8_t direction = 0;
+                    if (dx != 0.0f) direction |= HANDLE_LEFT;
+                    if (dy != 0.0f) direction |= HANDLE_TOP;
+                    window_manager_resize_window_relative(&g_window_manager, g_mouse_state.window, direction, dx, dy);
+                }
+
+                if (did_change_s) {
+                    uint8_t direction = 0;
+                    if (did_change_w && !did_change_x) direction |= HANDLE_RIGHT;
+                    if (did_change_h && !did_change_y) direction |= HANDLE_BOTTOM;
+                    window_manager_resize_window_relative(&g_window_manager, g_mouse_state.window, direction, dw, dh);
                 }
             }
-        } else {
-            if (did_change_p) {
-                uint8_t direction = 0;
-                if (dx != 0.0f) direction |= HANDLE_LEFT;
-                if (dy != 0.0f) direction |= HANDLE_TOP;
-                window_manager_resize_window_relative(&g_window_manager, g_mouse_state.window, direction, dx, dy);
-            }
-
-            if (did_change_s) {
-                uint8_t direction = 0;
-                if (did_change_w && !did_change_x) direction |= HANDLE_RIGHT;
-                if (did_change_h && !did_change_y) direction |= HANDLE_BOTTOM;
-                window_manager_resize_window_relative(&g_window_manager, g_mouse_state.window, direction, dw, dh);
-            }
         }
+    } else {
+        debug("window is fullscreen, no need for that stuff");
     }
 
     g_mouse_state.current_action = MOUSE_MODE_NONE;

@koekeishiya koekeishiya removed help wanted Community help appreciated obscure Weird behaviour that is difficult to debug labels Dec 28, 2019
@koekeishiya koekeishiya added the addressed on master; not released Fixed upstream, but not yet released label Dec 28, 2019
@koekeishiya
Copy link
Owner

Resolved on master by checking whether the window is fullscreen or not, although not the exact change made in your diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants