fix: lua error on in-flight path mutation (#11350) 56f08da6b1

* fix: lua error on in-flight path mutation

* fix: remove DrawOption

* fix: failing tests

Co-authored-by: Luigi Rosso <luigi-rosso@users.noreply.github.com>

.rive_head

@@ -1 +1 @@
-923b32059d2ea012fdb47ae141b304f57095a48b
+56f08da6b1998d346fa791770d7a62a50466e0f7

include/rive/artboard.hpp

@@ -102,6 +102,7 @@ private:
     bool m_didChange = true;
     Artboard* parentArtboard() const;
     ArtboardHost* m_host = nullptr;
+    static uint64_t sm_frameId;
     bool sharesLayoutWithHost() const;
     void cloneObjectDataBinds(const Core* object,
                               Core* clone,
@@ -128,6 +129,10 @@ private:
     void update(ComponentDirt value) override;
 
 public:
+    static uint64_t frameId() { return sm_frameId; }
+#ifdef TESTING
+    static void incFrameId() { sm_frameId++; }
+#endif
     void updateDataBinds(bool applyTargetToSource = true) override;
     void host(ArtboardHost* artboardHost);
     ArtboardHost* host() const;
@@ -229,13 +234,7 @@ public:
     Drawable* firstDrawable() { return m_FirstDrawable; };
     void addScriptedObject(ScriptedObject* object);
 
-    enum class DrawOption
+    void drawInternal(Renderer* renderer);
-    {
-        kNormal,
-        kHideBG,
-        kHideFG,
-    };
-    void draw(Renderer* renderer, DrawOption option);
     void draw(Renderer* renderer) override;
     void addToRenderPath(RenderPath* path, const Mat2D& transform);
     void addToRawPath(RawPath& path, const Mat2D* transform);

include/rive/lua/rive_lua_libs.hpp

@@ -270,6 +270,7 @@ public:
     static constexpr bool hasMetatable = true;
 
 private:
+    uint64_t m_renderFrameId = 0;
 };
 
 class ScriptedGradient

src/artboard.cpp

@@ -43,6 +43,8 @@
 
 using namespace rive;
 
+uint64_t Artboard::sm_frameId = 0;
+
 Artboard::Artboard()
 {
     // Artboards need to override default clip value to true.
@@ -1390,9 +1392,13 @@ bool Artboard::hitTestPoint(const Vec2D& position,
                                          isPrimaryHit);
 }
 
-void Artboard::draw(Renderer* renderer) { draw(renderer, DrawOption::kNormal); }
+void Artboard::draw(Renderer* renderer)
+{
+    sm_frameId++;
+    drawInternal(renderer);
+}
 
-void Artboard::draw(Renderer* renderer, DrawOption option)
+void Artboard::drawInternal(Renderer* renderer)
 {
     RIVE_PROF_SCOPE()
     m_didChange = false;
@@ -1418,69 +1424,61 @@ void Artboard::draw(Renderer* renderer, DrawOption option)
         renderer->transform(artboardTransform);
     }
 
-    if (option != DrawOption::kHideBG)
+    for (auto shapePaint : m_ShapePaints)
     {
-        for (auto shapePaint : m_ShapePaints)
+        if (!shapePaint->shouldDraw())
         {
-            if (!shapePaint->shouldDraw())
+            continue;
-            {
-                continue;
-            }
-            auto shapePaintPath = shapePaint->pickPath(this);
-            if (shapePaintPath == nullptr)
-            {
-                continue;
-            }
-            shapePaint->draw(renderer, shapePaintPath, worldTransform());
         }
+        auto shapePaintPath = shapePaint->pickPath(this);
+        if (shapePaintPath == nullptr)
+        {
+            continue;
+        }
+        shapePaint->draw(renderer, shapePaintPath, worldTransform());
     }
-
+    // Empty clips is a counter for clipping shapes that are empty, for
-    if (option != DrawOption::kHideFG)
+    // example because they are hidden in a solo. If emptyClips > 0, the
+    // drawables should not be drawn.
+    int emptyClips = 0;
+    // We stack clip operations to avoid calling a save + clip + restore on
+    // clipping that don't have any drawables in between. this is a common
+    // case with drawables in solos where the drawables are not drawn.
+    std::vector<Drawable*> pendingClipOperations;
+    for (auto drawable = m_FirstDrawable; drawable != nullptr;
+         drawable = drawable->prev)
     {
-        // Empty clips is a counter for clipping shapes that are empty, for
+        auto prevClips = emptyClips;
-        // example because they are hidden in a solo. If emptyClips > 0, the
+        emptyClips += drawable->emptyClipCount();
-        // drawables should not be drawn.
+        if (!drawable->willDraw() || emptyClips != prevClips || emptyClips > 0)
-        int emptyClips = 0;
-        // We stack clip operations to avoid calling a save + clip + restore on
-        // clipping that don't have any drawables in between. this is a common
-        // case with drawables in solos where the drawables are not drawn.
-        std::vector<Drawable*> pendingClipOperations;
-        for (auto drawable = m_FirstDrawable; drawable != nullptr;
-             drawable = drawable->prev)
         {
-            auto prevClips = emptyClips;
+            continue;
-            emptyClips += drawable->emptyClipCount();
-            if (!drawable->willDraw() || emptyClips != prevClips ||
-                emptyClips > 0)
-            {
-                continue;
-            }
-            if (drawable->isClipStart())
-            {
-                pendingClipOperations.push_back(drawable);
-                continue;
-            }
-            else if (pendingClipOperations.size() > 0)
-            {
-                // If there are clip operations pending and the next drawable is
-                // a clip end, the clipping operation does not clip anything and
-                // both can be skipped.
-                if (drawable->isClipEnd())
-                {
-                    pendingClipOperations.pop_back();
-                    continue;
-                }
-                else
-                {
-                    for (auto& pendingClip : pendingClipOperations)
-                    {
-                        pendingClip->draw(renderer);
-                    }
-                    pendingClipOperations.clear();
-                }
-            }
-            drawable->draw(renderer);
         }
+        if (drawable->isClipStart())
+        {
+            pendingClipOperations.push_back(drawable);
+            continue;
+        }
+        else if (pendingClipOperations.size() > 0)
+        {
+            // If there are clip operations pending and the next drawable is
+            // a clip end, the clipping operation does not clip anything and
+            // both can be skipped.
+            if (drawable->isClipEnd())
+            {
+                pendingClipOperations.pop_back();
+                continue;
+            }
+            else
+            {
+                for (auto& pendingClip : pendingClipOperations)
+                {
+                    pendingClip->draw(renderer);
+                }
+                pendingClipOperations.clear();
+            }
+        }
+        drawable->draw(renderer);
     }
     if (save)
     {

src/artboard_component_list.cpp

@@ -468,7 +468,7 @@ void ArtboardComponentList::draw(Renderer* renderer)
                     renderer->save();
                     auto transform = m_artboardTransforms[artboard];
                     renderer->transform(transform);
-                    artboard->draw(renderer);
+                    artboard->drawInternal(renderer);
                     renderer->restore();
                 }
                 if (i == endIndex)
@@ -490,7 +490,7 @@ void ArtboardComponentList::draw(Renderer* renderer)
                 renderer->save();
                 auto transform = m_artboardTransforms[artboard];
                 renderer->transform(transform);
-                artboard->draw(renderer);
+                artboard->drawInternal(renderer);
                 renderer->restore();
             }
         }

src/lua/renderer/lua_path.cpp

@@ -16,6 +16,11 @@ RenderPath* ScriptedPath::renderPath(lua_State* L)
     if (m_isRenderPathDirty)
     {
         m_isRenderPathDirty = false;
+        if (m_renderFrameId == Artboard::frameId())
+        {
+            luaL_error(L, "Path was modified between draws in the same frame.");
+        }
+        m_renderFrameId = Artboard::frameId();
         if (!m_renderPath)
         {
             ScriptingContext* context =

src/nested_artboard.cpp

@@ -163,7 +163,7 @@ void NestedArtboard::draw(Renderer* renderer)
         renderer->save();
     }
     renderer->transform(worldTransform());
-    m_Artboard->draw(renderer);
+    m_Artboard->drawInternal(renderer);
     if (m_needsSaveOperation)
     {
         renderer->restore();

utils/serializing_factory.cpp

@@ -1,6 +1,7 @@
 #include "utils/serializing_factory.hpp"
 #include "rive/decoders/bitmap_decoder.hpp"
 #include "rive/core/binary_reader.hpp"
+#include "rive/artboard.hpp"
 #include <cstring>
 #include <stdlib.h>
 #include <filesystem>
@@ -623,6 +624,7 @@ std::unique_ptr<Renderer> SerializingFactory::makeRenderer()
 
 void SerializingFactory::addFrame()
 {
+    Artboard::incFrameId();
     m_writer.writeVarUint((uint32_t)SerializeOp::frame);
 }