fix: improve z-order handling for BelowWindow mode and prevent ghost borders#66
Conversation
|
Addressed Issue #62 |
| return LRESULT(0); | ||
| } | ||
|
|
||
| // Check if tracking window still exists to avoid ghost borders |
There was a problem hiding this comment.
I don't doubt this fixes things, but it's kind of a band-aid solution because we already run the same logic here which suggests there's another underlying issue that causes the polling thread to fail. Plus, this solution might not work 100% of the time because it relies on specific window messages which may or may not get posted. Can you try if this artifact also fixes the issue 🙏: https://github.com/lukeyou05/tacky-borders/actions/runs/21192920265.
There was a problem hiding this comment.
so it's a kind of hit-or-miss situation, i'll look into this and check for any solution. In the meantime I'll test your artifact and will post here about it once done.
There was a problem hiding this comment.
I have tested your artifact, and the issue still persists as shown in the screenshot below
Configuration - border_z_order: AboveWindow
I have faced the exact issue as per the latest video message from the issue here
There was a problem hiding this comment.
Crucial Update: You were completely right about GlazeWM's fault, the black window was indeed, caused by it and not tacky-borders and I have been approaching it the wrong way the whole time. Your first artifact provided here has already fixed those issues.
The only remaining issue to address is the BelowWindow border hiding behind background window which occurs without GlazeWM, confirming its the application issue itself.
There was a problem hiding this comment.
Wait, but does your PR fix the black window issues? If so, there should still be something we can do within tacky-borders to fix it. I'd just need to figure out why the polling thread doesn't do it.
There was a problem hiding this comment.
sadly no, have tested different iterations and literally went overboard and tested it with glazewm integration which handles GlazeWM IPC within tacky borders for possible fixes but as said earlier, it didn't.
I have talked about the black window issue in detail in below comments
| self.update_position(None).log_if_err(); | ||
| } | ||
| } | ||
| ZOrderMode::BelowWindow => { |
There was a problem hiding this comment.
I was hesitant to do this because it's this very piece of code that caused issues like #23, #28, and #59 with AboveWindow. It can lead to a feedback loop if other windows also try to get the exact same Z-order spot (though it might be less likely with BelowWindow). But if I can't think of any other solutions then I might just add this anyways.
There was a problem hiding this comment.
i'll try my best in finding an alternate solution for this too, to ensure proper window handling.
There was a problem hiding this comment.
I did some debugging and it seems like EVENT_OBJECT_REORDER is the ONLY event sent when clicking off a nested child window onto the parent window, meaning your solution might be the only way to fix this without polling. I did however try filtering the event which may help prevent any potential flicker issues (doubtful)? Can you try your PR but replace this match arm with the below code and see if everything still works fine:
EVENT_OBJECT_REORDER => {
// Ignore OBJIDs not needed for window Z-order handling.
if _id_object != OBJID_CLIENT.0 {
return;
}
// Send reorder messages to all the border windows
for value in APP_STATE.borders.lock().unwrap().values() {
let border_window = HWND(*value as _);
if is_window_visible(border_window) {
post_message_w(Some(border_window), WM_APP_REORDER, WPARAM(0), LPARAM(0))
.context("EVENT_OBJECT_REORDER")
.log_if_err();
}
}
}There was a problem hiding this comment.
Will try this in morning, currently 2am and away from my setup 👍
There was a problem hiding this comment.
@lukeyou05 I have tested your code snippet after reverting back to the base commit of this pr, and shall confirm that, it has somehow fixed both the border lingering and border hiding issues for me, with GlazeWM but didn't fix the window trace issue which doesn't occur when running tacky-borders without GlazeWM, happens in both cases.
Video Clips
Without GlazeWM:
AboveWindow
AboveWindow.-.Without.GlazeWM.mp4
BelowWindow
BelowWindow.-.Without.GlazeWM.mp4
With GlazeWM:
AboveWindow
AboveWindow.-.With.GlazeWM.mp4
BelowWindow
BelowWindow.-.With.GlazeWM.mp4
As you can see in the clips, there's no flickering borders and other related issues you have cited earlier (specifically #23, #28, #59) and also the window trace issue that only appears with GlazeWM for both AboveWindow and BelowWindow, which should either be resolved by GlazeWM integration like Komorebi but for better window handling (which I have tried and failed) or take this up to GlazeWM to fix window handling from their side. As mentioned earlier, this has completely fixed the border_z_bugs that I was dealing with except for the window trace.
Also a note, the window trace issue happens without tacky-borders too, this make me 100% sure now the window trace issue is to be fixed in GlazeWM and not tacky-border.
A brief about the window trace issue with GlazeWM, quoted from comments:
Okay, while I was working on GlazeWM integration for tacky-borders, I have found that it just generally plays poor with it. The black window appearance I was talking about last time, I just found that it is a trace that GlazeWM leaves behind which is the current inactive window border color (The window is treated as inactive by tacky-borders and glazewm, due to poor handling, leave the border color trace in the size of the window that was closed).
The explanation provided in brackets is all just my assumption and might be wrong but that's what I believe might be the reason. I'm now almost sure that it's GlazeWM which is generally poor in handling with tacky-borders. I better take it up there and start working from that perspective. Tried my best.
There was a problem hiding this comment.
Well, I'm kind of motivated enough to do this project rather than focus on my academics project which I don't have interest in 😅 but still have to lock in for the marks. So decided to complete this by tomorrow, hope you focus on this for today.
As for the solution, the smart debouncing is added as a backup just to make sure it addresses all the issues I have faced related to border_z_order. In my case, it works pretty well. But still will try your solution too once I get time, gonna take a break from this for today to focus on mine.
There was a problem hiding this comment.
I have tested the new branch as per your request and I had the same behavior. I guess everything's sorted out. But, I have noticed something interesting.
As the screenshot shows, the border gets thinner in the title bar area but normal in the other sides. This behavior is noticed only for windows with a title-bar and only when I exit GlazeWM while keeping the said windows open.
This happens in the release build too, pretty sure it should be from GlazeWM side (maybe)
I guess this is fine to leave for now as people wouldn't notice it for most cases. This shall be addressed in the next update or a new PR. For now, I think this is good enough. Hope it's finally done :)
There was a problem hiding this comment.
By "same behavior", I assume you mean the debounce_reorders branch works correctly? I think we're basically good to go then but can you first revert your smart debouncing commit? I'll merge your PR and my branch after that.
As for the new issue you posted, that just looks like GlazeWM is hiding the native borders via DWMWA_BORDER_COLOR upon exit for some reason. The fix for that might be as simple as changing this from DWMWA_COLOR_NONE to DWMWA_COLOR_DEFAULT, but that doesn't address the underlying issue of why it's running that code when it doesn't need to, but I digress as that's something for GlazeWM's maintainers to look at.
There was a problem hiding this comment.
Well woke up from bed after hearing the notification, yes I'll revert back the smart debouncing commit in a few and as for the "same behavior", your assumption is right
As for the GlazeWM issue, I'll look into it some other day.
There was a problem hiding this comment.
@lukeyou05 reverted the commit successfully now, check please 👍
Louis047
force-pushed
the
fix/z-order-improvements
branch
2 times, most recently
from
58f7f9b to
abdebb1
Compare
|
Addressing the above issues after some testing, I have found that border_z_order: I'll start working on that now and see how things move now. Other border issues like the lingering one is fixed in both cases (running normally and running with GlazeWM), except for the black window appearance but that's left for GlazeWM, not tacky-borders. |
|
Okay, while I was working on GlazeWM integration for tacky-borders, I have found that it just generally plays poor with it. The black window appearance I was talking about last time, I just found that it is a trace that GlazeWM leaves behind which is the current inactive window border color (The window is treated as inactive by tacky-borders and glazewm, due to poor handling, leave the border color trace in the size of the window that was closed). The explanation provided in brackets is all just my assumption and might be wrong but that's what I believe might be the reason. I'm now almost sure that it's GlazeWM which is generally poor in handling with tacky-borders. I better take it up there and start working from that perspective. Tried my best. |
|
Well, again, I was completely wrong. Just found out that the |
|
I'll explain the issue better now: After focusing on the background window or a random window then focusing back to the nested child window, the window draw is proper. But when I focus on the parent nested window to the child, the border draw issue appears given that i have focused on the background window or a random window then back to the parent nested window instead of child. The key here is the child window not coordinating with the parent window when parent window is focused first after shifting focus from the background window to it. |
|
Tested with GlazeWM again, it seems that the issue is more prominent with it. But most of the cases related to border hiding are solved when GlazeWM is not used, which means most of the apps work well with tacky-borders without GlazeWM and when enabled, the issue becomes more visible for more apps and not just limited to system apps like it does for when GlazeWM is disabled. I know its a very long explanation and work but giving more details might provide leads to fix the niche issue which might impact the window manager experience for windows with this tool used. |
|
Well, guess what, the behavior is kind of random at this point but now that I'm 100% sure, GlazeWM is the main cause to not play well with tacky-borders, so I have to take it to there probably. Already dead tired in working on this pretty much the whole day lmao. Hope you look into this and I have given as much detail as possible really showcase the root cause. |
|
Thanks for helping debug the issue. I think there's still some things we can do from tacky-borders's side to fix this so I've left a few more comments on the review. |
|
I have left some comments in the review now, and seems like we are almost there @lukeyou05 |
…borders - Add z-order correction logic for BelowWindow mode in WM_APP_REORDER handler using GW_HWNDNEXT to detect when border drifts out of position - Add tracking window validity checks in WM_APP_LOCATIONCHANGE, WM_APP_REORDER, and WM_APP_FOREGROUND handlers to immediately cleanup borders when tracking window is destroyed - Prevents ghost borders from lingering after window close - Fixes nested window borders going behind background windows in BelowWindow mode
Louis047
force-pushed
the
fix/z-order-improvements
branch
from
4e1155d to
b7f4169
Compare
|
I left another comment in the review as well |
Only debounce when z-order is already correct. If z-order needs fixing, always process immediately regardless of timing. This prevents border flickering from rapid events while ensuring BelowWindow mode still works.
Louis047
force-pushed
the
fix/z-order-improvements
branch
from
346a77f to
65e92ef
Compare
|
Made new commits and comments to check on, I think we are almost there now👍 |
|
Update: For the window trace issue, I went overboard again and implemented a fix for it in GlazeWM repository. Tested it and it worked successfully. The behavior was only seen with ShareX but I'm pretty sure it would've fixed for nested floating windows as a whole. Also make sure to see the review and commits, we are almost set I guess. (I want it to be completed by at least this week as this was running in my mind the whole time instead of focusing on my academic project for which the deadline is next week lol) |
|
Couldn't help myself and worked on this before my bedtime 😄 , left some comments in the review section. Found something interesting. Also a request, after this PR merge I feel that most of the crucial issues are addressed and is ready for a release, hopefully. If you have any other issues to be addressed, do let me know 👍 (I want this to be released so bad lol) |
|
I think we're about good to go 🙏 but can you first revert your smart debouncing commit? I'll merge everything and create a new release afterwards. Also see the note I left about the new issue near the titlebars, could be a pretty simple fix. |
…ering" This reverts commit 65e92ef.
2 participants
Summary
This PR addresses two z-order related issues:
1. BelowWindow Z-Order Fix
When border_z_order is set to BelowWindow, nested window borders would incorrectly go behind background windows when focus changed.
Fix: Added z-order correction logic in the WM_APP_REORDER handler using GW_HWNDNEXT to detect when the border drifts out of position and reposition it correctly.
2. Ghost Border Prevention
Ghost borders (border outlines without window content) would linger after closing windows.
Fix: Added is_window() validity checks in three message handlers:
These checks immediately cleanup the border if the tracking window no longer exists.
Changes
Testing