Skip to content

Commit

Permalink
Fixed compiling the native library with MSVC and made the script work…
Browse files Browse the repository at this point in the history
… for that

I could not get a build working with clang and msys2 or other tools without resorting to visual studio
  • Loading branch information
hhyyrylainen committed Oct 27, 2023
1 parent 01f1fa8 commit 130ca10
Show file tree
Hide file tree
Showing 17 changed files with 219 additions and 39 deletions.
5 changes: 5 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ option(NULL_HAS_UNUSUAL_REPRESENTATION

# set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${PROJECT_SOURCE_DIR}/CMake")

# Also use a lib prefix on windows for consistency
if(WIN32)
set(CMAKE_SHARED_LIBRARY_PREFIX_CXX "lib")
endif()

# static building
set(CMAKE_FIND_LIBRARY_SUFFIXES ".a")
set(BUILD_SHARED_LIBS OFF)
Expand Down
130 changes: 118 additions & 12 deletions Scripts/NativeLibs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using ScriptsBase.Utilities;
Expand Down Expand Up @@ -92,6 +93,8 @@ public async Task<bool> Run(CancellationToken cancellationToken)
// Make sure this base folder exists
Directory.CreateDirectory(LibraryFolder);

await PackageTool.EnsureGodotIgnoreFileExistsInFolder(LibraryFolder);

foreach (var operation in options.Operations)
{
if (!await RunOperation(operation, cancellationToken))
Expand Down Expand Up @@ -165,7 +168,7 @@ private string GetLibraryDllName(Library library, PackagePlatform platform)
case PackagePlatform.Linux:
return "libthrive_native.so";
case PackagePlatform.Windows:
throw new NotImplementedException("TODO: name for this");
return "libthrive_native.dll";
case PackagePlatform.Windows32:
throw new NotSupportedException("32-bit support is not done currently");
case PackagePlatform.Mac:
Expand Down Expand Up @@ -290,13 +293,7 @@ private Task<bool> InstallLibraryForEditor(Library library, PackagePlatform plat

var linkFile = Path.Join(target, GetLibraryDllName(library, platform));

if (File.Exists(linkFile))
{
ColourConsole.WriteDebugLine($"Removing existing library file: {linkFile}");
File.Delete(linkFile);
}

File.CreateSymbolicLink(linkFile, Path.GetFullPath(linkTo));
CreateLinkTo(linkFile, linkTo);

if (platform != PlatformUtilities.GetCurrentPlatform())
{
Expand Down Expand Up @@ -348,6 +345,9 @@ private async Task<bool> BuildLocally(Library library, PackagePlatform platform,
// TODO: flag for clean builds
Directory.CreateDirectory(buildFolder);

// Ensure Godot doesn't try to import anything funny from the build folder
await PackageTool.EnsureGodotIgnoreFileExistsInFolder(buildFolder);

var installPath = Path.GetFullPath(GetLocalCMakeInstallTarget(platform, GetLibraryVersion(library)));

var startInfo = new ProcessStartInfo("cmake")
Expand All @@ -367,11 +367,48 @@ private async Task<bool> BuildLocally(Library library, PackagePlatform platform,
startInfo.ArgumentList.Add("-DCMAKE_BUILD_TYPE=RelWithDebInfo");
}

if (OperatingSystem.IsLinux())
if (!string.IsNullOrEmpty(options.Compiler) || !string.IsNullOrEmpty(options.CCompiler))
{
startInfo.ArgumentList.Add("-DCMAKE_CXX_COMPILER=clang++");
startInfo.ArgumentList.Add("-DCMAKE_C_COMPILER=clang");
ColourConsole.WriteNormalLine(
$"Using custom specified compiler: CXX: {options.Compiler}, C: {options.CCompiler}");

if (!string.IsNullOrEmpty(options.Compiler))
{
startInfo.ArgumentList.Add($"-DCMAKE_CXX_COMPILER={options.Compiler}");
}

if (!string.IsNullOrEmpty(options.CCompiler))
{
startInfo.ArgumentList.Add($"-DCMAKE_C_COMPILER={options.CCompiler}");
}
}
else
{
if (OperatingSystem.IsLinux())
{
// Use clang by default
startInfo.ArgumentList.Add("-DCMAKE_CXX_COMPILER=clang++");
startInfo.ArgumentList.Add("-DCMAKE_C_COMPILER=clang");
}
}

if (!string.IsNullOrEmpty(options.CmakeGenerator))
{
startInfo.ArgumentList.Add("-G");
startInfo.ArgumentList.Add(options.CmakeGenerator);
}

// TODO: add support for non-visual studio builds on windows
// When not using visual studio using ninja would be needed to avoid a dependency on it
// if (string.IsNullOrEmpty(ExecutableFinder.Which("ninja")))
// {
// ColourConsole.WriteErrorLine(
// "Could not find ninja executable, generating probably will fail. Please install it " +
// "and add to PATH or specify another generator system");
// }
//
// startInfo.ArgumentList.Add("-G");
// startInfo.ArgumentList.Add("Ninja");

startInfo.ArgumentList.Add($"-DCMAKE_INSTALL_PREFIX={installPath}");

Expand Down Expand Up @@ -402,7 +439,17 @@ private async Task<bool> BuildLocally(Library library, PackagePlatform platform,
startInfo.ArgumentList.Add(".");
startInfo.ArgumentList.Add("--config");

startInfo.ArgumentList.Add(options.DebugLibrary ? "Debug" : "RelWithDebInfo");
if (OperatingSystem.IsWindows())
{
ColourConsole.WriteWarningLine("TODO: Windows Jolt build with MSVC only supports Release mode, " +
"building Thrive in release mode as well, there won't be debug symbols");

startInfo.ArgumentList.Add(options.DebugLibrary ? "Debug" : "Release");
}
else
{
startInfo.ArgumentList.Add(options.DebugLibrary ? "Debug" : "RelWithDebInfo");
}

startInfo.ArgumentList.Add("--target");
startInfo.ArgumentList.Add("install");
Expand Down Expand Up @@ -461,14 +508,73 @@ private string GetPathToLibraryDll(Library library, PackagePlatform platform, st
{
var basePath = GetPathToLibrary(library, platform, version, distributableVersion);

if (platform is PackagePlatform.Windows or PackagePlatform.Windows32)
{
return Path.Combine(basePath, "bin", GetLibraryDllName(library, platform));
}

// This is for Linux
return Path.Combine(basePath, "lib", GetLibraryDllName(library, platform));
}

private void CreateLinkTo(string linkFile, string linkTo)
{
if (File.Exists(linkFile))
{
ColourConsole.WriteDebugLine($"Removing existing library file: {linkFile}");
File.Delete(linkFile);
}

if (!OperatingSystem.IsWindows() || options.UseSymlinks)
{
File.CreateSymbolicLink(linkFile, Path.GetFullPath(linkTo));
}
else
{
bool fallback = true;

if (OperatingSystem.IsWindows())
{
ColourConsole.WriteWarningLine("Not using symbolic link to install for editor. Any newly " +
"compiled library versions may not be visible to the editor without re-running " +
"this install command");
ColourConsole.WriteNormalLine("Symbolic links can be specifically enabled but require " +
"administrator privileges on Windows");

try
{
if (NativeMethods.CreateHardLink(linkFile, Path.GetFullPath(linkTo), IntPtr.Zero))
{
fallback = false;
}
}
catch (Exception e)
{
ColourConsole.WriteWarningLine($"Failed to call hardlink creation: {e}");
}
}

if (fallback)
{
ColourConsole.WriteWarningLine("Copying library instead of linking. The library won't update " +
"without re-running this tool!");
File.Copy(linkTo, linkFile);
}
}
}

private string GetNativeBuildFolder()
{
if (options.DebugLibrary)
return "build-debug";

return "build";
}

private static class NativeMethods
{
[DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
public static extern bool CreateHardLink(string lpFileName, string lpExistingFileName,
IntPtr lpSecurityAttributes);
}
}
1 change: 1 addition & 0 deletions Scripts/PackageTool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public static async Task EnsureGodotIgnoreFileExistsInFolder(string folder)

if (!File.Exists(ignoreFile))
{
ColourConsole.WriteDebugLine($"Creating .gdignore file in folder: {folder}");
await using var writer = File.Create(ignoreFile);
}
}
Expand Down
16 changes: 16 additions & 0 deletions Scripts/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,22 @@ public static IEnumerable<Example> Examples
[Option('p', "platform", Required = false, Default = null,
HelpText = "Use to override detected platforms for selected operation")]
public IList<PackagePlatform>? Platforms { get; set; } = new List<PackagePlatform>();

[Option('c', "compiler", Required = false, Default = null, MetaValue = "COMPILER",
HelpText = "Manually specify compiler to use")]
public string? Compiler { get; set; }

[Option("c-compiler", Required = false, Default = null, MetaValue = "COMPILER",
HelpText = "Manually specify C compiler to use")]
public string? CCompiler { get; set; }

[Option('g', "generator", Required = false, Default = null, MetaValue = "GENERATOR",
HelpText = "Manually specify which CMake generator to use")]
public string? CmakeGenerator { get; set; }

[Option('s', "symbolic-links", Required = false, Default = false,
HelpText = "If specified prefer to use symlinks even on Windows")]
public bool UseSymlinks { get; set; }
}

[Verb("test", HelpText = "Run tests using 'dotnet' command")]
Expand Down
Empty file added native_libs/.gdignore
Empty file.
19 changes: 16 additions & 3 deletions src/native/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,15 @@ target_precompile_headers(thrive_native PUBLIC "${PROJECT_BINARY_DIR}/Include.h"

set_target_properties(thrive_native PROPERTIES
CXX_STANDARD 20
CXX_EXTENSIONS OFF
INTERPROCEDURAL_OPTIMIZATION ON)
CXX_EXTENSIONS OFF)

if(UNIX OR CMAKE_GENERATOR MATCHES "Visual Studio")
# This seems to fail native Windows builds (when trying to use msys2)
set_target_properties(thrive_native PROPERTIES INTERPROCEDURAL_OPTIMIZATION ON)
else()
message(STATUS "Not enabling interprocedural optimization (lto) "
"as it is probably not working on this platform")
endif()

message(STATUS "TODO: static standard lib for easier distributing (make sure it is the clang one)")
# # Static standard lib
Expand All @@ -73,10 +80,16 @@ message(STATUS "TODO: static standard lib for easier distributing (make sure it
# target_link_libraries(thrive_native PRIVATE -static)
# endif()

# if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
# set_target_properties(thrive_native PROPERTIES LINK_FLAGS_RELEASE "-fuse-ld=lld")
# elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# set_target_properties(thrive_native PROPERTIES LINK_FLAGS_RELEASE "-fuse-ld=gold")
# endif()

if(NOT WIN32)
set_target_properties(thrive_native PROPERTIES LINK_FLAGS_RELEASE "-fuse-ld=gold")
else()
set_target_properties(thrive_native PROPERTIES LINK_FLAGS_RELEASE "")
# set_target_properties(thrive_native PROPERTIES LINK_FLAGS_RELEASE "")
endif()

install(TARGETS thrive_native)
14 changes: 12 additions & 2 deletions src/native/Include.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,17 @@
#ifdef WIN32
#define FORCE_INLINE __forceinline
#else
#define FORCE_INLINE __attribute__((always_inline))
#define FORCE_INLINE __attribute__((always_inline)) inline
#endif

#ifdef _MSC_VER
#define PACKED_STRUCT
#define BEGIN_PACKED_STRUCT __pragma( pack(push, 1) )
#define END_PACKED_STRUCT __pragma( pack(pop))
#else
#define PACKED_STRUCT __attribute__((packed))
#define BEGIN_PACKED_STRUCT
#define END_PACKED_STRUCT
#endif

#if _MSC_VER
Expand Down Expand Up @@ -67,7 +77,7 @@

namespace Thrive
{
constexpr float PI = 3.14159265;
constexpr float PI = 3.14159265f;
constexpr double PI_DOUBLE = 3.1415926535897932;

/// Always zero bytes in pointers that stuff extra info in them thanks to alignment requirements
Expand Down
4 changes: 2 additions & 2 deletions src/native/interop/CInterop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ using ShapePool = boost::singleton_pool<Thrive::Physics::ShapeWrapper, sizeof(Th
#endif

#ifdef JPH_ENABLE_ASSERTS
bool PhysicsAssert(const char* expression, const char* message, const char* file, uint line);
bool PhysicsAssert(const char* expression, const char* message, const char* file, unsigned int line);
#endif

int32_t CheckAPIVersion()
Expand Down Expand Up @@ -641,7 +641,7 @@ void PhysicsTrace(const char* fmt, ...)
}

#ifdef JPH_ENABLE_ASSERTS
bool PhysicsAssert(const char* expression, const char* message, const char* file, uint line)
bool PhysicsAssert(const char* expression, const char* message, const char* file, unsigned int line)
{
LOG_ERROR(std::string("Jolt assert failed in ") + file + ":" + std::to_string(line) + " (" + expression + ") " +
(message ? message : ""));
Expand Down
5 changes: 4 additions & 1 deletion src/native/interop/CStructures.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,17 @@ extern "C"
char RayData[PHYSICS_RAY_DATA_SIZE];
} PhysicsRayWithUserData;

typedef struct __attribute__((packed)) SubShapeDefinition
BEGIN_PACKED_STRUCT;
typedef struct PACKED_STRUCT SubShapeDefinition
{
JQuat Rotation;
JVecF3 Position;
uint32_t UserData;
PhysicsShape* Shape;
} SubShapeDefinition;

END_PACKED_STRUCT;

static const inline JQuat QuatIdentity = JQuat{0, 0, 0, 1};
}

Expand Down
16 changes: 8 additions & 8 deletions src/native/interop/JoltTypeConversions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,42 @@
namespace Thrive
{

FORCE_INLINE inline JPH::DVec3 DVec3FromCAPI(JVec3 vec)
FORCE_INLINE JPH::DVec3 DVec3FromCAPI(JVec3 vec)
{
return {vec.X, vec.Y, vec.Z};
}

FORCE_INLINE inline JVec3 DVec3ToCAPI(JPH::DVec3 vec)
FORCE_INLINE JVec3 DVec3ToCAPI(JPH::DVec3 vec)
{
return JVec3{vec.GetX(), vec.GetY(), vec.GetZ()};
}

FORCE_INLINE inline JPH::Vec3 Vec3FromCAPI(JVecF3 vec)
FORCE_INLINE JPH::Vec3 Vec3FromCAPI(JVecF3 vec)
{
return {vec.X, vec.Y, vec.Z};
}

FORCE_INLINE inline JVecF3 Vec3ToCAPI(JPH::Vec3 vec)
FORCE_INLINE JVecF3 Vec3ToCAPI(JPH::Vec3 vec)
{
return JVecF3{vec.GetX(), vec.GetY(), vec.GetZ()};
}

FORCE_INLINE inline JPH::Quat QuatFromCAPI(JQuat quat)
FORCE_INLINE JPH::Quat QuatFromCAPI(JQuat quat)
{
return {quat.X, quat.Y, quat.Z, quat.W};
}

FORCE_INLINE inline JQuat QuatToCAPI(JPH::Quat quat)
FORCE_INLINE JQuat QuatToCAPI(JPH::Quat quat)
{
return JQuat{quat.GetX(), quat.GetY(), quat.GetZ(), quat.GetW()};
}

FORCE_INLINE inline JColour ColorToCAPI(JPH::Float4 colour)
FORCE_INLINE JColour ColorToCAPI(JPH::Float4 colour)
{
return {colour.x, colour.y, colour.z, colour.w};
}

FORCE_INLINE inline JPH::Float4 ColorFromCAPI(JColour colour)
FORCE_INLINE JPH::Float4 ColorFromCAPI(JColour colour)
{
return {colour.R, colour.G, colour.B, colour.A};
}
Expand Down
2 changes: 1 addition & 1 deletion src/native/physics/ContactListener.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ContactListener : public JPH::ContactListener
// TODO: remove the chained listener feature if nothing is going to use it
JPH::ContactListener* chainedListener = nullptr;

uint32_t physicsStep = -1;
uint32_t physicsStep = std::numeric_limits<uint32_t>::max();

#ifdef JPH_DEBUG_RENDERER
JPH::DebugRenderer* debugDrawer = nullptr;
Expand Down
Loading

0 comments on commit 130ca10

Please sign in to comment.