Skip to content

Commit

Permalink
Xxxx hidden paths runtime render fixes solos
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bodymovin and bodymovin committed Nov 22, 2023
1 parent 647e408 commit 5df489c
Show file tree
Hide file tree
Showing 10 changed files with 216 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +1 @@
a0f076e31824c56cd13ad1cfe58597acdd90314e
ee674a819b3e4af245cb5b33bc60bab2741079ce
1 change: 1 addition & 0 deletions include/rive/shapes/path.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions include/rive/shapes/path_composer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions include/rive/shapes/shape.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/artboard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/shapes/path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 19 additions & 2 deletions src/shapes/path_composer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
}
5 changes: 4 additions & 1 deletion src/shapes/shape.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Binary file added test/assets/solos_collapse_tests.riv
Binary file not shown.
178 changes: 176 additions & 2 deletions test/path_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -21,7 +25,8 @@ enum class TestPathCommandType
LineTo,
CubicTo,
Reset,
Close
Close,
AddPath,
};

struct TestPathCommand
Expand All @@ -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
Expand Down Expand Up @@ -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<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);
}

0 comments on commit 5df489c

Please sign in to comment.