diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index bcdae98..56625af 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -1,5 +1,4 @@ use std::{ - env, io::{self, BufRead, BufReader, Read, Write}, path::PathBuf, process::{Child, ChildStdin, ChildStdout, Stdio}, @@ -7,16 +6,9 @@ use std::{ /// 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 and the local GPUI BGRA surface presenter). 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"; - +/// and `hardware`. macOS defaults to hardware because GPUI can now +/// present Servo's BGRA IOSurfaces directly; other platforms keep the +/// software context until they have an equivalent presenter. use ely_domain::SitePermissionDecision; use serde::Serialize; use thiserror::Error; @@ -24,7 +16,9 @@ use thiserror::Error; #[path = "servo_live_wire.rs"] mod wire; -use super::servo_sidecar_command::{SidecarCommandError, default_sidecar_command}; +use super::servo_sidecar_command::{ + SidecarCommandError, default_sidecar_command, rendering_context_from_env, +}; use wire::{ LiveFrameReport, LiveRequest, LiveResponse, LiveSurfaceHandle, log_frame_perf, log_iosurface_current, log_iosurface_handle, @@ -55,9 +49,7 @@ 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); - } + command.arg("--rendering-context").arg(rendering_context_from_env().cli_arg()); let mut child = command .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -420,53 +412,3 @@ pub(crate) enum ServoLiveError { #[error(transparent)] SidecarCommand(#[from] SidecarCommandError), } - -/// 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> { - let raw = env::var(RENDERING_CONTEXT_ENV).ok()?; - match rendering_context_selection(raw.as_str()) { - RenderingContextSelection::Forward(value) => Some(value), - RenderingContextSelection::Ignore => None, - } -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum RenderingContextSelection { - Forward(&'static str), - Ignore, -} - -fn rendering_context_selection(raw: &str) -> RenderingContextSelection { - match raw.to_lowercase().as_str() { - "software" => RenderingContextSelection::Forward("software"), - "hardware" => RenderingContextSelection::Forward("hardware"), - _ => RenderingContextSelection::Ignore, - } -} - -#[cfg(test)] -mod tests { - use super::{RenderingContextSelection, rendering_context_selection}; - - #[test] - fn hardware_env_forwards_to_the_sidecar() { - assert_eq!( - rendering_context_selection("hardware"), - RenderingContextSelection::Forward("hardware") - ); - } - - #[test] - fn software_env_still_forwards_to_the_sidecar() { - assert_eq!( - rendering_context_selection("software"), - RenderingContextSelection::Forward("software"), - ); - } -} diff --git a/crates/ely_app/src/services/servo_sidecar_command.rs b/crates/ely_app/src/services/servo_sidecar_command.rs index d65e694..547641c 100644 --- a/crates/ely_app/src/services/servo_sidecar_command.rs +++ b/crates/ely_app/src/services/servo_sidecar_command.rs @@ -48,6 +48,28 @@ impl SidecarCommandTarget { } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum SidecarRenderingContext { + Software, + Hardware, +} + +impl SidecarRenderingContext { + pub(super) fn cli_arg(self) -> &'static str { + match self { + Self::Software => "software", + Self::Hardware => "hardware", + } + } + + fn sidecar_features(self) -> &'static str { + match self { + Self::Software => SOFTWARE_SIDECAR_FEATURES, + Self::Hardware => HARDWARE_SIDECAR_FEATURES, + } + } +} + #[derive(Debug, Error)] pub(crate) enum SidecarCommandError { #[error("current executable path is unavailable: {0}")] @@ -68,7 +90,8 @@ pub(super) fn default_sidecar_command() -> Result Option { option_env!("ELY_WORKSPACE_MANIFEST").map(PathBuf::from) } -fn hardware_rendering_context_requested() -> bool { - env::var(RENDERING_CONTEXT_ENV).ok().as_deref().is_some_and(rendering_context_requests_hardware) +pub(super) fn rendering_context_from_env() -> SidecarRenderingContext { + let raw = env::var(RENDERING_CONTEXT_ENV).ok(); + rendering_context_selection(raw.as_deref()) } fn sidecar_features_from_env() -> &'static str { - env::var(RENDERING_CONTEXT_ENV) - .ok() - .as_deref() - .map(sidecar_features_for_rendering_context) - .unwrap_or(SOFTWARE_SIDECAR_FEATURES) + rendering_context_from_env().sidecar_features() } -fn sidecar_features_for_rendering_context(raw: &str) -> &'static str { - if rendering_context_requests_hardware(raw) { - HARDWARE_SIDECAR_FEATURES - } else { - SOFTWARE_SIDECAR_FEATURES +fn rendering_context_selection(raw: Option<&str>) -> SidecarRenderingContext { + match raw.map(str::to_lowercase).as_deref() { + Some("software") => SidecarRenderingContext::Software, + Some("hardware") => SidecarRenderingContext::Hardware, + _ => default_rendering_context(), } } -fn rendering_context_requests_hardware(raw: &str) -> bool { - raw.eq_ignore_ascii_case("hardware") +#[cfg(target_os = "macos")] +fn default_rendering_context() -> SidecarRenderingContext { + SidecarRenderingContext::Hardware +} + +#[cfg(not(target_os = "macos"))] +fn default_rendering_context() -> SidecarRenderingContext { + SidecarRenderingContext::Software } fn workspace_target_sidecar_path(manifest_path: &Path) -> Option { @@ -129,19 +155,39 @@ fn sidecar_binary_name() -> String { #[cfg(test)] mod tests { use super::{ - HARDWARE_SIDECAR_FEATURES, SOFTWARE_SIDECAR_FEATURES, - sidecar_features_for_rendering_context, + HARDWARE_SIDECAR_FEATURES, SOFTWARE_SIDECAR_FEATURES, SidecarRenderingContext, + rendering_context_selection, }; #[test] fn hardware_rendering_context_enables_hardware_sidecar_feature() { - assert_eq!(sidecar_features_for_rendering_context("hardware"), HARDWARE_SIDECAR_FEATURES); - assert_eq!(sidecar_features_for_rendering_context("HARDWARE"), HARDWARE_SIDECAR_FEATURES); + let context = rendering_context_selection(Some("hardware")); + assert_eq!(context, SidecarRenderingContext::Hardware); + assert_eq!(context.sidecar_features(), HARDWARE_SIDECAR_FEATURES); + assert_eq!( + rendering_context_selection(Some("HARDWARE")), + SidecarRenderingContext::Hardware + ); } #[test] - fn software_and_unknown_contexts_use_software_sidecar_feature() { - assert_eq!(sidecar_features_for_rendering_context("software"), SOFTWARE_SIDECAR_FEATURES); - assert_eq!(sidecar_features_for_rendering_context("garbage"), SOFTWARE_SIDECAR_FEATURES); + fn software_rendering_context_uses_software_sidecar_feature() { + let context = rendering_context_selection(Some("software")); + assert_eq!(context, SidecarRenderingContext::Software); + assert_eq!(context.sidecar_features(), SOFTWARE_SIDECAR_FEATURES); + } + + #[cfg(target_os = "macos")] + #[test] + fn macos_defaults_to_hardware_rendering_context() { + assert_eq!(rendering_context_selection(None), SidecarRenderingContext::Hardware); + assert_eq!(rendering_context_selection(Some("garbage")), SidecarRenderingContext::Hardware); + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn non_macos_defaults_to_software_rendering_context() { + assert_eq!(rendering_context_selection(None), SidecarRenderingContext::Software); + assert_eq!(rendering_context_selection(Some("garbage")), SidecarRenderingContext::Software); } }