From 18fd20b577d9c73864030d0ed8d7a829adfc4950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 10 May 2026 21:27:12 -0400 Subject: [PATCH] Plumb the rendering context kind from ely_app env var to the sidecar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T10.1: with the vendored `HardwareOffscreenContext` (048c5df) and the host-level kind dispatch (a7d3e89) in place, the sidecar binary still ignored the rendering context kind — every spawn was wired to the software path regardless of how the host process was built. This commit threads the choice end-to-end: * `ely_servo_sidecar` learns a `--rendering-context [software| hardware]` flag on its `live` subcommand. `LiveArgs` carries the parsed `RenderingContextKind` (defaulting to `Software` so existing invocations stay bit-identical) and `live.rs::run_live` routes it through to `SoftwareServoHost::new_with_config_dir_and_kind`. Unknown values produce a typed `SidecarArgsError::InvalidRenderingContext`; a missing value after the flag produces the existing `MissingArgumentValue`. * `ely_app` reads `ELY_SERVO_RENDERING_CONTEXT` (with values `software` / `hardware`, case-insensitive) and, if set, appends `--rendering-context VALUE` to the sidecar command line. Unset or unrecognised values fall through to the sidecar's own software default — a stale env var or a typo never breaks the browser startup. The sidecar arg parser is the source of truth for legality of explicit values; the env helper only gates which values reach it. * Five new unit tests in `args::tests` pin the new parse paths: default-is-software, explicit-software, explicit-hardware, bogus-value-rejected, missing-value-rejected. Run via `cargo test -p ely_servo_host --features servo-engine --bin ely_servo_sidecar` and now hit alongside the five existing snapshot tests for 10 passes. End-to-end perf expectation: with the sidecar binary built using `--features servo-engine,hardware-render` and the env var set to `hardware`, every spawned sidecar webview rasterises through the real GPU adapter (via the vendored `HardwareOffscreenContext`/surfman/CGL chain on macOS). The host still reads back RGBA into a `Vec` for the existing pipe protocol; the IOSurface zero-copy bridge that deletes that read-back is T10.2–T10.5 in docs/t10-iosurface-plan.md and lands in subsequent commits. cargo test --bin ely_app: 120 passed, 0 failed, 2 ignored. cargo test -p ely_servo_host --features servo-engine --bin ely_servo_sidecar: 10 passed. cargo test -p ely_servo_host --features servo-engine --test sidecar: 9 passed. cargo test -p ely_servo_host --features servo-engine,hardware-render --test hardware_rendering_context: 1 passed. --- crates/ely_app/src/services/servo_live.rs | 30 ++++++++ .../src/bin/ely_servo_sidecar/args.rs | 72 +++++++++++++++++++ .../src/bin/ely_servo_sidecar/live.rs | 3 +- 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index 0eea1a3..18118c3 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -1,9 +1,21 @@ use std::{ + env, io::{self, BufRead, BufReader, Read, Write}, path::PathBuf, process::{Child, ChildStdin, ChildStdout, Stdio}, }; +/// Environment variable that lets the user pick the rendering context +/// kind used by the spawned sidecar. Accepted values: `software` +/// (default — bit-identical to pre-flag builds) and `hardware` (real +/// GPU adapter via the vendored `HardwareOffscreenContext`; requires +/// the sidecar binary to be compiled with the `hardware-render` +/// feature). Anything else is silently dropped and the sidecar +/// defaults to software so a typo'd value never blocks the browser +/// from starting; the sidecar's own arg parser still errors loudly on +/// an unrecognised value when set explicitly via the flag. +const RENDERING_CONTEXT_ENV: &str = "ELY_SERVO_RENDERING_CONTEXT"; + use ely_domain::SitePermissionDecision; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -25,6 +37,9 @@ impl ServoLiveClient { let mut command = command_target.command(); command.arg("live").arg("--profile-data-dir").arg(profile_data_dir); + if let Some(rendering_context) = rendering_context_from_env() { + command.arg("--rendering-context").arg(rendering_context); + } let mut child = command .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -334,3 +349,18 @@ struct LiveFrameReport { #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: u64, } + +/// Map `ELY_SERVO_RENDERING_CONTEXT` to a CLI argument value if it's +/// one we recognise. Unset variable returns `None` (sidecar uses its +/// own default of software); unknown value also returns `None` +/// rather than `Some("garbage")` so a stale env var doesn't fail the +/// sidecar startup. The sidecar's own arg parser is the source of +/// truth for what values are valid — we just gate which ones we +/// forward. +fn rendering_context_from_env() -> Option<&'static str> { + match env::var(RENDERING_CONTEXT_ENV).ok()?.to_lowercase().as_str() { + "software" => Some("software"), + "hardware" => Some("hardware"), + _ => None, + } +} diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs index b6b152e..a8d7100 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/args.rs @@ -4,6 +4,7 @@ use ely_domain::{ DEFAULT_ZOOM_PERCENT, ProfileId, SiteOrigin, SitePermissionDecision, SitePermissionFeature, UrlText, validate_zoom_percent, }; +use ely_servo_host::RenderingContextKind; use serde::Deserialize; use thiserror::Error; @@ -14,6 +15,13 @@ pub(super) enum SidecarCommand { pub(super) struct LiveArgs { pub(super) profile_data_dir: PathBuf, + /// Rendering context the host's webviews are built against. + /// Defaults to [`RenderingContextKind::Software`], which keeps + /// the binary's behaviour bit-identical to pre-flag builds. + /// `Hardware` is only accepted when the `hardware-render` + /// feature is compiled in (otherwise the `SoftwareServoHost` + /// constructor returns `HardwareRenderUnavailable`). + pub(super) rendering_context_kind: RenderingContextKind, } pub(super) struct SnapshotArgs { @@ -98,6 +106,12 @@ pub(super) enum SidecarArgsError { source: serde_json::Error, }, + #[error( + "invalid --rendering-context value: {value:?} (expected \"software\" or \ + \"hardware\")" + )] + InvalidRenderingContext { value: String }, + #[error(transparent)] Domain(#[from] ely_domain::DomainError), } @@ -123,6 +137,7 @@ fn parse_command( fn parse_live_args(args: impl IntoIterator) -> Result { let mut args = args.into_iter(); let mut profile_data_dir = None; + let mut rendering_context_kind = RenderingContextKind::default(); while let Some(name) = args.next() { match name.as_str() { @@ -132,6 +147,14 @@ fn parse_live_args(args: impl IntoIterator) -> Result { + let value = next_argument(&mut args, "--rendering-context")?; + rendering_context_kind = match value.as_str() { + "software" => RenderingContextKind::Software, + "hardware" => RenderingContextKind::Hardware, + _ => return Err(SidecarArgsError::InvalidRenderingContext { value }), + }; + } _ => return Err(SidecarArgsError::UnknownArgument { value: name }), } } @@ -139,6 +162,7 @@ fn parse_live_args(args: impl IntoIterator) -> Result Result<(), SidecarArgsError> { @@ -490,4 +515,51 @@ mod tests { .into_iter() .collect() } + + fn parse_live(extra_args: &[&str]) -> Result { + let base = ["ely_servo_sidecar", "live", "--profile-data-dir", "/tmp/sidecar-live"]; + let argv: Vec = + base.iter().chain(extra_args.iter()).map(|s| (*s).to_string()).collect(); + let SidecarCommand::Live(args) = parse_command(argv)? else { + return Err(SidecarArgsError::UnknownCommand { value: "live-extracted-as-snapshot".into() }); + }; + Ok(args) + } + + #[test] + fn live_defaults_rendering_context_to_software() -> Result<(), SidecarArgsError> { + let args = parse_live(&[])?; + assert_eq!(args.rendering_context_kind, RenderingContextKind::Software); + Ok(()) + } + + #[test] + fn live_accepts_explicit_software_rendering_context() -> Result<(), SidecarArgsError> { + let args = parse_live(&["--rendering-context", "software"])?; + assert_eq!(args.rendering_context_kind, RenderingContextKind::Software); + Ok(()) + } + + #[test] + fn live_accepts_explicit_hardware_rendering_context() -> Result<(), SidecarArgsError> { + let args = parse_live(&["--rendering-context", "hardware"])?; + assert_eq!(args.rendering_context_kind, RenderingContextKind::Hardware); + Ok(()) + } + + #[test] + fn live_rejects_unknown_rendering_context_value() { + assert!(matches!( + parse_live(&["--rendering-context", "gpu"]), + Err(SidecarArgsError::InvalidRenderingContext { value }) if value == "gpu" + )); + } + + #[test] + fn live_requires_rendering_context_value() { + assert!(matches!( + parse_live(&["--rendering-context"]), + Err(SidecarArgsError::MissingArgumentValue { name: "--rendering-context" }) + )); + } } diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs index c39b43f..6b8b24d 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs @@ -29,9 +29,10 @@ const LIVE_FRAME_WAIT_INTERVAL: Duration = Duration::from_millis(2); pub(super) fn run_live(args: LiveArgs) -> Result<(), LiveSidecarError> { fs::create_dir_all(&args.profile_data_dir)?; - let mut host = SoftwareServoHost::new_with_config_dir( + let mut host = SoftwareServoHost::new_with_config_dir_and_kind( ServoSurfaceSize::new(1, 1), Some(args.profile_data_dir), + args.rendering_context_kind, )?; let mut sessions = HashMap::new(); let stdin = io::stdin();