Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail to build on windows using MSVC 14.42 due to ThorVG #95861

Closed
HKunogi opened this issue Aug 20, 2024 · 16 comments · Fixed by #96167
Closed

Fail to build on windows using MSVC 14.42 due to ThorVG #95861

HKunogi opened this issue Aug 20, 2024 · 16 comments · Fixed by #96167

Comments

@HKunogi
Copy link
Contributor

HKunogi commented Aug 20, 2024

Tested versions

  • Reproducible with commit 826de79 on windows

System information

Windows 11 - MSVC 14.42

Issue description

  • When trying to build on windows with C# support it throws the following errors due to thorvg, it builds fine on linux
D:\IDE\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\mutex(37): error C3861: '_Mtx_init_in_situ': identifier not found
D:\IDE\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\mutex(536): error C3861: '_Cnd_init_in_situ': identifier not found
D:\IDE\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.42.34226\include\condition_variable(56): error C3861: '_Cnd_init_in_situ': identifier not found
scons: *** [thirdparty\thorvg\src\loaders\svg\tvgSvgLoader.windows.editor.x86_64.obj] Error 2
scons: building terminated because of errors.

Steps to reproduce

  • Simply build the engine source with C# support using latest MSVC 14.42 with latest Windows 11 SDK on Windows.

Minimal reproduction project (MRP)

This is an engine build problem, thus it does not have an actual project.

@fire
Copy link
Member

fire commented Aug 20, 2024

@hermet linking you for tracking

@shadowfox87
Copy link

Hello, any update on this? We are unable to build on Windows. We already compiled v4.4 but this still hasn't been implemented.

@akien-mga
Copy link
Member

akien-mga commented Aug 25, 2024

You are using the Preview version of VS 2022, which may not be supported yet. It seems like they broke compatibility in some Windows SDK MSVC STL headers, which might require adjustments in downstream code, or may be a bug on MS' side which will be fixed eventually.

If you need a solution now, I suggest staying to the stable release of VS 2022.

@akien-mga
Copy link
Member

akien-mga commented Aug 25, 2024

I would also try to suggest cleaning all build artifacts (run your usual scons command with --clean added to it, or clean with git clean -fxd, but note this removes all untracked files).

From reading the PR that made this change in the MSVC STL, it seems like your error may come from a mismatched between code compiled with an older version, and the new version of that header.

@akien-mga
Copy link
Member

See https://github.com/microsoft/STL/wiki/Changelog#vs-2022-1710

image

Godot by default doesn't mix binaries from different compiler versions, when you compile Godot from source it should compile everything. One exception to that would be D3D12 dependencies compiled by Microsoft themselves, and ANGLE static compiled by us. Are you including either of those? Please give your full build command in the OP.

CC @bruvzg - if it's ANGLE static, we might need to provide different builds for different versions of MSVC :(

@bruvzg
Copy link
Member

bruvzg commented Aug 25, 2024

Current MSVC tools version is 14.41.34120, and it's building without issues. But the error might be related to the installation location, I think there were reports about SCons not detecting MSVC in non-standard locations (not on drive C:).

See https://github.com/microsoft/STL/wiki/Changelog#vs-2022-1710

This change is in current stable MSVC (17.11.1), so unlikely related.

Edit: seems like current SCons still using hardcoded paths to look for MSVC - https://github.com/SCons/scons/blob/master/SCons/Tool/MSCommon/vc.py#L927-L940

@hermet
Copy link

hermet commented Aug 27, 2024

Hello, just for your information, thorvg previsouly applied that option in v0.14.0 (thorvg/thorvg#2385)
and reverted in v0.14.5 (thorvg/thorvg#2611)

@akien-mga
Copy link
Member

Oh thanks for the headsup. I really thought thorvg was a red herring here, and just happened the first one to hit a STL mutex include. I hadn't seen that _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR was indeed defined in thorvg. I'll update to fix it.

vbettaque pushed a commit to vbettaque/godot that referenced this issue Aug 28, 2024
addmix pushed a commit to addmix/godot that referenced this issue Sep 6, 2024
akien-mga added a commit to akien-mga/godot that referenced this issue Sep 17, 2024
Fixes godotengine#95861.

(cherry picked from commit f16d4af)
radiantgurl pushed a commit to radiantgurl/godot that referenced this issue Sep 30, 2024
Fixes godotengine#95861.

(cherry picked from commit f16d4af)
@akien-mga
Copy link
Member

For users still running into this issue when trying to build the 4.3.stable tag (which can't have that fix retroactively), you can patch it up with this diff:

diff --git a/thirdparty/thorvg/src/common/tvgLock.h b/thirdparty/thorvg/src/common/tvgLock.h
index 59f68d0d44ba..d3a4e41c5cfd 100644
--- a/thirdparty/thorvg/src/common/tvgLock.h
+++ b/thirdparty/thorvg/src/common/tvgLock.h
@@ -25,8 +25,6 @@
 
 #ifdef THORVG_THREAD_SUPPORT
 
-#define _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
-
 #include <mutex>
 #include "tvgTaskScheduler.h"
 

RevoluPowered pushed a commit to the-mirror-gdp/godot that referenced this issue Nov 19, 2024
@Radnyx
Copy link

Radnyx commented Nov 23, 2024

Not sure if the ThorVG shipped with 4.3.stable still requires _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR. So here's a one-liner that defines it before building:

In platform/windows/detect.py:

env.AppendUnique(
    CPPDEFINES=[
+       "_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR",
        "WINDOWS_ENABLED",
        "WASAPI_ENABLED",
        "WINMIDI_ENABLED",
        "TYPED_METHOD_BIND",
        "WIN32",
        "WINVER=%s" % env["target_win_version"],
        "_WIN32_WINNT=%s" % env["target_win_version"],
    ]
)

@badlogic
Copy link

Neither @akien-mga 's nor @Radnyx diffs (which contradict each other, one removes the define, the other adds it) fix the issue on the windows runners. @akien-mga 's diff fixes the issue in tvgLock.h, but doesn't in tvgTaskScheduler.h.

Here's a working diff for building Godot 4.3-stable on GitHub Action Windows runners:

@@ -1,76 +1,32 @@
diff --git a/thirdparty/thorvg/src/common/tvgLock.h b/thirdparty/thorvg/src/common/tvgLock.h
index 59f68d0..d3a4e41 100644
--- a/thirdparty/thorvg/src/common/tvgLock.h
+++ b/thirdparty/thorvg/src/common/tvgLock.h
@@ -25,8 +25,6 @@
 
 #ifdef THORVG_THREAD_SUPPORT
 
-#define _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
-
 #include <mutex>
 #include "tvgTaskScheduler.h"
 
diff --git a/thirdparty/thorvg/src/renderer/tvgTaskScheduler.h b/thirdparty/thorvg/src/renderer/tvgTaskScheduler.h
index 93f8481..b1fb461 100644
--- a/thirdparty/thorvg/src/renderer/tvgTaskScheduler.h
+++ b/thirdparty/thorvg/src/renderer/tvgTaskScheduler.h
@@ -23,8 +23,6 @@
 #ifndef _TVG_TASK_SCHEDULER_H_
 #define _TVG_TASK_SCHEDULER_H_
 
-#define _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
-
 #include <mutex>
 #include <condition_variable>
 
@@ -111,4 +109,4 @@ struct TaskScheduler
 }  //namespace
 
 #endif //_TVG_TASK_SCHEDULER_H_
- 
+

@Radnyx
Copy link

Radnyx commented Nov 25, 2024

Neither @akien-mga 's nor @Radnyx diffs (which contradict each other, one removes the define, the other adds it) fix the issue on the windows runners. @akien-mga 's diff fixes the issue in tvgLock.h, but doesn't in tvgTaskScheduler.h.

I believe it's because the workflow turns warnings into errors. In which case yeah you'll need to remove the manual defines, as they generate a warning when predefining.

I'm still in favor of predefining to reduce the risk of breaking existing behavior.

@akien-mga
Copy link
Member

@badlogic's diff is the correct hotfix for this issue. The ThorVG version in 4.3.stable doesn't need that define.

@akien-mga akien-mga marked this as a duplicate of #102689 Feb 11, 2025
@Sailanarmo
Copy link

Not sure if the ThorVG shipped with 4.3.stable still requires _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR. So here's a one-liner that defines it before building:

In platform/windows/detect.py:

env.AppendUnique(
CPPDEFINES=[

  •   "_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR",
      "WINDOWS_ENABLED",
      "WASAPI_ENABLED",
      "WINMIDI_ENABLED",
      "TYPED_METHOD_BIND",
      "WIN32",
      "WINVER=%s" % env["target_win_version"],
      "_WIN32_WINNT=%s" % env["target_win_version"],
    
    ]
    )

I had to add this to the file as I still encountered the same compiler errors.

I'm running on 14.43 which just released I believe.

@fire
Copy link
Member

fire commented Feb 20, 2025

@Sailanarmo Can you make a new issue & pull request if encounter the compiler errors?

@akien-mga
Copy link
Member

This issue has been fixed in master and 4.3 already. It's not possible to change an already releases tag for 4.3-stable. So users who wish to build 4.3-stable need to work it around as described here:

#95861 (comment)

kuruk-mm added a commit to decentraland/godotengine that referenced this issue Feb 23, 2025
kuruk-mm added a commit to decentraland/godotengine that referenced this issue Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants