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

sys: Fixes for MinGW support #346

Closed
wants to merge 1 commit into from

Conversation

akien-mga
Copy link
Contributor

@akien-mga akien-mga commented Nov 22, 2021

Tested successfully in Godot to compile with MinGW-GCC and MinGW-Clang
for Windows.

This doesn't include needed changes to CMakeLists.txt to support using
a MinGW-GCC based toolchain to compile Embree with CMake.

Co-authored-by: @RandomShaper

Related to #279.

Tested successfully in Godot to compile with MinGW-GCC and MinGW-Clang
for Windows.

This doesn't include needed changes to CMakeLists.txt to support using
a MinGW-GCC based toolchain to compile Embree with CMake.

Co-authored-by: Pedro J. Estébanez <[email protected]>
@@ -99,7 +99,7 @@
#define dll_import
#endif

#ifdef __WIN32__
#if defined(__WIN32__) && !defined(__MINGW32__)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same might be relevant on line 94 for the definitions of dll_export and dll_import. But it's not part of the patch we're using in production for Godot (https://github.com/godotengine/godot/blob/master/thirdparty/embree/patches/godot-changes-misc.patch), so for this PR I chose to play it safe and only include what we tested.

@@ -99,7 +99,7 @@
#define dll_import
#endif

#ifdef __WIN32__
#if defined(__WIN32__) && !defined(__MINGW32__)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed to fix a crash issue when compiling Embree with MinGW-GCC, which we experienced upstream: godotengine/godot#45097

@svenwoop
Copy link
Collaborator

I merged this into our internal devel branch. The fix will be in the next release.

@svenwoop svenwoop closed this Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants