From 7bd00d6a42f255faed4f414ecc7f5d36be0a8a99 Mon Sep 17 00:00:00 2001 From: Andi Gallo Date: Thu, 3 Aug 2023 03:48:37 +0000 Subject: [PATCH] LibWeb: Snap table grid to device pixels in separate borders mode Build a grid snapped to device pixels and use it to construct the rectangles for the cell edges, same as for collapsed borders. This is especially important when border-spacing is set to 0 since it avoids gaps between adjacent cells which have borders set. --- .../LibWeb/Painting/BorderPainting.cpp | 4 +- .../LibWeb/Painting/BorderPainting.h | 2 +- .../LibWeb/Painting/InlinePaintable.cpp | 4 +- .../LibWeb/Painting/PaintableBox.cpp | 4 +- .../LibWeb/Painting/StackingContext.cpp | 8 +-- .../LibWeb/Painting/TableBordersPainting.cpp | 54 +++++++++++++------ .../LibWeb/Painting/TableBordersPainting.h | 2 +- 7 files changed, 49 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp b/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp index 94bb2f99ac4..2f89ae2453f 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp +++ b/Userland/Libraries/LibWeb/Painting/BorderPainting.cpp @@ -544,13 +544,11 @@ RefPtr get_cached_corner_bitmap(DevicePixelSize corners_size) return corner_bitmap; } -void paint_all_borders(PaintContext& context, CSSPixelRect const& bordered_rect, BorderRadiiData const& border_radii_data, BordersData const& borders_data) +void paint_all_borders(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii_data, BordersData const& borders_data) { if (borders_data.top.width <= 0 && borders_data.right.width <= 0 && borders_data.left.width <= 0 && borders_data.bottom.width <= 0) return; - auto border_rect = context.rounded_device_rect(bordered_rect); - 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); diff --git a/Userland/Libraries/LibWeb/Painting/BorderPainting.h b/Userland/Libraries/LibWeb/Painting/BorderPainting.h index adc0fbbec03..5a301dc795b 100644 --- a/Userland/Libraries/LibWeb/Painting/BorderPainting.h +++ b/Userland/Libraries/LibWeb/Painting/BorderPainting.h @@ -84,7 +84,7 @@ Optional borders_data_for_outline(Layout::Node const&, Color outlin RefPtr get_cached_corner_bitmap(DevicePixelSize corners_size); void paint_border(PaintContext& context, BorderEdge edge, DevicePixelRect const& rect, Gfx::AntiAliasingPainter::CornerRadius const& radius, Gfx::AntiAliasingPainter::CornerRadius const& opposite_radius, BordersData const& borders_data, Gfx::Path& path, bool last); -void paint_all_borders(PaintContext& context, CSSPixelRect const& bordered_rect, BorderRadiiData const& border_radii_data, BordersData const&); +void paint_all_borders(PaintContext& context, DevicePixelRect const& border_rect, BorderRadiiData const& border_radii_data, BordersData const&); Gfx::Color border_color(BorderEdge edge, BordersData const& borders_data); diff --git a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp index d1266b6d939..e7a65af2eb4 100644 --- a/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/InlinePaintable.cpp @@ -134,9 +134,9 @@ void InlinePaintable::paint(PaintContext& context, PaintPhase phase) const border_radii_data.inflate(outline_data->top.width + outline_offset_y, outline_data->right.width + outline_offset_x, outline_data->bottom.width + outline_offset_y, outline_data->left.width + outline_offset_x); borders_rect.inflate(outline_data->top.width + outline_offset_y, outline_data->right.width + outline_offset_x, outline_data->bottom.width + outline_offset_y, outline_data->left.width + outline_offset_x); - paint_all_borders(context, borders_rect, border_radii_data, *outline_data); + paint_all_borders(context, context.rounded_device_rect(borders_rect), border_radii_data, *outline_data); } else { - paint_all_borders(context, borders_rect, border_radii_data, borders_data); + paint_all_borders(context, context.rounded_device_rect(borders_rect), border_radii_data, borders_data); } return IterationDecision::Continue; diff --git a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp index b0800c0d8a1..feb34885783 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableBox.cpp @@ -241,7 +241,7 @@ void PaintableBox::paint(PaintContext& context, PaintPhase phase) const border_radius_data.inflate(outline_width + outline_offset_y, outline_width + outline_offset_x, outline_width + outline_offset_y, outline_width + outline_offset_x); borders_rect.inflate(outline_width + outline_offset_y, outline_width + outline_offset_x, outline_width + outline_offset_y, outline_width + outline_offset_x); - paint_all_borders(context, borders_rect, border_radius_data, borders_data.value()); + paint_all_borders(context, context.rounded_device_rect(borders_rect), border_radius_data, borders_data.value()); } } @@ -311,7 +311,7 @@ void PaintableBox::paint_border(PaintContext& context) const .bottom = box_model().border.bottom == 0 ? CSS::BorderData() : computed_values().border_bottom(), .left = box_model().border.left == 0 ? CSS::BorderData() : computed_values().border_left(), }; - paint_all_borders(context, absolute_border_box_rect(), normalized_border_radii_data(), borders_data); + paint_all_borders(context, context.rounded_device_rect(absolute_border_box_rect()), normalized_border_radii_data(), borders_data); } void PaintableBox::paint_backdrop_filter(PaintContext& context) const diff --git a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp index f616047656d..bc220659038 100644 --- a/Userland/Libraries/LibWeb/Painting/StackingContext.cpp +++ b/Userland/Libraries/LibWeb/Painting/StackingContext.cpp @@ -104,11 +104,13 @@ void StackingContext::paint_descendants(PaintContext& context, Layout::Node cons case StackingContextPaintPhase::BackgroundAndBorders: if (!child_is_inline_or_replaced && !child.is_floating()) { paint_node(child, context, PaintPhase::Background); - if ((!child.display().is_table_cell() && !child.display().is_table_inside()) || child.computed_values().border_collapse() == CSS::BorderCollapse::Separate) + bool is_table_with_collapsed_borders = child.display().is_table_inside() && child.computed_values().border_collapse() == CSS::BorderCollapse::Collapse; + if (!child.display().is_table_cell() && !is_table_with_collapsed_borders) paint_node(child, context, PaintPhase::Border); paint_descendants(context, child, phase); - if (child.computed_values().border_collapse() == CSS::BorderCollapse::Collapse) - paint_table_collapsed_borders(context, child); + if (child.display().is_table_inside() || child.computed_values().border_collapse() == CSS::BorderCollapse::Collapse) { + paint_table_borders(context, child); + } } break; case StackingContextPaintPhase::Floats: diff --git a/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp index df85b0d6c69..5535328e906 100644 --- a/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp +++ b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.cpp @@ -30,14 +30,14 @@ struct Traits : public GenericTraits { namespace Web::Painting { -static void collect_cell_boxes_with_collapsed_borders(Vector& cell_boxes, Layout::Node const& box) +static void collect_cell_boxes(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) { + if (child.display().is_table_cell()) { VERIFY(is(child) && child.paintable()); cell_boxes.append(static_cast(child).paintable_box()); } else { - collect_cell_boxes_with_collapsed_borders(cell_boxes, child); + collect_cell_boxes(cell_boxes, child); } }); } @@ -297,32 +297,36 @@ static void paint_collected_edges(PaintContext& context, Vector snap_cells_to_device_coordinates(HashMap const& cell_coordinates_to_box, size_t row_count, size_t column_count, PaintContext const& context) { - Vector y_line_coordinates; - y_line_coordinates.resize(row_count + 1); - Vector x_line_coordinates; - x_line_coordinates.resize(column_count + 1); + Vector y_line_start_coordinates; + Vector y_line_end_coordinates; + y_line_start_coordinates.resize(row_count + 1); + y_line_end_coordinates.resize(row_count + 1); + Vector x_line_start_coordinates; + Vector x_line_end_coordinates; + x_line_start_coordinates.resize(column_count + 1); + x_line_end_coordinates.resize(column_count + 1); for (auto const& kv : cell_coordinates_to_box) { auto const& cell_box = kv.value; auto start_row_index = cell_box->table_cell_coordinates()->row_index; auto end_row_index = start_row_index + cell_box->table_cell_coordinates()->row_span; auto cell_rect = cell_box->absolute_border_box_rect(); - y_line_coordinates[start_row_index] = max(context.rounded_device_pixels(cell_rect.y()), y_line_coordinates[start_row_index]); - y_line_coordinates[end_row_index] = max(context.rounded_device_pixels(cell_rect.y() + cell_rect.height()), y_line_coordinates[end_row_index]); + y_line_start_coordinates[start_row_index] = max(context.rounded_device_pixels(cell_rect.y()), y_line_start_coordinates[start_row_index]); + y_line_end_coordinates[end_row_index] = max(context.rounded_device_pixels(cell_rect.y() + cell_rect.height()), y_line_end_coordinates[end_row_index]); auto start_column_index = cell_box->table_cell_coordinates()->column_index; auto end_column_index = start_column_index + cell_box->table_cell_coordinates()->column_span; - x_line_coordinates[start_column_index] = max(context.rounded_device_pixels(cell_rect.x()), x_line_coordinates[start_column_index]); - x_line_coordinates[end_column_index] = max(context.rounded_device_pixels(cell_rect.x() + cell_rect.width()), x_line_coordinates[end_column_index]); + x_line_start_coordinates[start_column_index] = max(context.rounded_device_pixels(cell_rect.x()), x_line_start_coordinates[start_column_index]); + x_line_end_coordinates[end_column_index] = max(context.rounded_device_pixels(cell_rect.x() + cell_rect.width()), x_line_end_coordinates[end_column_index]); } HashMap cell_coordinates_to_device_rect; for (auto const& kv : cell_coordinates_to_box) { auto const& cell_box = kv.value; auto start_row_index = cell_box->table_cell_coordinates()->row_index; auto end_row_index = start_row_index + cell_box->table_cell_coordinates()->row_span; - auto height = y_line_coordinates[end_row_index] - y_line_coordinates[start_row_index]; + auto height = y_line_end_coordinates[end_row_index] - y_line_start_coordinates[start_row_index]; auto start_column_index = cell_box->table_cell_coordinates()->column_index; auto end_column_index = start_column_index + cell_box->table_cell_coordinates()->column_span; - auto width = x_line_coordinates[end_column_index] - x_line_coordinates[start_column_index]; - cell_coordinates_to_device_rect.set(kv.key, DevicePixelRect { x_line_coordinates[start_column_index], y_line_coordinates[start_row_index], width, height }); + auto width = x_line_end_coordinates[end_column_index] - x_line_start_coordinates[start_column_index]; + cell_coordinates_to_device_rect.set(kv.key, DevicePixelRect { x_line_start_coordinates[start_column_index], y_line_start_coordinates[start_row_index], width, height }); } return cell_coordinates_to_device_rect; } @@ -339,12 +343,24 @@ static DeviceBorderDataWithElementKind device_border_data_from_css_border_data(P }; } -void paint_table_collapsed_borders(PaintContext& context, Layout::Node const& box) +static void paint_separate_cell_borders(PaintableBox const* cell_box, HashMap const& cell_coordinates_to_device_rect, PaintContext& context) +{ + auto borders_data = cell_box->override_borders_data().has_value() ? PaintableBox::remove_element_kind_from_borders_data(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 = cell_coordinates_to_device_rect.get({ cell_box->table_cell_coordinates()->row_index, cell_box->table_cell_coordinates()->column_index }).value(); + paint_all_borders(context, cell_rect, cell_box->normalized_border_radii_data(), borders_data); +} + +void paint_table_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); + collect_cell_boxes(cell_boxes, box); Vector border_edge_painting_info_list; HashMap cell_coordinates_to_box; size_t row_count = 0; @@ -359,6 +375,10 @@ void paint_table_collapsed_borders(PaintContext& context, Layout::Node const& bo } auto cell_coordinates_to_device_rect = snap_cells_to_device_coordinates(cell_coordinates_to_box, row_count, column_count, context); for (auto const cell_box : cell_boxes) { + if (cell_box->computed_values().border_collapse() == CSS::BorderCollapse::Separate) { + paint_separate_cell_borders(cell_box, cell_coordinates_to_device_rect, context); + continue; + } auto css_borders_data = cell_box->override_borders_data().has_value() ? cell_box->override_borders_data().value() : PaintableBox::BordersDataWithElementKind { .top = { .border_data = cell_box->box_model().border.top == 0 ? CSS::BorderData() : cell_box->computed_values().border_top(), .element_kind = PaintableBox::ConflictingElementKind::Cell }, .right = { .border_data = cell_box->box_model().border.right == 0 ? CSS::BorderData() : cell_box->computed_values().border_right(), .element_kind = PaintableBox::ConflictingElementKind::Cell }, @@ -417,7 +437,7 @@ void paint_table_collapsed_borders(PaintContext& context, Layout::Node const& bo .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); + paint_all_borders(context, context.rounded_device_rect(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 index 9880cb5df00..fd43205af52 100644 --- a/Userland/Libraries/LibWeb/Painting/TableBordersPainting.h +++ b/Userland/Libraries/LibWeb/Painting/TableBordersPainting.h @@ -10,6 +10,6 @@ namespace Web::Painting { -void paint_table_collapsed_borders(PaintContext&, Layout::Node const&); +void paint_table_borders(PaintContext&, Layout::Node const&); }