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

Apps/Playground/Scripts/config.json

@@ -4434,8 +4434,6 @@
         {
             "title": "Vertex Pulling - Normals UVs Colors Tangents",
             "playgroundId": "#664OF3#6",
-            "excludeFromAutomaticTesting": true,
-            "reason": "Test crashes or hangs on Babylon Native",
             "referenceImage": "vertexPullingNormalsUVsColors.png"
         },
         {

Core/Graphics/InternalInclude/Babylon/Graphics/DeviceContext.h

@@ -120,6 +120,15 @@ namespace Babylon::Graphics
         bgfx::ViewId AcquireNewViewId();
         bgfx::ViewId PeekNextViewId() const;
 
+        // Bumped whenever a mid-frame flush resets the view counter. Cache this alongside any
+        // view id that is retained across draw calls and re-acquire when it changes.
+        uint32_t ViewIdGeneration() const;
+
+        // If the current frame is close to exhausting bgfx views, flush accumulated
+        // views (cross-thread bgfx::frame + view-counter reset) so rendering can
+        // continue within the same logical frame. Call at draw/clear op boundaries.
+        void FlushViewsIfNeeded();
+
         // TODO: find a different way to get the texture info for frame capture
         void AddTexture(bgfx::TextureHandle handle, uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format);
         void RemoveTexture(bgfx::TextureHandle handle);

Core/Graphics/InternalInclude/Babylon/Graphics/FrameBuffer.h

@@ -62,6 +62,11 @@ namespace Babylon::Graphics
 
         std::optional<bgfx::ViewId> m_viewId{};
 
+        // Generation that m_viewId was acquired in. A mid-frame view flush resets the device's
+        // view counter, which makes a retained id sort after freshly acquired ones; comparing
+        // against DeviceContext::ViewIdGeneration() detects that and forces a re-acquire.
+        uint32_t m_viewIdGeneration{0};
+
         Rect m_bgfxViewPort{0.0f, 0.0f, 1.0f, 1.0f};
         Rect m_desiredViewPort{0.0f, 0.0f, 1.0f, 1.0f};
 

Core/Graphics/InternalInclude/Babylon/Graphics/Texture.h

@@ -45,9 +45,17 @@ namespace Babylon::Graphics
         void ViewNumLayers(uint16_t);
 
         // View id reserved (by the Canvas polyfill) for the canvas->texture blit that fills
-        // this texture. UINT16_MAX means "unset"; consumers fall back to a freshly peeked view.
+        // this texture, together with the view-id generation it was reserved in. UINT16_MAX
+        // means "unset"; a generation mismatch means a mid-frame view flush has since reset the
+        // view counter and the reservation no longer orders before later views. In both cases
+        // consumers fall back to a freshly peeked view.
         bgfx::ViewId BlitViewId() const { return m_blitViewId; }
-        void BlitViewId(bgfx::ViewId viewId) { m_blitViewId = viewId; }
+        uint32_t BlitViewIdGeneration() const { return m_blitViewIdGeneration; }
+        void BlitViewId(bgfx::ViewId viewId, uint32_t generation)
+        {
+            m_blitViewId = viewId;
+            m_blitViewIdGeneration = generation;
+        }
 
     private:
         bgfx::TextureHandle m_handle{bgfx::kInvalidHandle};
@@ -62,6 +70,7 @@ namespace Babylon::Graphics
         uint16_t m_viewFirstLayer{0};
         uint16_t m_viewNumLayers{0};
         bgfx::ViewId m_blitViewId{UINT16_MAX};
+        uint32_t m_blitViewIdGeneration{0};
         uintptr_t m_deviceID;
         DeviceContext& m_deviceContext;
     };

Core/Graphics/Source/DeviceContext.cpp

@@ -137,6 +137,16 @@ namespace Babylon::Graphics
         return m_graphicsImpl.PeekNextViewId();
     }
 
+    uint32_t DeviceContext::ViewIdGeneration() const
+    {
+        return m_graphicsImpl.ViewIdGeneration();
+    }
+
+    void DeviceContext::FlushViewsIfNeeded()
+    {
+        m_graphicsImpl.FlushViewsIfNeeded();
+    }
+
     void DeviceContext::AddTexture(bgfx::TextureHandle handle, uint16_t width, uint16_t height, bool hasMips, uint16_t numLayers, bgfx::TextureFormat::Enum format)
     {
         std::scoped_lock lock{m_textureHandleToInfoMutex};

Core/Graphics/Source/DeviceImpl.cpp

@@ -331,9 +331,28 @@ namespace Babylon::Graphics
         // Close the gate: wait until JS thread has released all FrameCompletionScopes
         // (meaning all encoder work for this frame is done), then block new acquisitions.
         // After this point, no bgfx encoder calls can be in flight on the JS thread.
+        //
+        // While waiting, also service any mid-frame view-flush requests from the JS
+        // thread (see FlushViewsIfNeeded): the JS thread parks itself and we advance a
+        // non-presenting bgfx frame here on the render thread to reset the view counter,
+        // then hand a fresh encoder back so rendering continues within the same logical
+        // frame.
         {
             std::unique_lock lock{m_frameSyncMutex};
-            m_frameSyncCV.wait(lock, [this] { return m_pendingFrameScopes == 0; });
+            while (true)
+            {
+                m_frameSyncCV.wait(lock, [this] { return m_pendingFrameScopes == 0 || m_flushRequested; });
+
+                if (m_flushRequested)
+                {
+                    PerformMidFrameViewFlush();
+                    m_flushRequested = false;
+                    m_flushCompleteCV.notify_all();
+                    continue;
+                }
+
+                break;
+            }
             m_frameBlocked = true;
         }
 
@@ -463,17 +482,139 @@ namespace Babylon::Graphics
 
     bgfx::ViewId DeviceImpl::AcquireNewViewId()
     {
-        bgfx::ViewId viewId = m_nextViewId.fetch_add(1);
-        if (viewId >= bgfx::getCaps()->limits.maxViews)
+        // Saturating increment. A plain fetch_add that is undone on the throw path would be
+        // two separate atomic operations, so a concurrent PeekNextViewId could observe an
+        // out-of-range value in between, and repeated failed acquisitions could drive the
+        // counter past the cap. This loop fuses the "check the cap" and "take the id" steps,
+        // clamping at maxViews, so the counter is never observable above the cap no matter
+        // how many acquisitions fail.
+        const uint32_t maxViews = bgfx::getCaps()->limits.maxViews;
+
+        uint32_t viewId = m_nextViewId.load(std::memory_order_relaxed);
+        while (viewId < maxViews && !m_nextViewId.compare_exchange_weak(viewId, viewId + 1))
+        {
+        }
+
+        if (viewId >= maxViews)
         {
             throw std::runtime_error{"Too many views"};
         }
-        return viewId;
+
+        return static_cast<bgfx::ViewId>(viewId);
     }
 
     bgfx::ViewId DeviceImpl::PeekNextViewId() const
     {
-        return m_nextViewId.load();
+        // Saturated at maxViews by AcquireNewViewId, so this always narrows safely.
+        return static_cast<bgfx::ViewId>(m_nextViewId.load());
+    }
+
+    uint32_t DeviceImpl::ViewIdGeneration() const
+    {
+        return m_viewIdGeneration.load();
+    }
+
+    void DeviceImpl::FlushViewsIfNeeded()
+    {
+        // Reserve headroom below the hard cap: a single draw/clear operation can
+        // acquire a couple of views before the next flush check, and one view
+        // (maxViews - 1) is reserved for readback blits.
+        constexpr bgfx::ViewId kViewFlushMargin = 16;
+
+        // Maximum mid-frame flushes allowed in one logical frame. Measured: no test in the
+        // validation suite needs any at the real 256-view budget, and the heaviest content
+        // found so far (the excluded "Nested BBG", which renders in a setInterval) peaks at
+        // 5. 64 leaves generous headroom for legitimately heavy content while bounding a
+        // mechanism that is otherwise unlimited: each flush is a blocking round-trip to the
+        // render thread, so an unbounded number of them would degrade into an apparent hang
+        // rather than an error.
+        constexpr uint32_t kMaxMidFrameViewFlushes = 64;
+
+        const bgfx::ViewId maxViews = static_cast<bgfx::ViewId>(bgfx::getCaps()->limits.maxViews);
+        if (maxViews <= kViewFlushMargin)
+        {
+            return;
+        }
+
+        if (m_nextViewId.load() < static_cast<uint32_t>(maxViews - kViewFlushMargin))
+        {
+            return;
+        }
+
+        // Bound the rescue. Each flush is a blocking round-trip to the render thread, so
+        // content that needs an unbounded number of them (e.g. a snippet that renders in a
+        // setInterval without ever letting the frame present) would appear to hang rather
+        // than fail. Past the budget, stop flushing and let AcquireNewViewId throw
+        // "Too many views" — the pre-existing behaviour, and a far better diagnostic than a
+        // process that makes progress too slowly to ever finish.
+        if (m_midFrameFlushCount.load() >= kMaxMidFrameViewFlushes)
+        {
+            return;
+        }
+
+        // The flush advances a bgfx frame, which must happen on the render (bgfx API)
+        // thread. This method is only expected to be called from the JS thread while
+        // the render thread is parked in FinishRenderingCurrentFrame. If we're on the
+        // render thread (or affinity is unset), there is nothing safe to do here.
+        if (m_renderThreadAffinity.check())
+        {
+            return;
+        }
+
+        std::unique_lock lock{m_frameSyncMutex};
+
+        // The mid-frame flush advances a bgfx frame on the render thread, which
+        // can only be serviced while the render thread is parked in
+        // FinishRenderingCurrentFrame waiting for frame scopes to drain. That is
+        // only guaranteed while at least one FrameCompletionScope is active (the
+        // normal requestAnimationFrame render path). When a snippet drives frames
+        // manually (e.g. setInterval + engine.beginFrame/scene.render/
+        // engine.endFrame) there is no frame scope, the render thread is not
+        // parked to service the request, and parking the JS thread on
+        // m_flushCompleteCV would deadlock. Skip the flush in that case; the hard
+        // cap in AcquireNewViewId remains as a backstop. Likewise skip if the gate
+        // is currently closed (bgfx::frame() in progress).
+        if (m_frameBlocked || m_pendingFrameScopes == 0)
+        {
+            return;
+        }
+
+        m_flushRequested = true;
+        m_frameSyncCV.notify_all();
+        m_flushCompleteCV.wait(lock, [this] { return !m_flushRequested; });
+    }
+
+    // Called on the render thread from FinishRenderingCurrentFrame while holding
+    // m_frameSyncMutex, with the requesting JS thread parked in FlushViewsIfNeeded
+    // (so the frame encoder is idle). End the current encoder, advance a non-presenting
+    // bgfx frame to submit the accumulated views and reset the view counter, then begin
+    // a fresh encoder for the remainder of the logical frame.
+    void DeviceImpl::PerformMidFrameViewFlush()
+    {
+        ASSERT_THREAD_AFFINITY(m_renderThreadAffinity);
+
+        if (m_frameEncoder)
+        {
+            bgfx::end(m_frameEncoder);
+            m_frameEncoder = nullptr;
+        }
+
+        // BGFX_FRAME_FLUSH executes all queued rendering commands and resets bgfx's per-frame
+        // state (including the view counter) without presenting the backbuffer. A plain
+        // bgfx::frame() would flip a half-drawn backbuffer to the screen partway through the
+        // logical frame; bgfx remembers the flush in m_flushPrevFrame so the next real frame
+        // still flips exactly once.
+        bgfx::frame(BGFX_FRAME_FLUSH);
+        m_nextViewId.store(0);
+        m_midFrameFlushCount.fetch_add(1);
+
+        // Publish a new generation so holders of cached view ids (FrameBuffer's m_viewId, the
+        // Canvas blit reservation) can detect that their id predates the reset and re-acquire.
+        // Without this a cached high id would sort *after* every id handed out from the reset
+        // counter, inverting submission order relative to the JS-side draw order.
+        m_viewIdGeneration.fetch_add(1);
+
+        m_frameEncoder = bgfx::begin(true);
     }
 
     void DeviceImpl::UpdateBgfxState()
@@ -546,6 +687,7 @@ namespace Babylon::Graphics
         }
 
         m_nextViewId.store(0);
+        m_midFrameFlushCount.store(0);
     }
 
     void DeviceImpl::CaptureCallback(const BgfxCallback::CaptureData& data)

Core/Graphics/Source/DeviceImpl.h

@@ -94,6 +94,15 @@ namespace Babylon::Graphics
 
         bgfx::ViewId AcquireNewViewId();
         bgfx::ViewId PeekNextViewId() const;
+        uint32_t ViewIdGeneration() const;
+
+        // Mid-frame view flush. If the current logical frame has acquired close to
+        // the maximum number of bgfx views, flush the accumulated views via a
+        // cross-thread bgfx::frame() and reset the view counter so rendering can
+        // continue within the same Babylon frame instead of running out of views
+        // (which previously threw "Too many views"). Called from the JS thread at
+        // draw/clear operation boundaries where no encoder work is pending.
+        void FlushViewsIfNeeded();
 
         // Frame completion scope support
         void IncrementPendingFrameScopes();
@@ -128,6 +137,7 @@ namespace Babylon::Graphics
         void UpdateBgfxResolution();
         void RequestScreenShots();
         void Frame();
+        void PerformMidFrameViewFlush();
         void CaptureCallback(const BgfxCallback::CaptureData&);
 
         arcana::affinity m_renderThreadAffinity{};
@@ -139,7 +149,24 @@ namespace Babylon::Graphics
         // Read by all consumers via DeviceContext::GetActiveEncoder() → DeviceImpl::GetActiveEncoder().
         bgfx::Encoder* m_frameEncoder{nullptr};
 
-        std::atomic<bgfx::ViewId> m_nextViewId{0};
+        // Widened to uint32_t so that a run of failed acquisitions cannot wrap the counter
+        // back into the valid view range. AcquireNewViewId saturates it at limits.maxViews,
+        // so it is always safe to narrow back to a bgfx::ViewId.
+        std::atomic<uint32_t> m_nextViewId{0};
+
+        // Incremented every time PerformMidFrameViewFlush resets m_nextViewId in the middle of a
+        // logical frame. Anything that caches a view id across draw calls must also cache this
+        // value and re-acquire when it no longer matches, otherwise the cached (high) id would
+        // sort after ids acquired from the reset counter and invert submission order.
+        std::atomic<uint32_t> m_viewIdGeneration{0};
+
+        // Number of mid-frame view flushes performed during the current logical frame; reset
+        // when the frame is actually presented. The flush lets a logical frame exceed bgfx's
+        // per-frame view budget, but each one is a blocking round-trip to the render thread,
+        // so an unbounded number of them turns pathological content into an apparent hang
+        // instead of an error. FlushViewsIfNeeded stops rescuing past kMaxMidFrameViewFlushes
+        // and lets AcquireNewViewId throw, which is the pre-existing behaviour.
+        std::atomic<uint32_t> m_midFrameFlushCount{0};
 
         std::atomic<bool> m_captureNextFrame{false};
 
@@ -214,6 +241,13 @@ namespace Babylon::Graphics
         int m_pendingFrameScopes{0};
         bool m_frameBlocked{true};
 
+        // Mid-frame view-flush handshake (guarded by m_frameSyncMutex):
+        //   - JS thread sets m_flushRequested and waits on m_flushCompleteCV.
+        //   - Render thread (parked in FinishRenderingCurrentFrame) services the
+        //     request via PerformMidFrameViewFlush, clears the flag, and notifies.
+        bool m_flushRequested{false};
+        std::condition_variable m_flushCompleteCV{};
+
         std::mutex m_captureCallbacksMutex{};
         arcana::ticketed_collection<std::function<void(const BgfxCallback::CaptureData&)>> m_captureCallbacks{};
 

Core/Graphics/Source/FrameBuffer.cpp

@@ -82,6 +82,7 @@ namespace Babylon::Graphics
     {
         // BGFX requires us to create a new viewID, this will ensure that the view gets cleared.
         m_viewId = m_deviceContext.AcquireNewViewId();
+        m_viewIdGeneration = m_deviceContext.ViewIdGeneration();
 
         bgfx::setViewMode(m_viewId.value(), bgfx::ViewMode::Sequential);
         bgfx::setViewClear(m_viewId.value(), flags, rgba, depth, stencil);
@@ -197,12 +198,14 @@ namespace Babylon::Graphics
 
     void FrameBuffer::SetBgfxViewPortAndScissor(const Rect& viewPort, const Rect& scissor)
     {
-        if (m_viewId.has_value() && viewPort.Equals(m_bgfxViewPort) && scissor.Equals(m_bgfxScissor))
+        if (m_viewId.has_value() && m_viewIdGeneration == m_deviceContext.ViewIdGeneration() &&
+            viewPort.Equals(m_bgfxViewPort) && scissor.Equals(m_bgfxScissor))
         {
             return;
         }
 
         m_viewId = m_deviceContext.AcquireNewViewId();
+        m_viewIdGeneration = m_deviceContext.ViewIdGeneration();
 
         bgfx::setViewMode(m_viewId.value(), bgfx::ViewMode::Sequential);
         bgfx::setViewClear(m_viewId.value(), BGFX_CLEAR_NONE, 0, 1.0f, 0);

Plugins/NativeEngine/Source/NativeEngine.cpp

@@ -1409,6 +1409,8 @@ namespace Babylon
 
     void NativeEngine::CopyTexture(NativeDataStream::Reader& data)
     {
+        // Note: GetEncoder may perform a mid-frame view flush, which resets the view counter.
+        // Fetch it before reading the reservation below so the generation check sees that.
         bgfx::Encoder* encoder = GetEncoder();
 
         const auto textureSource = data.ReadPointer<Graphics::Texture>();
@@ -1421,8 +1423,16 @@ namespace Babylon
         // view id immediately after the canvas draws (before the scene render is recorded) and
         // hands it to the source texture; use it here. Non-canvas sources have no reserved id, so
         // fall back to PeekNextViewId() (a view greater than every view used so far). See #1683.
+        //
+        // A reservation is only usable while it is still ordered relative to views handed out
+        // now: if a mid-frame flush reset the counter since the reservation was made, the
+        // reserved (high) id would sort *after* the consumer's freshly acquired (low) id and
+        // reintroduce exactly the latency the reservation exists to prevent. In that case the
+        // flush has already submitted the canvas draws in a previous bgfx frame, so the source
+        // is complete and PeekNextViewId() — which precedes every view the consumer has yet to
+        // acquire — is both safe and correctly ordered.
         bgfx::ViewId blitView = textureSource->BlitViewId();
-        if (blitView == UINT16_MAX)
+        if (blitView == UINT16_MAX || textureSource->BlitViewIdGeneration() != m_deviceContext.ViewIdGeneration())
         {
             blitView = m_deviceContext.PeekNextViewId();
         }
@@ -2597,6 +2607,12 @@ namespace Babylon
 
     bgfx::Encoder* NativeEngine::GetEncoder()
     {
+        // Draw/clear/compute operations all fetch the encoder here before recording
+        // any state. This is a safe boundary to flush accumulated bgfx views (which
+        // may swap the active encoder) so a single Babylon frame that renders many
+        // passes (e.g. nested utility layers) never runs out of views.
+        m_deviceContext.FlushViewsIfNeeded();
+
         bgfx::Encoder* encoder = m_deviceContext.GetActiveEncoder();
         assert(encoder != nullptr);
         return encoder;