Use metal-cpp for public facing contract by bghgary · Pull Request #1603 · BabylonJS/BabylonNative

bghgary · 2026-02-13T22:25:49Z

No description provided.

Copilot

Pull request overview

This PR migrates the Apple Metal “public-facing contract” from Objective‑C Metal types (id<MTL*>, MTKView*, CAMetalLayer*) to metal-cpp types (MTL::*, CA::MetalLayer*) and adjusts build/install wiring and platform glue accordingly.

Changes:

Introduces a generated metal-cpp dependency (single-header + implementation TU) and links it into Graphics/device-facing targets on Apple.
Updates Metal-facing typedefs (RendererType.h, Platform.h) and several Apple call sites to use MTL::* / CA::* types.
Refactors TestUtils platform implementations to store WindowT directly (removing the prior ImplData indirection) and updates Playground/CI plumbing.

Reviewed changes

Copilot reviewed 42 out of 44 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
Plugins/TestUtils/Source/TestUtils_macOS.mm	Adds macOS-specific TestUtils implementation (window ops, output dir, framebuffer postprocess).
Plugins/TestUtils/Source/TestUtils_iOS.mm	Adds iOS-specific TestUtils stubs for unimplemented behaviors.
Plugins/TestUtils/Source/TestUtils_WinRT.cpp	Updates WinRT TestUtils initialization and exit behavior.
Plugins/TestUtils/Source/TestUtils_Win32.cpp	Updates Win32 TestUtils to use `m_window` directly and quick-exit on failure.
Plugins/TestUtils/Source/TestUtils_Unix.cpp	Updates X11 TestUtils to use `m_window` and quick-exit on failure.
Plugins/TestUtils/Source/TestUtils_Android.cpp	Aligns Android TestUtils with new internal API shape.
Plugins/TestUtils/Source/TestUtilsImplData.h	Removes the ImplData wrapper used to store `WindowT`.
Plugins/TestUtils/Source/TestUtils.h	Changes TestUtils instance creation to pass `WindowT` directly into the JS wrapper.
Plugins/TestUtils/Source/TestUtils.cpp	Wires framebuffer post-processing and re-homes plugin `Initialize`.
Plugins/TestUtils/Source/Apple/TestUtilsImpl.mm	Removes previous Apple combined implementation.
Plugins/TestUtils/Include/Babylon/Plugins/TestUtils.h	Removes exported `errorCode` and keeps `Initialize` API.
Plugins/TestUtils/CMakeLists.txt	Switches to per-platform TestUtils source file selection and enables ObjC ARC on Apple.
Plugins/ShaderCache/CMakeLists.txt	Adjusts `ShaderCacheInternal` interface linkage behavior.
Plugins/NativeTracing/CMakeLists.txt	Replaces manual Xcode ARC property with `enable_objc_arc(...)`.
Plugins/NativeOptimizations/CMakeLists.txt	Replaces manual Xcode ARC property with `enable_objc_arc(...)`.
Plugins/NativeEngine/CMakeLists.txt	Ensures `ShaderCache` is explicitly linked when enabled.
Plugins/NativeCamera/CMakeLists.txt	Enables ObjC ARC and adjusts Apple build flags/unity build behavior.
Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp	Migrates Metal texture handling to metal-cpp (`MTL::Texture*`, `NS::SharedPtr`).
Plugins/ExternalTexture/CMakeLists.txt	Uses `.cpp` implementation file naming for ExternalTexture sources.
Install/Install.cmake	Adds install rules for metal-cpp and reorders/expands install targets.
Dependencies/xr/CMakeLists.txt	Replaces manual Xcode ARC property with `enable_objc_arc(...)`.
Dependencies/CMakeLists.txt	Adds FetchContent for metal-cpp and generates single-header + implementation source.
Core/Graphics/Source/DeviceImpl_visionOS.mm	Simplifies device pixel ratio function signature/body.
Core/Graphics/Source/DeviceImpl_macOS.mm	Updates macOS pixel ratio logic and uses Metal layer scale.
Core/Graphics/Source/DeviceImpl_iOS.mm	Updates iOS pixel ratio logic and uses Metal layer scale.
Core/Graphics/Source/DeviceImpl_Metal.mm	Returns platform info as metal-cpp `MTL::*` pointers.
Core/Graphics/Include/RendererType/Metal/Babylon/Graphics/RendererType.h	Changes Metal renderer typedefs to metal-cpp pointers and includes Metal.hpp.
Core/Graphics/Include/Platform/visionOS/Babylon/Graphics/Platform.h	Switches `WindowT` to `CA::MetalLayer*` and includes Metal.hpp.
Core/Graphics/Include/Platform/macOS/Babylon/Graphics/Platform.h	Switches `WindowT` to `CA::MetalLayer*` and includes Metal.hpp.
Core/Graphics/Include/Platform/iOS/Babylon/Graphics/Platform.h	Switches `WindowT` to `CA::MetalLayer*` and includes Metal.hpp.
Core/Graphics/CMakeLists.txt	Links metal-cpp on Apple and refactors Graphics/GraphicsDevice* wiring.
CMakeLists.txt	Updates CMakeExtensions revision and adds FetchContent declaration for metal-cpp zip.
Apps/UnitTests/Source/Utils.Metal.mm	Migrates test texture creation/destruction to metal-cpp API.
Apps/UnitTests/Source/App.Apple.mm	Creates default Metal device via metal-cpp.
Apps/Playground/visionOS/LibNativeBridge.mm	Updates AppContext construction to pass metal-cpp window type.
Apps/Playground/macOS/main.mm	Adds a macOS Objective-C++ entrypoint enabling debug trace.
Apps/Playground/macOS/ViewController.mm	Passes Metal layer into AppContext using new window type.
Apps/Playground/iOS/LibNativeBridge.mm	Passes Metal layer into AppContext using new window type.
Apps/Playground/X11/App.cpp	Removes dependency on `TestUtils::errorCode` and always returns 0 on normal exit.
Apps/Playground/Win32/App.cpp	Removes `PostQuitMessage(errorCode)` dependency on removed `errorCode`.
Apps/Playground/Scripts/validation_native.js	No functional change (formatting-only newline change).
Apps/Playground/Scripts/config.json	Replaces single `comment` with per-API `comments` and extends exclusions for Metal.
Apps/Playground/CMakeLists.txt	Adjusts Apple entrypoint extension, ARC enabling, and link ordering.
.github/jobs/win32.yml	Simplifies path changes for validation test invocation.

Comments suppressed due to low confidence (2)

Core/Graphics/Source/DeviceImpl_iOS.mm:27

Same issue as macOS: WindowT is CA::MetalLayer* but this casts to CAMetalLayer* and reads an ObjC property. Use the metal-cpp API (window->contentsScale()) or an explicit reinterpret_cast if bridging is required.

        float scale = static_cast<float>(((CAMetalLayer*)window).contentsScale);
        if (std::isinf(scale) || scale <= 0)
        {
            scale = UIScreen.mainScreen.scale;
        }

Plugins/ExternalTexture/Source/ExternalTexture_Metal.cpp:295

RetainPtr is referenced but not defined in this translation unit (and there is no using bringing it into scope). With metal-cpp, the helper is typically NS::RetainPtr(...). As written this looks like a compile error; qualify the call (or replace with an explicit NS::SharedPtr construction) so ownership is clear.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-14T00:56:46Z

Apps/Playground/iOS/LibNativeBridge.mm

    appContext.emplace(
-        view,
+        (__bridge CA::MetalLayer*)view.layer,
        static_cast<size_t>(inWidth),
        static_cast<size_t>(inHeight),


Same __bridge issue as macOS: view.layer is an Objective-C object pointer, and CA::MetalLayer* is a metal-cpp wrapper type. __bridge is not the right cast here; use an explicit cast compatible with metal-cpp (typically reinterpret_cast<CA::MetalLayer*>(view.layer) after ensuring it is a CAMetalLayer).

Copilot · 2026-02-14T00:56:47Z

Dependencies/CMakeLists.txt

+    find_package(Python3 REQUIRED COMPONENTS Interpreter)
+    set(METAL_CPP_GENERATED_DIR "${CMAKE_BINARY_DIR}/metal-cpp")
+    execute_process(
+        COMMAND ${Python3_EXECUTABLE}
+            "${metal-cpp_SOURCE_DIR}/SingleHeader/MakeSingleHeader.py"
+            "${metal-cpp_SOURCE_DIR}/Foundation/Foundation.hpp"
+            "${metal-cpp_SOURCE_DIR}/Metal/Metal.hpp"
+            "${metal-cpp_SOURCE_DIR}/QuartzCore/QuartzCore.hpp"
+            -o "${METAL_CPP_GENERATED_DIR}/Include/Metal/Metal.hpp"
+        RESULT_VARIABLE METAL_CPP_RESULT)
+    if(NOT METAL_CPP_RESULT EQUAL 0)
+        message(FATAL_ERROR "Failed to generate metal-cpp single header")
+    endif()
+
+    # Generate source that includes the single header with the necessary implementation defines.
+    file(WRITE "${METAL_CPP_GENERATED_DIR}/Source/Metal.cpp"
+        "#define NS_PRIVATE_IMPLEMENTATION\n"
+        "#define MTL_PRIVATE_IMPLEMENTATION\n"
+        "#define CA_PRIVATE_IMPLEMENTATION\n"
+        "#include <Metal/Metal.hpp>\n")


The generated output paths under ${CMAKE_BINARY_DIR}/metal-cpp are written to (.../Include/Metal/Metal.hpp and .../Source/Metal.cpp) without first ensuring the parent directories exist. execute_process/file(WRITE ...) will fail if those directories haven't been created yet. Add file(MAKE_DIRECTORY ...) for the required Include/Metal and Source directories before generating these files.

Copilot · 2026-02-14T00:56:47Z

Plugins/TestUtils/Source/TestUtils.h

                    ParentT::InstanceMethod("getImageData", &TestUtils::GetImageData),
                    ParentT::InstanceMethod("getOutputDirectory", &TestUtils::GetOutputDirectory),
                    ParentT::InstanceMethod("getFrameBufferData", &TestUtils::GetFrameBufferData),
-                });
+                },
+                &window);


CreateInstance passes &window as the data pointer to DefineClass. Since window is a by-value parameter, this stores a pointer to a stack variable that becomes dangling as soon as CreateInstance returns; the constructor then reads it via info.Data(). Pass a heap-allocated/owned value (e.g., Napi::External with a finalizer, or store window somewhere with lifetime >= JS class) instead of the address of this local.

Copilot · 2026-02-14T00:56:47Z

Plugins/TestUtils/Source/TestUtils.h

        TestUtils(const Napi::CallbackInfo& info)
-            : TestUtils(info, JsRuntime::GetFromJavaScript(info.Env()))
+            : TestUtils(info, JsRuntime::GetFromJavaScript(info.Env()), *static_cast<Graphics::WindowT*>(info.Data()))
        {


This constructor unconditionally dereferences info.Data() as a Graphics::WindowT*. With the current CreateInstance implementation, that pointer is dangling; even after fixing that, this should only dereference a pointer whose lifetime/ownership is clearly managed (typically via Napi::External with a finalizer).

Copilot · 2026-02-14T00:56:47Z

Plugins/TestUtils/Source/TestUtils_iOS.mm

+    Napi::Value TestUtils::GetOutputDirectory(const Napi::CallbackInfo& /*info*/)
+    {
+        // Not implemented for iOS
+        return {};
+    }


Returning {} here produces an empty Napi::Value handle, which is not a valid JS value and can lead to crashes/asserts when used. Return info.Env().Undefined() or a string (e.g., an empty string or a platform-appropriate directory) to keep the API consistent with other platforms.

Copilot · 2026-02-14T00:56:48Z

Apps/Playground/macOS/ViewController.mm

    appContext.emplace(
-        engineView,
+        (__bridge CA::MetalLayer*)engineView.layer,
        width,


__bridge casts are for Objective-C/CF pointers; CA::MetalLayer* is a metal-cpp wrapper type. This cast is likely invalid and can fail to compile or produce UB. Prefer an explicit reinterpret_cast<CA::MetalLayer*>(engineView.layer) (after ensuring the layer really is a CAMetalLayer) or obtain the metal layer via metal-cpp APIs.

Copilot · 2026-02-14T00:56:48Z

Apps/Playground/visionOS/LibNativeBridge.mm


    _appContext.emplace(
-        self.metalLayer,
+        (__bridge CA::MetalLayer*)self.metalLayer,


Same __bridge issue as other Apple platforms: self.metalLayer is an ObjC object pointer, while CA::MetalLayer* is a metal-cpp wrapper type. __bridge is not applicable here; use a cast that matches metal-cpp's expected bridging (typically reinterpret_cast<CA::MetalLayer*>(self.metalLayer)).

Suggested change

(__bridge CA::MetalLayer*)self.metalLayer,

reinterpret_cast<CA::MetalLayer*>(self.metalLayer),

Copilot · 2026-02-14T00:56:49Z

Plugins/TestUtils/Source/TestUtils.cpp

+            m_runtime.Dispatch([callbackPtr{ std::move(callbackPtr) }, array{ std::move(array) }](Napi::Env env) mutable {
+                PostProcessFrameBufferData(array);
                auto arrayBuffer{ Napi::ArrayBuffer::New(env, const_cast<uint8_t*>(array.data()), array.size()) };
                auto typedArray{ Napi::Uint8Array::New(env, array.size(), arrayBuffer, 0) };
                callbackPtr->Value().Call({ typedArray });


Napi::ArrayBuffer::New(env, array.data(), array.size()) creates an external ArrayBuffer that points at array's storage. Since array is a local that is destroyed when this dispatch lambda returns, JavaScript can observe/use freed memory. Allocate/copy into a JS-owned buffer (or provide a finalizer that owns the backing storage) before calling the JS callback.

Copilot · 2026-02-14T00:56:49Z

Plugins/TestUtils/Source/TestUtils_macOS.mm

+        dispatch_async(dispatch_get_main_queue(), ^{
+            auto* window = [(NSView*)((__bridge CAMetalLayer*)m_window).delegate window];
+            CGFloat scale = window.backingScaleFactor;
+            [window setContentSize:NSMakeSize(width / scale, height / scale)];
+        });


m_window is a Graphics::WindowT which is now CA::MetalLayer* (metal-cpp), but this code uses an Objective-C __bridge cast to CAMetalLayer*. __bridge only applies to ObjC/CF pointers and is not valid for metal-cpp wrapper types. Prefer using the metal-cpp API directly or reinterpret_cast<CAMetalLayer*>(m_window) (if you truly need an ObjC view/layer here) and keep the conversion consistent across the codebase.

Copilot · 2026-02-14T00:56:49Z

Plugins/TestUtils/Source/TestUtils_macOS.mm

+        // swizzle the data from BGRA to RGBA on Apple platforms
+        for (size_t i = 0; i < data.size(); i += 4)
+        {
+            std::swap(data[i], data[i + 2]);
+        }


RequestScreenShot already converts bgfx's BGRA screenshot data into RGBA in Core/Graphics/Source/BgfxCallback.cpp (it writes bytes in RGBA order). Swapping R/B again here will flip the data back to BGRA and break consumers expecting RGBA. This post-processing should be removed or gated so it doesn't double-swap on platforms where the screenshot callback already outputs RGBA.

bghgary added 4 commits February 13, 2026 14:25

Use metal-cpp for public facing contract

74378ef

Fixes

e0284bb

Fixes

35d08f3

More fixes

4be05fd

bghgary requested a review from Copilot February 14, 2026 00:49

Copilot started reviewing on behalf of bghgary February 14, 2026 00:50 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

bghgary added 2 commits February 13, 2026 17:16

Fix android build

b0297d8

Fix

d8cb3ed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use metal-cpp for public facing contract#1603

Use metal-cpp for public facing contract#1603
bghgary wants to merge 6 commits intoBabylonJS:masterfrom
bghgary:metal-cpp

bghgary commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Copilot AI Feb 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	(__bridge CA::MetalLayer*)self.metalLayer,
	reinterpret_cast<CA::MetalLayer*>(self.metalLayer),

Conversation

bghgary commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant