diff --git a/.rive_head b/.rive_head index 096f99c8..ed532e65 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 5293fafa..49e01dc8 100644 --- a/include/rive/shapes/path.hpp +++ b/include/rive/shapes/path.hpp @@ -47,6 +47,7 @@ class Path : public PathBase 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 4475198b..6880ce0a 100644 --- a/include/rive/shapes/path_composer.hpp +++ b/include/rive/shapes/path_composer.hpp @@ -23,6 +23,8 @@ class PathComposer : public Component 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 2696c39a..b44bfc3b 100644 --- a/include/rive/shapes/shape.hpp +++ b/include/rive/shapes/shape.hpp @@ -45,6 +45,7 @@ class Shape : public ShapeBase, public ShapePaintContainer 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 bc22b8ac..efd88383 100644 --- a/src/artboard.cpp +++ b/src/artboard.cpp @@ -462,7 +462,7 @@ bool Artboard::updateComponents() { 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 ae7d0abd..71371f3e 100644 --- a/src/shapes/path.cpp +++ b/src/shapes/path.cpp @@ -266,6 +266,17 @@ void Path::update(ComponentDirt value) // } } +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 6e4764e5..adaef9cf 100644 --- a/src/shapes/path_composer.cpp +++ b/src/shapes/path_composer.cpp @@ -59,7 +59,7 @@ void PathComposer::update(ComponentDirt value) // 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 @@ void PathComposer::update(ComponentDirt value) } 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 @@ void PathComposer::update(ComponentDirt value) } } } + +// 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 33dd3f9c..2d5ddde6 100644 --- a/src/shapes/shape.cpp +++ b/src/shapes/shape.cpp @@ -203,10 +203,13 @@ bool Shape::isEmpty() { 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 00000000..6d0006f7 Binary files /dev/null and b/test/assets/solos_collapse_tests.riv differ diff --git a/test/path_test.cpp b/test/path_test.cpp index 38bbf202..55afb19f 100644 --- a/test/path_test.cpp +++ b/test/path_test.cpp @@ -6,8 +6,12 @@ #include #include #include +#include #include #include +#include +#include +#include "rive_file_reader.hpp" #include #include @@ -21,7 +25,8 @@ enum class TestPathCommandType LineTo, CubicTo, Reset, - Close + Close, + AddPath, }; struct TestPathCommand @@ -46,7 +51,11 @@ class TestRenderPath : public rive::RenderPath } 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 @@ TEST_CASE("ellipse path builds expected commands", "[path]") 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(); + REQUIRE(rootShape != nullptr); + REQUIRE(rootShape->name() == "Root-Shape"); + auto s1 = artboard->find("Solo-1"); + REQUIRE(s1 != nullptr); + auto solos = artboard->find(); + REQUIRE(solos.size() == 1); + auto solo = solos[0]; + REQUIRE(solo->children().size() == 2); + REQUIRE(solo->children()[0]->is()); + REQUIRE(solo->children()[0]->name() == "Rectangle-shape"); + REQUIRE(solo->children()[1]->is()); + REQUIRE(solo->children()[1]->name() == "Path-2"); + auto rectangleShape = solo->children()[0]->as(); + auto path = solo->children()[1]->as(); + REQUIRE(rectangleShape->isCollapsed() == false); + REQUIRE(path->isCollapsed() == true); + REQUIRE(path->commandPath() != nullptr); + auto pathComposer = rootShape->pathComposer(); + auto pathComposerPath = static_cast(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("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto s1 = artboard->find("Solo-name"); + REQUIRE(s1 != nullptr); + auto solos = artboard->find(); + REQUIRE(solos.size() == 1); + auto solo = solos[0]; + REQUIRE(solo->children().size() == 2); + REQUIRE(solo->children()[0]->is()); + REQUIRE(solo->children()[0]->name() == "Rectangle-shape"); + REQUIRE(solo->children()[1]->is()); + REQUIRE(solo->children()[1]->name() == "Path-2"); + auto rectangleShape = solo->children()[0]->as(); + auto path = solo->children()[1]->as(); + REQUIRE(rectangleShape->isCollapsed() == true); + REQUIRE(path->isCollapsed() == false); + REQUIRE(path->commandPath() != nullptr); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast(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("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast(clippingShape->renderPath()); + REQUIRE(clippingPath != nullptr); + std::unique_ptr 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("Rectangle-clipped"); + REQUIRE(rectangleClip != nullptr); + REQUIRE(rectangleClip->name() == "Rectangle-clipped"); + auto clippingShape = rectangleClip->clippingShapes()[0]; + REQUIRE(clippingShape != nullptr); + auto clippingPath = static_cast(clippingShape->renderPath()); + REQUIRE(clippingPath != nullptr); + std::unique_ptr 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); +}