Mac Support #90
Comments
|
In addition to the changes needed for #89, I'm finding that clang++ on macOS Monterey at least does not appear to be happy with in-place object instantiation/anonymous initialization. I by no means consider myself a C++ savant, so I'm not sure what the proper term for that is in this language, but for instance on line 173 of TFE_Asset/vueAsset.cpp: transforms.push_back({ rotScale, translation });fails in clang++, but: VueTransform transform = { rotScale, translation };
transforms.push_back(transform);works. That aside, stepping through the basics of a macOS build, I'll mention any particular gotchas here, then resolve the necessary changes between this and #89, then expect another PR with the code changes relevant to a functional build on both. That said, I'll make note that I may require someone else to run through building a Windows copy with those changes to ensure there are no regressions before merge, I don't have a Windows machine I could use for that currently. |
|
Is this still on the roadmap or being developed? |
|
I haven't touched anything personally in a while. There was some debate over the use of XDG variables in the past (not POSIX, don't apply to macOS) but most other stuff done for the Linux support, at least that I contributed to, should be POSIX and therefore constitute mac support as well. I seem to recall all of one time actually getting the OpenGL surface to pop up, but it was so long ago now, I don't know if that has since been paved over. All that to say, I suspect that it shouldn't be hard to get this over the finish line for macOS, but I'm not actively making any contributions at this point and couldn't tell you who is paying attention to portability concerns. If anyone does want to take a concerted swing at it I'm happy to help out. P.S. A bridge to this working may be experimenting with the codebase using XQuartz. That'll get you running on macOS without needing as much of the underlying toolkit stuff to be perfectly Cocoa-ready. |
|
(this is an upvote) |
|
I would also really like to see macOS support... |
|
At the time (two years ago) the work as I saw it entailed taking the Linux build at the time, trying to build it on Mac, and squashing compiler bugs as they arose. I don't know what state the Linux build is in today, but so long as Linux support bits are built in a POSIX-y way, the only Mac-specific bits would be Cocoa-related methinks, and imGUI may abstract most if not all of that away. Well that and the object instantiation mentioned above, clang doesn't like it. If that eases anyone's trepidation at taking a swing, that's what I would do if I had bandwidth to start studying this again, I don't remember running into a question I couldn't answer through available documentation back when I got an OpenGL surface to pop up on my Mac properly. Good luck! |
|
Is there still a chance the Mac version happens soon? ;-) |
|
Just leaving a comment as my vote for a macOS build. Thank you. |
|
Tried to build on an M1 Mac and thought I would note a few issues here, unclear if some of these are clang related or MacOS's STL at this time:
There are perhaps more issues, but this is what I had time to collect today. |
Are these 2 apple specific things? I don't see sprintf() being deprecated in any official standards document, and QUAD_MAX isn't predefined on linux/windows either. But the change is simple enough.
Yeah clang-only warnings,
Don't see those with clang on linux, can you please post the error messages? |
|
All the libraries that TFE depends on seem to be available on MacOS, so I thought I would try and build it and see how far I got. sprintf complains on windows as well I believe, but this is a simple macro to define to make the messages go away, and obviously it's just noise, not an absolute requirement to fix. Another one I missed, LINE_MAX My simple fix to get past the errors was just to add a read and write method that took size_t * instead of one of the project defined integer types. I was a little hesitant as this was my first 15 minutes looking at the actual code base and didn't want to modify something that might be reading or writing from data files where there are expected/required sizes of objects inside the files. I also modified CMakeLists line 59 to if(LINUX OR MACOS) to get it to build If I have a little more time, I could throw together a pull request on a few of the smaller changes |
How many bits does an "unsigned long" have on Apple? A bit strange that it does not match either the u32 or u64 specialization. Could you please test PR #419 ? Thanks! |
|
The sprintf->snprintf replacement should be fine, with the understanding this is an ANSI C99 extension. For the size_t thing, size_t is defined as the type returned by sizeof () on a particular platform, so ideally should only be used in this context. If something else is being coerced to size_t and back out, that may be something where the type should be redefined elsewhere. Could the issues be due to it being a pointer to size_t rather than size_t itself? C gets iffy on conversions between pointers of different types without a cast. This owes to some platforms historically having different pointer representations for different data types, even though this is quite uncommon nowadays. What I'd do is inspect whether these variables of size_t are truly defined as the result of the sizeof () operator. If they aren't, then size_t technically isn't the right type for them, even if they may for instance be array indices. This is especially true if the field is from a binary file, because it may have originated as a size_t on the host platform (MS-DOS, x86), but it has since been flattened into a machine long-word, and is no longer defined as the general result of sizeof () on , it is a 4-byte little-endian value which is best read out as bytes and composited into a uint32_t (or equivalent). Granted you may cast it to size_t in appropriate places, but just understand you need to make sure you have the reverse of that casting and compositing anywhere you need to go back to the binary format. |
I agree, I was trying minimal changes just to get it to build, this seems to be the culprit here (or atleast the first one that threw the errors):
Likely the case, from memoryRegion.cpp:475:
Presently, I have next to no understanding of the code base, but am aware it descends from dos, so until I have a better understanding of how the code base works, I'm hesitant to start making any bigger changes myself, I just wanted to see it build. An unsigned long, which is what size_t is defined as is 8 bytes, as is the size of pointers, just built a test program. |
Ah, change them to u64 and you should be good to go. And it'll make savegames from 32 and 64bit builds compatible as well. |
|
i.e.: index cf859d22..59565001 100644
--- a/TheForceEngine/TFE_Memory/memoryRegion.cpp
+++ b/TheForceEngine/TFE_Memory/memoryRegion.cpp
@@ -80,11 +80,11 @@ struct MemoryRegion
{
char name[32];
MemoryBlock** memBlocks;
- size_t blockArrCapacity;
+ u64 blockArrCapacity;
- size_t blockCount;
- size_t blockSize;
- size_t maxBlocks;
+ u64 blockCount;
+ u64 blockSize;
+ u64 maxBlocks;
};
static_assert(sizeof(RegionAllocHeader) == 16, "RegionAllocHeader is the wrong size.");
@@ -98,7 +98,7 @@ namespace TFE_Memory
static const u32 c_relativeOffsetMask = (1u << c_relativeBlockShift) - 1u;
void freeSlot(RegionAllocHeader* alloc, RegionAllocHeader* next, MemoryBlock* block);
- size_t alloc_align(size_t baseSize);
+ u64 alloc_align(u64 baseSize);
s32 getBinFromSize(u32 size);
bool allocateNewBlock(MemoryRegion* region);
void removeHeaderFromFreelist(MemoryBlock* block, RegionAllocHeader* header);
@@ -137,7 +137,7 @@ namespace TFE_Memory
}
}
- MemoryRegion* region_create(const char* name, size_t blockSize, size_t maxSize)
+ MemoryRegion* region_create(const char* name, u64 blockSize, u64 maxSize)
{
assert(name);
if (!name || !blockSize) { return nullptr; }
@@ -206,8 +206,8 @@ namespace TFE_Memory
if (header->size - size >= MIN_SPLIT_SIZE)
{
// Split.
- size_t split0 = size;
- size_t split1 = header->size - split0;
+ u64 split0 = size;
+ u64 split1 = header->size - split0;
RegionAllocHeader* next = (RegionAllocHeader*)((u8*)header + split0);
// Cleanup the free list.
@@ -231,7 +231,7 @@ namespace TFE_Memory
return (u8*)header + sizeof(RegionAllocHeader);
}
- void* region_alloc(MemoryRegion* region, size_t size)
+ void* region_alloc(MemoryRegion* region, u64 size)
{
assert(region);
if (size == 0) { return nullptr; }
@@ -283,7 +283,7 @@ namespace TFE_Memory
return nullptr;
}
- void* region_realloc(MemoryRegion* region, void* ptr, size_t size)
+ void* region_realloc(MemoryRegion* region, void* ptr, u64 size)
{
assert(region);
if (!ptr) { return region_alloc(region, size); }
@@ -336,8 +336,8 @@ namespace TFE_Memory
if (header->size - size >= MIN_SPLIT_SIZE)
{
// Split.
- size_t split0 = size;
- size_t split1 = header->size - split0;
+ u64 split0 = size;
+ u64 split1 = header->size - split0;
RegionAllocHeader* next = (RegionAllocHeader*)((u8*)header + split0);
// Reset the header.
@@ -408,9 +408,9 @@ namespace TFE_Memory
}
}
- size_t region_getMemoryUsed(MemoryRegion* region)
+ u64 region_getMemoryUsed(MemoryRegion* region)
{
- size_t used = 0;
+ u64 used = 0;
for (s32 i = 0; i < region->blockCount; i++)
{
used += (region->blockSize - region->memBlocks[i]->sizeFree);
@@ -418,13 +418,13 @@ namespace TFE_Memory
return used;
}
- void region_getBlockInfo(MemoryRegion* region, size_t* blockCount, size_t* blockSize)
+ void region_getBlockInfo(MemoryRegion* region, u64* blockCount, u64* blockSize)
{
*blockCount = region->blockCount;
*blockSize = region->blockSize;
}
- size_t region_getMemoryCapacity(MemoryRegion* region)
+ u64 region_getMemoryCapacity(MemoryRegion* region)
{
return region->blockCount * region->blockSize;
}
@@ -539,7 +539,7 @@ namespace TFE_Memory
return nullptr;
}
- size_t blockAllocStart = 0;
+ u64 blockAllocStart = 0;
file->readBuffer(region->name, 32);
if (region->blockArrCapacity == 0)
{
@@ -551,7 +551,7 @@ namespace TFE_Memory
}
else
{
- size_t blockArrCapacity, blockCount, blockSize, maxBlocks;
+ u64 blockArrCapacity, blockCount, blockSize, maxBlocks;
file->read(&blockArrCapacity);
file->read(&blockCount);
file->read(&blockSize);
@@ -669,7 +669,7 @@ namespace TFE_Memory
insertBlockIntoFreelist(block, alloc);
}
- size_t alloc_align(size_t baseSize)
+ u64 alloc_align(u64 baseSize)
{
return (baseSize + ALIGNMENT - 1) & ~(ALIGNMENT - 1);
}
@@ -771,7 +771,7 @@ namespace TFE_Memory
return false;
}
- size_t blockIndex = region->blockCount;
+ u64 blockIndex = region->blockCount;
assert(blockIndex < region->blockArrCapacity);
region->memBlocks[blockIndex] = (MemoryBlock*)malloc(sizeof(MemoryBlock) + region->blockSize);
if (!region->memBlocks[blockIndex])
@@ -799,12 +799,12 @@ namespace TFE_Memory
// Malloc = 0.005514 sec.
// Region = 0.000991 sec.
#define ALLOC_COUNT 20000
- const size_t _testAllocSize[] = { 16, 32, 24, 100, 200, 500, 327, 537, 200, 17, 57, 387, 874, 204, 100, 22 };
+ const u64 _testAllocSize[] = { 16, 32, 24, 100, 200, 500, 327, 537, 200, 17, 57, 387, 874, 204, 100, 22 };
void region_test()
{
u64 start = TFE_System::getCurrentTimeInTicks();
- const size_t mask = TFE_ARRAYSIZE(_testAllocSize) - 1;
+ const u64 mask = TFE_ARRAYSIZE(_testAllocSize) - 1;
void* alloc[ALLOC_COUNT];
for (s32 i = 0; i < ALLOC_COUNT; i++)
{
diff --git a/TheForceEngine/TFE_Memory/memoryRegion.h b/TheForceEngine/TFE_Memory/memoryRegion.h
index 034fe8bf..544633db 100644
--- a/TheForceEngine/TFE_Memory/memoryRegion.h
+++ b/TheForceEngine/TFE_Memory/memoryRegion.h
@@ -15,17 +15,17 @@ typedef u32 RelativePointer;
namespace TFE_Memory
{
- MemoryRegion* region_create(const char* name, size_t blockSize, size_t maxSize = 0u);
+ MemoryRegion* region_create(const char* name, u64 blockSize, u64 maxSize = 0u);
void region_clear(MemoryRegion* region);
void region_destroy(MemoryRegion* region);
- void* region_alloc(MemoryRegion* region, size_t size);
- void* region_realloc(MemoryRegion* region, void* ptr, size_t size);
+ void* region_alloc(MemoryRegion* region, u64 size);
+ void* region_realloc(MemoryRegion* region, void* ptr, u64 size);
void region_free(MemoryRegion* region, void* ptr);
- size_t region_getMemoryUsed(MemoryRegion* region);
- size_t region_getMemoryCapacity(MemoryRegion* region);
- void region_getBlockInfo(MemoryRegion* region, size_t* blockCount, size_t* blockSize);
+ u64 region_getMemoryUsed(MemoryRegion* region);
+ u64 region_getMemoryCapacity(MemoryRegion* region);
+ void region_getBlockInfo(MemoryRegion* region, u64* blockCount, u64* blockSize);
RelativePointer region_getRelativePointer(MemoryRegion* region, void* ptr);
void* region_getRealPointer(MemoryRegion* region, RelativePointer ptr);
|
|
Found one more instance of this, might u64 be a little large? As far as preserving compatibility with 32-bit systems, assuming this is actually a concern? index 9886fa91..805e7579 100644
--- a/TheForceEngine/TFE_Game/igame.cpp
+++ b/TheForceEngine/TFE_Game/igame.cpp
@@ -17,7 +17,7 @@ MemoryRegion* s_levelRegion = nullptr;
void displayMemoryUsage(const ConsoleArgList& args)
{
char res[256];
- size_t blockCount, blockSize;
+ u64 blockCount, blockSize;
region_getBlockInfo(s_gameRegion, &blockCount, &blockSize);
TFE_Console::addToHistory("-------------------------------------------------------------------");
TFE_Console::addToHistory("Region | Memory Used | Current Capacity | Block Count | BlockSize");
diff --git a/TheForceEngine/TFE_Memory/memoryRegion.cpp b/TheForceEngine/TFE_Memory/memoryRegion.cpp
index cf859d22..59565001 100644
--- a/TheForceEngine/TFE_Memory/memoryRegion.cpp
+++ b/TheForceEngine/TFE_Memory/memoryRegion.cpp
@@ -80,11 +80,11 @@ struct MemoryRegion
{
char name[32];
MemoryBlock** memBlocks;
- size_t blockArrCapacity;
+ u64 blockArrCapacity;
- size_t blockCount;
- size_t blockSize;
- size_t maxBlocks;
+ u64 blockCount;
+ u64 blockSize;
+ u64 maxBlocks;
};
static_assert(sizeof(RegionAllocHeader) == 16, "RegionAllocHeader is the wrong size.");
@@ -98,7 +98,7 @@ namespace TFE_Memory
static const u32 c_relativeOffsetMask = (1u << c_relativeBlockShift) - 1u;
void freeSlot(RegionAllocHeader* alloc, RegionAllocHeader* next, MemoryBlock* block);
- size_t alloc_align(size_t baseSize);
+ u64 alloc_align(u64 baseSize);
s32 getBinFromSize(u32 size);
bool allocateNewBlock(MemoryRegion* region);
void removeHeaderFromFreelist(MemoryBlock* block, RegionAllocHeader* header);
@@ -137,7 +137,7 @@ namespace TFE_Memory
}
}
- MemoryRegion* region_create(const char* name, size_t blockSize, size_t maxSize)
+ MemoryRegion* region_create(const char* name, u64 blockSize, u64 maxSize)
{
assert(name);
if (!name || !blockSize) { return nullptr; }
@@ -206,8 +206,8 @@ namespace TFE_Memory
if (header->size - size >= MIN_SPLIT_SIZE)
{
// Split.
- size_t split0 = size;
- size_t split1 = header->size - split0;
+ u64 split0 = size;
+ u64 split1 = header->size - split0;
RegionAllocHeader* next = (RegionAllocHeader*)((u8*)header + split0);
// Cleanup the free list.
@@ -231,7 +231,7 @@ namespace TFE_Memory
return (u8*)header + sizeof(RegionAllocHeader);
}
- void* region_alloc(MemoryRegion* region, size_t size)
+ void* region_alloc(MemoryRegion* region, u64 size)
{
assert(region);
if (size == 0) { return nullptr; }
@@ -283,7 +283,7 @@ namespace TFE_Memory
return nullptr;
}
- void* region_realloc(MemoryRegion* region, void* ptr, size_t size)
+ void* region_realloc(MemoryRegion* region, void* ptr, u64 size)
{
assert(region);
if (!ptr) { return region_alloc(region, size); }
@@ -336,8 +336,8 @@ namespace TFE_Memory
if (header->size - size >= MIN_SPLIT_SIZE)
{
// Split.
- size_t split0 = size;
- size_t split1 = header->size - split0;
+ u64 split0 = size;
+ u64 split1 = header->size - split0;
RegionAllocHeader* next = (RegionAllocHeader*)((u8*)header + split0);
// Reset the header.
@@ -408,9 +408,9 @@ namespace TFE_Memory
}
}
- size_t region_getMemoryUsed(MemoryRegion* region)
+ u64 region_getMemoryUsed(MemoryRegion* region)
{
- size_t used = 0;
+ u64 used = 0;
for (s32 i = 0; i < region->blockCount; i++)
{
used += (region->blockSize - region->memBlocks[i]->sizeFree);
@@ -418,13 +418,13 @@ namespace TFE_Memory
return used;
}
- void region_getBlockInfo(MemoryRegion* region, size_t* blockCount, size_t* blockSize)
+ void region_getBlockInfo(MemoryRegion* region, u64* blockCount, u64* blockSize)
{
*blockCount = region->blockCount;
*blockSize = region->blockSize;
}
- size_t region_getMemoryCapacity(MemoryRegion* region)
+ u64 region_getMemoryCapacity(MemoryRegion* region)
{
return region->blockCount * region->blockSize;
}
@@ -539,7 +539,7 @@ namespace TFE_Memory
return nullptr;
}
- size_t blockAllocStart = 0;
+ u64 blockAllocStart = 0;
file->readBuffer(region->name, 32);
if (region->blockArrCapacity == 0)
{
@@ -551,7 +551,7 @@ namespace TFE_Memory
}
else
{
- size_t blockArrCapacity, blockCount, blockSize, maxBlocks;
+ u64 blockArrCapacity, blockCount, blockSize, maxBlocks;
file->read(&blockArrCapacity);
file->read(&blockCount);
file->read(&blockSize);
@@ -669,7 +669,7 @@ namespace TFE_Memory
insertBlockIntoFreelist(block, alloc);
}
- size_t alloc_align(size_t baseSize)
+ u64 alloc_align(u64 baseSize)
{
return (baseSize + ALIGNMENT - 1) & ~(ALIGNMENT - 1);
}
@@ -771,7 +771,7 @@ namespace TFE_Memory
return false;
}
- size_t blockIndex = region->blockCount;
+ u64 blockIndex = region->blockCount;
assert(blockIndex < region->blockArrCapacity);
region->memBlocks[blockIndex] = (MemoryBlock*)malloc(sizeof(MemoryBlock) + region->blockSize);
if (!region->memBlocks[blockIndex])
@@ -799,12 +799,12 @@ namespace TFE_Memory
// Malloc = 0.005514 sec.
// Region = 0.000991 sec.
#define ALLOC_COUNT 20000
- const size_t _testAllocSize[] = { 16, 32, 24, 100, 200, 500, 327, 537, 200, 17, 57, 387, 874, 204, 100, 22 };
+ const u64 _testAllocSize[] = { 16, 32, 24, 100, 200, 500, 327, 537, 200, 17, 57, 387, 874, 204, 100, 22 };
void region_test()
{
u64 start = TFE_System::getCurrentTimeInTicks();
- const size_t mask = TFE_ARRAYSIZE(_testAllocSize) - 1;
+ const u64 mask = TFE_ARRAYSIZE(_testAllocSize) - 1;
void* alloc[ALLOC_COUNT];
for (s32 i = 0; i < ALLOC_COUNT; i++)
{
diff --git a/TheForceEngine/TFE_Memory/memoryRegion.h b/TheForceEngine/TFE_Memory/memoryRegion.h
index 034fe8bf..544633db 100644
--- a/TheForceEngine/TFE_Memory/memoryRegion.h
+++ b/TheForceEngine/TFE_Memory/memoryRegion.h
@@ -15,17 +15,17 @@ typedef u32 RelativePointer;
namespace TFE_Memory
{
- MemoryRegion* region_create(const char* name, size_t blockSize, size_t maxSize = 0u);
+ MemoryRegion* region_create(const char* name, u64 blockSize, u64 maxSize = 0u);
void region_clear(MemoryRegion* region);
void region_destroy(MemoryRegion* region);
- void* region_alloc(MemoryRegion* region, size_t size);
- void* region_realloc(MemoryRegion* region, void* ptr, size_t size);
+ void* region_alloc(MemoryRegion* region, u64 size);
+ void* region_realloc(MemoryRegion* region, void* ptr, u64 size);
void region_free(MemoryRegion* region, void* ptr);
- size_t region_getMemoryUsed(MemoryRegion* region);
- size_t region_getMemoryCapacity(MemoryRegion* region);
- void region_getBlockInfo(MemoryRegion* region, size_t* blockCount, size_t* blockSize);
+ u64 region_getMemoryUsed(MemoryRegion* region);
+ u64 region_getMemoryCapacity(MemoryRegion* region);
+ void region_getBlockInfo(MemoryRegion* region, u64* blockCount, u64* blockSize);
RelativePointer region_getRelativePointer(MemoryRegion* region, void* ptr);
void* region_getRealPointer(MemoryRegion* region, RelativePointer ptr);Made it to linking and can't find glew now, working on figuring out whatever MacOS does that is different than Linux now. |
Changing them to u32 will break existing savegames, since size_t == u64 on windows and linux.
Ooh, maybe you want to try my PR #418 which gets rid of GLEW entirely, all you need is a recent SDL2. |
|
Applied your apple compat branch on PR #418, and this latest patch file with the u64 replacements and it moves along, but there is another linker error with SDL now, CMAKE seems to be handing the include path just fine but something is going wrong with the linker library paths, so far, we seem to be moving in the right direction, though. I've probably spent more time on this than I should have today, though. |
Does this change to cmake work for you? Can you please post the linker error? Maybe someone else with index 8624b35c..d9949acf 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -32,8 +32,6 @@ endif()
if (UNIX AND NOT APPLE)
set(LINUX ON)
-elseif (UNIX AND APPLE)
- set(MACOS ON)
endif()
if(WIN32)
@@ -61,22 +59,16 @@ if(ENABLE_TFE)
add_executable(tfe)
set_target_properties(tfe PROPERTIES OUTPUT_NAME "theforceengine")
- if(LINUX)
+ if(UNIX)
find_package(PkgConfig REQUIRED)
- find_package(Threads REQUIRED)
find_package(SDL2 2.0.20 REQUIRED)
- pkg_check_modules(SDL2_IMAGE REQUIRED SDL2_image)
+ find_package(SDL2_Image REQUIRED)
pkg_check_modules(GLEW REQUIRED glew)
- set(OpenGL_GL_PREFERENCE GLVND)
- find_package(OpenGL REQUIRED)
target_include_directories(tfe PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
target_include_directories(tfe PRIVATE ${SDL2_INCLUDE_DIRS})
target_include_directories(tfe PRIVATE ${SDL2_IMAGE_INCLUDE_DIRS})
- target_link_libraries(tfe PRIVATE
- ${OPENGL_LIBRARIES}
+ target_link_libraries(tfe PRIVATE SDL2::SDL2main SDL2::SDL2 SDL2_image::SDL2_image
${GLEW_LIBRARIES}
- ${SDL2_LIBRARIES}
- ${SDL2_IMAGE_LIBRARIES}
)
if(NOT DISABLE_SYSMIDI)
|
|
Ideally whatever variables are in play with the size_t/u32/u64 business would be defined as a fixed size and endianness, promoting portability of binary assets across platforms and architectures. At the very least, binary I/O should read a byte at a time and use shifting and ORing to composite the value into whatever data type is used to then represent it in program logic, and likewise be partsed back out into individual bytes in a known endianness for saving. Think htonl or htons ala sockets handling. Pretty much the same idea. If you do that and always explicitly convert the binary value into a size_t in code the same way, then size_t can be used everywhere you've already parsed the data out, but then in turn whatever would write a size_t to a binary file anywhere convert it back. This would have the benefit of the code itself being clear this value is explicitly a sizeof ()/array index type, but binary handling would be clear this is stored as a 32-bit or 64-bit little-endian byte array, regardless of how the current host machine is passing it around in RAM. Sorry if you already know this, not assuming you don't just it's something on my mind a lot, I work with a lot of console (Famicom/Mega Drive particularly) code these days so "everything binary is a byte array in target byte order" is something I have to think about on the regular. Comes into play a little less with general purpose computers but is still very important for binary data. |
|
Oops also, these changes you've got diffs up of, are these checked into a branch/fork anywhere, or just posted here? I've got a macOS box, happy to pull it down and give it a try. |
There's #419 but it doesn't include the cmake update I posted above. You could also add #418 and #407 on top. 418 gets rid of GLEW and 407 fixes a copy-pasto which causes a crash on shutdown. |
|
Oh and also #282 for big-endian machines. I like it, but I can't test it since I have no BE userland for my MIPS toys. |
|
I don't have my diff checked in anywhere, I do make it all the way to the linking stage and my last issue was figuring out why cmake wasn't finding sdl2_image. It is unclear to me if there is an issue with the cmake file supplied by brew for sdl2_image When cmake runs it seems to find sdl2_image, but at the linking stage it does not include the library path, while it seems to for sdl2 itself. CMakeCache seems to show the library path folder in there, I was trying various tweaks to see if I could get cmake to cooperate, but no joy. I ran out of time due to school at the present time, but it would be nice to at least get it building and see what isn't working at that point. Since the project is depending on external libraries instead of system specific libraries, I don't see why it shouldn't mostly work. |
|
Doesn't include the library path as in it is missing a -L or a -l? I would imagine sdl2_image gets installed in the same prefix as other SDL2 stuff so if you find one, you should find all, but if it's missing specifically the -l include, I wonder if that could point to an issue with say pkg-config. Most UNIX-y build systems nowadays expect pkg-config to just tell it "add a -lSDL2_image" to the linker, and if the pkg-config file that gets scooped up for whatever reason thinks this is packed into the main SDL2 library, it may only provide -lSDL2, not -lSDL2_image. This is just conjecture though, I haven't looked at it yet, but I have seen in the past where a host winds up with multiple, copies of a library pc file, for instance one in /usr/lib/pkgconfig and one in /usr/share/pkgconfig on systems where these don't point to the same directory. Just a thought, that's the first thing I'd start investigating, because it's very unlikely that libSDL2.dylib and libSDL2_image.dylib are in different library paths. |
|
The libraries live in /opt/homebrew/Cellar/library_name and cmake is finding the library path here, but there are symlinks to all the libs in /opt/homebrew/lib. CMake does not seem to be inserting the full library path to SDL2_image in the linker options and I've been trying to workout why. It does seem to list the correct path in here, but my current cmake skills and time are limited to dig into it for a few days at least pkg-config returns the correct paths, so I believe something interesting is going on with cmake and/or homebrew.
|
|
Between those two files the main thing I see missing from the command line is: -L/opt/homebrew/lib You might try running the contents of link.txt directly with this added into the command line where the other -L options are. That'll at least isolate the problem to this missing from the linker command line. Since it's in pkg-config too, something else might be gobbling it up. What I do find interesting is you're getting two general paths to where the libs are: /opt/homebrew/lib Could that imply that there are conflicting pkg-config or possibly even CMake SDL detection scripts between those two prefixes? A curious bit is that the linking looks like so: So it establishes the cellar path as a library directory...then explicitly points at libSDL2.dylib in that directory...but then follows it also with a -lSDL2, so that is doubled up. There isn't a complementary -L option for either /opt/homebrew/lib or the Cellar directory for SDL2_image, so that's why -lSDL2_image isn't turning anything up. I wish I knew more about CMake to help you along but I've never used it from a packaging perspective. In any case, where I'd look next is if there's any difference between pkg-config and CMake files between: /opt/homebrew and /opt/homebrew/Cellar/{etc} Therein may lie the solution. Admittedly I don't use homebrew, so I'm also unfamiliar with their best practices regarding ensuring stuff is consistently locatable. Good luck, our environments are sufficiently different that I don't think I can offer further help. |
|
They are just symlinks to each other, homebrew seems to ship files that cmake uses to pickup library locations, I found a bug report from a couple years ago that suggested some trouble with sdl2_image and homebrew that was previously resolved, perhaps there is some other issue with it now, I know I could manually add the linker path to the files, just trying to get things working the "right" way. |
|
Oh yeah, I only suggested adding the path to confirm that's the only thing wrong, that way you're not chasing some unrelated gremlin. One other suggestion I have is the ultimate solution here may depend on whether you're intending to pack in SDL2 statically or expect the host machine to supply it. If you're going with static linkage, then I would just forego the homebrew and try and install SDL2 some other way, because if the homebrew-ization of it is what is causing trouble, doing something else will hopefully unstick you as a developer. On the other hand, if this is expected to be used with dynamic linkage, then it'd be worth some research to see how SDL2 is most commonly installed on macOS. Aside from homebrew, I see the SDL folks provide their own dmg: https://github.com/libsdl-org/SDL/releases/tag/release-2.30.5 If that installs differently than homebrew and is the more common SDL2 source for macOS users out there, that may be the way to go. A happy medium may be providing the dynamic libraries with an installation package for this and putting them in some sort of custom directory as not to interfere with any existing dylib on the target machine. Finally, back on the "pack it in" angle, SDL2 is in GitHub, like this project, so there is the prospect of pulling it in as a formal submodule dependency, that way both the project has a link to upstream and you're able to easily pack it in statically as part of the build rather than futzing with all the "find this library on the host" stuff. Many ways to crack this nut, best of luck on whichever you decide! |
|
Assuming TFE were officially supporting MacOS, it might make some sense to use the sdl components inside the tree, I see it's already there for Windows. There is a strong chance that the majority of MacOS users might not have sdl lying around like in the same way Linux users would. |
|
Yeah if that's what's happening for Windows, probably fine for macOS too. Way back when I did some Linux work on this stuff, I linked almost everything dynamically in the dinky little Makefile I made for my own purposes, so frankly I don't know how SDL2 is linked on Linux in this package's conventional build. I would do the same everywhere in that case though, since this project keeping Windows/macOS on a fixed version. Then you're not chasing gremlins related to an SDL version difference on different platforms. |
|
Hello All ;-) I just wanted to check if there is any hope to get the Mac version in the near future? I cannot help coding, but I can help testing ;) Please let me know. |
|
Gave it a shot today, after installing glib-2.0, pkg-config, SDL2, and SDL2_image, CMake is failing to find the latter: Annoyingly it does find SDL itself. The two are located via I don't use CMake at all so I don't know where to start on this one. Note this is using the dmg copies of SDL2 and SDL2_image as distributed by the SDL folks themselves, no idea how this behaves via homebrew. |
|
So replaced the second line with (I chose 2.6.0 based on release dates, not a dependency analysis): and CMake can actually complete and generate a Makefile. However, it then isn't able to find the SDL2_image stuff...a verbose listing of the build reveals Indeed /Library/Frameworks/SDL2.framework/Headers is pulled in by the found SDL2...but the corresponding directory for SDL2_image does not come in. This may point to further reason to probably just use a packed in SDL2, the situation on macOS trying to incorporate SDL2_image seems dire...anywho if I do discover anything else I'll report back. |
|
Got it, SDL2_image's CMake config file (on macOS at least) doesn't define SDL2_IMAGE_* prefixed include and link paths, rather, it exports a library interface of SDL2_image::SDL2_image. This turns this chunk from to With this, I'm able to get a 100% compile. However, the linking step then fails. Still, progress. Not sure then if the above still works on other UNIX-like targets, but it does fix the compile steps on macOS. |
|
For anyone who would like to drink from the fire-hose, this is the linker command-line and the resulting error: Presumably either the linking step is missing object definitions for the files containing those bits or they aren't being built in the first place. Not in a place I can sink more of my own research into this but hopefully this gets concerned parties further along. In short:
|
|
in TheForceEngine/TFE_FileSystem/CMakeFiles.txt index 5c524aa8..a4ba6151 100644
--- a/TheForceEngine/TFE_FileSystem/CMakeLists.txt
+++ b/TheForceEngine/TFE_FileSystem/CMakeLists.txt
@@ -4,7 +4,7 @@ if(WIN32)
"${CMAKE_CURRENT_SOURCE_DIR}/fileutil.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/paths.cpp"
)
-elseif(LINUX)
+elseif(UNIX)
target_sources(tfe PRIVATE
"${CMAKE_CURRENT_SOURCE_DIR}/filestream-posix.cpp"
"${CMAKE_CURRENT_SOURCE_DIR}/fileutil-posix.cpp"
''' |
Works for me(tm) on Linux, though I'm not sure whether it works with the github job (due to its old ubuntu version). |
This did the trick for part of it, which then lead to an issue finding "st_mtim" on struct stat. Digging in a little bit, this is defined in POSIX...so much for Apple's whole certification process, looks like Darwin's libc exposes this field instead as "st_mtimespec" leading to this needing to replace the single line at line 363 in TFE_FileSystem/fileutil-posix.cpp: But macOS gets to call itself UNIX while Linux does not...hooray for standards! Anywho, in addition to this, TFE_System/CrashHandler/CMakeLists.txt needs the same change from the LINUX to UNIX constant. With this all in place, it finally builds and links, although I did have to manually fudge the build to spit out the executable as "tfe" rather than "theforceengine". I was getting: Despite Darwin being a "UNIX" system (haw) and that implying it should have case-sensitive filenames, this seems to happen if the linker tries to create any file with a case-insensitive name that matches an existing path in the build directory. One such path is "TheForceEngine". If I copy out the link line and type this as tfe, it works. So in conclusion the remaining volley of build errors I found need fixing are:
Now, trying to run it, this happens, which I'm not going to look at today Granted I didn't put any assets in place, this was just trying to launch at all. Something with the OpenGL capability polling down in the renderer. I was able to get the OpenGL surface to pop up on macOS once upon a time...so it should just be some bug squashing in the renderer. If I do pick this thread up again sometime I might dig around in that further. Good luck! |
|
Thanks for these hints, I was also able to successfully compile an Intel 32bit build of the latest version of the source on my MacBook running Snow Leopard, however the binary also refuses to initialize the GPU due to insufficient GL capabilities (GeForce 9600M GT). |
|
@gilmorem560 does this patch help? index 960dc91e..82868a36 100644
--- a/TheForceEngine/TFE_RenderBackend/Win32OpenGL/renderBackend.cpp
+++ b/TheForceEngine/TFE_RenderBackend/Win32OpenGL/renderBackend.cpp
@@ -120,6 +120,13 @@ namespace TFE_RenderBackend
return nullptr;
}
+#ifdef __APPLE__
+ SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1);
+ SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 4);
+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
+#endif
SDL_GLContext context = SDL_GL_CreateContext(window);
if (!context) |
Same error on 10.6 Snow Leopard, 10.10 Yosemite and 10.13 High Sierra - didn't test on later OSX versions (yet). |
|
what if you change MAJOR_VERSION from 4 to 3, and MINOR_VERSION from 1 to 3? |
|
same error - tried it with a couple different numbers down to 2.0 but doesn't change anything unfortunately. |
|
So on my machine at least the capability check is failing for two reasons:
I think what's happening here is a result of mix-matching SDL and native OpenGL calls. For instance, in TFE_RenderBackend/Win32OpenGL/openGL_Caps.cpp, the OpenGL major/minor version are being requested via: Both gl_maj and gl_min are 0 on my machine after these lines run. However, if I swap these with I get back a major.minor of 2.1. Still, the minimum requirements (defined as DEV_TIER_2) appear to be OpenGL 3.3 capabilities and a maximum texture buffer size of at least 0x10000. I'm not sure if there is an SDL equivalent of the texture buffer size lookup. I tried briefly to find one to no avail. Some Googling around suggests that getting higher OpenGL profiles than 2.1 to work on macOS is a mixed bag. I wonder if this simply means macOS may need some significant work in the renderer. However, I do recall getting an OpenGL surface to pop up in Cocoa for this once upon a time...so there is probably a way. If I figure out any more I'll share that info here. |
Adding just the last three actually finally got me a surface, looks like without the hints OpenGL defaults to 2.1 on macOS. With the hints I get OpenGL 4.1 on my machine. This also resolves the matter of not getting a maximum texture buffer size. So in addition to adding this to the context setup right before creating the context (I chose 3.3 because that's the minimum the code is looking for): I also had to edit the capability check because it was only checking for extensions that can provide the 3.3 experience on lower versions. In other words, the check on capabilities goes from: to The reason being that the individual checks are being done by looking up certain extension names, extension names that are no longer present when a given extension gets promulgated into the actual profile. Doesn't mean that bit isn't supported, just means it's been incorporated into the core profile and no longer exposes that extension handle. This can probably be done more gracefully to allow macOS machines that do reach tier 3 (OpenGL 4.5) to also utilize whatever that adds in, but in any case this should be the root of getting a surface. After I make these changes, TFE launches and creates a full-screen, albeit blank, OpenGL surface. In the console I get the following: Looks like the next domino is the shader GLSL versions. Hopefully all of these notes help. |
|
https://stackoverflow.com/questions/68268197/is-glsl-1-30-always-supported-in-opengl-3-x-4-x Food for thought:
Sounds like the GLSL version may be suspect in this specific case. Since this is getting into the realm of potentially reworking components altogether, I think the solution to this one will need some attention from @luciusDXL and other central folks. In any case, the details in this thread should provide a roadmap for getting the build system and basic C++ bits behaving better. |
The Force Engine should compile and run on recent OS X platforms, including M1 Macs.
The text was updated successfully, but these errors were encountered: