macOS Apple Silicon support via Mesa Zink → KosmicKrisp → Metal - #2991
Draft
iamaperson000 wants to merge 28 commits into
Draft
macOS Apple Silicon support via Mesa Zink → KosmicKrisp → Metal#2991iamaperson000 wants to merge 28 commits into
iamaperson000 wants to merge 28 commits into
Conversation
This was referenced May 30, 2026
Collaborator
|
I see changes in the libs dir, I would prefer that third party libraries do not receive custom changes on our side: that creates complications if and when we choose to pull later versions. If there's issues with them, then I'd prefer to see them fixed at source unless there's a compelling reason to do otherwise. I'll have to dive in deeper, but that's something to bare in mind. If the changes in libs can be reduced down to the absolute minimum (ideally none) then it will be easier to assess the long term impact of such changes. |
| ProcessorMasks GetProcessorMasks() { | ||
| ProcessorMasks masks; | ||
|
|
||
| unsigned int numCores = std::thread::hardware_concurrency(); |
Collaborator
There was a problem hiding this comment.
Won't this report every core, including the efficiency cores? This will be disastrous for performance.
Could you use something like sysctlbyname("hw.perflevel0.physicalcpu", ... to count the number of performance cores?
Also during threads pinning, rather than do nothing (because you can't pin them on Mac) you could mark the threads as QOS_CLASS_USER_INTERACTIVE - without having too many performance threads.
Contributor
There was a problem hiding this comment.
Given that modern Intel CPUs have efficiency cores, wouldn't it make sense to split this out into its own change rather than it being Arm/Mac specific?
Contributor
Author
|
Thanks for catching that, and you were right! Should be good now, please let me know if there is anything else. Oh and for the libs dir, I mean I guess we could technically commit upstream but I feel that it would take months, and am not sure if it is worth it. Happy to do something else though. What are your thoughts? |
Adds the foundational macOS/Apple Silicon platform-support:
Platform-specific code (new files):
- rts/System/Platform/Mac/CpuTopology.cpp: stubs the CPU
topology API on top of std::thread::hardware_concurrency()
and sysctl for cache sizes (macOS exposes no portable
per-core P/E topology).
- rts/System/Platform/Mac/ThreadSupport.cpp: native pthread
wrappers (Suspend/Resume are no-ops on macOS).
Build system (CMake):
- if(APPLE)/NOT APPLE branches in rts/CMakeLists.txt,
rts/builds/{legacy,dedicated}/CMakeLists.txt,
rts/lib/glad/CMakeLists.txt, rts/System/CMakeLists.txt,
test/CMakeLists.txt, and tools/unitsync/CMakeLists.txt so
the Mac branch picks up the new Platform/Mac sources,
pulls libunwind only on non-Apple UNIX, and links
Foundation / objc / EGL (the latter via find_library with
Homebrew/MESA_PREFIX hints).
Mac-gated source changes:
- Rendering/GlobalRendering.cpp: EGL-on-CAMetalLayer path
(Kopper/Zink via Mesa) for context creation and
SwapBuffers, all behind #if defined(__APPLE__).
- Game/LoadScreen.cpp, Rendering/GL/{myGL.cpp,glxHandler.*}:
skip GLX (Mac has no X server) and skip the Lua intro
screen (EGL/Metal incompatibility), all behind #ifdef.
- System/Platform/{ThreadAffinityGuard.cpp,.h}: stub the
affinity API on macOS, which exposes no portable
equivalent of sched_setaffinity.
- System/MemPoolTypes.h, Sim/Units/Unit.cpp: Apple-only
fallbacks where pthread_t is an opaque pointer and where
std::views::enumerate is unavailable in older Apple Clang
libc++.
- test/other/testMutex.cpp: use os_unfair_lock instead of
linux/futex.h on macOS.
Mac-driven but platform-neutral fixes:
- System/Platform/Threading.cpp: replace std::ranges::find_if
with std::find_if (still C++17, compiles everywhere).
- System/SafeUtil.h: add missing <type_traits> include
needed by libc++.
- lib/smmalloc/smmalloc.h: relax POD static_assert to
is_trivially_copyable_v (avoids deprecated is_trivial).
- lib/smmalloc/smmalloc_generic.cpp, lib/assimp/include/
assimp/{matrix3x3,matrix4x4,quaternion,vector2,vector3}.inl:
add missing <cstdlib>/<cmath> includes (libstdc++
transitively included them; libc++ does not).
- AI/Wrappers/CUtils/Util.c: const-correct the macOS branch
of util_fileSelector to match Apple's scandir signature.
Linux and Windows code paths are either unchanged
(#ifdef-guarded) or pick up trivially-compatible standard
library calls; this commit is additive from their
perspective.
- FindSDL2.cmake: Add parent directory to include path so #include <SDL2/SDL.h> works with macOS SDL2 config (which sets include to /include/SDL2 directly) - FindLibunwind.cmake: Use INTERFACE IMPORTED library target on macOS (fixes -framework treated as file path by Unix Makefiles generator) - glad/CMakeLists.txt: Exclude glad_glx.c on Apple (no GLX on macOS) - legacy/headless CMakeLists.txt: Suppress -no_warn_duplicate_libraries on macOS (benign transitive dependency duplicates) - include/AL/: Vendored OpenAL EFX headers (macOS OpenAL.framework has no EFX support; needed for sound system compilation)
- rts/CMakeLists.txt: DevIL CMake module sets IL_INCLUDE_DIR to the directory containing il.h (e.g. /include/IL), but code uses #include <IL/il.h>. Add parent directory to include path. - gladstub.cpp: Add missing GL function stubs needed for headless build (glMultiDrawArraysIndirect, glTexStorage1D, glDebugMessage*, GLAD_GL_ATI_meminfo, GLAD_GL_NVX_gpu_memory_info)
- FastMath.h: break circular dependency between math::sqrtf and streflop_cond.h's std::hypot workaround on __APPLE__ by providing a temporary declaration before the include - Add Cpp23Compat.hpp: polyfill for std::views::enumerate (Apple libc++ doesn't support this C++23 feature yet), following the pattern of existing Cpp17Compat.hpp - headless CMakeLists: re-link engineCommonLibraries after GameHeadless to fix macOS single-pass linker symbol resolution - gflags: set GFLAGS_NAMESPACE to "google;gflags" since subdirectory builds default to "gflags" only but engine code uses "google"
smmalloc.h defines `#define INLINE inline` which leaks into GLTFParser.cpp and conflicts with simdjson's layout_mode::INLINE enum member, causing compilation failures whenever both headers are included in the same translation unit (surfaces first on macOS where the system simdjson header triggers this code path).
- assimp: Resolve ambiguous math function calls (fabsf, fabs, sqrt, etc.) that fail with Clang's stricter overload resolution. Use explicit std:: qualified calls and static_cast where needed. - smmalloc: Add missing <type_traits> include and switch to <cstdlib> for C++ header consistency.
- float3.h: Clang template instantiation compatibility - SafeUtil.h: Clang constexpr handling; use is_trivially_default_- constructible for portability - MemPoolTypes.h: factor pthread_t/Win32/Linux thread-id logging into a helper so libc++ on Apple (where pthread_t is a pointer) formats cleanly - Util.c: drop dead __APPLE__/non-__APPLE__ branch for util_fileSelector (both branches had identical const struct dirent* signatures) - SolLua bind/*.cpp: sol::nil -> sol::lua_nil (sol3 compatibility with libc++ where sol::nil is unavailable on some configurations)
The projectile / explosion-FX texture atlas Finalize() can fail (atlasAllocator Allocate() returns false), leaving atlasTex null. CProjectileDrawer::Init then calls GetTexID() -> GL::TextureBase::GetId() on null atlasTex, faulting at offset 0x8. Guard GetTexID() / DisOwnTexture() to no-op on a null atlasTex so a failed atlas degrades gracefully instead of crashing. This is a defensive fix that helps any platform whose atlas allocation can fail. Also: LuaTextures::Create now logs size/format/glError when glTexImage fails (was a silent return-nil), aiding diagnosis. LoadScreen exposes a runtime toggle for CLuaIntro under macOS via the SPRING_MAC_ENABLE_LUAINTRO env var; the existing #if defined(__APPLE__) block is preserved and the env-var check is inside it.
SaveWindowPosAndSize was storing backing pixels (e.g. 2560x1440 on a 1280x720 logical Retina window), so the next launch restored a 2x-too- big window that was then clamped to the screen, producing a portrait sliver. Store the logical size from SDL_GetWindowSize instead of the backing size from SDL_GL_GetDrawableSize. Affects HiDPI Linux setups symmetrically.
The engine checks for ARB_multitexture, ARB_texture_env_combine, ARB_texture_compression, ARB_texture_float, ARB_texture_non_power_of_two, and ARB_framebuffer_object extensions by name. Per the GL spec, these were folded into core GL 1.3-3.0; core-profile contexts no longer advertise them by name, but their functionality is guaranteed. The name-only check is a false-negative on any core-profile context. Skip it when the active context is core profile so the engine doesn't spuriously reject otherwise-valid configurations.
Replaces a hardcoded absolute libEGL.dylib path from an earlier bring-up checkpoint with a SPRING_MAC_LIBEGL CMake cache variable (default empty -> no Mesa link). Configure with: cmake -DSPRING_MAC_LIBEGL=/opt/homebrew/opt/mesa/lib/libEGL.dylib ... When set, the engine also skips linking Apple's OpenGL.framework and uses Mesa's libGL.dylib (looked up next to libEGL.dylib) instead. Loading OpenGL.framework on macOS 26 registers an NSWindow notification observer that bus-errors in +[NSOpenGLContext currentContext] during window setup.
Each EGL bring-up step now prints the result + last error to stderr. Quickly identifies whether failure is in eglGetDisplay, eglInitialize, eglBindAPI, eglChooseConfig, eglCreateContext, or eglMakeCurrent. On Homebrew's stock Mesa on macOS 26 the failure is at eglInitialize because that Mesa was built only for the X11 platform.
Engine-level changes to bring up the GL context on Apple Silicon (macOS 26 / M-series) through a Mesa libEGL built for the surfaceless platform, driving Zink (OpenGL-on-Vulkan) against the KosmicKrisp Vulkan driver (Vulkan-on-Metal): 1. CMake: when SPRING_MAC_LIBEGL is set, link Mesa libGL only if a libGL.dylib sits next to it. libGL is not strictly required at link time — all GL entry points get resolved through eglGetProcAddress at runtime, so libEGL alone is enough. 2. EGL: switch eglChooseConfig from EGL_WINDOW_BIT to EGL_PBUFFER_BIT. Mesa's surfaceless EGL platform doesn't expose window-bit configs; presentation happens via CAMetalLayer + KosmicKrisp WSI (Vulkan -> Metal). 3. EGL: walk OpenGL versions 4.6 -> 3.2 calling eglCreateContext with CORE profile, take the first that succeeds. Mesa/Zink rejects both empty attribs (returns default GL 2.1 which the engine then rejects) and (3.0 + CORE) since profile attrs only apply to 3.2+. 4. EGL: dump renderer/version/vendor/GLSL strings right after eglMakeCurrent to make Zink-on-KosmicKrisp issues visible. The matching change to skip the legacy ARB extension-name check in CheckGLExtensions on a CORE-profile context was landed in an earlier commit on this branch. Required runtime env: EGL_PLATFORM=surfaceless MESA_LOADER_DRIVER_OVERRIDE=zink GALLIUM_DRIVER=zink MESA_GL_VERSION_OVERRIDE=4.6 MESA_GLSL_VERSION_OVERRIDE=460 VK_ICD_FILENAMES=<mesa-install>/share/vulkan/icd.d/kosmickrisp_mesa_icd.aarch64.json DYLD_LIBRARY_PATH=<mesa-install>/lib Result: GL 4.6 (Core Profile) Mesa 26.2.0-devel, renderer 'zink Vulkan 1.3(Apple M4 (MESA_KOSMICKRISP))', GLSL 4.60. GL4 mode enabled. Clean engine startup + graceful shutdown.
The surfaceless Mesa EGL can't make a window surface, so the
engine renders into an off-screen pbuffer that eglSwapBuffers
never presents -> white window. Add a manual present: read the
rendered framebuffer back (glReadPixels, BGRA8) and blit it onto
the window's CAMetalLayer drawable via Metal, then present.
- rts/System/Platform/Mac/MetalPresent.{h,mm}: MRC-safe Metal
helper. MacMetalPresent_Init(layer) sets up MTLDevice/queue and
configures the CAMetalLayer (BGRA8, framebufferOnly=NO).
MacMetalPresent_PresentBGRA() uploads a CPU BGRA buffer to a
staging texture, blits it into nextDrawable, presents, commits.
Optional vertical flip for GL bottom-up readback.
- System/CMakeLists.txt: build MetalPresent.mm in the Mac
platform sources.
- builds/legacy/CMakeLists.txt: link Metal + QuartzCore
frameworks on Apple.
- GlobalRendering.cpp: stash the CAMetalLayer (g_metalLayer);
SPRING_MAC_PRESENT_TEST now drives the flash through this path.
Confirmed: the window shows the rendered clear color (red)
instead of staying white -- the present mechanism works end to
end (GL/Zink -> KosmicKrisp -> glReadPixels -> Metal -> window).
Next: wire MacMetalPresent_PresentBGRA into
CGlobalRendering::SwapBuffers (with flipY) so real frames
present, and successively fix the load-time crashes
(CProjectileDrawer atlas, etc.) to reach the draw loop. The
glReadPixels roundtrip is a stopgap; IOSurface GL/Metal interop
is the perf follow-up.
- SwapBuffers: on the macOS/EGL path, read the rendered default framebuffer back (glReadPixels BGRA, flipY) and blit it to the CAMetalLayer via MacMetalPresent each frame, then SDL_PumpEvents() so CoreAnimation composites (we replaced SDL_GL_SwapWindow which used to service the run loop). - InitEGLContext: size the pbuffer to the window's *backing* pixels (SDL_GetWindowSize points * backingScaleFactor) instead of logical points, so full Retina-resolution rendering isn't clipped. Init MacMetalPresent here. - MetalPresent.mm: log nil drawables. - Debug: SPRING_MAC_DUMP_FRAME=<prefix> dumps rendered frames to raw files (header w,h + BGRA) for offline inspection without screen capture. Confirmed the present path is correct: a dumped load-time frame is solid black because an earlier prototype disables the Lua loading-screen renderer on macOS (CLoadScreen::Draw only draws when luaIntro != nullptr; it's skipped here), so nothing is drawn during load. Real imagery requires reaching CGame's draw loop past the CProjectileDrawer atlas crash.
Replace the SwapBuffers path's full-frame CPU pixel copy + Y-flip memcpy + MTLTexture replaceRegion upload with an IOSurface-backed MTLTexture. The engine writes glReadPixels output directly into the IOSurface's CPU view (honoring its rowBytes via GL_PACK_ROW_LENGTH), and a one-triangle Metal render pass samples the same surface and Y-flips into the drawable. Adds: - MacMetalPresent_AcquireIOSurfaceBuffer(w, h, &rowBytes) returns a locked CPU pointer also bound as an MTLTexture; recreates the backing only when dimensions change. - MacMetalPresent_PresentIOSurface(flipY) unlocks the surface, encodes a cached render pipeline state (flip / non-flip), and presents the drawable. - IOSurface.framework added to the legacy build's link list. The original MacMetalPresent_PresentBGRA is kept for the early-splash callsite, and as a runtime fallback selectable via SPRING_MAC_LEGACY_PRESENT=1. Notes: - Apple Silicon's IOSurface picks 64-pixel row alignment, so at width 2940 rowBytes is 11776 (= 2944 pixels per row, 4 pixels of padding). Honor it via GL_PACK_ROW_LENGTH or the readback tears. - Logs '[MetalPresent] IOSurface zero-copy path active (WxH, rowBytes=N)' once on first acquire, and a corresponding 'LEGACY CPU-staging path active' line if the fallback is taken.
ReadWindowPosAndSize bound winSize / viewSize to GetMetalDrawableSize() on the macOS surfaceless path, but the engine renders into a backing- resolution pbuffer FBO. They were equal by accident at full Retina (drawable == backing == pbuffer size); with non-1x render scales they diverge -> glViewport(0,0,drawableW,drawableH) on a smaller FBO meant only one quadrant of geometry landed. Bind to the FBO size instead. This is the latent-bug shape; HiDPI Linux setups that ever render into a smaller FBO than the drawable would hit the same issue. No behavior change in the default same-size case.
Mesa 26.2 Zink grants a 4.6 compatibility-profile context on Apple Silicon via KosmicKrisp (verified). The EGL init now prefers compatibility (version walk 4.6 -> 3.2) and falls back to core only if compat is refused. Set SPRING_MAC_GL_CORE=1 to force core. Compatibility profile is a strict superset of core: every modern GL4 feature is available AND legacy paths (immediate mode, display lists, the fixed-function matrix stack, '#version ... compatibility' GLSL) keep working. Several Lua-built shaders in BAR rely on those legacy paths, so the compat profile is the easier integration point on the macOS path. Geometry shaders remain unavailable regardless of profile because Vulkan reports geometryShader=false on Metal; that is a separate problem and not affected by this change.
Adds a glReadPixels-based frame dump hook in SwapBuffers, gated by the SPRING_FRAME_CAPTURE env var. When set, the engine writes the default FBO contents to <prefix>.<frame>.raw before each present (use raw2png.py to convert). Useful for verifying headless rendering output without needing a window-system. The hook is no-op when the env var is unset, so the default behavior on every platform is unchanged.
Three env-gated features useful on any GL backend that uses glReadPixels for present: - SPRING_NO_PBO=1: disable double-buffered async PBO readback (default is ON; PBO hides the glReadPixels GPU pipeline drain behind 1 frame of present latency) - SPRING_DOWNSAMPLE_READBACK=N: blit-downsample by N before readback - SPRING_TIME_PRESENT=1: per-60-frame stage timing breakdown Also adds SPRING_MAC_NO_RETINA=1 to render the pbuffer at logical (1x) resolution instead of full backing (Retina) size — Apple-Silicon specific perf knob, no effect on other platforms. PBO async readback default-on gave ~3x FPS in busy scenes on the macOS Zink+KosmicKrisp path (sync 41 ms busy -> PBO 13 ms steady). Behavior is unchanged on platforms that don't engage the readback present path.
CLuaIntro was previously disabled on macOS as a workaround for the core-profile shader path. With the compat-profile context now the default (see earlier commit on this branch), the loading-screen text / splash / progress works correctly via the engine font renderer. Flip the macOS default to ENABLED, and switch the env-var escape hatch to opt-OUT: set SPRING_MAC_DISABLE_LUAINTRO=1 to skip CLuaIntro and fall back to the simple black load screen. Non-macOS platforms are unchanged (CLuaIntro has always been on by default there).
Metal (via Mesa Zink / KosmicKrisp on Apple Silicon) has no geometry- shader stage; Vulkan reports geometryShader = false on that path. But Mesa advertises GL_MAX_GEOMETRY_OUTPUT_VERTICES > 0 regardless, so the engine cannot detect the missing capability via GL introspection. Strip GS from Lua-loaded shader programs on macOS so the program at least links. Non-macOS platforms continue to honor the shader author's intent -- Linux / Windows GL drivers support geometry shaders, and any custom Lua shader using GS would have been silently broken on those platforms by the previous unconditional strip. Widgets that need GS-style point expansion have a Lua-layer NoGS fallback in the BAR widget tree (separate PR), so the engine-level strip is a fail-safe rather than the primary mechanism.
SDL emits mouse events in logical (point) coordinates. On the macOS surfaceless+pbuffer path the engine viewport is in backing-pixel coordinates (winSize / viewSize are tied to the pbuffer FBO; see GlobalRendering::ReadWindowPosAndSize). Without rescaling, windowed- mode clicks land at half the cursor position on Retina displays. Add two static helpers (ScaleMouseCoords, ScaleMouseDelta) gated by #if defined(__APPLE__), and route MOUSEMOTION / MOUSEBUTTONDOWN / MOUSEBUTTONUP through them. Non-macOS platforms are untouched - the #else branch matches the prior behavior exactly.
LuaParser.cpp:127 and LuaHandleSynced.cpp:435 call
LuaLibs::OpenSynced, which is only defined in Lua/LuaLibs.cpp.
A previous cherry-pick dropped that file from both the
dedicated server and unitsync source lists, leaving the bare
declaration in LuaLibs.h to satisfy compilation while breaking
the link on every platform.
Re-add ${ENGINE_SRC_ROOT_DIR}/Lua/LuaLibs.cpp to both targets
so OpenSynced is actually linked in.
The glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES) probe and the first L_WARNING log sat OUTSIDE the macOS-only #if, so any Linux/Windows Lua shader carrying a geometry stage would emit a spurious warning every compile and pay for an unnecessary GL query. Move both inside the __APPLE__ block alongside the existing "GS unconditionally stripped on macOS" log and geomSrcs.clear() call. Non-Apple builds now ignore non-empty geomSrcs as before.
This was referenced Jun 18, 2026
tomjn
suggested changes
Jun 18, 2026
tomjn
left a comment
tomjn
left a comment
Contributor
There was a problem hiding this comment.
Over-gating in glxHandler / myGL — the #ifndef __APPLE__ wraps are redundant
master already handles macOS in glxHandler.cpp via its existing inner guard:
#if !defined(HEADLESS) && !defined(_WIN32) && !defined(__APPLE__)
... real X11/GLX impl (includes glad_glx.h / SDL_syswm.h) ...
#else
void GLX::Load(SDL_Window*) {}
void GLX::Unload() {}
bool GLX::GetVideoMemInfoMESA(int*) { return false; }
#endifOn macOS the #else branch is already taken, so GLX::* exist as no-op stubs and the X11 headers are never included. glxHandler.h is pure C++ (no platform headers). So wrapping the whole file in an extra #ifndef __APPLE__ is redundant — and because it removes those stubs on macOS, it's the only reason the myGL.cpp gate became necessary. Dropping all three restores the minimal diff with identical behavior.
Apply all three together (the myGL one depends on the glxHandler stubs being present on macOS).
rts/Rendering/GL/glxHandler.h
On the added #ifndef __APPLE__ (line 1), delete it — empty suggestion:
On the added #endif // __APPLE__ (last line), delete it — empty suggestion:
rts/Rendering/GL/glxHandler.cpp
On the added #ifndef __APPLE__ (line 1), delete it — empty suggestion:
On the added #endif // __APPLE__ (last line), delete it — empty suggestion:
rts/Rendering/GL/myGL.cpp
Anchor a comment spanning the added #ifndef __APPLE__ … #endif block (the four added lines plus the return GLX::... line) and replace with the single stub call — once glxHandler is un-wrapped, GLX::GetVideoMemInfoMESA already returns false on macOS:
return GLX::GetVideoMemInfoMESA(memInfo);
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 18, 2026
float3.h included lib/streflop/streflop_cond.h directly, ahead of FastMath.h. FastMath.h defines MATH_SQRT_OVERRIDE before it includes streflop_cond.h (so streflop does not define its own math::sqrt(float) -- FastMath provides a faster one). The direct include meant streflop_cond.h could be processed before that override was set. Drop the direct include; FastMath.h pulls in streflop_cond.h transitively with the override in place. creg_cond.h keeps its original position -- it does not pull in streflop, so it does not need to move. Surfaced by the macOS port (beyond-all-reason#2991, commit cc3cad9). Co-authored-by: Mark Kropf <markkropf@gmail.com>
sprunk
pushed a commit
that referenced
this pull request
Jun 18, 2026
float3.h included lib/streflop/streflop_cond.h directly, ahead of FastMath.h. FastMath.h defines MATH_SQRT_OVERRIDE before it includes streflop_cond.h (so streflop does not define its own math::sqrt(float) -- FastMath provides a faster one). The direct include meant streflop_cond.h could be processed before that override was set. Drop the direct include; FastMath.h pulls in streflop_cond.h transitively with the override in place. creg_cond.h keeps its original position -- it does not pull in streflop, so it does not need to move. Surfaced by the macOS port (#2991, commit cc3cad9). Co-authored-by: Mark Kropf <markkropf@gmail.com>
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 18, 2026
std::ranges::find_if (C++20) is not available across all standard libraries we target (notably older Apple libc++). Rather than rewrite the call site to the iterator-pair form, add System/Cpp20Compat.hpp exposing Recoil::ranges::find_if -- it forwards to std::ranges::find_if when available (__cpp_lib_ranges) and otherwise provides a minimal range overload. This keeps the readable range-style call and follows the existing Cpp17Compat.hpp pattern. Surfaced by the macOS port (beyond-all-reason#2991).
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 18, 2026
smmalloc.h defines `#define INLINE inline`, a very generic token that collides with unrelated code (e.g. simdjson's layout_mode::INLINE enumerator) whenever both land in the same translation unit. Instead of undef-ing INLINE at each affected site, add System/recoil-smmalloc.h which includes smmalloc and drops the macro, and route the engine's single smmalloc include (MemPoolTypes.h) through it. The macro can no longer escape into engine code, so a future file mixing smmalloc with json/etc. won't reintroduce the collision. Surfaced by the macOS port (beyond-all-reason#2991, commit 16622a9). The upstream smmalloc leak is worth fixing there too. Co-authored-by: Mark Kropf <markkropf@gmail.com>
tomjn
pushed a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 18, 2026
LuaTextures::Create returned an empty string on glTexImage failure with no diagnostics. Log target/size/format/dataFormat/dataType/glError so texture-creation failures can be diagnosed. Extracted from 1e75080 in beyond-all-reason#2991; cross-platform, no behavior change beyond the added log.
sprunk
added a commit
that referenced
this pull request
Jun 18, 2026
`SafeUtil.h` uses `<memory>` for `std::addressof`, and `<type_traits>` for `std::is_trivially_copyable` / `std::is_trivially_constructible_v`, but pulled them in only transitively. Include them directly so the header is self-contained and does not rely on include order elsewhere. Also use `std::is_trivially_default_constructible` for the default-construction. Extracted from cc3cad9 in #2991; cross-platform, no behavior change. Co-authored-by: Mark Kropf <markkropf@gmail.com> Co-authored-by: sprunk <spr.ng@o2.pl>
sprunk
reviewed
Jun 19, 2026
| HANDLE threadHandle; | ||
| #elif defined(__APPLE__) | ||
| // macOS has no portable CPU affinity API; stub out | ||
| bool dummy; |
Collaborator
There was a problem hiding this comment.
why not just keep the section empty?
| } break; | ||
| case SDL_MOUSEBUTTONDOWN: { | ||
| #ifdef __APPLE__ | ||
| mousepos = ScaleMouseCoords(event.button.x, event.button.y); |
Collaborator
There was a problem hiding this comment.
would be nice to make everything just use ScaleMouseCoords here, and put the #ifdef APPLE within ScaleMouseCoords with the else clause just being return int2(x, y).
| mousepos = ScaleMouseCoords(event.motion.x, event.motion.y); | ||
|
|
||
| if (mouse != nullptr) { | ||
| int sdlW = 1, sdlH = 1; |
Collaborator
There was a problem hiding this comment.
why the 1, 1? do passed values mean anything?
| @@ -1,3 +1,4 @@ | |||
| #include <cmath> | |||
| bool IsBlack() const { | ||
| static const ai_real epsilon = ai_real(10e-3); | ||
| return math::fabs( r ) < epsilon && math::fabs( g ) < epsilon && math::fabs( b ) < epsilon; | ||
| return math::fabs( static_cast<float>(r) ) < epsilon && math::fabs( static_cast<float>(g) ) < epsilon && math::fabs( static_cast<float>(b) ) < epsilon; |
Collaborator
There was a problem hiding this comment.
i'm not sure if we want to touch vendored libs?
| static const int s_dsFactor = []() -> int { | ||
| if (const char* s = getenv("SPRING_DOWNSAMPLE_READBACK")) { | ||
| const int v = atoi(s); | ||
| if (v >= 2 && v <= 8) return v; |
Comment on lines
+1002
to
+1003
| #ifndef __APPLE__ | ||
| #ifndef __APPLE__ |
Collaborator
There was a problem hiding this comment.
better be safe
Suggested change
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ | |
| #ifndef __APPLE__ |
| // SDL_WINDOW_FULLSCREEN_DESKTOP for "fake" fullscreen that takes the size of the desktop; | ||
| // and 0 for windowed mode. | ||
|
|
||
| #if defined(__APPLE__) && !defined(HEADLESS) |
Collaborator
There was a problem hiding this comment.
why the headless check? headless probably shouldnt mind ogl vs non-ogl either way
| return false; | ||
|
|
||
| // Probe GL via eglGetProcAddress before the engine's glad pass runs. | ||
| // Using raw types because GL headers aren't included this high in the file. |
Collaborator
There was a problem hiding this comment.
why can't we #include the relevant headers?
| const double bsf = noRetina ? 1.0 : bsfTrue; | ||
| int pxW = (int)(winW * bsf + 0.5); | ||
| int pxH = (int)(winH * bsf + 0.5); | ||
| if (pxW <= 0 || pxH <= 0) { pxW = 1280; pxH = 720; } |
Collaborator
There was a problem hiding this comment.
why these values? actually if something goes wrong why not just raise an error rather than defaulting to potato resolution
Contributor
|
@iamaperson000 are you able to resolve the conflicts and rebase on to |
|
Is using GCC for the MacOS build a option? It wouldn't just reduce differences in C++ standard conformance but also reduce the likelihood of sync related issues in multiplayer. Because the same compiler has the same bugs, implementation defined behavior and so on... |
Contributor
No, Metal/Cocoa APIs require clang, however clang can be told to use a copy of libstdc++. A GCC build for just the sim with a clang renderer on top might be possible but it's a lot of work and restructuring that impacts the entire engine, not worth it IMO. |
This was referenced Jun 19, 2026
|
Is that true for the C++ headers Apple provides as well? Just throwing ideas out there but wouldn't another way be to put the small amount of code that currently actually needs Apple clang into a static library? That library would be compiled using the Apple specific compiler and the rest could use another. |
Contributor
|
Having a static library compiled in clang that we link into the main build should be pretty straight forward though, I don't think we'd need a separate dylib like unitsync, main issue is not sure about is how to structure it so that the renderer etc are isolated and compiled via clang. At least for the creation of the window or message box dialogs it's easier Something to note is that gcc on Mac is an alias of clang, but homebrew can install native gcc and most vendor dependencies |
Contributor
|
As for the headers, we could use that to remove Objective C but we'd still need clang for it to match apples ABI. A separate statically linked lib is still the better option even with pure C++,though I haven't discounted clang with libstdc++ as the lowest hanging fruit |
|
If using GCC isn't practically possible forget my comments. Having another option is nice to have but hopefully not necessary. I was just thinking about how compiler related differences could be reduced across platforms. Compilers have bugs like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125153 if you are unlucky something like that could mess with sync compatibility even if the same compiler but a slightly different version is used. Or it could only affect x86 but not the arm codegen so maybe using clang works out just fine. |
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 25, 2026
smmalloc.h defines `#define INLINE inline`, a very generic token that collides with unrelated code (e.g. simdjson's layout_mode::INLINE enumerator) whenever both land in the same translation unit. Instead of undef-ing INLINE at each affected site, add System/recoil-smmalloc.h which includes smmalloc and drops the macro, and route the engine's single smmalloc include (MemPoolTypes.h) through it. The macro can no longer escape into engine code, so a future file mixing smmalloc with json/etc. won't reintroduce the collision. Surfaced by the macOS port (beyond-all-reason#2991, commit 16622a9). The upstream smmalloc leak is worth fixing there too. Co-authored-by: Mark Kropf <markkropf@gmail.com> (cherry picked from commit 47e8470)
This was referenced Jun 29, 2026
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jun 29, 2026
Casting a floating-point heading/pitch expression directly to `short` is undefined behaviour when the truncated value does not fit in `short`, and this is genuinely reached: `heading * RAD2TAANG` equals +-32768 at heading = +-pi, one past short's 32767 maximum. Because the result is UB, arm64 and x86 produced different values at the angle extremes, which was observed as an arm64/x86 multiplayer desync. x86 already lowers `short(float)` as float->int->short (cvttss2si into a 32-bit register, then narrow), so making the `int()` step explicit leaves the already-correct x86 result unchanged while pinning arm64 (whose fcvtzs saturated differently) to the same value. The int->short narrowing is the intended 16-bit TA-angle wraparound. Sites fixed in CobInstance.cpp: WindChanged, StartBuilding, AimWeapon. Sibling casts were checked and left untouched: UnitScript.cpp:1058 casts asin(...)*RAD2TAANG (range +-16384, always fits short, no UB) and lines 1095/1099 already route through int; CobThread.cpp has no such casts. Pure correctness fix to synced simulation code; it does not alter results on the platform that was already correct. Cross-arch sync validation is recommended. Origin: discussed in PR beyond-all-reason#2991's review (credit BambaDamba). AI assistance: implemented with Claude Code (Anthropic) from a written plan; reasoning and build verification reviewed by a human.
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jul 9, 2026
smmalloc.h defines `#define INLINE inline`, a very generic token that collides with unrelated code (e.g. simdjson's layout_mode::INLINE enumerator) whenever both land in the same translation unit. Instead of undef-ing INLINE at each affected site, add System/recoil-smmalloc.h which includes smmalloc and drops the macro, and route the engine's single smmalloc include (MemPoolTypes.h) through it. The macro can no longer escape into engine code, so a future file mixing smmalloc with json/etc. won't reintroduce the collision. Surfaced by the macOS port (beyond-all-reason#2991, commit 16622a9). The upstream smmalloc leak is worth fixing there too. Co-authored-by: Mark Kropf <markkropf@gmail.com> (cherry picked from commit 47e8470)
tomjn
added a commit
to tomjn/RecoilEngine
that referenced
this pull request
Jul 9, 2026
Casting a floating-point heading/pitch expression directly to `short` is undefined behaviour when the truncated value does not fit in `short`, and this is genuinely reached: `heading * RAD2TAANG` equals +-32768 at heading = +-pi, one past short's 32767 maximum. Because the result is UB, arm64 and x86 produced different values at the angle extremes, which was observed as an arm64/x86 multiplayer desync. x86 already lowers `short(float)` as float->int->short (cvttss2si into a 32-bit register, then narrow), so making the `int()` step explicit leaves the already-correct x86 result unchanged while pinning arm64 (whose fcvtzs saturated differently) to the same value. The int->short narrowing is the intended 16-bit TA-angle wraparound. Sites fixed in CobInstance.cpp: WindChanged, StartBuilding, AimWeapon. Sibling casts were checked and left untouched: UnitScript.cpp:1058 casts asin(...)*RAD2TAANG (range +-16384, always fits short, no UB) and lines 1095/1099 already route through int; CobThread.cpp has no such casts. Pure correctness fix to synced simulation code; it does not alter results on the platform that was already correct. Cross-arch sync validation is recommended. Origin: discussed in PR beyond-all-reason#2991's review (credit BambaDamba). AI assistance: implemented with Claude Code (Anthropic) from a written plan; reasoning and build verification reviewed by a human.
Open
lostsquirrel1
pushed a commit
that referenced
this pull request
Jul 28, 2026
…nc) (#3075) * Sim/COB: avoid UB in float-to-short angle casts (fixes arm64/x86 desync) Casting a floating-point heading/pitch expression directly to `short` is undefined behaviour when the truncated value does not fit in `short`, and this is genuinely reached: `heading * RAD2TAANG` equals +-32768 at heading = +-pi, one past short's 32767 maximum. Because the result is UB, arm64 and x86 produced different values at the angle extremes, which was observed as an arm64/x86 multiplayer desync. x86 already lowers `short(float)` as float->int->short (cvttss2si into a 32-bit register, then narrow), so making the `int()` step explicit leaves the already-correct x86 result unchanged while pinning arm64 (whose fcvtzs saturated differently) to the same value. The int->short narrowing is the intended 16-bit TA-angle wraparound. Sites fixed in CobInstance.cpp: WindChanged, StartBuilding, AimWeapon. Sibling casts were checked and left untouched: UnitScript.cpp:1058 casts asin(...)*RAD2TAANG (range +-16384, always fits short, no UB) and lines 1095/1099 already route through int; CobThread.cpp has no such casts. Pure correctness fix to synced simulation code; it does not alter results on the platform that was already correct. Cross-arch sync validation is recommended. Origin: discussed in PR #2991's review (credit BambaDamba). AI assistance: implemented with Claude Code (Anthropic) from a written plan; reasoning and build verification reviewed by a human. * address review: extract RadAngleToCobShort helper with explanatory comment sprunk asked for a comment and a named helper around the float->short angle cast. Wrap the conversion in RadAngleToCobShort() and document why it exists: COB angles are circular 16-bit TA units (full turn == COBSCALE), so values past a half turn intentionally wrap modulo 2^16 via the int->short narrowing. The float->int step stays because the wrap is the desired behaviour and is deterministic across arm64/x86; clamping the range (as suggested) would break angles past a half turn rather than wrapping them. No behavioural change vs the previous short(int(...)) form.
lostsquirrel1
pushed a commit
that referenced
this pull request
Jul 29, 2026
) smmalloc.h defines `#define INLINE inline`, a very generic token that collides with unrelated code (e.g. simdjson's layout_mode::INLINE enumerator) whenever both land in the same translation unit. Instead of undef-ing INLINE at each affected site, add System/recoil-smmalloc.h which includes smmalloc and drops the macro, and route the engine's single smmalloc include (MemPoolTypes.h) through it. The macro can no longer escape into engine code, so a future file mixing smmalloc with json/etc. won't reintroduce the collision. Surfaced by the macOS port (#2991, commit 16622a9). The upstream smmalloc leak is worth fixing there too. Co-authored-by: Mark Kropf <markkropf@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an Apple Silicon macOS build path via Mesa 26.2 (Zink driver) → KosmicKrisp (Vulkan-on-Metal) → Metal. Renders BAR end-to-end on macOS 26 / M-series hardware.
Companion PRs
Cross-platform safety
All behavior changes are gated by one of:
Universal bug fixes (NOT macOS-specific) are called out per commit:
Env vars introduced
Tested
Known limitations
Notes for reviewers