Details

Reviewers

Brecht Van Lommel (brecht)
Harley Acheson (harley)

Maniphest Tasks

T89831: When IME is On, passing key inputs excluding caret operation to IME. (Windows)

Commits

rB836aeebf7077: IME Win32: Fix Duplicated Initial Character

Summary

When entering characters using IME on Windows, Japanese and Chinese will both result in the first keystroke being duplicated. You end up with an extra unwanted character. Following shows entering "Hello" and getting an extra "n" in front:

The central problem to fix is that we get WM_IME_STARTCOMPOSITION event too late, after the first character is entered. In processKeyEvent we could just ignore all keys when IME is on. That would fix this problem with duplicated initial key, but would not allow editing like backspace, arrows, deleting, etc. And we also have to deal with the issue of entering English characters too.

This patches add support for getting the conversion status flags and modes using ImmGetConversionStatus. With these we can determine if we are composing, if in English mode, and other details. Then we can ignore or pass along different keypresses, depending on the language.

Tested in Chinese, Japanese. and Korean with a variety of input editors and methods.

Diff Detail

Repository: rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Hi.

Thank you for your works.
I will reply later please.

I found that I get the current input mode of IME by ImmGetConversionStatus().
I think it may resolves that printing no letter with some modes of IME.
So I checked it. Its are below, because of a long post.
https://developer.blender.org/T89831#1192412

Harley Acheson (harley) removed a reviewer: Brecht Van Lommel (brecht).Jul 17 2021, 4:04 PM

Hi.

@Harley Acheson (harley)

When you created this (updated) version of the patch you probably did something like git diff > filename.diff but I'm hoping you can (next time) add more

OK.

@Yuki Hashimoto (hzuika)

Hi.

Thank you for your work.
I understand its.

I put a patch (not test yet) added checking the input is direct or handling IME to haley's diff.

I will test it for the problems remained.

I updated the patch, removed the diff at this post.

Hi.

@Harley Acheson (harley)

I changed below

+ (raw.data.keyboard.VKey >= 'A') && (raw.data.keyboard.VKey <= 'Z') &&

to (adding parenthesis)

+ ((raw.data.keyboard.VKey >= 'A') && (raw.data.keyboard.VKey <= 'Z')) &&

Is it OK?

@Yuki Hashimoto (hzuika)

This is a patch that resolve:

I could not input alphabet (a to z and A to Z) when English mode (IME OFF) with Chinese input.

IMEChar_and_Mode-UTC202107190655.diff25 KBDownload

Hi.

I updated the patch.

IMEChar_and_Mode-UTC202107201325.diff40 KBDownload

This patch are below:

Common
- Changing
  - added GHOST_ImeWin32::ime_conversion_status_ property.
  - added GHOST_ImeWin32::UpdateConversionStatus().
  - added GHOST_ImeWin32::is_conversion_mode().
  - added GHOST_ImeWin32::GetInputLanguage().
  - The timing checking IME input mode: WM_INPUT to WM_INPUTLANGCHANGE or WM_IME_NOTIFY.
- problem
  - Ctrl-a is not functioned with Japanese, Chinese and Korean in IME is on.
Japanese
- resolved
  - duplicated first letter in roma-ji.
  - duplicated 0-9 letter
  - duplicated symbol letter
Chinese (with Screen Keyboard)
- resolved
  - duplicated first letter.
  - duplicated 0-9 letter
- problem
  - duplicated symbol letter
Korean (with Screen Keyboard)
- resolved
  - duplicated first letter.
  - duplicated 0-9 letter
- problem
  - duplicated symbol letter

Could you test or review it please? @Harley Acheson (harley) @Yuki Hashimoto (hzuika)

@Takahiro Shizuki (sntulix)：

The only part that looks a little odd is that your ime_conversion_status_ is only ever set to true if conversions & (IME_CMODE_NATIVE|IME_CMODE_FULLSHAPE) and all the other tests seem unused and unnecessary. It is worth checking to make sure there is not an error there, for example since IME_CMODE_NOCONVERSION is never actually used.

It is also getting a bit more complex than I like. The following should be equivalent but with much complexity removed. it removes ime_status_ since that is no longer used. It also removes SetInputLanguage() and input_language_id_ and instead just reads the current input language directly when needed. Similarly is_conversion_mode just gets this state immediately so removes the need for UpdateConversionStatus and ime_conversion_status_

Of course I could have made some mistake though.

IME11.diff43 KBDownload

I fixed some of the patch by @Harley Acheson (harley)
This patch probably solves current IME problem without causing new problem.

IME_20210721_hzuika.diff51 KBDownload

is_conversion_mode is still written in this patch, but it is probably not needed.

I investigated current IME problem and summarized it.
This information may be too long and a bit redundant.

Problem

The current problem is caused by new Microsoft IME sending WM_IME_STARTCOMPOSITION messages later than the old IME. Blender will remove the GHOST_kEventKeyDown event when WM_IME_STARTCOMPOSITION is sent. However, with the new IME, when the slower WM_IME_STARTCOMPOSITION is sent, the GHOST_kEventKeyDown event is already finished. Therefore, the two events (GHOST_kEventKeyDown and GHOST_kEventImeComposition) are processed and the initial character is duplicated.

Previous IME

Press a key with Japanese IME
↓
WM_INPUT
Add GHOST_kEventKeyDown Event
↓
WM_IME_STARTCOMPOSITION
Remove GHOST_kEventKeyDown Event
↓
WM_IME_COMPOSITION
Add GHOST_kEventImeComposition Event
↓
Process GHOST_kEventImeComposition Event あ

Result string あ

New IME (Duplicated Initial character)

Press a key with Japanese IME
↓
WM_INPUT
Add GHOST_kEventKeyDown Event
↓
Process GHOST_kEventKeyDown Event a
↓
WM_IME_STARTCOMPOSITION
(Remove GHOST_kEventKeyDown Event)
↓
WM_IME_COMPOSITION
Add GHOST_kEventImeComposition Event
↓
Process GHOST_kEventImeComposition Event あ

Result string aあ

Current Approach

The current approach is to add GHOST_kEventKeyDown events only for characters that are not processed by IME.

Press any key when IME turns on
↓
WM_INPUT
The character is processed by IME? →No Add GHOST_kEventKeyDown Event
↓Yes
WM_IME_STARTCOMPOSITION
(Remove GHOST_kEventKeyDown Event)
↓
WM_IME_COMPOSITION
Add GHOST_kEventImeComposition Event
↓
Process GHOST_kEventImeComposition Event

How to check?

All alphabet characters are processed by IME.

All Japanese symbolic characters are also processed by IME.
All Korean symbolic characters are not processed by IME.

In the case of Chinese symbolic characters, it is difficult to check the character is processed by IME.

Not processed by Chinese IME

#%&*+-/0123456789=@{|}~

Processed by Chinese IME

!"$'(),.:;<>?[\]^_`
↓ converted
！“￥‘（），。：；《》？【、】……——·

A simple/stupid solution is to write down all the cases.

Note

I tested them using Microsoft IME. This check method may not apply to other third party IMEs.

is_conversion_mode is still written in this patch, but it is probably not needed.

I made a mistake. For Chinese IME, is_conversion_mode is required.

In D11929#308878, @Yuki Hashimoto (hzuika) wrote:

IME_20210721_hzuika.diff51 KBDownload

P.S.

Switch case statement is equivalent to the following if statement using strchr.

if(strchr("!\"$'(),.:;<>?[\\]^_", utf8_char[0]))

The following process is no longer needed in WM_IME_STARTCOMPOSITION.

eventManager->removeTypeEvents(GHOST_kEventKeyDown, window);

Hi @Harley Acheson (harley).

The only part that looks a little odd is that your ime_conversion_status_ is only ever set to true if conversions & (IME_CMODE_NATIVE|IME_CMODE_FULLSHAPE) and all the other tests seem unused and unnecessary. It is worth checking to make sure there is not an error there, for example since IME_CMODE_NOCONVERSION is never actually used.

The checking conversion status is supporting for Chinese IME's English mode, its necessary.
IME_CMODE_NOCONVERSION is necessary for ATOK 2017 (Japanese IME, non Microsoft.). It returns conversion status : *_NATIVE=1, *_FULLSHAPE:8, *_ROMAN:16, *_NOCONVERSION:256).
This may be a bug. But It is a domestic production. I think domestic production is culture, so I support this.

It is also getting a bit more complex than I like. The following should be equivalent but with much complexity removed. it removes ime_status_ since that is no longer used. It also removes SetInputLanguage() and input_language_id_ and instead just reads the current input language directly when needed. Similarly is_conversion_mode just gets this state immediately so removes the need for UpdateConversionStatus and ime_conversion_status_

It seems affect for more cpu clocks consumption. but I feel important fixed the bug at first.

Of course I could have made some mistake though.

Thank you for taking your precious time.

Hi @Yuki Hashimoto (hzuika).

In D11929#308895, @Yuki Hashimoto (hzuika) wrote:
In D11929#308878, @Yuki Hashimoto (hzuika) wrote:

IME_20210721_hzuika.diff51 KBDownload

P.S.

Switch case statement is equivalent to the following if statement using strchr.
if(strchr("!\"$'(),.:;<>?[\\]^_", utf8_char[0]))
The following process is no longer needed in WM_IME_STARTCOMPOSITION.
eventManager->removeTypeEvents(GHOST_kEventKeyDown, window);

I tested it. It works perfect, maybe.
The detail of tests are below.:

japanese:
- latest IME:
  - ctrl-a : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - hiragana mode
    - hiragana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. After typing '=', it stucks(?).
  - fullwidth katakana mode
    - katakana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. After typing '=', it stucks(?).
  - fullwidth alphabet (ａ-ｚＡ-Ｚ) mode
    - alphabet
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. After typing '=', it stucks(?).
  - halfwidth katakana mode
    - katakana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. After typing '=', it stucks(?).
  - halfwidth alphabet (a-zA-Z) mode # direct input
    - alphabet
      - input : ok
        
        no dupulicated : ok
      - symbolic
        
        input : ok
        
        no dupulicated : ok
      - 0-9
        
        input : ok
        
        no dupulicated : ok
      - numpad 0-9 and symbolic: ok
- previous IME:
  - ctrl-a : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - hiragana mode
    - hiragana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.
  - fullwidth katakana mode
    - katakana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.
  - fullwidth alphabet (ａ-ｚＡ-Ｚ) mode
    - alphabet
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.
  - halfwidth katakana mode
    - katakana
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.
  - halfwidth alphabet (a-zA-Z) mode # direct input
    - alphabet
      - input : ok
        
        no dupulicated : ok
      - symbolic
        
        input : ok
        
        no dupulicated : ok
      - 0-9
        
        input : ok
        
        no dupulicated : ok
      - numpad 0-9 and symbolic:
        
        0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

english
- latest IME :
  - ctrl-a : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - alphabet (a-zA-Z) mode
    - alphabet
      - input : ok
        
        no dupulicated : ok
      - symbolic
        
        input : ok
        
        no dupulicated : ok
      - 0-9
        
        input : ok
        
        no dupulicated : ok
      - numpad 0-9 and symbolic:
        
        0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

chinese
- latest IME:
  - ctrl-a : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - chinese mode
    - chinese
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print '=61'. # usb numpad
  - alphabet (a-zA-Z) mode
    - alphabet
      - input : ok
        
        no dupulicated : ok
      - symbolic
        
        input : ok
        
        no dupulicated : ok
      - 0-9
        
        input : ok
        
        no dupulicated : ok
      - numpad 0-9 and symbolic:
        
        0-9 and symbolic: ok, excluding '='. Typing '=' key print '=61'. # usb numpad

korean
- latest IME:
  - ctrl-a : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - korean mode
    - chinese
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - numpad 0-9 and symbolic:
      - 0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.
  - alphabet (a-zA-Z) mode
    - alphabet
      - input : ok
        
        no dupulicated : ok
      - symbolic
        
        input : ok
        
        no dupulicated : ok
      - 0-9
        
        input : ok
        
        no dupulicated : ok
      - numpad 0-9 and symbolic:
        
        0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

note
- Testing with hardware keyboard for japanese. Numpad is usb.
- I (sntulix) havn't understood Chinese and Korean and how to writing with it (the grammer), my checking is just typing.
- ATOK 2017 (ime for japanese)
  - halfwidth alphabet mode # conversioned, not direct input.
    - input : ng
      - note: Conversion Status of this mode: *_NATIVE=1, *_FULLSHAPE:8, *_ROMAN:16, *_NOCONVERSION:256). maybe, This is a bug of ATOK 2017. When if support this, check *_NOCONVERSION at first than other status in GHOST_ImeWin32::is_conversion_mode().

@Takahiro Shizuki (sntulix) Thank you for testing.

In D11929#309020, @Takahiro Shizuki (sntulix) wrote:

numpad 0-9 and symbolic:

0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

ATOK 2017 (ime for japanese)

halfwidth alphabet mode # conversioned, not direct input.

input : ng

note: Conversion Status of this mode: *_NATIVE=1, *_FULLSHAPE:8, *_ROMAN:16, *_NOCONVERSION:256). maybe, This is a bug of ATOK 2017. When if support this, check *_NOCONVERSION at first than other status in GHOST_ImeWin32::is_conversion_mode().

This bug is interesting. Unfortunately, I don't have a USB Numpad and ATOK IME, so I can not investigate these bugs.

If it was working correctly before you applied this patch, this patch is the cause of the bug. We need to investigate the cause and fix the patch.
However, if you had the same bug before applying the patch, it is outside the scope of the patch. This is a separate problem that needs to be solved.

The following has a few changes:

is_open() not actually needed. It is only called once and is only the result of a single function call.
using ascii instead of utf8_char[0]
using vk instead of utf8_char[0]
using strchr() instead of switch

IME12.diff44 KBDownload

Something I am unclear about: What is the difference between is_composing() and is_conversion_mode() ? Can we drop the former in with the latter? If not can one of the identifiers change to something more obvious？

In D11929#309083, @Harley Acheson (harley) wrote:

using vk instead of utf8_char[0]

Using vk is looks nice, but I chose utf8_char[0] (or ascii) for the following reason.

If using vk instead of utf8_char[0], additional check whether Ctrl key is pressed (ctrl_pressed) is required for use of shortcut keys. Because, vk check cannot distinguish between input keys (such as A or C) and shortcut keys (such as Ctrl + A (select all) or Ctrl + C (copy)).

if using utf8_char[0] (or ascii), Ctrl key check is already done in previous line, so additional ctrl_pressed check is not required.

if (ctrl_pressed && !alt_pressed) {
  utf8_char[0] = '\0';
}

@Yuki Hashimoto (hzuika) - vk check cannot distinguish between input keys (such as A or C) and shortcut keys (such as Ctrl + A (select all) or Ctrl + C (copy)).

Yes, you are right.

if using utf8_char[0] (or ascii)...

I'd prefer to use ascii between these two.

Note also that my use of strchr as in the patch above would have ALSO broken ctrl-c and ctrl-v. It needs a null check as done here:

IME13.diff44 KBDownload

In D11929#309115, @Harley Acheson (harley) wrote:

Note also that my use of strchr as in the patch above would have ALSO broken ctrl-c and ctrl-v. It needs a null check as done here:

I overlooked it. Thanks for fixing.

In D11929#309083, @Harley Acheson (harley) wrote:

Something I am unclear about: What is the difference between is_composing() and is_conversion_mode() ?

I will try to explain them.
is_conversion_mode() checks the IME's two main modes (English mode (direct input mode) and Native language mode (conversion mode)).
In English mode, GHOST_kEventKeyDown event is created, while In Native language mode, the event is not created to avoid a duplicated character.
For checking two modes, ImmGetOpenStatus was insufficient. When using ImmGetOpenStatus,

Japanese and Korean IME return false in English mode and true in Native language mode.
However, Chinese IME returns true in English mode and true in Native language mode.

Therefore, current approach check IME's mode using ImmGetConversionStatus for Chinese IME. In addition, some of the third party IME also needs to be checked by ImmGetConversionStatus.

is_composing represents IME's composing state.
When any key is pressed in Native language mode, IME start composing a text. is_composing is true until the text is confirmed and IME end composing.

Can we drop the former in with the latter? If not can one of the identifiers change to something more obvious？

No, is_conversion_mode and is_composing are different.
For example, is_conversion_mode changes to is_native_language_mode or is_enable, is_open etc... Which one is more obvious?

@Harley Acheson (harley): Do you have any question?
@Takahiro Shizuki (sntulix) : Is my understanding you correctly?

@Yuki Hashimoto (hzuika) - is_conversion_mode() checks the IME's two main modes (English mode (direct input mode) and Native language mode (conversion mode)).

Oh, that makes sense. Can we reverse this logic and call it is_english_mode instead? That might be clearer to other developers here.

In D11929#309135, @Harley Acheson (harley) wrote:

Oh, that makes sense. Can we reverse this logic and call it is_english_mode instead? That might be clearer to other developers here.

I use is_english_mode instead of is_conversion_mode and reversed the logic.

IME13_hzuika.diff45 KBDownload

IME13_hzuika_lf.diff44 KBDownload

The diff I just sent had a CRLF line break character, so I can not apply.
I changed to LF.

In D11929#309135, @Harley Acheson (harley) wrote:

@Yuki Hashimoto (hzuika) - is_conversion_mode() checks the IME's two main modes (English mode (direct input mode) and Native language mode (conversion mode)).

Oh, that makes sense. Can we reverse this logic and call it is_english_mode instead? That might be clearer to other developers here.

How about is_off() if it make the function name more suitable?

@Yuki Hashimoto (hzuika)

In D11929#309020, @Takahiro Shizuki (sntulix) wrote:

numpad 0-9 and symbolic:

0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

This is with Blender 3.0.0 Alpha downloding from official site. My USB Numpad's '=' key print '=' or '6' or '1'. (print '=61=6=61...' by pressing the key repeatedly.)
I see it is a other problem to this problem.

ATOK 2017 (ime for japanese)

halfwidth alphabet mode # conversioned, not direct input.

input : ng

note: Conversion Status of this mode: *_NATIVE=1, *_FULLSHAPE:8, *_ROMAN:16, *_NOCONVERSION:256). maybe, This is a bug of ATOK 2017. When if support this, check *_NOCONVERSION at first than other status in GHOST_ImeWin32::is_conversion_mode().

IME_CMODE_NOCONVERSION has not been checked from IME11.diff.
The check was:

+  ::ImmGetConversionStatus(imm_context, &conversions, &sentenses);
+  ::ImmReleaseContext(window_handle, imm_context);
+  if ((IME_CMODE_NATIVE & conversions) | (IME_CMODE_FULLSHAPE & conversions)) {
+    this->ime_conversion_status_ = true; /* conversion input */
+#ifdef DEBUG
+    printf("conversion on\n");
+#  endif
+  } else if (IME_CMODE_NOCONVERSION & conversions) {
+    this->ime_conversion_status_ = false; /* no conversion input */
+#ifdef DEBUG
+    printf("conversion off\n");
+#endif
+  } else {
+    this->ime_conversion_status_ = false; /* no conversion input */
+#ifdef DEBUG
+    printf("conversion off\n");
+#endif

Please check IME_CMODE_NOCONVERSION too.

Note: The status of ATOK 2017's hankaku mode is below:
https://dev-files.blender.org/file/data/aurx3ecickrsj3r2p4h3/PHID-FILE-wwxdt2minxigko7oxlyg/win10-atok-japanese-hankaku-print_no_letter.PNG

In D11929#309020, @Takahiro Shizuki (sntulix) wrote:

numpad 0-9 and symbolic:

0-9 and symbolic: ok, excluding '='. Typing '=' key print no letter.

This is with Blender 3.0.0 Alpha downloding from official site. My USB Numpad's '=' key print '=' or '6' or '1'. (print '=61=6=61...' by pressing the key repeatedly.)
I see it is a other problem to this problem.

This input mode is off with Japanese IME (latest).

Hi @Harley Acheson (harley).

In D11929#309016, @Takahiro Shizuki (sntulix) wrote:

Hi @Harley Acheson (harley).

It is also getting a bit more complex than I like. The following should be equivalent but with much complexity removed. it removes ime_status_ since that is no longer used. It also removes SetInputLanguage() and input_language_id_ and instead just reads the current input language directly when needed. Similarly is_conversion_mode just gets this state immediately so removes the need for UpdateConversionStatus and ime_conversion_status_

It seems affect for more cpu clocks consumption. but I feel important fixed the bug at first.

I think getting the status of IME is a bit long method. To doing it per every typing a key take much costs of cpu clocks. So I worry it makes late to user's feeling of inputs.
This is why I want to call ImmGetConversionStatus() at WM_IME_NOTIFY event instead of WM_INPUT event.

This is why I want to call ImmGetConversionStatus() at WM_IME_NOTIFY event instead of WM_INPUT event.

WM_IME_NOTIFY message is sent when IME is changed from windows to GHOST_SystemWin32::s_wndProc().
This message has params IME's changing status.

IMN_SETOPENSTATUS : IME on/off changed.
IMN_SETCONVERSIONMODE : IME input mode changed.
others

(https://docs.microsoft.com/en-us/windows/win32/intl/wm-ime-notify)

So it could limit cosumption of cpu clocks.

Okay I think the following is the minimal source changes to do this:

IME14.diff37 KBDownload

Back to caching language
Caches ImmGetConversionStatus, saving conversion modes and sentence modes
Handles IME_CMODE_NOCONVERSION

Whoops, my mistake. I'll remove ime_status in a cleanup pass later. This patch should only contain the minimum changes neccessary, to make it easier to approve:

IME15.diff36 KBDownload

@Takahiro Shizuki (sntulix) If this looks okay could you update this diff with this version?

@Harley Acheson (harley)

Than you for your update.

Caches ImmGetConversionStatus, saving conversion modes and sentence modes

I looked it. But it is different a little with I imaged about saving and using of cache.

 GHOST_ImeWin32::GHOST_ImeWin32()
     : is_composing_(false),
       ime_status_(false),
       input_language_id_(LANG_USER_DEFAULT),
+      conversion_modes_(IME_CMODE_ALPHANUMERIC),
+      sentence_mode_(IME_SMODE_NONE),
       system_caret_(false),
       caret_rect_(-1, -1, 0, 0),
       is_first(true),
       is_enable(true)
 {

Why you made GHOST_ImeWin32 class to has conversion_modes_ and sentence_mode_?
I have imaged "bool GHOST_ImeWin32::is_english_mode" property.
It is same style with "GetInputLanguage()" (and how to use input_language_id_) when processKeyEvent().
If the IME on | off status is "is_english_mode" property like input_language_id_, it has no necessary calling ImmGet*(), ImmReleaseContext() per every key typing (WM_INPUT), only calling when WM_IME_NOTIFY.
In the case of this property, UpdateConversionStatus() , not in is_english_mode() being checks IME conversion status and set to this property with bool.
If you ok with "is_english_mode" property, I will update the patch.

+bool GHOST_ImeWin32::is_english_mode(HWND window_handle)
+{
+  bool is_open = false;
+  HIMC imm_context = ::ImmGetContext(window_handle);
+  if (imm_context) {
+    is_open = ::ImmGetOpenStatus(imm_context);
+    ::ImmReleaseContext(window_handle, imm_context);
+  }
+
+  return (!is_open || conversion_modes_ & IME_CMODE_NOCONVERSION) ||
+         !(conversion_modes_ & (IME_CMODE_NATIVE | IME_CMODE_FULLSHAPE));
+}

It is ok that checking about IME_CMODE_NOCONVERSION (not tested yet) .

@Takahiro Shizuki (sntulix) - Why…conversion_modes_ and sentence_mode? I have imaged "bool GHOST_ImeWin32::is_english_mode"

I don’t mind if you prefer is_english_mode_ instead. Having the raw data though might come in handy later for other languages and other IMEs. Those mode flags say so much about the current state.

In D11929#309881, @Harley Acheson (harley) wrote:

@Takahiro Shizuki (sntulix) - Why…conversion_modes_ and sentence_mode? I have imaged "bool GHOST_ImeWin32::is_english_mode"

I don’t mind if you prefer is_english_mode_ instead. Having the raw data though might come in handy later for other languages and other IMEs. Those mode flags say so much about the current state.

I see it. Thank you!!

In D11929#309595, @Harley Acheson (harley) wrote:

Whoops, my mistake. I'll remove ime_status in a cleanup pass later. This patch should only contain the minimum changes neccessary, to make it easier to approve:

IME15.diff36 KBDownload

@Takahiro Shizuki (sntulix) If this looks okay could you update this diff with this version?

I updated.

IME15-UTC202107210618.diff37 KBDownload

changes
- Caches ImmGetConversionStatus, saving conversion modes and sentence modes
  - GHOST_ImeWin32::is_english_mode(): removed calling win api.
  - GHOST_ImeWin32::UpdateConversionStatus(): changed to using ImmGetOpenStatus() and ImmGetConversionStatus().
  - when WM_INPUTLANGCHANGE event, update the status having (reset cache).
note
- conversion_modes_ and sentence_mode_ are as it are.

@Takahiro Shizuki (sntulix) - I updated.

I meant update this revision. Click the "Update Diff" button at the top-right of this page so the contents shown below will be your latest version.

IME15-UTC202107210618.diff37 KBDownload

changes

Caches ImmGetConversionStatus, saving conversion modes and sentence modes

GHOST_ImeWin32::is_english_mode(): removed calling win api.

GHOST_ImeWin32::UpdateConversionStatus(): changed to using ImmGetOpenStatus() and ImmGetConversionStatus().

when WM_INPUTLANGCHANGE event, update the status having (reset cache).

note

conversion_modes_ and sentence_mode_ are as it are.

I tested.

japanese:
- latest IME:
  - IME off (halfwidth alphabet (a-zA-Z) mode # direct input)
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - a-z-A-Z : ok
    - 0-9 : ok
    - symbolic(#%&*+-/=@{|}~!"$'(),.:;<>?[\]^_`) : ok
    - numpad : ok
  - hiragana mode
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - hiragana
      - input : ok
        
        あ-、ぱ-、が-、ぁ- : ok
        
        kanji conversion : ok
        
        no dupulicated : ok
    - symbolic (fullwidth)
      - input : ok
        
        no dupulicated : ok
    - 0-9 (fullwidth)
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by ime)
      - 0-9 and symbolic: ok
  - fullwidth katakana mode
    - katakana
      - input : ok
        
        ア-、パ-、ガ-、ァ- : ok
        
        no dupulicated : ok
    - symbolic (fullwidth)
      - input : ok
        
        no dupulicated : ok
    - 0-9 (fullwidth)
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by ime input)
      - 0-9 and symbolic: ok
  - fullwidth alphabet (ａ-ｚＡ-Ｚ) mode
    - alphabet
      - input : ok
      - no dupulicated : ok
    - symbolic
      - input : ok
      - no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by ime input)
      - 0-9 and symbolic: ok
  - halfwidth katakana mode
    - katakana
      - input : ok
        
        ｱ-、ﾊﾟ-、ｶﾞ-、ｧ- : ok
        
        no dupulicated : ok
    - symbolic
      - input : ok
        
        no dupulicated : ok
    - 0-9
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by ime input)
      - 0-9 and symbolic: ok

previous IME:
- IME off (halfwidth alphabet (a-zA-Z) mode # direct input)
  - ctrl-a : ok
  - ctrl-c : ok
  - ctrl-x : ok
  - ctrl-v : ok
  - shift+ctrl-home : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - a-z-A-Z : ok
  - 0-9 : ok
  - symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^_`) : ok
  - numpad : ok
- hiragana mode
  - ctrl-a : ok
  - ctrl-c : ok
  - ctrl-x : ok
  - ctrl-v : ok
  - shift+ctrl-home : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - hiragana
    - input : ok
      - あ-、ぱ-、が-、ぁ- : ok
      - kanji conversion : ok
      - no dupulicated : ok
  - symbolic (fullwidth)
    - input : ok
      - no dupulicated : ok
  - 0-9 (fullwidth)
    - input : ok
    - no dupulicated : ok
  - by using numpad (halfwidth, by ime)
    - 0-9 and symbolic: ok
- fullwidth katakana mode
  - katakana
    - input : ok
      - ア-、パ-、ガ-、ァ- : ok
      - no dupulicated : ok
  - symbolic (fullwidth)
    - input : ok
      - no dupulicated : ok
  - 0-9 (fullwidth)
    - input : ok
    - no dupulicated : ok
  - by using numpad (halfwidth, by ime input)
    - 0-9 and symbolic: ok
- fullwidth alphabet (ａ-ｚＡ-Ｚ) mode
  - alphabet
    - input : ok
    - no dupulicated : ok
  - symbolic
    - input : ok
    - no dupulicated : ok
  - 0-9
    - input : ok
    - no dupulicated : ok
  - by using numpad (halfwidth, by ime input)
    - 0-9 and symbolic: ok
- halfwidth katakana mode
  - katakana
    - input : ok
      - ｱ-、ﾊﾟ-、ｶﾞ-、ｧ- : ok
      - no dupulicated : ok
  - symbolic
    - input : ok
      - no dupulicated : ok
  - 0-9
    - input : ok
    - no dupulicated : ok
  - by using numpad (halfwidth, by ime input)
    - 0-9 and symbolic: ok

english
- latest IME :
  - ctrl-a : ok
  - ctrl-c : ok
  - ctrl-x : ok
  - ctrl-v : ok
  - shift+ctrl-home : ok
  - home : ok
  - end : ok
  - tab (unfocusing) : ok
  - a-z-A-Z : ok
  - 0-9 : ok
  - symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^_`) : ok
  - numpad : ok

chinese
- latest IME:
  - chinese mode
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - chinese
      - input : ok
      - no dupulicated : ok
    - ~~symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^…_` halfwidth, by not ime)~~
      - ~~input : ok~~
      - ~~no dupulicated : ok~~
    - ~~0-9 (halfwidth, by not ime)~~
      - ~~input : ok~~
      - ~~no dupulicated : ok~~
    - ~~by using numpad (halfwidth, by not ime)~~
      - ~~0-9 and symbolic: ok~~
    - need test once more with symbolic (some letters convert fullwidth)
  - alphabet (a-zA-Z) mode
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - alphabet
      - input : ok
      - no dupulicated : ok
    - symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^…_` halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - 0-9 (halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by not ime)
      - 0-9 and symbolic: ok

korean
- latest IME:
  - korean mode
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - korean
      - input : ok
      - no dupulicated : ok
    - symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^_` halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - 0-9 (halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by not ime)
      - 0-9 and symbolic: ok
  - alphabet (a-zA-Z) mode
    - ctrl-a : ok
    - ctrl-c : ok
    - ctrl-x : ok
    - ctrl-v : ok
    - shift+ctrl-home : ok
    - home : ok
    - end : ok
    - tab (unfocusing) : ok
    - alphabet
      - input : ok
      - no dupulicated : ok
    - symbolic (#%&*+-/=@{|}~!"$'(),.:;<>?[\]^_` halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - 0-9 (halfwidth, by not ime)
      - input : ok
      - no dupulicated : ok
    - by using numpad (halfwidth, by not ime)
      - 0-9 and symbolic: ok

note
- Testing with win10 pro (20H2), hardware keyboard for japanese (Apple MacBook Pro 2013mid). Numpad is usb (FILCO FKB22MB).
- about numpad
  - excluding '=', because of other problem with this (This exists before patched version.).
- I (sntulix) havn't understood Chinese and Korean and how to writing with it (the grammer), my checking is just typing a key with its.
- Other IME product
  - japanese
    - ATOK 2017, Google IME (latest), CordovaSKK (latest)
      - input : ok
      - conversion : ok
      - no dupulicated : ok

Fix T89831

Fix duplication of the first letter.
Caching IME status (conversion modes and sentence modes).
added
- GHOST_ImeWin32::GetInputLanguage()
- GHOST_ImeWin32::UpdateConversionStatus()
- GHOST_ImeWin32::is_english_mode()
updated
- GHOST_SystemWin32::processKeyEvent
  - handling typing a key per language in case of IME on (chinese, korean, japanese) for prevent this duplication.

In D11929#310252, @Harley Acheson (harley) wrote:

@Takahiro Shizuki (sntulix) - I updated.

I meant update this revision. Click the "Update Diff" button at the top-right of this page so the contents shown below will be your latest version.

I see. I did “update”.
:)

Hi @Takahiro Shizuki (sntulix).
Could you test symbolic characters with Chinese mode again?
Some symbolic characters are checked by strchr, and processed by IME, then, are converted to full width characters.

In D11929#310318, @Yuki Hashimoto (hzuika) wrote:

Some symbolic characters are checked by strchr, and processed by IME, then, are converted to full width characters.

I had no problems in my test result.

In D11929#310318, @Yuki Hashimoto (hzuika) wrote:

Hi @Takahiro Shizuki (sntulix).
Could you test symbolic characters with Chinese mode again?
Some symbolic characters are checked by strchr, and processed by IME, then, are converted to full width characters.

Hi.

Sorry, I overlloked it.

I tested it once more.

Not processed by Chinese IME

#%&*+-/0123456789=@{|}~

Processed by Chinese IME

!"$'(),.:;<>?[\]^_`
↓ converted
！“￥‘（），。：；《》？【、】……——·

It are ok, exculding "‘". Is the looking of it halfwidth? (It is halfwidth when I tested.)

In D11929#310320, @Takahiro Shizuki (sntulix) wrote:

It are ok, exculding "‘". Is the looking of it halfwidth? (It is halfwidth when I tested.)

It's width looks halfwidth, but it is not ASCII. This is because of the font.
If you copy "‘" and paste to notepad.exe (MS Gothic), you can look it fullwidth.

In D11929#310328, @Yuki Hashimoto (hzuika) wrote:

In D11929#310320, @Takahiro Shizuki (sntulix) wrote:

It are ok, exculding "‘". Is the looking of it halfwidth? (It is halfwidth when I tested.)

It's width looks halfwidth, but it is not ASCII. This is because of the font.
If you copy "‘" and paste to notepad.exe (MS Gothic), you can look it fullwidth.

I see, ok.

I tested below.

chinese

latest IME:

chinese mode

Note: (In using Microsoft Chinese IME)
A key "." of QWERTY part of keyboard (jp106) is converted to "。" (utf8 char code: E3 80 82) by Chinese IME.
But the key "." of numpad part of keyboard (jp106) is "." (code: 0x2e).

In notepad.exe : "." (code: 0x2e)
In Blender 3.0.0 Alpha from official : "." (code: 0x2e)
In this patched : print no letter.

(reference)

Other IME product
- chinese
  - 捜狗拼音輸入法
    - chinese
      - input : ok
      - conversion : ok
      - no dupulicated : ok
      - halfwidth symbolic mode : ok
      - fullwidth symbolic mode : ok
      - input "." of numpad : ok
    - english
      - input : ok
      - no dupulicated : ok
      - halfwidth symbolic mode : ok
      - fullwidth symbolic mode : ok
      - input "." of numpad : ok

intern/ghost/intern/GHOST_SystemWin32.cpp
1238	A key "." of QWERTY part of keyboard (jp106) is converted to "。" (utf8code: E3 80 82) by Chinese IME. But the key "." of numpad part of keyboard (jp106) is "." (code: 0x2e).

In D11929#310453, @Takahiro Shizuki (sntulix) wrote:

A key "." of QWERTY part of keyboard (jp106) is converted to "。" (utf8 char code: E3 80 82) by Chinese IME.
But the key "." of numpad part of keyboard (jp106) is "." (code: 0x2e).

In notepad.exe : "." (code: 0x2e)

In Blender 3.0.0 Alpha from official : "." (code: 0x2e)

In this patched : print no letter.

I think you can distinguish these by using virtual keys(VK_OEM_PERIOD and VK_DECIMAL), but it is a little dirty. There may be a better way.

In D11929#310464, @Yuki Hashimoto (hzuika) wrote:

In D11929#310453, @Takahiro Shizuki (sntulix) wrote:

A key "." of QWERTY part of keyboard (jp106) is converted to "。" (utf8 char code: E3 80 82) by Chinese IME.
But the key "." of numpad part of keyboard (jp106) is "." (code: 0x2e).

In notepad.exe : "." (code: 0x2e)

In Blender 3.0.0 Alpha from official : "." (code: 0x2e)

In this patched : print no letter.

I think you can distinguish these by using virtual keys(VK_OEM_PERIOD and VK_DECIMAL), but it is a little dirty. There may be a better way.

I don't mind to not use that virtual keys. ok.
Though I'm not going to more proceed current method handling by key code (ascii code) . The reason for me is that I don't see the road of this method because of I can't image what happen about internal of Windows's handling keyboard.
I hope that I go back to search why WM_IME_STARTCOMPOSITION come after second WM_INPUT? (It seems key release.) and whether other application faced the duplication problem.
Off course, when you see the way, I go together.

By the way, this problem exists. Although the Duplication was resolved. This task has no problem now.
How about @Harley Acheson (harley)?

@Takahiro Shizuki (sntulix) - How about Harley?

This is really up to you guys.

I am happy with this patch as it is. This patch fixes the duplication issue, improves Chinese, Japanese, and Korean without causing any harm. Even if further improvements are later made, I still like the inclusion here of conversion_modes_, sentence_mode_, and is_english_mode.

I'd vote for me getting approval for this and committing it. Then we (on this thread) can work on other related issues with separate tasks and patches.

I'm also going to do some cleanup, possibly remove our usage of (deprecated) language IDs, and maybe work on getting Bengali working as an example of a non-CJK IME language.

Harley Acheson (harley) edited the summary of this revision. (Show Details)Jul 26 2021, 11:18 PM

Harley Acheson (harley) added a reviewer: Brecht Van Lommel (brecht).

I can review the code by reading, but will rely on others here to actually test it.

intern/ghost/intern/GHOST_ImeWin32.cpp
87	Use camel case like other method names.
intern/ghost/intern/GHOST_SystemWin32.cpp
1223–1244	This logic could be moved together with the other IME code in `GHOST_ImeWin32.cpp`, so this becomes something like: if (window->getImeInput()->IgnoreKeyEvent(ascii)) { return NULL }
1447	`UpdateConversionStatus` is now called only on language change or IME notify events. Should it also be called when the window is initially created?

This revision now requires changes to proceed.Jul 27 2021, 2:41 PM

@Takahiro Shizuki (sntulix) - if there is anything that is not clear about Brecht’s review comments I can help you or fix them myself. Don’t hesitate to comment here or contact me directly.

@Brecht Van Lommel (brecht) @Harley Acheson (harley)

In D11929#310950, @Brecht Van Lommel (brecht) wrote:

I can review the code by reading, but will rely on others here to actually test it.

Use camel case like other method names.

bool GHOST_ImeWin32::IsEnglishMode()

Is this OK?

This logic could be moved together with the other IME code in GHOST_ImeWin32.cpp, so this becomes something like:
if (window->getImeInput()->IgnoreKeyEvent(ascii)) {
  return NULL
}

I upload the diff that logic codes was changed to IgnoreKeyEvent(). Please check it.

D11929-sntulix-UTC202107280444.diff45 KBDownload

UpdateConversionStatus is now called only on language change or IME notify events. Should it also be called when the window is initially created?

I seems to has no duplication of letter when Blender's MainWindow created with no call UpdateConversionStatus().
(If "Let me set a different input method for each app window" of setting of IME is On.)
And when WM_IME_SETCONTEXT (This seem received when MainWindow is active.), It calls SetInputLanguage, but it seems to has no problem calling no UpdateConversionStatus().

But I found a bug case in Microsoft Japanese IME. When "Save as" (Blender File View) is focus, and not yet inputbox is active (and IME is off)
and a user change IME to On (Fullwidth Hiragana) by right-click menu of IME icon and focus the inputbox, types Japanese letter with IME, user gets duplication letter.
It seems to needs something.

intern/ghost/intern/GHOST_ImeWin32.cpp
87	Use camel case like other method names.

In D11929#311189, @Takahiro Shizuki (sntulix) wrote:

@Brecht Van Lommel (brecht) @Harley Acheson (harley)

But I found a bug case in Microsoft Japanese IME. When "Save as" (Blender File View) is focus, and not yet inputbox is active (and IME is off)
and a user change IME to On (Fullwidth Hiragana) by right-click menu of IME icon and focus the inputbox, types Japanese letter with IME, user gets duplication letter.
It seems to needs something.

I mistaked. The above case I said was not bug of this patch.
It exists before this patched because of the behavior of newer Microsoft IME.
It has not the problem.

In D11929#311189, @Takahiro Shizuki (sntulix) wrote:
bool GHOST_ImeWin32::IsEnglishMode()
Is this OK?

Yes.

I upload the diff that logic codes was changed to IgnoreKeyEvent(). Please check it.

D11929-sntulix-UTC202107280444.diff45 KBDownload

The suggestion was to move IgnoreKeyEvent into GHOST_ImeWin32.cpp so most of the IME logic is together in that file. This patch leaves it in GHOST_SystemWin32.cpp.

UpdateConversionStatus is now called only on language change or IME notify events. Should it also be called when the window is initially created?

I seems to has no duplication of letter when Blender's MainWindow created with no call UpdateConversionStatus().
(If "Let me set a different input method for each app window" of setting of IME is On.)
And when WM_IME_SETCONTEXT (This seem received when MainWindow is active.), It calls SetInputLanguage, but it seems to has no problem calling no UpdateConversionStatus().

Ok, I guess the WM_IME_NOTIFY event will happen before any keyboard event that needs to be ignored.

It’s my fault.

In D11929#311357, @Brecht Van Lommel (brecht) wrote:

The suggestion was to move IgnoreKeyEvent into GHOST_ImeWin32.cpp so most of the IME logic is together in that file. This patch leaves it in GHOST_SystemWin32.cpp.

I modified to define in GHOST_ImeWin32.

D11929-sntulix-UTC202107290731.diff38 KBDownload

However, Could I change the method's name suggestioned?
I feel it is grouped in GHOST_SystemWin32 because of the name has a word "event" and used in process KeyEvent.

suggestions:

1.
    if (window->getImeInput()->ImeKeyEvent(ascii)) {
      return NULL;
    }

2.
    if (window->getImeInput()->IsImeKeyEvent(ascii)) {
      return NULL;
    }

3.
    if (window->getImeInput()->IsKeyImeProcessing(ascii)) {
      return NULL;
    }

Note you can update the diff itself (even if you're unsure if they are correct), no need to attach them as a separate file. Since the updated diff is what I need to set it to accepted.

In D11929#311711, @Takahiro Shizuki (sntulix) wrote:

D11929-sntulix-UTC202107290731.diff38 KBDownload

Looks good.

However, Could I change the method's name suggestioned?

Sure, any of the names you suggested are fine with me.

@Takahiro Shizuki (sntulix)

In UpdateConversionStatus() you are setting conversion_modes_ and sentence_mode_ to 0 on failure. It might be more understandable to set those to IME_CMODE_ALPHANUMERIC and IME_SMODE_NONE.

added
- GHOST_ImeWin32::conversion_modes_ property
- GHOST_ImeWin32::sentence_mode_ property
- GHOST_ImeWin32::GetInputLanguage().
- GHOST_ImeWin32::UpdateConversionStatus()
- GHOST_ImeWin32::IsEnglishMode()
- GHOST_ImeWin32::IsImeKeyEvent()

changed
- GHOST_SystemWin32::processKeyEvent()
  - update to call GHOST_ImeWin32::IsImeKeyEvent() in it.

note
- GHOST_ImeWin32::UpdateConversionStatus()
  - On failure, set conversion_modes_ = IME_CMODE_ALPHANUMERIC; and sentence_mode_ = IME_SMODE_NONE;.

Thank you for your works.

In D11929#311943, @Brecht Van Lommel (brecht) wrote:

Note you can update the diff itself (even if you're unsure if they are correct), no need to attach them as a separate file. Since the updated diff is what I need to set it to accepted.

I see.

However, Could I change the method's name suggestioned?

Sure, any of the names you suggested are fine with me.

I updated the diff with IsImeKeyEvent().

In D11929#311967, @Harley Acheson (harley) wrote:

@Takahiro Shizuki (sntulix)

In UpdateConversionStatus() you are setting conversion_modes_ and sentence_mode_ to 0 on failure. It might be more understandable to set those to IME_CMODE_ALPHANUMERIC and IME_SMODE_NONE.

I updated the diff to set IME_CMODE_ALPHANUMERIC and IME_SMODE_NONE.

note:
I think this is understandable that writing it, not "0", me too.
But I have conflict in my head whether it is suitable that writing IME_CMODE_ALPHANUMERIC for the failure case in this method code.

Because of:

IME_CMODE_ALPHANUMERIC and IME_SMODE_NONE are defined 0 (default mode) at windows sdk.
And I tried below:

/*  At here, conversion_modes_= 9, 
 *            sentence_mode_= 8 in debug.
 */
::ImmGetConversionStatus(NULL, &conversion_modes_, &sentence_mode_);

/* After called ImmGetConversionStatus(NULL, ...); ,
 * conversion_modes_= 9, 
 * sentence_mode_= 8 in debug.
*/

ImmGetConversionStatus() seems to do setting nothing on failure.

@Takahiro Shizuki (sntulix) -
/* At here, conversion_modes_= 9,

sentence_mode_= 8 in debug. */

::ImmGetConversionStatus(NULL, &conversion_modes_, &sentence_mode_);

/* After called ImmGetConversionStatus(NULL, ...); ,

conversion_modes_= 9,

sentence_mode_= 8 in debug.

*/
ImmGetConversionStatus() seems to do setting nothing on failure.

Not really. When you pass that bad IMM Context to ImmGetConversionStatus it just does not update those values so they remain unchanged. It isn't that those modes are unchanged, just that you are not informed of the correct values.

This is why my version of that function was much simpler:

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

In this version we just don't update the values if bad window handle or no IMM context. They remain at the last-known values until you call UpdateConversionStatus with correct arguments.

In practice it shouldn't make much difference as you are only calling UpdateConversionStatus() when it will work. But generally keeping "last known" values are better than assuming they have become default values, for something like this that is informing of state. I might feel differently if there were values of Conversion Modes or Sentence Mode that were intended for "Indeterminate", but it is always in some actual mode.

The current implementation is fine with me.

Will leave it to @Harley Acheson (harley) to make the final decision on the UpdateConversionStatus implementation and commit this patch, I don't care strongly either way.

This revision is now accepted and ready to land.Jul 30 2021, 6:35 PM

In D11929#312207, @Harley Acheson (harley) wrote:

Not really. When you pass that bad IMM Context to ImmGetConversionStatus it just does not update those values so they remain unchanged. It isn't that those modes are unchanged, just that you are not informed of the correct values.

I wanted to know the default value in IME error case by this try.
But Microsoft set no value and no documentation at his document site. I think it’s ok if set IME_CMODE_ALPHANUMERIC in error case in UpdateConversionStatus() to safe.

In D11929#312339, @Takahiro Shizuki (sntulix) wrote:

In D11929#312207, @Harley Acheson (harley) wrote:

Not really. When you pass that bad IMM Context to ImmGetConversionStatus it just does not update those values so they remain unchanged. It isn't that those modes are unchanged, just that you are not informed of the correct values.

I wanted to know the default value in IME error case by this try.
But Microsoft set no value and no documentation at his document site. I think it’s ok if set IME_CMODE_ALPHANUMERIC in error case in UpdateConversionStatus() to safe.

The error I said includes a case IME crashed.

In D11929#312207, @Harley Acheson (harley) wrote:

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

Aren't you afraid of the case IME is crashing and conversion_modes_ and sentence_mode_ are last-known values?
If the last-known value is not alphabet mode, but IME do no response, I can't input any key.
(Though IME is restart immidiately by Windows usually, but if IME is freezing, the values is not changed and the user can't input a key.)

@Takahiro Shizuki (sntulix) - Aren't you afraid of the case IME is crashing and conversion_modes_ and sentence_mode_ are last-known values?

Error conditions and unknown states are not changed either way.

We are asking the system “what are the values of these two things?” and we are getting no answer to the question. So what do we do with our copies? Simplest is to not update them since we have not been told that they have changed. Or set them to default values, but there is just as much of a chance that is incorrect. Or set them to an indeterminate value like -1.

I’m assuming this condition won’t actually occur in real life. The call in response to WM_IME_NOTIFY will always work. The one in response to WM_INPUTLANGCHANGE will probably be quickly followed by WM_IME_NOTIFY. If IME really crashed we wouldn’t be getting IME-specific events to respond to. I might feel differently if we were calling UpdateConversionStatus in more places.

But I seriously don’t mind you having it either way. I’ll commit it as you prefer it to be.

In D11929#312359, @Harley Acheson (harley) wrote:

@Takahiro Shizuki (sntulix) - Aren't you afraid of the case IME is crashing and conversion_modes_ and sentence_mode_ are last-known values?

Error conditions and unknown states are not changed either way.

I understand your point but I want to save the work on blender when IME is crashed. Even if I can use only letters of ascii.

But I seriously don’t mind you having it either way. I’ll commit it as you prefer it to be.

I prefer current diff as it is.

Harley Acheson (harley) accepted this revision.Aug 1 2021, 8:35 PM

Closed by commit rB836aeebf7077: IME Win32: Fix Duplicated Initial Character (authored by Takahiro Shizuki (sntulix), committed by Harley Acheson (harley)). · Explain WhyAug 1 2021, 8:54 PM

This revision was automatically updated to reflect the committed changes.

Harley Acheson (harley) added a commit: rB836aeebf7077: IME Win32: Fix Duplicated Initial Character.

Hi.

I found Dialog Window (Save Dialog etc) has this bug yet.
May be, Dialog window's window procedure is separated from main window's. So we have to call setting IME status per window and dialog.

And, by adding below patch to ghost/intern/GHOST_SystemWin32.cpp, it was fixed.

Could you this differential update? or Do you think I had better create a new differential?

        case WM_ACTIVATE:
          /* The WM_ACTIVATE message is sent to both the window being activated and the window
           * being deactivated. If the windows use the same input queue, the message is sent
           * synchronously, first to the window procedure of the top-level window being
           * deactivated, then to the window procedure of the top-level window being activated.
           * If the windows use different input queues, the message is sent asynchronously,
           * so the window is activated immediately. */
          {
            GHOST_ModifierKeys modifiers;
            modifiers.clear();
            system->storeModifierKeys(modifiers);
            system->m_wheelDeltaAccum = 0;
            system->m_keycode_last_repeat_key = 0;
            event = processWindowEvent(LOWORD(wParam) ? GHOST_kEventWindowActivate :
                                                        GHOST_kEventWindowDeactivate,
                                       window);
            /* WARNING: Let DefWindowProc handle WM_ACTIVATE, otherwise WM_MOUSEWHEEL
             * will not be dispatched to OUR active window if we minimize one of OUR windows. */
            if (LOWORD(wParam) == WA_INACTIVE)
              window->lostMouseCapture();

#ifdef WITH_INPUT_IME
            window->getImeInput()->UpdateInputLanguage();
            window->getImeInput()->UpdateConversionStatus(hwnd);
#endif
            lResult = ::DefWindowProc(hwnd, msg, wParam, lParam);
            break;
          }

Best regards,
Shizuki

This revision is now accepted and ready to land.Oct 12 2021, 11:40 AM

@Takahiro Shizuki (sntulix), it's best to create a new differential.

Takahiro Shizuki (sntulix) mentioned this in T92172: Additional work to [D11929] IME Win32: Fix Duplicated Initial Character.Oct 13 2021, 8:03 AM

Takahiro Shizuki (sntulix) mentioned this in D12848: IME: Fix Multi-Window Duplicated First Character.Oct 13 2021, 10:22 AM

Brecht Van Lommel (brecht) closed this revision.Oct 13 2021, 6:00 PM

Zijun Zhou (Eary) mentioned this in T92410: Shortcuts sometimes malfunction in 3.0 alpha master.Nov 12 2021, 4:42 AM

Takahiro Shizuki (sntulix) mentioned this in T93421: Hotkeys malfunction after leaving text field with no input using IME(Chinese/Japanese/Korean).Nov 29 2021, 11:23 AM

炎臻 (PrettyFireNOI7) awarded a token.Nov 29 2021, 2:18 PM

Takahiro Shizuki (sntulix) mentioned this in T93986: Fix Hotkeys malfunction after leaving text field with no input using IME(Chinese/Japanese/Korean) Ref T93421.Dec 12 2021, 8:59 AM

Takahiro Shizuki (sntulix) mentioned this in D13551: Fix Hotkeys malfunction after leaving text field with no input using IME(Chinese/Japanese/Korean) Ref T93421.Dec 12 2021, 9:17 AM

Takahiro Shizuki (sntulix) mentioned this in T80532: Microsoft Chinese Pinyin Input BUG.Dec 16 2021, 4:26 AM