Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-06-06 16:45:39 +00:00
Code Issues Projects Releases Packages Wiki Activity

layout: Fix painting order of collapsed table borders (#35219)

Browse source 
In #35075 I painted them in front of the decorations (backgrounds and
borders) of any block-level box in the same stacking context. I did that
because other browsers paint them in front of the decorations of the
descendants of the table, but my approach also painted them in front of
decorations of following siblings of the table, which was wrong.

This patch makes it so that collapsed table orders are painted in the
same step as decorations. However, tables defer painting their collapsed
borders after the decorations of their descendants.

This matches Blink and WebKit, and brings us closer to Gecko (which is
slightly different in some corner cases).

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
This commit is contained in:
Oriol Brufau 2025-02-03 15:46:40 +01:00 • committed by GitHub
parent 2030b7affd
commit 7301af8468
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 461 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

23
components/layout_2020/display_list/mod.rs
View file

@ -245,6 +245,7 @@ impl Fragment {
containing_block: &PhysicalRect<Au>,
section: StackingContextSection,
is_hit_test_for_scrollable_overflow: bool,
is_collapsed_table_borders: bool,
) {
match self {
Fragment::Box(box_fragment) | Fragment::Float(box_fragment) => {
@ -254,6 +255,7 @@ impl Fragment {
box_fragment,
containing_block,
is_hit_test_for_scrollable_overflow,
is_collapsed_table_borders,
)
.build(builder, section),
Visibility::Hidden => (),
@ -515,6 +517,7 @@ struct BuilderForBoxFragment<'a> {
padding_edge_clip_chain_id: RefCell<Option<ClipChainId>>,
content_edge_clip_chain_id: RefCell<Option<ClipChainId>>,
is_hit_test_for_scrollable_overflow: bool,
is_collapsed_table_borders: bool,
}
impl<'a> BuilderForBoxFragment<'a> {
@ -522,6 +525,7 @@ impl<'a> BuilderForBoxFragment<'a> {
fragment: &'a BoxFragment,
containing_block: &'a PhysicalRect<Au>,
is_hit_test_for_scrollable_overflow: bool,
is_collapsed_table_borders: bool,
) -> Self {
let border_rect = fragment
.border_rect()
@ -562,6 +566,7 @@ impl<'a> BuilderForBoxFragment<'a> {
padding_edge_clip_chain_id: RefCell::new(None),
content_edge_clip_chain_id: RefCell::new(None),
is_hit_test_for_scrollable_overflow,
is_collapsed_table_borders,
}
}
@ -652,16 +657,14 @@ impl<'a> BuilderForBoxFragment<'a> {
return;
}
match section {
StackingContextSection::CollapsedTableBorders => {
self.build_collapsed_table_borders(builder);
return;
},
StackingContextSection::Outline => {
self.build_outline(builder);
return;
},
_ => {},
if self.is_collapsed_table_borders {
self.build_collapsed_table_borders(builder);
return;
}
if section == StackingContextSection::Outline {
self.build_outline(builder);
return;
}
self.build_hit_test(builder, self.border_rect);

68
components/layout_2020/display_list/stacking_context.rs
View file

@ -88,7 +88,6 @@ pub(crate) type ContainingBlockInfo<'a> = ContainingBlockManager<'a, ContainingB
pub(crate) enum StackingContextSection {
OwnBackgroundsAndBorders,
DescendantBackgroundsAndBorders,
CollapsedTableBorders,
Foreground,
Outline,
}
@ -262,6 +261,7 @@ pub(crate) enum StackingContextContent {
containing_block: PhysicalRect<Au>,
fragment: Fragment,
is_hit_test_for_scrollable_overflow: bool,
is_collapsed_table_borders: bool,
},
/// An index into [StackingContext::atomic_inline_stacking_containers].
@ -292,6 +292,7 @@ impl StackingContextContent {
containing_block,
fragment,
is_hit_test_for_scrollable_overflow,
is_collapsed_table_borders,
} => {
builder.current_scroll_node_id = *scroll_node_id;
builder.current_reference_frame_scroll_node_id = *reference_frame_scroll_node_id;
@ -301,6 +302,7 @@ impl StackingContextContent {
containing_block,
*section,
*is_hit_test_for_scrollable_overflow,
*is_collapsed_table_borders,
);
},
Self::AtomicInlineStackingContainer { index } => {
@ -669,8 +671,12 @@ impl StackingContext {
}
}
let mut fragment_builder =
BuilderForBoxFragment::new(box_fragment, containing_block, false);
let mut fragment_builder = BuilderForBoxFragment::new(
box_fragment,
containing_block,
false, /* is_hit_test_for_scrollable_overflow */
false, /* is_collapsed_table_borders */
);
let painter = super::background::BackgroundPainter {
style,
painting_area_override: Some(painting_area),
@ -728,16 +734,6 @@ impl StackingContext {
child.build_display_list(builder, &self.atomic_inline_stacking_containers);
}
// Additional step 4.5: Collapsed table borders
// This step isn't in the spec, but other browsers seem to paint them at this point.
while contents.peek().is_some_and(|(_, child)| {
child.section() == StackingContextSection::CollapsedTableBorders
}) {
let (i, child) = contents.next().unwrap();
self.debug_push_print_item(DebugPrintField::Contents, i);
child.build_display_list(builder, &self.atomic_inline_stacking_containers);
}
// Step 5: Float stacking containers
for (i, child) in self.float_stacking_containers.iter().enumerate() {
self.debug_push_print_item(DebugPrintField::FloatStackingContainers, i);
@ -928,6 +924,7 @@ impl Fragment {
containing_block: containing_block.rect,
fragment: fragment_clone,
is_hit_test_for_scrollable_overflow: false,
is_collapsed_table_borders: false,
});
},
}
@ -1101,7 +1098,12 @@ impl BoxFragment {
display_list,
&containing_block.scroll_node_id,
&containing_block.clip_chain_id,
BuilderForBoxFragment::new(self, &containing_block.rect, false),
BuilderForBoxFragment::new(
self,
&containing_block.rect,
false, /* is_hit_test_for_scrollable_overflow */
false, /* is_collapsed_table_borders */
),
)
.unwrap_or(containing_block.clip_chain_id);
@ -1173,7 +1175,12 @@ impl BoxFragment {
display_list,
&new_scroll_node_id,
&new_clip_chain_id,
BuilderForBoxFragment::new(self, &containing_block.rect, false),
BuilderForBoxFragment::new(
self,
&containing_block.rect,
false, /* is_hit_test_for_scrollable_overflow*/
false, /* is_collapsed_table_borders */
),
) {
new_clip_chain_id = clip_chain_id;
}
@ -1205,20 +1212,11 @@ impl BoxFragment {
containing_block: containing_block.rect,
fragment: fragment.clone(),
is_hit_test_for_scrollable_overflow: false,
is_collapsed_table_borders: false,
});
};
add_fragment(self.get_stacking_context_section());
if let Fragment::Box(box_fragment) = &fragment {
if matches!(
box_fragment.borrow().specific_layout_info,
Some(SpecificLayoutInfo::TableGridWithCollapsedBorders(_))
) {
add_fragment(StackingContextSection::CollapsedTableBorders);
}
}
if !self.style.get_outline().outline_width.is_zero() {
add_fragment(StackingContextSection::Outline);
}
@ -1245,6 +1243,7 @@ impl BoxFragment {
containing_block: containing_block.rect,
fragment: fragment.clone(),
is_hit_test_for_scrollable_overflow: true,
is_collapsed_table_borders: false,
});
}
@ -1293,6 +1292,25 @@ impl BoxFragment {
StackingContextBuildMode::SkipHoisted,
);
}
if matches!(&fragment, Fragment::Box(box_fragment) if matches!(
box_fragment.borrow().specific_layout_info,
Some(SpecificLayoutInfo::TableGridWithCollapsedBorders(_))
)) {
stacking_context
.contents
.push(StackingContextContent::Fragment {
scroll_node_id: new_scroll_node_id,
reference_frame_scroll_node_id: reference_frame_scroll_node_id_for_fragments,
clip_chain_id: new_clip_chain_id,
// TODO: should this use `self.get_stacking_context_section()`?
section: StackingContextSection::DescendantBackgroundsAndBorders,
containing_block: containing_block.rect,
fragment: fragment.clone(),
is_hit_test_for_scrollable_overflow: false,
is_collapsed_table_borders: true,
});
}
}
fn build_clip_frame_if_necessary(