Auto-populate is2DArray/depth in wrapNativeTexture from native layer count#18535
Conversation
Babylon Native used to require consumers to thread a texture's layer count through their own bridge to detect when a wrapped native texture was a Texture2DArray, because `wrapNativeTexture` only read width and height from the engine binding. Defaulting `is2DArray` and `depth` to unset/0 caused downstream code that picks a sampler variant by `InternalTexture.is2DArray` to mismatch the bgfx SRV view dimension and hit undefined behavior on some D3D11 drivers. With the matching `getTextureLayerCount` binding on the Babylon Native side, `wrapNativeTexture` can populate `is2DArray` and `depth` itself, removing the host-side plumbing. Also tighten `updateWrappedNativeTexture` to validate that the new handle's layer count matches the wrapped texture's recorded layer count, matching the existing dimension check. Requires the matching getTextureLayerCount binding in Babylon Native; older Babylon Native builds throw on the missing method. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18535/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18535/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18535/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
Mark `INativeEngine.getTextureLayerCount` optional and feature-detect (`typeof === "function"`) before calling, so older Babylon Native builds that don't expose the binding keep working: - `wrapNativeTexture`: if the binding is absent, skip auto-populating `is2DArray` / `depth`. The wrapped InternalTexture stays at the defaults, matching pre-existing behavior. - `updateWrappedNativeTexture`: if the binding is absent, skip the layer-count validation. Dimensions are still validated. Removes the runtime dependency on BabylonJS/BabylonNative#1733, so this PR can land independently. Hosts running on an updated native engine get the auto-detect for free; hosts on older native engines see no change. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Optional method on the interface is either a function (truthy) or undefined (falsy); the typeof === 'function' guard adds no value over a plain truthy check. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves correctness when wrapping externally-owned native textures in the Native engine by introspecting the native resource’s array-layer count (when available) and using it to configure the resulting InternalTexture as a 2D array texture, preventing sampler/view-dimension mismatches.
Changes:
- Add optional
INativeEngine.getTextureLayerCount()to query a native texture’s array-layer count. - In
ThinNativeEngine.wrapNativeTexture, detect layer count and setInternalTexture.is2DArray/depthaccordingly. - In
ThinNativeEngine.updateWrappedNativeTexture, validate that a replacement native handle has the same layer count as the wrapped texture.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/dev/core/src/Engines/thinNativeEngine.pure.ts | Auto-detects 2D array textures via native layer count and validates layer-count stability when swapping wrapped handles. |
| packages/dev/core/src/Engines/Native/nativeInterfaces.ts | Extends the native engine interface with an optional getTextureLayerCount capability. |
|
cc @bghgary :-) |
Pull request was converted to draft
- wrapNativeTexture: set baseDepth alongside depth so a wrapped Texture2DArray reports its initial depth consistently with the rest of the engine (baseDepth = depth = layerCount). - updateWrappedNativeTexture: put the layer-count mismatch error on a single line to satisfy prettier (printWidth 180). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the surrounding INativeEngine members (getTextureWidth/Height etc.), which carry no doc comments; the optional `?` already conveys availability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18535/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18535/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18535/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
Exercises the cross-repo path end to end: getTextureLayerCount feeds Babylon.js wrapNativeTexture (BabylonJS/Babylon.js#18535), which sets is2DArray / depth / baseDepth when a wrapped texture presents more than one layer. Wraps three ExternalTextures and asserts the resulting InternalTexture: - whole array (arraySize 2, no layerIndex) -> is2DArray, depth=baseDepth=2 - single slice (arraySize 2, layerIndex 0) -> plain 2D (as Teams wraps an NV12 plane) - single layer (arraySize 1) -> plain 2D The single-slice case guards against regressing a single array slice into a 2D array. Skips cleanly when the native binding or the consuming Babylon.js change is absent so it stays green across the cross-repo landing order. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercises the cross-repo path end to end: getTextureLayerCount feeds Babylon.js wrapNativeTexture (BabylonJS/Babylon.js#18535), which sets is2DArray / depth / baseDepth when a wrapped texture presents more than one layer. Wraps three ExternalTextures and asserts the resulting InternalTexture: - whole array (arraySize 2, no layerIndex) -> is2DArray, depth=baseDepth=2 - single slice (arraySize 2, layerIndex 0) -> plain 2D (one slice of an NV12 array) - single layer (arraySize 1) -> plain 2D The single-slice case guards against regressing a single array slice into a 2D array. Skips cleanly when the native binding or the consuming Babylon.js change is absent so it stays green across the cross-repo landing order. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-texture-auto-array
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test ResultsBaseline: Latest · Candidate: Dev
🔘 Not Significant — p ≥ 0.05 (1)
|
wrapNativeTexturenow auto-populatesis2DArrayanddepthon the wrappedInternalTexturevia a newengine.getTextureLayerCount(...)binding, so hosts no longer have to thread the layer count through their own bridge to wrap aTexture2DArraycorrectly.updateWrappedNativeTexturegains a matching layer-count validation.The binding is optional on
INativeEngine; both call sites guard with a truthy check, so older Babylon Native builds without the binding (BabylonJS/BabylonNative#1733) keep their current behavior — this PR is a no-op until that lands.[Created by Copilot on behalf of @bghgary]