From 03f79064920d3ad840da596280ac8cd1f0f50bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 15 Jan 2018 04:14:13 -0600 Subject: [PATCH] servo: Merge #19769 - layout: Simplify the pseudo-element setup (from emilio:simplify-layout-construct); r=nox It's kind of a mess. See individual commits for details. Source-Repo: https://github.com/servo/servo Source-Revision: 032fd388e4efdd9ecc682877d017b9d2f78e29e2 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : f365989a4d7c713caf92ed0ef34a228a6243644e --- servo/components/layout/construct.rs | 46 ++++++------ .../components/layout/display_list/builder.rs | 9 +-- servo/components/layout/fragment.rs | 6 +- servo/components/layout/generated_content.rs | 4 +- servo/components/layout/inline.rs | 2 +- servo/components/layout_thread/dom_wrapper.rs | 30 ++++---- .../script_layout_interface/wrapper_traits.rs | 75 ++++++++----------- 7 files changed, 74 insertions(+), 98 deletions(-) diff --git a/servo/components/layout/construct.rs b/servo/components/layout/construct.rs index f369d41daafe..79d72c8471c6 100644 --- a/servo/components/layout/construct.rs +++ b/servo/components/layout/construct.rs @@ -109,7 +109,10 @@ pub enum ConstructionItem { /// Inline fragments and associated {ib} splits that have not yet found flows. InlineFragments(InlineFragmentsConstructionResult), /// Potentially ignorable whitespace. - Whitespace(OpaqueNode, PseudoElementType<()>, ServoArc, RestyleDamage), + /// + /// FIXME(emilio): How could whitespace have any PseudoElementType other + /// than Normal? + Whitespace(OpaqueNode, PseudoElementType, ServoArc, RestyleDamage), /// TableColumn Fragment TableColumnFragment(Fragment), } @@ -257,7 +260,7 @@ impl InlineFragmentsAccumulator { fragments: IntermediateInlineFragments::new(), enclosing_node: Some(InlineFragmentNodeInfo { address: node.opaque(), - pseudo: node.get_pseudo_element_type().strip(), + pseudo: node.get_pseudo_element_type(), style: node.style(style_context), selected_style: node.selected_style(), flags: InlineFragmentNodeFlags::FIRST_FRAGMENT_OF_ELEMENT | @@ -695,7 +698,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let specific_fragment_info = SpecificFragmentInfo::UnscannedText(info); fragments.fragments.push_back(Fragment::from_opaque_node_and_style( node.opaque(), - node.get_pseudo_element_type().strip(), + node.get_pseudo_element_type(), style, selected_style, node.restyle_damage(), @@ -715,7 +718,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> }; fragments.fragments.push_back(Fragment::from_opaque_node_and_style( node.opaque(), - node.get_pseudo_element_type().strip(), + node.get_pseudo_element_type(), style.clone(), selected_style.clone(), node.restyle_damage(), @@ -851,7 +854,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> Box::new(UnscannedTextFragmentInfo::new(String::new(), None)) ); let fragment = Fragment::from_opaque_node_and_style(node.opaque(), - node.get_pseudo_element_type().strip(), + node.get_pseudo_element_type(), node_style.clone(), node.selected_style(), node.restyle_damage(), @@ -898,7 +901,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> if node.is_ignorable_whitespace(context) { return ConstructionResult::ConstructionItem(ConstructionItem::Whitespace( node.opaque(), - node.get_pseudo_element_type().strip(), + node.get_pseudo_element_type(), context.stylist.style_for_anonymous( &context.guards, &PseudoElement::ServoText, &style), node.restyle_damage())) @@ -951,7 +954,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new( block_flow)); let fragment = Fragment::from_opaque_node_and_style(node.opaque(), - node.get_pseudo_element_type().strip(), + node.get_pseudo_element_type(), style, node.selected_style(), node.restyle_damage(), @@ -1392,7 +1395,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> if fragment.node != node.opaque() { continue } - if fragment.pseudo != node.get_pseudo_element_type().strip() { + if fragment.pseudo != node.get_pseudo_element_type() { continue } @@ -1462,8 +1465,10 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal PostorderNodeMutTraversal { // Pseudo-element. - let style = node.style(self.style_context()); - let display = match node.get_pseudo_element_type() { - PseudoElementType::Normal => Display::Inline, - PseudoElementType::Before(maybe_display) | - PseudoElementType::After(maybe_display) | - PseudoElementType::DetailsContent(maybe_display) | - PseudoElementType::DetailsSummary(maybe_display) - => maybe_display.unwrap_or(style.get_box().display), - }; - (display, style.get_box().float, style.get_box().position) + (style.get_box().display, style.get_box().float, style.get_box().position) } Some(LayoutNodeType::Element(_)) => { - let style = node.style(self.style_context()); let original_display = style.get_box().original_display; + // FIXME(emilio, #19771): This munged_display business is pretty + // wrong. After we fix this we should be able to unify the + // pseudo-element path too. let munged_display = match original_display { Display::Inline | Display::InlineBlock => original_display, _ => style.get_box().display, @@ -1649,10 +1647,10 @@ impl NodeUtils for ConcreteThreadSafeLayoutNode fn construction_result_mut(self, data: &mut LayoutData) -> &mut ConstructionResult { match self.get_pseudo_element_type() { - PseudoElementType::Before(_) => &mut data.before_flow_construction_result, - PseudoElementType::After(_) => &mut data.after_flow_construction_result, - PseudoElementType::DetailsSummary(_) => &mut data.details_summary_flow_construction_result, - PseudoElementType::DetailsContent(_) => &mut data.details_content_flow_construction_result, + PseudoElementType::Before => &mut data.before_flow_construction_result, + PseudoElementType::After => &mut data.after_flow_construction_result, + PseudoElementType::DetailsSummary => &mut data.details_summary_flow_construction_result, + PseudoElementType::DetailsContent => &mut data.details_content_flow_construction_result, PseudoElementType::Normal => &mut data.flow_construction_result, } } diff --git a/servo/components/layout/display_list/builder.rs b/servo/components/layout/display_list/builder.rs index 780ca9e96074..9fd15478ac38 100644 --- a/servo/components/layout/display_list/builder.rs +++ b/servo/components/layout/display_list/builder.rs @@ -45,7 +45,6 @@ use msg::constellation_msg::{BrowsingContextId, PipelineId}; use net_traits::image::base::PixelFormat; use net_traits::image_cache::UsePlaceholder; use range::Range; -use script_layout_interface::wrapper_traits::PseudoElementType; use servo_config::opts; use servo_geometry::max_rect; use std::{cmp, f32}; @@ -2276,13 +2275,7 @@ impl FragmentDisplayListBuilding for Fragment { } fn fragment_type(&self) -> FragmentType { - match self.pseudo { - PseudoElementType::Normal => FragmentType::FragmentBody, - PseudoElementType::Before(_) => FragmentType::BeforePseudoContent, - PseudoElementType::After(_) => FragmentType::AfterPseudoContent, - PseudoElementType::DetailsSummary(_) => FragmentType::FragmentBody, - PseudoElementType::DetailsContent(_) => FragmentType::FragmentBody, - } + self.pseudo.fragment_type() } } diff --git a/servo/components/layout/fragment.rs b/servo/components/layout/fragment.rs index c06cb9bba7be..ae90f4d50d48 100644 --- a/servo/components/layout/fragment.rs +++ b/servo/components/layout/fragment.rs @@ -138,7 +138,7 @@ pub struct Fragment { pub restyle_damage: RestyleDamage, /// The pseudo-element that this fragment represents. - pub pseudo: PseudoElementType<()>, + pub pseudo: PseudoElementType, /// Various flags for this fragment. pub flags: FragmentFlags, @@ -622,7 +622,7 @@ impl Fragment { margin: LogicalMargin::zero(writing_mode), specific: specific, inline_context: None, - pseudo: node.get_pseudo_element_type().strip(), + pseudo: node.get_pseudo_element_type(), flags: FragmentFlags::empty(), debug_id: DebugId::new(), stacking_context_id: StackingContextId::root(), @@ -631,7 +631,7 @@ impl Fragment { /// Constructs a new `Fragment` instance from an opaque node. pub fn from_opaque_node_and_style(node: OpaqueNode, - pseudo: PseudoElementType<()>, + pseudo: PseudoElementType, style: ServoArc, selected_style: ServoArc, mut restyle_damage: RestyleDamage, diff --git a/servo/components/layout/generated_content.rs b/servo/components/layout/generated_content.rs index fd11b9968724..e21e2a7e8649 100644 --- a/servo/components/layout/generated_content.rs +++ b/servo/components/layout/generated_content.rs @@ -368,7 +368,7 @@ impl Counter { fn render(&self, layout_context: &LayoutContext, node: OpaqueNode, - pseudo: PseudoElementType<()>, + pseudo: PseudoElementType, style: ::ServoArc, list_style_type: ListStyleType, mode: RenderingMode) @@ -431,7 +431,7 @@ struct CounterValue { /// Creates fragment info for a literal string. fn render_text(layout_context: &LayoutContext, node: OpaqueNode, - pseudo: PseudoElementType<()>, + pseudo: PseudoElementType, style: ::ServoArc, string: String) -> Option { diff --git a/servo/components/layout/inline.rs b/servo/components/layout/inline.rs index a96d35869995..934e4aa1fc8a 100644 --- a/servo/components/layout/inline.rs +++ b/servo/components/layout/inline.rs @@ -1766,7 +1766,7 @@ pub struct InlineFragmentNodeInfo { pub address: OpaqueNode, pub style: ServoArc, pub selected_style: ServoArc, - pub pseudo: PseudoElementType<()>, + pub pseudo: PseudoElementType, pub flags: InlineFragmentNodeFlags, } diff --git a/servo/components/layout_thread/dom_wrapper.rs b/servo/components/layout_thread/dom_wrapper.rs index 8a8a2b571314..14a2e5f38671 100644 --- a/servo/components/layout_thread/dom_wrapper.rs +++ b/servo/components/layout_thread/dom_wrapper.rs @@ -63,7 +63,6 @@ use std::sync::atomic::Ordering; use style::CaseSensitivityExt; use style::applicable_declarations::ApplicableDeclarationBlock; use style::attr::AttrValue; -use style::computed_values::display; use style::context::SharedStyleContext; use style::data::ElementData; use style::dom::{DomChildren, LayoutIterator, NodeInfo, OpaqueNode}; @@ -804,7 +803,7 @@ pub struct ServoThreadSafeLayoutNode<'ln> { /// The pseudo-element type, with (optionally) /// a specified display value to override the stylesheet. - pseudo: PseudoElementType>, + pseudo: PseudoElementType, } impl<'a> PartialEq for ServoThreadSafeLayoutNode<'a> { @@ -994,7 +993,7 @@ impl ThreadSafeLayoutNodeChildrenIterator unsafe { parent.dangerous_first_child() } }) }, - PseudoElementType::DetailsContent(_) | PseudoElementType::DetailsSummary(_) => { + PseudoElementType::DetailsContent | PseudoElementType::DetailsSummary => { unsafe { parent.dangerous_first_child() } }, _ => None, @@ -1012,9 +1011,9 @@ impl Iterator for ThreadSafeLayoutNodeChildrenIterator Option { use ::selectors::Element; match self.parent_node.get_pseudo_element_type() { - PseudoElementType::Before(_) | PseudoElementType::After(_) => None, + PseudoElementType::Before | PseudoElementType::After => None, - PseudoElementType::DetailsSummary(_) => { + PseudoElementType::DetailsSummary => { let mut current_node = self.current_node.clone(); loop { let next_node = if let Some(ref node) = current_node { @@ -1034,7 +1033,7 @@ impl Iterator for ThreadSafeLayoutNodeChildrenIterator { + PseudoElementType::DetailsContent => { let node = self.current_node.clone(); let node = node.and_then(|node| { if node.is_element() && @@ -1053,7 +1052,7 @@ impl Iterator for ThreadSafeLayoutNodeChildrenIterator { + PseudoElementType::Before => { self.parent_node.get_details_summary_pseudo() .or_else(|| unsafe { self.parent_node.dangerous_first_child() }) .or_else(|| self.parent_node.get_after_pseudo()) @@ -1061,11 +1060,9 @@ impl Iterator for ThreadSafeLayoutNodeChildrenIterator { unsafe { node.dangerous_next_sibling() }.or_else(|| self.parent_node.get_after_pseudo()) }, - PseudoElementType::DetailsSummary(_) => self.parent_node.get_details_content_pseudo(), - PseudoElementType::DetailsContent(_) => self.parent_node.get_after_pseudo(), - PseudoElementType::After(_) => { - None - }, + PseudoElementType::DetailsSummary => self.parent_node.get_details_content_pseudo(), + PseudoElementType::DetailsContent => self.parent_node.get_after_pseudo(), + PseudoElementType::After => None, }; } node @@ -1083,7 +1080,7 @@ pub struct ServoThreadSafeLayoutElement<'le> { /// The pseudo-element type, with (optionally) /// a specified display value to override the stylesheet. - pseudo: PseudoElementType>, + pseudo: PseudoElementType, } impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { @@ -1096,15 +1093,14 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { } } - fn get_pseudo_element_type(&self) -> PseudoElementType> { + fn get_pseudo_element_type(&self) -> PseudoElementType { self.pseudo } - fn with_pseudo(&self, - pseudo: PseudoElementType>) -> Self { + fn with_pseudo(&self, pseudo: PseudoElementType) -> Self { ServoThreadSafeLayoutElement { element: self.element.clone(), - pseudo: pseudo, + pseudo, } } diff --git a/servo/components/script_layout_interface/wrapper_traits.rs b/servo/components/script_layout_interface/wrapper_traits.rs index 348a7a46cfb1..dc1f5db4c540 100644 --- a/servo/components/script_layout_interface/wrapper_traits.rs +++ b/servo/components/script_layout_interface/wrapper_traits.rs @@ -17,7 +17,6 @@ use servo_arc::Arc; use servo_url::ServoUrl; use std::fmt::Debug; use style::attr::AttrValue; -use style::computed_values::display::T as Display; use style::context::SharedStyleContext; use style::data::ElementData; use style::dom::{LayoutIterator, NodeInfo, TNode}; @@ -29,46 +28,46 @@ use style::stylist::RuleInclusion; use webrender_api::ClipId; #[derive(Clone, Copy, Debug, PartialEq)] -pub enum PseudoElementType { +pub enum PseudoElementType { Normal, - Before(T), - After(T), - DetailsSummary(T), - DetailsContent(T), + Before, + After, + DetailsSummary, + DetailsContent, } -impl PseudoElementType { +impl PseudoElementType { + pub fn fragment_type(&self) -> FragmentType { + match *self { + PseudoElementType::Normal => FragmentType::FragmentBody, + PseudoElementType::Before => FragmentType::BeforePseudoContent, + PseudoElementType::After => FragmentType::AfterPseudoContent, + PseudoElementType::DetailsSummary => FragmentType::FragmentBody, + PseudoElementType::DetailsContent => FragmentType::FragmentBody, + } + } + pub fn is_before(&self) -> bool { match *self { - PseudoElementType::Before(_) => true, + PseudoElementType::Before => true, _ => false, } } pub fn is_replaced_content(&self) -> bool { match *self { - PseudoElementType::Before(_) | PseudoElementType::After(_) => true, + PseudoElementType::Before | PseudoElementType::After => true, _ => false, } } - pub fn strip(&self) -> PseudoElementType<()> { - match *self { - PseudoElementType::Normal => PseudoElementType::Normal, - PseudoElementType::Before(_) => PseudoElementType::Before(()), - PseudoElementType::After(_) => PseudoElementType::After(()), - PseudoElementType::DetailsSummary(_) => PseudoElementType::DetailsSummary(()), - PseudoElementType::DetailsContent(_) => PseudoElementType::DetailsContent(()), - } - } - pub fn style_pseudo_element(&self) -> PseudoElement { match *self { PseudoElementType::Normal => unreachable!("style_pseudo_element called with PseudoElementType::Normal"), - PseudoElementType::Before(_) => PseudoElement::Before, - PseudoElementType::After(_) => PseudoElement::After, - PseudoElementType::DetailsSummary(_) => PseudoElement::DetailsSummary, - PseudoElementType::DetailsContent(_) => PseudoElement::DetailsContent, + PseudoElementType::Before => PseudoElement::Before, + PseudoElementType::After => PseudoElement::After, + PseudoElementType::DetailsSummary => PseudoElement::DetailsSummary, + PseudoElementType::DetailsContent => PseudoElement::DetailsContent, } } } @@ -197,7 +196,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo fn as_element(&self) -> Option; #[inline] - fn get_pseudo_element_type(&self) -> PseudoElementType> { + fn get_pseudo_element_type(&self) -> PseudoElementType { self.as_element().map_or(PseudoElementType::Normal, |el| el.get_pseudo_element_type()) } @@ -265,13 +264,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo fn get_rowspan(&self) -> u32; fn fragment_type(&self) -> FragmentType { - match self.get_pseudo_element_type() { - PseudoElementType::Normal => FragmentType::FragmentBody, - PseudoElementType::Before(_) => FragmentType::BeforePseudoContent, - PseudoElementType::After(_) => FragmentType::AfterPseudoContent, - PseudoElementType::DetailsSummary(_) => FragmentType::FragmentBody, - PseudoElementType::DetailsContent(_) => FragmentType::FragmentBody, - } + self.get_pseudo_element_type().fragment_type() } fn generate_scroll_root_id(&self, pipeline_id: PipelineId) -> ClipId { @@ -302,7 +295,7 @@ pub trait ThreadSafeLayoutElement /// Creates a new `ThreadSafeLayoutElement` for the same `LayoutElement` /// with a different pseudo-element type. - fn with_pseudo(&self, pseudo: PseudoElementType>) -> Self; + fn with_pseudo(&self, pseudo: PseudoElementType) -> Self; /// Returns the type ID of this node. /// Returns `None` if this is a pseudo-element; otherwise, returns `Some`. @@ -325,12 +318,12 @@ pub trait ThreadSafeLayoutElement fn style_data(&self) -> AtomicRef; #[inline] - fn get_pseudo_element_type(&self) -> PseudoElementType>; + fn get_pseudo_element_type(&self) -> PseudoElementType; #[inline] fn get_before_pseudo(&self) -> Option { if self.style_data().styles.pseudos.get(&PseudoElement::Before).is_some() { - Some(self.with_pseudo(PseudoElementType::Before(None))) + Some(self.with_pseudo(PseudoElementType::Before)) } else { None } @@ -339,7 +332,7 @@ pub trait ThreadSafeLayoutElement #[inline] fn get_after_pseudo(&self) -> Option { if self.style_data().styles.pseudos.get(&PseudoElement::After).is_some() { - Some(self.with_pseudo(PseudoElementType::After(None))) + Some(self.with_pseudo(PseudoElementType::After)) } else { None } @@ -349,7 +342,7 @@ pub trait ThreadSafeLayoutElement fn get_details_summary_pseudo(&self) -> Option { if self.get_local_name() == &local_name!("details") && self.get_namespace() == &ns!(html) { - Some(self.with_pseudo(PseudoElementType::DetailsSummary(None))) + Some(self.with_pseudo(PseudoElementType::DetailsSummary)) } else { None } @@ -358,13 +351,9 @@ pub trait ThreadSafeLayoutElement #[inline] fn get_details_content_pseudo(&self) -> Option { if self.get_local_name() == &local_name!("details") && - self.get_namespace() == &ns!(html) { - let display = if self.get_attr(&ns!(), &local_name!("open")).is_some() { - None // Specified by the stylesheet - } else { - Some(Display::None) - }; - Some(self.with_pseudo(PseudoElementType::DetailsContent(display))) + self.get_namespace() == &ns!(html) && + self.get_attr(&ns!(), &local_name!("open")).is_some() { + Some(self.with_pseudo(PseudoElementType::DetailsContent)) } else { None }