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

Backspace in InputText is broken on macOS #2578

Closed
sergeyvfx opened this issue May 23, 2019 · 10 comments
Closed

Backspace in InputText is broken on macOS #2578

sergeyvfx opened this issue May 23, 2019 · 10 comments

Comments

@sergeyvfx
Copy link

Version/Branch of Dear ImGui:

Version: v1.70, master at 7bc03f7
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: metal (using example_apple_metal Xcode project)
Compiler: Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Operating System: macOS 10.14.5

My Issue/Question:

Backspace doesn't behave correctly in the latest release 1.70 and master branches:

  • Typing single backspace key stroke does nothing.
  • Holding it down starts to delete text, but after a while starts typing spaces instead.

Tracked it down to 4a57507. What happens here is that the c is 127 and my guess is that isprint() was not considering it printable. However, now it is considered printable and is being handled as text input.

Only happens for backspace: delete and arrow keys are working fine (InputTextFilterCharacter is not even called for them, which is somewhat expected).

Only notices on macOS. Windows and Linux seemed fine (don't have them with ImGui project handy atm, but if it helps can test them later just to be sure).

Not sure where to go from this, leaving to people who are more familiar with the internals :)

Screenshots/Video

Screen Recording 2019-05-23 at 21.33.47.zip

(Github only allowed to attached compressed file, this is a short screencast of me holding down and then pressing backspace few times).

Standalone, minimal, complete and verifiable example:

Just use default demo application, multi-line text input.

@phoenixcatdog
Copy link

I had the same issue while trying to port my own working native imgui opengl2-3 version to the official version.

What happens is inside imgui_widgets.cpp

Backspace key code= 127 is stored in a queue, and it passes the filter as a valid key, what this does is increment the cursor value in the state structure. Then the code identifies that it is backspace and decrements the cursor value, so cursor value remains the same (1-1 = 0).

The code that does it comes after:
if (io.InputQueueCharacters.Size > 0)
[...]
if (InputTextFilterCharacter(&c, flags, callback, callback_user_data))
state->OnKeyPressed((int)c);

So a temporary solution is adding the line:
if (c == 127)
continue;

Before "if (InputTextFilterCharacter(".

Obviously this is a temporary solution, as it interferes with the common code for other libraries. But I don't have the necessary general knowledge to apply a global solution.

With this temporary fix the native macOS Opengl version works pretty well, does not feel like ALPHA software like is implied in the source code.

The only remaining issue is the dead keys handling, for inputting things like "camión" in Spanish or "über" in german without using IME.

I have a more or less working solution for dead keys in my native version that I will port to official imgui soon.

@sergeyvfx
Copy link
Author

Wouldn't it be more correct approach to modify InputTextFilterCharacter in a way which will discard characters which match values from io.KeyMap for ImGuiKey_Backspace, ImGuiKey_Delete and so on?

@phoenixcatdog
Copy link

Of course anything is better than my hack. That is just a temporary hack in order to assert that this is the problem.

Someone with more knowledge than me about the project in general, like ocornut, can now develop a proper solution to the problem with minimal amount of work.

The proper solution should work in the mac specific side of code without affecting other libs.

But ocornut did not have access to macs in the past. So if you feel you can improve the code, feel free to do it.

But I can not do that now because I don't understand how the project interfaces with all the different libraries.

Right now I will work on dead keys code and propose a patch for it. I will study on my own how SDL and other libraries handle backspace but later. My priority right now is making dead keys code work as soon as possible for me.

It ocornut or someone else could provide Hints for it, people working in the mac solution will be more efficient.

@phoenixcatdog
Copy link

One of the solutions that will work with generic code is catching the BACKSPACE key_code in the native mac code(I already have to do that for catching dead keys) and converting it to a private Unicode value in ImGuiKey_Backspace.

There is already code that filters out private unicode values in InputTextFilterCharacter:
// Filter private Unicode range. GLFW on OSX seems to send private characters for special keys like arrow keys (FIXME)
if (c >= 0xE000 && c <= 0xF8FF)

This way, Backspace will be automatically filtered without having to modify the generic imgui code.

I don't really understand right now how SDL and glfw are filtering BACKSPACE.

@phoenixcatdog
Copy link

phoenixcatdog commented Sep 14, 2019

OK, I have inspected the code.

It seems clear to me that the real difference is that Characters or unicode symbols are added to a queue, but BACKSPACE, ENTER or soon are not, but they are stored in some kind of memory that is recognized by IsKeyPressedMap().

So the filter has to be done there, not adding AddInputCharacter() in the first place for backspace, but saving it in memory somewhat so isKeyPressedMap() could recognize it.

I suspect this memory is io.KeysDown[key]

@phoenixcatdog
Copy link

Ok, it works, In
ImGui_ImplOSX_HandleEvent() in imgui_impl_osx.cpp

Just Filter out the Backspace, not adding an InputCharater, something like this:

if (!io.KeyCtrl && !(c >= 0xF700 && c <= 0xFFFF))
{
if (c != io.KeyMap[ImGuiKey_Backspace] )
io.AddInputCharacter((unsigned int)c);
}

This works without affecting the generic code. I have a primitive version of dead keys working on mac.

@sergeyvfx
Copy link
Author

Since 378035c, does it mean it's an application who needs to filter backspace codes on mac?
Or is it still expected more general solution will be committed to ImGui?

@ocornut
Copy link
Owner

ocornut commented Oct 12, 2019

Hello @sergeyvfx

Sorry I forgot to report on this topic, but 378035c (#2817) pushed yesterday should have fixed the issue described here when using imgui_impl_osx.mm.

I didn't realise that 127 is a semi-standard ascii representation for delete so we could tentatively also filter that out in io.AddInputCharacter() as well.

If you have a Mac, could you check what is currently being emitted by the SDL and GLFW back-ends? (keys and characters) when pressing backspace?

Thank you!

ocornut added a commit that referenced this issue Oct 12, 2019
@ocornut
Copy link
Owner

ocornut commented Oct 12, 2019

As it turns out, since InputText() never reacted probably to Ascii 127, I don't think we can do anything wrong also adding the filter there. That's done now.
AFAIK this issue should now be all cleared, what do you think @sergeyvfx ?

@sergeyvfx
Copy link
Author

If you have a Mac, could you check what is currently being emitted by the SDL and GLFW back-ends? (keys and characters) when pressing backspace?

From watching Keyboard, Mouse & Navigation stats:

SDL uses 76 for Delete, 42 for Backspace. (SDL_SCANCODE_DELETE
and SDL_SCANCODE_BACKSPACE respectively)
GLFW uses 261 for Delete, 259 for Backspace (GLFW_KEY_DELETE and GLFW_KEY_BACKSPACE respectively).

This part actually behaves same exact on all platforms (since ImGui receives higher-level enum values for keys). Is was really just macOS which is an odd ball.

As it turns out, since InputText() never reacted probably to Ascii 127, I don't think we can do anything wrong also adding the filter there. That's done now.

I think so too. All the unicode decoding is done prior that, so it can not be part of muli-byte sequence.
Also, was how previous versions worked. Quite sure if code 127 was expecting to do something you'd hear that :)

AFAIK this issue should now be all cleared, what do you think @sergeyvfx ?

Seems so. After updating to aeb6481 everything behaves as one would expect it to. Thanks for fix! :)

P.S. Not sure if you want to reference the issue somewhere else prior to closing. So leaving it up to you to click the "Close" button.

@ocornut ocornut closed this as completed Oct 12, 2019
@ocornut ocornut added the ime label Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants