fix(runtime): take a skinned path's winding from its bind pose (#14452) 89a770ce65 A clockwise fill needs to know which way a bone bound path winds, so PointsPath::winding() measured the deformed path once and cached the answer, flipping it only when the bones mirror. It measured whatever pose the first frame happened to show. A rig that loads folded inside out -- a nested artboard whose data bound size starts narrow, say -- cached the folded winding, and once the bones unfolded the path the composer added it backwards: counter-clockwise, so the GPU renderer's clockwise fill drew nothing. Renderers without a clockwise rule (CG, the silver replay) filled it anyway, so only the editor and the GPU showed it. Measure the path as bound instead. With every bone at its bind transform the tendons' inverse binds cancel, leaving each vertex at the skin's world transform, so the bind pose is known without deforming anything and does not depend on the pose a rig loads in. Mirroring is still followed as before, a fold without mirroring still keeps the reference (the pinned limitation), and a path bound with no area falls back to measuring its pose. Adds a regression test for a rig that loads folded. No committed silver changes. Co-authored-by: hernan <hernan@rive.app>
diff --git a/.rive_head b/.rive_head index 8f9790a..134b702 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -e04516db64ecea7c9c7947de19190f7cd19182f0 +89a770ce653b7ae8f7a3a3a0da73a229377f5d1f
diff --git a/include/rive/bones/skin.hpp b/include/rive/bones/skin.hpp index 42c7760..ff06b49 100644 --- a/include/rive/bones/skin.hpp +++ b/include/rive/bones/skin.hpp
@@ -32,6 +32,9 @@ StatusCode onAddedDirty(CoreContext* context) override; void buildDependencies() override; void deform(Span<Vertex*> vertices); + // The skinnable's world transform when it was bound. With every bone at + // its bind transform, deform maps a vertex through this alone. + const Mat2D& worldTransform() const { return m_WorldTransform; } void onDirty(ComponentDirt dirt) override; void update(ComponentDirt value) override; #ifdef WITH_RIVE_EDITOR
diff --git a/include/rive/shapes/points_path.hpp b/include/rive/shapes/points_path.hpp index 0aacfc7..33b0ac2 100644 --- a/include/rive/shapes/points_path.hpp +++ b/include/rive/shapes/points_path.hpp
@@ -19,8 +19,10 @@ #endif private: - // Measured winding with the bones' mirroring divided out, 0 when unknown. + // Winding with the bones' mirroring divided out, 0 when unknown. int m_windingReference = 0; + // Winding of the path as bound, 0 when it has no area there. + int bindWinding(); }; } // namespace rive
diff --git a/src/shapes/points_path.cpp b/src/shapes/points_path.cpp index 643037d..fb14862 100644 --- a/src/shapes/points_path.cpp +++ b/src/shapes/points_path.cpp
@@ -1,5 +1,6 @@ #include "rive/artboard.hpp" #include "rive/shapes/points_path.hpp" +#include "rive/shapes/cubic_vertex.hpp" #include "rive/shapes/vertex.hpp" #include "rive/shapes/path_vertex.hpp" #include "rive/shapes/shape.hpp" @@ -56,8 +57,66 @@ void PointsPath::markSkinDirty() { Super::markPathDirty(); } -// Measured once, then follows the bones' mirroring. Bones that fold the path -// inside out without mirroring keep the measured answer, as the flag did. +namespace +{ +// 1 when [path] winds clockwise, -1 when it winds the other way, 0 when it has +// too little area to tell. +int measureWinding(const RawPath& path) +{ + AABB bounds = path.bounds(); + float area = path.computeCoarseArea(bounds.center()); + float extent = std::max(bounds.width(), bounds.height()); + // The area sums products of the path's extent, so that is the scale of + // its rounding. A collapsed path has no winding worth caching. + if (std::abs(area) <= 1e-5f * extent * extent) + { + return 0; + } + return area < 0 ? -1 : 1; +} +} // namespace + +// With every bone at its bind transform the tendons' inverse binds cancel them, +// so deform leaves each point at the skin's world transform applied to it. +// Corner radii are left out, they round a corner without turning it around. +int PointsPath::bindWinding() +{ + const auto& points = m_Vertices; + size_t count = points.size(); + if (count < 2) + { + return 0; + } + const Mat2D& world = skin()->worldTransform(); + RawPath bound; + bound.move(world * Vec2D(points[0]->x(), points[0]->y())); + size_t segments = isPathClosed() ? count : count - 1; + for (size_t i = 0; i < segments; i++) + { + auto from = points[i]; + auto to = points[(i + 1) % count]; + Vec2D end = world * Vec2D(to->x(), to->y()); + if (from->is<CubicVertex>() || to->is<CubicVertex>()) + { + Vec2D out = from->is<CubicVertex>() + ? from->as<CubicVertex>()->outPoint() + : Vec2D(from->x(), from->y()); + Vec2D in = to->is<CubicVertex>() ? to->as<CubicVertex>()->inPoint() + : Vec2D(to->x(), to->y()); + bound.cubic(world * out, world * in, end); + } + else + { + bound.line(end); + } + } + bound.close(); + return measureWinding(bound); +} + +// Taken from the path as bound, then follows the bones' mirroring. Bones that +// fold the path inside out without mirroring keep that answer, as the flag did, +// so whatever pose a rig loads in can't change what its unfolded path draws. int PointsPath::winding() { int authored = isClockwise() ? 1 : -1; @@ -65,21 +124,22 @@ { return authored; } + if (m_windingReference == 0) + { + m_windingReference = bindWinding(); + } int sign = skin()->windingSign(); if (sign != 0 && m_windingReference != 0) { return m_windingReference * sign; } - AABB bounds = rawPath().bounds(); - float area = rawPath().computeCoarseArea(bounds.center()); - float extent = std::max(bounds.width(), bounds.height()); - // The area sums products of the path's extent, so that is the scale of - // its rounding. A collapsed path has no winding worth caching. - if (std::abs(area) <= 1e-5f * extent * extent) + // Mixed or collapsed bones leave no mirroring to follow, and a path bound + // with no area has no reference: measure the pose itself. + int measured = measureWinding(rawPath()); + if (measured == 0) { return authored; } - int measured = area < 0 ? -1 : 1; if (sign != 0) { m_windingReference = measured * sign;
diff --git a/tests/unit_tests/runtime/skinned_winding_test.cpp b/tests/unit_tests/runtime/skinned_winding_test.cpp index 37106d7..cdb224f 100644 --- a/tests/unit_tests/runtime/skinned_winding_test.cpp +++ b/tests/unit_tests/runtime/skinned_winding_test.cpp
@@ -31,7 +31,9 @@ rive::Skin* skin = new rive::Skin(); std::vector<rive::StraightVertex*> vertices; - QuadRig(bool clockwise) + // The top bone is bound at y = 0 but starts at [topStartY], so a value + // past the bottom bone loads the path folded inside out. + QuadRig(bool clockwise, float topStartY = 0.0f) { artboard.addObject(&artboard); auto add = [&](rive::Component* component, rive::Core* parent) { @@ -77,6 +79,7 @@ tendon->ty(bone->y()); add(tendon, skin); } + top->y(topStartY); REQUIRE(artboard.initialize() == rive::StatusCode::Ok); } @@ -265,6 +268,26 @@ CHECK(composed < 0); } +// The winding is taken from the path as bound, not from whatever pose the first +// frame shows: a rig that loads folded (say a data bound size that starts +// narrow) must still fill once its bones unfold it. +TEST_CASE("a folded first frame is not cached as the winding", "[skinwinding]") +{ + for (bool clockwise : {true, false}) + { + QuadRig rig(clockwise, 200.0f); + float folded = rig.composedArea(); + CHECK(rig.skin->windingSign() == 1); + CHECK((rig.deformedArea() < 0) == clockwise); + // Folded without mirroring, as in the test above. + CHECK(folded < 0); + + rig.top->y(0.0f); + CHECK(rig.composedArea() > 0); + CHECK((rig.deformedArea() > 0) == clockwise); + } +} + TEST_CASE("a real rig stays clockwise when its bones mirror", "[skinwinding]") { auto file = ReadRiveFile("assets/zombie_skins.riv");