Thank you for your looking.

@Harley Acheson (harley), can you check this one?

This does something, but does not seem quite right or enough.

Following is a capture from current code that shows the sort of error described (I think). Two separated windows, Chinese input. Type "a" on the first and get one, type a single "a" on the second gets two.

This patch does seem to improve this on a simple quick check. As in I apply patch, do the above and it works right. However, it actually makes other very similar situations worse.

• Open blender with defaults.
• Enter IME text correctly in Properties.
• Right-click on Properties header, select "Duplicate Area into new window",
• Do not enter text there, just move the new window aside.
• Enter text in main window Properties

Without patch this works fine. With patch applied I get a duplicated character

My gut feeling is that this might involve cancelling IME when we lose focus, not just updating language and status, but not sure.

Hi.

- Open blender with defaults.
- Enter IME text correctly in Properties.
- Right-click on Properties header, select "Duplicate Area into new window",
- Do not enter text there, just move the new window aside.
- Enter text in main window Properties

I found that it's situation is caused by failing ImmGetContext() at GHOST_ImeWin32::UpdateConversionStatus() when a user do above steps.
I don't know why is it happen, though it is fixed by changing the error trap code to do nothing when happening THIS failing ImmGetContext() as below.
But I don't know this code making no affect to other codes yet.

void GHOST_ImeWin32::UpdateConversionStatus(HWND window_handle)
{
  HIMC imm_context = ::ImmGetContext(window_handle);
  if (imm_context) {
    if (::ImmGetOpenStatus(imm_context)) {
      ::ImmGetConversionStatus(imm_context, &conversion_modes_, &sentence_mode_);
    }
    else {
      conversion_modes_ = IME_CMODE_ALPHANUMERIC;
      sentence_mode_ = IME_SMODE_NONE;
    }
    ::ImmReleaseContext(window_handle, imm_context);
  }
-  else {
-    conversion_modes_ = IME_CMODE_ALPHANUMERIC;
-    sentence_mode_ = IME_SMODE_NONE;
-  }
}

In D12848#337304, @Takahiro Shizuki (sntulix) wrote:

• add GHOST_ImeWin32::SaveConversionStatus(hwnd).

I add separately it from UpdateConversionStatus(). Because I don't know whether newer IME has no bug.

reference:
KB5003690 - "Updates an issue that causes the Japanese Input Method Editor (IME) to suddenly stop working while you are typing. "

BTW:
KB4564002 - "When using the Microsoft IME for Japanese or Chinese, the first character typed into an element using DataGridGView in a Windows Forms (WinForms) app might be in the English character set, even if IMEmode was enabled. "
#Microsoft said "it resolved." at 2020-11-11 in this link.
#adding: This resolving was about DataGridView cell only (KB4571744).

I need some feedbacks. but are you on BCon in all time?

Hi.

I checked the a bug and fixed.
Watch please.

If you prefer this, commit please.

Best regards,
Takahiro

I'm not sure if I like this. We have a duplication of code just to avoid that UpdateConversionStatus else.

I've never liked that else as I'd rather leave things alone when in an unknown state rather than clear them. We already initialize those variables to those values on window creation, so there should be no good reason to set them to that when we are without imm_context. And when I breakpoint that else I only ever see the default values being assigned to those variables, and they already have those values.

So I'd rather we test with something like this instead:

I've never liked that else as I'd rather leave things alone when in an unknown state rather than clear them. We already initialize those variables to those values on window creation, so there should be no good reason to set them to that when we are without imm_context. And when I breakpoint that else I only ever see the default values being assigned to those variables, and they already have those values.

This else code is the Fail-safe as Exception handling when IME crashs with conversion_modes_ != IME_CMODE_ALPHANUMERIC. The crash makes me can't input any key in GHOST_ImeWin32::IsImeKeyEvent() by IsEnglishMode(). Because IsEnglishMode returns FALSE, but IME return no character codes.
I had been worried if I can't input save .blend name, but I don't worry it now. Because Blender's save window has default .blend name and current .blend name.

So,

So I'd rather we test with something like this instead:

IME.diff17 KBDownload

I see.

@Takahiro Shizuki (sntulix) - This else code is the Fail-safe as Exception handling when IME crashs with conversion_modes_ != IME_CMODE_ALPHANUMERIC. The crash makes me can't input any key in GHOST_ImeWin32::IsImeKeyEvent() by IsEnglishMode(). Because IsEnglishMode returns FALSE, but IME return no character codes.

Then it seems odd that is the only part that you omit in your proposed “SaveConversionStatus”, and such an exception isn’t happening in “WM_ACTIVATE”. Having two almost-identical functions doing largely the same things is confusing and is likely to lead to mistakes by those coming after us. If that “else” section has a clear purpose then you could make it an option with a bool argument on “UpdateConversionStatus” perhaps?

Ultimately we have to rely on you and your testing of this as I don’t use an IME language myself.

In D12848#344269, @Harley Acheson (harley) wrote:

Then it seems odd that is the only part that you omit in your proposed “SaveConversionStatus”, and such an exception isn’t happening in “WM_ACTIVATE”. Having two almost-identical
functions doing largely the same things is confusing and is likely to lead to mistakes by those coming after us.

SaveConversionStatus()'s purpose is: This function is not for avoiding IME Crushing. With my environment, ImmGetContext() always fails at WM_ACTIVATE when changing focus between windows without when creating new window by Blender's menu command. This happen causes a bug current IME mode of MS-Windows and GHOST_ImeWin32::conversion_modes_, you found by else of UpdateConversionStatus(). Because of those resluts, I think it is need a method getting ConversionStatus without else.

The reason becoming exists almost same two methods (Update and Save) : I want make no change to UpdateConversionStatus(). Because I don't know why ImmGetContext() fails when swiching between windows yet. (I don't like to change a work finished by an unknown thing.)

And, Why it is 'Save' named: I thought that UpdateConversionStatus() in WM_ACTIVATE is called at once as beginning that a user focusing to typing to the window activated (like a initializing IME). but UpdateConversionStatus() was created to update the status per changing IME on/off at previous differential (D11929). So I named it "SaveConversionStatus()" (meaning InitConversionStatus() ).

If that “else” section has a clear purpose then you could make it an option with a bool argument on “UpdateConversionStatus” perhaps?

If I change for an option you said, I think it may be below:

Ultimately we have to rely on you and your testing of this as I don’t use an IME language myself.

O.K.

GHOST_SystemWin32::s_wndProc()::WM_ACTIVATE

if (LOWORD(wParam) == WA_ACTIVE || LOWORD(wParam) == WA_CLICKACTIVE) {

added checking WA_CLICKACTIVE when switching between windows by mouse click.

Here’s a quick update that ImmGetContext() return 0 at WA_ACTIVE.
It is happen when switched to main window after a new window closed with IME off.

Hi, @Harley Acheson (harley)

I made a patch about this with minimum changes and no duplication codes.

How about this?

best regards.

@Takahiro Shizuki (sntulix) - I made a patch about this with minimum changes and no duplication codes.

Interesting!

I think this is functionally equivalent but with less changes and it might be easier to follow?

In D12848#346656, @Harley Acheson (harley) wrote:

@Takahiro Shizuki (sntulix) - I made a patch about this with minimum changes and no duplication codes.

Interesting!

I think this is functionally equivalent but with less changes and it might be easier to follow?

IMEDup.diff35 KBDownload

I am glad to hear it.

By the way, there is a sad news, it is found another bug. It is similar below:

• Open blender with defaults.
• Enter IME text correctly in Properties.
• Right-click on Properties header, select "Duplicate Area into new window",
• Do not enter text there, just move the new window aside.
• Enter text in main window Properties

the new bug:

- Open blender with defaults.
- Enter IME text correctly in Properties.
- Right-click on Properties header, select "Duplicate Area into new window",
- (In this time, previous window's IME conversion mode is not English.)
- Do not enter text there, '''but press IME off key at once''', and move the new window aside.
- Enter text in main window Properties, but key typing print no text.

It seems the problem is failing ImmGetContext(). Though I can't find what are status of conversion_modes_ and sentence_mode_ yet.
I will continue to check this at tomorrow.

Hi, @Harley Acheson (harley).

I fixed this :)

About WM_ACTIVATE, it has some problem to get IME context. I think it may be a timing that MS-Windows process some work before IME usable.
And I found WM_IME SETCONTEXT always succeeds to get IME context when switching window. So I changed to call UpdateConversionModes in WM_IME SETCONTEXT.
It succeeds:

• The case after pressing no key and switch from new window to previous window.
• The case switching to new window with IME On.
• The case backing to previous window after just pressing IME Off on new window.

Hi, @Harley Acheson (harley)

I finished.
Would you review, please?

About this differntal:
I seems this problem is happen by mismatch between Blender's IME status and REAL IME status when switching windows.

This approach:
When activating a window, the window shall update ime status.
For the timing, I use "WM_IME_SETCONTEXT" event message. This send to a window when the window is activated.
(reference: https://docs.microsoft.com/en-us/windows/win32/intl/ime-messages)

In D12848#350098, @Takahiro Shizuki (sntulix) wrote:

Hi, @Harley Acheson (harley)

I finished.
Would you review, please?

About this differntal:
I seems this problem is happen by mismatch between Blender's IME status and REAL IME status when switching windows.

This approach:
When activating a window, the window shall update ime status.
For the timing, I use "WM_IME_SETCONTEXT" event message. This send to a window when the window is activated.
(reference: https://docs.microsoft.com/en-us/windows/win32/intl/ime-messages)

D128480.diff24 KBDownload

This patch consists of just a single line change: calling UpdateConversionStatus in WM_IME_SETCONTEXT handler. This simple change appears to fix the problem and seems quite harmless, since this is only on the window gaining focus.

A proposal to rename "UpdateConversionStatus" to "UpdateConversionModes" would need to be a separate patch submission. Although not sure I would support that refactor. The function gets "modes" but it is a wrapper for "ImmGetConversionStatus" so I'd prefer the names to match as that makes it easier to understand for others who are familiar with the IME API.

I think you should reduce this patch to its minimum change, so just the addition of UpdateConversionStatus in WM_IME_SETCONTEXT and then update this document with it (top-right corner "Update Diff"), the we can have our usual testers to make sure everything still works in all languages. Probably safe to go in both 3.0 and 3.1

Hi, @Harley Acheson (harley)

A proposal to rename "UpdateConversionStatus" to "UpdateConversionModes" would need to be a separate patch submission. Although not sure I would support that refactor. The function gets "modes" but it is a wrapper for "ImmGetConversionStatus" so I'd prefer the names to match as that makes it easier to understand for others who are familiar with the IME API.

I see.

I think you should reduce this patch to its minimum change, so just the addition of UpdateConversionStatus in WM_IME_SETCONTEXT and then update this document with it (top-right corner "Update Diff"), the we can have our usual testers to make sure everything still works in all languages. Probably safe to go in both 3.0 and 3.1

I did. Thank you.

@Harley Acheson (harley)

This patch consists of just a single line change: calling UpdateConversionStatus in WM_IME_SETCONTEXT handler. This simple change appears to fix the problem and seems quite harmless, since this is only on the window gaining focus.

I think so.