LibWeb: Merge FillPathUsingPaintStyle and FillPathUsingColor

Use `Variant<PaintStyle, Gfx::Color>` in new `FillPath` instead of
duplicating two almost identical display list items.

Libraries/LibWeb/Painting/BorderPainting.cpp

@@ -195,9 +195,7 @@ void paint_border(DisplayListRecorder& painter, BorderEdge edge, DevicePixelRect
         // If joined borders have the same color, combine them to draw together.
         if (ready_to_draw) {
             path.close_all_subpaths();
-            painter.fill_path({ .path = path,
+            painter.fill_path({ .path = path, .paint_style_or_color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
-                .color = color,
-                .winding_rule = Gfx::WindingRule::EvenOdd });
             path.clear();
         }
     };

Libraries/LibWeb/Painting/CheckBoxPaintable.cpp

@@ -11,6 +11,7 @@
 #include <LibWeb/Painting/CheckBoxPaintable.h>
 #include <LibWeb/Painting/DisplayListRecorder.h>
 #include <LibWeb/Painting/InputColors.h>
+#include <LibWeb/Painting/PaintStyle.h>
 
 namespace Web::Painting {
 
@@ -101,7 +102,7 @@ void CheckBoxPaintable::paint(DisplayListRecordingContext& context, PaintPhase p
             tick_color = shade(tick_color, 0.5f);
         context.display_list_recorder().fill_path({
             .path = check_mark_path(checkbox_rect),
-            .color = tick_color,
+            .paint_style_or_color = tick_color,
             .translation = checkbox_rect.location().to_type<float>(),
         });
     } else {

Libraries/LibWeb/Painting/DisplayList.cpp

@@ -214,8 +214,7 @@ void DisplayListPlayer::execute_impl(DisplayList& display_list, ScrollStateSnaps
         else HANDLE_COMMAND(PaintInnerBoxShadow, paint_inner_box_shadow)
         else HANDLE_COMMAND(PaintTextShadow, paint_text_shadow)
         else HANDLE_COMMAND(FillRectWithRoundedCorners, fill_rect_with_rounded_corners)
-        else HANDLE_COMMAND(FillPathUsingColor, fill_path_using_color)
+        else HANDLE_COMMAND(FillPath, fill_path)
-        else HANDLE_COMMAND(FillPathUsingPaintStyle, fill_path_using_paint_style)
         else HANDLE_COMMAND(StrokePath, stroke_path)
         else HANDLE_COMMAND(DrawEllipse, draw_ellipse)
         else HANDLE_COMMAND(FillEllipse, fill_ellipse)

Libraries/LibWeb/Painting/DisplayList.h

@@ -55,8 +55,7 @@ private:
     virtual void paint_inner_box_shadow(PaintInnerBoxShadow const&) = 0;
     virtual void paint_text_shadow(PaintTextShadow const&) = 0;
     virtual void fill_rect_with_rounded_corners(FillRectWithRoundedCorners const&) = 0;
-    virtual void fill_path_using_color(FillPathUsingColor const&) = 0;
+    virtual void fill_path(FillPath const&) = 0;
-    virtual void fill_path_using_paint_style(FillPathUsingPaintStyle const&) = 0;
     virtual void stroke_path(StrokePath const&) = 0;
     virtual void draw_ellipse(DrawEllipse const&) = 0;
     virtual void fill_ellipse(FillEllipse const&) = 0;

Libraries/LibWeb/Painting/DisplayListCommand.cpp

@@ -137,14 +137,9 @@ void FillRectWithRoundedCorners::dump(StringBuilder& builder) const
     builder.appendff("FillRectWithRoundedCorners rect={} color={}", rect, color);
 }
 
-void FillPathUsingColor::dump(StringBuilder& builder) const
+void FillPath::dump(StringBuilder& builder) const
 {
-    builder.appendff("FillPathUsingColor");
+    builder.appendff("FillPath");
-}
-
-void FillPathUsingPaintStyle::dump(StringBuilder& builder) const
-{
-    builder.appendff("FillPathUsingPaintStyle");
 }
 
 void StrokePath::dump(StringBuilder& builder) const

Libraries/LibWeb/Painting/DisplayListCommand.h

@@ -211,10 +211,11 @@ struct FillRectWithRoundedCorners {
     void dump(StringBuilder&) const;
 };
 
-struct FillPathUsingColor {
+struct FillPath {
     Gfx::IntRect path_bounding_rect;
     Gfx::Path path;
-    Color color;
+    float opacity { 1.0f };
+    PaintStyleOrColor paint_style_or_color;
     Gfx::WindingRule winding_rule;
     Gfx::FloatPoint aa_translation;
 
@@ -228,24 +229,6 @@ struct FillPathUsingColor {
     void dump(StringBuilder&) const;
 };
 
-struct FillPathUsingPaintStyle {
-    Gfx::IntRect path_bounding_rect;
-    Gfx::Path path;
-    PaintStyle paint_style;
-    Gfx::WindingRule winding_rule;
-    float opacity;
-    Gfx::FloatPoint aa_translation;
-
-    [[nodiscard]] Gfx::IntRect bounding_rect() const { return path_bounding_rect; }
-
-    void translate_by(Gfx::IntPoint const& offset)
-    {
-        path_bounding_rect.translate_by(offset);
-        aa_translation.translate_by(offset.to_type<float>());
-    }
-    void dump(StringBuilder&) const;
-};
-
 struct StrokePath {
     Gfx::Path::CapStyle cap_style;
     Gfx::Path::JoinStyle join_style;
@@ -484,8 +467,7 @@ using DisplayListCommand = Variant<
     PaintInnerBoxShadow,
     PaintTextShadow,
     FillRectWithRoundedCorners,
-    FillPathUsingColor,
+    FillPath,
-    FillPathUsingPaintStyle,
     StrokePath,
     DrawEllipse,
     FillEllipse,

Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp

@@ -575,18 +575,6 @@ void DisplayListPlayerSkia::fill_rect_with_rounded_corners(FillRectWithRoundedCo
     canvas.drawRRect(rounded_rect, paint);
 }
 
-void DisplayListPlayerSkia::fill_path_using_color(FillPathUsingColor const& command)
-{
-    auto& canvas = surface().canvas();
-    SkPaint paint;
-    paint.setAntiAlias(true);
-    paint.setColor(to_skia_color(command.color));
-    auto path = to_skia_path(command.path);
-    path.setFillType(to_skia_path_fill_type(command.winding_rule));
-    path.offset(command.aa_translation.x(), command.aa_translation.y());
-    canvas.drawPath(path, paint);
-}
-
 static SkTileMode to_skia_tile_mode(SVGLinearGradientPaintStyle::SpreadMethod spread_method)
 {
     switch (spread_method) {
@@ -649,14 +637,22 @@ static SkPaint paint_style_to_skia_paint(Painting::SVGGradientPaintStyle const&
     return paint;
 }
 
-void DisplayListPlayerSkia::fill_path_using_paint_style(FillPathUsingPaintStyle const& command)
+void DisplayListPlayerSkia::fill_path(FillPath const& command)
 {
     auto path = to_skia_path(command.path);
     path.offset(command.aa_translation.x(), command.aa_translation.y());
     path.setFillType(to_skia_path_fill_type(command.winding_rule));
-    auto paint = paint_style_to_skia_paint(*command.paint_style, command.bounding_rect().to_type<float>());
+
+    SkPaint paint;
+    if (command.paint_style_or_color.has<PaintStyle>()) {
+        auto const& paint_style = command.paint_style_or_color.get<PaintStyle>();
+        paint = paint_style_to_skia_paint(*paint_style, command.bounding_rect().to_type<float>());
+        paint.setAlphaf(command.opacity);
+    } else {
+        auto const& color = command.paint_style_or_color.get<Color>();
+        paint.setColor(to_skia_color(color));
+    }
     paint.setAntiAlias(true);
-    paint.setAlphaf(command.opacity);
     surface().canvas().drawPath(path, paint);
 }
 

Libraries/LibWeb/Painting/DisplayListPlayerSkia.h

@@ -40,8 +40,7 @@ private:
     void paint_inner_box_shadow(PaintInnerBoxShadow const&) override;
     void paint_text_shadow(PaintTextShadow const&) override;
     void fill_rect_with_rounded_corners(FillRectWithRoundedCorners const&) override;
-    void fill_path_using_color(FillPathUsingColor const&) override;
+    void fill_path(FillPath const&) override;
-    void fill_path_using_paint_style(FillPathUsingPaintStyle const&) override;
     void stroke_path(StrokePath const&) override;
     void draw_ellipse(DrawEllipse const&) override;
     void fill_ellipse(FillEllipse const&) override;

Libraries/LibWeb/Painting/DisplayListRecorder.cpp

@@ -64,37 +64,21 @@ void DisplayListRecorder::fill_rect(Gfx::IntRect const& rect, Color color)
     APPEND(FillRect { rect, color });
 }
 
-void DisplayListRecorder::fill_path(FillPathUsingColorParams params)
+void DisplayListRecorder::fill_path(FillPathParams params)
 {
-    if (params.color.alpha() == 0)
+    if (params.paint_style_or_color.has<Gfx::Color>() && params.paint_style_or_color.get<Gfx::Color>().alpha() == 0)
         return;
     auto aa_translation = params.translation.value_or(Gfx::FloatPoint {});
     auto path_bounding_rect = params.path.bounding_box().translated(aa_translation);
     auto path_bounding_int_rect = enclosing_int_rect(path_bounding_rect);
     if (path_bounding_int_rect.is_empty())
         return;
-    APPEND(FillPathUsingColor {
+    APPEND(FillPath {
         .path_bounding_rect = path_bounding_int_rect,
         .path = move(params.path),
-        .color = params.color,
-        .winding_rule = params.winding_rule,
-        .aa_translation = aa_translation,
-    });
-}
-
-void DisplayListRecorder::fill_path(FillPathUsingPaintStyleParams params)
-{
-    auto aa_translation = params.translation.value_or(Gfx::FloatPoint {});
-    auto path_bounding_rect = params.path.bounding_box().translated(aa_translation);
-    auto path_bounding_int_rect = enclosing_int_rect(path_bounding_rect);
-    if (path_bounding_int_rect.is_empty())
-        return;
-    APPEND(FillPathUsingPaintStyle {
-        .path_bounding_rect = path_bounding_int_rect,
-        .path = move(params.path),
-        .paint_style = params.paint_style,
-        .winding_rule = params.winding_rule,
         .opacity = params.opacity,
+        .paint_style_or_color = params.paint_style_or_color,
+        .winding_rule = params.winding_rule,
         .aa_translation = aa_translation,
     });
 }

Libraries/LibWeb/Painting/DisplayListRecorder.h

@@ -24,6 +24,7 @@
 #include <LibWeb/Painting/ClipFrame.h>
 #include <LibWeb/Painting/GradientData.h>
 #include <LibWeb/Painting/PaintBoxShadowParams.h>
+#include <LibWeb/Painting/PaintStyle.h>
 
 namespace Web::Painting {
 
@@ -39,22 +40,14 @@ class DisplayListRecorder {
 public:
     void fill_rect(Gfx::IntRect const& rect, Color color);
 
-    struct FillPathUsingColorParams {
+    struct FillPathParams {
         Gfx::Path path;
-        Gfx::Color color;
+        float opacity = 1.0f;
+        PaintStyleOrColor paint_style_or_color;
         Gfx::WindingRule winding_rule = Gfx::WindingRule::EvenOdd;
         Optional<Gfx::FloatPoint> translation = {};
     };
-    void fill_path(FillPathUsingColorParams params);
+    void fill_path(FillPathParams params);
-
-    struct FillPathUsingPaintStyleParams {
-        Gfx::Path path;
-        PaintStyle paint_style;
-        Gfx::WindingRule winding_rule = Gfx::WindingRule::EvenOdd;
-        float opacity;
-        Optional<Gfx::FloatPoint> translation = {};
-    };
-    void fill_path(FillPathUsingPaintStyleParams params);
 
     struct StrokePathParams {
         Gfx::Path::CapStyle cap_style;

Libraries/LibWeb/Painting/MarkerPaintable.cpp

@@ -84,7 +84,7 @@ void MarkerPaintable::paint(DisplayListRecordingContext& context, PaintPhase pha
             path.line_to({ left + sin_60_deg * (right - left), (top + bottom) / 2 });
             path.line_to({ left, bottom });
             path.close();
-            context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
+            context.display_list_recorder().fill_path({ .path = path, .paint_style_or_color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
             break;
         }
         case CSS::CounterStyleNameKeyword::DisclosureOpen: {
@@ -100,7 +100,7 @@ void MarkerPaintable::paint(DisplayListRecordingContext& context, PaintPhase pha
             path.line_to({ right, top });
             path.line_to({ (left + right) / 2, top + sin_60_deg * (bottom - top) });
             path.close();
-            context.display_list_recorder().fill_path({ .path = path, .color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
+            context.display_list_recorder().fill_path({ .path = path, .paint_style_or_color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
             break;
         }
         case CSS::CounterStyleNameKeyword::None:

Libraries/LibWeb/Painting/MediaPaintable.cpp

@@ -55,7 +55,7 @@ void MediaPaintable::fill_triangle(DisplayListRecorder& painter, Gfx::IntPoint l
     path.close();
     painter.fill_path({
         .path = path,
-        .color = color,
+        .paint_style_or_color = color,
         .winding_rule = Gfx::WindingRule::EvenOdd,
     });
 }
@@ -229,7 +229,7 @@ void MediaPaintable::paint_control_bar_speaker(DisplayListRecordingContext& cont
     path.line_to(device_point(0, 11));
     path.line_to(device_point(0, 4));
     path.close();
-    context.display_list_recorder().fill_path({ .path = path, .color = speaker_button_color, .winding_rule = Gfx::WindingRule::EvenOdd });
+    context.display_list_recorder().fill_path({ .path = path, .paint_style_or_color = speaker_button_color, .winding_rule = Gfx::WindingRule::EvenOdd });
 
     path.clear();
     path.move_to(device_point(13, 3));

Libraries/LibWeb/Painting/SVGPathPaintable.cpp

@@ -95,7 +95,7 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
         // the edge of the geometry) which represents the silhouette of the graphics associated with that element.
         context.display_list_recorder().fill_path({
             .path = closed_path(),
-            .color = Color::Black,
+            .paint_style_or_color = Gfx::Color(Color::Black),
             .winding_rule = to_gfx_winding_rule(graphics_element.clip_rule().value_or(SVG::ClipRule::Nonzero)),
             .translation = offset,
         });
@@ -113,15 +113,15 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
     if (auto paint_style = graphics_element.fill_paint_style(paint_context); paint_style.has_value()) {
         context.display_list_recorder().fill_path({
             .path = closed_path(),
-            .paint_style = *paint_style,
-            .winding_rule = winding_rule,
             .opacity = fill_opacity,
+            .paint_style_or_color = *paint_style,
+            .winding_rule = winding_rule,
             .translation = offset,
         });
     } else if (auto fill_color = graphics_element.fill_color(); fill_color.has_value()) {
         context.display_list_recorder().fill_path({
             .path = closed_path(),
-            .color = fill_color->with_opacity(fill_opacity),
+            .paint_style_or_color = fill_color->with_opacity(fill_opacity),
             .winding_rule = winding_rule,
             .translation = offset,
         });

Tests/LibWeb/Text/expected/display_list/simple-overflow-hidden.txt

@@ -1,7 +1,7 @@
 SaveLayer
   PushStackingContext opacity=1 isolate=false has_clip_path=false transform=[1 0 0 1 0 0]
     PushStackingContext opacity=1 isolate=false has_clip_path=false transform=[1 0 0 1 0 0]
-      FillPathUsingColor
+      FillPath
       FillRect rect=[10,10 300x150] color=rgb(240, 128, 128)
       DrawGlyphRun rect=[10,10 38x18] translation=[10,23.796875] color=rgb(0, 0, 0) scale=1
     PopStackingContext