[PATCH v2] darwin: Fix non-Metal build
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 <keno(a)juliacomputing.com> --- v2: Drop `-framework QuartzCore` from configure.ac as suggested by Dean Greer <gcenx83(a)gmail.com>. 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
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 <keno(a)juliacomputing.com> 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 <keno(a)juliacomputing.com> ---
v2: Drop `-framework QuartzCore` from configure.ac as suggested by Dean Greer <gcenx83(a)gmail.com>.
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
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
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 <testbot(a)winehq.org> 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
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 <gcenx83(a)gmail.com> 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 <testbot(a)winehq.org> 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
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&...
December 15, 2021 7:17 PM, "Keno Fischer" <keno(a)juliacomputing.com> 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
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 <cdavis(a)codeweavers.com> wrote:
December 15, 2021 7:17 PM, "Keno Fischer" <keno(a)juliacomputing.com> 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
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 <gcenx83(a)gmail.com> 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 <cdavis(a)codeweavers.com> wrote:
December 15, 2021 7:17 PM, "Keno Fischer" <keno(a)juliacomputing.com> 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
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?
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 <stefandoesinger(a)gmail.com> wrote:
Did you build without freetype by any chance?
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.
On Dec 28, 2021, at 8:22 AM, Dean Greer <gcenx83(a)gmail.com> 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
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 <bshanks(a)codeweavers.com> wrote:
On Dec 28, 2021, at 8:22 AM, Dean Greer <gcenx83(a)gmail.com> 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
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
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 <keno(a)juliacomputing.com> 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&...
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 <keno(a)juliacomputing.com> ---
v2: Drop `-framework QuartzCore` from configure.ac as suggested by Dean Greer <gcenx83(a)gmail.com>.
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.
participants (7)
-
Brendan Shanks -
Chip Davis -
Dean Greer -
Huw Davies -
Keno Fischer -
Marvin -
Stefan Dösinger