From 2bd75d4fdf560a68c829e3110cae4050e41ccbf3 Mon Sep 17 00:00:00 2001 From: Georg Streich Date: Wed, 17 Jan 2018 07:28:01 -0600 Subject: [PATCH] servo: Merge #19755 - Decouple metrics and gfx (from streichgeorg:decoupling_metrics_gfx); r=jdm Added gfx_traits::DisplayList so metrics doesn't depend on the gfx crate for gfx::display_list::DisplayList. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #19422 (github issue number if applicable). - [ ] There are tests for these changes OR - [X] These changes do not require tests because succesful compilation should be enough Source-Repo: https://github.com/servo/servo Source-Revision: 8281ee004336049df11cc0478cb3c8b5610c6bde --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 07b42adf960519242136f890a37a20eb6599339a --- servo/Cargo.lock | 1 - servo/components/gfx/display_list/mod.rs | 21 ++++++++++++++++++++- servo/components/gfx_traits/lib.rs | 5 +++++ servo/components/layout_thread/lib.rs | 2 +- servo/components/metrics/Cargo.toml | 1 - servo/components/metrics/lib.rs | 21 ++------------------- servo/tests/unit/metrics/paint_time.rs | 2 +- 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/servo/Cargo.lock b/servo/Cargo.lock index 1618b8a646e9..9c4340c30180 100644 --- a/servo/Cargo.lock +++ b/servo/Cargo.lock @@ -1762,7 +1762,6 @@ dependencies = [ name = "metrics" version = "0.0.1" dependencies = [ - "gfx 0.0.1", "gfx_traits 0.0.1", "ipc-channel 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/servo/components/gfx/display_list/mod.rs b/servo/components/gfx/display_list/mod.rs index c4c73e50dc7b..fff0d658aece 100644 --- a/servo/components/gfx/display_list/mod.rs +++ b/servo/components/gfx/display_list/mod.rs @@ -17,7 +17,7 @@ use app_units::Au; use euclid::{Transform3D, Point2D, Vector2D, Rect, Size2D, TypedRect, SideOffsets2D}; use euclid::num::{One, Zero}; -use gfx_traits::StackingContextId; +use gfx_traits::{self, StackingContextId}; use gfx_traits::print_tree::PrintTree; use ipc_channel::ipc::IpcSharedMemory; use msg::constellation_msg::PipelineId; @@ -146,6 +146,25 @@ impl DisplayList { } } +impl gfx_traits::DisplayList for DisplayList { + /// Analyze the display list to figure out if this may be the first + /// contentful paint (i.e. the display list contains items of type text, + /// image, non-white canvas or SVG). Used by metrics. + fn is_contentful(&self) -> bool { + for item in &self.list { + match item { + &DisplayItem::Text(_) | + &DisplayItem::Image(_) => { + return true + } + _ => (), + } + } + + false + } +} + /// Display list sections that make up a stacking context. Each section here refers /// to the steps in CSS 2.1 Appendix E. /// diff --git a/servo/components/gfx_traits/lib.rs b/servo/components/gfx_traits/lib.rs index 6f5b380dd431..4500a3816da4 100644 --- a/servo/components/gfx_traits/lib.rs +++ b/servo/components/gfx_traits/lib.rs @@ -105,3 +105,8 @@ pub fn node_id_from_clip_id(id: usize) -> Option { } None } + +pub trait DisplayList { + /// Returns true if this display list contains meaningful content. + fn is_contentful(&self) -> bool; +} diff --git a/servo/components/layout_thread/lib.rs b/servo/components/layout_thread/lib.rs index 2c72bd6f4b79..c37d3ada3512 100644 --- a/servo/components/layout_thread/lib.rs +++ b/servo/components/layout_thread/lib.rs @@ -1042,7 +1042,7 @@ impl LayoutThread { // Observe notifications about rendered frames if needed right before // sending the display list to WebRender in order to set time related // Progressive Web Metrics. - self.paint_time_metrics.maybe_observe_paint_time(self, epoch, &display_list); + self.paint_time_metrics.maybe_observe_paint_time(self, epoch, &*display_list); self.webrender_api.set_display_list( self.webrender_document, diff --git a/servo/components/metrics/Cargo.toml b/servo/components/metrics/Cargo.toml index 7a05ee9b1323..af1863667212 100644 --- a/servo/components/metrics/Cargo.toml +++ b/servo/components/metrics/Cargo.toml @@ -10,7 +10,6 @@ name = "metrics" path = "lib.rs" [dependencies] -gfx = {path = "../gfx"} gfx_traits = {path = "../gfx_traits"} ipc-channel = "0.9" log = "0.3.5" diff --git a/servo/components/metrics/lib.rs b/servo/components/metrics/lib.rs index cacf057729c1..59657738e571 100644 --- a/servo/components/metrics/lib.rs +++ b/servo/components/metrics/lib.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 http://mozilla.org/MPL/2.0/. */ -extern crate gfx; extern crate gfx_traits; extern crate ipc_channel; #[macro_use] @@ -17,8 +16,7 @@ extern crate servo_config; extern crate servo_url; extern crate time; -use gfx::display_list::{DisplayItem, DisplayList}; -use gfx_traits::Epoch; +use gfx_traits::{Epoch, DisplayList}; use ipc_channel::ipc::IpcSender; use msg::constellation_msg::PipelineId; use profile_traits::time::{ProfilerChan, ProfilerCategory, send_profile_data}; @@ -324,24 +322,9 @@ impl PaintTimeMetrics { return; } - let mut is_contentful = false; - // Analyze the display list to figure out if this may be the first - // contentful paint (i.e. the display list contains items of type text, - // image, non-white canvas or SVG). - for item in &display_list.list { - match item { - &DisplayItem::Text(_) | - &DisplayItem::Image(_) => { - is_contentful = true; - break; - } - _ => (), - } - } - self.pending_metrics.borrow_mut().insert(epoch, ( profiler_metadata_factory.new_metadata(), - is_contentful, + display_list.is_contentful(), )); // Send the pending metric information to the compositor thread. diff --git a/servo/tests/unit/metrics/paint_time.rs b/servo/tests/unit/metrics/paint_time.rs index e3eb6012c195..01020ea6fefe 100644 --- a/servo/tests/unit/metrics/paint_time.rs +++ b/servo/tests/unit/metrics/paint_time.rs @@ -69,7 +69,7 @@ fn test_common(display_list: &DisplayList, epoch: Epoch) -> PaintTimeMetrics { paint_time_metrics.maybe_observe_paint_time( &dummy_profiler_metadata_factory, epoch, - &display_list, + &*display_list, ); // Should not set any metric until navigation start is set.