Merge pull request #2948 from kacpherek/fix/hdr-crash-macos

🤖 Agent: read the diff at dddcc9972 ("Fix the crash on every start with HDR on in macOS, and start with HDR ") against the 26.3 source it cites; verify green, merges cleanly with main. Proof: cg25 union a051cc285 (local branch cg25/union in t3-ef805fdb: main 6d3489d30 + 38 contributor heads merged cleanly, no fixups): clippy -p horizon-client --all-targets -D warnings clean, horizon-client lib 1105 + bin 133 + integration tests pass, parity --changed 355/355 harnesses PASS on the Mac mini (Cargo.lock changed by #3210, so every harness ran; tab_list passed on its one retry).

Cargo.toml

@@ -61,8 +61,10 @@ large_enum_variant = "allow"
 # wgpu-hal 30.0.1 with gfx-rs/wgpu#10124: wgpu 30 creates a Vulkan descriptor
 # pool with no pool sizes while it creates the device, which Vulkan allows only
 # since 1.3.215, and Intel's Windows driver for 6th-10th gen iGPUs (HD 520/530,
-# UHD 620/630) crashes on it with an access violation (#746). Drop this and the
-# folder once a wgpu release has the fix.
+# UHD 620/630) crashes on it with an access violation (#746). It also fixes the
+# Metal backend's lookup of the CoreGraphics HDR colour space names, which handed
+# CoreGraphics a symbol's address as the string and crashed HDR output on macOS.
+# Drop this and the folder once a wgpu release has both fixes.
 [patch.crates-io]
 wgpu-hal = { path = "third_party/wgpu-hal" }
 

crates/horizon-client/src/game/options.rs

@@ -273,6 +273,7 @@ impl Game<'_> {
                 s.horizon.hdr,
                 s.horizon.hdr_paper_white as f32,
                 s.horizon.hdr_peak as f32,
+                &Self::options_path().with_file_name(horizon_client::graphics::HDR_MARKER),
             );
         }
         // Window.updateFullscreenIfChanged: only a changed option asks the

crates/horizon-client/src/gfx.rs

@@ -539,6 +539,9 @@ pub struct Renderer {
     hdr: Option<hdr::Hdr>,
     /// The HDR output the window's surface can present.
     hdr_output: Option<hdr::Output>,
+    /// The marker file of an HDR output that has not presented a frame yet
+    /// (`graphics::HDR_MARKER`).
+    hdr_marker: Option<std::path::PathBuf>,
     /// The shader pack's post-processing, and the dimension (legacy id) it was loaded for.
     pack_post: Option<(i32, Option<pack_post::PackPost>)>,
     /// The post passes of one of Horizon's own shader packs; never beside `pack_post`'s.
@@ -2644,6 +2647,7 @@ impl Renderer {
             world_post,
             hdr: None,
             hdr_output,
+            hdr_marker: None,
             pack_post: None,
             native: None,
             pass_timing: pack_timing::PassTiming::logging(),
@@ -3202,9 +3206,17 @@ impl Renderer {
     }
     /// HDR output (Horizon setting): HDR10 or scRGB at `paper_white` nits, the world's
     /// highlights expanded up to `peak` nits. Off where the surface can't present it.
-    pub fn set_hdr(&mut self, on: bool, paper_white: f32, peak: f32) {
+    pub fn set_hdr(&mut self, on: bool, paper_white: f32, peak: f32, marker: &std::path::Path) {
         let output = self.hdr_output.filter(|_| on && self.surface.is_some());
         if output.is_some() != self.hdr.is_some() {
+            // If the game dies before HDR output shows a frame, the marker is still there at
+            // the next start, which then comes up with HDR off.
+            if output.is_some() {
+                let _ = std::fs::write(marker, "HDR output was being switched on\n");
+                self.hdr_marker = Some(marker.to_owned());
+            } else if let Some(marker) = self.hdr_marker.take() {
+                let _ = std::fs::remove_file(marker);
+            }
             let format = self.config.format.remove_srgb_suffix();
             self.hdr = output.map(|output| hdr::Hdr::new(&self.device, format, output));
             self.configure_surface();
@@ -5231,6 +5243,12 @@ impl Renderer {
                 startup_step(format_args!("presenting the first frame"));
             }
             self.queue.present(surface_frame);
+            if self.hdr.is_some()
+                && let Some(marker) = self.hdr_marker.take()
+            {
+                let _ = std::fs::remove_file(marker);
+                log::info!("Horizon renderer: HDR output presented its first frame");
+            }
             if reconfigure && let Some(surface) = &self.surface {
                 Self::configure(
                     &self.device,

crates/horizon-client/src/graphics.rs

@@ -5,6 +5,9 @@
 use std::{fs, path::Path};
 
 pub const FILE: &str = "horizon-graphics.txt";
+/// Written beside it before HDR output is switched on and removed once the first HDR frame is
+/// presented. Found at the next start, it means HDR output took the game down, so HDR starts off.
+pub const HDR_MARKER: &str = "horizon-hdr-pending";
 
 /// Simple-level presets. `Custom` leaves every option as the player set it.
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)]
@@ -310,9 +313,17 @@ fn bool_text(v: bool) -> &'static str {
 }
 impl Graphics {
     pub fn load(dir: &Path) -> Self {
-        fs::read_to_string(dir.join(FILE))
+        let mut g: Self = fs::read_to_string(dir.join(FILE))
             .map(|t| Self::parse(&t))
-            .unwrap_or_default()
+            .unwrap_or_default();
+        if fs::remove_file(dir.join(HDR_MARKER)).is_ok() && g.hdr {
+            g.hdr = false;
+            g.save(dir);
+            log::warn!(
+                "the last start ended before HDR output showed a frame; HDR is off now (graphics option hdr, in {FILE})"
+            );
+        }
+        g
     }
     pub fn parse(text: &str) -> Self {
         let mut g = Self::default();

third_party/wgpu-hal/src/metal/surface.rs

@@ -228,9 +228,12 @@ static COLOR_SPACES: LazyLock<Result<ColorSpaces, crate::SurfaceError>> = LazyLo
         lib: &libloading::Library,
         name: &[u8],
     ) -> Result<&'static CFString, crate::SurfaceError> {
-        let sym = unsafe { lib.get(name) }
+        // The symbols are `CFStringRef` globals, so `dlsym` returns the address of the
+        // global and the symbol's type is a pointer to the reference. Typing it as
+        // the reference itself would hand CoreGraphics that address as the string.
+        let sym = unsafe { lib.get::<*const &'static CFString>(name) }
             .map_err(|_| crate::SurfaceError::Other("error resolving symbol in CoreGraphics"))?;
-        Ok(*sym)
+        Ok(unsafe { **sym })
     }
     let extended_display_p3 = lookup(&lib, b"kCGColorSpaceExtendedDisplayP3\0")?;
     let itur_bt2100_pq = lookup(&lib, b"kCGColorSpaceITUR_2100_PQ\0")?;