From a6b9640c99da121641dd63765835ab55aa1d378d Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 28 Aug 2024 01:36:54 -0700 Subject: [PATCH] compositor: Do not parse the Cargo.lock file while building (#33222) The compositor's `build.rs` script was parsing the `Cargo.lock` file in order to tag WebRender captures with the WebRender version. The embedder already knows what version of Servo we are using, which should be enough to infer the WebRender revision. This changes does that and generally does a bit of cleaning up of how captures are done. - The name of the capture directory is now `webrender-captures` - There is console output now when captures are done. Before it was hard to know if it succeeded. - Simplify the Compositor constructor a little to avoid passing arguments so much. Signed-off-by: Martin Robinson --- .gitignore | 2 +- Cargo.lock | 1 - components/compositing/Cargo.toml | 4 -- components/compositing/build.rs | 52 ------------------- components/compositing/compositor.rs | 76 +++++++++------------------- components/compositing/windowing.rs | 5 ++ components/servo/lib.rs | 3 +- ports/servoshell/desktop/embedder.rs | 4 ++ 8 files changed, 35 insertions(+), 112 deletions(-) delete mode 100644 components/compositing/build.rs diff --git a/.gitignore b/.gitignore index 01f330e3af1..51d7f4d48fd 100644 --- a/.gitignore +++ b/.gitignore @@ -24,7 +24,7 @@ Servo.app .config.mk.last /glfw -capture_webrender/ +webrender-captures/ /screenshots # Allow CSV files to be used as resources in tests diff --git a/Cargo.lock b/Cargo.lock index 03e504ffc64..6672579b162 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -902,7 +902,6 @@ dependencies = [ "style_traits", "surfman", "time 0.1.45", - "toml 0.5.11", "webrender", "webrender_api", "webrender_traits", diff --git a/components/compositing/Cargo.toml b/components/compositing/Cargo.toml index 21bd1a0cced..954d8a40897 100644 --- a/components/compositing/Cargo.toml +++ b/components/compositing/Cargo.toml @@ -1,6 +1,5 @@ [package] name = "compositing" -build = "build.rs" version.workspace = true authors.workspace = true license.workspace = true @@ -49,6 +48,3 @@ webxr = { git = "https://github.com/servo/webxr" } [dev-dependencies] surfman = { workspace = true } - -[build-dependencies] -toml = "0.5" diff --git a/components/compositing/build.rs b/components/compositing/build.rs deleted file mode 100644 index 828bf622fcb..00000000000 --- a/components/compositing/build.rs +++ /dev/null @@ -1,52 +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 std::env; -use std::fs::File; -use std::io::{Read, Write}; -use std::path::Path; - -fn main() { - let lockfile_path = Path::new(&env::var("CARGO_MANIFEST_DIR").unwrap()) - .join("..") - .join("..") - .join("Cargo.lock"); - let revision_file_path = - Path::new(&env::var_os("OUT_DIR").unwrap()).join("webrender_revision.rs"); - - let mut lockfile = String::new(); - File::open(lockfile_path) - .expect("Cannot open lockfile") - .read_to_string(&mut lockfile) - .expect("Failed to read lockfile"); - - match toml::from_str::(&lockfile) { - Ok(result) => { - let packages = result - .get("package") - .expect("Cargo lockfile should contain package list"); - - match *packages { - toml::Value::Array(ref arr) => { - let source = arr - .iter() - .find(|pkg| { - pkg.get("name").and_then(|name| name.as_str()).unwrap_or("") == - "webrender" - }) - .and_then(|pkg| pkg.get("source").and_then(|source| source.as_str())) - .unwrap_or("unknown"); - - let parsed: Vec<&str> = source.split('#').collect(); - let revision = if parsed.len() > 1 { parsed[1] } else { source }; - - let mut revision_module_file = File::create(revision_file_path).unwrap(); - write!(&mut revision_module_file, "\"{}\"", revision).unwrap(); - }, - _ => panic!("Cannot find package definitions in lockfile"), - } - }, - Err(e) => panic!("{}", e), - } -} diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 4f4be0cfeda..6e6b403a1a9 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -215,6 +215,10 @@ pub struct IOCompositor { /// The [`Instant`] of the last animation tick, used to avoid flooding the Constellation and /// ScriptThread with a deluge of animation ticks. last_animation_tick: Instant, + + /// The string representing the version of Servo that is running. This is used to tag + /// WebRender capture output. + version_string: String, } #[derive(Clone, Copy)] @@ -350,13 +354,14 @@ pub enum CompositeTarget { } impl IOCompositor { - fn new( + pub fn new( window: Rc, state: InitialCompositorState, composite_target: CompositeTarget, exit_after_load: bool, convert_mouse_to_touch: bool, top_level_browsing_context_id: TopLevelBrowsingContextId, + version_string: String, ) -> Self { let embedder_coordinates = window.get_coordinates(); let mut webviews = WebViewManager::default(); @@ -377,7 +382,7 @@ impl IOCompositor { .show(top_level_browsing_context_id) .expect("Infallible due to add"); - IOCompositor { + let compositor = IOCompositor { embedder_coordinates: window.get_coordinates(), window, port: state.receiver, @@ -415,27 +420,9 @@ impl IOCompositor { pending_frames: 0, waiting_on_present: false, last_animation_tick: Instant::now(), - } - } + version_string, + }; - pub fn create( - window: Rc, - state: InitialCompositorState, - composite_target: CompositeTarget, - exit_after_load: bool, - convert_mouse_to_touch: bool, - top_level_browsing_context_id: TopLevelBrowsingContextId, - ) -> Self { - let compositor = IOCompositor::new( - window, - state, - composite_target, - exit_after_load, - convert_mouse_to_touch, - top_level_browsing_context_id, - ); - - // Make sure the GL state is OK compositor.assert_gl_framebuffer_complete(); compositor } @@ -2489,42 +2476,25 @@ impl IOCompositor { .iter() .filter_map(|val| { val.as_ref() - .map(|dir| dir.join("capture_webrender").join(&capture_id)) + .map(|dir| dir.join("webrender-captures").join(&capture_id)) .ok() }) - .find(|val| match create_dir_all(val) { - Ok(_) => true, - Err(err) => { - eprintln!("Unable to create path '{:?}' for capture: {:?}", &val, err); - false - }, - }); + .find(|val| create_dir_all(val).is_ok()); - match available_path { - Some(capture_path) => { - let revision_file_path = capture_path.join("wr.txt"); + let Some(capture_path) = available_path else { + eprintln!("Couldn't create a path for WebRender captures."); + return; + }; - debug!( - "Trying to save webrender capture under {:?}", - &revision_file_path - ); - self.webrender_api - .save_capture(capture_path, CaptureBits::all()); + println!("Saving WebRender capture to {capture_path:?}"); + self.webrender_api + .save_capture(capture_path.clone(), CaptureBits::all()); - match File::create(revision_file_path) { - Ok(mut file) => { - let revision = include!(concat!(env!("OUT_DIR"), "/webrender_revision.rs")); - if let Err(err) = write!(&mut file, "{}", revision) { - eprintln!("Unable to write webrender revision: {:?}", err) - } - }, - Err(err) => eprintln!( - "Capture triggered, creating webrender revision info skipped: {:?}", - err - ), - } - }, - None => eprintln!("Unable to locate path to save captures"), + let version_file_path = capture_path.join("servo-version.txt"); + if let Err(error) = File::create(version_file_path) + .and_then(|mut file| write!(file, "{}", self.version_string)) + { + eprintln!("Unable to write servo version for WebRender Capture: {error:?}"); } } } diff --git a/components/compositing/windowing.rs b/components/compositing/windowing.rs index b65c5fe88eb..d1cf9866d01 100644 --- a/components/compositing/windowing.rs +++ b/components/compositing/windowing.rs @@ -224,6 +224,11 @@ pub trait EmbedderMethods { None } + /// Returns the version string of this embedder. + fn get_version_string(&self) -> Option { + None + } + /// Returns the protocol handlers implemented by that embedder. /// They will be merged with the default internal ones. fn get_protocol_handlers(&self) -> ProtocolRegistry { diff --git a/components/servo/lib.rs b/components/servo/lib.rs index ac6383a9297..69a25aebd13 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -474,7 +474,7 @@ where // The compositor coordinates with the client window to create the final // rendered page and display it somewhere. - let compositor = IOCompositor::create( + let compositor = IOCompositor::new( window, InitialCompositorState { sender: compositor_proxy, @@ -493,6 +493,7 @@ where opts.exit_after_load, opts.debug.convert_mouse_to_touch, top_level_browsing_context_id, + embedder.get_version_string().unwrap_or_default(), ); let servo = Servo { diff --git a/ports/servoshell/desktop/embedder.rs b/ports/servoshell/desktop/embedder.rs index 8c568f1f369..39e33d00d87 100644 --- a/ports/servoshell/desktop/embedder.rs +++ b/ports/servoshell/desktop/embedder.rs @@ -65,4 +65,8 @@ impl EmbedderMethods for EmbedderCallbacks { registry.register("resource", resource::ResourceProtocolHander::default()); registry } + + fn get_version_string(&self) -> Option { + crate::servo_version().into() + } }