Metal graphics backend - #6
Open
LLThreasher wants to merge 38 commits into
Open
Conversation
Finish the Metal graphics backend (engine/modules/graphics_metal): - Fix create_backend.hpp namespace (oge::graphics::vulkan → metal) - Remove duplicate CreateSwapchain/DestroySwapchain/RecreateSurface - Fix all enum mismatches with current oge::graphics API (TextureFormat, PrimitiveTopology, IndexFormat, DepthCompareOp, FrontFace, MemoryUsage, TextureUsage) - Add NS::SharedPtr null-comparison fixes (.get() == nullptr) - Implement BeginFrame/EndFrame with semaphore frame pacing - Implement CreateCommandList (MTLCommandBuffer wrapper) - Implement CreateBuffer/DestroyBuffer/FlushStagingBufferRanges - Implement CreateTexture/DestroyTexture with Metal descriptor - Implement CreateGraphicsPipeline/CreateComputePipeline/DestroyPipeline with MSL shader library loading - Implement CreateBindingGroupLayout/CreateBindingGroup + destroy - Implement CreateFence/WaitForFence/IsFenceSignaled/ResetFence - Add internal helpers: AcquireNextDrawable, RecreateSwapchain, CreateShaderFunction, CreateDepthTexture, DestroyTextureInternal - Wire BUILD_METAL option: conditional subdirectory in engine/CMakeLists, conditional linking in game_ctrl_ext and game_desktop - Conditional backend creation in client.cpp (BUILD_VULKAN/BUILD_METAL) Total: ~1473 lines (metal.cpp 1220, metal.hpp 242, create_backend.cpp 11). Builds and links on macOS with BUILD_METAL=ON BUILD_VULKAN=OFF. Co-Authored-By: Claude <noreply@anthropic.com>
- Guard oge::graphics::vulkan includes and calls with #ifdef OGE_USE_VULKAN
in engine/modules/api (was unconditional, broke Metal-only builds).
- Client constructor now takes const char* backendName (defaults to Vulkan),
stored as m_backendName and passed to OGE_Backend_Create at init time.
- main.cpp accepts optional backend arg before scene arg:
./Arterium [Metal] [SceneName]
Both args are optional and order-independent (Metal/Vulkan is detected).
Co-Authored-By: Claude <noreply@anthropic.com>
- Add metal_impl.cpp: single translation unit defining NS_PRIVATE_IMPLEMENTATION / CA_PRIVATE_IMPLEMENTATION / MTL_PRIVATE_IMPLEMENTATION to instantiate metal-cpp's static selector symbols (bkaradzic/metal-cpp fork uses static selectors instead of dynamic objc_msgSend). - Change metal-cpp from PRIVATE to PUBLIC in oge_graphics_metal so its -framework flags (Metal, QuartzCore, Foundation) propagate to consuming executables. Co-Authored-By: Claude <noreply@anthropic.com>
…ture alloc()->init() can leave fields at unexpected defaults in metal-cpp. Use texture2DDescriptor() which properly initializes as MTLTextureType2D and guard mipLevels/depth/layers with std::max(1u, ...) to prevent zero or garbage values reaching Metal validation. Fixes crash: 'MTLTextureDescriptor has arrayLength (256) greater than maximum allowed size of 1.' Co-Authored-By: Claude <noreply@anthropic.com>
texture2DDescriptor already creates a valid 2D descriptor with arrayLength=1 and depth=1. Calling setArrayLength(1) afterwards may trigger Metal validation weirdness in some cases. Co-Authored-By: Claude <noreply@anthropic.com>
- CreateTexture: use alloc()->init() + setTextureType(2D/2DArray) instead of texture2DDescriptor which rejects arrayLength > 1. The terrain renderer allocates a 256-layer block texture atlas — MTLTextureType2DArray is the correct type when layers > 1. - CreateGraphicsPipeline: check vertex function before passing to newRenderPipelineState (nil vertexFn is a Metal validation abort). Report clear error that Metal requires MSL, not SPIR-V. - Remove debug logging from CreateTexture. - App now starts and runs without crashing on Metal backend. (Rendering is blank until shaders are provided as MSL.) Co-Authored-By: Claude <noreply@anthropic.com>
- Add spirv-cross as optional dependency: find_library in engine/modules/graphics_metal/CMakeLists.txt, gated on SPIRV_CROSS_* variables found in /usr/local/lib (Vulkan SDK). - CreateShaderFunction converts SPIR-V bytecode to MSL at runtime using spirv_cross::CompilerMSL, then compiles into a Metal library. - Entry-point discovery: tries reflected SPIR-V name, 'main0' (SPIRV-Cross MSL convention), 'main' (GLSL convention), plus the caller-suggested name. - Shader libraries are compiled per-function (Metal can't extend an existing MTLLibrary). - Also add IGraphicsBackend::GetBackendName() so passes can query "Vulkan" / "Metal" and supply matching shaders at build time. Known gap: vertex descriptors not yet wired up in render pipeline creation — produces 'Vertex function has input attributes but no vertex descriptor was set' (non-fatal). Co-Authored-By: Claude <noreply@anthropic.com>
Map GraphicsPipelineDesc::vertexLayout to MTL::VertexDescriptor during CreateGraphicsPipeline. Each VertexAttributeFormat is converted to the corresponding MTL::VertexFormat with correct byte size, and assembled into a per-vertex buffer layout at binding 0. Adds ToMetalVertexFormat() helper covering Float32/2/3/4, Uint32/2/3/4, Uint16/2, and Uint8. Fixes: 'Vertex function has input attributes but no vertex descriptor was set' validation error. Co-Authored-By: Claude <noreply@anthropic.com>
Apple GPUs use unified memory where shared-mode buffers are automatically CPU/GPU coherent. didModifyRange only applies to managed storage mode which doesn't exist on Apple Silicon. Calling it on a shared buffer triggers a Metal debug-layer assertion. Replace FlushStagingBufferRanges body with a no-op — staging buffers on Apple Silicon need no explicit flush. Also add METAL_DEBUG CMake option and OGE_METAL_DEBUG define for future debug-layer integration. Co-Authored-By: Claude <noreply@anthropic.com>
Replace the dangerous reinterpret_cast<ICommandList&> with a proper MetalCommandBuffer class that wraps MTL::CommandBuffer / MTL::RenderCommandEncoder and implements all 22 ICommandList methods. Key implementations: - BeginRenderPass / EndRenderPass via MTL::RenderCommandEncoder - BindGraphicsPipeline / BindVertexBuffer / BindIndexBuffer - Draw / DrawIndexed with primitive type from pipeline - CopyBufferToTexture via MTL::BlitCommandEncoder - Dispatch (compute) - Nil-PSO guards prevent Metal debug-layer aborts Fixes SIGSEGV in TerrainPass2::onUpdate → BindGraphicsPipeline. Co-Authored-By: Claude <noreply@anthropic.com>
Default-constructed (invalid) pipeline handles map to uninitialized pool entries. Pool::Get() returns a non-null pointer even for invalid handles, giving garbage MTL::RenderPipelineState pointers that abort when passed to setRenderPipelineState:. Add IsValid() check before pool access to skip failed pipeline creations. Also remove verbose error logging — quiet skip is sufficient. Fixes MTLDebugRenderCommandEncoder setRenderPipelineState: assertion in TerrainPass2::onUpdate pipeline binding. Co-Authored-By: Claude <noreply@anthropic.com>
beginEncoder now creates a depth texture on first use and attaches it to the render pass descriptor. This matches the pipeline's depth attachment pixel format, fixing: 'For depth attachment, the renderPipelineState pixelFormat must be MTLPixelFormatInvalid, as no texture is set.' Also guard with IsValid() for default-constructed pipeline handles. Co-Authored-By: Claude <noreply@anthropic.com>
…Indexed - Always set depth attachment pixel format to match render pass (Depth32Float) - Store index buffer/offset/type in BindIndexBuffer for use in drawIndexedPrimitives - Create depth texture directly with MTL::TextureUsageRenderTarget - Add m_indexBuffer/m_indexBufferOffset/m_indexType members to MetalCommandBuffer Metal debug validation now passes for: - Render pass descriptor (depth texture has RenderTarget usage) - Pipeline depth format matches render pass - Index buffer non-nil in drawIndexedPrimitives Next: shader resource binding (samplers/textures) via BindBindingGroup. Co-Authored-By: Claude <noreply@anthropic.com>
Bind textures and samplers from binding groups to fragment shader slots (setFragmentTexture + setFragmentSamplerState). Bind uniform/storage buffers to vertex/fragment shader slots with dynamic offset support. Creates default linear-filtering samplers per texture slot. Fixes: 'missing Sampler binding at index 0' Metal debug validation. Co-Authored-By: Claude <noreply@anthropic.com>
SPIRV-Cross places uniforms at [[buffer(0)]] and vertex attributes at [[attribute(N)]]. These map to the same setVertexBuffer namespace in Metal — uniforms at slot 0 and vertex data must use a different slot. - BindVertexBuffer now uses slot 30 (kVertexBufferSlot) - Vertex descriptor layout stride set on buffer 30 - PushConstants uses slot 0 (setVertexBytes at [[buffer(0)]]) - BindBindingGroup uses slots 0+ for uniform/storage buffers Fixes: 'missing Buffer binding at index 0 for _24[0]' Co-Authored-By: Claude <noreply@anthropic.com>
Metal's viewport uses bottom-left origin (OpenGL convention) while Vulkan uses top-left. Set negative height and origin at y+h to flip the Y axis, so shaders, projection matrices, and textures render correctly without requiring shader-side coordinate adjustments. Co-Authored-By: Claude <noreply@anthropic.com>
UI pass uses UniformUint16x2 (UV coords) and UniformUint8x4 (color). These normalized integer formats were missing from ToMetalVertexFormat, causing fallthrough to float4 and garbled vertex data — artifacts connecting to top-left of screen. Add: UniformUint16 -> UShortNormalized UniformUint16x2 -> UShort2Normalized UniformUint8x4 -> UChar4Normalized Co-Authored-By: Claude <noreply@anthropic.com>
ToMetalVertexFormat overwrites outSize with format size, but it was called with &offset which should accumulate byte offsets. This meant every attribute after the first was placed at the wrong offset, and the stride was always the size of the last attribute instead of the sum of all attributes. Use separate byteOffset accumulator and size variable. Fixes triangle artifacts connecting vertices to top-left corner. Co-Authored-By: Claude <noreply@anthropic.com>
Helps diagnose which passes have working pipelines vs empty handles. Terrain pass uses storage-buffer-based rendering (no vertex layout) and may have SPIR-V->MSL issues specific to its complex shaders. Co-Authored-By: Claude <noreply@anthropic.com>
Previously depth testing was only declared in the pipeline descriptor's pixel format, but Metal requires a separate MTL::DepthStencilState object to be created (with compare function and write mask) and set on the render command encoder via setDepthStencilState. Without this, terrain passes with depthTest=true would silently fail the depth test using Metal's default (likely compare-never), producing no visible output. Co-Authored-By: Claude <noreply@anthropic.com>
Default depthCompareOp is Never, which rejects all fragments. Creating a DSS for every pipeline (even non-depth passes) caused transparent UI and debug overlays to be silently discarded by the depth test. Now DSS is only created when desc.depthTest is true (terrain pass). Co-Authored-By: Claude <noreply@anthropic.com>
For small textures (16x16 block atlas tiles), 256-byte-aligned BPR meant bytesPerImage (256*16=4096) far exceeded the actual source buffer size (64*16=1024), causing Metal to read out-of-bounds from the upload staging buffer. This silently produced black textures for terrain. Use exact BPR (width*4 for RGBA8) — Metal minimum alignment is 4 bytes for this format. Co-Authored-By: Claude <noreply@anthropic.com>
CopyBuffer was a no-op. If the terrain system stages face data to a staging buffer and then copies it to the GPU storage buffer, the data never reached the GPU — explaining zero positions and black quads. Also implement using blit command encoder with src/dst offsets. Co-Authored-By: Claude <noreply@anthropic.com>
CreateCommandList allocates a new MetalCommandBuffer via 'new' every call but never deleted. Added a commandList pointer to MetalFrameData; old command lists are deleted before creating a new one, and all are cleaned up in Shutdown. With 2 CreateCommandList calls per frame (transfer + graphics), this leaked ~200 bytes per frame initially and ~400 bytes every subsequent frame. Co-Authored-By: Claude <noreply@anthropic.com>
BindBindingGroup was creating a new MTL::SamplerState for every texture binding every frame (alloc()->init() + newSamplerState + release). With many draws per frame this saturated the Metal command pool and leaked GPU memory. Now a single default nearest-neighbor sampler is created lazily and reused for all texture bindings. Co-Authored-By: Claude <noreply@anthropic.com>
- setDepthStencilState(null) disables depth for non-terrain passes, preventing terrain depth values from occluding UI/debug/gizmo. - Wire blending enable/factors in pipeline descriptor when the engine requests blending=true (UI pass uses it for transparency). Fixes terrain drawn on top of UI, and transparency not working. Co-Authored-By: Claude <noreply@anthropic.com>
Metal's setDepthStencilState aborts on null. Create a default no-depth DSS (CompareFunction=Always, writeEnabled=false) and use it for passes without depth testing (UI, debug, gizmo). Co-Authored-By: Claude <noreply@anthropic.com>
Previously CreateCommandList created a new MTL::CommandBuffer from each queue (Transfer then Graphics), committing the old one between them. This churned 2 command buffers per frame, growing Metal's internal pool and leaking ~1MB/4sec even on simple scenes. Now one CB is created per frame on the graphics queue and reused for both upload blit encoders and render encoders (Metal supports sequential multi-encoder usage on a single command buffer). Co-Authored-By: Claude <noreply@anthropic.com>
Metal's ObjC runtime autoreleases temporary objects (descriptors, validation messages, internal strings). In a C++/SDL3 app there is no Cocoa run loop, so the autorelease pool is never drained and objects accumulate indefinitely. Add a .mm helper that pushes a new NSAutoreleasePool at BeginFrame and drains it at EndFrame, preventing ~1MB/4sec CPU memory leak. Co-Authored-By: Claude <noreply@anthropic.com>
Move extern declarations to file scope in metal.cpp. Add *.mm to GLOB_RECURSE so metal_autorelease.mm is compiled. Co-Authored-By: Claude <noreply@anthropic.com>
SDL3's Cocoa backend wraps the event loop in @autoreleasepool blocks, so Metal's temporary ObjC objects are already drained each frame. This reverts commits b1dd0c1 and edf6d54. Co-Authored-By: Claude <noreply@anthropic.com>
…wable leaks - Pool MetalCommandBuffer objects per frame-slot (matching Vulkan backend pattern): reuse via Reset() instead of new/delete each CreateCommandList call. Pool is reset in BeginFrame with cmdUsedCount = 0. - Fix MTL::CommandBuffer leak: CreateCommandList was called 3x per frame (Transfer, Present, + separate present CB in EndFrame). Only the last was committed; the first two were overwritten and leaked. Now a single CB is created on first call and reused by all command lists in the frame. - Fix CAMetalDrawable leak: nextDrawable() returns +1 but we never released our reference after presentDrawable. Add explicit release(). - Fix unbounded CB accumulation in Metal driver: dispatch_semaphore_signal was called immediately after commit() (CPU-side), so the CPU raced ahead of the GPU and accumulated unlimited pending command buffers. Move the signal into an addCompletedHandler callback so it fires on GPU completion. - Break circular include: metal_command_buffer.hpp forward-declares MetalBackend instead of including metal.hpp. metal.hpp includes metal_command_buffer.hpp for the full MetalCommandBuffer definition needed by MetalFrameData's std::vector pool.
…rame alloc Eliminates the last per-frame allocation. addCompletedHandler (both the std::function and ObjC block overloads) internally creates a new block copy each call. The std::function overload additionally creates a __block wrapper whose C++ destructor may not run in non-ARC ObjC++. Instead, stash the committed CB in frame.pendingCB so BeginFrame can call waitUntilCompleted on it the next cycle through the same slot. With maxFramesInFlight buffering the CB is guaranteed done by then; waitUntilCompleted returns immediately in the common case. Also caches MTL::RenderPassDescriptor per frame (reused instead of alloc/init each beginEncoder call).
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
2 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
Metal graphics backend implementation for macOS (Apple Silicon / Intel).
What works
Work in progress
Files
🤖 Generated with Claude Code