Integrate with yabai window animations #62
Comments
|
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). 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. |
|
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
Maybe this should be the very first approach, by listening to create and destroy events for proxy windows. If this version works we could investigate further modifications to allow for actual animation of borders as well. |
|
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? |
|
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. Calling I think the most straightforward solution would be the following (but also might suffer from performance issues due to many lookups):
It is likely faster to do a single Edit: |
|
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: |
|
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 koekeishiya/yabai@b91f95e and the JankyBorders POC patch is in my latest comment in this issue #62 (comment) |
|
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. |
|
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. |
|
Right, so the current/new yabai master seems to work quite well with the following JankyBorders patch applied to commit 5e7fa9d (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); |
|
I have included your suggested improvements on master and fixed another edge case, so I think it works very reliably now. |
|
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. |
|
I have an animated solution poc here: 37183f4 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. |
|
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. |
|
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. |
|
It is possible to further improve the performance of the border->proxy tracking by artificially stalling the frame_callback function (10fea2a). 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. |
|
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. |
|
This commit should fix the final quirks with intercepted animations: d1010ff 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. |
|
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) |
|
Latest yabai master should support this now (koekeishiya/yabai@7535e3a). 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. |
|
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. |
|
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. |
|
Just a note that I had to add some simple JankyBorders detection to fix mouse-drag integration when borders are enabled: koekeishiya/yabai@fdf44f3 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. |
Ok, give me a headsup once you change it, then I will change it too, will revert this for now then: |
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. |
|
Great, did a release as well. It is really nice to have this properly integrated now. |
|
It sort of works if you just do |
|
I think I have an acceptable fix for this problem. Will try to iron out some quirks and then post it. |
|
Ok so what I am basically doing to fix this problem is the following:
|
|
After reading your comment I had an idea, and it appears to work. I create a window in yabai that specifies that it wants WMBridge disabled using the 1 << 18 solution. I then create a window in my small test program that also disables WMBridge in the same way. In my small test program I can now correctly order all these windows relative to the yabai window. |
|
First run shows with WMBridge enabled -- doesn't work. Toggle WMBridge disabled -- works fine.
#include <Carbon/Carbon.h>
#include <stdint.h>
extern CGError CGSNewRegionWithRect(CGRect *rect, CFTypeRef *region);
extern CFTypeRef CGRegionCreateEmptyRegion(void);
extern int SLSMainConnectionID(void);
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);
extern CGError SLSReleaseWindow(int cid, uint32_t wid);
extern CGError SLSOrderWindow(int cid, uint32_t wid, int mode, uint32_t rel_wid);
extern CGError SLSSetWindowLevel(int cid, uint32_t wid, int level);
extern CGError SLSSetWindowSubLevel(int cid, uint32_t wid, int level);
extern int SLSGetSpaceManagementMode(int cid);
uint32_t create_window(int cid, float x, float y, float width, float height, bool disable_wm_bridge)
{
CFTypeRef frame_region;
CGRect frame = {{x, y}, {width, height}};
CGSNewRegionWithRect(&frame, &frame_region);
CFTypeRef empty_region = CGRegionCreateEmptyRegion();
uint32_t wid;
uint64_t tags = 0;
int options = 13 | ((disable_wm_bridge ? 1 : 0) << 18);
SLSNewWindowWithOpaqueShapeAndContext(cid, 2, frame_region, empty_region, options, &tags, 0, 0, 64, &wid, NULL);
SLSSetWindowLevel(cid, wid, 0);
SLSSetWindowSubLevel(cid, wid, -20);
CFRelease(frame_region);
CFRelease(empty_region);
return wid;
}
int main(int argc, char **argv)
{
int cid = SLSMainConnectionID();
int mode = SLSGetSpaceManagementMode(cid);
uint32_t a = create_window(cid, 100, 250, 100, 100, true);
uint32_t b = create_window(cid, 150, 250, 100, 100, true);
printf("ordering above rel_wid 0\n");
sleep(2);
SLSOrderWindow(cid, a, 1, 0);
SLSOrderWindow(cid, b, 1, a);
printf("ordering below rel_wid yabai_insert\n");
sleep(2);
SLSOrderWindow(cid, a, -1, 54177);
SLSOrderWindow(cid, b, -1, 54177);
printf("ordering above rel_wid yabai_insert\n");
sleep(2);
SLSOrderWindow(cid, a, 1, 54177);
SLSOrderWindow(cid, b, 1, 54177);
CFRunLoopRun();
return 0;
}Edit: I guess unmanaged windows cannot be ordered on top of or below managed windows, so this won't actually work for you, since JankyBorders need to be able to do order above/below both, which is why you re-create the window in your solution. I guess the conclusion is that all managed windows can only order relative to other managed windows, where as all unmanaged windows can order relative to unmanaged windows. |
|
Of course I am encountering the same problems as you did in yabai, where switching a managed window with another window is not possible atomically, such that I need to do some janky sleep calls, but thats fine I think. I agree with your conclusion. I had almost given up on this. Very nice to have figured it out in the end. Cheers! |
|
In yabai I do an atomic swap between a managed window and an unmanaged window. The problem for me was an atomic swap between two managed windows. You should be able to do an atomic swap between the managed border and the unmanaged border. They will have to coexist for some time though. To solve that in yabai I modify the system alpha of the managed window and do the ordering on the unmanaged window, in the same transaction. |
|
You are right, I should be able to do the same. I will just keep the managed border around and make it in invisible instead of destroying and then recreating it. |
|
I think a7b4ff7 smoothes the final edges of the new window animation integration: current.mp4The tracking of the yabai window proxy seems to be frame perfect and swapping the border window (managed->unmanaged and vice versa) does not lead to severe flickering anymore. There is one super minor detail where for 1 frame the unmanaged border is still ordered on top of the managed (real) window while the yabai proxy window is already made invisible. Maybe this could be fixed by notifying JB of the proxy end before the yabai proxy is ordered out, but I am not sure and its a really minor detail. |
|
This patch for yabai makes the integration even better: diff --git a/src/window_manager.c b/src/window_manager.c
index 1670a95..d70a088 100644
--- a/src/window_manager.c
+++ b/src/window_manager.c
@@ -411,7 +411,7 @@ void window_manager_resize_window(struct window *window, float width, float heig
CFRelease(size_ref);
}
-static inline void window_manager_notify_jankyborders(uint32_t proxy_wid, uint32_t real_wid, uint32_t proxy_event)
+static inline void window_manager_notify_jankyborders(uint32_t proxy_wid, uint32_t real_wid, uint32_t proxy_event, bool wait)
{
mach_port_t port;
if (g_bs_port && bootstrap_look_up(g_bs_port, "git.felix.jbevent", &port) == KERN_SUCCESS) {
@@ -422,6 +422,7 @@ static inline void window_manager_notify_jankyborders(uint32_t proxy_wid, uint32
uint32_t real_wid;
} data = { proxy_event, proxy_wid, window_space(proxy_wid), real_wid };
mach_send(port, &data, sizeof(data));
+ if (wait) usleep(20000);
}
}
@@ -464,7 +465,6 @@ static void window_manager_destroy_window_proxy(int animation_connection, struct
}
if (proxy->id) {
- window_manager_notify_jankyborders(proxy->id, real_wid, 1326);
SLSReleaseWindow(animation_connection, proxy->id);
proxy->id = 0;
}
@@ -495,7 +495,7 @@ static void *window_manager_build_window_proxy_thread_proc(void *data)
}
window_manager_create_window_proxy(animation->cid, alpha, &animation->proxy);
- window_manager_notify_jankyborders(animation->proxy.id, animation->wid, 1325);
+ window_manager_notify_jankyborders(animation->proxy.id, animation->wid, 1325, false);
scripting_addition_swap_window_proxy_in(animation->wid, animation->proxy.id);
dispatch_async(dispatch_get_main_queue(), ^{
if (animation->window && animation->window->id != 0 && __sync_bool_compare_and_swap(&animation->window->id_ptr, &animation->window->id, &animation->window->id)) {
@@ -553,6 +553,7 @@ static CVReturn window_manager_animate_window_list_thread_proc(CVDisplayLinkRef
for (int i = 0; i < animation_count; ++i) {
if (__atomic_load_n(&context->animation_list[i].skip, __ATOMIC_RELAXED)) continue;
+ window_manager_notify_jankyborders(context->animation_list[i].proxy.id, context->animation_list[i].wid, 1326, true);
scripting_addition_swap_window_proxy_out(context->animation_list[i].wid, context->animation_list[i].proxy.id);
table_remove(&g_window_manager.window_animations_table, &context->animation_list[i].wid);
window_manager_destroy_window_proxy(context->animation_connection, &context->animation_list[i].proxy, context->animation_list[i].wid);
@@ -632,7 +633,7 @@ void window_manager_animate_window_list_async(struct window_capture *window_list
float alpha = 1.0f;
SLSGetWindowAlpha(context->animation_connection, context->animation_list[i].wid, &alpha);
window_manager_create_window_proxy(context->animation_connection, alpha, &context->animation_list[i].proxy);
- window_manager_notify_jankyborders(context->animation_list[i].proxy.id, context->animation_list[i].wid, 1325);
+ window_manager_notify_jankyborders(context->animation_list[i].proxy.id, context->animation_list[i].wid, 1325, false);
CFTypeRef transaction = SLSTransactionCreate(context->animation_connection);
SLSTransactionOrderWindowGroup(transaction, context->animation_list[i].proxy.id, 1, context->animation_list[i].wid);
@@ -642,6 +643,7 @@ void window_manager_animate_window_list_async(struct window_capture *window_list
table_remove(&g_window_manager.window_animations_table, &context->animation_list[i].wid);
window_manager_destroy_window_proxy(existing_animation->cid, &existing_animation->proxy, existing_animation->wid);
+ window_manager_notify_jankyborders(existing_animation->proxy.id, existing_animation->wid, 1326, false);
} else {
pthread_t thread;
if (pthread_create(&thread, NULL, &window_manager_build_window_proxy_thread_proc, &context->animation_list[i]) == 0) {EDIT: Updated the patch to fix some remaining flicker by waiting after the mach_send call on proxy -> real window swap. |
|
Added koekeishiya/yabai@b8a4d84 |
|
There is, sometimes, something weird happening with the borders at the start (first 1 or 2 frames) of the animation only when I have background_color and blur enabled. It looks like it thinks the window is of a different size so the background/backdrop doesn't cover the entire area of the window it should follow. Edit: Only for 1-2 frames of the start of a new animation. Re-animating/Intercepting animations work fine. Edit: This is very noticable if I have 4 terminal windows in a square and use |
|
I tried to reproduce this but I didn't manage to: rotate.mp4I am not sure if this is related but I found that I was receiving proxy begin and proxy end messages in pairs for some operations (both the begin and end message come in very quick succession) for the same real wid and different proxy wids. Almost as if there where duplicates in the window animation list. For example this for a fullscreen-zoom operation: Proxy Begin: real_wid=80415 proxy_wid=80495
Proxy Begin: real_wid=80415 proxy_wid=80497
... animation plays...
Proxy End: real_wid=80415 proxy_wid=80495
Proxy End: real_wid=80415 proxy_wid=80497Apart from that, I think rendering the border proxy should maybe be done in a thread to not block the messaging pipeline for the entire time. This would likely reduce the initial lag when many windows are animated at the same time, e.g. in a space rotate operation. |
That is probably this part of the code. I did change the order, but maybe I shouldn't have done so. The 2nd notify used to be called after the swap had happened. Should I move it back down? |
|
I dont think this should make a difference. I had the impression that these pairs of messages do not originate from an intercept, but I will have a closer look. |
|
I just played a bit more with this and the pairs of proxy begin and proxy end events do indeed arise from an animation intercept, so thats all fine. However, I wonder why there is an instant intercept happening for the command yabai -m window --toggle zoom-parentwhen the window zooms to the parent node (doing nothing else but the above command) it produces Proxy Begin: real_wid=80415 proxy_wid=80495
Proxy Begin: real_wid=80415 proxy_wid=80497
Proxy End: real_wid=80415 proxy_wid=80495
... animation plays...
Proxy End: real_wid=80415 proxy_wid=80497where the three first events are delivered immediately after issuing the above command. The same does not happen on the above command when the parent-zoom is toggled off. |
|
Probably some bug in |
|
It might actually be that the same window gets flushed through multiple nodes, not that the capture itself that I referenced above is broken. I added some timing info yesterday and noticed that some user-initiated commands triggered the root animate function twice. Build with make CLI_FLAGS=-DPROFILE for this output. |
I was now able to reproduce this. Has probably to do with the fact that the system resize event can "skip" the mach message queue because I call |
|
I tried to thread all the time consuming things such that the handling of multiple yabai proxy windows should feel a lot more responsive 3d238c1 |
|
It appears to work quite well now from what I can tell. |
Thanks for catching this. It was an issue after implementing koekeishiya/yabai#2117 |
|
With the current implementation I am still seeing some crashes in the SLSWMBridge. I will have to do some more investigations: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libobjc.A.dylib 0x1808f7fe8 objc_release + 8
1 WindowManagement 0x219be3840 -[WMWindowPropertySnapshot initWithWindowIdentifier:] + 96
2 WindowManagement 0x219be8d30 -[WMClientWindowManager _createWindowPropertySnapshotForWindow:properties:] + 116
3 WindowManagement 0x219be5228 -[WMClientWindowManager performWindowTransaction:] + 780
4 SkyLight 0x186781544 -[SLSWindowManagementFallbackBridge performWindowManagementBridgeTransactionUsingBlock:] + 92
5 SkyLight 0x18678139c SLSWMBridgePerformTransaction(void (WMWindowTransaction*, CAFenceHandle*) block_pointer) + 60
6 SkyLight 0x1867811e8 -[SLSWMBridgedWindow window:didUpdateWithChangedProperties:] + 136
7 WindowManagement 0x219be2114 -[_WMWindow performUpdatesUsingBlock:] + 116
8 WindowManagement 0x219be096c -[_WMWindow setCGWindowID:] + 112
9 SkyLight 0x1867818cc SLSWMDeferToMainRunLoopAsync(bool, void () block_pointer) + 92
10 SkyLight 0x186782c64 -[SLSWMBridgedWindow initWithWindowID:] + 184
11 SkyLight 0x1866a47d8 CGSWindowConstructInternal + 564
12 SkyLight 0x186494ea8 SLSNewWindowWithOpaqueShapeAndContext + 984
13 SkyLight 0x186494a84 SLSNewWindow + 132
14 borders 0x1041e2170 border_create_window + 256
15 borders 0x1041e2a80 border_update_internal + 1248
16 borders 0x1041e2588 border_update + 136
17 borders 0x1041e0bb8 windows_window_create + 428
18 borders 0x1041e09b8 window_spawn_handler + 60 |
|
The crashes seem to be under control now, apparently it is not a good idea to fiddle with the event port of the application. Did some further improvements to reduce/eliminate any further flickering/ wrong border size. Edit: Just seen a few more of those... will keep looking |
|
Latest master should not have any remaining problems as far as I can tell. I did some work to ensure that flickering of the border is less of a problem (by artificially coalescing move, resize and proxy events when possible). I think I found the reason for the crashes documented above and that should also be fixed. |
|
@FelixKratz Released v7.0.0 just now. |
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.
The text was updated successfully, but these errors were encountered: