Address cube and Meshopt review follow-ups

Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp

@@ -290,3 +290,107 @@ TEST(NativeEngineCubeRenderTargets, ClearsEachFaceIndependentlyAndPreserves2DDef
     }
     device.FinishRenderingCurrentFrame();
 }
+
+TEST(NativeEngineCubeRenderTargets, RejectsMultisamplingAndZeroFillsFacesBeforeClear)
+{
+    Babylon::Graphics::Device device{g_deviceConfig};
+#if defined(USE_NOOP_METAL_DEVICE) || defined(SKIP_RENDER_TESTS)
+    GTEST_SKIP() << "GPU rendering/readback is unavailable in this test configuration";
+#endif
+    device.StartRenderingCurrentFrame();
+    Babylon::AppRuntime runtime{};
+    constexpr uint16_t size = 4;
+    constexpr uint32_t faceCount = 6;
+    std::array<uint8_t, size * size * 4 * faceCount> pixels{};
+    std::promise<void> completed;
+    auto future = completed.get_future();
+    runtime.Dispatch([&](Napi::Env env) {
+        try
+        {
+            device.AddToJavaScript(env);
+            Babylon::Plugins::NativeEngine::Initialize(env);
+            auto& context = Babylon::Graphics::DeviceContext::GetFromJavaScript(env);
+            auto frameScope = context.AcquireFrameCompletionScope();
+            auto engine = env.Global().Get("_native").As<Napi::Object>().Get("Engine").As<Napi::Function>().New({});
+            auto createTexture = engine.Get("createTexture").As<Napi::Function>();
+            auto initializeTexture = engine.Get("initializeTexture").As<Napi::Function>();
+            auto createFrameBuffer = engine.Get("createFrameBuffer").As<Napi::Function>();
+            auto value = createTexture.Call(engine, {});
+            env.Global().Set("_testCube", value);
+            EXPECT_THROW(initializeTexture.Call(engine, {
+                value, Napi::Number::New(env, size), Napi::Number::New(env, size),
+                Napi::Boolean::New(env, false), Napi::Number::New(env, bgfx::TextureFormat::RGBA8),
+                Napi::Boolean::New(env, true), Napi::Boolean::New(env, false),
+                Napi::Number::New(env, 4), Napi::Boolean::New(env, true)}), Napi::Error);
+            initializeTexture.Call(engine, {
+                value, Napi::Number::New(env, size), Napi::Number::New(env, size),
+                Napi::Boolean::New(env, false), Napi::Number::New(env, bgfx::TextureFormat::RGBA8),
+                Napi::Boolean::New(env, true), Napi::Boolean::New(env, false),
+                Napi::Number::New(env, 1), Napi::Boolean::New(env, true)});
+            auto* cube = value.As<Napi::Pointer<Babylon::Graphics::Texture>>().Get();
+            if (bgfx::isTextureValid(0, true, 1, bgfx::TextureFormat::D24S8, BGFX_TEXTURE_RT))
+            {
+                // Depth cubes take the per-face framebuffer clear path.
+                auto depthCube = createTexture.Call(engine, {});
+                env.Global().Set("_testDepthCube", depthCube);
+                EXPECT_NO_THROW(initializeTexture.Call(engine, {
+                    depthCube, Napi::Number::New(env, size), Napi::Number::New(env, size),
+                    Napi::Boolean::New(env, false), Napi::Number::New(env, bgfx::TextureFormat::D24S8),
+                    Napi::Boolean::New(env, true), Napi::Boolean::New(env, false),
+                    Napi::Number::New(env, 1), Napi::Boolean::New(env, true)}));
+                EXPECT_TRUE(depthCube.As<Napi::Pointer<Babylon::Graphics::Texture>>().Get()->IsCube());
+            }
+            EXPECT_THROW(createFrameBuffer.Call(engine, {
+                value, Napi::Number::New(env, size), Napi::Number::New(env, size),
+                Napi::Boolean::New(env, false), Napi::Boolean::New(env, false),
+                Napi::Number::New(env, 4), Napi::Number::New(env, 0)}), Napi::Error);
+
+            auto readback = std::make_shared<Babylon::Graphics::Texture>(context);
+            readback->Create2D(size * faceCount, size, false, 1, bgfx::TextureFormat::RGBA8,
+                BGFX_TEXTURE_BLIT_DST | BGFX_TEXTURE_READ_BACK);
+            for (uint16_t face = 0; face < faceCount; ++face)
+            {
+                bgfx::TextureRegion destination{};
+                destination.init(readback->Handle(), face * size, 0, size, size);
+                bgfx::TextureRegion source{};
+                source.init(cube->Handle(), 0, 0, size, size);
+                source.z = face;
+                source.depth = 1;
+                context.GetActiveEncoder()->blit(context.AcquireNewViewId(), destination, source);
+            }
+            context.ReadTextureAsync(readback->Handle(), gsl::make_span(pixels))
+                .then(arcana::inline_scheduler, arcana::cancellation::none(), [readback, &completed](arcana::expected<void, std::exception_ptr> result) {
+                    readback->Dispose();
+                    if (result.has_error())
+                    {
+                        completed.set_exception(result.error());
+                    }
+                    else
+                    {
+                        completed.set_value();
+                    }
+                });
+        }
+        catch (const std::exception& ex)
+        {
+            completed.set_exception(std::make_exception_ptr(std::runtime_error{ex.what()}));
+        }
+    });
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds{30};
+    while (future.wait_for(std::chrono::milliseconds{16}) != std::future_status::ready)
+    {
+        if (std::chrono::steady_clock::now() >= deadline)
+        {
+            device.FinishRenderingCurrentFrame();
+            FAIL() << "Cube face readback was not fulfilled within 30s";
+        }
+        device.FinishRenderingCurrentFrame();
+        device.StartRenderingCurrentFrame();
+    }
+    EXPECT_NO_THROW(future.get());
+    for (const uint8_t channel : pixels)
+    {
+        EXPECT_EQ(channel, 0);
+    }
+    device.FinishRenderingCurrentFrame();
+}

Core/Graphics/Source/Texture.cpp

@@ -50,11 +50,11 @@ namespace
     }
 
     void ClearRenderTarget(Babylon::Graphics::DeviceContext& context, bgfx::TextureHandle handle,
-        uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format, uint64_t flags)
+        uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format, uint64_t flags, bool cubeMap = false)
     {
         const bool depthStencil = format > bgfx::TextureFormat::UnknownDepth;
         const bool multisampled = (flags & BGFX_TEXTURE_RT_MSAA_MASK) > BGFX_TEXTURE_RT;
-        // bgfx::clear covers sampled color on every backend, including OpenGL.
+        // bgfx::clear covers sampled color on every backend, including OpenGL, and every cube face.
         // It does not cover depth, or a sampled MSAA resolve image: D3D12 and Vulkan
         // clear the multisample image, which has only mip 0.
         if (!depthStencil && !multisampled)
@@ -73,8 +73,10 @@ namespace
         // BGFX_TEXTURE_MSAA_SAMPLE keeps the sampled image multisampled, so it has no resolve mip chain.
         const bool resolveMipChain = multisampled && !depthStencil && info.numMips > 1 &&
             (flags & BGFX_TEXTURE_MSAA_SAMPLE) == 0;
+        // Cube faces are attached as layer * 6 + face.
+        const uint32_t numSlices = uint32_t{info.numLayers} * (cubeMap ? 6 : 1);
         auto scope = context.AcquireFrameCompletionScope();
-        for (uint16_t layer = 0; layer < info.numLayers; ++layer)
+        for (uint16_t layer = 0; layer < numSlices; ++layer)
         {
             for (uint8_t mip = 0; mip < info.numMips; ++mip)
             {
@@ -274,6 +276,12 @@ namespace Babylon::Graphics
 
         m_ownsHandle = true;
         SetMetadata(size, size, 0, hasMips, true, false, numLayers, format, flags);
+
+        // WebGL texImage2D(..., null) zero-fills every cube face; render targets must match.
+        if ((flags & BGFX_TEXTURE_RT_MASK) != 0)
+        {
+            ClearRenderTarget(m_deviceContext, m_handle, size, size, hasMips, numLayers, format, flags, /*cubeMap*/ true);
+        }
     }
 
     void Texture::UpdateCube(uint16_t layer, uint8_t side, uint8_t mip, uint16_t x, uint16_t y, uint16_t width, uint16_t height, const bgfx::Memory* mem, uint16_t pitch)

Plugins/NativeEngine/Source/NativeEngine.cpp

@@ -1743,6 +1743,10 @@ namespace Babylon
         // Optional array-layer count; also carries volume depth when is3D is set.
         const uint16_t numLayers = (info.Length() > 9 && !info[9].IsUndefined()) ? static_cast<uint16_t>(ReadUnsignedInteger(info[9], "Texture layer/depth count", UINT16_MAX)) : 1;
         const bool is3D = info.Length() > 10 && !info[10].IsUndefined() && info[10].As<Napi::Boolean>();
+        if (isCube && samples > 1)
+        {
+            throw Napi::Error::New(info.Env(), "Multisampled cube render targets are not supported");
+        }
 
         auto flags = BGFX_TEXTURE_NONE;
         if (renderTarget)
@@ -2739,6 +2743,10 @@ namespace Babylon
         const uint32_t samples = info[5].IsUndefined() ? 1 : info[5].As<Napi::Number>().Uint32Value();
         const double layer = info[6].IsUndefined() ? 0 : info[6].As<Napi::Number>().DoubleValue();
         const bool isCube = texture != nullptr && texture->IsCube();
+        if (isCube && samples > 1)
+        {
+            throw Napi::Error::New(info.Env(), "Multisampled cube render targets are not supported");
+        }
         const uint16_t maxLayer = texture == nullptr ? 0 : isCube ? 5 :
             texture->Is3D() ? texture->Depth() - 1 : texture->NumLayers() - 1;
         if (!std::isfinite(layer) || layer != std::floor(layer) || layer < 0 || layer > maxLayer)
@@ -2814,6 +2822,20 @@ namespace Babylon
     Napi::Value NativeEngine::CreateFrameBufferImpl(Napi::Env env, gsl::span<Graphics::Texture* const> colorTextures, uint16_t width, uint16_t height, bool generateStencilBuffer, bool generateDepth, uint32_t samples, uint16_t layer, uint16_t mip, gsl::span<const uint16_t> perAttachmentLayers, Graphics::Texture* explicitDepthTexture, bool autoGenerateMips, Graphics::Texture* depthStencilTexture)
     {
         const bgfx::Caps* caps = bgfx::getCaps();
+        if (samples > 1)
+        {
+            for (Graphics::Texture* texture : colorTextures)
+            {
+                if (texture != nullptr && texture->IsCube())
+                {
+                    throw Napi::Error::New(env, "Multisampled cube render targets are not supported");
+                }
+            }
+            if (depthStencilTexture != nullptr && depthStencilTexture->IsCube())
+            {
+                throw Napi::Error::New(env, "Multisampled cube render targets are not supported");
+            }
+        }
         const uint32_t colorCount = static_cast<uint32_t>(colorTextures.size());
         // One slot per color attachment, plus a single depth/stencil attachment only when one is
         // generated. bgfx caps the total via maxFBAttachments; reject out-of-range counts up front

Plugins/NativeMeshopt/Include/Babylon/Plugins/NativeMeshopt.h

@@ -7,7 +7,8 @@ namespace Babylon::Plugins::NativeMeshopt
 {
     // Exposes `_native.decodeMeshopt(source, count, stride, mode, filter?)`, a
     // synchronous native replacement for Babylon's WebAssembly meshopt decoder
-    // (zeux/meshoptimizer). Babylon.js routes its MeshoptCompression to this
-    // function when it is present.
+    // (zeux/meshoptimizer). This is a compatibility export: the pinned package
+    // and the current public MeshoptCompression implementation do not
+    // reference it and use the script-based decoder.
     void BABYLON_API Initialize(Napi::Env env);
 }

Plugins/NativeMeshopt/README.md

@@ -9,7 +9,7 @@ The plugin is **off by default**. Enable it with `-D BABYLON_NATIVE_PLUGIN_NATIV
 ## Limitations
 
 - **Decode only.** Encoding is an authoring-time concern that Babylon Native does not exercise.
-- **Compatibility entry point.** Babylon.js probes `_native.decodeMeshopt`; this free-function entry point uses the same decoder as `_native.MeshoptCodec.Decode`. The grouped API remains available.
+- **Compatibility entry point.** `_native.decodeMeshopt` is a compatibility export. It is not consumed by the pinned Babylon.js package or the current public `MeshoptCompression` implementation, which use the script-based decoder. The free function uses the same decoder as `_native.MeshoptCodec.Decode`. The grouped API remains available.
 
 ## Design
 

Plugins/NativeMeshopt/Source/NativeMeshopt.cpp

@@ -166,8 +166,9 @@ namespace Babylon::Plugins::NativeMeshopt
         codec.Set("Version", Napi::String::New(env, MeshoptVersionString()));
         native.Set("MeshoptCodec", codec);
 
-        // Legacy free-function name. Babylon.js feature-probes `_native.decodeMeshopt`, so keep
-        // this until the JavaScript side moves to `_native.MeshoptCodec`.
+        // Compatibility free-function name. The pinned Babylon.js package and the current public
+        // MeshoptCompression implementation do not reference `_native.decodeMeshopt`; both use
+        // the script-based decoder. Keep the export so older callers share MeshoptCodec.Decode.
         native.Set("decodeMeshopt", Napi::Function::New(env, DecodeMeshopt, "decodeMeshopt"));
     }
 }