Default Servo live rendering to hardware on macOS

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
+/// and `hardware`. macOS defaults to hardware because GPUI can now
-/// GPU adapter via the vendored `HardwareOffscreenContext`; requires
+/// present Servo's BGRA IOSurfaces directly; other platforms keep the
-/// the sidecar binary to be compiled with the `hardware-render`
+/// software context until they have an equivalent presenter.
-/// 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";
-
 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_from_env().cli_arg());
-            command.arg("--rendering-context").arg(rendering_context);
-        }
         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"),
-        );
-    }
-}

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<SidecarCommandTarget, SidecarC
     })?;
     let adjacent_sidecar = exe_dir.join(sidecar_binary_name());
     let workspace_manifest = workspace_manifest_path();
-    let prefer_cargo_hardware_sidecar = hardware_rendering_context_requested()
+    let prefer_cargo_hardware_sidecar = rendering_context_from_env()
+        == SidecarRenderingContext::Hardware
         && workspace_manifest.as_ref().is_some_and(|path| path.is_file());
     if adjacent_sidecar.is_file() && !prefer_cargo_hardware_sidecar {
         return Ok(SidecarCommandTarget::Binary(adjacent_sidecar));
@@ -93,28 +116,31 @@ fn workspace_manifest_path() -> Option<PathBuf> {
     option_env!("ELY_WORKSPACE_MANIFEST").map(PathBuf::from)
 }
 
-fn hardware_rendering_context_requested() -> bool {
+pub(super) fn rendering_context_from_env() -> SidecarRenderingContext {
-    env::var(RENDERING_CONTEXT_ENV).ok().as_deref().is_some_and(rendering_context_requests_hardware)
+    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)
+    rendering_context_from_env().sidecar_features()
-        .ok()
-        .as_deref()
-        .map(sidecar_features_for_rendering_context)
-        .unwrap_or(SOFTWARE_SIDECAR_FEATURES)
 }
 
-fn sidecar_features_for_rendering_context(raw: &str) -> &'static str {
+fn rendering_context_selection(raw: Option<&str>) -> SidecarRenderingContext {
-    if rendering_context_requests_hardware(raw) {
+    match raw.map(str::to_lowercase).as_deref() {
-        HARDWARE_SIDECAR_FEATURES
+        Some("software") => SidecarRenderingContext::Software,
-    } else {
+        Some("hardware") => SidecarRenderingContext::Hardware,
-        SOFTWARE_SIDECAR_FEATURES
+        _ => default_rendering_context(),
     }
 }
 
-fn rendering_context_requests_hardware(raw: &str) -> bool {
+#[cfg(target_os = "macos")]
-    raw.eq_ignore_ascii_case("hardware")
+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<PathBuf> {
@@ -129,19 +155,39 @@ fn sidecar_binary_name() -> String {
 #[cfg(test)]
 mod tests {
     use super::{
-        HARDWARE_SIDECAR_FEATURES, SOFTWARE_SIDECAR_FEATURES,
+        HARDWARE_SIDECAR_FEATURES, SOFTWARE_SIDECAR_FEATURES, SidecarRenderingContext,
-        sidecar_features_for_rendering_context,
+        rendering_context_selection,
     };
 
     #[test]
     fn hardware_rendering_context_enables_hardware_sidecar_feature() {
-        assert_eq!(sidecar_features_for_rendering_context("hardware"), HARDWARE_SIDECAR_FEATURES);
+        let context = rendering_context_selection(Some("hardware"));
-        assert_eq!(sidecar_features_for_rendering_context("HARDWARE"), HARDWARE_SIDECAR_FEATURES);
+        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() {
+    fn software_rendering_context_uses_software_sidecar_feature() {
-        assert_eq!(sidecar_features_for_rendering_context("software"), SOFTWARE_SIDECAR_FEATURES);
+        let context = rendering_context_selection(Some("software"));
-        assert_eq!(sidecar_features_for_rendering_context("garbage"), SOFTWARE_SIDECAR_FEATURES);
+        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);
     }
 }