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

Fix Ascii char conversion for different keyboard layouts. #6785

Closed

Conversation

sneakyevil
Copy link
Contributor

This is upon implementation of the code that I posted and I had issue at #6782.
The generic CP_ACP does seem to be prefering the English Layout despite there is different keyboard layout selected.
I implemented member inside ImGui_ImplWin32_Data that holds KeyboardCodePage and function ImGui_ImplWin32_UpdateKeyboardCodePage that updates the keyboard codepage when needed, the update function is called only at initilization and WndProc msg WM_INPUTLANGCHANGE.

@ocornut
Copy link
Owner

ocornut commented Sep 6, 2023

Thanks for the PR.

Would passing CP_THREAD_ACP to MultiByteToWideChar() give you the right output?

@sneakyevil
Copy link
Contributor Author

I tried before using CP_THREAD_ACP and it does work pretty weird.
Launching program from VS Debugger will work properly as should, but launching the program from explorer/x64dbg breaks same way as if I was using CP_ACP.

@ocornut
Copy link
Owner

ocornut commented Sep 7, 2023

Are you certain you were launching the right compiled executable? (Sorry for the silly question, but i know it does happen when switching tools like that)

@sneakyevil
Copy link
Contributor Author

There isn't any other executable to launch since I've got only one build that is used for testing rendering stuff.
Here is video of using the CP_THREAD_ACP first and then using the Keyboard Code Page while launching the program from VS Debugger, explorer and x32dbg. https://streamable.com/pgvjl7

@sneakyevil
Copy link
Contributor Author

Alright after some more testing I find out that there is this windows settings and changing the system locale does make the CP_ACP and CP_THREAD_ACP work properly as should, but is this actual solution to rely on "advanced" system locale settings than actually getting current code page of selected keyboard layout?
image

@ocornut
Copy link
Owner

ocornut commented Sep 7, 2023

I agree that requesting users to change Windows settings is not desirable.

I think your PR is probably good but it's really hard to find good documentation on this and I have an intuition there should be simpler solution out there so I am fishing for it. I've also seen forums implying LOWORD of GetKeyboardLayout() should be used rather than HIWORD which makes it more confusing.

@sneakyevil
Copy link
Contributor Author

GetKeyboardLayout for me returns as HIWORD is Slovak Language while LOWORD is like child language which is English Language. As displayed in windows 10 language selection:
image

There is documentation on the indentifiers on:
https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/windows-language-pack-default-values?view=windows-11

Value of GetKeyboardLayout for me returns as 0x041B0409, using the 0x041B (HIWORD) matches the https://learn.microsoft.com/en-us/globalization/keyboards/kbdsl

@ocornut
Copy link
Owner

ocornut commented Sep 7, 2023

I finished setting myself up with the same settings to confirm your findings (confirmed) and trying to find alternatives (not found yet).

What strikes me as odd is that I find so few references to similar code, whereas it would seem like NECESSARY code that many applications would use. Surely if that code is necessary it should be ultra common?

I can't imagine it would be only because "everyone" is using UNICODE builds?
If I google for "MultiByteToWideChar WM_CHAR" results 2 and 3 are from imgui github, which makes me even more suspicious that we are doing the right thing.

ocornut pushed a commit that referenced this pull request Sep 7, 2023
…ng in MBCS mode and creating a non-Unicode window. (#6785, #6782, #5725)
@ocornut
Copy link
Owner

ocornut commented Sep 7, 2023

I have pushed your commit as c9d3c29 (with cosmetic modifications).

I think there's something we might be missing. I can confirm same result as you but can't find other people doing the same thing as us. I wonder if it's because everyone switched to Unicode windows, where none of this matters. Of I wonder if also few people are using your setup (you are setup for English with Slovak keyboard, not Slovak with Slovak Keyboard).

There's no easy way to compare as e.g. SDL/GLFW have creating Unicode windows for a while.

I suppose pushing the change is the best way to find out,!

Thanks for your work.

@sneakyevil
Copy link
Contributor Author

I think there isn't anything we would be missing. If people mainly use unicode windows then it shouldn't even matter much. It rather gives better codepage handling for ascii windows. I sent the codepage code with GetKeyboardLayout & GetLocaleInfoA to few friends and it worked fine for Spanish & Chinese keyboard layout, even if it failed it would still fallback to the original CP_ACP just in case. Also the part with English and (other language) keyboard is probably more common since Windows 10. I before didn't have any issues with the CP_ACP when using ImGui under Windows 8.1 and I just got this issue right now on Windows 10.

@ocornut
Copy link
Owner

ocornut commented Sep 8, 2023

Also the part with English and (other language) keyboard is probably more common since Windows 10. I before didn't have any issues with the CP_ACP when using ImGui under Windows 8.1 and I just got this issue right now on Windows 10.

Perhaps, seems like this dual language vs layout setup may be new and in this era I would understand everyone's on Unicode Windows. But this would mean many old apps would break inputs on Windows 10 ?

@sneakyevil
Copy link
Contributor Author

They do indeed break inputs. If you search google for dummy things like fix input for non-unicode apps on windows there are results that show the administrative settings that I mention before. I tried to look at old src of notepad++ from 2011 if there was something similar to my code, but there wasn't anything to it not even use of the GetLocaleInfo api and the only thing I was thinking that it was builded as Unicode since there were ifdef for ANSI build. The only thing I found with GetLocaleInfo usage in notepad++ was at their github at this page https://github.com/notepad-plus-plus/notepad-plus-plus/blob/5008b8a0cccfff255c5f48b5782ef993b6f9b631/boostregex/boost/regex/v5/w32_regex_traits.hpp#L818 which seems like boost thing rather than notepad and it even looks worse than my implementation.

ocornut pushed a commit that referenced this pull request Dec 29, 2023
… input for text in utf-8 code page. (#7174)

Similar to #6785, #6782, #5725, #5961 for for GLFW backend.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants