From eec4acac966530f698694c26c0d98315cf4d89d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Thu, 1 Oct 2026 17:07:33 -0700 Subject: [PATCH 1/2] Address review follow-ups for cube targets, validation, and Meshopt Reject multisampled cube render targets instead of creating framebuffers that read back as zeros. Zero-fill cube render-target faces to match WebGL texImage2D(null), and read them before any clear. Bound validation convergence by elapsed time, inspect every render pass the next frame can use, and enroll only utility layers that render automatically. Poll every associated scene before combining readiness. Describe decodeMeshopt as a compatibility export that pinned Babylon.js 9.21.2 and current MeshoptCompression do not consume. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6c3c123-c355-4790-a3d4-94337ed6e052 --- Apps/Playground/Scripts/validation_native.js | 271 +++++++++++++++--- .../Tests.NativeEngine.CubeRenderTargets.cpp | 86 ++++++ Core/Graphics/Source/Texture.cpp | 15 +- Documentation/AddingNewValidationTests.md | 21 +- Plugins/NativeEngine/Source/NativeEngine.cpp | 18 ++ .../Include/Babylon/Plugins/NativeMeshopt.h | 5 +- Plugins/NativeMeshopt/README.md | 2 +- .../NativeMeshopt/Source/NativeMeshopt.cpp | 5 +- 8 files changed, 369 insertions(+), 54 deletions(-) diff --git a/Apps/Playground/Scripts/validation_native.js b/Apps/Playground/Scripts/validation_native.js index 4e6b3ac9c9..786ac7ede5 100644 --- a/Apps/Playground/Scripts/validation_native.js +++ b/Apps/Playground/Scripts/validation_native.js @@ -51,23 +51,25 @@ // Stopgap so native validation can pass. Examples should wait for their own // scene, material, GUI, and utility-scene resources; that belongs in the // examples, not this harness. Waiting here only because fixing each test - // individually is a much larger task. - const MAX_CONVERGENCE_TICKS = 240; + // individually is a much larger task. Bound by elapsed time: a fast render + // loop (Ubuntu GCC JavaScriptCore exhausted 240 callbacks in about 405 ms) + // must not fail asynchronous GUI work before it can complete. + const CONVERGENCE_DEADLINE_MS = 60 * 1000; const INITIAL_READINESS_TIMEOUT_MS = 10 * 60 * 1000; const READINESS_RECONCILE_INTERVAL_MS = 100; - const utilityLayerOwners = new WeakMap(); + const utilityLayerRenderers = new WeakMap(); const dracoDecoderConfiguration = Object.assign({}, BABYLON.DracoDecoder.DefaultConfiguration); const dracoEncoderConfiguration = Object.assign({}, BABYLON.DracoEncoder.DefaultConfiguration); const dracoDefaultNumWorkers = BABYLON.DracoCompression.DefaultNumWorkers; const dracoDecoderModule = globalThis.DracoDecoderModule; const dracoEncoderModule = globalThis.DracoEncoderModule; - // UtilityLayerRenderer exposes both sides of the association, but its utility - // scene can have no active camera when the layer is created before the main camera. + // Retain the renderer, not only its owner. manualRender never installs the + // after-render observer; shouldRender can change after the layer is created. const updateUtilityLayerCamera = BABYLON.UtilityLayerRenderer.prototype._updateCamera; BABYLON.UtilityLayerRenderer.prototype._updateCamera = function () { const result = updateUtilityLayerCamera.apply(this, arguments); - utilityLayerOwners.set(this.utilityLayerScene, this.originalScene); + utilityLayerRenderers.set(this.utilityLayerScene, this); return result; }; let havokInitializationPromise; @@ -95,6 +97,10 @@ return havokInitializationPromise; } + function utilityLayerRendersAutomatically(renderer) { + return !!(renderer && renderer._afterRenderObserver && renderer.shouldRender !== false); + } + function shouldRunTest(test, index) { if (testIndices.length > 0 && testIndices.indexOf(index) === -1) { return false; @@ -500,44 +506,147 @@ }); } + function hasPassId(value) { + return value !== undefined && value !== null; + } + + function addPass(passes, seen, id) { + if (!hasPassId(id) || seen[id]) { + return; + } + seen[id] = true; + passes.push(id); + } + + function addRenderTargetPass(passes, seen, renderTarget) { + if (!renderTarget) { + return; + } + if (typeof renderTarget._shouldRender === "function" && !renderTarget._shouldRender()) { + return; + } + if (hasPassId(renderTarget.renderPassId)) { + addPass(passes, seen, renderTarget.renderPassId); + } + } + + function addCameraRenderTargetPasses(passes, seen, camera) { + const targets = camera && camera.customRenderTargets; + if (!targets) { + return; + } + for (let i = 0; i < targets.length; i++) { + addRenderTargetPass(passes, seen, targets[i]); + } + } + + // Matches Scene._renderForCamera: outputRenderTarget.renderPassId wins when set. + // Multiview-to-single-view assigns the multiview texture only while rendering. + function effectiveRenderPassId(camera) { + if (camera._useMultiviewToSingleView && camera._multiviewTexture && hasPassId(camera._multiviewTexture.renderPassId)) { + return camera._multiviewTexture.renderPassId; + } + if (camera.outputRenderTarget && hasPassId(camera.outputRenderTarget.renderPassId)) { + return camera.outputRenderTarget.renderPassId; + } + if (hasPassId(camera.renderPassId)) { + return camera.renderPassId; + } + return 0; + } + + function renderPassesForNextFrame(scene) { + const passes = []; + const seen = {}; + const roots = scene.activeCameras && scene.activeCameras.length > 0 + ? scene.activeCameras + : (scene.activeCamera ? [scene.activeCamera] : []); + const rigModeNone = BABYLON.Constants && BABYLON.Constants.RIG_MODE_NONE !== undefined + ? BABYLON.Constants.RIG_MODE_NONE + : 0; + for (let i = 0; i < roots.length; i++) { + const camera = roots[i]; + if (!camera) { + continue; + } + const rigCameras = camera._rigCameras || camera.rigCameras; + const renderRigCameras = rigCameras && rigCameras.length > 0 && + camera.cameraRigMode !== rigModeNone && + !camera._renderingMultiview && + !camera._useMultiviewToSingleView; + if (renderRigCameras) { + addCameraRenderTargetPasses(passes, seen, camera); + for (let j = 0; j < rigCameras.length; j++) { + const rigCamera = rigCameras[j]; + if (!rigCamera) { + continue; + } + addPass(passes, seen, effectiveRenderPassId(rigCamera)); + addCameraRenderTargetPasses(passes, seen, rigCamera); + } + } else { + addPass(passes, seen, effectiveRenderPassId(camera)); + addCameraRenderTargetPasses(passes, seen, camera); + } + } + if (scene.renderTargetsEnabled !== false && scene.customRenderTargets) { + for (let i = 0; i < scene.customRenderTargets.length; i++) { + addRenderTargetPass(passes, seen, scene.customRenderTargets[i]); + } + } + return passes; + } + + function meshesUseReadyEffects(scene) { + let ready = true; + for (let i = 0; i < scene.meshes.length; i++) { + const mesh = scene.meshes[i]; + if (!mesh.isEnabled() || !mesh.subMeshes || mesh.subMeshes.length === 0) { + continue; + } + for (let j = 0; j < mesh.subMeshes.length; j++) { + const subMesh = mesh.subMeshes[j]; + const defines = subMesh.materialDefines; + if (defines && defines.isDirty) { + ready = false; + } + const effect = subMesh.effect; + if (effect && !effect.isReady()) { + ready = false; + } + } + } + return ready; + } + function isSceneConverged(scene) { const engine = scene.getEngine(); const previousRenderPassId = engine.currentRenderPassId; + let converged = true; try { if (!scene.isReady()) { - return false; + converged = false; } if (!areGuiTexturesReady(scene)) { - return false; + converged = false; } // Hot-swapping materials may report ready while their replacement effect is - // still compiling. Inspect the camera's draw wrappers, not an unused pass. - const cameraRenderPassId = scene.activeCamera && scene.activeCamera.renderPassId; - engine.currentRenderPassId = cameraRenderPassId === null || cameraRenderPassId === undefined - ? previousRenderPassId - : cameraRenderPassId; - for (let i = 0; i < scene.meshes.length; i++) { - const mesh = scene.meshes[i]; - if (!mesh.isEnabled() || !mesh.subMeshes || mesh.subMeshes.length === 0) { - continue; - } - for (let j = 0; j < mesh.subMeshes.length; j++) { - const subMesh = mesh.subMeshes[j]; - const defines = subMesh.materialDefines; - if (defines && defines.isDirty) { - return false; - } - const effect = subMesh.effect; - if (effect && !effect.isReady()) { - return false; - } + // still compiling. Inspect every pass the next frame can draw, not an unused one. + const passes = renderPassesForNextFrame(scene); + if (passes.length === 0) { + passes.push(previousRenderPassId); + } + for (let i = 0; i < passes.length; i++) { + engine.currentRenderPassId = passes[i]; + if (!meshesUseReadyEffects(scene)) { + converged = false; } } } finally { engine.currentRenderPassId = previousRenderPassId; } - return true; + return converged; } function getConvergenceScenes(scene) { @@ -545,18 +654,99 @@ const virtualScenes = scene.getEngine()._virtualScenes; for (let i = 0; i < virtualScenes.length; i++) { const virtualScene = virtualScenes[i]; - const utilityLayerOwner = utilityLayerOwners.get(virtualScene); + if (virtualScene === scene) { + continue; + } + const renderer = utilityLayerRenderers.get(virtualScene); + if (renderer) { + // Ownership alone includes manual layers and layers with shouldRender false. + if (renderer.originalScene === scene && utilityLayerRendersAutomatically(renderer)) { + scenes.push(virtualScene); + } + continue; + } // Non-utility virtual scenes can still be associated through a shared camera. - const sharesCamera = utilityLayerOwner === undefined && - virtualScene.activeCamera && - virtualScene.activeCamera.getScene() === scene; - if (virtualScene !== scene && (utilityLayerOwner === scene || sharesCamera)) { + if (virtualScene.activeCamera && virtualScene.activeCamera.getScene() === scene) { scenes.push(virtualScene); } } return scenes; } + function allScenesConverged(scenes) { + // Do not stop at the first unready scene. Later scenes must start their + // readiness and compilation work in the same callback. + let converged = true; + for (let i = 0; i < scenes.length; i++) { + if (!isSceneConverged(scenes[i])) { + converged = false; + } + } + return converged; + } + + function assertScheduling(condition, message) { + if (!condition) { + throw new Error("validation scheduling check failed: " + message); + } + } + + function passListContains(passes, id) { + return passes.indexOf(id) !== -1; + } + + function runSchedulingSelfCheck() { + const outputScene = { + activeCamera: { renderPassId: 3, outputRenderTarget: { renderPassId: 9 } }, + customRenderTargets: [ + { renderPassId: 4, _shouldRender: function () { return true; } }, + { renderPassId: 5, _shouldRender: function () { return false; } } + ] + }; + const outputPasses = renderPassesForNextFrame(outputScene); + assertScheduling(passListContains(outputPasses, 9) && !passListContains(outputPasses, 3), "output render-target pass"); + assertScheduling(passListContains(outputPasses, 4) && !passListContains(outputPasses, 5), "scheduled custom render targets"); + + const rigScene = { + activeCamera: { + renderPassId: 1, + cameraRigMode: 1, + customRenderTargets: [{ renderPassId: 6, _shouldRender: function () { return true; } }], + _rigCameras: [ + { renderPassId: 2 }, + { renderPassId: 8, outputRenderTarget: { renderPassId: 11 } } + ] + } + }; + const rigPasses = renderPassesForNextFrame(rigScene); + assertScheduling(passListContains(rigPasses, 2) && passListContains(rigPasses, 11) && passListContains(rigPasses, 6) && !passListContains(rigPasses, 1), "rig-camera passes"); + + const main = { getEngine: function () { return { _virtualScenes: virtualScenes }; } }; + const autoVirtual = {}; + const manualVirtual = {}; + const sharedVirtual = { activeCamera: { getScene: function () { return main; } } }; + const virtualScenes = [autoVirtual, manualVirtual, sharedVirtual]; + const autoRenderer = { originalScene: main, utilityLayerScene: autoVirtual, _afterRenderObserver: {}, shouldRender: false }; + utilityLayerRenderers.set(autoVirtual, autoRenderer); + utilityLayerRenderers.set(manualVirtual, { originalScene: main, utilityLayerScene: manualVirtual, _afterRenderObserver: null, shouldRender: true }); + assertScheduling(getConvergenceScenes(main).indexOf(autoVirtual) === -1, "disabled utility layer excluded"); + autoRenderer.shouldRender = true; + const enabled = getConvergenceScenes(main); + assertScheduling(enabled.indexOf(autoVirtual) !== -1 && enabled.indexOf(sharedVirtual) !== -1 && enabled.indexOf(manualVirtual) === -1, "automatic utility layer and shared camera"); + autoRenderer.shouldRender = false; + assertScheduling(getConvergenceScenes(main).indexOf(autoVirtual) === -1, "dynamic utility-layer disable"); + utilityLayerRenderers.delete(autoVirtual); + utilityLayerRenderers.delete(manualVirtual); + + const polled = []; + const combined = allScenesConverged([ + { isReady: function () { polled.push(1); return false; }, textures: [], meshes: [], getEngine: function () { return { currentRenderPassId: 0 }; } }, + { isReady: function () { polled.push(2); return true; }, textures: [], meshes: [], getEngine: function () { return { currentRenderPassId: 0 }; } } + ]); + assertScheduling(polled.join(",") === "1,2" && combined === false, "every associated scene is polled"); + } + runSchedulingSelfCheck(); + function processCurrentScene(test, renderImage, done, compareFunction) { currentScene.useConstantAnimationDeltaTime = true; // Capture options must not shift the pixel-comparison frame. @@ -574,7 +764,7 @@ let stopped = false; let pendingScreenshot = null; let evaluated = false; - let convergenceTicks = 0; + let convergenceStartedAt = 0; let readinessScenes = []; let readyScenes = []; let readinessReconcileTimer = null; @@ -633,16 +823,19 @@ // Recompute because utility layers can be attached or disposed while // convergence is pending, updating the engine's virtual-scene list. const convergenceScenes = getConvergenceScenes(currentScene); - if (!convergenceScenes.every(isSceneConverged)) { - if (convergenceTicks >= MAX_CONVERGENCE_TICKS) { + if (!allScenesConverged(convergenceScenes)) { + const now = Date.now(); + if (convergenceStartedAt === 0) { + convergenceStartedAt = now; + } + if (now - convergenceStartedAt >= CONVERGENCE_DEADLINE_MS) { stopped = true; evaluated = true; console.error("Scene '" + (test.title || "?") + "' did not converge within " + - MAX_CONVERGENCE_TICKS + " render-loop ticks (scene, material, or GUI readiness)."); + (CONVERGENCE_DEADLINE_MS / 1000) + "s (scene, material, or GUI readiness)."); failTest(done); return; } - convergenceTicks++; // Refresh material readiness without rendering extra animation/particle frames. for (let i = 0; i < convergenceScenes.length; i++) { convergenceScenes[i].incrementRenderId(); diff --git a/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp b/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp index cb72004352..e81edcade6 100644 --- a/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp +++ b/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp @@ -290,3 +290,89 @@ 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 pixels{}; + std::promise 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().Get("Engine").As().New({}); + auto createTexture = engine.Get("createTexture").As(); + auto initializeTexture = engine.Get("initializeTexture").As(); + auto createFrameBuffer = engine.Get("createFrameBuffer").As(); + 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>().Get(); + 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(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 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()})); + } + }); + while (future.wait_for(std::chrono::milliseconds{16}) != std::future_status::ready) + { + device.FinishRenderingCurrentFrame(); + device.StartRenderingCurrentFrame(); + } + EXPECT_NO_THROW(future.get()); + for (const uint8_t channel : pixels) + { + EXPECT_EQ(channel, 0); + } + device.FinishRenderingCurrentFrame(); +} diff --git a/Core/Graphics/Source/Texture.cpp b/Core/Graphics/Source/Texture.cpp index 2a842b143a..9819a0ac2f 100644 --- a/Core/Graphics/Source/Texture.cpp +++ b/Core/Graphics/Source/Texture.cpp @@ -8,6 +8,15 @@ namespace { + const bgfx::Memory* GetZeroImageMemory(uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format, bool cubeMap) + { + bgfx::TextureInfo info{}; + bgfx::calcTextureSize(info, width, height, /*depth*/ 1, cubeMap, hasMips, numLayers, format); + const bgfx::Memory* mem = bgfx::alloc(info.storageSize); + std::memset(mem->data, 0, mem->size); + return mem; + } + // Sampled MSAA color lives in a single-sample resolve image. Its mip chain is filled // only when resolve runs with BGFX_ATTACHMENT_AUTO_GEN_MIPS, which happens when the // backend leaves that framebuffer. A later readback resolves mip 0 and drops this flag. @@ -266,7 +275,11 @@ namespace Babylon::Graphics { Dispose(); - m_handle = bgfx::createTextureCube(size, hasMips, numLayers, format, flags); + // WebGL texImage2D(..., null) zero-fills every cube face. A cube created without + // memory is uninitialized, so render targets must upload explicit zeros. + const auto* mem = (flags & BGFX_TEXTURE_RT) ? GetZeroImageMemory(size, size, hasMips, numLayers, format, true) : nullptr; + + m_handle = bgfx::createTextureCube(size, hasMips, numLayers, format, flags, mem); if (!bgfx::isValid(m_handle)) { throw std::runtime_error{"Failed to create cube texture"}; diff --git a/Documentation/AddingNewValidationTests.md b/Documentation/AddingNewValidationTests.md index f9876d6181..d32d33da69 100644 --- a/Documentation/AddingNewValidationTests.md +++ b/Documentation/AddingNewValidationTests.md @@ -31,21 +31,24 @@ In order to add a new test scene, first thing to do is to add a few lines in `Ap # Readiness and deterministic capture The Native runner waits for scene readiness, GUI image readiness, and clean material -defines with ready effects in the active camera's render pass. Utility scenes using -the main scene's camera participate in the same check. Inspection restores the -previous render pass, including on errors. +defines with ready effects in every render pass the next frame can use, including an +output render target and each rig camera. Only utility layers that render automatically +participate; manual layers and layers with `shouldRender` false do not. Inspection +restores the previous render pass, including on errors. The initial readiness wait and its 10-minute timeout cover both the main scene and associated utility scenes. Their pending model/texture loads do not consume the subsequent convergence checks. Readiness polling does not render extra frames or consume `renderCount`. It refreshes -scene render IDs so material readiness is checked again on the next tick. A scene -that still has not converged after 240 waiting render-loop ticks fails explicitly -and follows normal once-only cleanup and suite continuation. Screenshot and -RenderDoc capture indices still count rendered frames only. Failures invalidate -pending screenshot callbacks so they cannot evaluate after cleanup or during the -next scene. +scene render IDs so material readiness is checked again on the next tick. Waiting is +bounded by wall-clock time, not callback count: a fast render loop can exhaust a tick +cap before asynchronous GUI work finishes. A scene that still has not converged after +60 seconds fails explicitly and follows normal once-only cleanup and suite continuation. +Each associated scene is polled before the results are combined, including every render +pass the next frame can use. Screenshot and RenderDoc capture indices still count +rendered frames only. Failures invalidate pending screenshot callbacks so they cannot +evaluate after cleanup or during the next scene. Each test restores both the seeded `Math.random` function and its seed, so a snippet that replaces `Math.random` cannot change the sequence used by the next test. diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 5dc06d1eed..eb1d233a0b 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/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(ReadUnsignedInteger(info[9], "Texture layer/depth count", UINT16_MAX)) : 1; const bool is3D = info.Length() > 10 && !info[10].IsUndefined() && info[10].As(); + 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().Uint32Value(); const double layer = info[6].IsUndefined() ? 0 : info[6].As().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,16 @@ namespace Babylon Napi::Value NativeEngine::CreateFrameBufferImpl(Napi::Env env, gsl::span colorTextures, uint16_t width, uint16_t height, bool generateStencilBuffer, bool generateDepth, uint32_t samples, uint16_t layer, uint16_t mip, gsl::span 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"); + } + } + } const uint32_t colorCount = static_cast(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 diff --git a/Plugins/NativeMeshopt/Include/Babylon/Plugins/NativeMeshopt.h b/Plugins/NativeMeshopt/Include/Babylon/Plugins/NativeMeshopt.h index 12f004aed5..9d3af55360 100644 --- a/Plugins/NativeMeshopt/Include/Babylon/Plugins/NativeMeshopt.h +++ b/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); } diff --git a/Plugins/NativeMeshopt/README.md b/Plugins/NativeMeshopt/README.md index 0d9d4b6c63..02752cc23b 100644 --- a/Plugins/NativeMeshopt/README.md +++ b/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 diff --git a/Plugins/NativeMeshopt/Source/NativeMeshopt.cpp b/Plugins/NativeMeshopt/Source/NativeMeshopt.cpp index f5b42f49f4..9e60bbd3bc 100644 --- a/Plugins/NativeMeshopt/Source/NativeMeshopt.cpp +++ b/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")); } } From 53e2e6d5ed14e7e5d1bfbd434f47814db125283c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Thu, 1 Oct 2026 18:04:23 -0700 Subject: [PATCH 2/2] Address Copilot review notes on the follow-up Fail the cube readback test if the promise is not fulfilled within 30 seconds. Reject a multisampled cube used as the depth attachment as well as a color attachment. Deduplicate render passes with a Set, and report scheduling self-check failures without aborting the validation suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6c3c123-c355-4790-a3d4-94337ed6e052 --- Apps/Playground/Scripts/validation_native.js | 14 ++++++++++---- .../Tests.NativeEngine.CubeRenderTargets.cpp | 5 +++++ Plugins/NativeEngine/Source/NativeEngine.cpp | 4 ++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/Apps/Playground/Scripts/validation_native.js b/Apps/Playground/Scripts/validation_native.js index 786ac7ede5..3c0580fd7a 100644 --- a/Apps/Playground/Scripts/validation_native.js +++ b/Apps/Playground/Scripts/validation_native.js @@ -511,10 +511,10 @@ } function addPass(passes, seen, id) { - if (!hasPassId(id) || seen[id]) { + if (!hasPassId(id) || seen.has(id)) { return; } - seen[id] = true; + seen.add(id); passes.push(id); } @@ -557,7 +557,7 @@ function renderPassesForNextFrame(scene) { const passes = []; - const seen = {}; + const seen = new Set(); const roots = scene.activeCameras && scene.activeCameras.length > 0 ? scene.activeCameras : (scene.activeCamera ? [scene.activeCamera] : []); @@ -745,7 +745,13 @@ ]); assertScheduling(polled.join(",") === "1,2" && combined === false, "every associated scene is polled"); } - runSchedulingSelfCheck(); + // Coverage for the scheduling helpers. A failure is reported, but it must not + // abort the suite if a host embeds this script or Babylon internals shift. + try { + runSchedulingSelfCheck(); + } catch (e) { + console.error(e); + } function processCurrentScene(test, renderImage, done, compareFunction) { currentScene.useConstantAnimationDeltaTime = true; diff --git a/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp b/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp index e81edcade6..4c33ab285e 100644 --- a/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp +++ b/Apps/UnitTests/Source/Tests.NativeEngine.CubeRenderTargets.cpp @@ -364,8 +364,13 @@ TEST(NativeEngineCubeRenderTargets, RejectsMultisamplingAndZeroFillsFacesBeforeC 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) + { + FAIL() << "Cube face readback was not fulfilled within 30s"; + } device.FinishRenderingCurrentFrame(); device.StartRenderingCurrentFrame(); } diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index eb1d233a0b..ba92d2f153 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -2831,6 +2831,10 @@ namespace Babylon 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(colorTextures.size()); // One slot per color attachment, plus a single depth/stencil attachment only when one is