Fix on-window-detected floating rule causing brief tiling flash#2024
Open
willzeng274 wants to merge 1 commit intonikitabobko:mainfrom
Open
Fix on-window-detected floating rule causing brief tiling flash#2024willzeng274 wants to merge 1 commit intonikitabobko:mainfrom
willzeng274 wants to merge 1 commit intonikitabobko:mainfrom
Conversation
064f643 to
4e99854
Compare
When exec-and-forget launches an app, multiple notifications fire rapidly (kAXWindowCreatedNotification, didActivateApplicationNotification, etc.), each calling scheduleRefreshSession(). Due to Swift cooperative cancellation, a session already running past checkCancellation() continues executing even after being "cancelled" by a newer session. Two sessions end up interleaved on @mainactor at every await suspension point. Race 1 (tiling flash): Session A places the new window in the tiling container (synchronous), then suspends inside tryOnWindowDetected() while evaluating rules via async AX calls. Session B gets CPU, runs layoutWorkspaces(), sees the window in the tiling container, and calls setAxFrame() -- producing a visible flash at the tiled position. Race 2 (sibling shift): layoutTiles() distributes tile space among all children by weight. While the new window sits in the tiling container awaiting rule evaluation, every concurrent layoutWorkspaces() call shrinks sibling windows to reserve a tile slot for it -- a slot that is never visually filled -- causing siblings to briefly shift. Fix: add isAwaitingOnWindowDetected flag to Window, set when the window enters allWindowsMap, cleared via defer after tryOnWindowDetected completes (covers both the normal path and the closedWindowsCache restore path). layoutWorkspaces() skips flagged windows entirely so no concurrent session can setAxFrame() the window while its rules are still being evaluated. layoutTiles() also excludes flagged windows from space allocation so siblings keep their full tile width during detection. Session A's own layoutWorkspaces() runs after the flag is cleared and positions the window correctly in its final container the first time. nikitabobko#2016
4e99854 to
b41287a
Compare
Owner
|
I didn't manage to reproduce the bug. But I believe that it could be theoretically happening. I am not sure that I like the idea of introducing an additional boolean flag. This flickering is not the only problem that is caused by sessions interleaving (ref: #1311) Question: does the following patch fix the issue? Detailsdiff --git a/Sources/AppBundle/layout/layoutRecursive.swift b/Sources/AppBundle/layout/layoutRecursive.swift
index bc005b9b..f718a735 100644
--- a/Sources/AppBundle/layout/layoutRecursive.swift
+++ b/Sources/AppBundle/layout/layoutRecursive.swift
@@ -4,6 +4,7 @@ extension Workspace {
@MainActor
func layoutWorkspace() async throws {
if isEffectivelyEmpty { return }
+ try checkCancellation()
let rect = workspaceMonitor.visibleRectPaddedByOuterGaps
// If monitors are aligned vertically and the monitor below has smaller width, then macOS may not allow the
// window on the upper monitor to take full width. rect.height - 1 resolves this problem
@@ -15,6 +16,7 @@ extension Workspace {
extension TreeNode {
@MainActor
fileprivate func layoutRecursive(_ point: CGPoint, width: CGFloat, height: CGFloat, virtual: Rect, _ context: LayoutContext) async throws {
+ try checkCancellation()
let physicalRect = Rect(topLeftX: point.x, topLeftY: point.y, width: width, height: height)
switch nodeCases {
case .workspace(let workspace):
@@ -68,6 +70,7 @@ private struct LayoutContext {
extension Window {
@MainActor
fileprivate func layoutFloatingWindow(_ context: LayoutContext) async throws {
+ try checkCancellation()
let workspace = context.workspace
let windowRect = try await getAxRect() // Probably not idempotent
let currentMonitor = windowRect?.center.monitorApproximation
@@ -95,6 +98,7 @@ extension Window {
@MainActor
fileprivate func layoutFullscreen(_ context: LayoutContext) {
+ try checkCancellation()
let monitorRect = noOuterGapsInFullscreen
? context.workspace.workspaceMonitor.visibleRect
: context.workspace.workspaceMonitor.visibleRectPaddedByOuterGaps
@@ -105,6 +109,7 @@ extension Window {
extension TilingContainer {
@MainActor
fileprivate func layoutTiles(_ point: CGPoint, width: CGFloat, height: CGFloat, virtual: Rect, _ context: LayoutContext) async throws {
+ try checkCancellation()
var point = point
var virtualPoint = virtual.topLeftCorner
@@ -140,6 +145,7 @@ extension TilingContainer {
@MainActor
fileprivate func layoutAccordion(_ point: CGPoint, width: CGFloat, height: CGFloat, virtual: Rect, _ context: LayoutContext) async throws {
+ try checkCancellation()
guard let mruIndex: Int = mostRecentChild?.ownIndex else { return }
for (index, child) in children.enumerated() {
let padding = CGFloat(config.accordionPadding) |
Author
|
hmm... for some reason I can't reproduce the issue anymore either, not sure what caused it tbh, I will report if I encounter the bug again. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two related visual glitches when an
on-window-detectedrule applieslayout floating:See the commit message for root cause and fix details.
Related discussion: #2016
Summary: