From 2058ac0c7f9a9c2208b58688b5b692991c7416e0 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 10 Jan 2019 14:03:46 +0100 Subject: [PATCH 01/20] Make some cosmetic changes to ReflowGoal methods --- components/script_layout_interface/message.rs | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index 144961cb8f5..b7415d88cdc 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -143,18 +143,18 @@ impl ReflowGoal { pub fn needs_display_list(&self) -> bool { match *self { ReflowGoal::Full | ReflowGoal::TickAnimations => true, - ReflowGoal::LayoutQuery(ref querymsg, _) => match querymsg { - &QueryMsg::NodesFromPointQuery(..) | - &QueryMsg::TextIndexQuery(..) | - &QueryMsg::ElementInnerTextQuery(_) => true, - &QueryMsg::ContentBoxQuery(_) | - &QueryMsg::ContentBoxesQuery(_) | - &QueryMsg::NodeGeometryQuery(_) | - &QueryMsg::NodeScrollGeometryQuery(_) | - &QueryMsg::NodeScrollIdQuery(_) | - &QueryMsg::ResolvedStyleQuery(..) | - &QueryMsg::OffsetParentQuery(_) | - &QueryMsg::StyleQuery(_) => false, + ReflowGoal::LayoutQuery(ref querymsg, _) => match *querymsg { + QueryMsg::NodesFromPointQuery(..) | + QueryMsg::TextIndexQuery(..) | + QueryMsg::ElementInnerTextQuery(_) => true, + QueryMsg::ContentBoxQuery(_) | + QueryMsg::ContentBoxesQuery(_) | + QueryMsg::NodeGeometryQuery(_) | + QueryMsg::NodeScrollGeometryQuery(_) | + QueryMsg::NodeScrollIdQuery(_) | + QueryMsg::ResolvedStyleQuery(..) | + QueryMsg::OffsetParentQuery(_) | + QueryMsg::StyleQuery(_) => false, }, } } @@ -164,18 +164,18 @@ impl ReflowGoal { pub fn needs_display(&self) -> bool { match *self { ReflowGoal::Full | ReflowGoal::TickAnimations => true, - ReflowGoal::LayoutQuery(ref querymsg, _) => match querymsg { - &QueryMsg::NodesFromPointQuery(..) | - &QueryMsg::TextIndexQuery(..) | - &QueryMsg::ElementInnerTextQuery(_) => true, - &QueryMsg::ContentBoxQuery(_) | - &QueryMsg::ContentBoxesQuery(_) | - &QueryMsg::NodeGeometryQuery(_) | - &QueryMsg::NodeScrollGeometryQuery(_) | - &QueryMsg::NodeScrollIdQuery(_) | - &QueryMsg::ResolvedStyleQuery(..) | - &QueryMsg::OffsetParentQuery(_) | - &QueryMsg::StyleQuery(_) => false, + ReflowGoal::LayoutQuery(ref querymsg, _) => match *querymsg { + QueryMsg::NodesFromPointQuery(..) | + QueryMsg::TextIndexQuery(..) | + QueryMsg::ElementInnerTextQuery(_) => true, + QueryMsg::ContentBoxQuery(_) | + QueryMsg::ContentBoxesQuery(_) | + QueryMsg::NodeGeometryQuery(_) | + QueryMsg::NodeScrollGeometryQuery(_) | + QueryMsg::NodeScrollIdQuery(_) | + QueryMsg::ResolvedStyleQuery(..) | + QueryMsg::OffsetParentQuery(_) | + QueryMsg::StyleQuery(_) => false, }, } } From 17ee21bf9d4200ea53d5a924c139f6c9add8b2b7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 10 Jan 2019 14:05:36 +0100 Subject: [PATCH 02/20] Rename NewLayoutThreadInfo to LayoutThreadInit Following the convention of our other init structs. --- components/layout_thread/lib.rs | 4 ++-- components/script/script_thread.rs | 4 ++-- components/script_layout_interface/message.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index b60838365de..47774b86ec3 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -75,7 +75,7 @@ use parking_lot::RwLock; use profile_traits::mem::{self as profile_mem, Report, ReportKind, ReportsChan}; use profile_traits::time::{self as profile_time, profile, TimerMetadata}; use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; -use script_layout_interface::message::{Msg, NewLayoutThreadInfo, NodesFromPointQueryType, Reflow}; +use script_layout_interface::message::{Msg, LayoutThreadInit, NodesFromPointQueryType, Reflow}; use script_layout_interface::message::{QueryMsg, ReflowComplete, ReflowGoal, ScriptReflow}; use script_layout_interface::rpc::TextIndexResponse; use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse, StyleResponse}; @@ -842,7 +842,7 @@ impl LayoutThread { reports_chan.send(reports); } - fn create_layout_thread(&self, info: NewLayoutThreadInfo) { + fn create_layout_thread(&self, info: LayoutThreadInit) { LayoutThread::create( info.id, self.top_level_browsing_context_id, diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 9247ca6afba..fd08bb5f80c 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -118,7 +118,7 @@ use net_traits::{ }; use profile_traits::mem::{self as profile_mem, OpaqueSender, ReportsChan}; use profile_traits::time::{self as profile_time, profile, ProfilerCategory}; -use script_layout_interface::message::{self, Msg, NewLayoutThreadInfo, ReflowGoal}; +use script_layout_interface::message::{self, Msg, LayoutThreadInit, ReflowGoal}; use script_traits::webdriver_msg::WebDriverScriptCommand; use script_traits::CompositorEvent::{ CompositionEvent, KeyboardEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent, TouchEvent, @@ -1936,7 +1936,7 @@ impl ScriptThread { let layout_pair = unbounded(); let layout_chan = layout_pair.0.clone(); - let msg = message::Msg::CreateLayoutThread(NewLayoutThreadInfo { + let msg = message::Msg::CreateLayoutThread(LayoutThreadInit { id: new_pipeline_id, url: load_data.url.clone(), is_parent: false, diff --git a/components/script_layout_interface/message.rs b/components/script_layout_interface/message.rs index b7415d88cdc..2d92cfb157c 100644 --- a/components/script_layout_interface/message.rs +++ b/components/script_layout_interface/message.rs @@ -82,7 +82,7 @@ pub enum Msg { /// Creates a new layout thread. /// /// This basically exists to keep the script-layout dependency one-way. - CreateLayoutThread(NewLayoutThreadInfo), + CreateLayoutThread(LayoutThreadInit), /// Set the final Url. SetFinalUrl(ServoUrl), @@ -214,7 +214,7 @@ pub struct ScriptReflow { pub dom_count: u32, } -pub struct NewLayoutThreadInfo { +pub struct LayoutThreadInit { pub id: PipelineId, pub url: ServoUrl, pub is_parent: bool, From 64755705fbdeb0aaf257bbdd39577646f2718f28 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 10 Jan 2019 14:35:43 +0100 Subject: [PATCH 03/20] Move CSSReporter from script_layout_interface to script --- Cargo.lock | 1 - components/script/dom/bindings/trace.rs | 2 - components/script/dom/window.rs | 43 +++++++++++++++-- components/script_layout_interface/Cargo.toml | 1 - components/script_layout_interface/lib.rs | 3 -- .../script_layout_interface/reporter.rs | 48 ------------------- 6 files changed, 40 insertions(+), 58 deletions(-) delete mode 100644 components/script_layout_interface/reporter.rs diff --git a/Cargo.lock b/Cargo.lock index af16f9929d2..2315f317351 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3315,7 +3315,6 @@ dependencies = [ "html5ever 0.22.5 (registry+https://github.com/rust-lang/crates.io-index)", "ipc-channel 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.44 (registry+https://github.com/rust-lang/crates.io-index)", - "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "malloc_size_of 0.0.1", "malloc_size_of_derive 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "metrics 0.0.1", diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index e645fb181d5..2ea4de3d340 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -85,7 +85,6 @@ use net_traits::{Metadata, NetworkError, ReferrerPolicy, ResourceFetchTiming, Re use offscreen_gl_context::GLLimits; use profile_traits::mem::ProfilerChan as MemProfilerChan; use profile_traits::time::ProfilerChan as TimeProfilerChan; -use script_layout_interface::reporter::CSSErrorReporter; use script_layout_interface::rpc::LayoutRPC; use script_layout_interface::OpaqueStyleAndLayoutData; use script_traits::DrawAPaintImageResult; @@ -455,7 +454,6 @@ unsafe_no_jsmanaged_fields!(Instant); unsafe_no_jsmanaged_fields!(RelativePos); unsafe_no_jsmanaged_fields!(OpaqueStyleAndLayoutData); unsafe_no_jsmanaged_fields!(PathBuf); -unsafe_no_jsmanaged_fields!(CSSErrorReporter); unsafe_no_jsmanaged_fields!(DrawAPaintImageResult); unsafe_no_jsmanaged_fields!(DocumentId); unsafe_no_jsmanaged_fields!(ImageKey); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 3f702cbeabd..f8669ccd858 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -69,7 +69,7 @@ use base64; use bluetooth_traits::BluetoothRequest; use canvas_traits::webgl::WebGLChan; use crossbeam_channel::{unbounded, Sender, TryRecvError}; -use cssparser::{Parser, ParserInput}; +use cssparser::{Parser, ParserInput, SourceLocation}; use devtools_traits::{ScriptToDevtoolsControlMsg, TimelineMarker, TimelineMarkerType}; use dom_struct::dom_struct; use embedder_traits::EmbedderMsg; @@ -94,7 +94,6 @@ use profile_traits::ipc as ProfiledIpc; use profile_traits::mem::ProfilerChan as MemProfilerChan; use profile_traits::time::ProfilerChan as TimeProfilerChan; use script_layout_interface::message::{Msg, QueryMsg, Reflow, ReflowGoal, ScriptReflow}; -use script_layout_interface::reporter::CSSErrorReporter; use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC}; use script_layout_interface::rpc::{ NodeScrollIdResponse, ResolvedStyleResponse, TextIndexResponse, @@ -120,7 +119,7 @@ use std::mem; use std::rc::Rc; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; -use style::error_reporting::ParseErrorReporter; +use style::error_reporting::{ContextualParseError, ParseErrorReporter}; use style::media_queries; use style::parser::ParserContext as CssParserContext; use style::properties::{ComputedValues, PropertyId}; @@ -2242,3 +2241,41 @@ impl Window { )); } } + +#[derive(Clone, MallocSizeOf)] +pub struct CSSErrorReporter { + pub pipelineid: PipelineId, + // Arc+Mutex combo is necessary to make this struct Sync, + // which is necessary to fulfill the bounds required by the + // uses of the ParseErrorReporter trait. + #[ignore_malloc_size_of = "Arc is defined in libstd"] + pub script_chan: Arc>>, +} +unsafe_no_jsmanaged_fields!(CSSErrorReporter); + +impl ParseErrorReporter for CSSErrorReporter { + fn report_error(&self, url: &ServoUrl, location: SourceLocation, error: ContextualParseError) { + if log_enabled!(log::Level::Info) { + info!( + "Url:\t{}\n{}:{} {}", + url.as_str(), + location.line, + location.column, + error + ) + } + + //TODO: report a real filename + let _ = self + .script_chan + .lock() + .unwrap() + .send(ConstellationControlMsg::ReportCSSError( + self.pipelineid, + "".to_owned(), + location.line, + location.column, + error.to_string(), + )); + } +} diff --git a/components/script_layout_interface/Cargo.toml b/components/script_layout_interface/Cargo.toml index caae48d943b..1edde2cb613 100644 --- a/components/script_layout_interface/Cargo.toml +++ b/components/script_layout_interface/Cargo.toml @@ -21,7 +21,6 @@ gfx_traits = {path = "../gfx_traits"} html5ever = "0.22" ipc-channel = "0.11" libc = "0.2" -log = "0.4" time = "0.1.17" malloc_size_of = { path = "../malloc_size_of" } malloc_size_of_derive = "0.1" diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index d4e25c1b3c7..ad333209033 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -11,12 +11,9 @@ #[macro_use] extern crate html5ever; #[macro_use] -extern crate log; -#[macro_use] extern crate malloc_size_of_derive; pub mod message; -pub mod reporter; pub mod rpc; pub mod wrapper_traits; diff --git a/components/script_layout_interface/reporter.rs b/components/script_layout_interface/reporter.rs deleted file mode 100644 index 0a558564a2c..00000000000 --- a/components/script_layout_interface/reporter.rs +++ /dev/null @@ -1,48 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -use cssparser::SourceLocation; -use ipc_channel::ipc::IpcSender; -use msg::constellation_msg::PipelineId; -use script_traits::ConstellationControlMsg; -use servo_url::ServoUrl; -use std::sync::{Arc, Mutex}; -use style::error_reporting::{ContextualParseError, ParseErrorReporter}; - -#[derive(Clone, MallocSizeOf)] -pub struct CSSErrorReporter { - pub pipelineid: PipelineId, - // Arc+Mutex combo is necessary to make this struct Sync, - // which is necessary to fulfill the bounds required by the - // uses of the ParseErrorReporter trait. - #[ignore_malloc_size_of = "Arc is defined in libstd"] - pub script_chan: Arc>>, -} - -impl ParseErrorReporter for CSSErrorReporter { - fn report_error(&self, url: &ServoUrl, location: SourceLocation, error: ContextualParseError) { - if log_enabled!(log::Level::Info) { - info!( - "Url:\t{}\n{}:{} {}", - url.as_str(), - location.line, - location.column, - error - ) - } - - //TODO: report a real filename - let _ = self - .script_chan - .lock() - .unwrap() - .send(ConstellationControlMsg::ReportCSSError( - self.pipelineid, - "".to_owned(), - location.line, - location.column, - error.to_string(), - )); - } -} From fc75719ff213427945b8ad7580d102eb5148a314 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 10:37:41 +0100 Subject: [PATCH 04/20] Remove BlockFlowDisplayListBuilding Replaced by inherent methods on BlockFlow, the only implementor of that trait. --- components/layout/block.rs | 7 +- components/layout/display_list/builder.rs | 85 ++--------------------- components/layout/display_list/mod.rs | 1 - components/layout/table.rs | 4 +- components/layout/table_caption.rs | 5 +- components/layout/table_cell.rs | 5 +- components/layout/table_row.rs | 5 +- components/layout/table_rowgroup.rs | 5 +- components/layout/table_wrapper.rs | 4 +- components/layout_thread/lib.rs | 2 +- components/script/script_thread.rs | 2 +- 11 files changed, 26 insertions(+), 99 deletions(-) diff --git a/components/layout/block.rs b/components/layout/block.rs index 8c4a8a771aa..0e4bf140d42 100644 --- a/components/layout/block.rs +++ b/components/layout/block.rs @@ -27,9 +27,10 @@ use crate::context::LayoutContext; use crate::display_list::items::DisplayListSection; -use crate::display_list::StackingContextCollectionState; -use crate::display_list::{BlockFlowDisplayListBuilding, BorderPaintingMode}; -use crate::display_list::{DisplayListBuildState, StackingContextCollectionFlags}; +use crate::display_list::{ + BorderPaintingMode, DisplayListBuildState, StackingContextCollectionFlags, + StackingContextCollectionState, +}; use crate::floats::{ClearType, FloatKind, Floats, PlacementInfo}; use crate::flow::{ BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, ForceNonfloatedFlag, GetBaseFlow, diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index e3dd66b8a2b..6bf719ff17e 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -2344,81 +2344,6 @@ bitflags! { } } -pub trait BlockFlowDisplayListBuilding { - fn collect_stacking_contexts_for_block( - &mut self, - state: &mut StackingContextCollectionState, - flags: StackingContextCollectionFlags, - ); - - fn transform_clip_to_coordinate_space( - &mut self, - state: &mut StackingContextCollectionState, - preserved_state: &mut SavedStackingContextCollectionState, - ); - fn setup_clipping_for_block( - &mut self, - state: &mut StackingContextCollectionState, - preserved_state: &mut SavedStackingContextCollectionState, - stacking_context_type: Option, - established_reference_frame: Option, - flags: StackingContextCollectionFlags, - ) -> ClippingAndScrolling; - fn setup_clip_scroll_node_for_position( - &mut self, - state: &mut StackingContextCollectionState, - border_box: Rect, - ); - fn setup_clip_scroll_node_for_overflow( - &mut self, - state: &mut StackingContextCollectionState, - border_box: Rect, - ); - fn setup_clip_scroll_node_for_css_clip( - &mut self, - state: &mut StackingContextCollectionState, - preserved_state: &mut SavedStackingContextCollectionState, - stacking_relative_border_box: Rect, - ); - fn create_pseudo_stacking_context_for_block( - &mut self, - stacking_context_type: StackingContextType, - parent_stacking_context_id: StackingContextId, - parent_clip_and_scroll_info: ClippingAndScrolling, - state: &mut StackingContextCollectionState, - ); - fn create_real_stacking_context_for_block( - &mut self, - parent_stacking_context_id: StackingContextId, - parent_clipping_and_scrolling: ClippingAndScrolling, - established_reference_frame: Option, - state: &mut StackingContextCollectionState, - ); - fn build_display_list_for_block( - &mut self, - state: &mut DisplayListBuildState, - border_painting_mode: BorderPaintingMode, - ); - fn build_display_list_for_block_no_damage( - &self, - state: &mut DisplayListBuildState, - border_painting_mode: BorderPaintingMode, - ); - fn build_display_list_for_background_if_applicable_with_background( - &self, - state: &mut DisplayListBuildState, - background: &style_structs::Background, - background_color: RGBA, - ); - - fn stacking_context_type( - &self, - flags: StackingContextCollectionFlags, - ) -> Option; - - fn is_reference_frame(&self, context_type: Option) -> bool; -} - /// This structure manages ensuring that modification to StackingContextCollectionState is /// only temporary. It's useful for moving recursively down the flow tree and ensuring /// that the state is restored for siblings. To use this structure, we must call @@ -2498,7 +2423,7 @@ impl SavedStackingContextCollectionState { } } -impl BlockFlowDisplayListBuilding for BlockFlow { +impl BlockFlow { fn transform_clip_to_coordinate_space( &mut self, state: &mut StackingContextCollectionState, @@ -2576,7 +2501,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { } } - fn collect_stacking_contexts_for_block( + pub fn collect_stacking_contexts_for_block( &mut self, state: &mut StackingContextCollectionState, flags: StackingContextCollectionFlags, @@ -2970,7 +2895,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { self.base.collect_stacking_contexts_for_children(state); } - fn build_display_list_for_block_no_damage( + pub fn build_display_list_for_block_no_damage( &self, state: &mut DisplayListBuildState, border_painting_mode: BorderPaintingMode, @@ -3005,7 +2930,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { state.processing_scrolling_overflow_element = false; } - fn build_display_list_for_block( + pub fn build_display_list_for_block( &mut self, state: &mut DisplayListBuildState, border_painting_mode: BorderPaintingMode, @@ -3016,7 +2941,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow { self.build_display_list_for_block_no_damage(state, border_painting_mode); } - fn build_display_list_for_background_if_applicable_with_background( + pub fn build_display_list_for_background_if_applicable_with_background( &self, state: &mut DisplayListBuildState, background: &style_structs::Background, diff --git a/components/layout/display_list/mod.rs b/components/layout/display_list/mod.rs index ed4320ac7b2..1af9d7981c1 100644 --- a/components/layout/display_list/mod.rs +++ b/components/layout/display_list/mod.rs @@ -2,7 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -pub use self::builder::BlockFlowDisplayListBuilding; pub use self::builder::BorderPaintingMode; pub use self::builder::DisplayListBuildState; pub use self::builder::FlexFlowDisplayListBuilding; diff --git a/components/layout/table.rs b/components/layout/table.rs index 83d3552a988..f7bf902e452 100644 --- a/components/layout/table.rs +++ b/components/layout/table.rs @@ -7,9 +7,9 @@ use crate::block::{BlockFlow, CandidateBSizeIterator, ISizeAndMarginsComputer}; use crate::block::{ISizeConstraintInput, ISizeConstraintSolution}; use crate::context::LayoutContext; -use crate::display_list::{BlockFlowDisplayListBuilding, BorderPaintingMode}; use crate::display_list::{ - DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState, + BorderPaintingMode, DisplayListBuildState, StackingContextCollectionFlags, + StackingContextCollectionState, }; use crate::flow::{ BaseFlow, EarlyAbsolutePositionInfo, Flow, FlowClass, GetBaseFlow, ImmutableFlowUtils, diff --git a/components/layout/table_caption.rs b/components/layout/table_caption.rs index b217aa65f8b..883d5d6501d 100644 --- a/components/layout/table_caption.rs +++ b/components/layout/table_caption.rs @@ -6,8 +6,9 @@ use crate::block::BlockFlow; use crate::context::LayoutContext; -use crate::display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState}; -use crate::display_list::{StackingContextCollectionFlags, StackingContextCollectionState}; +use crate::display_list::{ + DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState, +}; use crate::flow::{Flow, FlowClass, OpaqueFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use app_units::Au; diff --git a/components/layout/table_cell.rs b/components/layout/table_cell.rs index b5d95b1e4dd..d72a6670576 100644 --- a/components/layout/table_cell.rs +++ b/components/layout/table_cell.rs @@ -6,8 +6,9 @@ use crate::block::{BlockFlow, ISizeAndMarginsComputer, MarginsMayCollapseFlag}; use crate::context::LayoutContext; -use crate::display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState}; -use crate::display_list::{StackingContextCollectionFlags, StackingContextCollectionState}; +use crate::display_list::{ + DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState, +}; use crate::flow::{Flow, FlowClass, FlowFlags, GetBaseFlow, OpaqueFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use crate::layout_debug; diff --git a/components/layout/table_row.rs b/components/layout/table_row.rs index ca33eb2584e..22db572c166 100644 --- a/components/layout/table_row.rs +++ b/components/layout/table_row.rs @@ -6,8 +6,9 @@ use crate::block::{BlockFlow, ISizeAndMarginsComputer}; use crate::context::LayoutContext; -use crate::display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState}; -use crate::display_list::{StackingContextCollectionFlags, StackingContextCollectionState}; +use crate::display_list::{ + DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState, +}; use crate::flow::{ EarlyAbsolutePositionInfo, Flow, FlowClass, GetBaseFlow, ImmutableFlowUtils, OpaqueFlow, }; diff --git a/components/layout/table_rowgroup.rs b/components/layout/table_rowgroup.rs index b2afd3c2aba..814cbe0d2d2 100644 --- a/components/layout/table_rowgroup.rs +++ b/components/layout/table_rowgroup.rs @@ -6,8 +6,9 @@ use crate::block::{BlockFlow, ISizeAndMarginsComputer}; use crate::context::LayoutContext; -use crate::display_list::{BlockFlowDisplayListBuilding, DisplayListBuildState}; -use crate::display_list::{StackingContextCollectionFlags, StackingContextCollectionState}; +use crate::display_list::{ + DisplayListBuildState, StackingContextCollectionFlags, StackingContextCollectionState, +}; use crate::flow::{Flow, FlowClass, OpaqueFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; use crate::layout_debug; diff --git a/components/layout/table_wrapper.rs b/components/layout/table_wrapper.rs index f63cf3f28f0..9e8f137e70a 100644 --- a/components/layout/table_wrapper.rs +++ b/components/layout/table_wrapper.rs @@ -17,9 +17,7 @@ use crate::block::{ use crate::block::{ISizeConstraintSolution, MarginsMayCollapseFlag}; use crate::context::LayoutContext; use crate::display_list::StackingContextCollectionState; -use crate::display_list::{ - BlockFlowDisplayListBuilding, DisplayListBuildState, StackingContextCollectionFlags, -}; +use crate::display_list::{DisplayListBuildState, StackingContextCollectionFlags}; use crate::floats::FloatKind; use crate::flow::{Flow, FlowClass, FlowFlags, ImmutableFlowUtils, OpaqueFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index 47774b86ec3..aebfeebe440 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -75,7 +75,7 @@ use parking_lot::RwLock; use profile_traits::mem::{self as profile_mem, Report, ReportKind, ReportsChan}; use profile_traits::time::{self as profile_time, profile, TimerMetadata}; use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; -use script_layout_interface::message::{Msg, LayoutThreadInit, NodesFromPointQueryType, Reflow}; +use script_layout_interface::message::{LayoutThreadInit, Msg, NodesFromPointQueryType, Reflow}; use script_layout_interface::message::{QueryMsg, ReflowComplete, ReflowGoal, ScriptReflow}; use script_layout_interface::rpc::TextIndexResponse; use script_layout_interface::rpc::{LayoutRPC, OffsetParentResponse, StyleResponse}; diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index fd08bb5f80c..99965a521a6 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -118,7 +118,7 @@ use net_traits::{ }; use profile_traits::mem::{self as profile_mem, OpaqueSender, ReportsChan}; use profile_traits::time::{self as profile_time, profile, ProfilerCategory}; -use script_layout_interface::message::{self, Msg, LayoutThreadInit, ReflowGoal}; +use script_layout_interface::message::{self, LayoutThreadInit, Msg, ReflowGoal}; use script_traits::webdriver_msg::WebDriverScriptCommand; use script_traits::CompositorEvent::{ CompositionEvent, KeyboardEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent, TouchEvent, From f8c4a87a307f31393bab183f58a61d86bb6721c3 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 10:54:23 +0100 Subject: [PATCH 05/20] Remove BaseFlowDisplayListBuilding --- components/layout/display_list/builder.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 6bf719ff17e..f566da00708 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -3144,15 +3144,7 @@ impl FlexFlowDisplayListBuilding for FlexFlow { } } -trait BaseFlowDisplayListBuilding { - fn build_display_items_for_debugging_tint( - &self, - state: &mut DisplayListBuildState, - node: OpaqueNode, - ); -} - -impl BaseFlowDisplayListBuilding for BaseFlow { +impl BaseFlow { fn build_display_items_for_debugging_tint( &self, state: &mut DisplayListBuildState, From 6e0a7dc3b227046bed9ec29f0b5d40a5c4946e73 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:04:39 +0100 Subject: [PATCH 06/20] Remove FragmentDisplayListBuilding --- components/layout/display_list/builder.rs | 271 ++++------------------ 1 file changed, 46 insertions(+), 225 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index f566da00708..85599cc12ac 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -581,230 +581,6 @@ impl<'a> DisplayListBuildState<'a> { /// The logical width of an insertion point: at the moment, a one-pixel-wide line. const INSERTION_POINT_LOGICAL_WIDTH: Au = Au(1 * AU_PER_PX); -pub trait FragmentDisplayListBuilding { - fn collect_stacking_contexts_for_blocklike_fragment( - &mut self, - state: &mut StackingContextCollectionState, - ) -> bool; - - fn create_stacking_context_for_inline_block( - &mut self, - base: &BaseFlow, - state: &mut StackingContextCollectionState, - ) -> bool; - - /// Adds the display items necessary to paint the background of this fragment to the display - /// list if necessary. - fn build_display_list_for_background_if_applicable( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - display_list_section: DisplayListSection, - absolute_bounds: Rect, - ); - - /// Same as build_display_list_for_background_if_applicable, but lets you - /// override the actual background used - fn build_display_list_for_background_if_applicable_with_background( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - background: &style_structs::Background, - background_color: RGBA, - display_list_section: DisplayListSection, - absolute_bounds: Rect, - ); - - /// Adds the display items necessary to paint a webrender image of this fragment to the - /// appropriate section of the display list. - fn build_display_list_for_webrender_image( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - display_list_section: DisplayListSection, - absolute_bounds: Rect, - webrender_image: WebRenderImageInfo, - index: usize, - ); - - /// Calculates the webrender image for a paint worklet. - /// Returns None if the worklet is not registered. - /// If the worklet has missing image URLs, it passes them to the image cache for loading. - fn get_webrender_image_for_paint_worklet( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - paint_worklet: &PaintWorklet, - size: Size2D, - ) -> Option; - - /// Adds the display items necessary to paint the background linear gradient of this fragment - /// to the appropriate section of the display list. - fn build_display_list_for_background_gradient( - &self, - state: &mut DisplayListBuildState, - display_list_section: DisplayListSection, - absolute_bounds: Rect, - gradient: &Gradient, - style: &ComputedValues, - index: usize, - ); - - /// Adds the display items necessary to paint the borders of this fragment to a display list if - /// necessary. - fn build_display_list_for_borders_if_applicable( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - inline_node_info: Option, - border_painting_mode: BorderPaintingMode, - bounds: Rect, - display_list_section: DisplayListSection, - clip: Rect, - ); - - /// Add display item for image border. - /// - /// Returns `Some` if the addition was successful. - fn build_display_list_for_border_image( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - base: BaseDisplayItem, - bounds: Rect, - image: &ComputedImage, - border_width: SideOffsets2D, - ) -> Option<()>; - - /// Adds the display items necessary to paint the outline of this fragment to the display list - /// if necessary. - fn build_display_list_for_outline_if_applicable( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - bounds: Rect, - clip: Rect, - ); - - /// Adds the display items necessary to paint the box shadow of this fragment to the display - /// list if necessary. - fn build_display_list_for_box_shadow_if_applicable( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - display_list_section: DisplayListSection, - absolute_bounds: Rect, - clip: Rect, - ); - - /// Adds display items necessary to draw debug boxes around a scanned text fragment. - fn build_debug_borders_around_text_fragments( - &self, - state: &mut DisplayListBuildState, - style: &ComputedValues, - stacking_relative_border_box: Rect, - stacking_relative_content_box: Rect, - text_fragment: &ScannedTextFragmentInfo, - clip: Rect, - ); - - /// Adds display items necessary to draw debug boxes around this fragment. - fn build_debug_borders_around_fragment( - &self, - state: &mut DisplayListBuildState, - stacking_relative_border_box: Rect, - clip: Rect, - ); - - /// Adds the display items for this fragment to the given display list. - /// - /// Arguments: - /// - /// * `state`: The display building state, including the display list currently - /// under construction and other metadata useful for constructing it. - /// * `dirty`: The dirty rectangle in the coordinate system of the owning flow. - /// * `clip`: The region to clip the display items to. - /// * `overflow_content_size`: The size of content associated with this fragment - /// that must have overflow handling applied to it. For a scrollable block - /// flow, it is expected that this is the size of the child boxes. - fn build_display_list( - &mut self, - state: &mut DisplayListBuildState, - stacking_relative_border_box: Rect, - border_painting_mode: BorderPaintingMode, - display_list_section: DisplayListSection, - clip: Rect, - overflow_content_size: Option>, - ); - - /// build_display_list, but don't update the restyle damage - /// - /// Must be paired with a self.restyle_damage.remove(REPAINT) somewhere - fn build_display_list_no_damage( - &self, - state: &mut DisplayListBuildState, - stacking_relative_border_box: Rect, - border_painting_mode: BorderPaintingMode, - display_list_section: DisplayListSection, - clip: Rect, - overflow_content_size: Option>, - ); - - /// Builds the display items necessary to paint the selection and/or caret for this fragment, - /// if any. - fn build_display_items_for_selection_if_necessary( - &self, - state: &mut DisplayListBuildState, - stacking_relative_border_box: Rect, - display_list_section: DisplayListSection, - clip: Rect, - ); - - /// Creates the text display item for one text fragment. This can be called multiple times for - /// one fragment if there are text shadows. - /// - /// `text_shadow` will be `Some` if this is rendering a shadow. - fn build_display_list_for_text_fragment( - &self, - state: &mut DisplayListBuildState, - text_fragment: &ScannedTextFragmentInfo, - stacking_relative_content_box: Rect, - text_shadows: &[SimpleShadow], - clip: Rect, - ); - - /// Creates the display item for a text decoration: underline, overline, or line-through. - fn build_display_list_for_text_decoration( - &self, - state: &mut DisplayListBuildState, - color: &RGBA, - stacking_relative_box: &LogicalRect, - clip: Rect, - ); - - /// A helper method that `build_display_list` calls to create per-fragment-type display items. - fn build_fragment_type_specific_display_items( - &self, - state: &mut DisplayListBuildState, - stacking_relative_border_box: Rect, - clip: Rect, - ); - - /// Creates a stacking context for associated fragment. - fn create_stacking_context( - &self, - id: StackingContextId, - base_flow: &BaseFlow, - context_type: StackingContextType, - established_reference_frame: Option, - parent_clipping_and_scrolling: ClippingAndScrolling, - ) -> StackingContext; - - fn unique_id(&self) -> u64; - - fn fragment_type(&self) -> FragmentType; -} - /// Get the border radius for the rectangle inside of a rounded border. This is useful /// for building the clip for the content inside the border. fn build_border_radius_for_inner_rect( @@ -823,7 +599,7 @@ fn build_border_radius_for_inner_rect( border::inner_radii(radii, border_widths) } -impl FragmentDisplayListBuilding for Fragment { +impl Fragment { fn collect_stacking_contexts_for_blocklike_fragment( &mut self, state: &mut StackingContextCollectionState, @@ -881,6 +657,8 @@ impl FragmentDisplayListBuilding for Fragment { true } + /// Adds the display items necessary to paint the background of this fragment to the display + /// list if necessary. fn build_display_list_for_background_if_applicable( &self, state: &mut DisplayListBuildState, @@ -902,6 +680,8 @@ impl FragmentDisplayListBuilding for Fragment { ) } + /// Same as build_display_list_for_background_if_applicable, but lets you + /// override the actual background used fn build_display_list_for_background_if_applicable_with_background( &self, state: &mut DisplayListBuildState, @@ -1028,6 +808,8 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// Adds the display items necessary to paint a webrender image of this fragment to the + /// appropriate section of the display list. fn build_display_list_for_webrender_image( &self, state: &mut DisplayListBuildState, @@ -1088,6 +870,9 @@ impl FragmentDisplayListBuilding for Fragment { }); } + /// Calculates the webrender image for a paint worklet. + /// Returns None if the worklet is not registered. + /// If the worklet has missing image URLs, it passes them to the image cache for loading. fn get_webrender_image_for_paint_worklet( &self, state: &mut DisplayListBuildState, @@ -1148,6 +933,8 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// Adds the display items necessary to paint the background linear gradient of this fragment + /// to the appropriate section of the display list. fn build_display_list_for_background_gradient( &self, state: &mut DisplayListBuildState, @@ -1220,6 +1007,8 @@ impl FragmentDisplayListBuilding for Fragment { }); } + /// Adds the display items necessary to paint the box shadow of this fragment to the display + /// list if necessary. fn build_display_list_for_box_shadow_if_applicable( &self, state: &mut DisplayListBuildState, @@ -1269,6 +1058,8 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// Adds the display items necessary to paint the borders of this fragment to a display list if + /// necessary. fn build_display_list_for_borders_if_applicable( &self, state: &mut DisplayListBuildState, @@ -1381,6 +1172,9 @@ impl FragmentDisplayListBuilding for Fragment { ))); } + /// Add display item for image border. + /// + /// Returns `Some` if the addition was successful. fn build_display_list_for_border_image( &self, state: &mut DisplayListBuildState, @@ -1483,6 +1277,8 @@ impl FragmentDisplayListBuilding for Fragment { Some(()) } + /// Adds the display items necessary to paint the outline of this fragment to the display list + /// if necessary. fn build_display_list_for_outline_if_applicable( &self, state: &mut DisplayListBuildState, @@ -1532,6 +1328,7 @@ impl FragmentDisplayListBuilding for Fragment { ))); } + /// Adds display items necessary to draw debug boxes around a scanned text fragment. fn build_debug_borders_around_text_fragments( &self, state: &mut DisplayListBuildState, @@ -1594,6 +1391,7 @@ impl FragmentDisplayListBuilding for Fragment { ))); } + /// Adds display items necessary to draw debug boxes around this fragment. fn build_debug_borders_around_fragment( &self, state: &mut DisplayListBuildState, @@ -1621,6 +1419,8 @@ impl FragmentDisplayListBuilding for Fragment { ))); } + /// Builds the display items necessary to paint the selection and/or caret for this fragment, + /// if any. fn build_display_items_for_selection_if_necessary( &self, state: &mut DisplayListBuildState, @@ -1702,6 +1502,17 @@ impl FragmentDisplayListBuilding for Fragment { ))); } + /// Adds the display items for this fragment to the given display list. + /// + /// Arguments: + /// + /// * `state`: The display building state, including the display list currently + /// under construction and other metadata useful for constructing it. + /// * `dirty`: The dirty rectangle in the coordinate system of the owning flow. + /// * `clip`: The region to clip the display items to. + /// * `overflow_content_size`: The size of content associated with this fragment + /// that must have overflow handling applied to it. For a scrollable block + /// flow, it is expected that this is the size of the child boxes. fn build_display_list( &mut self, state: &mut DisplayListBuildState, @@ -1729,6 +1540,9 @@ impl FragmentDisplayListBuilding for Fragment { state.current_clipping_and_scrolling = previous_clipping_and_scrolling; } + /// build_display_list, but don't update the restyle damage + /// + /// Must be paired with a self.restyle_damage.remove(REPAINT) somewhere fn build_display_list_no_damage( &self, state: &mut DisplayListBuildState, @@ -1886,6 +1700,7 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// A helper method that `build_display_list` calls to create per-fragment-type display items. fn build_fragment_type_specific_display_items( &self, state: &mut DisplayListBuildState, @@ -2087,6 +1902,7 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// Creates a stacking context for associated fragment. fn create_stacking_context( &self, id: StackingContextId, @@ -2139,6 +1955,10 @@ impl FragmentDisplayListBuilding for Fragment { ) } + /// Creates the text display item for one text fragment. This can be called multiple times for + /// one fragment if there are text shadows. + /// + /// `text_shadow` will be `Some` if this is rendering a shadow. fn build_display_list_for_text_fragment( &self, state: &mut DisplayListBuildState, @@ -2290,6 +2110,7 @@ impl FragmentDisplayListBuilding for Fragment { } } + /// Creates the display item for a text decoration: underline, overline, or line-through. fn build_display_list_for_text_decoration( &self, state: &mut DisplayListBuildState, From 7a704d2f04127292e73c5b622033b9a10b5cb3db Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:20:15 +0100 Subject: [PATCH 07/20] Kill establishes_containing_block_for_absolute We inline the related tests at the two call sites. --- components/layout/display_list/builder.rs | 30 ++++++----------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 85599cc12ac..f6cc801eb3c 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -71,19 +71,6 @@ use webrender_api::{ExternalScrollId, FilterOp, GlyphInstance, ImageRendering, L use webrender_api::{LayoutSize, LayoutTransform, LayoutVector2D, LineStyle, NinePatchBorder}; use webrender_api::{NinePatchBorderSource, NormalBorder, ScrollSensitivity, StickyOffsetBounds}; -fn establishes_containing_block_for_absolute( - flags: StackingContextCollectionFlags, - positioning: StylePosition, - established_reference_frame: bool, -) -> bool { - if established_reference_frame { - return true; - } - - !flags.contains(StackingContextCollectionFlags::POSITION_NEVER_CREATES_CONTAINING_BLOCK) && - StylePosition::Static != positioning -} - trait RgbColor { fn rgb(r: u8, g: u8, b: u8) -> Self; } @@ -2360,11 +2347,11 @@ impl BlockFlow { flags, ); - if establishes_containing_block_for_absolute( - flags, - self.positioning(), - established_reference_frame.is_some(), - ) { + let creates_containing_block = !flags + .contains(StackingContextCollectionFlags::POSITION_NEVER_CREATES_CONTAINING_BLOCK); + let abspos_containing_block = established_reference_frame.is_some() || + (creates_containing_block && self.positioning() != StylePosition::Static); + if abspos_containing_block { state.containing_block_clipping_and_scrolling = state.current_clipping_and_scrolling; } @@ -2842,11 +2829,8 @@ impl InlineFlowDisplayListBuilding for InlineFlow { for fragment in self.fragments.fragments.iter_mut() { state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; - if establishes_containing_block_for_absolute( - StackingContextCollectionFlags::empty(), - fragment.style.get_box().position, - false, - ) { + let abspos_containing_block = fragment.style.get_box().position != StylePosition::Static; + if abspos_containing_block { state.containing_block_clipping_and_scrolling = state.current_clipping_and_scrolling; } From 7d57c9b3ab14637644457bc3642656cf0cecb74c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:31:47 +0100 Subject: [PATCH 08/20] Remove InlineFlowDisplayListBuilding This lets us remove a few call indirections where a function just calls another function defined in another module. --- components/layout/display_list/builder.rs | 115 +--------------------- components/layout/display_list/mod.rs | 1 - components/layout/inline.rs | 99 +++++++++++++++++-- 3 files changed, 98 insertions(+), 117 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index f6cc801eb3c..ee6acf48f36 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -27,7 +27,7 @@ use crate::flow::{BaseFlow, Flow, FlowFlags}; use crate::flow_ref::FlowRef; use crate::fragment::SpecificFragmentInfo; use crate::fragment::{CanvasFragmentSource, CoordinateSystem, Fragment, ScannedTextFragmentInfo}; -use crate::inline::{InlineFlow, InlineFragmentNodeFlags}; +use crate::inline::InlineFragmentNodeFlags; use crate::list_item::ListItemFlow; use crate::model::MaybeAuto; use crate::table_cell::CollapsedBordersForCell; @@ -587,7 +587,7 @@ fn build_border_radius_for_inner_rect( } impl Fragment { - fn collect_stacking_contexts_for_blocklike_fragment( + pub fn collect_stacking_contexts_for_blocklike_fragment( &mut self, state: &mut StackingContextCollectionState, ) -> bool { @@ -615,7 +615,7 @@ impl Fragment { } } - fn create_stacking_context_for_inline_block( + pub fn create_stacking_context_for_inline_block( &mut self, base: &BaseFlow, state: &mut StackingContextCollectionState, @@ -1500,7 +1500,7 @@ impl Fragment { /// * `overflow_content_size`: The size of content associated with this fragment /// that must have overflow handling applied to it. For a scrollable block /// flow, it is expected that this is the size of the child boxes. - fn build_display_list( + pub fn build_display_list( &mut self, state: &mut DisplayListBuildState, stacking_relative_border_box: Rect, @@ -2804,111 +2804,6 @@ impl BlockFlow { } } -pub trait InlineFlowDisplayListBuilding { - fn collect_stacking_contexts_for_inline(&mut self, state: &mut StackingContextCollectionState); - fn build_display_list_for_inline_fragment_at_index( - &mut self, - state: &mut DisplayListBuildState, - index: usize, - ); - fn build_display_list_for_inline(&mut self, state: &mut DisplayListBuildState); -} - -impl InlineFlowDisplayListBuilding for InlineFlow { - fn collect_stacking_contexts_for_inline(&mut self, state: &mut StackingContextCollectionState) { - self.base.stacking_context_id = state.current_stacking_context_id; - self.base.clipping_and_scrolling = Some(state.current_clipping_and_scrolling); - self.base.clip = state - .clip_stack - .last() - .cloned() - .unwrap_or_else(Rect::max_rect); - - let previous_cb_clipping_and_scrolling = state.containing_block_clipping_and_scrolling; - - for fragment in self.fragments.fragments.iter_mut() { - state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; - - let abspos_containing_block = fragment.style.get_box().position != StylePosition::Static; - if abspos_containing_block { - state.containing_block_clipping_and_scrolling = - state.current_clipping_and_scrolling; - } - - // We clear this here, but it might be set again if we create a stacking context for - // this fragment. - fragment.established_reference_frame = None; - - if !fragment.collect_stacking_contexts_for_blocklike_fragment(state) { - if !fragment.establishes_stacking_context() { - fragment.stacking_context_id = state.current_stacking_context_id; - } else { - fragment.create_stacking_context_for_inline_block(&self.base, state); - } - } - - // Reset the containing block clipping and scrolling before each loop iteration, - // so we don't pollute subsequent fragments. - state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; - } - } - - fn build_display_list_for_inline_fragment_at_index( - &mut self, - state: &mut DisplayListBuildState, - index: usize, - ) { - let fragment = self.fragments.fragments.get_mut(index).unwrap(); - let stacking_relative_border_box = self - .base - .stacking_relative_border_box_for_display_list(fragment); - fragment.build_display_list( - state, - stacking_relative_border_box, - BorderPaintingMode::Separate, - DisplayListSection::Content, - self.base.clip, - None, - ); - } - - fn build_display_list_for_inline(&mut self, state: &mut DisplayListBuildState) { - debug!( - "Flow: building display list for {} inline fragments", - self.fragments.len() - ); - - // We iterate using an index here, because we want to avoid doing a doing - // a double-borrow of self (one mutable for the method call and one immutable - // for the self.fragments.fragment iterator itself). - for index in 0..self.fragments.fragments.len() { - let (establishes_stacking_context, stacking_context_id) = { - let fragment = self.fragments.fragments.get(index).unwrap(); - ( - self.base.stacking_context_id != fragment.stacking_context_id, - fragment.stacking_context_id, - ) - }; - - let parent_stacking_context_id = state.current_stacking_context_id; - if establishes_stacking_context { - state.current_stacking_context_id = stacking_context_id; - } - - self.build_display_list_for_inline_fragment_at_index(state, index); - - if establishes_stacking_context { - state.current_stacking_context_id = parent_stacking_context_id - } - } - - if !self.fragments.fragments.is_empty() { - self.base - .build_display_items_for_debugging_tint(state, self.fragments.fragments[0].node); - } - } -} - pub trait ListItemFlowDisplayListBuilding { fn build_display_list_for_list_item(&mut self, state: &mut DisplayListBuildState); } @@ -2950,7 +2845,7 @@ impl FlexFlowDisplayListBuilding for FlexFlow { } impl BaseFlow { - fn build_display_items_for_debugging_tint( + pub fn build_display_items_for_debugging_tint( &self, state: &mut DisplayListBuildState, node: OpaqueNode, diff --git a/components/layout/display_list/mod.rs b/components/layout/display_list/mod.rs index 1af9d7981c1..48c95c8ce61 100644 --- a/components/layout/display_list/mod.rs +++ b/components/layout/display_list/mod.rs @@ -6,7 +6,6 @@ pub use self::builder::BorderPaintingMode; pub use self::builder::DisplayListBuildState; pub use self::builder::FlexFlowDisplayListBuilding; pub use self::builder::IndexableText; -pub use self::builder::InlineFlowDisplayListBuilding; pub use self::builder::ListItemFlowDisplayListBuilding; pub use self::builder::StackingContextCollectionFlags; pub use self::builder::StackingContextCollectionState; diff --git a/components/layout/inline.rs b/components/layout/inline.rs index 34161a16702..bef95a64f96 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -4,9 +4,10 @@ use crate::block::AbsoluteAssignBSizesTraversal; use crate::context::{LayoutContext, LayoutFontContext}; -use crate::display_list::items::OpaqueNode; -use crate::display_list::StackingContextCollectionState; -use crate::display_list::{DisplayListBuildState, InlineFlowDisplayListBuilding}; +use crate::display_list::items::{DisplayListSection, OpaqueNode}; +use crate::display_list::{ + BorderPaintingMode, DisplayListBuildState, StackingContextCollectionState, +}; use crate::floats::{FloatKind, Floats, PlacementInfo}; use crate::flow::{BaseFlow, Flow, FlowClass, ForceNonfloatedFlag}; use crate::flow::{EarlyAbsolutePositionInfo, FlowFlags, GetBaseFlow, OpaqueFlow}; @@ -20,11 +21,12 @@ use crate::text; use crate::traversal::PreorderFlowTraversal; use crate::ServoArc; use app_units::{Au, MIN_AU}; -use euclid::{Point2D, Size2D}; +use euclid::{Point2D, Rect, Size2D}; use gfx::font::FontMetrics; use gfx_traits::print_tree::PrintTree; use range::{Range, RangeIndex}; use script_layout_interface::wrapper_traits::PseudoElementType; +use servo_geometry::MaxRect; use std::cmp::max; use std::collections::VecDeque; use std::sync::Arc; @@ -1427,6 +1429,25 @@ impl InlineFlow { } None } + + fn build_display_list_for_inline_fragment_at_index( + &mut self, + state: &mut DisplayListBuildState, + index: usize, + ) { + let fragment = self.fragments.fragments.get_mut(index).unwrap(); + let stacking_relative_border_box = self + .base + .stacking_relative_border_box_for_display_list(fragment); + fragment.build_display_list( + state, + stacking_relative_border_box, + BorderPaintingMode::Separate, + DisplayListSection::Content, + self.base.clip, + None, + ); + } } impl Flow for InlineFlow { @@ -1830,11 +1851,77 @@ impl Flow for InlineFlow { fn update_late_computed_block_position_if_necessary(&mut self, _: Au) {} fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) { - self.collect_stacking_contexts_for_inline(state); + self.base.stacking_context_id = state.current_stacking_context_id; + self.base.clipping_and_scrolling = Some(state.current_clipping_and_scrolling); + self.base.clip = state + .clip_stack + .last() + .cloned() + .unwrap_or_else(Rect::max_rect); + + let previous_cb_clipping_and_scrolling = state.containing_block_clipping_and_scrolling; + + for fragment in self.fragments.fragments.iter_mut() { + state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; + + let abspos_containing_block = fragment.style.get_box().position != Position::Static; + if abspos_containing_block { + state.containing_block_clipping_and_scrolling = + state.current_clipping_and_scrolling; + } + + // We clear this here, but it might be set again if we create a stacking context for + // this fragment. + fragment.established_reference_frame = None; + + if !fragment.collect_stacking_contexts_for_blocklike_fragment(state) { + if !fragment.establishes_stacking_context() { + fragment.stacking_context_id = state.current_stacking_context_id; + } else { + fragment.create_stacking_context_for_inline_block(&self.base, state); + } + } + + // Reset the containing block clipping and scrolling before each loop iteration, + // so we don't pollute subsequent fragments. + state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; + } } fn build_display_list(&mut self, state: &mut DisplayListBuildState) { - self.build_display_list_for_inline(state); + debug!( + "Flow: building display list for {} inline fragments", + self.fragments.len() + ); + + // We iterate using an index here, because we want to avoid doing a doing + // a double-borrow of self (one mutable for the method call and one immutable + // for the self.fragments.fragment iterator itself). + for index in 0..self.fragments.fragments.len() { + let (establishes_stacking_context, stacking_context_id) = { + let fragment = self.fragments.fragments.get(index).unwrap(); + ( + self.base.stacking_context_id != fragment.stacking_context_id, + fragment.stacking_context_id, + ) + }; + + let parent_stacking_context_id = state.current_stacking_context_id; + if establishes_stacking_context { + state.current_stacking_context_id = stacking_context_id; + } + + self.build_display_list_for_inline_fragment_at_index(state, index); + + if establishes_stacking_context { + state.current_stacking_context_id = parent_stacking_context_id + } + } + + if !self.fragments.fragments.is_empty() { + self.base + .build_display_items_for_debugging_tint(state, self.fragments.fragments[0].node); + } } fn repair_style(&mut self, _: &ServoArc) {} From f75578cbf10fb7f0484ef2391238565c59df5fb7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:42:42 +0100 Subject: [PATCH 09/20] Remove ListItemFlowDisplayListBuilding --- components/layout/display_list/builder.rs | 29 ----------------------- components/layout/display_list/mod.rs | 1 - components/layout/list_item.rs | 26 +++++++++++++++++--- 3 files changed, 23 insertions(+), 33 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index ee6acf48f36..0e292defc0c 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -28,7 +28,6 @@ use crate::flow_ref::FlowRef; use crate::fragment::SpecificFragmentInfo; use crate::fragment::{CanvasFragmentSource, CoordinateSystem, Fragment, ScannedTextFragmentInfo}; use crate::inline::InlineFragmentNodeFlags; -use crate::list_item::ListItemFlow; use crate::model::MaybeAuto; use crate::table_cell::CollapsedBordersForCell; use app_units::{Au, AU_PER_PX}; @@ -2804,34 +2803,6 @@ impl BlockFlow { } } -pub trait ListItemFlowDisplayListBuilding { - fn build_display_list_for_list_item(&mut self, state: &mut DisplayListBuildState); -} - -impl ListItemFlowDisplayListBuilding for ListItemFlow { - fn build_display_list_for_list_item(&mut self, state: &mut DisplayListBuildState) { - // Draw the marker, if applicable. - for marker in &mut self.marker_fragments { - let stacking_relative_border_box = self - .block_flow - .base - .stacking_relative_border_box_for_display_list(marker); - marker.build_display_list( - state, - stacking_relative_border_box, - BorderPaintingMode::Separate, - DisplayListSection::Content, - self.block_flow.base.clip, - None, - ); - } - - // Draw the rest of the block. - self.block_flow - .build_display_list_for_block(state, BorderPaintingMode::Separate) - } -} - pub trait FlexFlowDisplayListBuilding { fn build_display_list_for_flex(&mut self, state: &mut DisplayListBuildState); } diff --git a/components/layout/display_list/mod.rs b/components/layout/display_list/mod.rs index 48c95c8ce61..ad567c433fd 100644 --- a/components/layout/display_list/mod.rs +++ b/components/layout/display_list/mod.rs @@ -6,7 +6,6 @@ pub use self::builder::BorderPaintingMode; pub use self::builder::DisplayListBuildState; pub use self::builder::FlexFlowDisplayListBuilding; pub use self::builder::IndexableText; -pub use self::builder::ListItemFlowDisplayListBuilding; pub use self::builder::StackingContextCollectionFlags; pub use self::builder::StackingContextCollectionState; pub use self::conversions::ToLayout; diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs index 08ae74f4cbe..3416ff444b3 100644 --- a/components/layout/list_item.rs +++ b/components/layout/list_item.rs @@ -7,8 +7,10 @@ use crate::block::BlockFlow; use crate::context::{with_thread_local_font_context, LayoutContext}; -use crate::display_list::StackingContextCollectionState; -use crate::display_list::{DisplayListBuildState, ListItemFlowDisplayListBuilding}; +use crate::display_list::items::DisplayListSection; +use crate::display_list::{ + BorderPaintingMode, DisplayListBuildState, StackingContextCollectionState, +}; use crate::floats::FloatKind; use crate::flow::{Flow, FlowClass, OpaqueFlow}; use crate::fragment::Overflow; @@ -189,7 +191,25 @@ impl Flow for ListItemFlow { } fn build_display_list(&mut self, state: &mut DisplayListBuildState) { - self.build_display_list_for_list_item(state); + // Draw the marker, if applicable. + for marker in &mut self.marker_fragments { + let stacking_relative_border_box = self + .block_flow + .base + .stacking_relative_border_box_for_display_list(marker); + marker.build_display_list( + state, + stacking_relative_border_box, + BorderPaintingMode::Separate, + DisplayListSection::Content, + self.block_flow.base.clip, + None, + ); + } + + // Draw the rest of the block. + self.block_flow + .build_display_list_for_block(state, BorderPaintingMode::Separate) } fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) { From 21cca5bb53072b45103743aefe5287be80ed1c28 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:44:35 +0100 Subject: [PATCH 10/20] Kill RgbColor --- components/layout/display_list/builder.rs | 31 +++++++---------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 0e292defc0c..b91ce24127d 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -65,25 +65,12 @@ use style::values::{Either, RGBA}; use style_traits::cursor::CursorKind; use style_traits::CSSPixel; use style_traits::ToCss; -use webrender_api::{self, BorderDetails, BorderRadius, BorderSide, BoxShadowClipMode, ColorF}; -use webrender_api::{ExternalScrollId, FilterOp, GlyphInstance, ImageRendering, LayoutRect}; -use webrender_api::{LayoutSize, LayoutTransform, LayoutVector2D, LineStyle, NinePatchBorder}; -use webrender_api::{NinePatchBorderSource, NormalBorder, ScrollSensitivity, StickyOffsetBounds}; - -trait RgbColor { - fn rgb(r: u8, g: u8, b: u8) -> Self; -} - -impl RgbColor for ColorF { - fn rgb(r: u8, g: u8, b: u8) -> Self { - ColorF { - r: (r as f32) / (255.0 as f32), - g: (g as f32) / (255.0 as f32), - b: (b as f32) / (255.0 as f32), - a: 1.0 as f32, - } - } -} +use webrender_api::{ + self, BorderDetails, BorderRadius, BorderSide, BoxShadowClipMode, ColorF, ColorU, + ExternalScrollId, FilterOp, GlyphInstance, ImageRendering, LayoutRect, LayoutSize, + LayoutTransform, LayoutVector2D, LineStyle, NinePatchBorder, NinePatchBorderSource, + NormalBorder, ScrollSensitivity, StickyOffsetBounds, +}; static THREAD_TINT_COLORS: [ColorF; 8] = [ ColorF { @@ -1340,7 +1327,7 @@ impl Fragment { webrender_api::BorderDisplayItem { widths: SideOffsets2D::new_all_same(Au::from_px(1)).to_layout(), details: BorderDetails::Normal(border::simple( - ColorF::rgb(0, 0, 200), + ColorU::new(0, 0, 200, 1).into(), webrender_api::BorderStyle::Solid, )), }, @@ -1371,7 +1358,7 @@ impl Fragment { webrender_api::LineDisplayItem { orientation: webrender_api::LineOrientation::Horizontal, wavy_line_thickness, - color: ColorF::rgb(0, 200, 0), + color: ColorU::new(0, 200, 0, 1).into(), style: LineStyle::Dashed, }, ))); @@ -1397,7 +1384,7 @@ impl Fragment { webrender_api::BorderDisplayItem { widths: SideOffsets2D::new_all_same(Au::from_px(1)).to_layout(), details: BorderDetails::Normal(border::simple( - ColorF::rgb(0, 0, 200), + ColorU::new(0, 0, 200, 1).into(), webrender_api::BorderStyle::Solid, )), }, From a92dd09fc99a4646a919efd68c9dd9fac30e0f49 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:50:37 +0100 Subject: [PATCH 11/20] Remove FlexFlowDisplayListBuilding --- components/layout/display_list/builder.rs | 13 ------------- components/layout/display_list/mod.rs | 1 - components/layout/flex.rs | 9 ++++++--- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index b91ce24127d..13f16e379e9 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -22,7 +22,6 @@ use crate::display_list::items::{PopAllTextShadowsDisplayItem, PushTextShadowDis use crate::display_list::items::{StackingContext, StackingContextType, StickyFrameData}; use crate::display_list::items::{TextOrientation, WebRenderImageInfo}; use crate::display_list::ToLayout; -use crate::flex::FlexFlow; use crate::flow::{BaseFlow, Flow, FlowFlags}; use crate::flow_ref::FlowRef; use crate::fragment::SpecificFragmentInfo; @@ -2790,18 +2789,6 @@ impl BlockFlow { } } -pub trait FlexFlowDisplayListBuilding { - fn build_display_list_for_flex(&mut self, state: &mut DisplayListBuildState); -} - -impl FlexFlowDisplayListBuilding for FlexFlow { - fn build_display_list_for_flex(&mut self, state: &mut DisplayListBuildState) { - // Draw the rest of the block. - self.as_mut_block() - .build_display_list_for_block(state, BorderPaintingMode::Separate) - } -} - impl BaseFlow { pub fn build_display_items_for_debugging_tint( &self, diff --git a/components/layout/display_list/mod.rs b/components/layout/display_list/mod.rs index ad567c433fd..302728fbf22 100644 --- a/components/layout/display_list/mod.rs +++ b/components/layout/display_list/mod.rs @@ -4,7 +4,6 @@ pub use self::builder::BorderPaintingMode; pub use self::builder::DisplayListBuildState; -pub use self::builder::FlexFlowDisplayListBuilding; pub use self::builder::IndexableText; pub use self::builder::StackingContextCollectionFlags; pub use self::builder::StackingContextCollectionState; diff --git a/components/layout/flex.rs b/components/layout/flex.rs index 44055ae7d18..fd85b0c24c1 100644 --- a/components/layout/flex.rs +++ b/components/layout/flex.rs @@ -6,8 +6,9 @@ use crate::block::{AbsoluteAssignBSizesTraversal, BlockFlow, MarginsMayCollapseFlag}; use crate::context::LayoutContext; -use crate::display_list::StackingContextCollectionState; -use crate::display_list::{DisplayListBuildState, FlexFlowDisplayListBuilding}; +use crate::display_list::{ + BorderPaintingMode, DisplayListBuildState, StackingContextCollectionState, +}; use crate::floats::FloatKind; use crate::flow::{Flow, FlowClass, FlowFlags, GetBaseFlow, ImmutableFlowUtils, OpaqueFlow}; use crate::fragment::{Fragment, FragmentBorderBoxIterator, Overflow}; @@ -1090,7 +1091,9 @@ impl Flow for FlexFlow { } fn build_display_list(&mut self, state: &mut DisplayListBuildState) { - self.build_display_list_for_flex(state); + // Draw the rest of the block. + self.as_mut_block() + .build_display_list_for_block(state, BorderPaintingMode::Separate) } fn collect_stacking_contexts(&mut self, state: &mut StackingContextCollectionState) { From aacaf1bd7011f6b0800940eff7bbfbafcff6c3cf Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 11:56:27 +0100 Subject: [PATCH 12/20] Remove FlowConstructionUtils --- components/layout/construct.rs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index cef9d5f5be5..a0bfcfd074e 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -2045,23 +2045,7 @@ where } } -// This must not be public because only the layout constructor can call these -// methods. -trait FlowConstructionUtils { - /// Adds a new flow as a child of this flow. Removes the flow from the given leaf set if - /// it's present. - fn add_new_child(&mut self, new_child: FlowRef); - - /// Finishes a flow. Once a flow is finished, no more child flows or boxes may be added to it. - /// This will normally run the bubble-inline-sizes (minimum and preferred -- i.e. intrinsic -- - /// inline-size) calculation, unless the global `bubble_inline-sizes_separately` flag is on. - /// - /// All flows must be finished at some point, or they will not have their intrinsic inline- - /// sizes properly computed. (This is not, however, a memory safety problem.) - fn finish(&mut self); -} - -impl FlowConstructionUtils for FlowRef { +impl FlowRef { /// Adds a new flow as a child of this flow. Fails if this flow is marked as a leaf. fn add_new_child(&mut self, mut new_child: FlowRef) { { From 495a0dd41a7e1c53b37e3a7ad751c30239179839 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:02:31 +0100 Subject: [PATCH 13/20] Remove LayoutDamageComputation --- components/layout/incremental.rs | 11 +++-------- components/layout_thread/lib.rs | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/components/layout/incremental.rs b/components/layout/incremental.rs index e3007511f8e..4e313f088bb 100644 --- a/components/layout/incremental.rs +++ b/components/layout/incremental.rs @@ -22,13 +22,8 @@ bitflags! { } } -pub trait LayoutDamageComputation { - fn compute_layout_damage(self) -> SpecialRestyleDamage; - fn reflow_entire_document(self); -} - -impl<'a> LayoutDamageComputation for &'a mut dyn Flow { - fn compute_layout_damage(self) -> SpecialRestyleDamage { +impl dyn Flow { + pub fn compute_layout_damage(&mut self) -> SpecialRestyleDamage { let mut special_damage = SpecialRestyleDamage::empty(); let is_absolutely_positioned = self .base() @@ -91,7 +86,7 @@ impl<'a> LayoutDamageComputation for &'a mut dyn Flow { special_damage } - fn reflow_entire_document(self) { + pub fn reflow_entire_document(&mut self) { let self_base = self.mut_base(); self_base .restyle_damage diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index aebfeebe440..d0c4cd9e4da 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -45,7 +45,7 @@ use layout::display_list::items::{OpaqueNode, WebRenderImageInfo}; use layout::display_list::{IndexableText, ToLayout, WebRenderDisplayListConverter}; use layout::flow::{Flow, GetBaseFlow, ImmutableFlowUtils, MutableOwnedFlowUtils}; use layout::flow_ref::FlowRef; -use layout::incremental::{LayoutDamageComputation, RelayoutMode, SpecialRestyleDamage}; +use layout::incremental::{RelayoutMode, SpecialRestyleDamage}; use layout::layout_debug; use layout::parallel; use layout::query::{ From 9825b85acd07d6d0a50534f9e839c4a50a8ba56e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:03:52 +0100 Subject: [PATCH 14/20] Remove CompletelyEncloses --- components/layout/display_list/items.rs | 62 +++++++++++-------------- 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/components/layout/display_list/items.rs b/components/layout/display_list/items.rs index 030464fd881..4b1df780087 100644 --- a/components/layout/display_list/items.rs +++ b/components/layout/display_list/items.rs @@ -565,11 +565,11 @@ impl ClippingRegion { // // http://www.inrg.csie.ntu.edu.tw/algorithm2014/presentation/D&C%20Lee-84.pdf for existing_complex_region in &mut self.complex { - if existing_complex_region.completely_encloses(&new_complex_region) { + if completely_encloses(&existing_complex_region, &new_complex_region) { *existing_complex_region = new_complex_region; return; } - if new_complex_region.completely_encloses(existing_complex_region) { + if completely_encloses(&new_complex_region, &existing_complex_region) { return; } } @@ -618,38 +618,32 @@ impl fmt::Debug for ClippingRegion { } } -pub trait CompletelyEncloses { - fn completely_encloses(&self, other: &Self) -> bool; -} - -impl CompletelyEncloses for ComplexClipRegion { - // TODO(pcwalton): This could be more aggressive by considering points that touch the inside of - // the border radius ellipse. - fn completely_encloses(&self, other: &Self) -> bool { - let left = self.radii.top_left.width.max(self.radii.bottom_left.width); - let top = self.radii.top_left.height.max(self.radii.top_right.height); - let right = self - .radii - .top_right - .width - .max(self.radii.bottom_right.width); - let bottom = self - .radii - .bottom_left - .height - .max(self.radii.bottom_right.height); - let interior = LayoutRect::new( - LayoutPoint::new(self.rect.origin.x + left, self.rect.origin.y + top), - LayoutSize::new( - self.rect.size.width - left - right, - self.rect.size.height - top - bottom, - ), - ); - interior.origin.x <= other.rect.origin.x && - interior.origin.y <= other.rect.origin.y && - interior.max_x() >= other.rect.max_x() && - interior.max_y() >= other.rect.max_y() - } +// TODO(pcwalton): This could be more aggressive by considering points that touch the inside of +// the border radius ellipse. +fn completely_encloses(this: &ComplexClipRegion, other: &ComplexClipRegion) -> bool { + let left = this.radii.top_left.width.max(this.radii.bottom_left.width); + let top = this.radii.top_left.height.max(this.radii.top_right.height); + let right = this + .radii + .top_right + .width + .max(this.radii.bottom_right.width); + let bottom = this + .radii + .bottom_left + .height + .max(this.radii.bottom_right.height); + let interior = LayoutRect::new( + LayoutPoint::new(this.rect.origin.x + left, this.rect.origin.y + top), + LayoutSize::new( + this.rect.size.width - left - right, + this.rect.size.height - top - bottom, + ), + ); + interior.origin.x <= other.rect.origin.x && + interior.origin.y <= other.rect.origin.y && + interior.max_x() >= other.rect.max_x() && + interior.max_y() >= other.rect.max_y() } /// Metadata attached to each display item. This is useful for performing auxiliary threads with From b9b70445cb0525f7efd7f40894c3ba941968521b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:07:33 +0100 Subject: [PATCH 15/20] Remove ComputedValuesCursorUtility --- components/layout/display_list/builder.rs | 74 ++++++++++------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 13f16e379e9..18b5b0608bf 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -692,7 +692,7 @@ impl Fragment { bounds, bounds, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), display_list_section, ); state.add_display_item(DisplayItem::Rectangle(CommonDisplayItem::new( @@ -823,7 +823,7 @@ impl Fragment { placement.bounds, placement.clip_rect, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), display_list_section, ); @@ -938,7 +938,7 @@ impl Fragment { placement.bounds, placement.clip_rect, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), display_list_section, ); @@ -1004,7 +1004,7 @@ impl Fragment { bounds, clip, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), display_list_section, ); let border_radius = border::radii(absolute_bounds, style.get_border()); @@ -1088,7 +1088,7 @@ impl Fragment { bounds, clip, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), display_list_section, ); @@ -1287,7 +1287,7 @@ impl Fragment { bounds, clip, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), DisplayListSection::Outlines, ); state.add_display_item(DisplayItem::Border(CommonDisplayItem::with_data( @@ -1318,7 +1318,7 @@ impl Fragment { stacking_relative_border_box, clip, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), DisplayListSection::Content, ); state.add_display_item(DisplayItem::Border(CommonDisplayItem::with_data( @@ -1347,7 +1347,7 @@ impl Fragment { baseline, clip, self.node, - style.get_cursor(CursorKind::Default), + get_cursor(&style, CursorKind::Default), DisplayListSection::Content, ); // TODO(gw): Use a better estimate for wavy line thickness. @@ -1375,7 +1375,7 @@ impl Fragment { stacking_relative_border_box, clip, self.node, - self.style.get_cursor(CursorKind::Default), + get_cursor(&self.style, CursorKind::Default), DisplayListSection::Content, ); state.add_display_item(DisplayItem::Border(CommonDisplayItem::with_data( @@ -1417,7 +1417,7 @@ impl Fragment { stacking_relative_border_box, clip, self.node, - self.style.get_cursor(CursorKind::Default), + get_cursor(&self.style, CursorKind::Default), display_list_section, ); state.add_display_item(DisplayItem::Rectangle(CommonDisplayItem::new( @@ -1463,7 +1463,7 @@ impl Fragment { insertion_point_bounds, clip, self.node, - self.style.get_cursor(cursor), + get_cursor(&self.style, cursor), display_list_section, ); state.add_display_item(DisplayItem::Rectangle(CommonDisplayItem::new( @@ -1645,9 +1645,7 @@ impl Fragment { content_size, content_size, self.node, - self.style - .get_cursor(CursorKind::Default) - .or_else(|| Some(CursorKind::Default)), + get_cursor(&self.style, CursorKind::Default).or(Some(CursorKind::Default)), DisplayListSection::Content, ); state.add_display_item(DisplayItem::Rectangle(CommonDisplayItem::new( @@ -1698,7 +1696,7 @@ impl Fragment { stacking_relative_content_box, stacking_relative_border_box, self.node, - self.style.get_cursor(CursorKind::Default), + get_cursor(&self.style, CursorKind::Default), DisplayListSection::Content, ) }; @@ -1974,7 +1972,7 @@ impl Fragment { stacking_relative_content_box, clip, self.node, - self.style().get_cursor(cursor), + get_cursor(&self.style, cursor), DisplayListSection::Content, ); @@ -2098,7 +2096,7 @@ impl Fragment { stacking_relative_box, clip, self.node, - self.style.get_cursor(CursorKind::Default), + get_cursor(&self.style, CursorKind::Default), DisplayListSection::Content, ); @@ -2828,30 +2826,24 @@ impl BaseFlow { } } -trait ComputedValuesCursorUtility { - fn get_cursor(&self, default_cursor: CursorKind) -> Option; -} - -impl ComputedValuesCursorUtility for ComputedValues { - /// Gets the cursor to use given the specific ComputedValues. `default_cursor` specifies - /// the cursor to use if `cursor` is `auto`. Typically, this will be `PointerCursor`, but for - /// text display items it may be `TextCursor` or `VerticalTextCursor`. - #[inline] - fn get_cursor(&self, default_cursor: CursorKind) -> Option { - match ( - self.get_inherited_ui().pointer_events, - &self.get_inherited_ui().cursor, - ) { - (PointerEvents::None, _) => None, - ( - PointerEvents::Auto, - &Cursor { - keyword: CursorKind::Auto, - .. - }, - ) => Some(default_cursor), - (PointerEvents::Auto, &Cursor { keyword, .. }) => Some(keyword), - } +/// Gets the cursor to use given the specific ComputedValues. `default_cursor` specifies +/// the cursor to use if `cursor` is `auto`. Typically, this will be `PointerCursor`, but for +/// text display items it may be `TextCursor` or `VerticalTextCursor`. +#[inline] +fn get_cursor(values: &ComputedValues, default_cursor: CursorKind) -> Option { + match ( + values.get_inherited_ui().pointer_events, + &values.get_inherited_ui().cursor, + ) { + (PointerEvents::None, _) => None, + ( + PointerEvents::Auto, + &Cursor { + keyword: CursorKind::Auto, + .. + }, + ) => Some(default_cursor), + (PointerEvents::Auto, &Cursor { keyword, .. }) => Some(keyword), } } From 059c9f4f7880a747c889d0b277b68edebb58e77a Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:08:34 +0100 Subject: [PATCH 16/20] Remove ComputedValueUtils --- components/layout/construct.rs | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index a0bfcfd074e..8df40d5b893 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -1015,7 +1015,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> } let node_style = node.style(self.style_context()); - if is_empty && node_style.has_padding_or_border() { + if is_empty && has_padding_or_border(&node_style) { // An empty inline box needs at least one fragment to draw its background and borders. let info = SpecificFragmentInfo::UnscannedText(Box::new( UnscannedTextFragmentInfo::new(Box::::from(""), None), @@ -2191,26 +2191,19 @@ where ) } -/// Convenience methods for computed CSS values -trait ComputedValueUtils { - /// Returns true if this node has non-zero padding or border. - fn has_padding_or_border(&self) -> bool; -} +/// Returns true if this node has non-zero padding or border. +fn has_padding_or_border(values: &ComputedValues) -> bool { + let padding = values.get_padding(); + let border = values.get_border(); -impl ComputedValueUtils for ComputedValues { - fn has_padding_or_border(&self) -> bool { - let padding = self.get_padding(); - let border = self.get_border(); - - !padding.padding_top.is_definitely_zero() || - !padding.padding_right.is_definitely_zero() || - !padding.padding_bottom.is_definitely_zero() || - !padding.padding_left.is_definitely_zero() || - border.border_top_width.px() != 0. || - border.border_right_width.px() != 0. || - border.border_bottom_width.px() != 0. || - border.border_left_width.px() != 0. - } + !padding.padding_top.is_definitely_zero() || + !padding.padding_right.is_definitely_zero() || + !padding.padding_bottom.is_definitely_zero() || + !padding.padding_left.is_definitely_zero() || + border.border_top_width.px() != 0. || + border.border_right_width.px() != 0. || + border.border_bottom_width.px() != 0. || + border.border_left_width.px() != 0. } /// Maintains a stack of anonymous boxes needed to ensure that the flow tree is *legal*. The tree From 887cc6255684864c1c5e282c30743f7126b346a1 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:11:02 +0100 Subject: [PATCH 17/20] Remove ObjectElement --- components/layout/construct.rs | 59 +++++++++++----------------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 8df40d5b893..21e3611f446 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -419,8 +419,17 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> SpecificFragmentInfo::Media(Box::new(MediaFragmentInfo::new(data))) }, Some(LayoutNodeType::Element(LayoutElementType::HTMLObjectElement)) => { + let elem = node.as_element().unwrap(); + let type_and_data = ( + elem.get_attr(&ns!(), &local_name!("type")), + elem.get_attr(&ns!(), &local_name!("data")), + ); + let object_data = match type_and_data { + (None, Some(uri)) if is_image_data(uri) => ServoUrl::parse(uri).ok(), + _ => None, + }; let image_info = Box::new(ImageFragmentInfo::new( - node.object_data(), + object_data, None, node, &self.layout_context, @@ -1976,7 +1985,15 @@ where Some(LayoutNodeType::Element(LayoutElementType::HTMLCanvasElement)) | Some(LayoutNodeType::Element(LayoutElementType::SVGSVGElement)) => true, Some(LayoutNodeType::Element(LayoutElementType::HTMLObjectElement)) => { - self.has_object_data() + let elem = self.as_element().unwrap(); + let type_and_data = ( + elem.get_attr(&ns!(), &local_name!("type")), + elem.get_attr(&ns!(), &local_name!("data")), + ); + match type_and_data { + (None, Some(uri)) => is_image_data(uri), + _ => false, + } }, Some(LayoutNodeType::Element(_)) => false, None => self.get_pseudo_element_type().is_replaced_content(), @@ -2007,44 +2024,6 @@ where } } -/// Methods for interacting with HTMLObjectElement nodes -trait ObjectElement { - /// Returns true if this node has object data that is correct uri. - fn has_object_data(&self) -> bool; - - /// Returns the "data" attribute value parsed as a URL - fn object_data(&self) -> Option; -} - -impl ObjectElement for N -where - N: ThreadSafeLayoutNode, -{ - fn has_object_data(&self) -> bool { - let elem = self.as_element().unwrap(); - let type_and_data = ( - elem.get_attr(&ns!(), &local_name!("type")), - elem.get_attr(&ns!(), &local_name!("data")), - ); - match type_and_data { - (None, Some(uri)) => is_image_data(uri), - _ => false, - } - } - - fn object_data(&self) -> Option { - let elem = self.as_element().unwrap(); - let type_and_data = ( - elem.get_attr(&ns!(), &local_name!("type")), - elem.get_attr(&ns!(), &local_name!("data")), - ); - match type_and_data { - (None, Some(uri)) if is_image_data(uri) => ServoUrl::parse(uri).ok(), - _ => None, - } - } -} - impl FlowRef { /// Adds a new flow as a child of this flow. Fails if this flow is marked as a leaf. fn add_new_child(&mut self, mut new_child: FlowRef) { From e57d09abb83b26f4471477653819f3a1637c04aa Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 12:27:27 +0100 Subject: [PATCH 18/20] Make Window::scroll_offsets store keys as OpaqueNode values This is the type that is supposed to signal that we will never ever try to get back a Node from it in an unsafe way, unlike UntrustedNodeAddress. --- components/script/dom/bindings/trace.rs | 3 ++- components/script/dom/window.rs | 20 ++++++++------------ components/script/script_thread.rs | 3 ++- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 2ea4de3d340..e92d1d5c452 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -115,6 +115,7 @@ use std::sync::{Arc, Mutex}; use std::time::{Instant, SystemTime}; use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto}; use style::context::QuirksMode; +use style::dom::OpaqueNode; use style::element_state::*; use style::media_queries::MediaList; use style::properties::PropertyDeclarationBlock; @@ -418,7 +419,7 @@ unsafe_no_jsmanaged_fields!(BufferQueue, QuirksMode, StrTendril); unsafe_no_jsmanaged_fields!(Runtime); unsafe_no_jsmanaged_fields!(HeaderMap, Method); unsafe_no_jsmanaged_fields!(WindowProxyHandler); -unsafe_no_jsmanaged_fields!(UntrustedNodeAddress); +unsafe_no_jsmanaged_fields!(UntrustedNodeAddress, OpaqueNode); unsafe_no_jsmanaged_fields!(LengthOrPercentageOrAuto); unsafe_no_jsmanaged_fields!(RGBA); unsafe_no_jsmanaged_fields!(StorageType); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index f8669ccd858..b557246e6b1 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -102,7 +102,7 @@ use script_layout_interface::{PendingImageState, TrustedNodeAddress}; use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult}; use script_traits::{ConstellationControlMsg, DocumentState, LoadData}; use script_traits::{ScriptMsg, ScriptToConstellationChan, ScrollState, TimerEvent, TimerEventId}; -use script_traits::{TimerSchedulerMsg, UntrustedNodeAddress, WindowSizeData, WindowSizeType}; +use script_traits::{TimerSchedulerMsg, WindowSizeData, WindowSizeType}; use selectors::attr::CaseSensitivity; use servo_config::opts; use servo_geometry::{f32_rect_to_au_rect, MaxRect}; @@ -119,6 +119,7 @@ use std::mem; use std::rc::Rc; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; +use style::dom::OpaqueNode; use style::error_reporting::{ContextualParseError, ParseErrorReporter}; use style::media_queries; use style::parser::ParserContext as CssParserContext; @@ -247,7 +248,7 @@ pub struct Window { error_reporter: CSSErrorReporter, /// A list of scroll offsets for each scrollable element. - scroll_offsets: DomRefCell>>, + scroll_offsets: DomRefCell>>, /// All the MediaQueryLists we need to update media_query_lists: DOMTracker, @@ -389,7 +390,7 @@ impl Window { /// Sets a new list of scroll offsets. /// /// This is called when layout gives us new ones and WebRender is in use. - pub fn set_scroll_offsets(&self, offsets: HashMap>) { + pub fn set_scroll_offsets(&self, offsets: HashMap>) { *self.scroll_offsets.borrow_mut() = offsets } @@ -1601,11 +1602,7 @@ impl Window { } pub fn scroll_offset_query(&self, node: &Node) -> Vector2D { - if let Some(scroll_offset) = self - .scroll_offsets - .borrow() - .get(&node.to_untrusted_node_address()) - { + if let Some(scroll_offset) = self.scroll_offsets.borrow().get(&node.to_opaque()) { return *scroll_offset; } Vector2D::new(0.0, 0.0) @@ -1620,10 +1617,9 @@ impl Window { // The scroll offsets are immediatly updated since later calls // to topScroll and others may access the properties before // webrender has a chance to update the offsets. - self.scroll_offsets.borrow_mut().insert( - node.to_untrusted_node_address(), - Vector2D::new(x_ as f32, y_ as f32), - ); + self.scroll_offsets + .borrow_mut() + .insert(node.to_opaque(), Vector2D::new(x_ as f32, y_ as f32)); let NodeScrollIdResponse(scroll_id) = self.layout_rpc.node_scroll_id(); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 99965a521a6..48dad65a725 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -146,6 +146,7 @@ use std::result::Result; use std::sync::Arc; use std::thread; use std::time::{Duration, SystemTime}; +use style::dom::OpaqueNode; use style::thread_state::{self, ThreadState}; use time::{at_utc, get_time, precise_time_ns, Timespec}; use url::percent_encoding::percent_decode; @@ -1914,7 +1915,7 @@ impl ScriptThread { if node_address == UntrustedNodeAddress(ptr::null()) { window.update_viewport_for_scroll(-scroll_offset.x, -scroll_offset.y); } else { - scroll_offsets.insert(node_address, -*scroll_offset); + scroll_offsets.insert(OpaqueNode(node_address.0 as usize), -*scroll_offset); } } window.set_scroll_offsets(scroll_offsets) From 087e6d649bd76edd1f0fa48928dddaceafffed8c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 15:59:37 +0100 Subject: [PATCH 19/20] Report the stylesheet URL when reporting CSS errors --- components/script/dom/window.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index b557246e6b1..dce37b4f0bc 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -2268,7 +2268,7 @@ impl ParseErrorReporter for CSSErrorReporter { .unwrap() .send(ConstellationControlMsg::ReportCSSError( self.pipelineid, - "".to_owned(), + url.to_string(), location.line, location.column, error.to_string(), From cf15336a51af938b1cc6d9f161d0ebdd6abda36b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 14 Jan 2019 16:10:15 +0100 Subject: [PATCH 20/20] Simplify get_cursor --- components/layout/display_list/builder.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 18b5b0608bf..18fb0980fcd 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -59,7 +59,6 @@ use style::values::computed::image::Image as ComputedImage; use style::values::computed::Gradient; use style::values::generics::background::BackgroundSize; use style::values::generics::image::{GradientKind, Image, PaintWorklet}; -use style::values::generics::ui::Cursor; use style::values::{Either, RGBA}; use style_traits::cursor::CursorKind; use style_traits::CSSPixel; @@ -2831,20 +2830,14 @@ impl BaseFlow { /// text display items it may be `TextCursor` or `VerticalTextCursor`. #[inline] fn get_cursor(values: &ComputedValues, default_cursor: CursorKind) -> Option { - match ( - values.get_inherited_ui().pointer_events, - &values.get_inherited_ui().cursor, - ) { - (PointerEvents::None, _) => None, - ( - PointerEvents::Auto, - &Cursor { - keyword: CursorKind::Auto, - .. - }, - ) => Some(default_cursor), - (PointerEvents::Auto, &Cursor { keyword, .. }) => Some(keyword), + let inherited_ui = values.get_inherited_ui(); + if inherited_ui.pointer_events == PointerEvents::None { + return None; } + Some(match inherited_ui.cursor.keyword { + CursorKind::Auto => default_cursor, + keyword => keyword, + }) } /// Adjusts `content_rect` as necessary for the given spread, and blur so that the resulting