LibWeb+LibGfx: Keep path properties when cloning and implement fill-rule

Author: https://github.com/Gingeh
Commit: 3467076dbc
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/1996
Reviewed-by: https://github.com/AtkinsSJ ✅

Tests/LibWeb/Screenshot/clip-path-basic-shapes.html

@@ -121,6 +121,9 @@
 <div class="outer">
     <div class="inner" style="clip-path: polygon(nonzero, 65px 0px, 35px 80px, 105px 30px, 25px 30px, 95px 80px)"></div>
 </div>
+<div class="outer">
+    <div class="inner" style="clip-path: polygon(evenodd, 65px 0px, 35px 80px, 105px 30px, 25px 30px, 95px 80px)"></div>
+</div>
 <div class="outer">
     <div class="inner" style="clip-path: polygon(100px 0%, 50% 50px, 100% 100%)"></div>
 </div>

Tests/LibWeb/Text/expected/css/calc-coverage.txt

@@ -34,10 +34,10 @@ border-top-width: 'calc(2px)' -> 'calc(2px)'
 border-top-width: 'calc(2px * var(--n))' -> 'calc(2px * 2)'
 bottom: 'calc(2px)' -> 'calc(2px)'
 bottom: 'calc(2px * var(--n))' -> 'calc(2px * 2)'
-clip-path: 'polygon(calc(0px) calc(2px), calc(2px) calc(0px), calc(2px) calc(2px))' -> 'polygon(calc(0px) calc(2px),calc(2px) calc(0px),calc(2px) calc(2px))'
+clip-path: 'polygon(calc(0px) calc(2px), calc(2px) calc(0px), calc(2px) calc(2px))' -> 'polygon(nonzero, calc(0px) calc(2px), calc(2px) calc(0px), calc(2px) calc(2px))'
-clip-path: 'polygon(calc(0px * var(--n)) calc(2px * var(--n)), calc(2px * var(--n)) calc(0px * var(--n)), calc(2px * var(--n)) calc(2px * var(--n)))' -> 'polygon(calc(0px * 2) calc(2px * 2),calc(2px * 2) calc(0px * 2),calc(2px * 2) calc(2px * 2))'
+clip-path: 'polygon(calc(0px * var(--n)) calc(2px * var(--n)), calc(2px * var(--n)) calc(0px * var(--n)), calc(2px * var(--n)) calc(2px * var(--n)))' -> 'polygon(nonzero, calc(0px * 2) calc(2px * 2), calc(2px * 2) calc(0px * 2), calc(2px * 2) calc(2px * 2))'
-clip-path: 'polygon(calc(0%) calc(2%), calc(2%) calc(0%), calc(2%) calc(2%))' -> 'polygon(calc(0%) calc(2%),calc(2%) calc(0%),calc(2%) calc(2%))'
+clip-path: 'polygon(calc(0%) calc(2%), calc(2%) calc(0%), calc(2%) calc(2%))' -> 'polygon(nonzero, calc(0%) calc(2%), calc(2%) calc(0%), calc(2%) calc(2%))'
-clip-path: 'polygon(calc(0% * var(--n)) calc(2% * var(--n)), calc(2% * var(--n)) calc(0% * var(--n)), calc(2% * var(--n)) calc(2% * var(--n)))' -> 'polygon(calc(0% * 2) calc(2% * 2),calc(2% * 2) calc(0% * 2),calc(2% * 2) calc(2% * 2))'
+clip-path: 'polygon(calc(0% * var(--n)) calc(2% * var(--n)), calc(2% * var(--n)) calc(0% * var(--n)), calc(2% * var(--n)) calc(2% * var(--n)))' -> 'polygon(nonzero, calc(0% * 2) calc(2% * 2), calc(2% * 2) calc(0% * 2), calc(2% * 2) calc(2% * 2))'
 column-count: 'calc(2)' -> 'calc(2)'
 column-count: 'calc(2 * var(--n))' -> '4'
 column-gap: 'calc(2px)' -> 'calc(2px)'

Userland/Libraries/LibGfx/Path.h

@@ -39,6 +39,7 @@ public:
     [[nodiscard]] virtual bool is_empty() const = 0;
     virtual Gfx::FloatPoint last_point() const = 0;
     virtual Gfx::FloatRect bounding_box() const = 0;
+    virtual void set_fill_type(Gfx::WindingRule winding_rule) = 0;
     virtual bool contains(FloatPoint point, Gfx::WindingRule) const = 0;
 
     virtual NonnullOwnPtr<PathImpl> clone() const = 0;
@@ -86,6 +87,7 @@ public:
     Gfx::FloatPoint last_point() const { return impl().last_point(); }
     Gfx::FloatRect bounding_box() const { return impl().bounding_box(); }
     bool contains(FloatPoint point, Gfx::WindingRule winding_rule) const { return impl().contains(point, winding_rule); }
+    void set_fill_type(Gfx::WindingRule winding_rule) { impl().set_fill_type(winding_rule); }
 
     Gfx::Path clone() const { return Gfx::Path { impl().clone() }; }
     Gfx::Path copy_transformed(Gfx::AffineTransform const& transform) const { return Gfx::Path { impl().copy_transformed(transform) }; }

Userland/Libraries/LibGfx/PathSkia.cpp

@@ -29,6 +29,12 @@ PathImplSkia::PathImplSkia()
 {
 }
 
+PathImplSkia::PathImplSkia(PathImplSkia const& other)
+    : m_last_move_to(other.m_last_move_to)
+    , m_path(adopt_own(*new SkPath(other.sk_path())))
+{
+}
+
 PathImplSkia::~PathImplSkia() = default;
 
 void PathImplSkia::clear()
@@ -217,21 +223,24 @@ bool PathImplSkia::contains(FloatPoint point, Gfx::WindingRule winding_rule) con
     return temp_path.contains(point.x(), point.y());
 }
 
+void PathImplSkia::set_fill_type(Gfx::WindingRule winding_rule)
+{
+    m_path->setFillType(to_skia_path_fill_type(winding_rule));
+}
+
 NonnullOwnPtr<PathImpl> PathImplSkia::clone() const
 {
-    auto new_path = PathImplSkia::create();
+    return adopt_own(*new PathImplSkia(*this));
-    new_path->sk_path().addPath(*m_path);
-    return new_path;
 }
 
 NonnullOwnPtr<PathImpl> PathImplSkia::copy_transformed(Gfx::AffineTransform const& transform) const
 {
-    auto new_path = PathImplSkia::create();
+    auto new_path = adopt_own(*new PathImplSkia(*this));
     auto matrix = SkMatrix::MakeAll(
         transform.a(), transform.c(), transform.e(),
         transform.b(), transform.d(), transform.f(),
         0, 0, 1);
-    new_path->sk_path().addPath(*m_path, matrix);
+    new_path->sk_path().transform(matrix);
     return new_path;
 }
 

Userland/Libraries/LibGfx/PathSkia.h

@@ -36,6 +36,7 @@ public:
     virtual Gfx::FloatPoint last_point() const override;
     virtual Gfx::FloatRect bounding_box() const override;
     virtual bool contains(FloatPoint point, Gfx::WindingRule) const override;
+    virtual void set_fill_type(Gfx::WindingRule winding_rule) override;
 
     virtual NonnullOwnPtr<PathImpl> clone() const override;
     virtual NonnullOwnPtr<PathImpl> copy_transformed(Gfx::AffineTransform const&) const override;
@@ -46,6 +47,7 @@ public:
 
 private:
     PathImplSkia();
+    PathImplSkia(PathImplSkia const& other);
 
     Gfx::FloatPoint m_last_move_to;
     NonnullOwnPtr<SkPath> m_path;

Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -1457,15 +1457,15 @@ RefPtr<CSSStyleValue> Parser::parse_basic_shape_value(TokenStream<ComponentValue
         if (arguments.size() < 1)
             return nullptr;
 
-        Optional<FillRule> fill_rule;
+        Optional<Gfx::WindingRule> fill_rule;
         auto first_argument = arguments[0];
         TokenStream first_argument_tokens { first_argument };
 
         first_argument_tokens.discard_whitespace();
         if (first_argument_tokens.next_token().is_ident("nonzero"sv)) {
-            fill_rule = FillRule::Nonzero;
+            fill_rule = Gfx::WindingRule::Nonzero;
         } else if (first_argument_tokens.next_token().is_ident("evenodd"sv)) {
-            fill_rule = FillRule::Evenodd;
+            fill_rule = Gfx::WindingRule::EvenOdd;
         }
 
         if (fill_rule.has_value()) {
@@ -1475,7 +1475,7 @@ RefPtr<CSSStyleValue> Parser::parse_basic_shape_value(TokenStream<ComponentValue
 
             arguments.remove(0);
         } else {
-            fill_rule = FillRule::Nonzero;
+            fill_rule = Gfx::WindingRule::Nonzero;
         }
 
         if (arguments.size() < 1)

Userland/Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.cpp

@@ -176,6 +176,7 @@ String Ellipse::to_string() const
 Gfx::Path Polygon::to_path(CSSPixelRect reference_box, Layout::Node const& node) const
 {
     Gfx::Path path;
+    path.set_fill_type(fill_rule);
     bool first = true;
     for (auto const& point : points) {
         Gfx::FloatPoint resolved_point {
@@ -196,12 +197,15 @@ String Polygon::to_string() const
 {
     StringBuilder builder;
     builder.append("polygon("sv);
-    bool first = true;
+    switch (fill_rule) {
+    case Gfx::WindingRule::Nonzero:
+        builder.append("nonzero"sv);
+        break;
+    case Gfx::WindingRule::EvenOdd:
+        builder.append("evenodd"sv);
+    }
     for (auto const& point : points) {
-        if (!first)
+        builder.appendff(", {} {}", point.x, point.y);
-            builder.append(',');
-        builder.appendff("{} {}", point.x, point.y);
-        first = false;
     }
     builder.append(')');
     return MUST(builder.to_string());

Userland/Libraries/LibWeb/CSS/StyleValues/BasicShapeStyleValue.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/Variant.h>
+#include <LibGfx/WindingRule.h>
 #include <LibWeb/CSS/CSSStyleValue.h>
 #include <LibWeb/CSS/LengthBox.h>
 #include <LibWeb/CSS/PercentageOr.h>
@@ -84,8 +85,7 @@ struct Polygon {
 
     bool operator==(Polygon const&) const = default;
 
-    // FIXME: Actually use the fill rule
+    Gfx::WindingRule fill_rule;
-    FillRule fill_rule;
     Vector<Point> points;
 };