[macOS] Issues with window.is_maximized()

[landaire]

Description

I apologize for the poor title, but I'm not quite sure how to phrase this as the bug originates in the relationship between egui and winit, and I believe the root cause lies on the winit side.

While investigating a reported deadlock in emilk/egui#3494 involving fetching window.is_maximized() at runtime I found that its implementation can impact egui's ability to make forward progress on rendering.

The call stack for it can be found here: https://gist.github.com/landaire/bf648a199411a4a813cb7e1419bbc4d9

This isn't actually a deadlock, but it looks like in is_zoomed() we change the window style which causes egui to receive consistent redraw requests. I do not have good knowledge of egui, or really any rendering for that fact, but I think the style change somehow prevents us from making significant rendering progress.

This prevents the issue from happening:

diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs
index 19dc605e..3691ab88 100644
--- a/src/platform_impl/macos/window_delegate.rs
+++ b/src/platform_impl/macos/window_delegate.rs
@@ -1191,14 +1191,14 @@ impl WindowDelegate {
         let required = NSWindowStyleMask::Titled | NSWindowStyleMask::Resizable;
         let needs_temp_mask = !curr_mask.contains(required);
         if needs_temp_mask {
-            self.set_style_mask(required);
+            // self.set_style_mask(required);
         }
 
         let is_zoomed = self.window().isZoomed();
 
         // Roll back temp styles
         if needs_temp_mask {
-            self.set_style_mask(curr_mask);
+            // self.set_style_mask(curr_mask);
         }
 
         is_zoomed

A candidate fix for winit could be the following:

diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs
index 19dc605e..ba6004ae 100644
--- a/src/platform_impl/macos/window_delegate.rs
+++ b/src/platform_impl/macos/window_delegate.rs
@@ -1288,7 +1288,7 @@ impl WindowDelegate {
 
     #[inline]
     pub fn is_maximized(&self) -> bool {
-        self.is_zoomed()
+        self.ivars().maximized.get()
     }
 
     #[inline]

But I don't know if there are scenarios where the ivar does not match what is returned from is_zoomed(). But it looks like it's set on window_will_enter_fullscreen and set_maximized which I think should keep it in sync?

I can't really explain why the window styling change prevents egui from making forward progress either, but the behavior is very obvious in a debugger. If you set a breakpoint in update_viewport_info you'll notice it's called consistently. However, if you take either of the above patches you'll notice that it's only called a handful of times (egui is running in the background, so it normally shouldn't be redrawing when it doesn't have focus I believe).

macOS version

ProductName:            macOS
ProductVersion:         15.1
BuildVersion:           24B83

Winit version

0.30.7

[landaire]

I should add that winit is quite possibly doing the correct thing here, but since it seems like mutating the window style can be avoided and is triggering theissue I thought it was worthwhile to file this.

[madsmtm]

Great catch!

I generally feel that our fullscreen support is really cursed (which is easy for me to say, 'cause I'm not the one that wrote it ;) ), so I'm fine with considering this a bug in Winit. I cannot tell you if your proposed fix would break something else though, so I'm weary to change it ;/.

Another option (not sure which is best, just noting it down for reference) is to re-implement parts of the algorithm described in isZoomed.

[landaire]

I cannot tell you if your proposed fix would break something else though, so I'm weary to change it ;/.

I don't think the patch which changes the window style is an appropriate fix, but wanted to include it to demonstrate that removing the set_style_mask call caused the egui testcase to work fine.

I think using the ivar is probably closer to what we want, and assuming we handle all window events that can impact the zoom state it should be fine. I am not a macOS UI expert though :p

I have no idea if this is truly related since their reproducer was quite different, but I did note that OpenJDK devs ran into something somewhat similar: https://bugs.openjdk.org/browse/JDK-8176490

The bug details recommend a similar fix of caching the zoom state to avoid calling NSWindow.isZoomed altogether.