diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -3965,8 +3965,7 @@ dependencies = [ [[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", @@ -3991,8 +3990,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", @@ -6907,8 +6905,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", @@ -6937,8 +6934,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", @@ -6972,8 +6968,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", ] @@ -6981,8 +6976,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", ] @@ -6990,8 +6984,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", ] @@ -6999,8 +6992,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", ] @@ -7008,8 +7000,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", @@ -7064,8 +7055,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", @@ -7074,8 +7064,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 @@ -192,3 +192,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" } + +# Shared OCLP Metal compatibility backport; see docs/oclp-wgpu-fork.md. +wgpu = { git = "https://github.com/ITJamie/wgpu-fork", rev = "21c08e23135ba27ed1c6b68177ecc20c0d076dd9" } diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -1447,14 +1447,13 @@ mod tests { } } - /// Issue #250: the window's device is created without wgpu's indirect-call validation pipeline, - /// which Intel Macs' Metal compiler fails to build (the device was lost, the window never opened). #[test] - fn window_has_no_indirect_call_validation_pipeline() { + fn window_preserves_configured_indirect_call_validation() { let eframe::egui_wgpu::WgpuSetup::CreateNew(n) = window_wgpu_options().wgpu_setup else { panic!("expected CreateNew") }; - if std::env::var_os("WGPU_VALIDATION_INDIRECT_CALL").is_none() { - assert!(!n.instance_descriptor.flags.contains(eframe::wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL)); - } + let eframe::egui_wgpu::WgpuSetup::CreateNew(defaults) = eframe::egui_wgpu::WgpuConfiguration::default().wgpu_setup else { + panic!("expected CreateNew") + }; + assert_eq!(n.instance_descriptor.flags, defaults.instance_descriptor.flags.with_env()); } /// Issue #234: Windows opens links with the URL protocol handler (the default browser), not diff --git a/apps/lightcraft/src/native_menu.rs b/apps/lightcraft/src/native_menu.rs --- a/apps/lightcraft/src/native_menu.rs +++ b/apps/lightcraft/src/native_menu.rs @@ -489,9 +489,7 @@ impl NativeMenu { // events never reach `shortcuts::handle`, so this is the one guard for AppKit's // accelerators and clicks: while the blocking library window or a quit/settings notice // owns input, a queued command is dropped (Quit stays for the existing confirmation). - let (cmd, params) = if key == QUIT { - (key, serde_json::json!({})) - } else if key == "app.about" || key == SETTINGS { + let (cmd, params) = if key == QUIT || key == "app.about" || key == SETTINGS { (key, serde_json::json!({})) } else if let Some(it) = self.items.get(&key) { (it.cmd.clone(), it.params.clone()) diff --git a/apps/lightcraft/src/window_gpu.rs b/apps/lightcraft/src/window_gpu.rs --- a/apps/lightcraft/src/window_gpu.rs +++ b/apps/lightcraft/src/window_gpu.rs @@ -147,7 +147,6 @@ pub fn base_options() -> WgpuConfiguration { if let WgpuSetup::CreateNew(n) = &mut c.wgpu_setup { n.instance_descriptor.backends = backend::window_backends(); n.instance_descriptor.backend_options = backend::backend_options(); - // no indirect-call validation pipeline: Intel Metal fails to compile it and the window never opens (#250) n.instance_descriptor.flags = backend::instance_flags_from_env(n.instance_descriptor.flags); } c diff --git a/contributors/people.toml b/contributors/people.toml --- a/contributors/people.toml +++ b/contributors/people.toml @@ -74,3 +74,6 @@ sync_github_name = true [people.dotCooCoo] real_name = "Robert Lee" sync_github_name = true + +[people.ITJamie] +sync_github_name = true diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -38,7 +38,7 @@ impl Session { } // the preset's depth is judged against the format written, known only now (`path`); a // depth the call passes itself is the caller's, and validated as such - if !p.get("bitDepth").is_some_and(|d| !d.is_null()) { + if p.get("bitDepth").is_none_or(|d| d.is_null()) { crate::export::ExportOptions::drop_unfit_depth(&mut out, fallback); } Ok(out) diff --git a/crates/gpu/src/backend.rs b/crates/gpu/src/backend.rs --- a/crates/gpu/src/backend.rs +++ b/crates/gpu/src/backend.rs @@ -147,17 +147,12 @@ pub fn backend_options() -> wgpu::BackendOptions { o } -/// wgpu's instance flags for every instance LightCraft creates, from `flags` (wgpu's defaults with its -/// environment variables applied) without `VALIDATION_INDIRECT_CALL`, unless -/// `WGPU_VALIDATION_INDIRECT_CALL` asks for it (any value but `0`). That validation builds an internal -/// compute pipeline when each device is created; Intel Macs' Metal compiler fails on it ("Compiler -/// encountered an internal error"), the device is lost and the window never opens (#250). It only -/// guards indirect draws and dispatches, which neither LightCraft nor egui issue. +/// Preserve configured instance flags unless the explicit indirect-validation override changes them. pub fn instance_flags(flags: wgpu::InstanceFlags, env: Option<&str>) -> wgpu::InstanceFlags { - if env.is_some_and(|v| v != "0") { - flags | wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL - } else { - flags - wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL + match env { + None => flags, + Some("0") => flags - wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL, + Some(_) => flags | wgpu::InstanceFlags::VALIDATION_INDIRECT_CALL, } } @@ -346,15 +341,16 @@ pub fn window_renderer_order( #[allow(clippy::unwrap_used)] mod tests { - /// #250: the indirect-call validation pipeline is off unless asked for; other flags are kept. #[test] - fn indirect_call_validation_is_off_unless_asked_for() { + fn indirect_call_validation_preserves_configuration_unless_overridden() { use wgpu::InstanceFlags as F; let base = F::VALIDATION_INDIRECT_CALL | F::DEBUG; - assert_eq!(instance_flags(base, None), F::DEBUG); + assert_eq!(instance_flags(base, None), base); assert_eq!(instance_flags(base, Some("0")), F::DEBUG); assert_eq!(instance_flags(F::empty(), Some("1")), F::VALIDATION_INDIRECT_CALL); assert_eq!(instance_flags(F::empty(), None), F::empty()); + assert_eq!(instance_flags(F::DEBUG, None), F::DEBUG); + assert_eq!(instance_flags(F::DEBUG, Some("1")), base); } use super::*; diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -299,7 +299,6 @@ impl Gpu { desc.backends = backends; // DX12 shaders compile with FXC (issue #471) desc.backend_options = crate::backend::backend_options(); - // no indirect-call validation pipeline: Intel Metal fails to compile it (#250) desc.flags = crate::backend::instance_flags_from_env(desc.flags); let instance = wgpu::Instance::new(desc); let adapter = pollster::block_on( diff --git a/crates/gpu/src/nn.rs b/crates/gpu/src/nn.rs --- a/crates/gpu/src/nn.rs +++ b/crates/gpu/src/nn.rs @@ -86,7 +86,6 @@ fn make_device(backends: wgpu::Backends) -> Result { desc.backends = backends; // DX12 shaders compile with FXC (issue #471) desc.backend_options = crate::backend::backend_options(); - // no indirect-call validation pipeline: Intel Metal fails to compile it (#250) desc.flags = crate::backend::instance_flags_from_env(desc.flags); let instance = wgpu::Instance::new(desc); let wanted = std::env::var("LIGHTCRAFT_GPU_ADAPTER").ok().map(|s| s.trim().to_lowercase()).filter(|s| !s.is_empty()); diff --git a/crates/ui-egui/src/panels/rules_move.rs b/crates/ui-egui/src/panels/rules_move.rs --- a/crates/ui-egui/src/panels/rules_move.rs +++ b/crates/ui-egui/src/panels/rules_move.rs @@ -154,7 +154,7 @@ pub fn move_node(rs: &mut RuleSet, from: &[usize], dest: &[usize]) -> Option chain.len() && dest.starts_with(&chain)) && list_at(&next, &chain).is_some_and(|l| l.rules.is_empty()) { + while !(chain.is_empty() || dest.len() > chain.len() && dest.starts_with(&chain)) && list_at(&next, &chain).is_some_and(|l| l.rules.is_empty()) { remove_at(&mut next, &chain)?; if !shift_after_removal(&mut dest, &chain) { return None; diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -25,6 +25,9 @@ per-pixel decision. At 6000 × 4000 with lens warp + perspective the mask takes ~2.7 ms (32 threads; ~24.5 ms on one) instead of ~30 ms (~580 ms), and only when warped geometry rebuilds. +The [shared OCLP wgpu fork proposal](oclp-wgpu-fork.md) records the pinned Metal +compiler workarounds, organization ownership proposal and hardware checks. + ## Backends, environment variables and troubleshooting (issue #136) wgpu loads the driver of **every** backend in an instance's set while it enumerates adapters — even when it then picks another one. A Vulkan driver that crashes there (issue #136: an access violation 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,109 @@ +# Proposal: share the OCLP wgpu compatibility fork + +LightCraft uses the same wgpu 30.0.1 backport proposed for +[ArtCraft Launcher](https://github.com/storytold/craft-launcher/pull/917) and +[PhotoCraft](https://github.com/storytold/photocraft/pull/9711). +It restores configured internal indirect-call validation for window, rendering +and model devices while preserving the existing startup recovery chain. + +## Dependency and ownership + +- Prototype: +- Branch: `codex/metal-msl-intel-v30.0.1` +- Immutable commit: `21c08e23135ba27ed1c6b68177ecc20c0d076dd9` +- Base: upstream `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. Shader bounds checks +remain enabled. `WGPU_VALIDATION_INDIRECT_CALL` still explicitly overrides the +configured validation flag; without it, LightCraft preserves wgpu's defaults. + +Related reports are [#250](https://github.com/storytold/lightcraft/issues/250) +and [#1939](https://github.com/storytold/lightcraft/issues/1939). The earlier +[#1356](https://github.com/storytold/lightcraft/pull/1356) proposed vendoring +wgpu-hal for MSL probing and was closed without merging. This proposal instead +shares a release-based fork, including the Intel compiler workarounds. The MSL +fallback follows alecanales' original +[patch](https://github.com/storytold/photocraft/pull/5910). + +A maintainer should decide whether to create `storytold/wgpu` and preserve the +backport commit and branch there. Then switch the patch URL in `Cargo.toml`, +keep the reviewed commit pin, and regenerate `Cargo.lock`. The apps should share +this dependency and return to an upstream release after both fixes land and +hardware validation passes. + +## Validation + +```sh +cargo build --locked -p lightcraft --features heif +cargo xtask ci +``` + +Use a fresh `LIGHTCRAFT_CONFIG_DIR` and a separate `--library` directory as +specified in [the control protocol](control-protocol.md). Import only synthetic +images, wait for the Detail render to finish, verify `ui.inspect`'s `perf.gpu`, +and inspect an internal screenshot. Reject a screenshot that falls back to +headless CPU rendering as evidence of native-window success. + +On the OCLP Mac, set `LIGHTCRAFT_GPU_BACKEND=metal`, +`WGPU_VALIDATION_INDIRECT_CALL=1`, `MTL_DEBUG_LAYER=0` and +`MTL_SHADER_VALIDATION=0`. External Metal validation layers are incompatible +with its patched framework; internal wgpu validation stays enabled. +`WGPU_POWER_PREF=high` or `low` selects the window's adapter preference; confirm +the actual adapter in logs. The photo-rendering device independently requests +high performance, so an Intel window does not establish Intel compute coverage +on a dual-GPU Mac. + +On Apple M4 Max, an isolated native run used Metal for both the window and +photo rendering, applied +0.35 exposure to a synthetic 512 × 384 PNG and exported +it without a reported GPU fallback. The completed Detail view and native +screenshot were visually inspected. The debug window responded after +approximately two seconds. + +Both the [Clippy prerequisite](https://github.com/storytold/lightcraft/pull/3085) +and this compatibility branch independently passed all eight `cargo xtask ci` +stages: formatting, strict workspace Clippy, HEIF, tests/doctests, parity, +layers, assets and WebAssembly. Validation used Rust 1.96.1 and the documented +`CRAFT_FONTS_DIR` input from `storytold/craft-fonts`, commit +`aced4137c022f1f587624d48e81ec77e1914a9cc`. No fonts are added to this PR. +The full-CI log reports 5,183 passes and 42 ignored tests; the CI report records +147 availability-skip entries for corpus data, models and missing +script-specific fonts. + +On MacBookPro11,5 running macOS 15.8.1 through OCLP: + +- Native build, strict workspace Clippy, all 83 app tests and all 12 focused + backend/validation regression tests passed. +- Separate runs confirmed AMD Radeon R9 M370X and Intel Iris Pro Graphics for + the window. Both used AMD for photo compute, completed the synthetic photo's + +0.35 exposure edit and PNG export without a reported GPU fallback, and + produced native screenshots that were visually inspected. +- All 30 focused AMD GPU-versus-CPU edit comparisons passed without skips: + worst mean difference 0.0001 and maximum difference one 8-bit level. +- The debug window responded after approximately 64 seconds with the AMD + window and 63 seconds with the Intel window. Automatic MSL probing selected + 3.1 instead of the OS maximum 3.2. + +Internal indirect-call validation remained enabled; external Metal validation +layers were disabled on OCLP. These runs do not establish Intel photo-compute, +model-inference or broader editing-workflow coverage. + +## Known limits of the shared backport + +The backport's full wgpu suite on Apple M4 Max ran 1,010 tests: 1,002 passed, +eight failed and six were skipped: + +- `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. + +This is a prototype, not a clean full-suite pass. Other OCLP machines, +Monterey, other desktop platforms, raw-camera workflows and model inference +still need validation before release adoption. Launcher and PhotoCraft results +do not establish LightCraft coverage.