From 3f373fd54d2602ef45810ab8a4484231c1c80325 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 12 Jun 2026 08:50:36 -0700 Subject: [PATCH 1/2] Restore layerIndex parameter on deprecated ExternalTexture::AddToContextAsync PR #1646 deprecated AddToContextAsync in favor of the synchronous CreateForJavaScript, but the retained shim dropped the optional layerIndex parameter. That broke backwards compatibility for existing callers that pass a layer index (e.g. AddToContextAsync(env, 1)) -- they no longer compile. Restore the parameter on the deprecated shim and forward it to CreateForJavaScript, so both AddToContextAsync(env) and AddToContextAsync(env, layerIndex) compile and behave as before. The deprecation is retained to keep steering consumers toward CreateForJavaScript. Add a unit test (AddToContextAsyncWithLayerIndex) exercising the two-argument form against a 3-layer array texture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Source/Tests.ExternalTexture.cpp | 56 +++++++++++++++++++ .../Include/Babylon/Plugins/ExternalTexture.h | 3 +- .../Source/ExternalTexture_Shared.h | 4 +- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp index 2e699de8d..93e28cf49 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp @@ -166,3 +166,59 @@ TEST(ExternalTexture, AddToContextAsyncAndUpdate) device.FinishRenderingCurrentFrame(); #endif } + +TEST(ExternalTexture, AddToContextAsyncWithLayerIndex) +{ +#ifdef SKIP_EXTERNAL_TEXTURE_TESTS + GTEST_SKIP(); +#else + Babylon::Graphics::Device device{g_deviceConfig}; + + device.StartRenderingCurrentFrame(); + + // Array texture (3 layers) so a non-zero layer index is valid. + auto nativeTexture = Helpers::CreateTexture(device.GetPlatformInfo().Device, 256, 256, 3); + Babylon::Plugins::ExternalTexture externalTexture{nativeTexture}; + Helpers::DestroyTexture(nativeTexture); + + std::promise addToContext{}; + std::promise promiseResolved{}; + + Babylon::AppRuntime runtime{}; + runtime.Dispatch([&device, &addToContext, &promiseResolved, externalTexture](Napi::Env env) { + device.AddToJavaScript(env); + + Babylon::Polyfills::Console::Initialize(env, [](const char* message, auto) { + std::cout << message << std::endl; + }); + + Babylon::Polyfills::Window::Initialize(env); + + Babylon::Plugins::NativeEngine::Initialize(env); + + // Backwards-compat: the deprecated AddToContextAsync must still accept a layer + // index and forward it to CreateForJavaScript (views only that array slice). + auto jsPromise = externalTexture.AddToContextAsync(env, 1); + addToContext.set_value(); + + auto jsOnFulfilled = Napi::Function::New(env, [&promiseResolved](const Napi::CallbackInfo& info) { + promiseResolved.set_value(); + }); + + auto jsOnRejected = Napi::Function::New(env, [&promiseResolved](const Napi::CallbackInfo& info) { + promiseResolved.set_exception(std::make_exception_ptr(info[0].As())); + }); + + jsPromise.Get("then").As().Call(jsPromise, {jsOnFulfilled, jsOnRejected}); + }); + + // Wait for AddToContextAsync to be called. + addToContext.get_future().wait(); + + // Close the frame in which the deprecated shim's synchronous CreateForJavaScript ran. + device.FinishRenderingCurrentFrame(); + + // Wait for promise to resolve. + promiseResolved.get_future().wait(); +#endif +} diff --git a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h index b87636525..1291ee363 100644 --- a/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h +++ b/Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h @@ -40,8 +40,9 @@ namespace Babylon::Plugins // Deprecated: use CreateForJavaScript instead. Retained as a shim for existing consumers. // Returns a Promise that is already resolved with the value from CreateForJavaScript. + // If layerIndex is set, the JavaScript texture views only that array layer (single-slice). [[deprecated("Use CreateForJavaScript instead.")]] - Napi::Promise AddToContextAsync(Napi::Env) const; + Napi::Promise AddToContextAsync(Napi::Env, std::optional layerIndex = {}) const; // Updates to a new texture. If layerIndex is set, views only that array layer (single-slice). void Update(Graphics::TextureT, std::optional = {}, std::optional layerIndex = {}); diff --git a/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h b/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h index 290e62492..637f95cd8 100644 --- a/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h +++ b/Plugins/ExternalTexture/Source/ExternalTexture_Shared.h @@ -136,10 +136,10 @@ namespace Babylon::Plugins m_impl->Update(ptr, overrideFormat, layerIndex); } - Napi::Promise ExternalTexture::AddToContextAsync(Napi::Env env) const + Napi::Promise ExternalTexture::AddToContextAsync(Napi::Env env, std::optional layerIndex) const { auto deferred = Napi::Promise::Deferred::New(env); - deferred.Resolve(CreateForJavaScript(env)); + deferred.Resolve(CreateForJavaScript(env, layerIndex)); return deferred.Promise(); } } From 4ed9fab64fc887744116c91280a580af97b23576 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 12 Jun 2026 09:05:19 -0700 Subject: [PATCH 2/2] Fail the layerIndex test on promise rejection Per review feedback: std::future::wait() returns even when the promise rejected (set_exception also makes the future ready), so the test could pass on rejection. Use EXPECT_NO_THROW(...get()) which rethrows on rejection so it fails the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Apps/UnitTests/Source/Tests.ExternalTexture.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp index 93e28cf49..55083d209 100644 --- a/Apps/UnitTests/Source/Tests.ExternalTexture.cpp +++ b/Apps/UnitTests/Source/Tests.ExternalTexture.cpp @@ -218,7 +218,7 @@ TEST(ExternalTexture, AddToContextAsyncWithLayerIndex) // Close the frame in which the deprecated shim's synchronous CreateForJavaScript ran. device.FinishRenderingCurrentFrame(); - // Wait for promise to resolve. - promiseResolved.get_future().wait(); + // get() (not wait()) so a rejected promise rethrows and fails the test. + EXPECT_NO_THROW(promiseResolved.get_future().get()); #endif }