Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows is not location aware for modifier keys #3611

Open
junglie85 opened this issue Mar 28, 2024 · 15 comments
Open

Windows is not location aware for modifier keys #3611

junglie85 opened this issue Mar 28, 2024 · 15 comments
Labels
B - bug Dang, that shouldn't have happened DS - windows

Comments

@junglie85
Copy link

Description

Cannot detect when left or right version of a modifier is pressed, it just shows as a single modifier without the left/right info. e.g. if left shift is down, it does not detect that right shift is down. I can also press left shift followed by right shift, then release left shift and it only reports a release when I finally release right shift.

[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        SHIFT,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lshift_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.rshift_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        0x0,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lshift_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.rshift_state() == winit::keyboard::ModifiersKeyState::Pressed = false

This seems pretty buggy to me.

Windows version

Microsoft Windows [Version 10.0.19045.4170]

Winit version

0.29.15

@junglie85 junglie85 added B - bug Dang, that shouldn't have happened DS - windows labels Mar 28, 2024
@junglie85
Copy link
Author

Same goes for all other modifiers (except Alt, which is really Alt and AltGr), in which case I get 4 modifier events:

[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        ALT,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false    
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        CONTROL | ALT,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        0x0,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        0x0,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false

But if I just press AltGr, I get 2 events immediately before I even release:

[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        CONTROL,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false    
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:270] _modifiers = Modifiers {
    state: ModifiersState(
        0x0,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:271] _modifiers.lalt_state() == winit::keyboard::ModifiersKeyState::Pressed = false
[age\src\os.rs:272] _modifiers.ralt_state() == winit::keyboard::ModifiersKeyState::Pressed = false

@kchibisov
Copy link
Member

l/rshift states are arbitrary and shouldn't be used at all, because they even tell you that the value isUndefined`, this API can not work in 100% cases cross platform and can only do so on e.g. macOS, which is where it's used.

Left/Right shift are clearly different from your logs, however you had a repeat: true when you press left -> right and right -> left modifiers.

The thing that you shouldn't use this API unless you really need it is documented on the API.

@junglie85
Copy link
Author

I give up! You tell me on a previous issue that I should use the modifiers to detect these keys states. And now you tell me that I shouldn’t use modifiers to detect this.

It’s entirely possibly to detect this using the Windows C API. What is the correct way to detect this using won’t on windows?

@kchibisov
Copy link
Member

I said numerous times that if you want to test for key you use logical_key, if you want to test for state, like bind, you use ModifiersState event. I'm not sure how to make it even more clear, just tell me the use case and I'll describe how I'd approach it.

@junglie85
Copy link
Author

I would like to test for if left shift is pressed, held or released and also if right shift is pressed, held or released, and to be able to do this for both at the same time.

@kchibisov
Copy link
Member

If you want to try and do this in 100% cases in the current state of things there's no clear way to do this cross platform, because keymap queries are not done. I'll work on that for 0.31, but no promise.

If you only care when your app was focused, you can manually track them by using the Pressed?Released state and logical_key + location property. This is sufficient for you. In your case you don't need to care about modifiers, since you care about buttons.

Modifier input is different, because it doesn't directly map to physical key and as was said numerous times, Shift modifier could be active without any physical shift key being pressed.

If you want a binding, like Shift+Shift, you there's an option to detect when ModifiersState::contains(SHIFT) && key.logical_key == NamedKey::Shift, this will also work reliably.

Keep in mind that if user focuses your application with shifts latched, and you trigger something based on that, it'll clearly be very surprising for them; most apps require to repress.

@junglie85
Copy link
Author

Any example you are able to give would be very appreciated. It seems like it should be obvious but I’m just not getting it, and am getting a bit frustrated as a result. Sorry that I let that impact my communication style.

@junglie85
Copy link
Author

Thanks for the explanation. I don’t agree that will work; it’s what I’m currently doing and as I tried to explain in #3610 (comment), it doesn’t capture all the relevant events for this to work.

@kchibisov
Copy link
Member

It doesn't work because it's marked as repeat for whatever reason, which is a bug.

#[derive(Default)]
struct InputTracker {
    left_shift: bool,
    right_shift: bool,
}

let mut tracker = InputTracker::default();
match window_event {
    WindowEvent::KeyboardInput {event, .. } {
        // We don't really care about repeat, it's still pressed.
        if event.logical_key() == NamedKey::Shift {
            let location = event.location();
            let is_pressed = event.is_pressed();
            if location == KeyLocation::Left {
                tracker.left_shift = is_pressed;
            }
            if location == KeyLocation::Right {
                tracker.right_shift = is_pressed;
            }
        }
    }
    // Doesn't really matter what state it's.
    WindowEvent::Focused(_) => {
        tracket = Default::default();
    }
}

Maybe something like that will work for you?

@junglie85
Copy link
Author

Thanks for the suggestion. I hadn't clocked that the repeat was a bug.

Using that implementation, I still have the same problem - only one of the release events is emitted and results in one of the keys being marked as pressed when it is released:

[age\src\app.rs:183:29] &tracker = InputTracker {
    left_shift: true,
    right_shift: false,
}
[age\src\app.rs:183:29] &tracker = InputTracker {
    left_shift: true,
    right_shift: true,
}
[age\src\app.rs:183:29] &tracker = InputTracker {
    left_shift: false,
    right_shift: true,
}

@kchibisov
Copy link
Member

This is also a bug then, with either Win32 (maybe it's intended there) or maybe how our windows code works.

For now you can assume on windows that ModifiersChanged without shift also clears the presses.

@junglie85
Copy link
Author

Thanks for clarifying. I still don't think that helps because the ModifiersChanged event is not emitted until I release the second shift key in this example. There's no way of knowing from this information when I actually release the first shift key:

[age\src\os.rs:273:17] _modifiers = Modifiers {
    state: ModifiersState(
        SHIFT,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:255:17] event = KeyEvent {
    physical_key: Code(
        ShiftLeft,
    ),
    logical_key: Named(
        Shift,
    ),
    text: None,
    location: Left,
    state: Pressed,
    repeat: false,
    platform_specific: KeyEventExtra {
        text_with_all_modifers: None,
        key_without_modifiers: Named(
            Shift,
        ),
    },
}
[age\src\os.rs:255:17] event = KeyEvent {
    physical_key: Code(
        ShiftRight,
    ),
    logical_key: Named(
        Shift,
    ),
    text: None,
    location: Right,
    state: Pressed,
    repeat: true,
    platform_specific: KeyEventExtra {
        text_with_all_modifers: None,
        key_without_modifiers: Named(
            Shift,
        ),
    },
}
[age\src\os.rs:273:17] _modifiers = Modifiers {
    state: ModifiersState(
        0x0,
    ),
    pressed_mods: ModifiersKeys(
        0x0,
    ),
}
[age\src\os.rs:255:17] event = KeyEvent {
    physical_key: Code(
        ShiftRight,
    ),
    logical_key: Named(
        Shift,
    ),
    text: None,
    location: Right,
    state: Released,
    repeat: false,
    platform_specific: KeyEventExtra {
        text_with_all_modifers: None,
        key_without_modifiers: Named(
            Shift,
        ),
    },
}

If I were to try and visualise that, it would be something like this:

Left Shift Press----------------------------Real Left Shift Release-------------Observed Left Shift Release
----------------------Right Shift Press-----------------------------------------Right Shift Release

@kchibisov
Copy link
Member

@junglie85 no, I mean that you use my suggest and to detect the second release, you use the modifiers event, since it means that both shifts are released, and resetting with it instead of setting is more robust, so it's fine.

@junglie85
Copy link
Author

Oh, I see. I was ignoring the repeats but I actually need to account for those because that will continue to tell me that the "other" key is still pressed. I was ignoring those for reasons but I can rework that. Thanks :-)

@junglie85
Copy link
Author

@kchibisov Unfortunately that approach does not work, because neither a KeyboardInput nor a ModifiersChanged event is emitted. I've tried to visualise what the problem is so it's hopefully easier to understand:

Here, without applying the suggested modifier state:
Without Modifiers

And here with the suggestion applied:
With Modifiers

As can be seen, applying the modifier state correctly shows all keys as released. But it does not help with the missing Released event for the right shift key. That to me is a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - windows
Development

No branches or pull requests

2 participants