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

Workaround to disable the incomplete UI-less mode IME in SDL(Windows) #1953

Closed
wants to merge 3 commits into from
Closed

Conversation

vby
Copy link
Contributor

@vby vby commented Jul 18, 2018

  1. Set io.ImeSetInputScreenPosFn use SDL_SetTextInputRect
  2. SDL try to support UI-less mode IME on Windows, but the implementation is incomplete, that make the default IME window not shown. I made a workaround to disable it.

@ocornut
Copy link
Owner

ocornut commented Jul 18, 2018

Hello @vby and thanks a lot for the PR! I was actually puzzled by this issue a few months ago, noticed how SDL was severely messing up with the IME (and when I looked at the SDL code I can't find a way that even the SDL IME can work under Win32).

Why can't the code you added in ImGui_ImplSDL2_CreateWindow() be normally handled in ImGui_ImplSDL2_Init ?

It seems a little strange to be adding this extra entry point, hiding the raw SDL_CreateWindow away from the demo is problematic.

If for some reason the code cannot be handled in ImGui_ImplSDL2_Init, then here is my suggestion:

  • Keep the normal SDL_CreateWindow() call in all the main.cpp.
  • Instead add an extra helper, ImGui_ImplSDL2_HookIme(window) which will be called explicitly from main.cpp. The function can do the rest of the processing from your ImGui_ImplSDL2_CreateWindow() and set a flag so that the SDL_WINDOWEVENT handler can react. I think this will make the code flow more explicit and simple for the user.
  • Does SDL_ShowWindow() really needs to be called?
  • Add a description to ImGui_ImplSDL2_HookIme() in the .cpp file to explain the situation.

Thank you,
Omar

@vby
Copy link
Contributor Author

vby commented Jul 18, 2018

SDL will initalize UI-less mode IME when the first window got focus, we need setup the hook before it.
The workaround:

  1. Forward SDL_CreateWindow with SDL_WINDOW_HIDDEN flag
  2. Associate IME context to 0 that make the SDL think the IME is not ailviliable when it initalize IME
  3. Call SDL_ShowWindow to show the window
  4. SDL will associate IME context to 0 when a window got focus but IME is not ailviliable(SDL think), so
    we need reassociate IME context to default when SDL_WINDOWEVENT_FOCUS_GAINED

It seems hard to implement ImGui_ImplSDL2_HookIme. English is not mother tongue, I can't give a good explain in the code comment, sorry about that.
Another compile-time solution is define the macro SDL_DISABLE_WINDOWS_IME.

@ocornut
Copy link
Owner

ocornut commented Jan 17, 2019

Hello @vby,

I looked at this today.
Unfortunately, even as the Windows IME appears, I found an issue.
The keypresses are passed to the application while the Native IME is active.

If I press Backspace it is forwarded to both IME and imgui,
If I presse Enter, imgui receives the Enter keyboard before receiving the SDL_TEXTINPUT event, which validate a text field and ignore the text input.

When you tested it, did it works OK?

-Omar

@ocornut
Copy link
Owner

ocornut commented Feb 27, 2019

Ping @vby

@vby
Copy link
Contributor Author

vby commented Feb 28, 2019

@ocornut I found out a way to implement the ImGui_ImplSDL2_HookIme, but need an extra function ImGui_ImplSDL2_Init, you can merge it with the SDL_init maybe.
I have not found the issue which your described on my PC(win10+Microsoft IME), if the new implement have the same issue, please tell me which system version and IME you are using.

@ocornut
Copy link
Owner

ocornut commented Feb 28, 2019

Nice finding the workaround!

I confirmed the issue with keys here.

I am using Windows 10 + Microsoft IME + SDL 2.0.8, Keyboard set in Japanese.
IME appears correctly. I can input e.g. テスト and it display at the right position on screen. But if I press Backspace BOTH imgui InputText() receive the backspace (delete last character of InputText) and the IME receive the backspace (IME text becomes テス).

@vby
Copy link
Contributor Author

vby commented Mar 1, 2019

@ocornut The issue is about the VK_PROCESSKEY, I added a new wndproc hook to fix it.

@ocornut
Copy link
Owner

ocornut commented Mar 1, 2019

@ocornut The issue is about the VK_PROCESSKEY, I added a new wndproc hook to fix it

This fix this issue but now the Space key (not Space char!) doesn't get to imgui when e.g. Japanese language is active.

@vby
Copy link
Contributor Author

vby commented Mar 1, 2019

I don't think it is an issue, the Space is a char not a key down when IME is active.

@ocornut
Copy link
Owner

ocornut commented Mar 1, 2019

When IME popup is not active we need the Space key to use keyboard navigation (activate item, etc.).
See Demo>Configuration>NavEnableKeyboard

Right now even when IME popup is not active it we can't use Space anymore.

@vby
Copy link
Contributor Author

vby commented Mar 1, 2019

It is not about whether the IME popup is shown, IME popup is not shown, but the IME is still enabled.

The imgui Windows examples seems has the same issue, because the Microsoft Japanese IME processed Space to emit a space char(half-width or full-width) that prevent the TranslateMessage to emit a char and Windows will replace the wParam to VK_PROCESSKEY, while the Chinese IME do not.

Consider maybe some IME will use Space as a composition char, and press Space will show composition window and do the action bound by imgui, that was a little strange.

I add fix that emit normal key down(by SDL wndproc) only if not WantTextInput as a middle ground.

@ocornut
Copy link
Owner

ocornut commented Mar 3, 2019

The imgui Windows examples seems has the same issue, because the Microsoft Japanese IME processed Space to emit a space char(half-width or full-width) that prevent the TranslateMessage to emit a char and Windows will replace the wParam to VK_PROCESSKEY, while the Chinese IME do not.

This is a problem in imgui_impl_win32.cpp and maybe need to be fixed accross more back-end.
It is problematic to expect that the Space key can be used by the application while IME is active?
Keyboard navigation of dear imgui uses both Space and Enter for slightly different purpose.

@ocornut ocornut added the nav keyboard/gamepad navigation label Mar 3, 2019
@ocornut
Copy link
Owner

ocornut commented Mar 3, 2019

Since other back-ends have the same issue, I will merge your SDL PR for now.
I would appreciate your assistance if you can help me find a solution for using IME with keyboard navigation.

@vby
Copy link
Contributor Author

vby commented Mar 4, 2019

I added a fix that disable IME if not io.WantTextInput, avoid problems introduce by IME, other back-ends can also fix by the same way.

There is a small problem between the io.ImeSetInputScreenPosFn and io.WantTextInput, io.ImeSetInputScreenPosFn called by EndFrame but io.WantTextInput set by NewFrame, that will make io.ImeSetInputScreenPosFn called before application get the io.WantTextInput state, the IME may not enable, make the backend a little hard to write. It's better if we have io.TextInputPos(or io.TextInputRect due to the candidate window orientation) and io.WantSetTextInputPos like io.MousePos and io.WantSetMousePos.

CJK players always suffering from the IME problems in video games, we usually switch to English language(keyboard layout) when playing and switch to native language when typing, there are many games even can't use IME.

The default IME window is not well supported on some full-screen mode, it is fantastic if adding built-in IME windows(Reading, Composition, Candidate, Input Locale indicator, etc.), or a widget like DXUTguiIME.cpp. Do you have any plan or idea about this feature, much thanks for your attention to the IME compatibility.

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2019

Thank you for the update!

Can you clarify how the new ImGui_ImplSDL2_Init() function is required? I think the name may be a problem because we already have an internal bool ImGui_ImplSDL2_Init(SDL_Window* window) function.

Could the new ImGui_ImplSDL2_Init() call be removed if we add specific handling of the SDL_WINDOWEVENT event prior to ImGui_ImplSDL2_InitForXXX and ImGui_ImplSDL2_HookIme() event?

It's better if we have io.TextInputPos(or io.TextInputRect due to the candidate window orientation) and io.WantSetTextInputPos like io.MousePos and io.WantSetMousePos.

This is a good idea, I will look into this. The design for this already changed in the docking branch (for multi-viewport we need to specify the viewport). You are right it would be more flexible and consistent to do it this way.

Can you explain how dear imgui may set the io.TextInputRect if this was a rectangle?

The default IME window is not well supported on some full-screen mode, it is fantastic if adding built-in IME windows(Reading, Composition, Candidate, Input Locale indicator, etc.), or a widget like DXUTguiIME.cpp. Do you have any plan or idea about this feature, much thanks for your attention to the IME compatibility.

I don't have a specific plan, mostly because this would be too much work for me.
But I think it would be interesting if someone adapted something like DXUTguiIME.cpp into a standalone imgui widget/code.
This could be a good candidate for something to include in https://github.com/ocornut/imgui_club repo.

@vby
Copy link
Contributor Author

vby commented Mar 4, 2019

Can you clarify how the new ImGui_ImplSDL2_Init() function is required?

bool ImGui_ImplSDL2_Init(SDL_Window* window) is called after SDL_CreateWindow, but SDL_CreateWindow will StartTextInput that initialize the UIless IME, ImGui_ImplSDL2_Init for prevent StartTextInput to be called. ImGui_ImplSDL2_HookIme can be place into bool ImGui_ImplSDL2_Init(SDL_Window* window).

Can you explain how dear imgui may set the io.TextInputRect if this was a rectangle

The candidate window may have two directions, due to the available space under the input position, so we need two point(the cursor line) to draw the candidate window more accurate, point also is fine.

@ocornut
Copy link
Owner

ocornut commented Mar 4, 2019

bool ImGui_ImplSDL2_Init(SDL_Window* window) is called after SDL_CreateWindow, but SDL_CreateWindow will StartTextInput that initialize the UIless IME, ImGui_ImplSDL2_Init for prevent StartTextInput to be called. ImGui_ImplSDL2_HookIme can be place into bool ImGui_ImplSDL2_Init(SDL_Window* window).

OK, so I think we can come up with ANOTHER design, maybe the right one this time :)

We can have 1 function ImGui_ImplSDL2_InitImeHook(), no parameter, to be called by the user before SDL_CreateWindow(). This function will do the job of your current ImGui_ImplSDL2_Init() and set a flag that will also make the main init function call the equivalent of current ImGui_ImplSDL2_HookIme.

More-over, it makes sense for multi-viewport branch (docking) because from the POV of the user, the hook is not specific to a single window, but all windows.

This should work. And on the side I will rework the TextInputPos API as well.

@ocornut
Copy link
Owner

ocornut commented Sep 15, 2019

Hello @vby this is useful and I would like to get this into dear imgui, any reason why you closed this?

@ocornut
Copy link
Owner

ocornut commented Sep 20, 2019

@vby ?

peterAdams1014 pushed a commit to Euclideon/vaultclient that referenced this pull request Oct 15, 2019
@vby
Copy link
Contributor Author

vby commented Oct 16, 2019

Hello @vby this is useful and I would like to get this into dear imgui, any reason why you closed this?

The workaround is just make the example works, it is too intrusive to use within an existing SDL based project, and may have some corner cases not working as expected. So I closed the PR, finding a new way or wait for SDL to fix this issue.

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

ocornut commented Jun 8, 2021

Here
https://twitter.com/Timewatcher/status/1402265025461604363
The developers of Inochi2D suggest that there has been a similar working patch in SDL discussions:
libsdl-org/SDL#2243
Haven't investigated yet.

ocornut added a commit that referenced this pull request Feb 7, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends ime nav keyboard/gamepad navigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants