Skip to content

Update to winit 0.29 - #3649

Merged
emilk merged 51 commits into
emilk:masterfrom
fornwall:winit-0.29
Dec 18, 2023
Merged

emilk merged 51 commits into
emilk:masterfrom
fornwall:winit-0.29

Conversation

@fornwall

@fornwall fornwall commented Nov 27, 2023

Copy link
Copy Markdown
Contributor

@fornwall fornwall mentioned this pull request Nov 27, 2023
@fornwall

fornwall commented Nov 27, 2023

Copy link
Copy Markdown
Contributor Author

Current status: At least on mac, running cargo run --release -p egui_demo_app brings up the window and it can be resized, but not interacted with.

Comment thread crates/egui-winit/src/lib.rs Outdated
Comment thread crates/egui-winit/src/lib.rs Outdated
Comment thread crates/eframe/src/native/run.rs Outdated
Comment thread crates/egui-winit/src/lib.rs Outdated
Comment thread crates/egui-winit/src/lib.rs Outdated
@emilk emilk added eframe Relates to epi and eframe egui-winit porblems related to winit labels Nov 28, 2023
@emilk

emilk commented Nov 30, 2023

Copy link
Copy Markdown
Owner

@PingPongun thanks for the patch, but let's wait with the resvg update for a separate PR - this one is big enough as is 😓

@YgorSouza

Copy link
Copy Markdown
Contributor

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.mp4

Here is my setup:

OS: Fedora Linux 38 (Workstation Edition) x86_64 
Host: Laptop (12th Gen Intel Core) A6 
Kernel: 6.6.2-101.fc38.x86_64 
Resolution: 2256x1504 
DE: GNOME 44.6 
WM: Mutter 
WM Theme: Adwaita 
Theme: Adwaita-dark [GTK2/3] 
Icons: Adwaita [GTK2/3] 
CPU: 12th Gen Intel i7-1260P (16) @ 4.700GHz 
GPU: Intel Alder Lake-P GT2 [Iris Xe Graphics] 

@YgorSouza

Copy link
Copy Markdown
Contributor

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).

Comment thread crates/egui-winit/src/lib.rs Outdated
@ghost ghost mentioned this pull request Dec 5, 2023
@emilk

emilk commented Dec 12, 2023

Copy link
Copy Markdown
Owner

According to @Wumpf this seems to work pretty well on Windows now.

@emilk

emilk commented Dec 18, 2023

Copy link
Copy Markdown
Owner

IME seems to work on my Mac.

Are there any more blockers to merging this?

@Vickerinox

Vickerinox commented Dec 18, 2023

Copy link
Copy Markdown

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

@emilk

emilk commented Dec 18, 2023

Copy link
Copy Markdown
Owner

Thanks @Vickerinox - I think that is an acceptable level of regression. If anyone wants that feature to work they can make a PR :)

@emilk
emilk merged commit 8e5959d into emilk:master Dec 18, 2023
emilk pushed a commit that referenced this pull request Jan 17, 2024
<!--
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 :(
TicClick added a commit to TicClick/egui that referenced this pull request May 5, 2024
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.
emilk pushed a commit that referenced this pull request May 10, 2024
…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.
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…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.
emilk pushed a commit that referenced this pull request Jun 10, 2026
## 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file eframe Relates to epi and eframe egui-winit porblems related to winit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update to winit 0.29 Add non-portable keys to egui::Key Forward scancodes via egui-winit