Plumb the rendering context kind from ely_app env var to the sidecar
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<u8>` 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.
This commit is contained in:
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Item = String>) -> Result<LiveArgs, SidecarArgsError> {
|
||||
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<Item = String>) -> Result<LiveArgs, S
|
||||
next_argument(&mut args, "--profile-data-dir")?,
|
||||
)?)
|
||||
}
|
||||
"--rendering-context" => {
|
||||
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<Item = String>) -> Result<LiveArgs, S
|
||||
Ok(LiveArgs {
|
||||
profile_data_dir: profile_data_dir
|
||||
.ok_or(SidecarArgsError::MissingRequiredArgument { name: "--profile-data-dir" })?,
|
||||
rendering_context_kind,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -367,6 +391,7 @@ mod tests {
|
||||
use ely_domain::{
|
||||
DEFAULT_ZOOM_PERCENT, DomainError, ProfileId, SitePermissionDecision, SitePermissionFeature,
|
||||
};
|
||||
use ely_servo_host::RenderingContextKind;
|
||||
|
||||
#[test]
|
||||
fn parses_snapshot_profile_identity() -> Result<(), SidecarArgsError> {
|
||||
@@ -490,4 +515,51 @@ mod tests {
|
||||
.into_iter()
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn parse_live(extra_args: &[&str]) -> Result<super::LiveArgs, SidecarArgsError> {
|
||||
let base = ["ely_servo_sidecar", "live", "--profile-data-dir", "/tmp/sidecar-live"];
|
||||
let argv: Vec<String> =
|
||||
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" })
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user