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: mouse and keyboard #8791

Merged
merged 13 commits into from
Mar 5, 2024

Conversation

zaucy
Copy link
Contributor

@zaucy zaucy commented Mar 3, 2024

Windows mouse and keyboard working! I also tweaked the message loop so that it didn't get stuck. The peek message loop was almost never returning for me during testing.

Release Notes:

  • Added windows mouse and keyboard support
    windows-mouse-and-keyboard

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 3, 2024
@zaucy zaucy force-pushed the windows-mouse-and-keyboard branch from 8e20d15 to 5386e34 Compare March 3, 2024 23:57
@zaucy zaucy mentioned this pull request Mar 4, 2024
crates/gpui/src/platform/windows/platform.rs Show resolved Hide resolved
crates/gpui/src/platform/windows/window.rs Outdated Show resolved Hide resolved
crates/gpui/src/platform/windows/window.rs Outdated Show resolved Hide resolved
crates/gpui/src/platform/windows/window.rs Show resolved Hide resolved
crates/gpui/src/platform/windows/window.rs Outdated Show resolved Hide resolved
@zaucy zaucy requested a review from kvark March 4, 2024 08:00
@zaucy zaucy mentioned this pull request Mar 4, 2024
@zaucy zaucy requested a review from kazatsuyu March 4, 2024 08:24
crates/gpui/src/platform/windows/window.rs Outdated Show resolved Hide resolved
let mut callbacks = self.callbacks.borrow_mut();
if let Some(callback) = callbacks.input.as_mut() {
let modifiers = self.current_modifiers();
let msg_char = wparam.0 as u8 as char;
Copy link
Contributor

Choose a reason for hiding this comment

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

https://learn.microsoft.com/en-us/windows/win32/inputdev/wm-char
Since WM_CHAR accepts UTF-16 code units, this conversion can cause problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree this is an issue, but I think it should maybe be covered in another PR. I only say this because the char encoding situation is a little wack on windows. Recently they started emphasizing UTF-8 with the RegisterClassA call even tho previously they said new applications should use RegisterClassW.

https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

Until recently, Windows has emphasized "Unicode" -W variants over -A APIs. However, recent releases have used the ANSI code page and -A APIs as a means to introduce UTF-8 support to apps. If the ANSI code page is configured for UTF-8, then -A APIs typically operate in UTF-8. This model has the benefit of supporting existing code built with -A APIs without any code changes.

That being said I could change this to what this PR does if this is a blocker: https://github.com/zed-industries/zed/pull/8809/files#diff-f5cac42313d3717a63ced25b77b50370494328242025c977fa77d17d5ad1bb40R192-R193

let Ok(first_char) = char::decode_utf16(src).collect::<Vec<_>>()[0] else {
    return None;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see. Then we should use -A APIs

@zaucy zaucy requested a review from kazatsuyu March 4, 2024 17:11
@mikayla-maki mikayla-maki merged commit 36c4831 into zed-industries:main Mar 5, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants