diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -3886,8 +3886,7 @@ checksum = "743fb55ba31b18fb1ecef6bdc9aa2743314978ac084044301a7eee33fb99a20d" [[package]] name = "naga" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a616d2fb8c89516ac2723a581f69d6c18576046bed761bd6b305e5618e6ae130" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "arrayvec", "bit-set 0.10.0", @@ -3912,8 +3911,7 @@ dependencies = [ [[package]] name = "naga-types" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "590afbf58a6f4f62873cd5cff4468061844bafa1cdf399cc954537c22d768d49" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "hashbrown 0.17.1", "indexmap", @@ -8294,8 +8292,7 @@ checksum = "a28ac98ddc8b9274cb41bb4d9d4d5c425b6020c50c46f25559911905610b4a88" [[package]] name = "wgpu" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "527ccdf43dd5b2e8676eed9984ce00e2bbb0a1b85b70c1969dcb6cd2eb55ab9e" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "arrayvec", "bitflags 2.13.2", @@ -8324,8 +8321,7 @@ dependencies = [ [[package]] name = "wgpu-core" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c018fce9b6270aa203c2fdd56f3cce996713534bd757e4ea58c8560b121f14" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "arrayvec", "bit-set 0.10.0", @@ -8359,8 +8355,7 @@ dependencies = [ [[package]] name = "wgpu-core-deps-apple" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "061f3d319a40d39d00b1ecc2c33b89fe21d4e6fe01859df3500a3a8ecccd6b68" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "wgpu-hal", ] @@ -8368,8 +8363,7 @@ dependencies = [ [[package]] name = "wgpu-core-deps-emscripten" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d98b86cf4abf524a902dd35f18ca6a3f08fc2ae9847c8f10b48e30491b1f0b86" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "wgpu-hal", ] @@ -8377,8 +8371,7 @@ dependencies = [ [[package]] name = "wgpu-core-deps-wasm" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d260ca2ca8adc654f513a869bf3cac0b47f5b8d3f87803d6942015f8de597aae" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "wgpu-hal", ] @@ -8386,8 +8379,7 @@ dependencies = [ [[package]] name = "wgpu-core-deps-windows-linux-android" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7586165fd5f6d881cb9ce4bb71f40d6caab2c0f1837e3fc1d9788a197fb6004f" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "wgpu-hal", ] @@ -8395,8 +8387,7 @@ dependencies = [ [[package]] name = "wgpu-hal" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6b7fb58561a792bc237628ba0792e332de418fefe145f13b5ed8201e6d52f58" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "android_system_properties", "arrayvec", @@ -8451,8 +8442,7 @@ dependencies = [ [[package]] name = "wgpu-naga-bridge" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2f62e73117bb7a62bfd9c5a5841438a823f6566c6442a808ee269d2d055c081" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "naga", "wgpu-types", @@ -8461,8 +8451,7 @@ dependencies = [ [[package]] name = "wgpu-types" version = "30.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "99dad6f1fbdbbdb4c278a6508b059d44688f5cebddf78d005a46a31340269286" +source = "git+https://github.com/ITJamie/wgpu-fork?rev=21c08e23135ba27ed1c6b68177ecc20c0d076dd9#21c08e23135ba27ed1c6b68177ecc20c0d076dd9" dependencies = [ "bitflags 2.13.2", "bytemuck", diff --git a/Cargo.toml b/Cargo.toml --- a/Cargo.toml +++ b/Cargo.toml @@ -196,3 +196,6 @@ eframe = { git = "https://github.com/storytold/egui.git", rev = "6d518fc79968157 egui_demo_lib = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } egui_inspection = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } winit = { git = "https://github.com/storytold/winit.git", rev = "a517b3846ee8457d84ee4975110bfda6797c44dc" } + +# OCLP Metal fixes; proposed organization-owned fork: docs/oclp-wgpu-fork.md. +wgpu = { git = "https://github.com/ITJamie/wgpu-fork", rev = "21c08e23135ba27ed1c6b68177ecc20c0d076dd9" } diff --git a/apps/photocraft/src/gpu_startup.rs b/apps/photocraft/src/gpu_startup.rs --- a/apps/photocraft/src/gpu_startup.rs +++ b/apps/photocraft/src/gpu_startup.rs @@ -360,14 +360,6 @@ pub fn dx12_default_applied(adapters: &[Candidate], chosen: usize, os: Os, vulka applied.then_some(maker) } -/// wgpu compiles indirect-call validation shaders inside `request_device`; Metal's compiler -/// crashes on them on old Intel Macs (Iris Pro, GPUFamily 1 on Monterey), so no mode could start -/// (#3107). Only Intel Macs skip them: PhotoCraft issues no indirect draws or dispatches, but every -/// other GPU keeps wgpu's default. `WGPU_VALIDATION_INDIRECT_CALL=1` still wins. -fn skips_indirect_validation(os: Os, intel: bool) -> bool { - os == Os::Mac && intel -} - /// Configure eframe's wgpu setup for `plan`: instance backends, the DX12 shader compiler, adapter /// selection (Intel and Qualcomm on Windows, software adapters for `cpu`) and the marker update /// once the adapter is chosen. `note` receives a remark for System Info (e.g. the DX12 default). @@ -376,9 +368,6 @@ pub fn configure(setup: &mut egui_wgpu::WgpuSetup, plan: &Plan, os: Os, wanted: if let Some(b) = backends(plan, os) { create.instance_descriptor.backends = b; } - if skips_indirect_validation(os, cfg!(target_arch = "x86_64")) { - create.instance_descriptor.flags = create.instance_descriptor.flags.difference(wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL).with_env(); - } if os == Os::Windows { let exe = std::env::current_exe().ok(); let env = std::env::var("WGPU_DX12_COMPILER").ok(); @@ -1148,26 +1137,14 @@ mod tests { } #[test] - fn intel_macs_skip_the_indirect_validation_shaders() { - // Metal's compiler crashed on them on Intel Iris Pro, losing the device in every mode (#3107). - assert!(skips_indirect_validation(Os::Mac, true)); - // Apple silicon, Windows and Linux keep wgpu's validation. - for (os, intel) in [(Os::Mac, false), (Os::Windows, true), (Os::Other, true), (Os::Other, false)] { - assert!(!skips_indirect_validation(os, intel), "{os:?} intel={intel}"); - } - for (os, safe_gpu) in [(Os::Windows, false), (Os::Other, true), (Os::Other, false)] { - let mut setup = egui_wgpu::WgpuSetup::without_display_handle(); - let egui_wgpu::WgpuSetup::CreateNew(before) = &setup else { panic!("a new instance") }; - let before = before.instance_descriptor.flags; - configure(&mut setup, &plan(Auto, None, None, safe_gpu, os), os, None, Arc::new(Mutex::new(None)), Arc::default()); - let egui_wgpu::WgpuSetup::CreateNew(create) = &setup else { panic!("a new instance") }; - assert_eq!(create.instance_descriptor.flags, before, "{os:?}: flags untouched"); - } - if cfg!(target_arch = "x86_64") && std::env::var_os("WGPU_VALIDATION_INDIRECT_CALL").is_none() { + fn macos_preserves_configured_indirect_validation() { + for flags in [wgpu::InstanceFlags::default().with_env(), wgpu::InstanceFlags::empty(), wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL] { let mut setup = egui_wgpu::WgpuSetup::without_display_handle(); + let egui_wgpu::WgpuSetup::CreateNew(create) = &mut setup else { panic!("a new instance") }; + create.instance_descriptor.flags = flags; configure(&mut setup, &plan(Auto, None, None, false, Os::Mac), Os::Mac, None, Arc::new(Mutex::new(None)), Arc::default()); let egui_wgpu::WgpuSetup::CreateNew(create) = &setup else { panic!("a new instance") }; - assert!(!create.instance_descriptor.flags.contains(wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL)); + assert_eq!(create.instance_descriptor.flags, flags); } } } diff --git a/contributors/people.toml b/contributors/people.toml --- a/contributors/people.toml +++ b/contributors/people.toml @@ -282,3 +282,6 @@ sync_github_name = false [people.jasallen] real_name = "Jason DeBoever" sync_github_name = true + +[people.ITJamie] +sync_github_name = true diff --git a/docs/development.md b/docs/development.md --- a/docs/development.md +++ b/docs/development.md @@ -36,6 +36,9 @@ building locally; the downloaded app is still subject to Windows application-con ## Build and run +The [OCLP wgpu fork proposal](oclp-wgpu-fork.md) records the pinned Metal +compiler workarounds, organization ownership proposal, and hardware checks. + ```sh cargo run --release -p photocraft -- path/to/image.psd # desktop app cargo run --release -p photocraft -- --control 7878 --control-token-file .private/control.token \ diff --git a/docs/oclp-wgpu-fork.md b/docs/oclp-wgpu-fork.md new file mode 100644 --- /dev/null +++ b/docs/oclp-wgpu-fork.md @@ -0,0 +1,110 @@ +# Proposal: use the shared OCLP wgpu compatibility fork + +PhotoCraft uses the same pinned wgpu 30.0.1 backport proposed for +[ArtCraft Launcher](https://github.com/storytold/craft-launcher/pull/917). +It replaces the Intel-wide indirect-validation bypass with compiler workarounds +in wgpu while preserving PhotoCraft's other GPU startup and recovery behavior. + +## Dependency and ownership + +- Prototype repository: +- Branch: `codex/metal-msl-intel-v30.0.1` +- Immutable commit: `21c08e23135ba27ed1c6b68177ecc20c0d076dd9` +- Based on upstream tag `v30.0.1`, preserving the released API used by egui. +- Backports: and + . + +The fork probes supported Metal Shading Language versions, uses pipeline +workgroup limits when the Intel compiler rejects the shader attribute, and +emits runtime-sized storage arrays as pointers on Intel. Internal indirect-call +validation and shader bounds checks remain available and enabled by default. +Explicit wgpu environment settings still apply. + +This follows the compiler-fallback approach discussed in +[#4011](https://github.com/storytold/photocraft/issues/4011) and alecanales' +[#5910](https://github.com/storytold/photocraft/pull/5910). It does not include +that PR's separate renderer startup error-handling changes. + +A maintainer should decide whether to create `storytold/wgpu` and preserve the +backport branch and commit there. Once available, switch the patch URL in +`Cargo.toml` to the organization fork, keep the reviewed commit pin, and +regenerate `Cargo.lock`. PhotoCraft and ArtCraft Launcher should share that +compatibility dependency. Return to an upstream release after both fixes land +and hardware validation passes. + +## Validation + +```sh +cargo build --locked -p photocraft --features heif +cargo test --locked -p photocraft --features heif +cargo clippy -p photocraft --features heif --all-targets -- -D warnings +cargo xtask layers +cargo xtask wasm +``` + +For an isolated desktop run, set `PHOTOCRAFT_CONFIG_DIR` to a fresh directory. +Enable the authenticated control API as described in +[the control protocol](control-protocol.md), with its read/write roots limited +to synthetic test images. Verify both the window renderer and GPU image canvas, +then open and save the synthetic image and inspect an internal `ui.screenshot`. + +On the OCLP test Mac, use `WGPU_BACKEND=metal`, +`WGPU_VALIDATION_INDIRECT_CALL=1`, `MTL_DEBUG_LAYER=0`, and +`MTL_SHADER_VALIDATION=0`. The last two settings disable incompatible external +Metal validation layers for the test process; internal wgpu validation remains +enabled. Use `WGPU_POWER_PREF=low` for the Intel preference and `high` for AMD, +and confirm the selected adapter in logs. Do not disable validation globally +in production code as a substitute for a compiler workaround. + +### Recorded PhotoCraft checks + +On Apple M4 Max, the native build, affected-package Clippy, all 183 desktop +app tests, and the 31-crate architecture check passed. Focused GPU tests passed +for blend modes and cached composition at U8, U16 and F32 depths, with external +Metal validation enabled. An isolated native run opened and exported a synthetic +PNG with the GPU canvas active; its internal screenshot was visually inspected. +The debug window responded after approximately three seconds. + +`cargo xtask test-corpus --changed` skipped because no covered source paths +changed. The first WebAssembly attempt failed because the Homebrew toolchain +lacked the target; the isolated Rust 1.96.1 toolchain passed all 24 crates and +the optional HEIF check. The 13 startup/control integration tests and native +AppKit menu integration harness also passed. + +Repeated package runs, including a serial run, failed +`single_instance::tests::a_sandboxed_second_launch_hands_its_paths_over_a_unix_socket` +(182 passed, one failed each); it passed when rerun alone. The initial +183-test run passed. This IPC failure remains untriaged. + +On MacBookPro11,5 running macOS 15.8.1 through OCLP, the native build and +affected-package Clippy passed. All 183 app tests, 13 startup/control integration +tests and the native AppKit menu harness passed. Separate AMD Radeon R9 M370X +and Intel Iris Pro +Graphics runs confirmed the selected adapter, an active GPU canvas, and no +reported device loss. Both opened and exported the synthetic PNG; their internal +screenshots were visually inspected. The debug window responded after about +33 seconds on AMD and 42 seconds on Intel. Automatic probing selected MSL 3.1 +instead of the OS maximum 3.2; the Intel workgroup-attribute probe triggered +the compiler workaround. Internal indirect-call validation remained enabled, +with external Metal layers disabled as described above. + +## Known limits of the shared backport + +The prior launcher debug runs on MacBookPro11,5, macOS 15.8.1 through OCLP, +took roughly 35–45 seconds to reach swapchain creation. These are separate +from the PhotoCraft window timings above; editing performance still needs +measurement. + +The backport's full wgpu suite on Apple M4 Max ran 1,010 tests: 1,002 passed, +eight failed, and six were skipped. Failures were: + +- `subgroup_operations`: an expected-failure mismatch. +- `all_passthrough_shaders_binary`, `all_passthrough_shaders_source`, and + `metallib_passthrough_shader`: missing DXC or offline Metal tools. +- `binding_array_storage_buffers`, `partial_binding_array_storage_buffers`, + `validate_capabilities`, and the `Computepass Encoding` benchmark: untriaged + binding-array/capability failures. + +The shared backport is a prototype, not a clean full-suite pass. Other OCLP +hardware and normal cross-platform CI still need validation before adopting it +for releases.