Skip to content

Commit

Permalink
layout: Make Fragment hold ArcRefCell inside (servo#34923)
Browse files Browse the repository at this point in the history 
Push the interior mutability into enum variants of `Fragment`, so that
they can be cloned. This saves memory in the `Fragment` tree as the
`Fragment` enum is now a relatively wee 16 bytes and the interior parts
can be a variety of sizes. Before, every `Fragment` was the size of the
biggest kind (`BoxFragment` - 248 bytes).

This a step on the way toward incremental layout.

Signed-off-by: Martin Robinson <[email protected]>
Co-authored-by: Oriol Brufau <[email protected]>
  • Loading branch information
@mrobinson @Loirooriol
mrobinson and Loirooriol authored Jan 13, 2025
1 parent c936dd6 commit de780dc
Show file tree
Hide file tree
Showing 18 changed files with 259 additions and 235 deletions.
125 changes: 68 additions & 57 deletions components/layout_2020/display_list/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ impl Fragment {
) {
match self {
Fragment::Box(box_fragment) | Fragment::Float(box_fragment) => {
let box_fragment = &*box_fragment.borrow();
match box_fragment.style.get_inherited_box().visibility {
Visibility::Visible => BuilderForBoxFragment::new(
box_fragment,
Expand All @@ -259,6 +260,7 @@ impl Fragment {
},
Fragment::AbsoluteOrFixedPositioned(_) => {},
Fragment::Positioning(positioning_fragment) => {
let positioning_fragment = positioning_fragment.borrow();
if let Some(style) = positioning_fragment.style.as_ref() {
let rect = positioning_fragment
.rect
Expand All @@ -272,65 +274,74 @@ impl Fragment {
);
}
},
Fragment::Image(image) => match image.style.get_inherited_box().visibility {
Visibility::Visible => {
builder.is_contentful = true;

let image_rendering = image
.style
.get_inherited_box()
.image_rendering
.to_webrender();
let rect = image
.rect
.translate(containing_block.origin.to_vector())
.to_webrender();
let clip = image
.clip
.translate(containing_block.origin.to_vector())
.to_webrender();
let common = builder.common_properties(clip, &image.style);

if let Some(image_key) = image.image_key {
builder.wr().push_image(
&common,
rect,
image_rendering,
wr::AlphaType::PremultipliedAlpha,
image_key,
wr::ColorF::WHITE,
);
}
},
Visibility::Hidden => (),
Visibility::Collapse => (),
Fragment::Image(image) => {
let image = image.borrow();
match image.style.get_inherited_box().visibility {
Visibility::Visible => {
builder.is_contentful = true;

let image_rendering = image
.style
.get_inherited_box()
.image_rendering
.to_webrender();
let rect = image
.rect
.translate(containing_block.origin.to_vector())
.to_webrender();
let clip = image
.clip
.translate(containing_block.origin.to_vector())
.to_webrender();
let common = builder.common_properties(clip, &image.style);

if let Some(image_key) = image.image_key {
builder.wr().push_image(
&common,
rect,
image_rendering,
wr::AlphaType::PremultipliedAlpha,
image_key,
wr::ColorF::WHITE,
);
}
},
Visibility::Hidden => (),
Visibility::Collapse => (),
}
},
Fragment::IFrame(iframe) => match iframe.style.get_inherited_box().visibility {
Visibility::Visible => {
builder.is_contentful = true;
let rect = iframe.rect.translate(containing_block.origin.to_vector());

let common = builder.common_properties(rect.to_webrender(), &iframe.style);
builder.wr().push_iframe(
rect.to_webrender(),
common.clip_rect,
&wr::SpaceAndClipInfo {
spatial_id: common.spatial_id,
clip_chain_id: common.clip_chain_id,
},
iframe.pipeline_id.into(),
true,
);
},
Visibility::Hidden => (),
Visibility::Collapse => (),
Fragment::IFrame(iframe) => {
let iframe = iframe.borrow();
match iframe.style.get_inherited_box().visibility {
Visibility::Visible => {
builder.is_contentful = true;
let rect = iframe.rect.translate(containing_block.origin.to_vector());

let common = builder.common_properties(rect.to_webrender(), &iframe.style);
builder.wr().push_iframe(
rect.to_webrender(),
common.clip_rect,
&wr::SpaceAndClipInfo {
spatial_id: common.spatial_id,
clip_chain_id: common.clip_chain_id,
},
iframe.pipeline_id.into(),
true,
);
},
Visibility::Hidden => (),
Visibility::Collapse => (),
}
},
Fragment::Text(t) => match t.parent_style.get_inherited_box().visibility {
Visibility::Visible => {
self.build_display_list_for_text_fragment(t, builder, containing_block)
},
Visibility::Hidden => (),
Visibility::Collapse => (),
Fragment::Text(text) => {
let text = &*text.borrow();
match text.parent_style.get_inherited_box().visibility {
Visibility::Visible => {
self.build_display_list_for_text_fragment(text, builder, containing_block)
},
Visibility::Hidden => (),
Visibility::Collapse => (),
}
},
}
}
Expand Down
55 changes: 25 additions & 30 deletions components/layout_2020/display_list/stacking_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ use wr::{ClipChainId, SpatialTreeItemKey, StickyOffsetBounds};

use super::clip_path::build_clip_path_clip_chain_if_necessary;
use super::DisplayList;
use crate::cell::ArcRefCell;
use crate::display_list::conversions::{FilterToWebRender, ToWebRender};
use crate::display_list::{BuilderForBoxFragment, DisplayListBuilder};
use crate::fragment_tree::{
Expand Down Expand Up @@ -142,8 +141,7 @@ impl DisplayList {

let mut root_stacking_context = StackingContext::create_root(&self.wr, debug);
for fragment in &fragment_tree.root_fragments {
fragment.borrow_mut().build_stacking_context_tree(
fragment,
fragment.build_stacking_context_tree(
self,
&containing_block_info,
&mut root_stacking_context,
Expand Down Expand Up @@ -260,7 +258,7 @@ pub(crate) enum StackingContextContent {
clip_chain_id: wr::ClipChainId,
section: StackingContextSection,
containing_block: PhysicalRect<Au>,
fragment: ArcRefCell<Fragment>,
fragment: Fragment,
is_hit_test_for_scrollable_overflow: bool,
},

Expand Down Expand Up @@ -296,7 +294,7 @@ impl StackingContextContent {
builder.current_scroll_node_id = *scroll_node_id;
builder.current_reference_frame_scroll_node_id = *reference_frame_scroll_node_id;
builder.current_clip_chain_id = *clip_chain_id;
fragment.borrow().build_display_list(
fragment.build_display_list(
builder,
containing_block,
*section,
Expand Down Expand Up @@ -632,13 +630,11 @@ impl StackingContext {
else {
debug_panic!("Expected a fragment, not a stacking container");
};
let fragment = fragment.borrow();
let box_fragment = match &*fragment {
let box_fragment = match fragment {
Fragment::Box(box_fragment) | Fragment::Float(box_fragment) => box_fragment,
_ => {
debug_panic!("Expected a box-generated fragment");
},
_ => debug_panic!("Expected a box-generated fragment"),
};
let box_fragment = &*box_fragment.borrow();

// The `StackingContextFragment` we found is for the root DOM element:
debug_assert_eq!(
Expand Down Expand Up @@ -849,16 +845,17 @@ pub(crate) enum StackingContextBuildMode {

impl Fragment {
pub(crate) fn build_stacking_context_tree(
&mut self,
fragment_ref: &ArcRefCell<Fragment>,
&self,
display_list: &mut DisplayList,
containing_block_info: &ContainingBlockInfo,
stacking_context: &mut StackingContext,
mode: StackingContextBuildMode,
) {
let containing_block = containing_block_info.get_containing_block_for_fragment(self);
let fragment_clone = self.clone();
match self {
Fragment::Box(fragment) | Fragment::Float(fragment) => {
let fragment = fragment.borrow();
if mode == StackingContextBuildMode::SkipHoisted &&
fragment.style.clone_position().is_absolutely_positioned()
{
Expand All @@ -876,7 +873,7 @@ impl Fragment {
}

fragment.build_stacking_context_tree(
fragment_ref,
fragment_clone,
display_list,
containing_block,
containing_block_info,
Expand All @@ -890,15 +887,15 @@ impl Fragment {
None => unreachable!("Found hoisted box with missing fragment."),
};

fragment_ref.borrow_mut().build_stacking_context_tree(
fragment_ref,
fragment_ref.build_stacking_context_tree(
display_list,
containing_block_info,
stacking_context,
StackingContextBuildMode::IncludeHoisted,
);
},
Fragment::Positioning(fragment) => {
let fragment = fragment.borrow();
fragment.build_stacking_context_tree(
display_list,
containing_block,
Expand All @@ -917,7 +914,7 @@ impl Fragment {
.scroll_node_id,
clip_chain_id: containing_block.clip_chain_id,
containing_block: containing_block.rect,
fragment: fragment_ref.clone(),
fragment: fragment_clone,
is_hit_test_for_scrollable_overflow: false,
});
},
Expand Down Expand Up @@ -971,8 +968,8 @@ impl BoxFragment {
}

fn build_stacking_context_tree(
&mut self,
fragment: &ArcRefCell<Fragment>,
&self,
fragment: Fragment,
display_list: &mut DisplayList,
containing_block: &ContainingBlock,
containing_block_info: &ContainingBlockInfo,
Expand All @@ -988,8 +985,8 @@ impl BoxFragment {
}

fn build_stacking_context_tree_maybe_creating_reference_frame(
&mut self,
fragment: &ArcRefCell<Fragment>,
&self,
fragment: Fragment,
display_list: &mut DisplayList,
containing_block: &ContainingBlock,
containing_block_info: &ContainingBlockInfo,
Expand Down Expand Up @@ -1052,8 +1049,8 @@ impl BoxFragment {
}

fn build_stacking_context_tree_maybe_creating_stacking_context(
&mut self,
fragment: &ArcRefCell<Fragment>,
&self,
fragment: Fragment,
display_list: &mut DisplayList,
containing_block: &ContainingBlock,
containing_block_info: &ContainingBlockInfo,
Expand Down Expand Up @@ -1128,8 +1125,8 @@ impl BoxFragment {
}

fn build_stacking_context_tree_for_children(
&mut self,
fragment: &ArcRefCell<Fragment>,
&self,
fragment: Fragment,
display_list: &mut DisplayList,
containing_block: &ContainingBlock,
containing_block_info: &ContainingBlockInfo,
Expand Down Expand Up @@ -1273,8 +1270,7 @@ impl BoxFragment {
};

for child in &self.children {
child.borrow_mut().build_stacking_context_tree(
child,
child.build_stacking_context_tree(
display_list,
&new_containing_block_info,
stacking_context,
Expand Down Expand Up @@ -1381,7 +1377,7 @@ impl BoxFragment {
}

fn build_sticky_frame_if_necessary(
&mut self,
&self,
display_list: &mut DisplayList,
parent_scroll_node_id: &ScrollTreeNodeId,
containing_block_rect: &PhysicalRect<Au>,
Expand Down Expand Up @@ -1411,7 +1407,7 @@ impl BoxFragment {
offsets.bottom.map(|v| v.to_used_value(scroll_frame_height)),
offsets.left.map(|v| v.to_used_value(scroll_frame_width)),
);
self.resolved_sticky_insets = Some(offsets);
*self.resolved_sticky_insets.borrow_mut() = Some(offsets);

if scroll_frame_size.is_none() {
return None;
Expand Down Expand Up @@ -1610,8 +1606,7 @@ impl PositioningFragment {
containing_block_info.new_for_non_absolute_descendants(&new_containing_block);

for child in &self.children {
child.borrow_mut().build_stacking_context_tree(
child,
child.build_stacking_context_tree(
display_list,
&new_containing_block_info,
stacking_context,
Expand Down
2 changes: 1 addition & 1 deletion components/layout_2020/flexbox/layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ impl FlexContainer {
// per flex item, in the original order.
let (fragment, mut child_positioning_context) =
flex_item_fragments.next().unwrap();
let fragment = Fragment::Box(fragment);
let fragment = Fragment::Box(ArcRefCell::new(fragment));
child_positioning_context.adjust_static_position_of_hoisted_fragments(
&fragment,
PositioningContextLength::zero(),
Expand Down
Loading

0 comments on commit de780dc

Please sign in to comment.