Update to winit 0.29 - #3649
Update to winit 0.29#3649
Conversation
|
Current status: At least on mac, running |
|
@PingPongun thanks for the patch, but let's wait with the resvg update for a separate PR - this one is big enough as is 😓 |
|
I was testing to see if this branch fixes #3673, which it does, but on my machine it makes the main viewport on the demo app stop repainting whenever it is in the foreground and the extra viewport is open. The person who opened the issue did not observe the same behavior on their machine, which has a similar but different setup. winit_029_demo_app.mp4Here is my setup: |
|
Another problem I found: the modifier keys are ignored. Both the left and the right ones. They don't appear as down in the inspection window in the demo app, and they don't have any other effect either (aside from making text upper case, in the case of the shift key). |
|
According to @Wumpf this seems to work pretty well on Windows now. |
|
IME seems to work on my Mac. Are there any more blockers to merging this? |
|
Just tried it out on Linux Mint 21.2, everything seems to be working perfectly aside from window dragging, which causes the window to fly into the top left corner of the screen (see video) I've tested to make sure every letter on my keyboard works + modifiers, as well as the various demos in the demo app. Nothing obvious that's different aside form the previously mentioned bug. out.mp4 |
|
Thanks @Vickerinox - I think that is an acceptable level of regression. If anyone wants that feature to work they can make a PR :) |
<!-- Please read the "Making a PR" section of [`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md) before opening a Pull Request! * Keep your PR:s small and focused. * If applicable, add a screenshot or gif. * If it is a non-trivial addition, consider adding a demo for it to `egui_demo_lib`, or a new example. * Do NOT open PR:s from your `master` branch, as that makes it hard for maintainers to add commits to your PR. * Remember to run `cargo fmt` and `cargo cranky`. * Open the PR as a draft until you have self-reviewed it and run `./scripts/check.sh`. * When you have addressed a PR comment, mark it as resolved. Please be patient! I will review your PR, but my time is limited! --> This breaking change seems to have snuck in during #3649. Previously it would never consume these, today it consumes them unconditionally, and with this change it'll ~~only consume them if egui wants text input~~ never consume them. This is a blocker for updating our application, sadly :(
this partly restores event-emitting behaviour to the state before emilk#3649, when shortcuts such as Ctrl-C used to work regardless of the active layout. the difference is that physical keys are only used in case of the logical ones' absence now among the named keys.
…ve (#4461) resolves #4081 (see discussion starting from #3653 (comment) for extra context) this partly restores event-emitting behaviour to the state before #3649, when shortcuts such as `Ctrl` + `C` used to work regardless of the active layout. the difference is that physical keys are only used in case of the logical ones' absence now among the named keys. while originally I have only limited this to clipboard shortcuts (Ctrl+C/V/X), honestly it felt like a half-assed solution. as a result, I decided to expand this behaviour to all key events to stick to the original logic, in case there are other workflows and hotkeys people rely on or expect to work out of the box. let me know if this is an issue.
…ve (emilk#4461) resolves emilk#4081 (see discussion starting from emilk#3653 (comment) for extra context) this partly restores event-emitting behaviour to the state before emilk#3649, when shortcuts such as `Ctrl` + `C` used to work regardless of the active layout. the difference is that physical keys are only used in case of the logical ones' absence now among the named keys. while originally I have only limited this to clipboard shortcuts (Ctrl+C/V/X), honestly it felt like a half-assed solution. as a result, I decided to expand this behaviour to all key events to stick to the original logic, in case there are other workflows and hotkeys people rely on or expect to work out of the box. let me know if this is an issue.
## Problem winit has always delivered distinct physical variants for every keyboard key — \`KeyCode::ShiftLeft\` vs \`KeyCode::ShiftRight\`, \`KeyCode::ControlLeft\`/\`ControlRight\`, \`AltLeft\`/\`AltRight\`, \`SuperLeft\`/\`SuperRight\`, plus the ISO 102nd key \`KeyCode::IntlBackslash\` (the one between LShift and Z, labelled \`<>|\` on French AZERTY and \`\\|\` on UK QWERTY). Today none of these reach egui: - Pressing Shift / Ctrl / Alt alone produces *no* \`Event::Key\` at all. \`key_from_key_code\` and \`key_from_named_key\` both return \`None\` for modifiers, so the \`if let Some(active_key)\` branch in \`on_keyboard_input\` is skipped. The collapsed \`Modifiers\` bools are the only trace of the press, and they don't distinguish left vs right. - \`KeyCode::IntlBackslash\` has no arm in \`key_from_key_code\`, so on French / UK ISO keyboards the \`<>|\` key is completely invisible to egui apps — neither \`key\` nor \`physical_key\` is ever set. ## Who hits this - Games / kiosk frontends / pincab UIs that bind \`LeftFlipper = LShift\` vs \`RightFlipper = RShift\` (or \`LeftMagna = LCtrl\` vs \`RightMagna = RCtrl\`) — currently impossible inside egui without shelling out to platform APIs (\`device_query\`, raw X11, etc.). - Anyone on an ISO keyboard who wants to capture the 102nd key in an input-binding UI. Previously discussed: context in #2977 (closed by #3649 which added \`physical_key\`, but only for keys already in \`egui::Key\`). ## Change Two small additions, no behaviour change for existing code: **\`crates/egui/src/data/key.rs\`** — new variants at the end of \`Key\`: - \`ShiftLeft\`, \`ShiftRight\`, \`ControlLeft\`, \`ControlRight\`, \`AltLeft\`, \`AltRight\`, \`SuperLeft\`, \`SuperRight\` - \`IntlBackslash\` plus their entries in \`Key::ALL\`, \`Key::from_name\`, and \`Key::name\` (the \`key_from_name\` roundtrip test at the bottom of the file still passes). **\`crates/egui-winit/src/lib.rs\`** — new arms in \`key_from_key_code\`: \`\`\`rust KeyCode::ShiftLeft => Key::ShiftLeft, KeyCode::ShiftRight => Key::ShiftRight, // ...ControlLeft/Right, AltLeft/Right, SuperLeft/Right... KeyCode::IntlBackslash => Key::IntlBackslash, \`\`\` The existing \`Modifiers\` struct is untouched — shortcut matching (\"Ctrl+C\"), \`consume_shortcut\`, etc. still see the collapsed state. The new variants are purely additive and only surface as physical \`Event::Key\` presses when someone is specifically looking for them. ## Test - Existing \`test_key_from_name\` test still passes (updated the sentinel to \`Key::IntlBackslash as usize + 1\`). - Manual smoke test: pressing left vs right Shift, Ctrl, Alt each produces an \`Event::Key { key: Key::ShiftLeft/Right/..., physical_key: Some(Key::ShiftLeft/Right/...), ... }\`; pressing the AZERTY 102nd key yields \`Key::IntlBackslash\`. Character-key behaviour and \`Modifiers\` bools are unchanged. ## Not included - **Web backend** (\`eframe_web\`): \`PhysicalKey\` isn't fully exposed there yet per the existing \`physical_key\` docs, so these new variants are only emitted on native. Happy to extend to web in a follow-up if wanted. - \`ModifiersSymmetric\` / per-side sticky state: would be a bigger API change in \`Modifiers\`. This PR stays at the minimum: forward what winit already gives us for the event path. Closes no issue directly but addresses the underlying gap noted in the thread of #2977 (scancode forwarding) for the modifier / Intl-key subset.
scancodesvia egui-winit #2977