From f544132fe88ac729ca5e0ad05f71758c586df166 Mon Sep 17 00:00:00 2001 From: Andi Gallo Date: Sat, 1 Jul 2023 03:06:21 +0000 Subject: [PATCH] LibWeb: Some improvements for painting of collapsed table borders Follow the specification in making the borders centered on the grid lines. This avoids visual bugs due to double-rendering of borders on either side of an edge and paves the way for a full implementation of the harmonization algorithm for collapsed borders. Currently, this still lacks complete handling of row and column spans. Also, the box model for cells still considers the full width of the internal borders instead of just half, as the specification requires. Some additional handling of rounding issues will be needed to avoid very subtle visual bugs. Despite these limitations, this improves the appearance of all the tables with collapsed borders I've tried while limiting the amount of change to something reasonable. --- Userland/Libraries/LibWeb/CMakeLists.txt | 1 + .../LibWeb/Layout/TableFormattingContext.cpp | 4 +- .../LibWeb/Layout/TableFormattingContext.h | 2 + .../LibWeb/Painting/StackingContext.cpp | 28 +- .../LibWeb/Painting/TableBordersPainting.cpp | 248 ++++++++++++++++++ .../LibWeb/Painting/TableBordersPainting.h | 15 ++ 6 files changed, 269 insertions(+), 29 deletions(-) create mode 100644 Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp create mode 100644 Userland/Libraries/LibWeb/Painting/TableBordersPainting.h diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index 36eca3d26d..84cba59122 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -475,6 +475,7 @@ set(SOURCES Painting/SVGTextPaintable.cpp Painting/ShadowPainting.cpp Painting/StackingContext.cpp + Painting/TableBordersPainting.cpp Painting/TextPaintable.cpp Painting/VideoPaintable.cpp PerformanceTimeline/EntryTypes.cpp diff --git a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp index 1f651143ae..bd621a4ab6 100644 --- a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.cpp @@ -857,7 +857,7 @@ void TableFormattingContext::position_cell_boxes() } } -static bool border_is_less_specific(const CSS::BorderData& a, const CSS::BorderData& b) +bool TableFormattingContext::border_is_less_specific(const CSS::BorderData& a, const CSS::BorderData& b) { // Implements criteria for steps 1, 2 and 3 of border conflict resolution algorithm. static HashMap const line_style_score = { @@ -900,7 +900,7 @@ static bool border_is_less_specific(const CSS::BorderData& a, const CSS::BorderD static const CSS::BorderData& winning_border_style(const CSS::BorderData& a, const CSS::BorderData& b) { - return border_is_less_specific(a, b) ? b : a; + return TableFormattingContext::border_is_less_specific(a, b) ? b : a; } const CSS::BorderData& TableFormattingContext::border_data_conflicting_edge(TableFormattingContext::ConflictingEdge const& conflicting_edge) diff --git a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h index afc9d1dd44..7802d22040 100644 --- a/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h +++ b/Userland/Libraries/LibWeb/Layout/TableFormattingContext.h @@ -32,6 +32,8 @@ public: return verify_cast(*table_box().containing_block()); } + static bool border_is_less_specific(const CSS::BorderData& a, const CSS::BorderData& b); + private: CSSPixels run_caption_layout(LayoutMode, CSS::CaptionSide); CSSPixels compute_capmin(); diff --git a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp index 1fc254d270..b591195c09 100644 --- a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace Web::Painting { @@ -72,33 +73,6 @@ static PaintPhase to_paint_phase(StackingContext::StackingContextPaintPhase phas } } -static void collect_cell_boxes_with_collapsed_borders(Vector& cell_boxes, Layout::Node const& box) -{ - box.for_each_child([&](auto& child) { - if (child.display().is_table_cell() && child.computed_values().border_collapse() == CSS::BorderCollapse::Collapse) { - VERIFY(is(child) && child.paintable()); - cell_boxes.append(static_cast(child).paintable_box()); - } else { - collect_cell_boxes_with_collapsed_borders(cell_boxes, child); - } - }); -} - -static void paint_table_collapsed_borders(PaintContext& context, Layout::Node const& box) -{ - Vector cell_boxes; - collect_cell_boxes_with_collapsed_borders(cell_boxes, box); - for (auto const cell_box : cell_boxes) { - auto borders_data = cell_box->override_borders_data().has_value() ? cell_box->override_borders_data().value() : BordersData { - .top = cell_box->box_model().border.top == 0 ? CSS::BorderData() : cell_box->computed_values().border_top(), - .right = cell_box->box_model().border.right == 0 ? CSS::BorderData() : cell_box->computed_values().border_right(), - .bottom = cell_box->box_model().border.bottom == 0 ? CSS::BorderData() : cell_box->computed_values().border_bottom(), - .left = cell_box->box_model().border.left == 0 ? CSS::BorderData() : cell_box->computed_values().border_left(), - }; - paint_all_borders(context, cell_box->absolute_border_box_rect(), cell_box->normalized_border_radii_data(), borders_data); - } -} - void StackingContext::paint_descendants(PaintContext& context, Layout::Node const& box, StackingContextPaintPhase phase) const { if (auto* paintable = box.paintable()) { diff --git a/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp new file mode 100644 index 0000000000..da18ca3d85 --- /dev/null +++ b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp @@ -0,0 +1,248 @@ +/* + * Copyright (c) 2023, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include + +struct CellCoordinates { + size_t row_index; + size_t column_index; + + bool operator==(CellCoordinates const& other) const + { + return row_index == other.row_index && column_index == other.column_index; + } +}; + +namespace AK { +template<> +struct Traits : public GenericTraits { + static unsigned hash(CellCoordinates const& key) { return pair_int_hash(key.row_index, key.column_index); } +}; +} + +namespace Web::Painting { + +static void collect_cell_boxes_with_collapsed_borders(Vector& cell_boxes, Layout::Node const& box) +{ + box.for_each_child([&](auto& child) { + if (child.display().is_table_cell() && child.computed_values().border_collapse() == CSS::BorderCollapse::Collapse) { + VERIFY(is(child) && child.paintable()); + cell_boxes.append(static_cast(child).paintable_box()); + } else { + collect_cell_boxes_with_collapsed_borders(cell_boxes, child); + } + }); +} + +enum class EdgeDirection { + Horizontal, + Vertical, +}; + +struct BorderEdgePaintingInfo { + DevicePixelRect rect; + CSS::BorderData border_data; + EdgeDirection direction; +}; + +static BorderEdgePaintingInfo make_right_cell_edge(PaintContext& context, CSSPixelRect const& right_cell_rect, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + auto device_right_cell_rect = context.rounded_device_rect(right_cell_rect); + DevicePixelRect right_border_rect = { + device_right_cell_rect.x() - context.enclosing_device_pixels(borders_data.right.width / 2), + cell_rect.y() - context.enclosing_device_pixels(borders_data.top.width / 2), + context.enclosing_device_pixels(borders_data.right.width), + max(cell_rect.height(), device_right_cell_rect.height()) + context.enclosing_device_pixels(borders_data.top.width / 2) + context.enclosing_device_pixels(borders_data.bottom.width / 2), + }; + return BorderEdgePaintingInfo { + .rect = right_border_rect, + .border_data = borders_data.right, + .direction = EdgeDirection::Vertical, + }; +} + +static BorderEdgePaintingInfo make_down_cell_edge(PaintContext& context, CSSPixelRect const& down_cell_rect, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + auto device_down_cell_rect = context.rounded_device_rect(down_cell_rect); + DevicePixelRect down_border_rect = { + cell_rect.x() - context.enclosing_device_pixels(borders_data.left.width / 2), + device_down_cell_rect.y() - context.enclosing_device_pixels(borders_data.bottom.width / 2), + max(cell_rect.width(), device_down_cell_rect.width()) + context.enclosing_device_pixels(borders_data.left.width / 2) + context.enclosing_device_pixels(borders_data.right.width / 2), + context.enclosing_device_pixels(borders_data.bottom.width), + }; + return BorderEdgePaintingInfo { + .rect = down_border_rect, + .border_data = borders_data.bottom, + .direction = EdgeDirection::Horizontal, + }; +} + +static BorderEdgePaintingInfo make_first_row_top_cell_edge(PaintContext& context, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + DevicePixelRect top_border_rect = { + cell_rect.x() - context.enclosing_device_pixels(borders_data.left.width / 2), + cell_rect.y() - context.enclosing_device_pixels(borders_data.top.width / 2), + cell_rect.width(), + context.enclosing_device_pixels(borders_data.top.width), + }; + return BorderEdgePaintingInfo { + .rect = top_border_rect, + .border_data = borders_data.top, + .direction = EdgeDirection::Horizontal, + }; +} + +static BorderEdgePaintingInfo make_last_row_bottom_cell_edge(PaintContext& context, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + DevicePixelRect bottom_border_rect = { + cell_rect.x() - context.enclosing_device_pixels(borders_data.left.width / 2), + cell_rect.y() + cell_rect.height() - context.enclosing_device_pixels(borders_data.bottom.width / 2), + cell_rect.width() + context.enclosing_device_pixels(borders_data.left.width / 2) + context.enclosing_device_pixels(borders_data.right.width / 2), + context.enclosing_device_pixels(borders_data.bottom.width), + }; + return BorderEdgePaintingInfo { + .rect = bottom_border_rect, + .border_data = borders_data.bottom, + .direction = EdgeDirection::Horizontal, + }; +} + +static BorderEdgePaintingInfo make_first_column_left_cell_edge(PaintContext& context, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + DevicePixelRect left_border_rect = { + cell_rect.x() - context.enclosing_device_pixels(borders_data.left.width / 2), + cell_rect.y() - context.enclosing_device_pixels(borders_data.top.width / 2), + context.enclosing_device_pixels(borders_data.left.width), + cell_rect.height(), + }; + return BorderEdgePaintingInfo { + .rect = left_border_rect, + .border_data = borders_data.left, + .direction = EdgeDirection::Vertical, + }; +} + +static BorderEdgePaintingInfo make_last_column_right_cell_edge(PaintContext& context, DevicePixelRect const& cell_rect, BordersData const& borders_data) +{ + DevicePixelRect right_border_rect = { + cell_rect.x() + cell_rect.width() - context.enclosing_device_pixels(borders_data.right.width / 2), + cell_rect.y() - context.enclosing_device_pixels(borders_data.top.width / 2), + context.enclosing_device_pixels(borders_data.right.width), + cell_rect.height() + context.enclosing_device_pixels(borders_data.top.width / 2) + context.enclosing_device_pixels(borders_data.bottom.width / 2), + }; + return BorderEdgePaintingInfo { + .rect = right_border_rect, + .border_data = borders_data.right, + .direction = EdgeDirection::Vertical, + }; +} + +static void paint_collected_edges(PaintContext& context, Vector& border_edge_painting_info_list) +{ + // This sorting step isn't part of the specification, but it matches the behavior of other browsers at border intersections, which aren't + // part of border conflict resolution in the specification but it's still desirable to handle them in a way which is consistent with it. + quick_sort(border_edge_painting_info_list, [](auto const& a, auto const& b) { + return Layout::TableFormattingContext::border_is_less_specific(a.border_data, b.border_data); + }); + + for (auto const& border_edge_painting_info : border_edge_painting_info_list) { + auto const& border_data = border_edge_painting_info.border_data; + CSSPixels width = border_data.width; + if (width <= 0) + continue; + auto color = border_data.color; + auto border_style = border_data.line_style; + auto p1 = border_edge_painting_info.rect.top_left(); + auto p2 = border_edge_painting_info.direction == EdgeDirection::Horizontal + ? border_edge_painting_info.rect.top_right() + : border_edge_painting_info.rect.bottom_left(); + + if (border_style == CSS::LineStyle::Dotted) { + Gfx::AntiAliasingPainter aa_painter { context.painter() }; + aa_painter.draw_line(p1.to_type(), p2.to_type(), color, width.to_double(), Gfx::Painter::LineStyle::Dotted); + } else if (border_style == CSS::LineStyle::Dashed) { + context.painter().draw_line(p1.to_type(), p2.to_type(), color, width.to_double(), Gfx::Painter::LineStyle::Dashed); + } else { + // FIXME: Support the remaining line styles instead of rendering them as solid. + context.painter().fill_rect(Gfx::IntRect(border_edge_painting_info.rect.location(), border_edge_painting_info.rect.size()), color); + } + } +} + +void paint_table_collapsed_borders(PaintContext& context, Layout::Node const& box) +{ + // Partial implementation of painting according to the collapsing border model: + // https://www.w3.org/TR/CSS22/tables.html#collapsing-borders + Vector cell_boxes; + collect_cell_boxes_with_collapsed_borders(cell_boxes, box); + Vector border_edge_painting_info_list; + HashMap cell_coordinates_to_box; + size_t row_count = 0; + size_t column_count = 0; + for (auto const cell_box : cell_boxes) { + cell_coordinates_to_box.set(CellCoordinates { + .row_index = cell_box->table_cell_coordinates()->row_index, + .column_index = cell_box->table_cell_coordinates()->column_index }, + cell_box); + row_count = max(row_count, cell_box->table_cell_coordinates()->row_index + cell_box->table_cell_coordinates()->row_span); + column_count = max(column_count, cell_box->table_cell_coordinates()->column_index + cell_box->table_cell_coordinates()->column_span); + } + for (auto const cell_box : cell_boxes) { + auto borders_data = cell_box->override_borders_data().has_value() ? cell_box->override_borders_data().value() : BordersData { + .top = cell_box->box_model().border.top == 0 ? CSS::BorderData() : cell_box->computed_values().border_top(), + .right = cell_box->box_model().border.right == 0 ? CSS::BorderData() : cell_box->computed_values().border_right(), + .bottom = cell_box->box_model().border.bottom == 0 ? CSS::BorderData() : cell_box->computed_values().border_bottom(), + .left = cell_box->box_model().border.left == 0 ? CSS::BorderData() : cell_box->computed_values().border_left(), + }; + auto cell_rect = context.rounded_device_rect(cell_box->absolute_border_box_rect()); + auto maybe_right_cell = cell_coordinates_to_box.get(CellCoordinates { + .row_index = cell_box->table_cell_coordinates()->row_index, + .column_index = cell_box->table_cell_coordinates()->column_index + cell_box->table_cell_coordinates()->column_span }); + auto maybe_down_cell = cell_coordinates_to_box.get(CellCoordinates { + .row_index = cell_box->table_cell_coordinates()->row_index + cell_box->table_cell_coordinates()->row_span, + .column_index = cell_box->table_cell_coordinates()->column_index }); + if (maybe_right_cell.has_value()) + border_edge_painting_info_list.append(make_right_cell_edge(context, maybe_right_cell.value()->absolute_border_box_rect(), cell_rect, borders_data)); + if (maybe_down_cell.has_value()) + border_edge_painting_info_list.append(make_down_cell_edge(context, maybe_down_cell.value()->absolute_border_box_rect(), cell_rect, borders_data)); + if (cell_box->table_cell_coordinates()->row_index == 0) + border_edge_painting_info_list.append(make_first_row_top_cell_edge(context, cell_rect, borders_data)); + if (cell_box->table_cell_coordinates()->row_index + cell_box->table_cell_coordinates()->row_span == row_count) + border_edge_painting_info_list.append(make_last_row_bottom_cell_edge(context, cell_rect, borders_data)); + if (cell_box->table_cell_coordinates()->column_index == 0) + border_edge_painting_info_list.append(make_first_column_left_cell_edge(context, cell_rect, borders_data)); + if (cell_box->table_cell_coordinates()->column_index + cell_box->table_cell_coordinates()->column_span == column_count) + border_edge_painting_info_list.append(make_last_column_right_cell_edge(context, cell_rect, borders_data)); + } + + paint_collected_edges(context, border_edge_painting_info_list); + + for (auto const cell_box : cell_boxes) { + auto const& border_radii_data = cell_box->normalized_border_radii_data(); + auto top_left = border_radii_data.top_left.as_corner(context); + auto top_right = border_radii_data.top_right.as_corner(context); + auto bottom_right = border_radii_data.bottom_right.as_corner(context); + auto bottom_left = border_radii_data.bottom_left.as_corner(context); + if (!top_left && !top_right && !bottom_left && !bottom_right) { + continue; + } else { + auto borders_data = cell_box->override_borders_data().has_value() ? cell_box->override_borders_data().value() : BordersData { + .top = cell_box->box_model().border.top == 0 ? CSS::BorderData() : cell_box->computed_values().border_top(), + .right = cell_box->box_model().border.right == 0 ? CSS::BorderData() : cell_box->computed_values().border_right(), + .bottom = cell_box->box_model().border.bottom == 0 ? CSS::BorderData() : cell_box->computed_values().border_bottom(), + .left = cell_box->box_model().border.left == 0 ? CSS::BorderData() : cell_box->computed_values().border_left(), + }; + paint_all_borders(context, cell_box->absolute_border_box_rect(), cell_box->normalized_border_radii_data(), borders_data); + } + } +} + +} diff --git a/Userland/Libraries/LibWeb/Painting/TableBordersPainting.h b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.h new file mode 100644 index 0000000000..9880cb5df0 --- /dev/null +++ b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.h @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2023, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Web::Painting { + +void paint_table_collapsed_borders(PaintContext&, Layout::Node const&); + +}