LibWeb+LibGfx: Remove Path::close_all_subpaths()

As it turns out, SkPath already behaves the way we need for SVG and HTML
canvas elements. Less work for us, yay! This removes a 5% item from the
profile when scrolling on https://imdb.com/

Note that there's a tiny screenshot test expectation change due to
minor antialiasing differences when we no longer do our redundant
subpath modifications.
This commit is contained in:
Andreas Kling 2025-09-24 13:07:37 +02:00 committed by Andreas Kling
parent 989f6ddb42
commit 321809320b
10 changed files with 8 additions and 79 deletions

View File

@ -477,7 +477,6 @@ void TinyVGDecodedImageData::draw(Painter& painter) const
auto draw_path = command.path;
if (command.fill.has_value()) {
auto fill_path = draw_path;
fill_path.close_all_subpaths();
command.fill->visit(
[&](Color color) { painter.fill_path(fill_path, color, WindingRule::EvenOdd); },
[&](NonnullRefPtr<SVGGradientPaintStyle> const& style) {

View File

@ -25,7 +25,6 @@ public:
virtual void clear() = 0;
virtual void move_to(Gfx::FloatPoint const&) = 0;
virtual void line_to(Gfx::FloatPoint const&) = 0;
virtual void close_all_subpaths() = 0;
virtual void close() = 0;
virtual void elliptical_arc_to(FloatPoint point, FloatSize radii, float x_axis_rotation, bool large_arc, bool sweep) = 0;
virtual void arc_to(FloatPoint point, float radius, bool large_arc, bool sweep) = 0;
@ -73,7 +72,6 @@ public:
void clear() { impl().clear(); }
void move_to(Gfx::FloatPoint const& point) { impl().move_to(point); }
void line_to(Gfx::FloatPoint const& point) { impl().line_to(point); }
void close_all_subpaths() { impl().close_all_subpaths(); }
void close() { impl().close(); }
enum class CapStyle {

View File

@ -57,51 +57,6 @@ void PathImplSkia::line_to(Gfx::FloatPoint const& point)
m_path->lineTo(point.x(), point.y());
}
void PathImplSkia::close_all_subpaths()
{
SkPath new_path;
SkPath::Iter iter(*m_path, false);
SkPoint points[4];
SkPath::Verb verb;
bool need_close = false;
while ((verb = iter.next(points)) != SkPath::kDone_Verb) {
switch (verb) {
case SkPath::kMove_Verb:
if (need_close) {
new_path.close();
}
new_path.moveTo(points[0]);
need_close = true;
break;
case SkPath::kLine_Verb:
new_path.lineTo(points[1]);
break;
case SkPath::kQuad_Verb:
new_path.quadTo(points[1], points[2]);
break;
case SkPath::kCubic_Verb:
new_path.cubicTo(points[1], points[2], points[3]);
break;
case SkPath::kClose_Verb:
new_path.close();
need_close = false;
break;
case SkPath::kConic_Verb:
new_path.conicTo(points[1], points[2], iter.conicWeight());
break;
case SkPath::kDone_Verb:
break;
}
}
if (need_close) {
new_path.close();
}
*m_path = new_path;
}
void PathImplSkia::close()
{
m_path->close();

View File

@ -21,7 +21,6 @@ public:
virtual void clear() override;
virtual void move_to(Gfx::FloatPoint const&) override;
virtual void line_to(Gfx::FloatPoint const&) override;
virtual void close_all_subpaths() override;
virtual void close() override;
virtual void elliptical_arc_to(FloatPoint point, FloatSize radii, float x_axis_rotation, bool large_arc, bool sweep) override;
virtual void arc_to(FloatPoint point, float radius, bool large_arc, bool sweep) override;

View File

@ -217,11 +217,6 @@ String Polygon::to_string(SerializationMode) const
Gfx::Path Path::to_path(CSSPixelRect, Layout::Node const&) const
{
auto result = path_instructions.to_gfx_path();
// The UA must close a path with an implicit closepath command ("z" or "Z") if it is not present in the string for
// properties that require a closed loop (such as shape-outside and clip-path).
// https://drafts.csswg.org/css-shapes/#funcdef-basic-shape-path
// FIXME: For now, all users want a closed path, so we'll always close it.
result.close_all_subpaths();
result.set_fill_type(fill_rule);
return result;
}

View File

@ -405,12 +405,10 @@ void CanvasRenderingContext2D::fill_internal(Gfx::Path const& path, Gfx::Winding
paint_shadow_for_fill_internal(path, winding_rule);
auto path_to_fill = path;
path_to_fill.close_all_subpaths();
auto& state = this->drawing_state();
painter->fill_path(path_to_fill, state.fill_style.to_gfx_paint_style(), state.filter, state.global_alpha, state.current_compositing_and_blending_operator, winding_rule);
painter->fill_path(path, state.fill_style.to_gfx_paint_style(), state.filter, state.global_alpha, state.current_compositing_and_blending_operator, winding_rule);
did_draw(path_to_fill.bounding_box());
did_draw(path.bounding_box());
}
void CanvasRenderingContext2D::fill(StringView fill_rule)
@ -674,7 +672,6 @@ void CanvasRenderingContext2D::clip_internal(Gfx::Path& path, Gfx::WindingRule w
if (!painter)
return;
path.close_all_subpaths();
painter->clip(path, winding_rule);
}
@ -968,9 +965,6 @@ void CanvasRenderingContext2D::paint_shadow_for_fill_internal(Gfx::Path const& p
if (!painter)
return;
auto path_to_fill = path;
path_to_fill.close_all_subpaths();
auto& state = this->drawing_state();
if (state.current_compositing_and_blending_operator == Gfx::CompositingAndBlendingOperator::Copy)
@ -981,11 +975,11 @@ void CanvasRenderingContext2D::paint_shadow_for_fill_internal(Gfx::Path const& p
Gfx::AffineTransform transform;
transform.translate(state.shadow_offset_x, state.shadow_offset_y);
painter->set_transform(transform);
painter->fill_path(path_to_fill, state.shadow_color.with_opacity(state.global_alpha), winding_rule, state.shadow_blur, state.current_compositing_and_blending_operator);
painter->fill_path(path, state.shadow_color.with_opacity(state.global_alpha), winding_rule, state.shadow_blur, state.current_compositing_and_blending_operator);
painter->restore();
did_draw(path_to_fill.bounding_box());
did_draw(path.bounding_box());
}
void CanvasRenderingContext2D::paint_shadow_for_stroke_internal(Gfx::Path const& path)

View File

@ -282,7 +282,6 @@ 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, .paint_style_or_color = color, .winding_rule = Gfx::WindingRule::EvenOdd });
path.clear();
}

View File

@ -84,16 +84,6 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
auto path = computed_path()->copy_transformed(paint_transform);
path.offset(offset);
// Fills are computed as though all subpaths are closed (https://svgwg.org/svg2-draft/painting.html#FillProperties)
auto closed_path = [&] {
// We need to fill the path before applying the stroke, however the filled
// path must be closed, whereas the stroke path may not necessary be closed.
// Copy the path and close it for filling, but use the previous path for stroke
auto copy = path;
copy.close_all_subpaths();
return copy;
};
auto svg_viewport = [&] {
if (maybe_view_box.has_value())
return Gfx::FloatRect { maybe_view_box->min_x, maybe_view_box->min_y, maybe_view_box->width, maybe_view_box->height };
@ -106,7 +96,7 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
// within a clipPath conceptually defines a 1-bit mask (with the possible exception of anti-aliasing along
// 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(),
.path = path,
.paint_style_or_color = Gfx::Color(Color::Black),
.winding_rule = to_gfx_winding_rule(graphics_element.clip_rule().value_or(SVG::ClipRule::Nonzero)),
.should_anti_alias = should_anti_alias(),
@ -125,7 +115,7 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
auto winding_rule = to_gfx_winding_rule(graphics_element.fill_rule().value_or(SVG::FillRule::Nonzero));
if (auto paint_style = graphics_element.fill_paint_style(paint_context); paint_style.has_value()) {
context.display_list_recorder().fill_path({
.path = closed_path(),
.path = path,
.opacity = fill_opacity,
.paint_style_or_color = *paint_style,
.winding_rule = winding_rule,
@ -133,7 +123,7 @@ void SVGPathPaintable::paint(DisplayListRecordingContext& context, PaintPhase ph
});
} else if (auto fill_color = graphics_element.fill_color(); fill_color.has_value()) {
context.display_list_recorder().fill_path({
.path = closed_path(),
.path = path,
.paint_style_or_color = fill_color->with_opacity(fill_opacity),
.winding_rule = winding_rule,
.should_anti_alias = should_anti_alias(),

Binary file not shown.

Before

Width:  |  Height:  |  Size: 5.1 KiB

After

Width:  |  Height:  |  Size: 5.1 KiB

View File

@ -1,6 +1,6 @@
<!DOCTYPE html>
<link rel="match" href="../expected/canvas-path-rect-ref.html" />
<meta name="fuzzy" content="maxDifference=0-1;totalPixels=0-148">
<meta name="fuzzy" content="maxDifference=0-1;totalPixels=0-158">
<style>
* {
margin: 0;