Skip to content

Windows: Fix Alt key press entering menu loop#2665

Merged
msiglreith merged 1 commit intorust-windowing:masterfrom
SamRodri:master
Feb 1, 2023
Merged

Windows: Fix Alt key press entering menu loop#2665
msiglreith merged 1 commit intorust-windowing:masterfrom
SamRodri:master

Conversation

@SamRodri
Copy link
Copy Markdown
Contributor

@SamRodri SamRodri commented Jan 31, 2023

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Fixes #2661 by detecting if the window has a native menu before delegating to the modal menu loop.

@SamRodri
Copy link
Copy Markdown
Contributor Author

I'm not sure about this fix, I never used the native menu thing and don't know how to create one to test it, the GetMenu does return NULL in my app.

Also I don't understand why Windows starts a menu loop when there is no menu, I'm actually assuming that's whats happening based on Stackoverflow.

@SamRodri
Copy link
Copy Markdown
Contributor Author

Ok, I tested with a menu created like this:

unsafe {
    let hwnd = window.hwnd();
    let menu = CreateMenu();
    AppendMenuA(menu, MF_STRING, 0, b"Test".as_ptr());
    SetMenu(hwnd, menu);
}

It seems to get the focus.

@SamRodri SamRodri marked this pull request as ready for review January 31, 2023 14:05
@SamRodri SamRodri requested a review from msiglreith as a code owner January 31, 2023 14:05
@madsmtm madsmtm added B - bug Dang, that shouldn't have happened DS - win32 Affects the Win32/Windows backend labels Jan 31, 2023
@kchibisov
Copy link
Copy Markdown
Member

r @amrbashir

@kchibisov kchibisov mentioned this pull request Jan 31, 2023
11 tasks
Copy link
Copy Markdown
Contributor

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite familiar with the keyboard handling, but I am fine with this change and can't see an immediate regression since winit used to always return 0 for this message.

I'd better wait for @msiglreith confirmation.

@msiglreith
Copy link
Copy Markdown
Member

Could you provide some more information or an example to reproduce? I can't reproduce the bug from the issue mentioned right now.

Also, please add some more comments in the code why this line was added.

@SamRodri
Copy link
Copy Markdown
Contributor Author

Modify the example window to only print relevant events:

#![allow(clippy::single_match)]

use simple_logger::SimpleLogger;
use winit::{
    event::{Event, WindowEvent},
    event_loop::EventLoop,
    window::WindowBuilder,
};

fn main() {
    SimpleLogger::new().init().unwrap();
    let event_loop = EventLoop::new();

    let window = WindowBuilder::new()
        .with_title("A fantastic window!")
        .with_inner_size(winit::dpi::LogicalSize::new(128.0, 128.0))
        .build(&event_loop)
        .unwrap();

    event_loop.run(move |event, _, control_flow| {
        control_flow.set_wait();

        match event {
            Event::WindowEvent { event, window_id } => match event {
                WindowEvent::CloseRequested if window_id == window.id() => control_flow.set_exit(),
                WindowEvent::KeyboardInput { input, .. } => println!("{:?}", input),
                _ => {}
            },
            Event::MainEventsCleared => {
                println!("Cleared");
                window.request_redraw();
            }
            _ => (),
        }
    });
}

Run it, it will not print "Cleared" after pressing and releasing LAlt.

Will add comments.

Copy link
Copy Markdown
Member

@msiglreith msiglreith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@msiglreith msiglreith merged commit 4e1c46f into rust-windowing:master Feb 1, 2023
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 - win32 Affects the Win32/Windows backend

Development

Successfully merging this pull request may close these issues.

Windows, alt key press hangs the event loop

5 participants