From bcaf6bd9aa3612f79e75e4a0aa4148e1d79e8f87 Mon Sep 17 00:00:00 2001 From: Pu Xingyu Date: Wed, 9 Nov 2016 03:59:22 -0600 Subject: [PATCH] servo: Merge #14130 - layout: Mark flex items properly during construction (from stshine:construct-flexbox); r=pcwalton Set the flag of the fragment of children in a flex container according to the direction of the container. The mark is done on the fragment because flex item enstablish a stacking context when its z-index is non-zero ,despite its `position' property. Part of #14123. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [ ] There are tests for these changes OR - [X] These changes do not require tests because refactoring r? @pcwalton Source-Repo: https://github.com/servo/servo Source-Revision: 333c397f03bc2d143e0d8c112138d5758d37f6f5 --- servo/components/layout/block.rs | 27 +++++++++++++-------------- servo/components/layout/construct.rs | 26 ++++++++++++++++++++++---- servo/components/layout/flex.rs | 14 ++++++++++++-- servo/components/layout/flow.rs | 11 +++++++++++ servo/components/layout/fragment.rs | 17 +++++++++++++++++ servo/components/layout/model.rs | 2 +- 6 files changed, 76 insertions(+), 21 deletions(-) diff --git a/servo/components/layout/block.rs b/servo/components/layout/block.rs index 7702783bf4cc..45de87867781 100644 --- a/servo/components/layout/block.rs +++ b/servo/components/layout/block.rs @@ -41,6 +41,7 @@ use flow::{ImmutableFlowUtils, LateAbsolutePositionInfo, MutableFlowUtils, Opaqu use flow::IS_ABSOLUTELY_POSITIONED; use flow_list::FlowList; use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, Overflow}; +use fragment::{IS_INLINE_FLEX_ITEM, IS_BLOCK_FLEX_ITEM}; use fragment::SpecificFragmentInfo; use gfx::display_list::{ClippingRegion, StackingContext}; use gfx_traits::ScrollRootId; @@ -484,7 +485,7 @@ pub enum BlockType { FloatNonReplaced, InlineBlockReplaced, InlineBlockNonReplaced, - FlexItem, + InlineFlexItem, } #[derive(Clone, PartialEq)] @@ -520,8 +521,6 @@ bitflags! { flags BlockFlowFlags: u8 { #[doc = "If this is set, then this block flow is the root flow."] const IS_ROOT = 0b0000_0001, - #[doc = "Whether this block flow is a child of a flex container."] - const IS_FLEX = 0b0001_0000, } } @@ -561,8 +560,8 @@ impl BlockFlow { } else { BlockType::AbsoluteNonReplaced } - } else if self.is_flex() { - BlockType::FlexItem + } else if self.is_inline_flex_item() { + BlockType::InlineFlexItem } else if self.base.flags.is_float() { if self.is_replaced_content() { BlockType::FloatReplaced @@ -638,8 +637,8 @@ impl BlockFlow { shared_context, containing_block_inline_size); } - BlockType::FlexItem => { - let inline_size_computer = FlexItem; + BlockType::InlineFlexItem => { + let inline_size_computer = InlineFlexItem; inline_size_computer.compute_used_inline_size(self, shared_context, containing_block_inline_size); @@ -1506,7 +1505,7 @@ impl BlockFlow { } // If you remove the might_have_floats_in conditional, this will go off. - debug_assert!(!self.is_flex()); + debug_assert!(!self.is_inline_flex_item()); // Compute the available space for us, based on the actual floats. let rect = self.base.floats.available_rect(Au(0), @@ -1778,12 +1777,12 @@ impl BlockFlow { padding.block_start.is_definitely_zero() && padding.block_end.is_definitely_zero() } - pub fn mark_as_flex(&mut self) { - self.flags.insert(IS_FLEX) + pub fn is_inline_flex_item(&self) -> bool { + self.fragment.flags.contains(IS_INLINE_FLEX_ITEM) } - pub fn is_flex(&self) -> bool { - self.flags.contains(IS_FLEX) + pub fn is_block_flex_item(&self) -> bool { + self.fragment.flags.contains(IS_BLOCK_FLEX_ITEM) } } @@ -2589,7 +2588,7 @@ pub struct FloatNonReplaced; pub struct FloatReplaced; pub struct InlineBlockNonReplaced; pub struct InlineBlockReplaced; -pub struct FlexItem; +pub struct InlineFlexItem; impl ISizeAndMarginsComputer for AbsoluteNonReplaced { /// Solve the horizontal constraint equation for absolute non-replaced elements. @@ -3080,7 +3079,7 @@ impl ISizeAndMarginsComputer for InlineBlockReplaced { } } -impl ISizeAndMarginsComputer for FlexItem { +impl ISizeAndMarginsComputer for InlineFlexItem { // Replace the default method directly to prevent recalculating and setting margins again // which has already been set by its parent. fn compute_used_inline_size(&self, diff --git a/servo/components/layout/construct.rs b/servo/components/layout/construct.rs index 0b489459352a..579489e8de30 100644 --- a/servo/components/layout/construct.rs +++ b/servo/components/layout/construct.rs @@ -25,6 +25,7 @@ use flow::{MutableFlowUtils, MutableOwnedFlowUtils}; use flow_ref::FlowRef; use fragment::{CanvasFragmentInfo, ImageFragmentInfo, InlineAbsoluteFragmentInfo, SvgFragmentInfo}; use fragment::{Fragment, GeneratedContentInfo, IframeFragmentInfo}; +use fragment::{IS_INLINE_FLEX_ITEM, IS_BLOCK_FLEX_ITEM}; use fragment::{InlineAbsoluteHypotheticalFragmentInfo, TableColumnFragmentInfo}; use fragment::{InlineBlockFragmentInfo, SpecificFragmentInfo, UnscannedTextFragmentInfo}; use fragment::WhitespaceStrippingResult; @@ -33,6 +34,7 @@ use inline::{FIRST_FRAGMENT_OF_ELEMENT, InlineFlow, InlineFragmentNodeFlags}; use inline::{InlineFragmentNodeInfo, LAST_FRAGMENT_OF_ELEMENT}; use linked_list::prepend_from; use list_item::{ListItemFlow, ListStyleTypeContent}; +use model::Direction; use multicol::{MulticolColumnFlow, MulticolFlow}; use parallel; use script_layout_interface::{LayoutElementType, LayoutNodeType, is_image_data}; @@ -1928,9 +1930,16 @@ impl Legalizer { &[PseudoElement::ServoAnonymousBlock], SpecificFragmentInfo::Generic, BlockFlow::from_fragment); - flow::mut_base(FlowRef::deref_mut(&mut - block_wrapper)).flags - .insert(MARGINS_CANNOT_COLLAPSE); + { + let flag = if parent.as_flex().main_mode() == Direction::Inline { + IS_INLINE_FLEX_ITEM + } else { + IS_BLOCK_FLEX_ITEM + }; + let mut block = FlowRef::deref_mut(&mut block_wrapper).as_mut_block(); + block.base.flags.insert(MARGINS_CANNOT_COLLAPSE); + block.fragment.flags.insert(flag); + } block_wrapper.add_new_child((*child).clone()); block_wrapper.finish(); parent.add_new_child(block_wrapper); @@ -1938,7 +1947,16 @@ impl Legalizer { } (FlowClass::Flex, _) => { - flow::mut_base(FlowRef::deref_mut(child)).flags.insert(MARGINS_CANNOT_COLLAPSE); + { + let flag = if parent.as_flex().main_mode() == Direction::Inline { + IS_INLINE_FLEX_ITEM + } else { + IS_BLOCK_FLEX_ITEM + }; + let mut block = FlowRef::deref_mut(child).as_mut_block(); + block.base.flags.insert(MARGINS_CANNOT_COLLAPSE); + block.fragment.flags.insert(flag); + } parent.add_new_child((*child).clone()); true } diff --git a/servo/components/layout/flex.rs b/servo/components/layout/flex.rs index 337abc3227a6..b2ee35558579 100644 --- a/servo/components/layout/flex.rs +++ b/servo/components/layout/flex.rs @@ -395,6 +395,10 @@ impl FlexFlow { } } + pub fn main_mode(&self) -> Direction { + self.main_mode + } + /// Returns a line start after the last item that is already in a line. /// Note that when the container main size is infinite(i.e. A column flexbox with auto height), /// we do not need to do flex resolving and this can be considered as a fast-path, so the @@ -613,8 +617,6 @@ impl FlexFlow { // // TODO(#2265, pcwalton): Do this in the cascade instead. block.base.flags.set_text_align(containing_block_text_align); - // FIXME(stshine): should this be done during construction? - block.mark_as_flex(); let margin = block.fragment.style().logical_margin(); let auto_len = @@ -810,6 +812,14 @@ impl Flow for FlexFlow { FlowClass::Flex } + fn as_mut_flex(&mut self) -> &mut FlexFlow { + self + } + + fn as_flex(&self) -> &FlexFlow { + self + } + fn as_block(&self) -> &BlockFlow { &self.block_flow } diff --git a/servo/components/layout/flow.rs b/servo/components/layout/flow.rs index 3da277d924df..fae309d2ae2b 100644 --- a/servo/components/layout/flow.rs +++ b/servo/components/layout/flow.rs @@ -30,6 +30,7 @@ use block::{BlockFlow, FormattingContextType}; use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use euclid::{Point2D, Size2D}; +use flex::FlexFlow; use floats::{Floats, SpeculatedFloatPlacement}; use flow_list::{FlowList, MutFlowListIterator}; use flow_ref::{FlowRef, WeakFlowRef}; @@ -86,6 +87,16 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static { panic!("called as_mut_block() on a non-block flow") } + /// If this is a flex flow, returns the underlying object. Fails otherwise. + fn as_flex(&self) -> &FlexFlow { + panic!("called as_flex() on a non-flex flow") + } + + /// If this is a flex flow, returns the underlying object, borrowed mutably. Fails otherwise. + fn as_mut_flex(&mut self) -> &mut FlexFlow { + panic!("called as_mut_flex() on a non-flex flow") + } + /// If this is an inline flow, returns the underlying object. Fails otherwise. fn as_inline(&self) -> &InlineFlow { panic!("called as_inline() on a non-inline flow") diff --git a/servo/components/layout/fragment.rs b/servo/components/layout/fragment.rs index 1fa3ae0612a0..265f140ab63a 100644 --- a/servo/components/layout/fragment.rs +++ b/servo/components/layout/fragment.rs @@ -123,6 +123,9 @@ pub struct Fragment { /// The pseudo-element that this fragment represents. pub pseudo: PseudoElementType<()>, + /// Various flags for this fragment. + pub flags: FragmentFlags, + /// A debug ID that is consistent for the life of this fragment (via transform etc). /// This ID should not be considered stable across multiple layouts or fragment /// manipulations. @@ -917,6 +920,7 @@ impl Fragment { specific: specific, inline_context: None, pseudo: node.get_pseudo_element_type().strip(), + flags: FragmentFlags::empty(), debug_id: DebugId::new(), stacking_context_id: StackingContextId::new(0), } @@ -945,6 +949,7 @@ impl Fragment { specific: specific, inline_context: None, pseudo: pseudo, + flags: FragmentFlags::empty(), debug_id: DebugId::new(), stacking_context_id: StackingContextId::new(0), } @@ -969,6 +974,7 @@ impl Fragment { specific: specific, inline_context: None, pseudo: self.pseudo, + flags: FragmentFlags::empty(), debug_id: DebugId::new(), stacking_context_id: StackingContextId::new(0), } @@ -996,6 +1002,7 @@ impl Fragment { specific: info, inline_context: self.inline_context.clone(), pseudo: self.pseudo.clone(), + flags: FragmentFlags::empty(), debug_id: self.debug_id.clone(), stacking_context_id: StackingContextId::new(0), } @@ -3111,6 +3118,16 @@ impl Overflow { } } +bitflags! { + pub flags FragmentFlags: u8 { + // TODO(stshine): find a better name since these flags can also be used for grid item. + /// Whether this fragment represents a child in a row flex container. + const IS_INLINE_FLEX_ITEM = 0b0000_0001, + /// Whether this fragment represents a child in a column flex container. + const IS_BLOCK_FLEX_ITEM = 0b0000_0010, + } +} + /// Specified distances from the margin edge of a block to its content in the inline direction. /// These are returned by `guess_inline_content_edge_offsets()` and are used in the float placement /// speculation logic. diff --git a/servo/components/layout/model.rs b/servo/components/layout/model.rs index 4d2ef8e8c875..d57bb2628fa5 100644 --- a/servo/components/layout/model.rs +++ b/servo/components/layout/model.rs @@ -505,7 +505,7 @@ impl ToGfxMatrix for ComputedMatrix { } // Used to specify the logical direction. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum Direction { Inline, Block