shifathosn
/
photocraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ui): one Cmd+V pastes one layer (#1638) (#1743) 

On macOS AppKit runs Edit > Paste's key equivalent and takes the
key-down, but winit still delivers the key-up (it forwards key-ups
while Cmd is held). The native menu fed egui a press plus a made-up
release, and clipboard_keys treats a Cmd+V release without a paste
event as an image paste (egui-winit drops Cmd+V when the clipboard
holds no text). So one Cmd+V pasted three times: the menu's press,
its synthetic release, and winit's real release.

- native_menu: a key equivalent adds only its press, inserted ahead of
  its key-up when that is already queued; winit delivers the release.
- clipboard_keys: a Cmd+V key press that reached egui marks the paste
  as seen, so its release doesn't paste again; any V/Insert release
  clears the mark (releasing Ctrl before V left it stale and swallowed
  the next image paste on Linux/Windows).
This commit is contained in:
Gert Pellin - GPE 2026-10-09 17:08:03 +02:00 committed by GitHub
parent 58289b2822
commit 310e46e6a0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 94 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
apps/photocraft/tests/mac_menu_appkit.rs
View File

@ -134,7 +134,8 @@ fn main() {
.iter()
.filter_map(|e| if let egui::Event::Key { key, pressed, modifiers, .. } = e { Some((*key, *pressed, modifiers.command)) } else { None })
.collect();
assert_eq!(keys, [(egui::Key::R, true, true), (egui::Key::R, false, true)], "the key press, then its release");
// AppKit took only the key-down; winit delivers the key-up itself (#1638).
assert_eq!(keys, [(egui::Key::R, true, true)], "the key press");
frame(&mut app, raw);
let ran: Vec<String> = photocraft_ui_egui::shortcut_dispatch::take_log(&ctx).into_iter().map(|(id, _)| id).collect();
assert_eq!(ran, ["view.rulers"], "⌘R ran Rulers, whichever ⌘R item AppKit matched");

53
crates/ui-egui/src/native_menu.rs
View File

@ -633,12 +633,23 @@ impl NativeMenu {
/// Before egui sees this frame's input: key equivalents become the key presses they were,
/// and clicks wait for [`NativeMenu::run`].
///
/// AppKit takes only the key-down: winit still delivers the key-up (it forwards key-ups while
/// ⌘ is held). So a key equivalent adds just its press, ahead of its key-up when that is
/// already queued. A second, made-up release made ⌘V paste three times (#1638): a ⌘V release
/// without a press is how an image paste reaches us (see [`crate::shortcuts::clipboard_keys`]).
pub fn raw_input(&mut self, raw: &mut egui::RawInput) {
for e in self.backend.drain() {
match e {
Event::Key(chord) => {
raw.events.extend(chord.key_event(true));
raw.events.extend(chord.key_event(false));
if let Some(press @ egui::Event::Key { key: pressed_key, .. }) = chord.key_event(true) {
let at = raw
.events
.iter()
.position(|e| matches!(e, egui::Event::Key { key, pressed: false, .. } if *key == pressed_key))
.unwrap_or(raw.events.len());
raw.events.insert(at, press);
}
}
Event::Click(id) => self.clicks.push(id),
}
@ -934,7 +945,7 @@ mod tests {
if let Some(m) = app.services.native_menu.as_mut() {
m.raw_input(&mut raw);
}
assert_eq!(raw.events.len(), 2, "a press and a release");
assert_eq!(raw.events.len(), 1, "the press: winit delivers the release");
frame(&ctx, raw, |ui| {
run(&mut app, ui.ctx());
crate::shortcuts::handle(&mut app, ui.ctx());
@ -947,6 +958,42 @@ mod tests {
assert_eq!(synced.borrow().len(), 1);
}
/// #1638: ⌘V through the Mac menu pasted three layers. AppKit takes the key-down for Edit ›
/// Paste's key equivalent, but winit still delivers the key-up (it forwards key-ups while ⌘ is
/// held). The menu's press plus that real release must paste once, whether the release
/// arrives in a later frame or already sits in the same input as the menu event.
#[test]
fn a_paste_key_equivalent_pastes_once() {
let release = || egui::Event::Key { key: Key::V, physical_key: Some(Key::V), pressed: false, repeat: false, modifiers: Modifiers::COMMAND };
for same_frame in [false, true] {
let mut app = app(true);
let ctx = egui::Context::default();
app.run("select.rect", json!({"x": 0, "y": 0, "width": 8, "height": 4})).unwrap();
app.run("edit.copy", json!({})).unwrap();
let layers = |app: &PhotocraftApp| app.session.active().unwrap().doc.layer_count();
let before = layers(&app);
let paste = Chord::parse("Cmd+V").unwrap();
app.services.native_menu = Some(NativeMenu::new(Box::new(Fake { synced: Default::default(), events: vec![Event::Key(paste)] })));
let step = |app: &mut PhotocraftApp, events: Vec<egui::Event>| {
let mut raw = egui::RawInput { events, ..Default::default() };
eframe::App::raw_input_hook(app, &ctx, &mut raw);
frame(&ctx, raw, |ui| {
run(app, ui.ctx());
crate::shortcuts::handle(app, ui.ctx());
});
};
if same_frame {
// A quick tap: the real key-up is already queued when the menu event is drained.
step(&mut app, vec![egui::Event::ModifiersChanged(Modifiers::COMMAND), release()]);
} else {
step(&mut app, vec![egui::Event::ModifiersChanged(Modifiers::COMMAND)]);
step(&mut app, vec![release()]);
}
step(&mut app, vec![egui::Event::ModifiersChanged(Modifiers::NONE)]);
assert_eq!(layers(&app), before + 1, "one ⌘V pastes one layer (key-up in the same frame: {same_frame})");
}
}
#[test]
fn clicks_run_the_command_and_resync() {
let synced = std::rc::Rc::new(std::cell::RefCell::new(Vec::new()));

51
crates/ui-egui/src/shortcuts.rs
View File

@ -172,7 +172,8 @@ fn clipboard_press(e: &egui::Event, mods: Modifiers, is_windows: bool) -> Option
/// egui-winit turns ⌘C / ⌘X / ⌘V into `Event::Copy` / `Cut` / `Paste` and drops the key press, and
/// drops ⌘V entirely when the clipboard holds no text (an image). Outside text fields, give the
/// pixel commands their key presses back: Copy/Cut/Paste become ⌘C/⌘X/⌘V presses (with ⇧/⌥ as
/// held, so ⇧⌘C is Copy Merged), and a ⌘V release without a paste event becomes a ⌘V press.
/// held, so ⇧⌘C is Copy Merged), and a ⌘V release without a ⌘V press (a paste event, or the Mac
/// menu's key equivalent) becomes a ⌘V press.
///
/// Ctrl+Insert copies and Shift+Insert pastes: egui-winit maps them on Windows (a Shift+Insert
/// release without a paste event pastes the image, like ⌘V), and we map them here on Linux.
@ -212,15 +213,28 @@ pub fn clipboard_keys(ctx: &egui::Context, typing: bool, raw: &mut egui::RawInpu
out.push(press(key, held));
continue;
}
if let egui::Event::Key { key, pressed: false, modifiers, .. } = &e
&& let Some(held) = match key {
Key::V if modifiers.command => Some(*modifiers),
Key::Insert if cfg!(target_os = "windows") && modifiers.shift && !modifiers.ctrl => Some(Modifiers::COMMAND),
_ => None,
match &e {
// A ⌘V press egui-winit didn't swallow (the Mac menu's key equivalent, Shift+Insert on
// Linux): its release must not paste a second time (#1638).
egui::Event::Key { key: Key::V, pressed: true, modifiers, .. } if modifiers.command => {
ctx.data_mut(|d| d.insert_temp(seen, true));
}
&& !ctx.data_mut(|d| d.remove_temp::<bool>(seen)).unwrap_or(false)
{
out.push(press(Key::V, held));
// A paste key's release ends that press, with or without ⌘ still held (else a stale
// mark would swallow the next image paste); without a press it is the paste itself.
egui::Event::Key { key: key @ (Key::V | Key::Insert), pressed: false, modifiers, .. } => {
let had_press = ctx.data_mut(|d| d.remove_temp::<bool>(seen)).unwrap_or(false);
let held = match key {
Key::V if modifiers.command => Some(*modifiers),
Key::Insert if cfg!(target_os = "windows") && modifiers.shift && !modifiers.ctrl => Some(Modifiers::COMMAND),
_ => None,
};
if let Some(held) = held
&& !had_press
{
out.push(press(Key::V, held));
}
}
_ => {}
}
out.push(e);
}
@ -487,6 +501,25 @@ mod tests {
assert!(matches!(r.events.as_slice(), [_, egui::Event::Paste(_)]));
}
/// #1638: every ⌘V is one press, however its key-down and key-up arrive.
#[test]
fn each_paste_key_is_one_press() {
let ctx = egui::Context::default();
let key = |key, pressed, modifiers| egui::Event::Key { key, physical_key: None, pressed, repeat: false, modifiers };
let presses = |events: Vec<egui::Event>| {
let mut r = egui::RawInput { events, ..Default::default() };
clipboard_keys(&ctx, false, &mut r);
r.events.iter().filter(|e| matches!(e, egui::Event::Key { key: Key::V, pressed: true, .. })).count()
};
// A ⌘V press that reached us (the Mac menu's key equivalent), then its release.
assert_eq!(presses(vec![key(Key::V, true, Modifiers::COMMAND)]), 1);
assert_eq!(presses(vec![key(Key::V, false, Modifiers::COMMAND)]), 0, "the release of a press doesn't paste again");
// A text paste whose V is released after ⌘: the next image paste still pastes.
assert_eq!(presses(vec![egui::Event::Paste("x".into())]), 1);
assert_eq!(presses(vec![egui::Event::ModifiersChanged(Modifiers::NONE), key(Key::V, false, Modifiers::NONE)]), 0);
assert_eq!(presses(vec![egui::Event::ModifiersChanged(Modifiers::COMMAND), key(Key::V, false, Modifiers::COMMAND)]), 1, "image paste");
}
/// #530: egui-winit sends Cut for Shift+Delete on Windows; on the canvas it opens Fill.
#[test]
fn windows_shift_delete_fills_instead_of_cutting() {