[PATCH v2] darwin: Fix non-Metal build

[Keno Fischer]

The build was relying on QuartzCore definitions even if
Metal was not found, so move the appropriate import out
of the ifdef and add the QuartzCore framework to the
make line.

Signed-off-by: Keno Fischer 
---

v2: Drop `-framework QuartzCore` from configure.ac as
    suggested by Dean Greer .

configure                       | 2 +-
 configure.ac                    | 2 +-
 dlls/winemac.drv/Makefile.in    | 2 +-
 dlls/winemac.drv/cocoa_window.m | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index d57fc6cf43b..bf65b916b7d 100755
--- a/configure
+++ b/configure
@@ -10321,7 +10321,7 @@ fi
     fi
     if test "$ac_cv_header_Metal_Metal_h" = "yes"
     then
-        METAL_LIBS="-framework Metal -framework QuartzCore"
+        METAL_LIBS="-framework Metal"

fi

diff --git a/configure.ac b/configure.ac
index de79b47ea2a..1a36e98f3e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -757,7 +757,7 @@ case $host_os in
     fi
     if test "$ac_cv_header_Metal_Metal_h" = "yes"
     then
-        AC_SUBST(METAL_LIBS,"-framework Metal -framework QuartzCore")
+        AC_SUBST(METAL_LIBS,"-framework Metal")
     fi

dnl Check for MTLDevice registryID property
diff --git a/dlls/winemac.drv/Makefile.in b/dlls/winemac.drv/Makefile.in
index 6329e8e76c8..fc3dddbdae7 100644
--- a/dlls/winemac.drv/Makefile.in
+++ b/dlls/winemac.drv/Makefile.in
@@ -1,7 +1,7 @@
 MODULE    = winemac.drv
 IMPORTS   = uuid rpcrt4 user32 gdi32 advapi32 win32u
 DELAYIMPORTS = ole32 shell32 imm32
-EXTRALIBS = -framework AppKit -framework Carbon -framework Security -framework OpenGL -framework IOKit -framework CoreVideo $(METAL_LIBS)
+EXTRALIBS = -framework AppKit -framework Carbon -framework Security -framework OpenGL -framework IOKit -framework CoreVideo -framework QuartzCore $(METAL_LIBS)

EXTRADLLFLAGS = -mcygwin

diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m
index d0672b7fb06..a18fc069604 100644
--- a/dlls/winemac.drv/cocoa_window.m
+++ b/dlls/winemac.drv/cocoa_window.m
@@ -25,8 +25,8 @@
 #import <CoreVideo/CoreVideo.h>
 #ifdef HAVE_METAL_METAL_H
 #import <Metal/Metal.h>
-#import <QuartzCore/QuartzCore.h>
 #endif
+#import <QuartzCore/QuartzCore.h>

#import "cocoa_window.h"

[Dean Greer]

The patch shouldn’t be touching configure only configure.ac, the configure
changes will get auto generated when merged.

However this patch doesn’t fully resolve the build issues with winemac.drv.

I’m currently unable to build with the MacOSX10.11.SDK and that’s the
minimum I usually test with. Seems I’ll need to open multiple tickets so
these issues can be resolved before the release.

On Sun, Dec 12, 2021 at 2:32 AM Keno Fischer 
wrote:
> The build was relying on QuartzCore definitions even if
> Metal was not found, so move the appropriate import out
> of the ifdef and add the QuartzCore framework to the
> make line.
> Signed-off-by: Keno Fischer
>
> v2: Drop `-framework QuartzCore` from configure.ac as
>     suggested by Dean Greer .
>
> configure                       | 2 +-
>  configure.ac                    | 2 +-
>  dlls/winemac.drv/Makefile.in    | 2 +-
>  dlls/winemac.drv/cocoa_window.m | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index d57fc6cf43b..bf65b916b7d 100755
> --- a/configure
> +++ b/configure
> @@ -10321,7 +10321,7 @@ fi
>      fi
>      if test "$ac_cv_header_Metal_Metal_h" = "yes"
>      then
> -
>    METAL_LIBS="-framework Metal -framework QuartzCore"
> -
>    METAL_LIBS="-framework Metal"
>
> fi
>
> diff --git a/configure.ac b/configure.ac
> index de79b47ea2a..1a36e98f3e8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -757,7 +757,7 @@ case $host_os in
>      fi
>      if test "$ac_cv_header_Metal_Metal_h" = "yes"
>      then
> -
>    AC_SUBST(METAL_LIBS,"-framework Metal -framework QuartzCore")
> -
>    AC_SUBST(METAL_LIBS,"-framework Metal")
>
> fi
>
> dnl Check for MTLDevice registryID property
>
> diff --git a/dlls/winemac.drv/Makefile.in b/dlls/winemac.drv/Makefile.in
> index 6329e8e76c8..fc3dddbdae7 100644
> --- a/dlls/winemac.drv/Makefile.in
> +++ b/dlls/winemac.drv/Makefile.in
> @@ -1,7 +1,7 @@
>  MODULE    = winemac.drv
>  IMPORTS   = uuid rpcrt4 user32 gdi32 advapi32 win32u
>  DELAYIMPORTS = ole32 shell32 imm32
> -EXTRALIBS = -framework AppKit -framework Carbon -framework Security
> -framework OpenGL -framework IOKit -framework CoreVideo $(METAL_LIBS)
> +EXTRALIBS = -framework AppKit -framework Carbon -framework Security
> -framework OpenGL -framework IOKit -framework CoreVideo -framework
> QuartzCore $(METAL_LIBS)
>
> EXTRADLLFLAGS = -mcygwin
>
> diff --git a/dlls/winemac.drv/cocoa_window.m
> b/dlls/winemac.drv/cocoa_window.m
> index d0672b7fb06..a18fc069604 100644
> --- a/dlls/winemac.drv/cocoa_window.m
> +++ b/dlls/winemac.drv/cocoa_window.m
> @@ -25,8 +25,8 @@
>  #import <CoreVideo/CoreVideo.h>
>  #ifdef HAVE_METAL_METAL_H
>  #import <Metal/Metal.h>
> -#import <QuartzCore/QuartzCore.h>
>  #endif
> +#import <QuartzCore/QuartzCore.h>
>
> #import "cocoa_window.h"
>
> --
> 2.25.1

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=103984

Your paranoid android.

=== debian11 (build log) ===

Task: Patch failed to apply

=== debian11 (build log) ===

Task: Patch failed to apply

[Dean Greer]

Even with this patch wine-7.0-rc1 without Metal fails to compile

:info:build dlls/winemac.drv/cocoa_window.m:310:12: error: cannot find
interface declaration for 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:316:17: error: cannot find
interface declaration for 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:320:60: error: property 'path'
not found on object of type 'id'
:info:build dlls/winemac.drv/cocoa_window.m:2010:9: error: use of
undeclared identifier 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:2010:23: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2010:44: error: expected
expression
:info:build dlls/winemac.drv/cocoa_window.m:2010:31: error: use of
undeclared identifier 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:2011:41: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2014:34: error: use of
undeclared identifier 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:2014:26: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2016:26: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2018:13: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2019:93: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2023:9: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2225:9: error: use of
undeclared identifier 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:2225:23: error: use of
undeclared identifier 'mask'
:info:build dlls/winemac.drv/cocoa_window.m:2225:44: error: expected
expression
:info:build dlls/winemac.drv/cocoa_window.m:2225:31: error: use of
undeclared identifier 'CAShapeLayer'
:info:build dlls/winemac.drv/cocoa_window.m:2225:46: error: missing '[' at
start of message send expression

On Sun, Dec 12, 2021 at 1:13 PM Marvin  wrote:
> Hi,
>
> While running your changed tests, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
>
> Full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=103984
>
> Your paranoid android.
>
> === debian11 (build log) ===
>
> Task: Patch failed to apply
>
> === debian11 (build log) ===
>
> Task: Patch failed to apply

[Dean Greer]

As this framework was never linked prior outside of a Metal build this
patch is invalid

Instead opened a bug
https://bugs.winehq.org/show_bug.cgi?id=52216

On Mon, Dec 13, 2021 at 9:24 AM Dean Greer  wrote:
> Even with this patch wine-7.0-rc1 without Metal fails to compile
>
> :info:build dlls/winemac.drv/cocoa_window.m:310:12: error: cannot find
> interface declaration for 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:316:17: error: cannot find
> interface declaration for 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:320:60: error: property 'path'
> not found on object of type 'id'
> :info:build dlls/winemac.drv/cocoa_window.m:2010:9: error: use of
> undeclared identifier 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:2010:23: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2010:44: error: expected
> expression
> :info:build dlls/winemac.drv/cocoa_window.m:2010:31: error: use of
> undeclared identifier 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:2011:41: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2014:34: error: use of
> undeclared identifier 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:2014:26: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2016:26: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2018:13: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2019:93: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2023:9: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2225:9: error: use of
> undeclared identifier 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:2225:23: error: use of
> undeclared identifier 'mask'
> :info:build dlls/winemac.drv/cocoa_window.m:2225:44: error: expected
> expression
> :info:build dlls/winemac.drv/cocoa_window.m:2225:31: error: use of
> undeclared identifier 'CAShapeLayer'
> :info:build dlls/winemac.drv/cocoa_window.m:2225:46: error: missing '[' at
> start of message send expression
>
> On Sun, Dec 12, 2021 at 1:13 PM Marvin  wrote:
> > Hi,
> >
> > While running your changed tests, I think I found new failures.
> > Being a bot and all I'm not very good at pattern recognition, so I might
> > be
> > wrong, but could you please double-check?
> >
> > Full results can be found at:
> > https://testbot.winehq.org/JobDetails.pl?Key=103984
> >
> > Your paranoid android.
> >
> > === debian11 (build log) ===
> >
> > Task: Patch failed to apply
> >
> > === debian11 (build log) ===
> >
> > Task: Patch failed to apply

[Keno Fischer]

Hi Dean,

sorry, I missed your follow up messages to this.
> Even with this patch wine-7.0-rc1 without Metal fails to compile

Hmm, that's odd. The error you're seeing is precisely the one I was
attempting to address with this patch. As you can see in [1], we did
successfully build wine for Darwin with this patch.
> As this framework was never linked prior outside of a Metal build this
> patch is invalid

Ok, I don't know the history here. Getting rid of QuartzCore in the
non-Metal build would of course address the issue as well.
I'll drop this patch then and wait for a proper fix.

[1] https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=15191&...

[Chip Davis]

December 15, 2021 7:17 PM, "Keno Fischer"  wrote:
> Hi Dean,
>
> sorry, I missed your follow up messages to this.
> > Even with this patch wine-7.0-rc1 without Metal fails to compile
>
> Hmm, that's odd. The error you're seeing is precisely the one I was
> attempting to address with this patch. As you can see in [1], we did
> successfully build wine for Darwin with this patch.
> > As this framework was never linked prior outside of a Metal build this
> > patch is invalid
>
> Ok, I don't know the history here. Getting rid of QuartzCore in the
> non-Metal build would of course address the issue as well.
> I'll drop this patch then and wait for a proper fix.

I'm afraid this is all my fault. In a5cf847aa45c38bae1500ef19a9bac38bbd8d5ba, I turned on layer-backed views in winemac.drv. In 01f027b2db3196a8fe43be3d6718227794362985, I moved to using a mask layer for window regions. These introduced a dependency on QuartzCore that was not present before. I am sorry--this was an oversight on my part. We really do want to link to QuartzCore unconditionally.

Chip

[Dean Greer]

I figured as much but didn’t have chance to really dig into it to verify
just figured it was wine-6.17 where winemac.drv broken.

On Wed, Dec 15, 2021 at 9:55 PM Chip Davis  wrote:
> December 15, 2021 7:17 PM, "Keno Fischer"  wrote:
> > Hi Dean,
> >
> > sorry, I missed your follow up messages to this.
> > > Even with this patch wine-7.0-rc1 without Metal fails to compile
> >
> > Hmm, that's odd. The error you're seeing is precisely the one I was
> > attempting to address with this patch. As you can see in [1], we did
> > successfully build wine for Darwin with this patch.
> > > As this framework was never linked prior outside of a Metal build this
> > > patch is invalid
> >
> > Ok, I don't know the history here. Getting rid of QuartzCore in the
> > non-Metal build would of course address the issue as well.
> > I'll drop this patch then and wait for a proper fix.
>
> I'm afraid this is all my fault. In
> a5cf847aa45c38bae1500ef19a9bac38bbd8d5ba, I turned on layer-backed views in
> winemac.drv. In 01f027b2db3196a8fe43be3d6718227794362985, I moved to using
> a mask layer for window regions. These introduced a dependency on
> QuartzCore that was not present before. I am sorry--this was an oversight
> on my part. We really do want to link to QuartzCore unconditionally.
>
> Chip

[Dean Greer]

Rebuilt my environments and managed to get wine-7.0-rc1/2 to compile using
the proposed patch.

However here’s what I’m seeing within my 10.9VM when using winemac.drv
Currently rebuilding with X11 support to see if the issue is just with
winemac.drv.

The last version I’d verified (6.0.2 & 6.11) functioned correctly within
this 10.9VM

Copied the Van compile onto the real install (Mojave) and winecfg launched
without issue.

On Wed, Dec 15, 2021 at 9:58 PM Dean Greer  wrote:
> I figured as much but didn’t have chance to really dig into it to verify
> just figured it was wine-6.17 where winemac.drv broken.
>
> On Wed, Dec 15, 2021 at 9:55 PM Chip Davis  wrote:
> > December 15, 2021 7:17 PM, "Keno Fischer" 
> > wrote:
> > > Hi Dean,
> > >
> > > sorry, I missed your follow up messages to this.
> > > > Even with this patch wine-7.0-rc1 without Metal fails to compile
> > >
> > > Hmm, that's odd. The error you're seeing is precisely the one I was
> > > attempting to address with this patch. As you can see in [1], we did
> > > successfully build wine for Darwin with this patch.
> > > > As this framework was never linked prior outside of a Metal build this
> > > > patch is invalid
> > >
> > > Ok, I don't know the history here. Getting rid of QuartzCore in the
> > > non-Metal build would of course address the issue as well.
> > > I'll drop this patch then and wait for a proper fix.
> >
> > I'm afraid this is all my fault. In
> > a5cf847aa45c38bae1500ef19a9bac38bbd8d5ba, I turned on layer-backed views in
> > winemac.drv. In 01f027b2db3196a8fe43be3d6718227794362985, I moved to using
> > a mask layer for window regions. These introduced a dependency on
> > QuartzCore that was not present before. I am sorry--this was an oversight
> > on my part. We really do want to link to QuartzCore unconditionally.
> >
> > Chip

[Stefan Dösinger]

Am Montag, 20. Dezember 2021, 16:35:06 CET schrieb Dean Greer:
> Rebuilt my environments and managed to get wine-7.0-rc1/2 to compile using
> the proposed patch.
>
> However here’s what I’m seeing within my 10.9VM when using winemac.drv
> Currently rebuilding with X11 support to see if the issue is just with
> winemac.drv.

Did you build without freetype by any chance?

[Dean Greer]

Doubtful as I have --with-freetype passed to configure at build time.

This same build that failed to display anything within the 10.9VM works
normally when copied outside to my Mojave install.

On Mon, Dec 20, 2021 at 1:36 PM Stefan Dösinger 
wrote:
> Did you build without freetype by any chance?

[Dean Greer]

Tested this on an actual 10.9 install (10.8 is harder to get setup) and
still the same issue nothing gets displayed other than the bar as shown in
one of the prior emails.

[Brendan Shanks]

> On Dec 28, 2021, at 8:22 AM, Dean Greer  wrote:
>
> Tested this on an actual 10.9 install (10.8 is harder to get setup) and still the same issue nothing gets displayed other than the bar as shown in one of the prior emails.

I built and ran Wine on 10.13, and I get the same result. The regression commit is "winemac.drv: Remove now unused -[WineContentView drawRect:].”, 3f845b34deada0dd58e3674119af47ce85851c24.

The problem seems to be that WineContentView updateLayer isn’t being called for some reason. What’s weird is, if I add back an empty implementation of drawRect: like this, it does work.

diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m
index d0672b7fb06..1ee582e6b59 100644
--- a/dlls/winemac.drv/cocoa_window.m
+++ b/dlls/winemac.drv/cocoa_window.m
@@ -477,6 +477,10 @@ - (BOOL) isFlipped
         return YES;
     }

+    - (void) drawRect:(NSRect)rect
+    {
+    }
+
     - (BOOL) wantsUpdateLayer
     {
         return YES /*!_everHadGLContext*/;

Dean, does this change make a difference for you on 10.9?

And Chip, any idea why this would make a difference or if this is the right thing to do? drawRect: isn’t even being called, it’s existence alone seems to make things work.

(Note that this is on a 2010 MacBook which is about as old a machine as 10.13 supported. In particular, it has a GeForce 320M which doesn’t support Metal, but I don’t think this should make a difference. I’ll test out a 10.13 machine with Metal to be sure.)

Brendan

[Dean Greer]

Applying that patch does indeed get winemac.drv functional.

While it now works I get the following issue

[image: Screen Shot 2021-12-31 at 4.35.51 PM.png]
As can be seen the UI is off and interaction with the rendered items is off.

On Fri, Dec 31, 2021 at 12:44 AM Brendan Shanks 
wrote:
> > On Dec 28, 2021, at 8:22 AM, Dean Greer  wrote:
> >
> > Tested this on an actual 10.9 install (10.8 is harder to get setup) and
>
> still the same issue nothing gets displayed other than the bar as shown in
> one of the prior emails.
>
> I built and ran Wine on 10.13, and I get the same result. The regression
> commit is "winemac.drv: Remove now unused -[WineContentView drawRect:].”,
> 3f845b34deada0dd58e3674119af47ce85851c24.
>
> The problem seems to be that WineContentView updateLayer isn’t being
> called for some reason. What’s weird is, if I add back an empty
> implementation of drawRect: like this, it does work.
>
> diff --git a/dlls/winemac.drv/cocoa_window.m
> b/dlls/winemac.drv/cocoa_window.m
> index d0672b7fb06..1ee582e6b59 100644
> --- a/dlls/winemac.drv/cocoa_window.m
> +++ b/dlls/winemac.drv/cocoa_window.m
> @@ -477,6 +477,10 @@ - (BOOL) isFlipped
>          return YES;
>      }
> -
> - (void) drawRect:(NSRect)rect
> - {
> - }
> -
> - (BOOL) wantsUpdateLayer
> {
>        return YES /*!_everHadGLContext*/;
>
> Dean, does this change make a difference for you on 10.9?
>
> And Chip, any idea why this would make a difference or if this is the
> right thing to do? drawRect: isn’t even being called, it’s existence alone
> seems to make things work.
>
> (Note that this is on a 2010 MacBook which is about as old a machine as
> 10.13 supported. In particular, it has a GeForce 320M which doesn’t support
> Metal, but I don’t think this should make a difference. I’ll test out a
> 10.13 machine with Metal to be sure.)
>
> Brendan

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=104703

Your paranoid android.

=== debian11 (build log) ===

Task: Patch failed to apply

=== debian11 (build log) ===

Task: Patch failed to apply

[Dean Greer]

I’d said the patch is invalid as it didn’t work when I’ve attempted to
compile wine with it applied. I could only get it to compile when using the
MacOSX10.11.SDK without disabling Metal but that’s unfortunately worthless
as Macports rev-upgrade will report the build as broken due to
metal.framework being linked.

Last I checked Macports didn’t ignore weak linked libraries/frameworks so
weak linking isn’t an option unfortunately.

I’ve not ran any regression tests yet but did narrow it down to wine-6.17
being when winemac.drv first broke that’s when Chip did some changes so
possibly that series of patches cause this breakage.

On Wed, Dec 15, 2021 at 8:17 PM Keno Fischer 
wrote:
> Hi Dean,
>
> sorry, I missed your follow up messages to this.
> > Even with this patch wine-7.0-rc1 without Metal fails to compile
>
> Hmm, that's odd. The error you're seeing is precisely the one I was
> attempting to address with this patch. As you can see in [1], we did
> successfully build wine for Darwin with this patch.
> > As this framework was never linked prior outside of a Metal build this
> > patch is invalid
>
> Ok, I don't know the history here. Getting rid of QuartzCore in the
> non-Metal build would of course address the issue as well.
> I'll drop this patch then and wait for a proper fix.
>
> [1]
> https://dev.azure.com/JuliaPackaging/Yggdrasil/_build/results?buildId=15191&...

[Huw Davies]

On Sun, Dec 12, 2021 at 02:31:58AM -0500, Keno Fischer wrote:
> The build was relying on QuartzCore definitions even if
> Metal was not found, so move the appropriate import out
> of the ifdef and add the QuartzCore framework to the
> make line.
> Signed-off-by: Keno Fischer
>
> v2: Drop `-framework QuartzCore` from configure.ac as
>     suggested by Dean Greer .

I've sent in v3 of this as I didn't want it to get lost.

The window painting issue discussed later in the thread is ongoing.

Huw.