Restore layerIndex parameter on deprecated ExternalTexture::AddToContextAsync#1757
Merged
bghgary merged 2 commits intoJun 12, 2026
Merged
Conversation
…extAsync PR BabylonJS#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>
Contributor
There was a problem hiding this comment.
Pull request overview
Restores backward compatibility for Babylon::Plugins::ExternalTexture::AddToContextAsync by reintroducing the optional layerIndex parameter on the deprecated shim and forwarding it to CreateForJavaScript, plus adds a unit test that exercises the two-argument call form on an array texture.
Changes:
- Re-add
std::optional<uint16_t> layerIndexto deprecatedAddToContextAsyncand forward it toCreateForJavaScript. - Update the public header docs to describe layer-slice behavior for the shim.
- Add a unit test covering
AddToContextAsync(env, layerIndex)on a 3-layer array texture.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Plugins/ExternalTexture/Source/ExternalTexture_Shared.h | Restores the deprecated shim’s layerIndex parameter and forwards it to CreateForJavaScript. |
| Plugins/ExternalTexture/Include/Babylon/Plugins/ExternalTexture.h | Restores the public API signature (with default arg) and documents layer-slice behavior. |
| Apps/UnitTests/Source/Tests.ExternalTexture.cpp | Adds a new unit test for the two-argument AddToContextAsync call. |
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>
bkaradzic-microsoft
approved these changes
Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Created by Copilot on behalf of @bghgary]
#1646 kept
AddToContextAsyncas a deprecated shim but dropped its optionallayerIndexparameter:Napi::Promise AddToContextAsync(Napi::Env, std::optional<uint16_t> layerIndex = {}) const;Napi::Promise AddToContextAsync(Napi::Env) const;So callers passing a layer index (e.g.
AddToContextAsync(env, 1)) no longer compile. This restores the parameter and forwards it toCreateForJavaScript(keeping[[deprecated]]), plus a test (AddToContextAsyncWithLayerIndex) covering the two-argument form on a 3-layer array texture.