FelixKratz / JankyBorders

A lightweight window border system for macOS
GNU General Public License v3.0
1.1k stars 18 forks source link

Integrate with yabai window animations #62

Closed koekeishiya closed 7 months ago

koekeishiya commented 8 months ago

Let me preface this by saying that I haven't given this a lot of thought, but I figured it might be something worth investigating..

Currently when using borders with yabai window animations, borders are of course updated to the new frame immediately; not following or waiting for the animation to finish. My question is, would there be a way for these tools to integrate more smoothtly, and if so what might that look like.

I don't know how big of an overlap there is between users that enable both of these features, and I guess the degree to how distracting it is, will vary as well.

Feel free to close if you personally don't think this is that big of an issue.

Edit: The most trivial version could be to hide the border for a given window (based on the window id) when an animation is about to start, and reveal it again when the animation finishes.

FelixKratz commented 8 months ago

I have animations enabled and this issue is the reason why I made the animation duration a lot shorter than I would ideally want, so yeah it is definitely worth discussing.

There are three ways I see this could be tackled:

I think it would make more sense to have a heuristic for this problem on the borders side (given the largely disproportionate user base). Maybe something (janky...) like this:

  int cid = SLSMainConnectionID();
  int wid_cid = 0;
  SLSGetWindowOwner(cid, wid, &wid_cid);

  pid_t pid = 0;
  SLSConnectionGetPID(wid_cid, &pid);

  static char pid_name_buffer[PROC_PIDPATHINFO_MAXSIZE];
  proc_name(pid, &pid_name_buffer, sizeof(pid_name_buffer));
  static int count = 0;
  if (strcmp(pid_name_buffer, "yabai") == 0) {
    count++;
    printf("%d: Yabai Window detected: %d\n", count, wid);
    CGRect frame;
    SLSGetWindowBounds(cid, wid, &frame);
    // Find parent window based on frame and space (maybe not robust for windows sharing origin, size and space, e.g. stacks)
    CGAffineTransform transform = {0}, transform_old = {0};
    for(int i = 0; i < 120; i++) {
      SLSGetWindowTransform(cid, wid, &transform);
      printf("%f %f %f %f %f %f\n", transform.tx, transform.ty, transform.a, transform.b, transform.c, transform.d);
      if (CGAffineTransformEqualToTransform(transform, transform_old)) break;
      // set the parent window border transform.
      usleep(8000);

      // allow some grace before stopping the polling
      if (i % 12 == 11) transform_old = transform;
    }
  }

This would need to run in a different thread and essentially polls the transform of the proxy window for a while. Determining the appropriate parent window of the proxy window is not robustly possible though (e.g. in a stack where space, origin and size are the same for multiple windows). However, after the animation finishes, the borders can be updated to the parent real location such that this should not be that big of a deal.

Ideally there would be an api where transform changes of windows can be registered to, but there seems to be no notification for that in the window notifications send by the window server.

Alternatively, the border could simply be removed for the duration of the animation by listening to window create and window destroy of proxy windows.

koekeishiya commented 8 months ago

It is possible for yabai to tag proxy windows with the id of the real window. This can be done by either setting the title of the proxy window, or maybe by using CGSSetConnectionProperty to assign a key-value pair (key is proxy wid, value is real wid) on yabai's connection. Then you could probably read that key's value from borders.

Alternatively, the border could simply be removed for the duration of the animation by listening to window create and window destroy of proxy windows.

Maybe this should be the very first approach, by listening to create and destroy events for proxy windows. The only requirement I see would be to know which window the proxy is temporarily replacing.

If this version works we could investigate further modifications to allow for actual animation of borders as well. A naive solution could be to detect when a proxy window is created, grab the real window id, and poll the transform (as you mentioned) until the proxy window is destroyed, in which the transform is reset and the final position/size of the border is updated.

FelixKratz commented 8 months ago

I think reading the window title (via SLSWindowIteratorCopyTitle) is locked behind screen capture permission.

I think reading connection properties (via SLSCopyConnectionProperty) should work though so that would be a robust approach.

Probably, the wids of all proxied windows need to be stored in the connection properties though, because I think there is only ever one single connection for an animation of several windows.

Possibly something like:

  CFArrayRef wids = NULL;
  SLSCopyConnectionProperty(cid, yabai_cid, CFSTR("proxied_wids"), &wids);

where I then loop over all entries and hide/show the border?

koekeishiya commented 8 months ago

Hmm not sure, need to think about the connection lifetime in yabai. In the current version, every animation gets its own connection, and this connection is used to create proxy windows for all windows associated with that animation. Not sure if there is any real need for having a separate connection for each animation anyway.

Calling SLSGetWindowOwner on a proxy window should result in that animation connection. Which means that this connection needs to hold the information that JankyBorders need to access. When an animation finishes, yabai destroys all proxy windows, and then also destroy the connection. At this point JankyBorders needs to have either cached the proxy_wid -> real_wid mapping, because it is probably not reliable to look up the connection or connection properties at this time.

I think the most straightforward solution would be the following (but also might suffer from performance issues due to many lookups):

  1. yabai begins to prepare an animation.
  2. yabai prepares all proxy windows (using multiple threads).

    2a). For each window;; when a proxy window is created, call CGSSetConnectionProperty with proxy_wid as key, and real_wid as value.

2b). For each window;; JankyBorders is notified that proxy_window is created. Look up connection and copy property with proxy_wid as key.

2c). JankyBorders needs to cache mapping between proxy_wid and real_wid, to restore the border upon proxy_destroyed event, and hide the border.

  1. yabai plays animation.
  2. yabai finishes animation and destroys proxy windows.
  3. JankyBorder receives proxy_destroyed events, looks up proxy_wid in cache to retrieve real_wid and shows the border.

It is likely faster to do a single CGSSetConnectionProperty to set all window ids, and a single SLSCopyConnectionProperty to retrieve all of them, but then JankyBorders would need to know which proxy_created and proxy_destroyed event to respond to, because the information is not set on the connection until all window proxies have been created.

Edit: proxy_wid above refers to the actual window id as a number, and not the string "proxy_wid". Same with "real_wid". So you'd get the window id of the newly created window and pass that along as the key to SLSCopyConnectionProperty.

FelixKratz commented 8 months ago

I think your solution sounds good. I am not sure, but I think the overhead of setting and copying the connection properties is really small compared to the high res screenshot needed for the proxy window so that should probably not be a problem.

Edit: What if the connection property was only set for the last window but as a dictionary? If the property is not set -> do nothing; if the property is set -> loop through all proxy_wid-real_wid entries of the dict and react appropriately. Not sure the connection property value can be any core foundation instance though.

koekeishiya commented 8 months ago

It seems to work fine; I can set and read the property just fine, but for some reason the border won't actually remain hidden. I suspect this is just me not fully grasping how the borders code works.

(EDIT: It works as expected for some operations, like warp or swap in yabai, but not when using --toggle zoom-fullscreen).

EDIT2: Patches removed to reduce clutter in issue. The yabai master added support in https://github.com/koekeishiya/yabai/commit/b91f95e005cc9a074bf20523c4ebb72499e4100a and the JankyBorders POC patch is in my latest comment in this issue https://github.com/FelixKratz/JankyBorders/issues/62#issuecomment-1934953688

FelixKratz commented 8 months ago

Current master has this integration and it works well with yabai patched. There are still some edge cases that seem to not work though. The order of creating and destroying proxy windows when an animation exists is:

Such that there is a brief moment where there are two proxies for one window. Destroying the old proxy shows the border again while it really should stay hidden because there is an additional proxy.

koekeishiya commented 8 months ago

I have a version locally that seems to fix all issues, including the one you are mentioning with create new proxy and destroy old proxy. I'll push the yabai changes and I can link the new JankyBorders diff.

koekeishiya commented 8 months ago

Right, so the current/new yabai master seems to work quite well with the following JankyBorders patch applied to commit 5e7fa9d13737407b768bafae894352cc1d714bbc

(NB: Code should be improved before included upstream; this is just quick and dirty POC).

diff --git a/src/events.c b/src/events.c
index 37e7310..978216a 100644
--- a/src/events.c
+++ b/src/events.c
@@ -1,9 +1,13 @@
+#include <sys/proc_info.h>
+#include <libproc.h>
+
 #include "events.h"
 #include "windows.h"
 #include "border.h"
 #include "misc/window.h"

 extern struct table g_windows;
+extern struct table g_proxy;
 extern pid_t g_pid;

 static void dump_event(void* data, size_t data_length) {
@@ -32,11 +36,86 @@ static void window_spawn_handler(uint32_t event, struct window_spawn_data* data,
   if (!wid || !sid) return;

   if (event == EVENT_WINDOW_CREATE) {
+    int cid = SLSMainConnectionID();
+    int wid_cid = 0;
+    SLSGetWindowOwner(cid, wid, &wid_cid);
+
+    pid_t pid = 0;
+    SLSConnectionGetPID(wid_cid, &pid);
+
+    static char pid_name_buffer[PROC_PIDPATHINFO_MAXSIZE];
+    proc_name(pid, &pid_name_buffer, sizeof(pid_name_buffer));
+
+    if (strcmp(pid_name_buffer, "yabai") == 0) {
+        char num_str[255] = {0};
+        snprintf(num_str, sizeof(num_str), "%d", wid);
+        CFStringRef key_ref = CFStringCreateWithCString(NULL, num_str, kCFStringEncodingMacRoman);
+        CFTypeRef value_ref = NULL;
+        SLSCopyConnectionProperty(cid, wid_cid, key_ref, &value_ref);
+        CFRelease(key_ref);
+        if (value_ref) {
+            uint32_t real_wid = 0;
+            if (CFNumberGetValue(value_ref, kCFNumberSInt32Type, &real_wid)) {
+                // remove existing entry for real window and add new one.
+                // this is necessary because yabai will create a new proxy window for the same real window,
+                // when interrupting an existing animation. this proxy window is stale and should not
+                // trigger a border_draw event upon its destruction.
+                //
+                // this reverse-mapping from real_wid -> proxy_wid is used in the destroy_event.
+                // we should only draw the border if the destroyed proxy is the currently active proxy.
+                table_remove(&g_proxy, &real_wid);
+                table_add(&g_proxy, &real_wid, (void*)(intptr_t)wid);
+
+                // map proxy_wid to real_wid
+                table_add(&g_proxy, &wid, (void*)(intptr_t)real_wid);
+
+                // hide any associated border
+                struct border* border = table_find(&g_windows, &real_wid);
+                if (border) {
+                    border->disable = true;
+                    border_hide(border);
+                }
+            }
+        }
+        return;
+    }
+
+
     if (windows_window_create(windows, wid, sid)) {
       debug("Window Created: %d %d\n", wid, sid);
       windows_determine_and_focus_active_window(windows);
     }
   } else if (event == EVENT_WINDOW_DESTROY) {
+    // it is not possible to retrieve the window connection at this point, so we do not know
+    // if this is a yabai window or not. look up proxy table, if match, it is a yabai window.
+    void *proxy_value = table_find(&g_proxy, &wid);
+    if (proxy_value) {
+        // remove proxy window from table
+        table_remove(&g_proxy, &wid);
+
+        uint32_t real_wid = (uint32_t)(intptr_t)proxy_value;
+
+        // use reverse-mapping to detect the last created proxy window for the real window
+        void *value = table_find(&g_proxy, &real_wid);
+        if (value) {
+            uint32_t proxy_wid = (uint32_t)(intptr_t)value;
+
+            // if the destroyed proxy window is the last/current proxy for the real window
+            if (wid == proxy_wid) {
+                // remove reverse-map from real window to proxy window
+                table_remove(&g_proxy, &real_wid);
+
+                // draw border if any is associated
+                struct border* border = table_find(&g_windows, &real_wid);
+                if (border) {
+                    border->disable = false;
+                    border_draw(border);
+                }
+            }
+        }
+        return;
+    }
+
     if (windows_window_destroy(windows, wid, sid)) {
       debug("Window Destroyed: %d %d\n", wid, sid);
     }
@@ -48,6 +127,11 @@ static void window_modify_handler(uint32_t event, uint32_t* window_id, size_t _,
   uint32_t wid = *window_id;
   struct table* windows = &g_windows;

+  // if the modified window is currently being proxied by yabai, ignore any events.
+  // the border will be updated when the yabai proxy window is destroyed.
+  void *proxy_value = table_find(&g_proxy, &wid);
+  if (proxy_value) return;
+
   if (event == EVENT_WINDOW_MOVE) {
     debug("Window Move: %d\n", wid);
     windows_window_move(windows, wid);
diff --git a/src/main.c b/src/main.c
index 8245deb..6a553ab 100644
--- a/src/main.c
+++ b/src/main.c
@@ -20,6 +20,7 @@
 pid_t g_pid;
 mach_port_t g_server_port;
 struct table g_windows;
+struct table g_proxy;
 struct mach_server g_mach_server;
 struct settings g_settings = { .active_window = { .stype = COLOR_STYLE_SOLID,
                                                   .color = 0xffe1e3e4 },
@@ -135,6 +136,7 @@ int main(int argc, char** argv) {

   pid_for_task(mach_task_self(), &g_pid);
   table_init(&g_windows, 1024, hash_windows, cmp_windows);
+  table_init(&g_proxy, 1024, hash_windows, cmp_windows);

   g_server_port = create_connection_server_port();

diff --git a/src/misc/extern.h b/src/misc/extern.h
index 90bf8ce..c34f189 100644
--- a/src/misc/extern.h
+++ b/src/misc/extern.h
@@ -5,6 +5,7 @@ extern mach_port_t SLSServerPort(void* zero);
 extern mach_port_t mig_get_special_reply_port(void);
 extern mach_port_t mig_dealloc_special_reply_port(mach_port_t port);
 extern int SLSMainConnectionID();
+extern CGError SLSCopyConnectionProperty(int cid, int target_cid, CFStringRef key, CFTypeRef *value);
 extern CGError SLSWindowManagementBridgeSetDelegate(void* delegate);
 extern CGError SLSGetEventPort(int cid, mach_port_t* port_out);
 extern CGEventRef SLEventCreateNextEvent(int cid);
FelixKratz commented 8 months ago

I have included your suggested improvements on master and fixed another edge case, so I think it works very reliably now.

koekeishiya commented 8 months ago

Works well, and I think this is good enough for now. I think this was the last issue that prevented me personally from using JankyBorders. I assume that the transform polling solution we discussed earlier could be attempted without further changes to the yabai code, but I'm not sure if it will be performant enough.

Feel free to tag me if you are interested in attempting an animated border solution in the future.

FelixKratz commented 8 months ago

I have an animated solution poc here: https://github.com/FelixKratz/JankyBorders/commit/37183f4dce37fe458ada9274895530a511009dca it still breaks in some cases but I thought Id push it anyways to discuss if it is even fast enough. I think there is some delay, how much that can be noticed is probably depending on the viewer. Not sure if I would use it instead of the hide and show tough.

koekeishiya commented 8 months ago

It seems to work well enough performance-wise when an animation is played out fully, but breaks down when animations are intercepted.

It does look slightly better with animated borders than hiding in my opinion when taking yabai opacity and borders background-blur into consideration.

FelixKratz commented 8 months ago

Thats because the proxy window has a different size and the border frame was not updated to that new frame yet, such that the transforms dont match. Should be trivial to fix.

FelixKratz commented 8 months ago

It is possible to further improve the performance of the border->proxy tracking by artificially stalling the frame_callback function (https://github.com/FelixKratz/JankyBorders/commit/10fea2ad1dd0f1ac5d00b1e167c7ffa57ec258e3). This works because the CVDisplayLink calls the frame_callback as soon as the previous frame was flushed to the display, giving us an entire frame time to perform the calculations for the next frame. The problem is that we are sometimes too fast here because yabai still has the remaining frame time to update the transform of the window to the next frame (making the border exactly one frame late -- because it was too early in the frame cycle). AFAIK yabai does not care about the actual display frame timing in its animations, if it did, the result would probably be an even better border->proxy tracking.

koekeishiya commented 8 months ago

I'm not actually sure how all this works in modern macOS after the transition to adaptive sync. (By using CVDisplayLink this is basically free). You are correct that yabai does no work to hit the actual display refresh cycle -- it only makes sure that the animation itself plays at a fixed framerate.

I'm not deeply familiar with the CVDisplayLink API (although I get the basics for how it works), so I don't know how easy it will be to port yabai's animation system to this API, while still retaining the current flexibilty.

FelixKratz commented 8 months ago

This commit should fix the final quirks with intercepted animations: d1010ff09b8c275d6770d7e481dd8b4babc004d3 I have merged this onto the master branch.

In the end the CVDisplayLink provides a high priority thread which handles the draw calls so It should be pretty similar to the current thread based animation approach, where the animation pthread is replaced by the display link thread (essentially window_manager_animate_window_list_thread_proc would be the frame_callback function called by the CVDisplayLink, where instead of the loop, a single frames is drawn until finally all cleanup is performed just as it is done currently). As always, the devil will be in the details though.

FelixKratz commented 8 months ago

Here is a patch for yabai that uses CVDisplayLink. The resulting animations are buttery smooth and the border->proxy tracking is close to perfect as well. But this is only a very rough poc:

diff --git a/makefile b/makefile
index c412829..140e310 100644
--- a/makefile
+++ b/makefile
@@ -1,5 +1,5 @@
 FRAMEWORK_PATH = -F/System/Library/PrivateFrameworks
-FRAMEWORK      = -framework Carbon -framework Cocoa -framework CoreServices -framework SkyLight
+FRAMEWORK      = -framework Carbon -framework Cocoa -framework CoreServices -framework CoreVideo -framework SkyLight
 BUILD_FLAGS    = -std=c99 -Wall -g -O0 -fvisibility=hidden -mmacosx-version-min=11.0 -fno-objc-arc -arch x86_64 -arch arm64
 BUILD_PATH     = ./bin
 DOC_PATH       = ./doc
diff --git a/src/manifest.m b/src/manifest.m
index 51c15f4..8dc8c15 100644
--- a/src/manifest.m
+++ b/src/manifest.m
@@ -1,5 +1,6 @@
 #include <Carbon/Carbon.h>
 #include <Cocoa/Cocoa.h>
+#include <CoreVideo/CoreVideo.h>
 #include <objc/objc-runtime.h>
 #include <mach/mach_time.h>
 #include <mach-o/dyld.h>
diff --git a/src/view.h b/src/view.h
index 1f69ebd..e696653 100644
--- a/src/view.h
+++ b/src/view.h
@@ -42,7 +42,7 @@ struct window_animation_context
 {
     int animation_connection;
     float animation_duration;
-    int animation_frame_rate;
+    uint64_t initial_time;
     struct window_animation *animation_list;
     int animation_count;
 };
diff --git a/src/window_manager.c b/src/window_manager.c
index 7b3683c..9e8a9ad 100644
--- a/src/window_manager.c
+++ b/src/window_manager.c
@@ -495,43 +495,77 @@ static void *window_manager_build_window_proxy_thread_proc(void *data)
     return NULL;
 }

-static void *window_manager_animate_window_list_thread_proc(void *data)
+static CVReturn window_manager_animate_window_list_thread_proc(CVDisplayLinkRef display_link, const CVTimeStamp* now, const CVTimeStamp* output_time, CVOptionFlags flags, CVOptionFlags* flags_out, void* data)
 {
     struct window_animation_context *context = data;
     int animation_count = context->animation_count;

-    ANIMATE(context->animation_connection, context->animation_frame_rate, context->animation_duration, ease_out_cubic, {
+    uint64_t current_time = output_time->hostTime;
+    if (!context->initial_time) context->initial_time = now->hostTime;
+
+    float t = (float)(current_time - context->initial_time) / (float) (context->animation_duration * CVGetHostClockFrequency());
+
+    bool finished = false;
+    if (t < 0.0f)
+        t = 0.0f;
+    if (t >= 1.0f) {
+        t = 1.0f;
+        finished = true;
+    }
+
+    float mt = ease_out_cubic(t);
+    CFTypeRef transaction = SLSTransactionCreate(context->animation_connection);
+
+    for (int i = 0; i < animation_count; i++) {
+        context->animation_list[i].proxy.tx =
+            lerp(context->animation_list[i].proxy.frame.origin.x, mt,
+                 context->animation_list[i].x);
+        context->animation_list[i].proxy.ty =
+            lerp(context->animation_list[i].proxy.frame.origin.y, mt,
+                 context->animation_list[i].y);
+        context->animation_list[i].proxy.tw =
+            lerp(context->animation_list[i].proxy.frame.size.width, mt,
+                 context->animation_list[i].w);
+        context->animation_list[i].proxy.th =
+            lerp(context->animation_list[i].proxy.frame.size.height, mt,
+                 context->animation_list[i].h);
+
+        CGAffineTransform transform = CGAffineTransformMakeTranslation(
+            -context->animation_list[i].proxy.tx,
+            -context->animation_list[i].proxy.ty);
+        CGAffineTransform scale = CGAffineTransformMakeScale(
+            context->animation_list[i].proxy.frame.size.width /
+                context->animation_list[i].proxy.tw,
+            context->animation_list[i].proxy.frame.size.height /
+                context->animation_list[i].proxy.th);
+        SLSTransactionSetWindowTransform(
+            transaction, context->animation_list[i].proxy.id, 0, 0,
+            CGAffineTransformConcat(transform, scale));
+    }
+
+    SLSTransactionCommit(transaction, 0);
+    CFRelease(transaction);
+
+    if (finished) {
+        pthread_mutex_lock(&g_window_manager.window_animations_lock);
+        SLSDisableUpdate(context->animation_connection);
         for (int i = 0; i < animation_count; ++i) {
             if (context->animation_list[i].skip) continue;

-            context->animation_list[i].proxy.tx = lerp(context->animation_list[i].proxy.frame.origin.x,    mt, context->animation_list[i].x);
-            context->animation_list[i].proxy.ty = lerp(context->animation_list[i].proxy.frame.origin.y,    mt, context->animation_list[i].y);
-            context->animation_list[i].proxy.tw = lerp(context->animation_list[i].proxy.frame.size.width,  mt, context->animation_list[i].w);
-            context->animation_list[i].proxy.th = lerp(context->animation_list[i].proxy.frame.size.height, mt, context->animation_list[i].h);
-
-            CGAffineTransform transform = CGAffineTransformMakeTranslation(-context->animation_list[i].proxy.tx, -context->animation_list[i].proxy.ty);
-            CGAffineTransform scale = CGAffineTransformMakeScale(context->animation_list[i].proxy.frame.size.width / context->animation_list[i].proxy.tw, context->animation_list[i].proxy.frame.size.height / context->animation_list[i].proxy.th);
-            SLSTransactionSetWindowTransform(transaction, context->animation_list[i].proxy.id, 0, 0, CGAffineTransformConcat(transform, scale));
+            table_remove(&g_window_manager.window_animations_table, &context->animation_list[i].wid);
+            scripting_addition_swap_window_proxy_out(context->animation_list[i].wid, context->animation_list[i].proxy.id);
+            window_manager_destroy_window_proxy(context->animation_connection, &context->animation_list[i].proxy);
         }
-    });
-
-    pthread_mutex_lock(&g_window_manager.window_animations_lock);
-    SLSDisableUpdate(context->animation_connection);
-    for (int i = 0; i < animation_count; ++i) {
-        if (context->animation_list[i].skip) continue;
+        SLSReenableUpdate(context->animation_connection);
+        pthread_mutex_unlock(&g_window_manager.window_animations_lock);

-        table_remove(&g_window_manager.window_animations_table, &context->animation_list[i].wid);
-        scripting_addition_swap_window_proxy_out(context->animation_list[i].wid, context->animation_list[i].proxy.id);
-        window_manager_destroy_window_proxy(context->animation_connection, &context->animation_list[i].proxy);
+        SLSReleaseConnection(context->animation_connection);
+        free(context->animation_list);
+        free(context);
+        CVDisplayLinkStop(display_link);
+        CVDisplayLinkRelease(display_link);
     }
-    SLSReenableUpdate(context->animation_connection);
-    pthread_mutex_unlock(&g_window_manager.window_animations_lock);
-
-    SLSReleaseConnection(context->animation_connection);
-    free(context->animation_list);
-
-    free(context);
-    return NULL;
+    return kCVReturnSuccess;
 }

 void window_manager_animate_window_list_async(struct window_capture *window_list, int window_count)
@@ -540,7 +574,6 @@ void window_manager_animate_window_list_async(struct window_capture *window_list

     SLSNewConnection(0, &context->animation_connection);
     context->animation_duration = g_window_manager.window_animation_duration;
-    context->animation_frame_rate = g_window_manager.window_animation_frame_rate;
     context->animation_list = malloc(window_count * sizeof(struct window_animation));
     context->animation_count = window_count;

@@ -623,9 +656,11 @@ void window_manager_animate_window_list_async(struct window_capture *window_list
     SLSReenableUpdate(context->animation_connection);
     pthread_mutex_unlock(&g_window_manager.window_animations_lock);

-    pthread_t thread;
-    pthread_create(&thread, NULL, &window_manager_animate_window_list_thread_proc, context);
-    pthread_detach(thread);
+    CVDisplayLinkRef link;
+    CVDisplayLinkCreateWithActiveCGDisplays(&link);
+    CVDisplayLinkSetOutputCallback(link, window_manager_animate_window_list_thread_proc, context);
+
+    CVDisplayLinkStart(link);
 }

 void window_manager_animate_window_list(struct window_capture *window_list, int window_count)
koekeishiya commented 7 months ago

Latest yabai master should support this now (https://github.com/koekeishiya/yabai/commit/7535e3ae655a3af5ffaeae275db8f546a643d275). Looks very good, nice work.

I have one minor nitpick regarding this implementation which is that the CVDisplayLink currently links with all active displays (if I understand correctly), meaning that if a user has multiple monitors connected that support different refresh rates it will potentially refresh at a lower rate than the optimal/maximum supported rate. E.g internal mbp with pro-motion (120hz) and an external limited to 60hz, even if the animation only affects windows on the internal pro-motion display.

FelixKratz commented 7 months ago

Using

CVReturn CVDisplayLinkCreateWithCGDisplay(CGDirectDisplayID displayID, CVDisplayLinkRef  _Nullable *displayLinkOut);

instead of

CVReturn CVDisplayLinkCreateWithActiveCGDisplays(CVDisplayLinkRef  _Nullable *displayLinkOut);

to link to the refresh rate of a particular display should resolve this nitpick.

koekeishiya commented 7 months ago

I'll probably leave it as is until someone else complains. yabai doesn't actually know the display_ids of windows in the animation without doing some additional work.

koekeishiya commented 7 months ago

Just a note that I had to add some simple JankyBorders detection to fix mouse-drag integration when borders are enabled: https://github.com/koekeishiya/yabai/commit/fdf44f34cd9cb624705c28b6543b712c19279600

I used to have a similar workaround for yabai border windows too, so it is not a big deal. I don't think JankyBorders can do anything here because of the API that yabai uses to detect windows.

FelixKratz commented 7 months ago

I'll probably leave it as is until someone else complains. yabai doesn't actually know the display_ids of windows in the animation without doing some additional work.

Ok, give me a headsup once you change it, then I will change it too, will revert this for now then: https://github.com/FelixKratz/JankyBorders/commit/445eab3045b411d01f9904b91421d56c3626cf87

koekeishiya commented 7 months ago

Ok, give me a headsup once you change it, then I will change it too,

Will do. Hopefully never, as I think it would add quite a bit of overhead in yabai as I would need to get the display-id of the origin-frame and the display-id of the target-frame for all windows in the animation to decide which displays to include in the link.

I've pushed a new yabai release v6.0.10 that includes the changes required for this issue.

FelixKratz commented 7 months ago

Great, did a release as well. It is really nice to have this properly integrated now.

koekeishiya commented 7 months ago

@FelixKratz I really want to get rid of the window animation flickering in yabai, and I can achieve that by using SLSWindowManagementBridgeSetDelegate(NULL);, however that causes yabai to stop automatically sending window_proxy created and destroyed events through the SLSRegisterNotifyProc API (events 1325 and 1326) which breaks the detection in JankyBorders.

I've tried to send these manually (and other custom event numbers) using various SLSPost/BroadcastNotification functions, but they don't seem to get detected in JankyBorders. Could a mach interface in JankyBorders maybe be a substitute, or do you have any thoughts?

Edit: Signatures

extern CGError SLSPostNotificationToConnection(int target_cid, void *data, int32_t data_size, uint32_t event);
extern CGError SLSTransactionPostNotificationToConnection(CFTypeRef transaction, int target_cid, uint32_t event, void *data, int32_t data_length);
extern CGError SLSPostBroadcastNotification(uint32_t event, void *data, int32_t data_length);
extern CGError SLSTransactionPostBroadcastNotification(CFTypeRef transaction, uint32_t event, void *data, int32_t data_length);

Edit: This is something I really want to go through with on the yabai side, as it makes the animations noticably better.

FelixKratz commented 7 months ago

You need to call SLSWindowManagementBridgeSetDelegate(NULL); before NSApplicationLoad, then it will work but I am not sure if the flickering comes back...

Edit: calling it before NSApplicationLoad negates the effect entirely.. sorry

koekeishiya commented 7 months ago

Calling it before does indeed make the flicker return.

FelixKratz commented 7 months ago

I wonder if it would be possible to directly send messages to the event port: https://github.com/FelixKratz/JankyBorders/blob/865d0cc12764495713e70e9ac6aae2da34c91816/src/main.c#L154

if I advertise it properly.

koekeishiya commented 7 months ago

Maybe SLPSPostEventRecordTo(window_psn, bytes); can post to the event port you refer to there? This is how I send arbitrary event data to other processes for autofocus functionality.

FelixKratz commented 7 months ago

Ah yes, maybe, lets try that.

So I just confirmed that it is possible to send a message directly to the event port by doing this on the JB side:

  mach_port_t port;
  CGError err = SLSGetEventPort(cid, &port);

  mach_port_t bs_port;
  if (task_get_special_port(mach_task_self(),
                            TASK_BOOTSTRAP_PORT,
                            &bs_port) != KERN_SUCCESS) {
    return false;
  }

  if (bootstrap_register(bs_port,
                         "git.felix.jbevent",
                         port    ) != KERN_SUCCESS) {
    return false;
  }

and then this on the sender side:

struct mach_message {
  mach_msg_header_t header;
  mach_msg_size_t msgh_descriptor_count;
  mach_msg_ool_descriptor_t descriptor;
};

mach_port_t mach_get_bs_port(char* bs_name) {
  mach_port_name_t task = mach_task_self();

  mach_port_t bs_port = 0;
  if (task_get_special_port(task,
                            TASK_BOOTSTRAP_PORT,
                            &bs_port            ) != KERN_SUCCESS) {
    return 0;
  }

  mach_port_t port = 0;
  if (bootstrap_look_up(bs_port,
                        bs_name,
                        &port   ) != KERN_SUCCESS) {
    return 0;
  }

  return port;
}

void mach_send_message(mach_port_t port, void* message, uint32_t len) {
  if (!message || !port) return;

  struct mach_message msg = { 0 };
  msg.header.msgh_remote_port = port;
  msg.header.msgh_bits = MACH_MSGH_BITS_SET(MACH_MSG_TYPE_COPY_SEND
                                            & MACH_MSGH_BITS_REMOTE_MASK,
                                            0,
                                            0,
                                            MACH_MSGH_BITS_COMPLEX       );

  msg.header.msgh_size = sizeof(struct mach_message);

  msg.msgh_descriptor_count = 1;
  msg.descriptor.address = message;
  msg.descriptor.size = len;
  msg.descriptor.copy = MACH_MSG_VIRTUAL_COPY;
  msg.descriptor.deallocate = false;
  msg.descriptor.type = MACH_MSG_OOL_DESCRIPTOR;

  mach_msg(&msg.header,
           MACH_SEND_MSG,
           sizeof(struct mach_message),
           0,
           MACH_PORT_NULL,
           MACH_MSG_TIMEOUT_NONE,
           MACH_PORT_NULL             );
}

int main(int argc, char **argv) {
  struct {
    uint64_t sid;
    uint32_t did;
  } data;

  mach_port_t port = mach_get_bs_port("git.felix.jbevent");
  printf("port: %d\n", port);
  mach_send_message(port, &data, sizeof(data));
}
koekeishiya commented 7 months ago

I actually have a hard time identifying the borders process from within yabai. It seems to not trigger the process created notification in macOS, and it is also not listed when enumerating running processes:

static void
process_manager_add_running_processes(struct process_manager *pm)
{
    ProcessSerialNumber psn = { kNoProcess, kNoProcess };
    while (GetNextProcess(&psn) == noErr) {
       // borders is not a part of this enumeration
    }
}
koekeishiya commented 7 months ago

I don't mind implementing the solution where I just send a mach message to your bootstrapped port as mentioned above. IF that is fine with you.

FelixKratz commented 7 months ago

Thats totally fine and will probably work reliably. The message should contain the event type, the id of the window and the space of the window. Thats basically the same information that was in the system events.

We could also pack the proxied window id into the message to replace the connection property solution.

koekeishiya commented 7 months ago

Sounds good. I'll remove the connection property and include the information inside the event data. I'm thinking this:

static void mach_send(mach_port_t port, void *data, uint32_t size)
{
    struct {
        mach_msg_header_t header;
        mach_msg_size_t descriptor_count;
        mach_msg_ool_descriptor_t descriptor;
    } msg = {0};

    msg.header.msgh_bits        = MACH_MSGH_BITS_SET(MACH_MSG_TYPE_COPY_SEND & MACH_MSGH_BITS_REMOTE_MASK, 0, 0, MACH_MSGH_BITS_COMPLEX);
    msg.header.msgh_size        = sizeof(msg);
    msg.header.msgh_remote_port = port;
    msg.descriptor_count        = 1;
    msg.descriptor.address      = data;
    msg.descriptor.size         = size;
    msg.descriptor.copy         = MACH_MSG_VIRTUAL_COPY;
    msg.descriptor.deallocate   = false;
    msg.descriptor.type         = MACH_MSG_OOL_DESCRIPTOR;

    mach_msg(&msg.header, MACH_SEND_MSG, sizeof(msg), 0, 0, 0, 0);
}

static void window_manager_notify_jankyborders(uint32_t proxy_wid, uint32_t real_wid, uint32_t proxy_event)
{
    mach_port_t port;
    if (g_bs_port && bootstrap_look_up(g_bs_port, "git.felix.jbevent", &port) == KERN_SUCCESS) {
        struct {
            uint32_t event;
            uint32_t proxy_wid;
            uint64_t proxy_sid;
            uint32_t real_wid;
        } data = { proxy_event, proxy_wid, window_space(proxy_wid), real_wid };
        mach_send(port, &data, sizeof(data));
    }
}

Should I just reuse 1325 and 1326 (macOS event numbers)?

FelixKratz commented 7 months ago

I have implemented the message receiving logic here: https://github.com/FelixKratz/JankyBorders/commit/c630f5036132e3e77f51e94df4666c82a90664d9 now only the event handling needs to be properly updated.

We could reuse the event numbering if you like.

koekeishiya commented 7 months ago

I opted to reuse the macOS numbers. Changes here: https://github.com/koekeishiya/yabai/commit/85e2a992f16ab8d44d953db1f14b2ef437da114b

FelixKratz commented 7 months ago

I just pulled the changes and made the final changes to JB as well, seems to be working great (https://github.com/FelixKratz/JankyBorders/commit/276d505575ea7bc5e0767485ad9ca32f68f0b2ab).

I noticed something else:

3   SkyLight                               0x18fe7a514 _SLEventRecordCreateData(SLSEventRecord const*, SLEventCreateDataOptions, unsigned int) + 1112
4   SkyLight                               0x18fe7b67c CGSEncodeEventRecord + 76
5   SkyLight                               0x19008b5f0 SLPSPostEventRecordTo + 104
6   yabai                                  0x10416dfc0 window_manager_make_key_window + 88 (window_manager.c:1228)

I seem to be getting a crash in this function now but it seems to be unrelated to the JB commit because in the previous commit (aeafd88) I get this crash reliably as well. Offending command: yabai -m window --focus north

koekeishiya commented 7 months ago

I can reproduce this sometimes; fixed in https://github.com/koekeishiya/yabai/issues/2130

FelixKratz commented 7 months ago

Great, now everything seems to work.

koekeishiya commented 7 months ago

This just looks so much better than before. Cheers.

I've done a bit of breaking changes in yabai, so this will be released as yabai v7.0.0 at some point. I'm not completely sure when exactly that will be, because I don't really know what else I may want to do, and how much more programming I feel like doing. There has been a lot the last couple of weeks, which I am also a bit surprised by.

I'll let you know when I do make a release, and you can try to sync a release on your side, if that sounds reasonable.

FelixKratz commented 7 months ago

It looks really nice. I am also using SLSWindowManagementBridgeSetDelegate(NULL); in sketchybar and here as well because the overhead of the window management is just insane. At some point I had 50% of cpu time spent in XPC calls to the window manager in sketchybar. And I think most problems with the built-in borders in yabai where also caused by this.

Thank you for the collaboration on this. I will post a release as soon as you are ready with yabai v7.0.0.

FelixKratz commented 7 months ago

One minor thing I have noticed, which I was not able to solve so far is that the borders are always ordered below the proxy windows. When only moving one window this is not noticeable, however, if I swap two windows, they are both proxied and then it is noticeable, as the front window border passes below both windows.

I am not sure why adding this before tracking the transform does not properly seem to work:

    SLSSetWindowLevel(cid, border->wid, window_level(cid, wid));
    SLSSetWindowSubLevel(cid, border->wid, window_sub_level(wid));
    SLSOrderWindow(cid, border->wid, -1, wid);

It seems as if it is impossible to order a window relative to the proxy window. Because even

SLSOrderWindow(cid, border->wid, 1, wid);

does not order the border above the proxy, even after swapping these lines in yabai:

    window_manager_notify_jankyborders(animation->proxy.id, animation->wid, 1325);
    scripting_addition_swap_window_proxy_in(animation->wid, animation->proxy.id);

Do you have any idea why that could be?

koekeishiya commented 7 months ago

Uh not sure, my first guess is maybe that I don't set appropriate window tags when creating the proxy windows static void window_manager_create_window_proxy(int animation_connection, float alpha, struct window_proxy *proxy) line 428.

FelixKratz commented 7 months ago

I have tried some combination of window tags but I don't see any difference.

This is an effect only noticed when calling SLSWindowManagementBridgeSetDelegate(NULL);. If I remove the line from yabai, the ordering works again. So I suspect there is some kind of "Bridged" window order call that does nothing anymore when removing the delegate. I'll keep looking if I find a way to fix this.

koekeishiya commented 7 months ago

I put together some debug code; you can apply this patch to yabai when you want to investigate further. I basically retrieve the WMDelegate instance and inspect it. It has three methods. The window animations and SLSOrderWindow all pass through performWindowManagementBridgeTransactionUsingBlock, which is why that is the swizzle target so far. When I swizzle this function and just return, it stops everything basically making it a NOOP. This is not really what we want though, as we want the fallback logic in some cases, but not all -- not just NOPing the bridged work.

diff --git a/src/yabai.c b/src/yabai.c
index 0afe9ed..83baab0 100644
--- a/src/yabai.c
+++ b/src/yabai.c
@@ -24,6 +24,7 @@
 #define MINOR  0
 #define PATCH 15

+static id (* SLSWMBridgeDelegate)(void);
 struct signal *g_signal_event[SIGNAL_TYPE_COUNT];
 struct process_manager g_process_manager;
 struct display_manager g_display_manager;
@@ -51,6 +52,50 @@ int g_connection;
 bool g_verbose;
 pid_t g_pid;

+static void dump_class_info(Class c)
+{
+    const char *name = class_getName(c);
+    NSLog(@"DUMP::%s\n", name);
+
+    unsigned int count = 0;
+
+    Ivar *ivar_list = class_copyIvarList(c, &count);
+    for (int i = 0; i < count; i++) {
+        Ivar ivar = ivar_list[i];
+        const char *ivar_name = ivar_getName(ivar);
+        NSLog(@"%s ivar: %s", name, ivar_name);
+    }
+    if (ivar_list) free(ivar_list);
+
+    objc_property_t *property_list = class_copyPropertyList(c, &count);
+    for (int i = 0; i < count; i++) {
+        objc_property_t property = property_list[i];
+        const char *prop_name = property_getName(property);
+        NSLog(@"%s property: %s", name, prop_name);
+    }
+    if (property_list) free(property_list);
+
+    Method *method_list = class_copyMethodList(c, &count);
+    for (int i = 0; i < count; i++) {
+        Method method = method_list[i];
+        const char *method_name = sel_getName(method_getName(method));
+        NSLog(@"%s method: %s", name, method_name);
+    }
+    if (method_list) free(method_list);
+}
+
+static inline id get_ivar_value(id instance, const char *name)
+{
+    id result = nil;
+    object_getInstanceVariable(instance, name, (void **) &result);
+    return result;
+}
+
+static inline void set_ivar_value(id instance, const char *name, id value)
+{
+    object_setInstanceVariable(instance, name, value);
+}
+
 static int client_send_message(int argc, char **argv)
 {
     if (argc <= 1) {
@@ -167,6 +212,84 @@ static void exec_config_file(void)
     }
 }

+struct BlockDescriptor {
+    unsigned long reserved;
+    unsigned long size;
+    void *rest[1];
+};
+
+struct Block {
+    void *isa;
+    int flags;
+    int reserved;
+    void *invoke;
+    struct BlockDescriptor *descriptor;
+};
+
+static const char *BlockSig(id blockObj)
+{
+    struct Block *block = (void *)blockObj;
+    struct BlockDescriptor *descriptor = block->descriptor;
+
+    int copyDisposeFlag = 1 << 25;
+    int signatureFlag = 1 << 30;
+
+    assert(block->flags & signatureFlag);
+
+    int index = 0;
+    if(block->flags & copyDisposeFlag)
+        index += 2;
+
+    return descriptor->rest[index];
+}
+
+static void *BlockHandler(id blockObj)
+{
+    struct Block *block = (void *)blockObj;
+    return block->invoke;
+}
+
+@implementation NSObject(swizzle)
+- (void)fake_performAsynchronousBridgedWindowManagementOperation:(id)instance
+{
+    printf("%s\n", __FUNCTION__);
+    return;
+}
+- (void)fake_performSynchronousBridgedWindowManagementOperation:(id)instance
+{
+    printf("%s\n", __FUNCTION__);
+    return;
+}
+- (void)fake_performWindowManagementBridgeTransactionUsingBlock:(id)arg1
+{
+    uint64_t address = (uint64_t)arg1;
+    printf("\n");
+    printf("FUNC: %s\n", __FUNCTION__);
+    printf("ARG1: %p\n", arg1);
+    printf("ARG1 class: %s\n", [NSStringFromClass([arg1 class]) UTF8String]);
+    printf("ARG1 description: %s\n", [[arg1 description] UTF8String]);
+    printf("ARG1 signature: %s\n", BlockSig(arg1));
+    printf("ARG1 handler: %p\n", BlockHandler(arg1));
+    printf("\n");
+
+    void *addr[40];
+    int frame_count = backtrace(addr, sizeof(addr)/sizeof(*addr));
+    if (frame_count > 1) {
+        char **syms = backtrace_symbols(addr, frame_count);
+        for (int f = 0; f < frame_count; ++f) {
+            printf("%s\n", syms[f]);
+        }
+        printf("\n");
+        free(syms);
+    } else {
+        printf("*** Failed to generate backtrace.");
+    }
+
+    [self fake_performWindowManagementBridgeTransactionUsingBlock:arg1];
+    return;
+}
+@end
+
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
 static inline bool configure_settings_and_acquire_lock(void)
@@ -180,6 +303,12 @@ static inline bool configure_settings_and_acquire_lock(void)
     snprintf(g_socket_file, sizeof(g_socket_file), SOCKET_PATH_FMT, user);
     snprintf(g_lock_file, sizeof(g_lock_file), LCFILE_PATH_FMT, user);

+    CGSGetConnectionPortById             = macho_find_symbol("/System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/SkyLight", "_CGSGetConnectionPortById");
+    SLSWMBridgeDelegate                  = macho_find_symbol("/System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/SkyLight", "__ZL19SLSWMBridgeDelegatev");
+    SLSWindowManagementBridgeSetDelegate = macho_find_symbol("/System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/SkyLight", "_SLSWindowManagementBridgeSetDelegate");
+    // if (SLSWindowManagementBridgeSetDelegate) SLSWindowManagementBridgeSetDelegate(NULL);
+    id delegate = SLSWMBridgeDelegate ? SLSWMBridgeDelegate() : NULL;
+
     NSApplicationLoad();
     g_pid = getpid();
     g_event_bytes = malloc(0xf8);
@@ -189,10 +318,6 @@ static inline bool configure_settings_and_acquire_lock(void)
     g_layer_below_window_level  = CGWindowLevelForKey(LAYER_BELOW);
     g_layer_above_window_level  = CGWindowLevelForKey(LAYER_ABOVE);

-    CGSGetConnectionPortById             = macho_find_symbol("/System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/SkyLight", "_CGSGetConnectionPortById");
-    SLSWindowManagementBridgeSetDelegate = macho_find_symbol("/System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/SkyLight", "_SLSWindowManagementBridgeSetDelegate");
-    if (SLSWindowManagementBridgeSetDelegate) SLSWindowManagementBridgeSetDelegate(NULL);
-
     signal(SIGCHLD, SIG_IGN);
     signal(SIGPIPE, SIG_IGN);
     CGSetLocalEventsSuppressionInterval(0.0f);
@@ -200,6 +325,60 @@ static inline bool configure_settings_and_acquire_lock(void)
     mouse_state_init(&g_mouse_state);
     task_get_special_port(mach_task_self(), TASK_BOOTSTRAP_PORT, &g_bs_port);

+    if (delegate) {
+        dump_class_info([delegate class]);
+
+        id manager = get_ivar_value(delegate, "_manager");
+        dump_class_info([manager class]);
+
+        // set_ivar_value(delegate, "_manager", NULL);
+        // set_ivar_value(manager, "_delegate", NULL);
+        // [manager setDelegate: NULL];
+
+        Class class = [delegate class];
+
+        {
+            SEL originalSelector = @selector(performWindowManagementBridgeTransactionUsingBlock:);
+            SEL swizzledSelector = @selector(fake_performWindowManagementBridgeTransactionUsingBlock:);
+
+            Method originalMethod = class_getInstanceMethod(class, originalSelector);
+            Method swizzledMethod = class_getInstanceMethod(class, swizzledSelector);
+
+            IMP originalImp = method_getImplementation(originalMethod);
+            IMP swizzledImp = method_getImplementation(swizzledMethod);
+
+            class_replaceMethod(class, swizzledSelector, originalImp, method_getTypeEncoding(originalMethod));
+            class_replaceMethod(class, originalSelector, swizzledImp, method_getTypeEncoding(swizzledMethod));
+        }
+        {
+            SEL originalSelector = @selector(performAsynchronousBridgedWindowManagementOperation:);
+            SEL swizzledSelector = @selector(fake_performAsynchronousBridgedWindowManagementOperation:);
+
+            Method originalMethod = class_getInstanceMethod(class, originalSelector);
+            Method swizzledMethod = class_getInstanceMethod(class, swizzledSelector);
+
+            IMP originalImp = method_getImplementation(originalMethod);
+            IMP swizzledImp = method_getImplementation(swizzledMethod);
+
+            class_replaceMethod(class, swizzledSelector, originalImp, method_getTypeEncoding(originalMethod));
+            class_replaceMethod(class, originalSelector, swizzledImp, method_getTypeEncoding(swizzledMethod));
+        }
+        {
+            SEL originalSelector = @selector(performSynchronousBridgedWindowManagementOperation:);
+            SEL swizzledSelector = @selector(fake_performSynchronousBridgedWindowManagementOperation:);
+
+            Method originalMethod = class_getInstanceMethod(class, originalSelector);
+            Method swizzledMethod = class_getInstanceMethod(class, swizzledSelector);
+
+            IMP originalImp = method_getImplementation(originalMethod);
+            IMP swizzledImp = method_getImplementation(swizzledMethod);
+
+            class_replaceMethod(class, swizzledSelector, originalImp, method_getTypeEncoding(originalMethod));
+            class_replaceMethod(class, originalSelector, swizzledImp, method_getTypeEncoding(swizzledMethod));
+        }
+    }
+
+
 #if 0
     hook_nsobject_autorelease();
     hook_autoreleasepool_drain();
koekeishiya commented 7 months ago

So there are actually multiple things going on. When the program launches. Getting the WMBridge instance and dumping it results in :

2024-03-01 13:22:01.215 yabai[93295:664422] DUMP::SLSWindowManagementFallbackBridge
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge ivar: _manager
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge property: hash
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge property: superclass
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge property: description
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge property: debugDescription
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge method: init
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge method: .cxx_destruct
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge method: performAsynchronousBridgedWindowManagementOperation:
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge method: performSynchronousBridgedWindowManagementOperation:
2024-03-01 13:22:01.215 yabai[93295:664422] SLSWindowManagementFallbackBridge method: performWindowManagementBridgeTransactionUsingBlock:
2024-03-01 13:22:01.215 yabai[93295:664422] DUMP::WMClientWindowManager
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _activeWindows
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _cachedWindowIdentifierToWindowSnapshot
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _xpcConnection
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _windowsToMinimizePostTransactionHandling
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _serverLaunchNotificationToken
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager ivar: _delegate
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager property: delegate
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager property: hash
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager property: superclass
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager property: description
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager property: debugDescription
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: init
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: .cxx_destruct
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: delegate
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: invalidate
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: setDelegate:
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: _xpcConnection
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: notifyClickDidEndOnDesktopBackgroundWindow:
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: performAsynchronousBridgedWindowManagementOperation:
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: performSynchronousBridgedWindowManagementOperation:
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: performWindowTransaction:
2024-03-01 13:22:01.215 yabai[93295:664422] WMClientWindowManager method: proposeKeyWindowWithInfo:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: proposeMainWindowWithInfo:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: stages
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _reconnectIfNecessary
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _reconnect
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _createXPCConnection
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: updateStages:displayChangeSeed:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _applyLegacyXPCWindowProperties:toSceneSettings:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _convertLegacyXPCActionToScenes:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _createWindowPropertySnapshotForWindow:properties:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _handleInterruptionFromConnection:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _handleInvalidationFromConnection:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _performXPCTransaction:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: _registerForLaunchNotification
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: applyAgentPropertySnapshots:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: hideApplicationResponse:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: makeKeyWindowWithWindowIdentifier:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: proposeKeyAndMainApplicationResponse:mainWindow:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: resignKeyWindowWithWindowIdentifier:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: stageFrames
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: unhideApplicationResponse:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: windowDeminiaturizationResponse:
2024-03-01 13:22:01.216 yabai[93295:664422] WMClientWindowManager method: windowMiniaturizationResponse:

After calling NSApplicationLoad, getting the WMBridge instance and dumping it results in:

2024-03-01 13:23:53.143 yabai[93467:665277] DUMP::NSWMWindowCoordinator
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator ivar: _transaction
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator ivar: _windowManager
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator property: hash
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator property: superclass
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator property: description
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator property: debugDescription
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: init
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: .cxx_destruct
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: applicationDidCancelTermination
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: applicationDidFinishRestoration
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: applicationWillBeginTermination
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: _finishLoginTransition
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:hideApplicationResponse:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:requestMakeKeyWindowWithIdentifier:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:requestResignKeyWindowWithIdentifier:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:unhideApplicationResponse:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:updateStages:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:updateStages:displayChangeSeed:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:windowDeminiaturizationResponse:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: clientWindowManager:windowMiniaturizationResponse:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: finishLoginTransitionWithCompletionHandler:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: handleDesktopBackgroundClickEnd:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: initializeStageFramesIfNeeded
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: performAsynchronousBridgedWindowManagementOperation:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: performCycleWindowsReversed:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: performSynchronousBridgedWindowManagementOperation:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: performTransactionUsingBlock:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: performWindowManagementBridgeTransactionUsingBlock:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: proposeKeyWindow:previousWindow:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: proposeMainWindow:previousWindow:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: setWindowTags:onWindow:clear:
2024-03-01 13:23:53.143 yabai[93467:665277] NSWMWindowCoordinator method: startLoginTransition
2024-03-01 13:23:53.143 yabai[93467:665277] DUMP::nil

Calling SLSWindowManagementBridgeSetDelegate(NULL); and then getting the WMBridge again, results in the exact same bridge as before NSApplicationLoad was called. So it probably resets the delegate back to the fallback version, but additionally sets some flag to indicate that the bridge is not to be used.

This flag is then checked internally by the windowserver before it attempts to perform some operation, deciding whether it should use a bridged operation or the "old" way.

koekeishiya commented 7 months ago

I noticed that there is code inside SLSNewWindow that actually checks whether the specific window to be created should use the WMBridge or not. By default, SLSNewWindow will enable it, but I can actually opt out for specific windows using extern CGError SLSNewWindowWithOpaqueShapeAndContext(int cid, int type, CFTypeRef region, CFTypeRef opaque_shape, int options, uint64_t *tags, float x, float y, int tag_size, uint32_t *wid, void *context); and by making sure that the 18th bit is set in the options argument.

# these are basically equivalent; SLSNewWindow passes the integer value 13 as options instead.
SLSNewWindow(animation_connection, 2, 0, 0, frame_region, &proxy->id);

# same as above; but we also set the 18th bit.
SLSNewWindowWithOpaqueShapeAndContext(animation_connection, 2, frame_region, empty_region, 13|(1 << 18), &tags, 0, 0, 64, &proxy->id, NULL);

This can be used instead of disabling the entire WMBridge, however, the window still won't respond to order-operations from JankyBorders.

Edit: There is also a different code path inside SLSNewWindowWithOpaqueShapeAndContext where it checks some bool result of some CFPreferencesCopyValue, operation, skipping the WMBridge setup in a different way than the 18th bit version I mentioned above. Have not looked further into which kind of preference property this is.

FelixKratz commented 7 months ago

These are really nice results. Thanks for sharing them.

What I find interesting is that the borders can be ordered below and on top of the window with the (unsupported and undocumented) option:

borders order=above

and the default ordering

borders order=below

Now, I can try to make the proxies order below the window by putting into payload.m l809 (and leaving the proxied window visible)

    SLSTransactionOrderWindowGroup(transaction, proxy_wid, -1, wid);
    // SLSTransactionSetWindowSystemAlpha(transaction, wid, 0);

but still, they appear above the actual window. So it seems that the ordering of the proxy window even doesn't work from yabai itself (rather Dock), where it does work in JB (with WMBridgeDelegate = NULL).

Determining the difference might bring us closer to understanding this problem. Maybe it has to do with the connection used to create the windows?

koekeishiya commented 7 months ago

I noticed the same thing with yabai proxy windows vs yabai "insert feedback" windows. On the latest master I've switched to the window-specific removal of WMBridge. In this version the "insert feedback" windows now order correctly in relation to other windows, but the proxy windows still retain the original problem.

I haven't looked further into this since my last comment, however now the only difference is the call to create the specific windows, and not some global state that has been modified in yabai itself.