fix and expand the support of the window_minimized_event #552

InnocentusLime · 2025-05-03T20:27:12Z

Why?

macroquad has a bug: not-fl3/macroquad#547.
There is a good fix we can implement, that has been suggested by @profan - the same issue, but in godot (godotengine/godot#28061). Essentially, this PR part 1 of the fix.

Context

miniquad already has some ready-to-use handlers for a potential fix. There is window_minimized_event. Despite the name, on X Window system and WASM it gets fired when a window loses focus.

I decided not to change that and instead focus on the following:

Adding a similar support for OSX (special thanks to @jonatino for the help!)
Adding a similar support for Windows
Fixing a related bug on WASM

Bug Reproduction

The following application is sufficient to catch the bug

use macroquad::prelude::*;

#[macroquad::main("Catching Keys")]
async fn main() {
    loop {
        clear_background(WHITE);

        let col = if is_key_down(KeyCode::A) {
            GREEN
        } else {
            RED
        };
        draw_re
8000
ctangle(0.0, 0.0, 32.0, 32.0, col);

        next_frame().await
    }
}

The steps are simple:

Run the app
Hold the A key
While still holding the key -- focus on a different window
Release the A key
Focus again on the window
The square remains green (because is_key_down is still returning true)

Testing The Fix

Use the same setup
Add the following lines to your Cargo.toml
Repeat the same steps
The square will now turn red on step (6)!

[patch.crates-io]
# hosts the second part of the fix
macroquad = { git = "https://github.com/InnocentusLime/macroquad.git", branch="lab-repeat-keys" }
# This branch with the fix
miniquad = { git = "https://github.com/InnocentusLime/miniquad.git", branch="fix-window-minimized" }

Tests:

Works on Windows
Works on MacOS/OsX (tested by @birhburh)
Works on WASM
Works on X11 (tested by @profan)

Technical Notes About the fixed `js/gl.js`

There were a few things that did not make the minimized events fire correctly:

~~lastFocus == hasFocus caused the app being notified of focus only when it didn't change~~ (fixed in 8d7cb80)
the visibilitychange listener had a mistake. Elements do not lose focus when the browser changes tabs of the browser window becomes inactive. Instead document.visibilityState becomes not equal to "visible"

jonatino · 2025-05-03T21:18:20Z

I've also made similar changes here for reference:

Miniquad commits:
windows & wasm: VoltaSoftware@59549b6
android & linux: VoltaSoftware@5a87f41
macos: VoltaSoftware@3e32633
ios: VoltaSoftware@9a8af70

Macroquad commit:
VoltaSoftware/macroquad@49972d9?w=1#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759

InnocentusLime · 2025-05-03T21:22:06Z

I've also made similar changes here for reference:

Miniquad commits: windows & wasm: VoltaSoftware@59549b6 android & linux: VoltaSoftware@5a87f41 macos: VoltaSoftware@3e32633 ios: VoltaSoftware@9a8af70

Macroquad commit: VoltaSoftware/macroquad@49972d9?w=1#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759

Thank you! Gives me more confidence, that this fix is on the right track :D

profan · 2025-05-03T22:08:40Z

Verified on X11 with Ubuntu 24.04.2 LTS (with a somewhat esoteric window manager to be fair, running awesomewm, a tiling wm), not sure if we want/need someone running Wayland to try it as well or if it's fine with just X11?

Seems to be working as expected :)

edit: had a friend try it under Hyprland (Wayland) on Arch as well and it seems to be working there too

InnocentusLime · 2025-05-03T22:24:27Z

Verified on X11 with Ubuntu 24.04.2 LTS (with a somewhat esoteric window manager to be fair, running awesomewm, a tiling wm), not sure if we want/need someone running Wayland to try it as well or if it's fine with just X11?

Seems to be working as expected :)

edit: had a friend try it under Hyprland (Wayland) on Arch as well and it seems to be working there too

Good question. I have no idea what policy macroquad has towards Wayland. Last time I checked the support was "unstable". So technically speaking, bugs for Wayland are to be expected are not required to be fixed by this PR.

That said, if Wayland does get fixed too -- good!

InnocentusLime · 2025-05-04T16:03:54Z

@not-fl3 the PR has been tested on all declared platforms and seems to work! Any chance of it getting merged? :)

jonatino · 2025-05-04T17:04:27Z

@InnocentusLime I think your osx changes are missing the event for when the window gets reactivated.

You should use this instead because these are the actual activated/deactivated windows events for macos: VoltaSoftware@3e32633

https://developer.apple.com/documentation/appkit/nswindowdelegate/windowdidbecomekey(_:)
https://developer.apple.com/documentation/appkit/nswindowdelegate/windowdidresignkey(_:)

InnocentusLime · 2025-05-04T17:52:48Z

@InnocentusLime I think your osx changes are missing the event for when the window gets reactivated.

You should use this instead because these are the actual activated/deactivated windows events for macos: VoltaSoftware@3e32633

https://developer.apple.com/documentation/appkit/nswindowdelegate/windowdidbecomekey(_:)
https://developer.apple.com/documentation/appkit/nswindowdelegate/windowdidresignkey(_:)

Interesting. I just did what Godot did in their PR. I suppose I can just alter the commit to use those instead. Thx! :)

InnocentusLime · 2025-05-04T17:55:21Z

@jonatino
Although, it looks like DidMove should trigger the minimized event too just in case.
I will add the handlers instead.

narodnik · 2025-05-05T10:59:30Z

Hey this is the same as this issue: #517

While your fix should be applied probably, it doesn't fix the overall issue with the key APIs. For that we should copy what Qt does with making available the actual scancodes, as well as merging the char and various key events into a unified API.

InnocentusLime · 2025-05-05T11:20:13Z

Hey this is the same as this issue: #517

While your fix should be applied probably, it doesn't fix the overall issue with the key APIs. For that we should copy what Qt does with making available the actual scancodes, as well as merging the char and various key events into a unified API.

The API might be a bit confusing right now, but KeyCode is actually the key scancode. It is why not-fl3/macroquad#686 exists actually

narodnik · 2025-05-05T13:58:16Z

KeyCode is not the scan code. It changes between key presses. For example Colon and Semicolon. With scancodes they have the same value.

profan · 2025-05-05T14:08:44Z

@narodnik that's odd, considering there's this issue: not-fl3/macroquad#686 which seems to imply all the key codes are unrelated to the keyboard layout, so we have some awkward middleground right now then?

InnocentusLime · 2025-05-05T14:30:41Z

KeyCode is not the scan code. It changes between key presses. For example Colon and Semicolon. With scancodes they have the same value.

This is factually incorrect. There is no Colon in the KeyCode enum.

https://github.com/not-fl3/miniquad/blob/master/src/event.rs#L21-L145

I have reviewed the windows implementation. I can vouch that it works with scancodes.
Yes, KeyCode is not exactly a raw scancode value. That is true, but it behaves exactly like it

narodnik · 2025-05-05T15:04:56Z

Linux wayland does not use scancodes. It converts them to key syms, then converts the key syms to key codes. For tracking key state, you should use the raw key passed in (which it does), but this does not match the key codes used in miniquad. The miniquad KeyCode does not work for key repeat.

Check unsafe extern "C" fn keyboard_handle_key() in src/native/linux_wayland.rs.

unsafe extern "C" fn keyboard_handle_key(
    ...
    key: u32,
    ...
) {
    // ...

    // https://wayland-book.com/seat/keyboard.html
    // To translate this to an XKB scancode, you must add 8 to the evdev scancode.
    let keysym = libxkb.keymap_key_get_sym_without_mod(xkb_keymap, key + 8);
    let keycode = keycodes::translate_keysym(keysym);
    let keymods = display.keymap.get_keymods(libxkb, xkb_state);
    match state {
        0 => {
            display.keyboard_context.track_key_up(key);
            display.events.push(WaylandEvent::KeyUp(keycode, keymods));
        }

bolphen · 2025-05-05T15:05:12Z

Just to weigh in on this: as I understand, what this PR does is to fire the window_minimized_event / window_restored_event upon focus lost / gain, and has nothing to do with keyboard input. Moreover these events are already being fired on Linux (both X11 and Wayland) before this PR; the PR just implements them on the other platforms (which btw is a good indication that the Wayland backend is actually pretty on a par with the other platforms).

There's also a related issue by Fedor himself #28 although I think we can all agree that this is basically window_minimized now.

InnocentusLime · 2025-05-05T17:00:02Z

Just to weigh in on this: as I understand, what this PR does is to fire the window_minimized_event / window_restored_event upon focus lost / gain, and has nothing to do with keyboard input. Moreover these events are already being fired on Linux (both X11 and Wayland) before this PR; the PR just implements them on the other platforms (which btw is a good indication that the Wayland backend is actually pretty on a par with the other platforms).

There's also a related issue by Fedor himself #28 although I think we can all agree that this is basically window_minimized now.

You are absolutely correct.

Yes, the key mapping is completely unrelated to this PR.
Yes, this PR makes window_minimized_event and window_restored_event properly cross-platform

InnocentusLime · 2025-05-05T17:43:27Z

@narodnik

Linux wayland does not use scancodes. It converts them to key syms, then converts the key syms to key codes. For tracking key state, you should use the raw key passed in (which it does), but this does not match the key codes used in miniquad. The miniquad KeyCode does not work for key repeat.

Apologies. I may not have a good grasp of how wayland works.

Regardless, I believe KeyCode should remain related to physical key locations and nothing else. Otherwise it becomes difficult to use for gamedev purposes.

This PR is also in fact unrelated to #517, despite what you said. So I would suggest moving the keyboard processing related discussion there

InnocentusLime · 2025-05-14T19:16:50Z

@not-fl3

not-fl3 · 2025-05-15T08:52:07Z

Oh, thanks for PR, missed it!

InnocentusLime force-pushed the fix-window-minimized branch from 4aab146 to 1ff3c7f Compare May 4, 2025 07:54

InnocentusLime mentioned this pull request May 4, 2025

fix the sticky key down not-fl3/macroquad#962

Merged

InnocentusLime force-pushed the fix-window-minimized branch from 1ff3c7f to a33b678 Compare May 4, 2025 16:29

fix and expand the support of the window_minimized_event

6a00a94

InnocentusLime force-pushed the fix-window-minimized branch from a33b678 to 6a00a94 Compare May 4, 2025 17:57

not-fl3 merged commit 52bdf3f into not-fl3:master May 15, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix and expand the support of the window_minimized_event #552

fix and expand the support of the window_minimized_event #552

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix and expand the support of the window_minimized_event #552

fix and expand the support of the window_minimized_event #552

Uh oh!

Conversation

Uh oh!

Why?

Context

Bug Reproduction

Testing The Fix

Technical Notes About the fixed js/gl.js

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Technical Notes About the fixed `js/gl.js`