Xxxx hidden paths runtime render fixes solos This PR adds support for soloing individual paths and nested shapes. It also aligns editor and runtime behavior. Diffs= ee674a819 Xxxx hidden paths runtime render fixes solos (#6252) Co-authored-by: hernan <hernan@rive.app>
diff --git a/.rive_head b/.rive_head index 096f99c..ed532e6 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -a0f076e31824c56cd13ad1cfe58597acdd90314e +ee674a819b3e4af245cb5b33bc60bab2741079ce
diff --git a/include/rive/shapes/path.hpp b/include/rive/shapes/path.hpp index 5293faf..49e01dc 100644 --- a/include/rive/shapes/path.hpp +++ b/include/rive/shapes/path.hpp
@@ -47,6 +47,7 @@ StatusCode onAddedClean(CoreContext* context) override; void buildDependencies() override; virtual const Mat2D& pathTransform() const; + bool collapse(bool value) override; CommandPath* commandPath() const { return m_CommandPath.get(); } void update(ComponentDirt value) override;
diff --git a/include/rive/shapes/path_composer.hpp b/include/rive/shapes/path_composer.hpp index 4475198..6880ce0 100644 --- a/include/rive/shapes/path_composer.hpp +++ b/include/rive/shapes/path_composer.hpp
@@ -23,6 +23,8 @@ CommandPath* localPath() const { return m_LocalPath.get(); } CommandPath* worldPath() const { return m_WorldPath.get(); } + + void pathCollapseChanged(); }; } // namespace rive #endif
diff --git a/include/rive/shapes/shape.hpp b/include/rive/shapes/shape.hpp index 2696c39..b44bfc3 100644 --- a/include/rive/shapes/shape.hpp +++ b/include/rive/shapes/shape.hpp
@@ -45,6 +45,7 @@ void addDefaultPathSpace(PathSpace space); StatusCode onAddedDirty(CoreContext* context) override; bool isEmpty(); + void pathCollapseChanged(); }; } // namespace rive
diff --git a/src/artboard.cpp b/src/artboard.cpp index bc22b8a..efd8838 100644 --- a/src/artboard.cpp +++ b/src/artboard.cpp
@@ -462,7 +462,7 @@ { continue; } - component->m_Dirt &= ComponentDirt::Collapsed; + component->m_Dirt = ComponentDirt::None; component->update(d); // If the update changed the dirt depth by adding dirt
diff --git a/src/shapes/path.cpp b/src/shapes/path.cpp index ae7d0ab..71371f3 100644 --- a/src/shapes/path.cpp +++ b/src/shapes/path.cpp
@@ -266,6 +266,17 @@ // } } +bool Path::collapse(bool value) +{ + bool changed = Super::collapse(value); + if (changed && m_Shape != nullptr) + { + m_Shape->pathCollapseChanged(); + } + + return changed; +} + #ifdef ENABLE_QUERY_FLAT_VERTICES class DisplayCubicVertex : public CubicVertex
diff --git a/src/shapes/path_composer.cpp b/src/shapes/path_composer.cpp index 6e4764e..adaef9c 100644 --- a/src/shapes/path_composer.cpp +++ b/src/shapes/path_composer.cpp
@@ -59,7 +59,7 @@ // Get all the paths into local shape space. for (auto path : m_Shape->paths()) { - if (!path->isHidden()) + if (!path->isHidden() && !path->isCollapsed()) { const auto localTransform = inverseWorld * path->pathTransform(); m_LocalPath->addPath(path->commandPath(), localTransform); @@ -80,7 +80,7 @@ } for (auto path : m_Shape->paths()) { - if (!path->isHidden()) + if (!path->isHidden() && !path->isCollapsed()) { const Mat2D& transform = path->pathTransform(); m_WorldPath->addPath(path->commandPath(), transform); @@ -89,3 +89,20 @@ } } } + +// Instead of adding dirt and rely on the recursive behavior of the addDirt method, +// we need to explicitly add dirt to the dependents. The reason is that a collapsed +// shape will not clear its dirty path flag in the current frame since it is collapsed. +// So in a future frame if it is uncollapsed, we mark its path flag as dirty again, +// but since it was already dirty, the recursive part will not kick in and the dependents +// won't update. +// This scenario is not common, but it can happen when a solo toggles between an empty +// group and a path for example. +void PathComposer::pathCollapseChanged() +{ + addDirt(ComponentDirt::Path); + for (auto d : dependents()) + { + d->addDirt(ComponentDirt::Path, true); + } +} \ No newline at end of file
diff --git a/src/shapes/shape.cpp b/src/shapes/shape.cpp index 33dd3f9..2d5ddde 100644 --- a/src/shapes/shape.cpp +++ b/src/shapes/shape.cpp
@@ -203,10 +203,13 @@ { for (auto path : m_Paths) { - if (!path->isHidden()) + if (!path->isHidden() && !path->isCollapsed()) { return false; } } return true; } + +// Do constraints need to be marked as dirty too? From tests it doesn't seem they do. +void Shape::pathCollapseChanged() { m_PathComposer.pathCollapseChanged(); }
diff --git a/test/assets/solos_collapse_tests.riv b/test/assets/solos_collapse_tests.riv new file mode 100644 index 0000000..6d0006f --- /dev/null +++ b/test/assets/solos_collapse_tests.riv Binary files differ
diff --git a/test/path_test.cpp b/test/path_test.cpp index 38bbf20..55afb19 100644 --- a/test/path_test.cpp +++ b/test/path_test.cpp
@@ -6,8 +6,12 @@ #include <rive/shapes/path_composer.hpp> #include <rive/shapes/rectangle.hpp> #include <rive/shapes/shape.hpp> +#include <rive/shapes/clipping_shape.hpp> #include <utils/no_op_factory.hpp> #include <utils/no_op_renderer.hpp> +#include <rive/solo.hpp> +#include <rive/animation/linear_animation_instance.hpp> +#include "rive_file_reader.hpp" #include <catch.hpp> #include <cstdio> @@ -21,7 +25,8 @@ LineTo, CubicTo, Reset, - Close + Close, + AddPath, }; struct TestPathCommand @@ -46,7 +51,11 @@ } void fillRule(rive::FillRule value) override {} - void addPath(rive::CommandPath* path, const rive::Mat2D& transform) override {} + void addPath(rive::CommandPath* path, const rive::Mat2D& transform) override + { + commands.emplace_back( + TestPathCommand{TestPathCommandType::AddPath, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}); + } void addRenderPath(rive::RenderPath* path, const rive::Mat2D& transform) override {} void moveTo(float x, float y) override @@ -260,3 +269,168 @@ REQUIRE(path->commands[6].command == TestPathCommandType::Close); } + +TEST_CASE("nested solo with shape expanded and path collapsed", "[path]") +{ + TestNoOpFactory emptyFactory; + auto file = ReadRiveFile("../../test/assets/solos_collapse_tests.riv", &emptyFactory); + + auto artboard = file->artboard("test-1-shape-with-shape-and-path")->instance(); + // Root-Shape + artboard->advance(0.0f); + auto rootShape = artboard->children()[0]->as<rive::Shape>(); + REQUIRE(rootShape != nullptr); + REQUIRE(rootShape->name() == "Root-Shape"); + auto s1 = artboard->find<rive::Solo>("Solo-1"); + REQUIRE(s1 != nullptr); + auto solos = artboard->find<rive::Solo>(); + REQUIRE(solos.size() == 1); + auto solo = solos[0]; + REQUIRE(solo->children().size() == 2); + REQUIRE(solo->children()[0]->is<rive::Shape>()); + REQUIRE(solo->children()[0]->name() == "Rectangle-shape"); + REQUIRE(solo->children()[1]->is<rive::Path>()); + REQUIRE(solo->children()[1]->name() == "Path-2"); + auto rectangleShape = solo->children()[0]->as<rive::Shape>(); + auto path = solo->children()[1]->as<rive::Path>(); + REQUIRE(rectangleShape->isCollapsed() == false); + REQUIRE(path->isCollapsed() == true); + REQUIRE(path->commandPath() != nullptr); + auto pathComposer = rootShape->pathComposer(); + auto pathComposerPath = static_cast<TestRenderPath*>(pathComposer->localPath()); + // Path is skipped and the nested shape forms its own drawable, so size is 0 + REQUIRE(pathComposerPath->commands.size() == 0); +} + +TEST_CASE("nested solo clipping with shape collapsed and path expanded", "[path]") +{ + TestNoOpFactory emptyFactory; + auto file = ReadRiveFile("../../test/assets/solos_collapse_tests.riv", &emptyFactory); + + auto artboard = file->artboard("test-2-clip-with-shape-and-path")->instance(); + // Root-Shape + artboard->advance(0.0f); + auto rectangleClip = artboard->find<rive::Shape>("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto s1 = artboard->find<rive::Solo>("Solo-name"); + REQUIRE(s1 != nullptr); + auto solos = artboard->find<rive::Solo>(); + REQUIRE(solos.size() == 1); + auto solo = solos[0]; + REQUIRE(solo->children().size() == 2); + REQUIRE(solo->children()[0]->is<rive::Shape>()); + REQUIRE(solo->children()[0]->name() == "Rectangle-shape"); + REQUIRE(solo->children()[1]->is<rive::Path>()); + REQUIRE(solo->children()[1]->name() == "Path-2"); + auto rectangleShape = solo->children()[0]->as<rive::Shape>(); + auto path = solo->children()[1]->as<rive::Path>(); + REQUIRE(rectangleShape->isCollapsed() == true); + REQUIRE(path->isCollapsed() == false); + REQUIRE(path->commandPath() != nullptr); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast<TestRenderPath*>(clippingShape->renderPath()); + // One path is skipped, otherwise size would be 3 + REQUIRE(clippingPath->commands.size() == 2); +} + +TEST_CASE("nested solo clipping with animation", "[path]") +{ + TestNoOpFactory emptyFactory; + auto file = ReadRiveFile("../../test/assets/solos_collapse_tests.riv", &emptyFactory); + + auto artboard = file->artboard("test-5-clip-with-group-and-path-and-shape")->instance(); + artboard->advance(0.0f); + auto rectangleClip = artboard->find<rive::Shape>("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast<TestRenderPath*>(clippingShape->renderPath()); + REQUIRE(clippingPath != nullptr); + std::unique_ptr<rive::LinearAnimationInstance> animation = artboard->animationAt(0); + // First a single shape is drawn as part of the solo + REQUIRE(clippingPath->commands[0].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[1].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 2); + animation->advanceAndApply(2.5f); + // Then a different shape is drawn as part of the solo + REQUIRE(clippingPath->commands[2].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[3].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 4); + animation->advanceAndApply(2.5f); + // Then an empty group is focused, so there is no path drawn + REQUIRE(clippingPath->commands[4].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands.size() == 5); + animation->advanceAndApply(2.5f); + // Then an empty group is focused, so there is no path drawn + REQUIRE(clippingPath->commands[5].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[6].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 7); + animation->advanceAndApply(1.0f); + // Then back to the group so no path is added + REQUIRE(clippingPath->commands[7].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands.size() == 8); + animation->advanceAndApply(2.5f); + // Finally a new shape is added + REQUIRE(clippingPath->commands[8].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[9].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 10); +} + +TEST_CASE("double nested solos clipping with animation", "[path]") +{ + TestNoOpFactory emptyFactory; + auto file = ReadRiveFile("../../test/assets/solos_collapse_tests.riv", &emptyFactory); + + auto artboard = file->artboard("test-6-clip-with-nested-solos")->instance(); + artboard->advance(0.0f); + auto rectangleClip = artboard->find<rive::Shape>("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast<TestRenderPath*>(clippingShape->renderPath()); + REQUIRE(clippingPath != nullptr); + std::unique_ptr<rive::LinearAnimationInstance> animation = artboard->animationAt(0); + // First a single shape is drawn as part of the solo + REQUIRE(clippingPath->commands[0].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[1].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 2); + animation->advanceAndApply(1.5f); // 1.5s in timeline + // Changed nested group but path does not change. + // Reset and AddPath are called anyway because it is marked as dirty + REQUIRE(clippingPath->commands[2].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[3].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 4); + animation->advanceAndApply(1.0f); // 2.5s in timeline + // Both solos are pointing to empty groups + REQUIRE(clippingPath->commands[4].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands.size() == 5); + animation->advanceAndApply(1.0f); // 3.5s in timeline + // Nothing changes, it hasn't reached any new keyframe + REQUIRE(clippingPath->commands.size() == 5); + animation->advanceAndApply(1.0f); // 4.5s in timeline + // Outer solo is pointing to inner solo, but inner solo is pointing to empty group + REQUIRE(clippingPath->commands.size() == 5); + animation->advanceAndApply(1.0f); // 5.5s in timeline + // Outer solo is pointing to inner solo, inner solo is pointing to shape + REQUIRE(clippingPath->commands[5].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[6].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 7); + animation->advanceAndApply(1.0f); // 6.5s in timeline + // Outer solo pointing to empty group + REQUIRE(clippingPath->commands[7].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands.size() == 8); + animation->advanceAndApply(2.0f); // 8.5s in timeline + // Outer solo pointing to inner solo. Inner solo pointing to rect shape + REQUIRE(clippingPath->commands[8].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[9].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 10); + animation->advanceAndApply(2.0f); // 10.5s in timeline + // Outer solo pointing to inner solo. Inner solo pointing to path + REQUIRE(clippingPath->commands[10].command == TestPathCommandType::Reset); + REQUIRE(clippingPath->commands[11].command == TestPathCommandType::AddPath); + REQUIRE(clippingPath->commands.size() == 12); +}