From a41d5861179165fab099719f3ed7026e649c7f05 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sat, 2 Aug 2025 21:16:51 +0200 Subject: [PATCH] LibWeb: Merge FillPathUsingPaintStyle and FillPathUsingColor Use `Variant` in new `FillPath` instead of duplicating two almost identical display list items. --- Libraries/LibWeb/Painting/BorderPainting.cpp | 4 +-- .../LibWeb/Painting/CheckBoxPaintable.cpp | 3 ++- Libraries/LibWeb/Painting/DisplayList.cpp | 3 +-- Libraries/LibWeb/Painting/DisplayList.h | 3 +-- .../LibWeb/Painting/DisplayListCommand.cpp | 9 ++----- .../LibWeb/Painting/DisplayListCommand.h | 26 +++---------------- .../LibWeb/Painting/DisplayListPlayerSkia.cpp | 26 ++++++++----------- .../LibWeb/Painting/DisplayListPlayerSkia.h | 3 +-- .../LibWeb/Painting/DisplayListRecorder.cpp | 26 ++++--------------- .../LibWeb/Painting/DisplayListRecorder.h | 17 ++++-------- Libraries/LibWeb/Painting/MarkerPaintable.cpp | 4 +-- Libraries/LibWeb/Painting/MediaPaintable.cpp | 4 +-- .../LibWeb/Painting/SVGPathPaintable.cpp | 8 +++--- .../display_list/simple-overflow-hidden.txt | 2 +- 14 files changed, 42 insertions(+), 96 deletions(-) diff --git a/Libraries/LibWeb/Painting/BorderPainting.cpp b/Libraries/LibWeb/Painting/BorderPainting.cpp index 70a307cc152..ff03560aca4 100644 --- a/Libraries/LibWeb/Painting/BorderPainting.cpp +++ b/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, - .color = color, - .winding_rule = Gfx::WindingRule::EvenOdd }); + painter.fill_path({ .path = path, .paint_style_or_color = color, .winding_rule = Gfx::WindingRule::EvenOdd }); path.clear(); } }; diff --git a/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp b/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp index b2c295b6a7f..06745f762bd 100644 --- a/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp +++ b/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp @@ -11,6 +11,7 @@ #include #include #include +#include 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(), }); } else { diff --git a/Libraries/LibWeb/Painting/DisplayList.cpp b/Libraries/LibWeb/Painting/DisplayList.cpp index c4d4e921a92..f4734956569 100644 --- a/Libraries/LibWeb/Painting/DisplayList.cpp +++ b/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(FillPathUsingPaintStyle, fill_path_using_paint_style) + else HANDLE_COMMAND(FillPath, fill_path) else HANDLE_COMMAND(StrokePath, stroke_path) else HANDLE_COMMAND(DrawEllipse, draw_ellipse) else HANDLE_COMMAND(FillEllipse, fill_ellipse) diff --git a/Libraries/LibWeb/Painting/DisplayList.h b/Libraries/LibWeb/Painting/DisplayList.h index 614e0522a4c..b8b4424d1d3 100644 --- a/Libraries/LibWeb/Painting/DisplayList.h +++ b/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_using_paint_style(FillPathUsingPaintStyle const&) = 0; + virtual void fill_path(FillPath const&) = 0; virtual void stroke_path(StrokePath const&) = 0; virtual void draw_ellipse(DrawEllipse const&) = 0; virtual void fill_ellipse(FillEllipse const&) = 0; diff --git a/Libraries/LibWeb/Painting/DisplayListCommand.cpp b/Libraries/LibWeb/Painting/DisplayListCommand.cpp index fca042c9293..14bdd9b94ff 100644 --- a/Libraries/LibWeb/Painting/DisplayListCommand.cpp +++ b/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"); -} - -void FillPathUsingPaintStyle::dump(StringBuilder& builder) const -{ - builder.appendff("FillPathUsingPaintStyle"); + builder.appendff("FillPath"); } void StrokePath::dump(StringBuilder& builder) const diff --git a/Libraries/LibWeb/Painting/DisplayListCommand.h b/Libraries/LibWeb/Painting/DisplayListCommand.h index 1689306bed7..b38ea109747 100644 --- a/Libraries/LibWeb/Painting/DisplayListCommand.h +++ b/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()); - } - 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, - FillPathUsingPaintStyle, + FillPath, StrokePath, DrawEllipse, FillEllipse, diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp index c91d9a8e4d4..b2df2b1898a 100644 --- a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp +++ b/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()); + + SkPaint paint; + if (command.paint_style_or_color.has()) { + auto const& paint_style = command.paint_style_or_color.get(); + paint = paint_style_to_skia_paint(*paint_style, command.bounding_rect().to_type()); + paint.setAlphaf(command.opacity); + } else { + auto const& color = command.paint_style_or_color.get(); + paint.setColor(to_skia_color(color)); + } paint.setAntiAlias(true); - paint.setAlphaf(command.opacity); surface().canvas().drawPath(path, paint); } diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h index fc6a26b2429..b4bfae81d8c 100644 --- a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.h +++ b/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_using_paint_style(FillPathUsingPaintStyle const&) override; + void fill_path(FillPath const&) override; void stroke_path(StrokePath const&) override; void draw_ellipse(DrawEllipse const&) override; void fill_ellipse(FillEllipse const&) override; diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.cpp b/Libraries/LibWeb/Painting/DisplayListRecorder.cpp index 7c36c86e3a0..4aa296e32b1 100644 --- a/Libraries/LibWeb/Painting/DisplayListRecorder.cpp +++ b/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() && params.paint_style_or_color.get().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, }); } diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.h b/Libraries/LibWeb/Painting/DisplayListRecorder.h index d401972c943..5721b752298 100644 --- a/Libraries/LibWeb/Painting/DisplayListRecorder.h +++ b/Libraries/LibWeb/Painting/DisplayListRecorder.h @@ -24,6 +24,7 @@ #include #include #include +#include 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 translation = {}; }; - void fill_path(FillPathUsingColorParams params); - - struct FillPathUsingPaintStyleParams { - Gfx::Path path; - PaintStyle paint_style; - Gfx::WindingRule winding_rule = Gfx::WindingRule::EvenOdd; - float opacity; - Optional translation = {}; - }; - void fill_path(FillPathUsingPaintStyleParams params); + void fill_path(FillPathParams params); struct StrokePathParams { Gfx::Path::CapStyle cap_style; diff --git a/Libraries/LibWeb/Painting/MarkerPaintable.cpp b/Libraries/LibWeb/Painting/MarkerPaintable.cpp index 804085c1074..c25c9d557a4 100644 --- a/Libraries/LibWeb/Painting/MarkerPaintable.cpp +++ b/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: diff --git a/Libraries/LibWeb/Painting/MediaPaintable.cpp b/Libraries/LibWeb/Painting/MediaPaintable.cpp index 8ed489b20d8..919cb47a0fd 100644 --- a/Libraries/LibWeb/Painting/MediaPaintable.cpp +++ b/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)); diff --git a/Libraries/LibWeb/Painting/SVGPathPaintable.cpp b/Libraries/LibWeb/Painting/SVGPathPaintable.cpp index ebd71941065..cbe02fd1433 100644 --- a/Libraries/LibWeb/Painting/SVGPathPaintable.cpp +++ b/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, }); diff --git a/Tests/LibWeb/Text/expected/display_list/simple-overflow-hidden.txt b/Tests/LibWeb/Text/expected/display_list/simple-overflow-hidden.txt index a0182d2faff..8587af53ac3 100644 --- a/Tests/LibWeb/Text/expected/display_list/simple-overflow-hidden.txt +++ b/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