Graphics/Canvas: fix bgfx view exhaustion (shared nanovg view + guarded mid-frame flush)

[bkaradzic-microsoft]

Second half of the Canvas/bgfx work whose first half landed as #1800. Independent of it; this one is about bgfx view exhaustion, not pixel readback.

The problem

nanovg_babylon acquired a fresh bgfx view for every draw call. bgfx caps view ids, so a scene with enough canvas draws runs out and dies with Error: Too many views.

The changes

1. Share a view across non-filtered draws. Only filtered draws need a private view (the filter stack renders into it). Non-filtered draws now acquire lazily and reuse one.

2. Add a mid-frame view flush. Sharing raises the ceiling but does not remove it, so views can now be recycled mid-frame.

The deadlock this could have introduced

The flush handshake parks the requesting JS thread and relies on the render thread being parked in FinishRenderingCurrentFrame to service it. That only holds while a FrameCompletionScope is active — the normal requestAnimationFrame path.

Snippets that drive frames manually (setInterval + engine.beginFrame/scene.render/engine.endFrame) acquire no scope, so the handshake would never be serviced and the snippet would hang. FlushViewsIfNeeded skips the flush when m_pendingFrameScopes == 0; the AcquireNewViewId cap remains the backstop, so those snippets behave exactly as they do today.

The guard is folded into commit 2 rather than added as a follow-up, so no commit in this branch's history contains the deadlocking version.

Evidence

Re-enables "Vertex Pulling - Normals UVs Colors Tangents" (idx 665), excluded as "Test crashes or hangs on Babylon Native".

This is the only config.json change in the PR — a single hunk dropping that one test's excludeFromAutomaticTesting/reason pair. The un-exclusion is intentional and is the evidence for the fix.

Negative control: with the nanovg commit reverted, that test fails with Error: Too many views. With it applied, it passes (2.224% diff, allowed 2.5%).

Full Win32 D3D11 validation suite: 301/301, 0 failures.

On the other five un-exclusions

The shotgun branch this comes from also un-excluded idx 185, 253, 653, 654 and 659. I tested each individually and they are not earned by this PR, so they are not included:

test	result on this branch alone
185 Particle Helper	fails, 12.117% (deterministic over 3 runs)
253 MultiSample render targets	fails, Error: Failed to create frame buffer — needs the MRT work
653 Particles - Billboard False	fails, 12.302%
654 Particles - Multiply Blend	fails
659 Particles - Helper - Fire	fails

They passed on the shotgun branch only because other changes were present alongside. They will come back with the PR that actually earns them.

Worth noting for anyone reading suite output: the runner aborts on the first failure, so an un-exclusion early in the config can silently mask everything after it. Un-excluding all six produced a green-looking ran=127 passed=126 failed=1 that had simply stopped at idx 185 and never reached the other five.

---

Update: mid-frame flush invalidates cached view ids (d298801)

Review raised that PerformMidFrameViewFlush resets m_nextViewId while a logical frame is in flight, which strands view ids cached across draw calls. That was correct, and broader than the canvas blit alone — FrameBuffer::m_viewId is retained by SetBgfxViewPortAndScissor and only cleared by Bind() at frame start.

Added a view-id generation counter bumped on each flush. FrameBuffer re-acquires on mismatch; CopyTexture falls back to PeekNextViewId() (safe, because a flush has already submitted the canvas draws in a prior bgfx frame).

The flush never fires in the suite as shipped (256 views, margin 16 — instrumented count was 0), so it was verified by temporarily forcing a flush every 8 views:

build	result
with the fix	417 flushes, 318 stale ids re-acquired, 301/301 pass
generation check neutralized	Native Canvas fails at 21.9% pixel divergence

[Copilot]

🟡 Not ready to approve

The mid-frame view-id reset can break existing view-ordering guarantees used for the canvas→texture blit path, potentially reintroducing one-frame latency/incorrect sampling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR addresses bgfx view-id exhaustion triggered by Canvas/nanovg rendering by (1) reusing a shared bgfx view for non-filtered draws and (2) introducing a mid-frame “view flush” mechanism that advances a bgfx frame and resets the view counter so rendering can continue without hitting the hard max-views cap.

Changes:

• Add filter-stack introspection (HasFilters) and use it in nanovg to reuse a single canvas view for consecutive non-filtered draws.
• Add DeviceImpl::FlushViewsIfNeeded() plus a cross-thread handshake to perform a mid-frame bgfx frame() and reset the view-id counter.
• Re-enable a previously excluded validation test in Apps/Playground/Scripts/config.json.

File summaries

File	Description
Polyfills/Canvas/Source/nanovg/nanovg_filterstack.h	Adds HasFilters() to detect whether intermediate filtered rendering is required.
Polyfills/Canvas/Source/nanovg/nanovg_babylon.cpp	Reuses the canvas framebuffer view for non-filtered draws; refreshes view only when needed for ordering.
Plugins/NativeEngine/Source/NativeEngine.cpp	Calls DeviceContext::FlushViewsIfNeeded() at encoder acquisition boundaries.
Core/Graphics/Source/DeviceImpl.h	Declares mid-frame view flush API and synchronization fields.
Core/Graphics/Source/DeviceImpl.cpp	Implements the mid-frame flush handshake and performs bgfx::frame() + view-id reset.
Core/Graphics/Source/DeviceContext.cpp	Plumbs FlushViewsIfNeeded() to DeviceImpl.
Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h	Exposes FlushViewsIfNeeded() in the public DeviceContext contract.
Apps/Playground/Scripts/config.json	Removes an automatic-test exclusion for “Vertex Pulling - Normals UVs Colors Tangents”.

Review details

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[bkaradzic-microsoft]

CI failures: root-caused and fixed

The 9 CI failures were pre-existing on this branch (they also occur on 2e72406f, before the view-id commit). All of them are the same abort, on both Win32 D3D11 and Linux OpenGL, while running the test this PR un-excludes:

BX_ASSERT ... Failed to allocate transient vertex buffer (requested 3407872, available 391936)

Why it passed locally but aborted in CI

Not a data difference. I instrumented nvgRenderFlush locally and got byte-identical numbers to CI:

NVGDIAG nverts=3407872 avail=391936

The difference is only whether bgfx's asserts are compiled in. nvgRenderFlush was written to degrade gracefully on overflow, but the check ran after the allocation:

bgfx::allocTransientVertexBuffer(&gl->tvb, gl->nverts, s_nvgLayout);
int allocated = gl->tvb.size/gl->tvb.stride;
if (allocated < gl->nverts) { gl->nverts = allocated; ... }   // unreachable if bgfx asserts

bgfx asserts inside allocTransientVertexBuffer, so with asserts enabled the process aborts and the clamp never runs. Locally, with asserts off, bgfx returned a truncated buffer, the post-hoc clamp took effect, and the test passed at 2.224% (tolerance 2.5%). So the truncation path was already being exercised locally all along — CI just never got to it.

Fix

db653645 clamps before allocating, which is what the assert message itself directs:

const uint32_t availVerts = bgfx::getAvailTransientVertexBuffer(uint32_t(gl->nverts), s_nvgLayout);
if (availVerts < uint32_t(gl->nverts)) { BX_WARN(...); gl->nverts = int(availVerts); }
if (gl->nverts > 0) { /* alloc, copy, draw */ }

The gl->nverts > 0 guard also now skips the draw calls, since gl->tvb would otherwise still describe a previous flush's allocation. This mirrors the getAvailTransientVertexBuffer guard already used elsewhere in this file.

Both configurations now behave identically. Local suite: ran=301 passed=301 failed=0.

Note for reviewers

This makes the abort impossible, but the underlying oddity stands: a canvas asking for 3.4M vertices (~54 MB) in one flush is a lot, and it only fits because the dropped geometry happens to be near-invisible. I deliberately did not raise Init::limits.transientVbSize to ~64 MB to accommodate it, since that cost would be paid by every app on every platform for one pathological playground. If this test proves unstable across CI backends I will drop the un-exclusion and track the canvas geometry volume separately, keeping the (independently correct) robustness fix here.

[bkaradzic-microsoft]

Root cause of the CI failures: a D3D11 debug-layer log flood, not a test failure

The six Win32 D3D11 jobs were reported as failing, but they were CANCELLED, not FAILED — every one ran for exactly 65 minutes (job timeout-minutes: 60 plus setup), so they were killed by the timeout.

The job log for Win32_x64_D3D11 is 2.19 GB across 11,057,446 lines. Of those, 11,015,198 are a single repeated line:

renderer_d3d11.cpp (3805): BGFX D3D11 message <N> is too large (4096 bytes) to dump.

The flood starts at line 42,249 and never stops. The last real log line before it is:

Running Vertex Pulling - Normals UVs Colors Tangents

That test (#664OF3#6, idx 665) is the one this PR un-excluded — its exclusion reason on master is literally "Test crashes or hangs on Babylon Native". The job wasn't computing for an hour; it was formatting and writing debug-layer messages.

Why my previous commit caused it

The earlier fix clamped the nanovg vertex count to getAvailTransientVertexBuffer before allocating, and rendered "what fits". That was wrong: the draw calls index the transient buffer using offsets recorded while the geometry was built (call->vertexOffset, paths[i].strokeOffset, …). Truncating the allocation without dropping the affected draws means every draw past the truncation point reads outside the buffer.

That explains the platform split exactly:

• Linux OpenGL — passed, because it has no equivalent per-draw validation, so the out-of-bounds draws were silent.
• Win32 D3D11 — the debug layer emits an oversized diagnostic per draw call. RelWithDebInfo (what CI builds) has it enabled; my earlier local runs were Release, which is why I never saw it.

So the truncation didn't just render the wrong thing — it generated enough log volume to consume the entire job budget.

The fix

A partial allocation cannot be rendered correctly, so this now skips the whole flush instead of truncating it. The frame is missing that canvas content, but no out-of-bounds draw is issued and the process doesn't abort — which is still strictly better than the original behaviour (a bgfx assert/abort in an assert-enabled build).

I also restored the config.json exclusion for idx 665, so this PR no longer changes config.json at all — it's now purely the view-management code fixes.

Worth recording: idx 665 does pass locally under the exact CI configuration that was hanging (Win32 D3D11, RelWithDebInfo), at 2.248% vs. the 2.5% threshold, in 3 seconds. I've deliberately left it excluded anyway, because it only passes because the oversized canvas flush is skipped — i.e. it would be passing for the wrong reason, and it would lock in a reference expectation that the canvas is absent. Re-enabling it should wait for a proper chunked-flush fix that can render a canvas exceeding the transient budget. I'd rather not bank a green tick on missing content.

Verification

Rebased onto current master (2b4e16d8), so this also picks up #1801 and #1754, which it was missing.

Built RelWithDebInfo (matching CI) and ran the full suite on Win32 D3D11:

Run complete. ran=300 passed=300 failed=0 missingRef=0 skipped=420

	before	after
idx 665 alone	60 min → timeout	6.2 s
log size	2.19 GB	0.2 MB
is too large … to dump lines	11,015,198	0

(ran=300 rather than 301 is expected — idx 665 is excluded again.)

Note for the workflow, independent of this PR

A single misbehaving test can currently burn a full 60-minute job and produce a multi-gigabyte log. bgfx's D3D11 debug-message pump has no rate limit, so any out-of-bounds draw in a hot loop reproduces this. Capping repeated identical debug messages would turn a 60-minute timeout into a fast, readable failure — happy to open a separate issue if that's useful.