[DO-NOT-MERGE] Partner integration test: #1652 + #1646#1674
Draft
bghgary wants to merge 49 commits intoBabylonJS:masterfrom
Draft
[DO-NOT-MERGE] Partner integration test: #1652 + #1646#1674bghgary wants to merge 49 commits intoBabylonJS:masterfrom
bghgary wants to merge 49 commits intoBabylonJS:masterfrom
Conversation
- Add Tests.ExternalTexture.Render.cpp: end-to-end test that renders a texture array through a ShaderMaterial to an external render target, verifying each slice (red, green, blue) via pixel readback. - Add tests.externalTexture.render.ts: JS test with sampler2DArray shader. - Add RenderDoc.h/cpp to UnitTests for optional GPU capture support. - Add Utils helpers: CreateTestTextureArrayWithData, CreateRenderTargetTexture, ReadBackRenderTarget, DestroyRenderTargetTexture (D3D11, Metal, stubs for D3D12/OpenGL). - Fix ExternalTexture_Shared.h: pass m_impl->NumLayers() instead of hardcoded 1 in Attach(), preserving texture array metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove layerIndex parameter from Impl::Update declaration to match the updated signature in ExternalTexture_Shared.h. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrate HeadlessScreenshotApp, StyleTransferApp, and PrecompiledShaderTest from AddToContextAsync (promise-based) to CreateForJavaScript (synchronous). This removes the extra frame pump and promise callbacks that were previously required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…llers - Move RenderDoc.h/cpp to D3D11-only CMake block with HAS_RENDERDOC define - Guard RenderDoc calls with HAS_RENDERDOC instead of WIN32 - Update StyleTransferApp and PrecompiledShaderTest to use CreateForJavaScript Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On OpenGL, TextureT is unsigned int (not a pointer), so reinterpret_cast fails. Add NativeHandleToUintPtr helper using if constexpr to handle both pointer types (D3D11/Metal/D3D12) and integer types (OpenGL). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- RenderDoc.h/cpp now accept void* device instead of ID3D11Device* - Move RenderDoc to WIN32 block (not D3D11-only) since it works with any API - Fix OpenGL build: use NativeHandleToUintPtr helper for TextureT cast - Add Linux support (dlopen librenderdoc.so) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move DestroyTestTexture after FinishRenderingCurrentFrame so bgfx::frame() processes the texture creation command before the native resource is released. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ensure the JS startup dispatch completes before calling deviceUpdate.Finish() and FinishRenderingCurrentFrame(). This guarantees that bgfx::frame() processes the CreateForJavaScript texture creation command, making the texture available for subsequent render frames. The old async API had an implicit sync point (addToContext.wait) that the new sync API lost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eation - Rename CreateForJavaScriptWithTextureArray to CreateForJavaScript and use arraySize=1 since texture array rendering is covered by RenderTextureArray. The old test crashed on CI (STATUS_BREAKPOINT in bgfx when creating texture arrays via encoder on WARP). - Revert two-step create+override approach back to single createTexture2D call with _external parameter (overrideInternal from JS thread doesn't work since the D3D11 resource isn't created until bgfx::frame). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CreateForJavaScript already exists in Tests.ExternalTexture.cpp, causing a linker duplicate symbol error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… test) The D3D11-specific CreateForJavaScript test crashed on CI due to bgfx assertions when calling createTexture2D with _external on the encoder thread. The cross-platform CreateForJavaScript test in Tests.ExternalTexture.cpp already covers this functionality. The texture array rendering is covered by RenderTextureArray. Also revert app startup ordering to Finish->Wait (matching the pattern used by HeadlessScreenshotApp on master). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The bgfx callback's fatal() handler was silently calling debugBreak() on DebugCheck assertions with no output, making CI crashes impossible to diagnose. Now logs the file, line, error code and message to stderr before breaking, so the assertion details appear in CI logs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add DISM/d3dconfig step to CI to enable D3D debug layer, which will provide detailed D3D11 validation messages for the CreateShaderResourceView E_INVALIDARG failure. Kept the _external createTexture2D path (reverted the AfterRenderScheduler approach) so we can see the actual D3D debug output that explains the SRV mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The bgfx _external texture path triggers E_INVALIDARG in CreateShaderResourceView on CI's WARP D3D11 driver. The overrideInternal alternative doesn't support full array textures (hardcodes ArraySize=1). Since the _external path works on real GPUs, skip the render test on CI via BABYLON_NATIVE_SKIP_RENDER_TESTS and keep the direct _external path. Also adds D3D debug layer enablement to CI for future diagnostics, and logs bgfx fatal errors to stderr before crashing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Switch from _external parameter (crashes on WARP) to create+overrideInternal two-step approach. The overrideInternal path is compatible with WARP but sets ArraySize=1 in the SRV, so the RenderTextureArray test (which needs full array access) is skipped on CI. The render test works on real GPUs where the _external path succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The overrideInternal call fires on AfterRenderScheduler after the first bgfx::frame(). An additional frame pump ensures the native texture backing is applied before the scene render. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove deleted Tests.ExternalTexture.D3D11.cpp from Install/Test/CMakeLists.txt - Add extra frame pump after CreateForJavaScript in HeadlessScreenshotApp and StyleTransferApp so overrideInternal has time to apply the native texture backing before the first render. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…er FrameCompletionScope acquisition.
…ompletionScope instead of deferred member release.
This reverts commit 0d315bb.
…cquired FrameCompletionScope in SubmitCommands.
…ck on FrameCompletionScope.
…enderScheduler fires one frame late when called outside RAF.
…te leaking into nanovg rendering.
… getFrameBufferData outside RAF.
…nder-test # Conflicts: # .github/jobs/win32.yml
…erResourceView The createTexture2D _external path now works on WARP after the bgfx update in master. Drop the placeholder + AfterRenderScheduler + overrideInternal two-step dance and the extra frame pump that existed to apply it. This also restores full array-slice SRV so RenderTextureArray no longer needs a sanitizer-only skip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove 'pump an extra frame' block from StyleTransferApp and HeadlessScreenshotApp (no longer needed after the _external revert). - Update ExternalTexture Readme to match the one-call createTexture2D path and direct-construction consumer pattern. - Drop unused DispatchSync declaration from UnitTests/Utils.h. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SKIP_RENDER_TESTS flag was originally added because WARP could not handle the _external path in bgfx. bgfx PR BabylonJS#1669 fixed CreateShaderResourceView for the _external parameter, so the render test should pass on WARP now. Let CI verify. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Reword BABYLON_NATIVE_SKIP_RENDER_TESTS option description. WARP is available on Win32, so 'no real GPU' is inaccurate; the real reason to skip is a missing per-backend test harness. - Add braces around if constexpr / else branches in NativeHandleToUintPtr per coding style. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- RenderDoc.cpp: drop absolute Windows path; use plain #include "renderdoc_app.h" - Tests.ExternalTexture.Render.cpp: handle renderSlice rejection and add wait timeout to prevent test hangs - Tests.ExternalTexture.cpp: add ExternalTexture.Update test (Update() path was dropped in the refactor) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore AddToContextAsync as a deprecated shim around CreateForJavaScript for source compat with existing consumers - Tighten render test: color tolerance 25 -> 2; require exact 100%% match (was 90%%) - Drop BABYLON_NATIVE_SKIP_RENDER_TESTS option; gate SKIP_RENDER_TESTS directly on GRAPHICS_API STREQUAL "D3D12" Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # Apps/HeadlessScreenshotApp/Win32/App.cpp # Apps/PrecompiledShaderTest/Source/App.cpp # Apps/StyleTransferApp/Win32/App.cpp
bghgary
force-pushed
the
partner-test-1652-1646
branch
from
d42b95a to
4c79b1d
Compare
Matches the established pattern from ExternalTexture.AddToContextAsyncAndUpdate on master: wait for JS work to complete while the frame is still open, then Finish. The new CreateForJavaScript and RenderTextureArray tests in this PR had accidentally dropped this pattern, which works on master but deadlocks under the reworked threading model in BabylonJS#1652 (Finish closes the frame gate while JS is still acquiring FrameCompletionScopes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
2 participants
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.
Test vehicle for partner integration: combines #1652 + #1646. This PR exists solely to run CI and give partners a single branch to consume. Do not merge — the two source PRs land independently.
Contents
origin/master@ d63e9fborigin/rework-thread-model(Reworked threading model. #1652: Reworked threading model)bghgary/external-texture-render-test(Add synchronous ExternalTexture.CreateForJavaScript and deprecate AddToContextAsync #1646: Synchronous ExternalTexture API)Conflict resolution
3 apps (
HeadlessScreenshotApp,PrecompiledShaderTest,StyleTransferApp) conflicted where #1652 added a "reopen the gate so JS can continue running" block in the same region #1646 removed the old extra-frame-pump pattern. Kept #1652's new block in all three.Testing
ExternalTexture.*tests: hung for >5 min and were killed; root cause not yet investigated (could be local environment or a real threading/ExternalTexture interaction). Flagged for partner testing awareness.