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

Bug in SDL_TEXTINPUT event when typing in hangul in linux fcitx IME #6164

Open
imprity opened this issue Aug 30, 2022 · 9 comments
Open

Bug in SDL_TEXTINPUT event when typing in hangul in linux fcitx IME #6164

imprity opened this issue Aug 30, 2022 · 9 comments
Assignees
Milestone

Comments

@imprity
Copy link

imprity commented Aug 30, 2022

SDL Version is 2.25.0
I am using Linux Mint 20.2 Cinnamon 5.0.7
fcitx version is 4.2.9.7

There is a bug when typing hangul using fcitx.

Let's say that I'm trying to type 마. (U+B9C8 in unicode)

To do that I have to type ㅁ(U+3141) key and ㅏ(U+314F) key on keyboard.

And when I do that SDL_TextEditingEvent.text is 마 as expected.

But after that when I type "/" key, SDL_TEXTINPUT event reports "/" first then reports "마" later which is not the case on windows.

I have written a simple text editing code to demonstrate this bug.

#include <SDL.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdio.h>
#include <assert.h>

//Screen dimension constants
const int SCREEN_WIDTH = 600;
const int SCREEN_HEIGHT = 400;

char edit_buffer[2048];
char composite_buffer[2048];

int main( int argc, char* args[] )
{
    memset(edit_buffer, 0, 2048);
    memset(composite_buffer,0, 2048);

    if(SDL_Init(SDL_INIT_VIDEO) != 0)
    {
        printf("Failed to init SDL\n");
        return 1;
    }

    SDL_Window *window = SDL_CreateWindow("Test Windows",
                                          SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
                                          SCREEN_WIDTH, SCREEN_HEIGHT,
                                          0);
    SDL_Renderer *renderer = SDL_CreateRenderer(window, -1, 0);

    SDL_StartTextInput();

    bool quit = false;

    //clear screen
    printf("\x1B[2J");
    //set cursor pos to zero
    printf("\x1B[H");

    SDL_Event event;

    while(!quit)
    {
        while(SDL_PollEvent(&event))
        {
            if(event.type == SDL_QUIT)
            {
                quit = true;
            }
            else if(event.type == SDL_TEXTINPUT)
            {
                strcat(edit_buffer, event.text.text);
            }
            else if(event.type == SDL_TEXTEDITING)
            {
            
            event.edit
                memcpy(composite_buffer, event.edit.text, strlen(event.edit.text) + 1);
            }
        }

        //clear screen
        printf("\x1B[2J");
        //set cursor pos to zero
        printf("\x1B[H");

        printf("----edit_string----\n");
        printf("%s\n", edit_buffer);
        printf("----edit_string----\n");

        printf("----composite----\n");
        printf("%s\n", composite_buffer);
        printf("----composite----\n");

        SDL_RenderClear(renderer);
        SDL_RenderPresent(renderer);
    }

    SDL_StopTextInput();

    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    SDL_Quit();
    return 0;
}
@slouken slouken added this to the 2.26.0 milestone Aug 30, 2022
@slouken slouken self-assigned this Oct 25, 2022
@slouken
Copy link
Collaborator

slouken commented Nov 13, 2022

I'm not going to have time to look at this for this milestone. Anyone who is familiar with Linux IME input, feel free to pick this up!

@slouken slouken removed this from the 2.26.0 milestone Nov 13, 2022
@pionere
Copy link
Contributor

pionere commented Nov 14, 2022

I'm not really familiar with the code, but just having a quick glance at the code from line 1075 of SDL_keyboard.c seems to be wrong (added by #5398).

The branch is completely useless. Either it should set event.editExt..., or be completely eliminated.

@Guldoman
Copy link
Contributor

I'm not really familiar with the code, but just having a quick glance at the code from line 1075 of SDL_keyboard.c seems to be wrong (added by #5398).

The branch is completely useless. Either it should set event.editExt..., or be completely eliminated.

That branch is needed to keep compatibility with the behavior pre-SDL_HINT_IME_SUPPORT_EXTENDED_TEXT.

@pionere
Copy link
Contributor

pionere commented Nov 14, 2022

That branch is needed to keep compatibility with the behavior pre-SDL_HINT_IME_SUPPORT_EXTENDED_TEXT.

What do you mean? Lines 1075-1079 are the same as lines 1061-1065. There is nothing in-between which uses the event object.

@Guldoman
Copy link
Contributor

Ah yeah, I was thinking about the difference with 1069-1073.
It looks like it wasn't rebased correctly when going from #5378 to #5398.

I doubt this would cause this issue tho.

@pionere
Copy link
Contributor

pionere commented Nov 14, 2022

I doubt this would cause this issue tho.

That is quite possible. Nevertheless, I'm going to create a PR for that.

@Guldoman
Copy link
Contributor

Testing the behavior using SDL main I get this:

Video.del.2022-11-14.17-44-08.webm

Pressing then (which are a and k on my keyboard) result correctly in .
Pressing / seems to work fine.
Pressing seems to break it.

Notice how some keypress events are leaked from the IME. This doesn't happen with other IMEs like Anthy and Mozc.

The same behavior can be seen in other editors that don't use SDL. So I think this is an Hangul IME issue.

@imprity
Copy link
Author

imprity commented Jan 13, 2023

Sorry for the late response but I was busy doing some stuffs. And I was very surprised to see that SDL2 is now SDL3, So I decided to test SDL3 with the updated code.

#include <SDL3/SDL.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdio.h>
#include <assert.h>

//Screen dimension constants
const int SCREEN_WIDTH = 600;
const int SCREEN_HEIGHT = 400;

char edit_buffer[2048];
char composite_buffer[2048];

int main( int argc, char* args[] )
{
    memset(edit_buffer, 0, 2048);
    memset(composite_buffer,0, 2048);

    if(SDL_Init(SDL_INIT_VIDEO) != 0)
    {
        printf("Failed to init SDL\n");
        return 1;
    }

    SDL_Window *window = SDL_CreateWindow("Test Windows",
                                          SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
                                          SCREEN_WIDTH, SCREEN_HEIGHT,
                                          0);
    SDL_Renderer *renderer = SDL_CreateRenderer(window, -1, 0);

    SDL_StartTextInput();

    bool quit = false;

    //clear screen
    printf("\x1B[2J");
    //set cursor pos to zero
    printf("\x1B[H");

    SDL_Event event;

    while(!quit)
    {
        while(SDL_PollEvent(&event))
        {
            if(event.type == SDL_QUIT)
            {
                quit = true;
            }
            else if(event.type == SDL_TEXTINPUT)
            {
                strcat(edit_buffer, event.text.text);
            }
            else if(event.type == SDL_TEXTEDITING)
            {
                memcpy(composite_buffer, event.edit.text, strlen(event.edit.text) + 1);
            }

            //clear screen
            printf("\x1B[2J");
            //set cursor pos to zero
            printf("\x1B[H");

            printf("----edit_string----\n");
            printf(u8"%s\n", edit_buffer);
            printf("----edit_string----\n");

            printf("----composite----\n");
            printf(u8"%s\n", composite_buffer);
            printf("----composite----\n");

            size_t raw_str_size = strlen(edit_buffer);

            printf("----edit buffer raw hex----\n");
            for(size_t i=0; i<raw_str_size; i++){
                printf("%hhx ", edit_buffer[i]);
            }
            printf("\n");
            printf("----edit buffer raw hex----\n");

            raw_str_size = strlen(composite_buffer);

            printf("----composite buffer raw hex----\n");
            for(size_t i=0; i<raw_str_size; i++){
                printf("%hhx ", composite_buffer[i]);
            }
            printf("\n");
            printf("----composite buffer raw hex----\n");
        }

        SDL_RenderClear(renderer);
        SDL_RenderPresent(renderer);
    }

    SDL_StopTextInput();

    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    SDL_Quit();
    return 0;
}

And I'm happy to report that this code absolutely does not work anymore, even for simple ascii!~ yay....

I assume it's because new APIs are being introduced or something. So I'll just close this issue with this comment.

@imprity imprity closed this as completed Jan 13, 2023
@imprity
Copy link
Author

imprity commented Jan 13, 2023

well SDL3 crashes so I tried with latest release from https://github.com/libsdl-org/SDL/releases and problem I described consists.

I changed my mind and decided to reopen the issue. I do know that linux IME is not perfect and it may as well be an hangul IME issue but I'm hoping that this issue post will draw attention to some IME guru that can hopefully fix this.

@imprity imprity reopened this Jan 13, 2023
@slouken slouken added this to the 3.2.0 milestone Nov 7, 2023
@slouken slouken modified the milestones: 3.2.0, 3.0 ABI May 21, 2024
@slouken slouken modified the milestones: 3.0 ABI, 3.2.0 Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants