From c5aea50ec0b0988d132b3f98a367fbc7d4b9428d Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Fri, 11 Sep 2026 13:28:51 -0700 Subject: [PATCH 1/3] Use supported backing storage for D24 render targets Validate InitializeTexture's format and effective creation flags before allocating. Keep supported D24 requests unchanged; use D24S8 backing only when a D24 render target is unsupported and the packed format is supported. Reject invalid or unsupported requests without replacing an existing texture. Add native regressions for backing formats, single-sampled framebuffer attachment, multisample allocation flags, ordinary color textures, and JavaScript errors. No dependency pins or visual fixtures change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Apps/UnitTests/CMakeLists.txt | 1 + .../Tests.NativeEngine.TextureFormats.cpp | 186 ++++++++++++++++++ Plugins/NativeEngine/Source/NativeEngine.cpp | 23 ++- 3 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp diff --git a/Apps/UnitTests/CMakeLists.txt b/Apps/UnitTests/CMakeLists.txt index 1c7d187b0..4da2d633b 100644 --- a/Apps/UnitTests/CMakeLists.txt +++ b/Apps/UnitTests/CMakeLists.txt @@ -37,6 +37,7 @@ set(SOURCES "Source/Tests.JavaScript.cpp" "Source/Tests.NativeEngine.InstanceData.cpp" "Source/Tests.NativeEngine.Teardown.cpp" + "Source/Tests.NativeEngine.TextureFormats.cpp" "Source/Tests.ShaderCache.cpp" "Source/Tests.ShaderCompilation.cpp" "Source/Tests.ShaderCompilation.FragCoord.cpp" diff --git a/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp new file mode 100644 index 000000000..cbc73f201 --- /dev/null +++ b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp @@ -0,0 +1,186 @@ +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +extern Babylon::Graphics::Configuration g_deviceConfig; + +namespace +{ + void RunTextureTest(const std::function& test) + { + Babylon::Graphics::Device device{g_deviceConfig}; + device.StartRenderingCurrentFrame(); + std::promise completed; + auto completion = completed.get_future(); + { + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&](Napi::Env env) { + try + { + device.AddToJavaScript(env); + Babylon::Plugins::NativeEngine::Initialize(env); + auto run = env.RunScript(R"( + (function(test) { + const engine = new _native.Engine(); + const texture = engine.createTexture(); + try { + test(engine, texture); + } finally { + engine.deleteTexture(texture); + engine.dispose(); + } + }) + )").As(); + run.Call({Napi::Function::New(env, [&](const Napi::CallbackInfo& info) { + test(info[0].As(), info[1]); + })}); + completed.set_value(); + } + catch (const Napi::Error& error) + { + completed.set_exception(std::make_exception_ptr(std::runtime_error{Napi::GetErrorString(error)})); + } + catch (...) + { + completed.set_exception(std::current_exception()); + } + }); + if (completion.wait_for(std::chrono::seconds{30}) != std::future_status::ready) + { + // Do not enter a potentially blocked runtime destructor after a GPU assertion. + std::cerr << "Timed out waiting for NativeEngine texture format test" << std::endl; + std::quick_exit(1); + } + } + device.FinishRenderingCurrentFrame(); + ASSERT_NO_THROW(completion.get()); + } + + void InitializeTexture(Napi::Object engine, Napi::Value texture, uint32_t format, bool renderTarget, bool srgb = false, uint32_t samples = 1) + { + const auto env = engine.Env(); + engine.Get("initializeTexture").As().Call(engine, { + texture, Napi::Number::New(env, 16), Napi::Number::New(env, 16), Napi::Boolean::New(env, false), + Napi::Number::New(env, format), Napi::Boolean::New(env, renderTarget), Napi::Boolean::New(env, srgb), + Napi::Number::New(env, samples)}); + } + + void ExpectInitializationError(Napi::Object engine, Napi::Value texture, uint32_t format, bool renderTarget, bool srgb, const char* message, uint32_t samples = 1) + { + try + { + InitializeTexture(engine, texture, format, renderTarget, srgb, samples); + FAIL() << "initializeTexture accepted an invalid texture request"; + } + catch (const Napi::Error& error) + { + EXPECT_NE(Napi::GetErrorString(error).find(message), std::string::npos); + } + } + + bool IsSupported(bgfx::TextureFormat::Enum format, uint64_t flags) + { + return bgfx::isTextureValid(0, false, 1, format, flags | BGFX_TEXTURE_BLIT_DST); + } +} + +TEST(NativeEngineTextureFormats, D24RenderTargetUsesSupportedBackingStorage) +{ + RunTextureTest([](Napi::Object engine, Napi::Value value) { + auto* texture = value.As>().Get(); + for (const uint32_t samples : {1u, 4u}) + { + SCOPED_TRACE(samples); + texture->Dispose(); + const auto flags = BGFX_TEXTURE_RT | (samples == 4 ? BGFX_TEXTURE_RT_MSAA_X4 : BGFX_TEXTURE_NONE); + const auto expectedFormat = IsSupported(bgfx::TextureFormat::D24, flags) + ? bgfx::TextureFormat::D24 : bgfx::TextureFormat::D24S8; + if (!IsSupported(expectedFormat, flags)) + { + ExpectInitializationError(engine, value, bgfx::TextureFormat::D24, true, false, "Unsupported texture format", samples); + EXPECT_FALSE(texture->IsValid()); + continue; + } + + InitializeTexture(engine, value, bgfx::TextureFormat::D24, true, false, samples); + ASSERT_TRUE(texture->IsValid()); + EXPECT_EQ(texture->Format(), expectedFormat); + EXPECT_EQ(texture->Width(), 16); + EXPECT_EQ(texture->Height(), 16); + EXPECT_EQ(texture->Flags(), flags); + + if (samples == 1) + { + bgfx::Attachment attachment{}; + attachment.init(texture->Handle(), bgfx::Access::Write, 0, 1, 0, BGFX_RESOLVE_NONE); + const auto frameBuffer = bgfx::createFrameBuffer(1, &attachment, false); + ASSERT_TRUE(bgfx::isValid(frameBuffer)); + bgfx::destroy(frameBuffer); + } + } + }); +} + +TEST(NativeEngineTextureFormats, DoesNotSubstituteNonRenderTargetD24) +{ + RunTextureTest([](Napi::Object engine, Napi::Value value) { + auto* texture = value.As>().Get(); + if (!IsSupported(bgfx::TextureFormat::D24, BGFX_TEXTURE_NONE)) + { + ExpectInitializationError(engine, value, bgfx::TextureFormat::D24, false, false, "Unsupported texture format"); + EXPECT_FALSE(texture->IsValid()); + return; + } + InitializeTexture(engine, value, bgfx::TextureFormat::D24, false); + EXPECT_TRUE(texture->IsValid()); + EXPECT_EQ(texture->Format(), bgfx::TextureFormat::D24); + }); +} + +TEST(NativeEngineTextureFormats, PreservesSupportedColorFormats) +{ + RunTextureTest([](Napi::Object engine, Napi::Value value) { + for (const bool renderTarget : {false, true}) + { + InitializeTexture(engine, value, bgfx::TextureFormat::RGBA8, renderTarget); + auto* texture = value.As>().Get(); + EXPECT_TRUE(texture->IsValid()); + EXPECT_EQ(texture->Format(), bgfx::TextureFormat::RGBA8); + EXPECT_EQ(texture->Flags(), renderTarget ? BGFX_TEXTURE_RT : BGFX_TEXTURE_NONE); + } + }); +} + +TEST(NativeEngineTextureFormats, RejectedInitializationPreservesExistingTexture) +{ + RunTextureTest([](Napi::Object engine, Napi::Value value) { + InitializeTexture(engine, value, bgfx::TextureFormat::RGBA8, true); + auto* texture = value.As>().Get(); + const auto originalHandle = texture->Handle(); + for (const uint32_t format : {static_cast(bgfx::TextureFormat::Count), UINT32_MAX}) + { + ExpectInitializationError(engine, value, format, true, false, "Invalid texture format"); + EXPECT_EQ(texture->Handle().idx, originalHandle.idx); + EXPECT_EQ(texture->Format(), bgfx::TextureFormat::RGBA8); + } + + const auto flags = BGFX_TEXTURE_RT | BGFX_TEXTURE_SRGB; + if (!IsSupported(bgfx::TextureFormat::D24, flags) && !IsSupported(bgfx::TextureFormat::D24S8, flags)) + { + ExpectInitializationError(engine, value, bgfx::TextureFormat::D24, true, true, "Unsupported texture format"); + EXPECT_EQ(texture->Handle().idx, originalHandle.idx); + EXPECT_EQ(texture->Format(), bgfx::TextureFormat::RGBA8); + } + }); +} diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 1fbef693c..70f7fc928 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1635,7 +1635,12 @@ namespace Babylon const uint16_t width = static_cast(info[1].As().Uint32Value()); const uint16_t height = static_cast(info[2].As().Uint32Value()); const bool hasMips = info[3].As(); - const bgfx::TextureFormat::Enum format = static_cast(info[4].As().Uint32Value()); + const uint32_t formatValue = info[4].As().Uint32Value(); + if (formatValue >= bgfx::TextureFormat::Count) + { + throw Napi::Error::New(info.Env(), "Invalid texture format"); + } + auto format = static_cast(formatValue); const bool renderTarget = info[5].As(); const bool srgb = info[6].As(); const uint32_t samples = info[7].IsUndefined() ? 1 : info[7].As().Uint32Value(); @@ -1650,6 +1655,22 @@ namespace Babylon flags |= BGFX_TEXTURE_SRGB; } + // Texture::Create2D also adds BLIT_DST for Babylon-owned textures. + const auto createFlags = flags | BGFX_TEXTURE_BLIT_DST; + if (!bgfx::isTextureValid(0, false, 1, format, createFlags)) + { + // Some backends support 24-bit depth only with packed stencil storage. + if (renderTarget && format == bgfx::TextureFormat::D24 && + bgfx::isTextureValid(0, false, 1, bgfx::TextureFormat::D24S8, createFlags)) + { + format = bgfx::TextureFormat::D24S8; + } + else + { + throw Napi::Error::New(info.Env(), "Unsupported texture format for requested flags"); + } + } + texture->Create2D(width, height, hasMips, 1, format, flags); } From f6dcb17cc2f122967327c71d6944d0c861a4ed2f Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Fri, 11 Sep 2026 13:49:20 -0700 Subject: [PATCH 2/3] Make texture format validation and regressions portable Reject fractional, non-finite, negative, and out-of-range numbers before converting a texture format to an enum. Cover values that previously wrapped or truncated to valid formats and replaced an existing texture. Use Napi::Eval instead of the non-JSI Env::RunScript extension. Compare Error::Message rather than a stack string, since JavaScriptCore stacks do not include the error message. Preserve both in unexpected-error output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- .../Tests.NativeEngine.TextureFormats.cpp | 22 +++++++++++++------ Plugins/NativeEngine/Source/NativeEngine.cpp | 5 +++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp index cbc73f201..34a5519d6 100644 --- a/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp +++ b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -11,6 +12,7 @@ #include #include #include +#include #include extern Babylon::Graphics::Configuration g_deviceConfig; @@ -30,7 +32,7 @@ namespace { device.AddToJavaScript(env); Babylon::Plugins::NativeEngine::Initialize(env); - auto run = env.RunScript(R"( + auto run = Napi::Eval(env, R"( (function(test) { const engine = new _native.Engine(); const texture = engine.createTexture(); @@ -41,7 +43,7 @@ namespace engine.dispose(); } }) - )").As(); + )", "native-texture-format-test.js").As(); run.Call({Napi::Function::New(env, [&](const Napi::CallbackInfo& info) { test(info[0].As(), info[1]); })}); @@ -49,7 +51,7 @@ namespace } catch (const Napi::Error& error) { - completed.set_exception(std::make_exception_ptr(std::runtime_error{Napi::GetErrorString(error)})); + completed.set_exception(std::make_exception_ptr(std::runtime_error{error.Message() + "\n" + Napi::GetErrorString(error)})); } catch (...) { @@ -67,7 +69,7 @@ namespace ASSERT_NO_THROW(completion.get()); } - void InitializeTexture(Napi::Object engine, Napi::Value texture, uint32_t format, bool renderTarget, bool srgb = false, uint32_t samples = 1) + void InitializeTexture(Napi::Object engine, Napi::Value texture, double format, bool renderTarget, bool srgb = false, uint32_t samples = 1) { const auto env = engine.Env(); engine.Get("initializeTexture").As().Call(engine, { @@ -76,7 +78,7 @@ namespace Napi::Number::New(env, samples)}); } - void ExpectInitializationError(Napi::Object engine, Napi::Value texture, uint32_t format, bool renderTarget, bool srgb, const char* message, uint32_t samples = 1) + void ExpectInitializationError(Napi::Object engine, Napi::Value texture, double format, bool renderTarget, bool srgb, const char* message, uint32_t samples = 1) { try { @@ -85,7 +87,7 @@ namespace } catch (const Napi::Error& error) { - EXPECT_NE(Napi::GetErrorString(error).find(message), std::string::npos); + EXPECT_NE(error.Message().find(message), std::string::npos) << error.Message(); } } @@ -168,8 +170,14 @@ TEST(NativeEngineTextureFormats, RejectedInitializationPreservesExistingTexture) InitializeTexture(engine, value, bgfx::TextureFormat::RGBA8, true); auto* texture = value.As>().Get(); const auto originalHandle = texture->Handle(); - for (const uint32_t format : {static_cast(bgfx::TextureFormat::Count), UINT32_MAX}) + for (const double format : { + static_cast(bgfx::TextureFormat::Count), static_cast(UINT32_MAX), + 4294967296.0 + static_cast(bgfx::TextureFormat::RGBA8), -1.0, + static_cast(bgfx::TextureFormat::RGBA8) + 0.5, + std::numeric_limits::quiet_NaN(), std::numeric_limits::infinity(), + -std::numeric_limits::infinity()}) { + SCOPED_TRACE(format); ExpectInitializationError(engine, value, format, true, false, "Invalid texture format"); EXPECT_EQ(texture->Handle().idx, originalHandle.idx); EXPECT_EQ(texture->Format(), bgfx::TextureFormat::RGBA8); diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 70f7fc928..2ad14fef2 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1635,8 +1635,9 @@ namespace Babylon const uint16_t width = static_cast(info[1].As().Uint32Value()); const uint16_t height = static_cast(info[2].As().Uint32Value()); const bool hasMips = info[3].As(); - const uint32_t formatValue = info[4].As().Uint32Value(); - if (formatValue >= bgfx::TextureFormat::Count) + const double formatValue = info[4].As().DoubleValue(); + if (!std::isfinite(formatValue) || formatValue < 0 || formatValue >= static_cast(bgfx::TextureFormat::Count) || + std::floor(formatValue) != formatValue) { throw Napi::Error::New(info.Env(), "Invalid texture format"); } From d103cb534089f03d8e3b5dd7c3d832dcd6511731 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Fri, 11 Sep 2026 16:05:22 -0700 Subject: [PATCH 3/3] Include the rejected texture format in validation errors Report the original numeric value and the finite-integer enum range while preserving the existing Invalid texture format prefix. Add a regression for the NaN diagnostic alongside allocation-preservation coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp | 4 +++- Plugins/NativeEngine/Source/NativeEngine.cpp | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp index 34a5519d6..79029f37f 100644 --- a/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp +++ b/Apps/UnitTests/Source/Tests.NativeEngine.TextureFormats.cpp @@ -78,7 +78,7 @@ namespace Napi::Number::New(env, samples)}); } - void ExpectInitializationError(Napi::Object engine, Napi::Value texture, double format, bool renderTarget, bool srgb, const char* message, uint32_t samples = 1) + void ExpectInitializationError(Napi::Object engine, Napi::Value texture, double format, bool renderTarget, bool srgb, const std::string& message, uint32_t samples = 1) { try { @@ -170,6 +170,8 @@ TEST(NativeEngineTextureFormats, RejectedInitializationPreservesExistingTexture) InitializeTexture(engine, value, bgfx::TextureFormat::RGBA8, true); auto* texture = value.As>().Get(); const auto originalHandle = texture->Handle(); + ExpectInitializationError(engine, value, std::numeric_limits::quiet_NaN(), true, false, + "Invalid texture format NaN: expected a finite integer in [0, " + std::to_string(bgfx::TextureFormat::Count) + ")"); for (const double format : { static_cast(bgfx::TextureFormat::Count), static_cast(UINT32_MAX), 4294967296.0 + static_cast(bgfx::TextureFormat::RGBA8), -1.0, diff --git a/Plugins/NativeEngine/Source/NativeEngine.cpp b/Plugins/NativeEngine/Source/NativeEngine.cpp index 2ad14fef2..8088c47b9 100644 --- a/Plugins/NativeEngine/Source/NativeEngine.cpp +++ b/Plugins/NativeEngine/Source/NativeEngine.cpp @@ -1639,7 +1639,8 @@ namespace Babylon if (!std::isfinite(formatValue) || formatValue < 0 || formatValue >= static_cast(bgfx::TextureFormat::Count) || std::floor(formatValue) != formatValue) { - throw Napi::Error::New(info.Env(), "Invalid texture format"); + throw Napi::Error::New(info.Env(), "Invalid texture format " + info[4].ToString().Utf8Value() + + ": expected a finite integer in [0, " + std::to_string(bgfx::TextureFormat::Count) + ")"); } auto format = static_cast(formatValue); const bool renderTarget = info[5].As();