diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml index 5f93f997..9dc8fe0b 100644 --- a/.github/workflows/freebsd.yml +++ b/.github/workflows/freebsd.yml @@ -28,18 +28,21 @@ jobs: steps: - uses: actions/checkout@v4 - name: Build and test in a FreeBSD VM - uses: vmactions/freebsd-vm@v1 + uses: vmactions/freebsd-vm@c46abacb49f09938ca4e1702d15d836285d694cc # v1 with: release: "14.3" usesh: true + cpu: 4 mem: 12288 # Nothing to bring back; copying the target dir out of the VM fails on large builds. copyback: false prepare: | - pkg install -y curl pkgconf cmake python3 libxkbcommon wayland libX11 libXcursor libXrandr libXi libxcb mesa-libs vulkan-loader gtk3 fontconfig freetype2 alsa-lib + pkg install -y git curl pkgconf cmake python3 libxkbcommon wayland libX11 libXcursor libXrandr libXi libxcb mesa-libs vulkan-loader gtk3 fontconfig freetype2 alsa-lib curl -fsSL https://sh.rustup.rs | sh -s -- -y --profile minimal run: | . "$HOME/.cargo/env" + # The checkout is synced in under another owner; let tests that call git read it. + git config --global --add safe.directory "*" rustc --version CARGO_PROFILE_DEV_DEBUG=0 cargo build --workspace --all-targets --locked CARGO_PROFILE_DEV_DEBUG=0 cargo test --workspace --locked diff --git a/crates/engine/src/shortcuts.rs b/crates/engine/src/shortcuts.rs index 4165cde8..52888624 100644 --- a/crates/engine/src/shortcuts.rs +++ b/crates/engine/src/shortcuts.rs @@ -363,6 +363,22 @@ pub struct Binding { pub panel: Option, } +/// Off macOS the Control and Command keys are the same key. The built-in tables are written for +/// macOS, so a binding on the Mac Control key (`⌃T`) can land on the same key as a Command binding +/// (`⌘T`) in the same context: the Command binding wins and the Control one is left out there +/// (that command has no default key on that platform). macOS keeps both. +fn drop_control_collisions(b: &mut Vec, p: Platform) { + if p.is_mac() { + return; + } + let primary: Vec<(String, Option)> = + b.iter().filter(|x| x.chord().is_some_and(|c| !c.mods.ctrl)).map(|x| (x.context().to_string(), x.chord().map(|c| c.effective(p)))).collect(); + b.retain(|x| { + let Some(c) = x.chord() else { return true }; + !(c.mods.ctrl && primary.iter().any(|(ctx, k)| ctx == x.context() && *k == Some(c.effective(p)))) + }); +} + impl Binding { fn new(command: &str, keys: &str, panel: Option<&str>) -> Self { Binding { command: command.to_string(), keys: normalize(keys).unwrap_or_else(|_| keys.to_string()), panel: panel.map(str::to_string) } @@ -564,8 +580,15 @@ impl Shortcuts { } /// Premiere-default entries FilmCraft Default adopts: commands with no shortcut of their own - /// whose Premiere key is free in that context. + /// whose Premiere key is free in that context on this platform. pub fn audit(&self) -> Vec { + self.audit_for(Platform::current()) + } + + /// [`Self::audit`] for platform `p`. The Premiere table is Premiere's macOS keyboard: off macOS + /// the Control and Command keys are the same key, so an entry whose key would collide there + /// with one already taken (`⌃T` vs `⌘T`) is left out, and the command keeps no default key. + pub(crate) fn audit_for(&self, platform: Platform) -> Vec { let base = self.base_defaults(); let mut added: Vec = Vec::new(); for (c, k, p) in presets::PREMIERE { @@ -573,7 +596,8 @@ impl Shortcuts { continue; } let b = Binding::new(c, k, panel_opt(p)); - let taken = base.iter().chain(added.iter()).any(|o| o.context() == b.context() && o.chord() == b.chord()); + let key = b.chord().map(|c| c.effective(platform)); + let taken = base.iter().chain(added.iter()).any(|o| o.context() == b.context() && o.chord().map(|c| c.effective(platform)) == key); if !taken { added.push(b); } @@ -583,8 +607,13 @@ impl Shortcuts { /// The bindings of a built-in preset. pub fn builtin(&self, name: &str) -> Option> { + self.builtin_for(name, Platform::current()) + } + + /// [`Self::builtin`] as it is built on platform `p`. + pub(crate) fn builtin_for(&self, name: &str, p: Platform) -> Option> { let mut b = self.base_defaults(); - b.extend(self.audit()); + b.extend(self.audit_for(p)); let table: Vec = match name { DEFAULT_PRESET => Vec::new(), PREMIERE_PRESET => presets::premiere(), @@ -593,6 +622,7 @@ impl Shortcuts { _ => return None, }; self.apply_table(&mut b, &table); + drop_control_collisions(&mut b, p); Some(b) } diff --git a/crates/engine/src/shortcuts_tests.rs b/crates/engine/src/shortcuts_tests.rs index 0bf0b63b..d4147199 100644 --- a/crates/engine/src/shortcuts_tests.rs +++ b/crates/engine/src/shortcuts_tests.rs @@ -218,3 +218,21 @@ fn list_searches_labels_and_keys() { let v = s.execute("shortcuts.list", json!({"panel": "History", "assigned": true})).unwrap(); assert_eq!(v.as_array().unwrap().len(), 2, "{v}"); } + +#[test] +fn default_keymaps_have_no_conflicts_on_any_platform() { + // The Premiere audit used to check keys on macOS only, so off macOS `⌃9`/`⌘9`, `⌃⇧M`/`⌘⇧M` and + // `⌃T`/`⌘T` ended up on the same Ctrl key; CI only ran on macOS, so nobody saw it. + use crate::shortcuts::Platform; + let s = Session::default(); + for p in [Platform::Mac, Platform::Windows, Platform::Linux] { + let mut sc = s.shortcuts.clone(); + sc.bindings = sc.builtin_for(crate::shortcuts::DEFAULT_PRESET, p).unwrap(); + assert!(sc.conflicts(p).is_empty(), "{p:?}: {:?}", sc.conflicts(p)); + } + // macOS keeps every Premiere key it had + let mac = s.shortcuts.builtin_for(crate::shortcuts::DEFAULT_PRESET, Platform::Mac).unwrap(); + for (cmd, keys) in [("multicam.cutToCamera9", "Ctrl+9"), ("markers.addRange", "Ctrl+Shift+M"), ("graphics.newText", "Cmd+T")] { + assert!(mac.iter().any(|b| b.command == cmd && b.keys == keys), "{cmd} {keys} on macOS"); + } +} diff --git a/crates/mpeg2v/src/synth_tests.rs b/crates/mpeg2v/src/synth_tests.rs index cc275877..0d4dd545 100644 --- a/crates/mpeg2v/src/synth_tests.rs +++ b/crates/mpeg2v/src/synth_tests.rs @@ -758,7 +758,7 @@ fn seed_sweep() { ]; for seed in 1..=8u64 { for c in cfgs { - check(Cfg { seed: (seed * 0x9E37_79B9_7F4A_7C15) ^ c.w as u64, ..c }); + check(Cfg { seed: seed.wrapping_mul(0x9E37_79B9_7F4A_7C15) ^ c.w as u64, ..c }); } } }