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

Non-unicode IME support #3653

Open
wants to merge 6 commits into
base: docking
Choose a base branch
from
Open

Non-unicode IME support #3653

wants to merge 6 commits into from

Conversation

Othereum
Copy link

This PR adds IME support for non-unicode projects. Korean, Japanese, and Chinese are affected by this PR.

@ocornut
Copy link
Owner

ocornut commented Dec 14, 2020

Thank you @Othereum for this PR.

I have a question: what is the reason for testing #if HAS_WIN32_IME to process the WM_IME_CHAR message?
Could we just handle WM_IM_CHAR in every builds? Does it needs to be filtered by !UNICODE as well?

@Othereum
Copy link
Author

Well, maybe HAS_WIN32_IME doesn't need to be checked. However, UNICODE must be checked.

https://docs.microsoft.com/en-us/windows/win32/intl/wm-ime-char

As you can see here, if the project uses unicode character set, wParam has a unicode encoded value. Therefore, you just need to add the input immediately without any further action.

@ocornut
Copy link
Owner

ocornut commented Dec 14, 2020

Well, maybe HAS_WIN32_IME doesn't need to be checked. However, UNICODE must be checked.

Then I suggest you then remove that check and move the other function was it was previously to minimize the patch?

About unicode checking, I have also been used to test the UNICODE define but I wonder if we should simply just called IsWindowUnicode() to decide whether the process the message.

@Othereum
Copy link
Author

Oh, you're right. Then why don't we merge it into master branch instead of docking branch? Should I make a new PR?

@ocornut
Copy link
Owner

ocornut commented Dec 14, 2020 via email

@ocornut ocornut added the ime label Dec 14, 2020
@Othereum
Copy link
Author

Othereum commented Dec 14, 2020

I retested it when I got home and it didn't work properly when I enabled UTF-8 support in the system locale settings. So I fixed it using the ImmGetCompositionStringW() function.
Therefore, I think that the HAS_WIN32_IME check should be retained, and this PR should be merged into the docking branch.

@Othereum
Copy link
Author

I'm still waiting for your review, @ocornut :D

@ocornut
Copy link
Owner

ocornut commented Dec 15, 2020

Hello,
I'll wait until after 1.80 to test and review this thoroughly. Thank you for your patience!

@learn-more
Copy link
Contributor

Well, maybe HAS_WIN32_IME doesn't need to be checked. However, UNICODE must be checked.

Then I suggest you then remove that check and move the other function was it was previously to minimize the patch?

About unicode checking, I have also been used to test the UNICODE define but I wonder if we should simply just called IsWindowUnicode() to decide whether the process the message.

fyi:
The result of IsWindowUnicode can change during the lifetime of a window.
Initially it's determined by how the class is registered (RegisterClassExA or RegisterClassExW),
but it can be changed by calling SetWindowLongPtrW(hwnd, GWLP_WNDPROC,..) to turn it into unicode, or SetWindowLongPtrA(hwnd, GWLP_WNDPROC,..) to turn it back to ANSI.

@ocornut
Copy link
Owner

ocornut commented Jan 12, 2022

@learn-more

The result of IsWindowUnicode can change during the lifetime of a window.

Is that something that realistically happen?

@ocornut
Copy link
Owner

ocornut commented Jan 12, 2022

@Othereum I wanted to look at this today but I am confused.
On the example_win32_dx11 project if I change back "Character" to "Use Multi-Byte Character Set" everything seems to work without this patch.
Can you clarify what are the steps to reproduce the issue prior to applying your patch?

I verified that my project compiles with:

#ifdef UNICODE
#error
#endif
#ifdef _UNICODE
#error
#endif

Aka neither are defined.

@learn-more
Copy link
Contributor

@learn-more

The result of IsWindowUnicode can change during the lifetime of a window.

Is that something that realistically happen?

I am in the middle of moving, so don't have access to my notes, but if I recall correctly this is something that can happen when a window is subclassed, or flags of the window are altered.

ocornut added a commit that referenced this pull request Jan 15, 2024
…87, favor of writing to 'void* ImGuiViewport::PlatformHandleRaw'.

Amend 3a90dc3 (#2589, #2598, #3108, #3113, #3653, #4642)
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.

3 participants