Hi Akihiko

I’ve re-worked the patch to match your suggestion. I have compiled
and tested it on Sonoma and Monterey and both builds worked correctly.

New patch is below. I’m new to sending patches to QEMU so please let 
me know if I need to do anything else to get it incorporated into the
repo.

Dave 

diff --git a/ui/cocoa.m b/ui/cocoa.m
index eb99064bee..bbf9704b8c 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -54,6 +54,10 @@
 #define MAC_OS_X_VERSION_10_13 101300
 #endif
 
+#ifndef MAC_OS_VERSION_14_0
+#define MAC_OS_VERSION_14_0 140000
+#endif
+
 /* 10.14 deprecates NSOnState and NSOffState in favor of
  * NSControlStateValueOn/Off, which were introduced in 10.13.
  * Define for older versions
@@ -365,6 +369,9 @@ - (id)initWithFrame:(NSRect)frameRect
         screen.width = frameRect.size.width;
         screen.height = frameRect.size.height;
         kbd = qkbd_state_init(dcl.con);
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0
+        [self setClipsToBounds:YES];
+#endif
 
     }
     return self;


> On 23 Feb 2024, at 11:28, Akihiko Odaki <akihiko.od...@daynix.com> wrote:
> 
> On 2024/02/23 2:10, Peter Maydell wrote:
>> On Thu, 22 Feb 2024 at 06:08, Michael Tokarev <m...@tls.msk.ru> wrote:
>>> 
>>> [Adding a few more Ccs]
>>> 
>>> 17.02.2024 18:58, David Parsons :
>>>> macOS Sonoma changes the NSView.clipsToBounds to false by default where it 
>>>> was true in
>>>> earlier version of macOS. This causes the window contents to be obscured 
>>>> by the window
>>>> frame. This fixes the issue by conditionally setting the clipping on 
>>>> Sonoma to true.
> 
> Thanks for posting a patch for this critical problem.
> 
>>>> 
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1994
>>>> Signed-off-by: David Parsons <d...@daveparsons.net>
>>>> 
>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>> index eb99064bee..c9e3b96004 100644
>>>> --- a/ui/cocoa.m
>>>> +++ b/ui/cocoa.m
>>>> @@ -365,6 +365,9 @@ - (id)initWithFrame:(NSRect)frameRect
>>>>           screen.width = frameRect.size.width;
>>>>           screen.height = frameRect.size.height;
>>>>           kbd = qkbd_state_init(dcl.con);
>>>> +        if (@available(macOS 14, *)) {
>>>> +            [self setClipsToBounds:YES];
>>>> +        }
>>>> 
>>>>       }
>>>>       return self;
>>>> 
>>> 
>>> Hi David!
>>> 
>>> While the code change is tiny, I for one know nothing about MacOS and
>>> its cocoa thing, so to me (with my trivial-patches hat on) this is a
>>> no-go.  I'd love to have a review from someone more knowlegeable in
>>> this area.
>> Mmm. Akihiko is the expert here, but I do notice that we don't
>> seem to be handling the "macos-version-specific" stuff in a
>> way we've done it before (we don't use @available elsewhere).
>> I did wonder if we could call the setClipsToBounds method unconditionally;
>> The release notes say
>> https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-14#NSView
>> "This property is available back to macOS 10.9. This availability is
>> intended to allow code targeting older OSes to set this property to
>> true without guarding the setter in an availability check."
>> but I think that might only mean "you can do this building on a new
>> SDK that's targeting an old version", not "you can do this
>> when building on an older SDK" (at least judging from the
>> comments in the gitlab issue).
> 
> Apparently it is that case.
> 
> Please check if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0
> instead of using @available. See commit 5e24600a7c1c ("ui/cocoa.m: Fix macOS 
> 10.14 deprecation warnings") for example.
> 
>> The other option would be to fix whatever it is that we're
>> presumably not getting right that means this default change
>> caused the bug. My guess is that we are in the case
>> "Confusing a view’s bounds and its dirty rect. The dirty rect
>>  passed to .drawRect() should be used to determine what to draw,
>>  not where to draw it. Use NSView.bounds when determining the
>>  layout of what your view draws."
>> But unless the fix for that is really obvious and easy I guess
>> that flipping the default back to its old value is the better
>> approach.
> 
> It is a chore to convert the coordinates using NSView.bounds. Let's keep 
> using clipsToBounds.
> 
>> -- PMM
> 
> 

Reply via email to