Skip to content

Commit

Permalink
Remove rayon_croissant and clean up contains_floats (#29960)
Browse files Browse the repository at this point in the history 
Remove rayon_croissant and refactor the way that information about
floats in flows bubbles up. This simplifies the code a good deal and
lets us take advantage of some more optimized functions provided by
rayon. This removes 2 crates from the dependency tree.

In addition, this allows avoiding passing `contains_floats` up from
every box tree construction function. This makes things simpler, but
also opens up the possibility of passing more of these flags up in the
future (such as `contains_counters`).
  • Loading branch information
@mrobinson
mrobinson committed Jul 19, 2023
1 parent 1886c82 commit 505d42c
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 144 deletions.
17 changes: 0 additions & 17 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion components/layout_2020/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ net_traits = { path = "../net_traits" }
parking_lot = { workspace = true }
range = { path = "../range" }
rayon = { workspace = true }
rayon_croissant = "0.2.0"
script_layout_interface = { path = "../script_layout_interface" }
script_traits = { path = "../script_traits" }
serde = { workspace = true }
Expand Down
119 changes: 39 additions & 80 deletions components/layout_2020/flow/construct.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use crate::formatting_contexts::IndependentFormattingContext;
use crate::positioned::AbsolutelyPositionedBox;
use crate::style_ext::{ComputedValuesExt, DisplayGeneratingBox, DisplayInside, DisplayOutside};
use rayon::iter::{IntoParallelIterator, ParallelIterator};
use rayon_croissant::ParallelIteratorExt;
use servo_arc::Arc;
use std::borrow::Cow;
use std::convert::{TryFrom, TryInto};
Expand All @@ -34,18 +33,19 @@ impl BlockFormattingContext {
where
Node: NodeExt<'dom>,
{
let (contents, contains_floats) = BlockContainer::construct(
let contents = BlockContainer::construct(
context,
info,
contents,
propagated_text_decoration_line,
is_list_item,
);
let bfc = Self {
let contains_floats = contents.contains_floats();

Self {
contents,
contains_floats: contains_floats == ContainsFloats::Yes,
};
bfc
contains_floats,
}
}

pub fn construct_for_text_runs<'dom>(
Expand All @@ -61,6 +61,7 @@ impl BlockFormattingContext {
inline_level_boxes,
text_decoration_line,
has_first_formatted_line: true,
contains_floats: false,
};
let contents = BlockContainer::InlineFormattingContext(ifc);
let bfc = Self {
Expand Down Expand Up @@ -163,9 +164,6 @@ struct BlockContainerBuilder<'dom, 'style, Node> {
/// The style of the anonymous block boxes pushed to the list of block-level
/// boxes, if any (see `end_ongoing_inline_formatting_context`).
anonymous_style: Option<Arc<ComputedValues>>,

/// Whether the resulting block container contains any float box.
contains_floats: ContainsFloats,
}

impl BlockContainer {
Expand All @@ -175,7 +173,7 @@ impl BlockContainer {
contents: NonReplacedContents,
propagated_text_decoration_line: TextDecorationLine,
is_list_item: bool,
) -> (BlockContainer, ContainsFloats)
) -> BlockContainer
where
Node: NodeExt<'dom>,
{
Expand All @@ -191,7 +189,6 @@ impl BlockContainer {
),
ongoing_inline_boxes_stack: Vec::new(),
anonymous_style: None,
contains_floats: ContainsFloats::No,
};

if is_list_item {
Expand Down Expand Up @@ -222,43 +219,28 @@ impl BlockContainer {
.is_empty()
{
if builder.block_level_boxes.is_empty() {
let container = BlockContainer::InlineFormattingContext(
return BlockContainer::InlineFormattingContext(
builder.ongoing_inline_formatting_context,
);
return (container, builder.contains_floats);
}
builder.end_ongoing_inline_formatting_context();
}

let mut contains_floats = builder.contains_floats;
let mapfold = |contains_floats: &mut ContainsFloats, creator: BlockLevelJob<'dom, _>| {
let (block_level_box, box_contains_floats) = creator.finish(context);
*contains_floats |= box_contains_floats;
block_level_box
};
let block_level_boxes = if context.use_rayon {
builder
.block_level_boxes
.into_par_iter()
.mapfold_reduce_into(
&mut contains_floats,
mapfold,
|| ContainsFloats::No,
|left, right| {
*left |= right;
},
)
.map(|block_level_job| block_level_job.finish(context))
.collect()
} else {
builder
.block_level_boxes
.into_iter()
.map(|x| mapfold(&mut contains_floats, x))
.map(|block_level_job| block_level_job.finish(context))
.collect()
};
let container = BlockContainer::BlockLevelBoxes(block_level_boxes);

(container, contains_floats)
BlockContainer::BlockLevelBoxes(block_level_boxes)
}
}

Expand Down Expand Up @@ -662,8 +644,6 @@ where
contents: Contents,
box_slot: BoxSlot<'dom>,
) {
self.contains_floats = ContainsFloats::Yes;

if !self.has_ongoing_inline_formatting_context() {
let kind = BlockLevelCreator::OutOfFlowFloatBox {
contents,
Expand All @@ -681,6 +661,7 @@ where
display_inside,
contents,
)));
self.ongoing_inline_formatting_context.contains_floats = true;
self.current_inline_level_boxes().push(box_.clone());
box_slot.set(LayoutBox::InlineLevel(box_))
}
Expand Down Expand Up @@ -748,17 +729,18 @@ impl<'dom, Node> BlockLevelJob<'dom, Node>
where
Node: NodeExt<'dom>,
{
fn finish(self, context: &LayoutContext) -> (ArcRefCell<BlockLevelBox>, ContainsFloats) {
fn finish(self, context: &LayoutContext) -> ArcRefCell<BlockLevelBox> {
let info = &self.info;
let (block_level_box, contains_floats) = match self.kind {
BlockLevelCreator::SameFormattingContextBlock(contents) => {
let (contents, contains_floats) = contents.finish(context, info);
let block_level_box = ArcRefCell::new(BlockLevelBox::SameFormattingContextBlock {
let block_level_box = match self.kind {
BlockLevelCreator::SameFormattingContextBlock(intermediate_block_container) => {
let contents = intermediate_block_container.finish(context, info);
let contains_floats = contents.contains_floats();
ArcRefCell::new(BlockLevelBox::SameFormattingContextBlock {
base_fragment_info: info.into(),
contents,
style: Arc::clone(&info.style),
});
(block_level_box, contains_floats)
contains_floats,
})
},
BlockLevelCreator::Independent {
display_inside,
Expand All @@ -772,35 +754,32 @@ where
contents,
propagated_text_decoration_line,
);
(
ArcRefCell::new(BlockLevelBox::Independent(context)),
ContainsFloats::No,
)
ArcRefCell::new(BlockLevelBox::Independent(context))
},
BlockLevelCreator::OutOfFlowAbsolutelyPositionedBox {
display_inside,
contents,
} => {
let block_level_box = ArcRefCell::new(
BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(ArcRefCell::new(
AbsolutelyPositionedBox::construct(context, info, display_inside, contents),
)),
);
(block_level_box, ContainsFloats::No)
},
} => ArcRefCell::new(BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(
ArcRefCell::new(AbsolutelyPositionedBox::construct(
context,
info,
display_inside,
contents,
)),
)),
BlockLevelCreator::OutOfFlowFloatBox {
display_inside,
contents,
} => {
let block_level_box = ArcRefCell::new(BlockLevelBox::OutOfFlowFloatBox(
FloatBox::construct(context, info, display_inside, contents),
));
(block_level_box, ContainsFloats::Yes)
},
} => ArcRefCell::new(BlockLevelBox::OutOfFlowFloatBox(FloatBox::construct(
context,
info,
display_inside,
contents,
))),
};
self.box_slot
.set(LayoutBox::BlockLevel(block_level_box.clone()));
(block_level_box, contains_floats)
block_level_box
}
}

Expand All @@ -809,7 +788,7 @@ impl IntermediateBlockContainer {
self,
context: &LayoutContext,
info: &NodeAndStyleInfo<Node>,
) -> (BlockContainer, ContainsFloats)
) -> BlockContainer
where
Node: NodeExt<'dom>,
{
Expand All @@ -826,28 +805,8 @@ impl IntermediateBlockContainer {
is_list_item,
),
IntermediateBlockContainer::InlineFormattingContext(ifc) => {
// If that inline formatting context contained any float, those
// were already taken into account during the first phase of
// box construction.
(
BlockContainer::InlineFormattingContext(ifc),
ContainsFloats::No,
)
BlockContainer::InlineFormattingContext(ifc)
},
}
}
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub(crate) enum ContainsFloats {
No,
Yes,
}

impl std::ops::BitOrAssign for ContainsFloats {
fn bitor_assign(&mut self, other: Self) {
if other == ContainsFloats::Yes {
*self = ContainsFloats::Yes;
}
}
}
2 changes: 2 additions & 0 deletions components/layout_2020/flow/inline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub(crate) struct InlineFormattingContext {
// Whether this IFC contains the 1st formatted line of an element
// https://www.w3.org/TR/css-pseudo-4/#first-formatted-line
pub(super) has_first_formatted_line: bool,
pub(super) contains_floats: bool,
}

#[derive(Debug, Serialize)]
Expand Down Expand Up @@ -172,6 +173,7 @@ impl InlineFormattingContext {
inline_level_boxes: Default::default(),
text_decoration_line,
has_first_formatted_line,
contains_floats: false,
}
}

Expand Down
23 changes: 23 additions & 0 deletions components/layout_2020/flow/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,42 @@ pub(crate) enum BlockContainer {
InlineFormattingContext(InlineFormattingContext),
}

impl BlockContainer {
fn contains_floats(&self) -> bool {
match self {
BlockContainer::BlockLevelBoxes(boxes) => boxes
.iter()
.any(|block_level_box| block_level_box.borrow().contains_floats()),
BlockContainer::InlineFormattingContext { .. } => true,
}
}
}

#[derive(Debug, Serialize)]
pub(crate) enum BlockLevelBox {
SameFormattingContextBlock {
base_fragment_info: BaseFragmentInfo,
#[serde(skip_serializing)]
style: Arc<ComputedValues>,
contents: BlockContainer,
contains_floats: bool,
},
OutOfFlowAbsolutelyPositionedBox(ArcRefCell<AbsolutelyPositionedBox>),
OutOfFlowFloatBox(FloatBox),
Independent(IndependentFormattingContext),
}

impl BlockLevelBox {
fn contains_floats(&self) -> bool {
match self {
BlockLevelBox::SameFormattingContextBlock {
contains_floats, ..
} => *contains_floats,
BlockLevelBox::OutOfFlowFloatBox { .. } => true,
_ => false,
}
}

fn find_block_margin_collapsing_with_parent(
&self,
collected_margin: &mut CollapsedMargin,
Expand Down Expand Up @@ -496,6 +518,7 @@ impl BlockLevelBox {
base_fragment_info: tag,
style,
contents,
..
} => Fragment::Box(positioning_context.layout_maybe_position_relative_fragment(
layout_context,
containing_block,
Expand Down

0 comments on commit 505d42c

Please sign in to comment.