Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: KeyPressed and KeyReleased events fire twice on Linux  (Read 12214 times)

0 Members and 1 Guest are viewing this topic.

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
KeyPressed and KeyReleased events fire twice on Linux
« on: January 02, 2013, 01:59:08 pm »
Sometimes when I release a key, 3 events are fired: KeyReleased, KeyPressed and KeyReleased in this order. I could reproduce the problem on Ubuntu 12.04 LTS, on Windows it doesn't seem to appear. I use the Git revision dd48427.

Small and complete example:
#include <SFML/Window.hpp>
#include <iostream>

int main()
{
        sf::Window window(sf::VideoMode(640, 480), "SFML Test");
        window.setKeyRepeatEnabled(false);
       
        sf::Clock clock;
       
        int i = 0;
        for (;;)
        {
                sf::Event event;
                while (window.pollEvent(event))
                {
                        switch (event.type)
                        {
                                case sf::Event::Closed:
                                        return 0;
                                       
                                case sf::Event::KeyPressed:
                                        if (event.key.code == sf::Keyboard::F)
                                                std::cout << ++i << "\tPressed \t"
                                                << clock.getElapsedTime().asMilliseconds() << std::endl;
                                        break;
                                       
                                case sf::Event::KeyReleased:
                                        if (event.key.code == sf::Keyboard::F)
                                                std::cout << "\tReleased\t"
                                                << clock.getElapsedTime().asMilliseconds() << std::endl;
                                        break;
                        }
                }
               
                window.display();
        }
}

Output (this was luck, sometimes you need really long until the bug appears). You see the problem at the 6th release event.
1       Pressed         3583
        Released        4355
2       Pressed         5262
        Released        6740
3       Pressed         7244
        Released        8220
4       Pressed         8782
        Released        9740
5       Pressed         10222
        Released        11339
6       Pressed         11781
        Released        13100
7       Pressed         13100
        Released        13100
8       Pressed         13650
        Released        14547
« Last Edit: January 02, 2013, 02:02:49 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #1 on: January 02, 2013, 02:23:36 pm »
Linux (X11) doesn't natively handle the repeated key events the same way as SFML, so I have to use a ugly hack workaround to get the expected results. I wouldn't be surprised if the corresponding code was not 100% robust, but I can't find where the mistake could be.

In case you want to give it a try, the code is in WindowImplX11.cpp, lines 556 to 593.
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #2 on: January 02, 2013, 02:36:10 pm »
Linux (X11) doesn't natively handle the repeated key events the same way as SFML
If you mean setKeyRepeatEnabled(), it is not the problem -- I disabled key repetition in my code.

Or should I nevertheless take a look at the lines you wrote?
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #3 on: January 02, 2013, 02:42:11 pm »
Yes, this piece of code handles everything; it also applies the KeyRepeatEnabled flag, since there's no native X11 function that automatically does it.
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #4 on: January 02, 2013, 09:00:41 pm »
Okay, I have come to a few results. First, I have realized how much I love Visual Studio, when I once have to work (and debug) with Code::Blocks. I'm really glad I don't have to develop with it...

This is the original code:
    // Detect repeated key events
    if ((windowEvent.type == KeyPress) || (windowEvent.type == KeyRelease))
    {
        if (windowEvent.xkey.keycode < 256)
        {
            // To detect if it is a repeated key event, we check the current state of the key.
            // - If the state is "down", KeyReleased events must obviously be discarded.
            // - KeyPress events are a little bit harder to handle: they depend on the EnableKeyRepeat state,
            //   and we need to properly forward the first one.
            char keys[32];
            XQueryKeymap(m_display, keys);
            if (keys[windowEvent.xkey.keycode / 8] & (1 << (windowEvent.xkey.keycode % 8)))
            {
                // KeyRelease event + key down = repeated event --> discard
                if (windowEvent.type == KeyRelease)
                {
                    m_lastKeyReleaseEvent = windowEvent;
                    return false;
                }

                // KeyPress event + key repeat disabled + matching KeyRelease event = repeated event --> discard
                if ((windowEvent.type == KeyPress) && !m_keyRepeat &&
                    (m_lastKeyReleaseEvent.xkey.keycode == windowEvent.xkey.keycode) &&
                    (m_lastKeyReleaseEvent.xkey.time == windowEvent.xkey.time))
                {
                    return false;
                }
            }
        }
    }

The debugger showed that duplicate events occur outside the 3rd if statement, that is when the key is not held down. Additionally, I think it would be correct to set m_lastKeyReleaseEvent always to the last release event, not only if it is discarded, but I might be wrong here.

I have tried to adapt your code. According to your comment, the KeyPressed condition is independent of heldDown (variable saying that the key is currently held down). In the KeyReleased condition, I discard the event if the key is down or it's a duplicate (which may occur when the key is already up again). I also assign the last key event always. The body of the 2nd if statement then looks like this:
            char keys[32];
            XQueryKeymap(m_display, keys);

            bool duplicate = (m_lastKeyReleaseEvent.xkey.keycode == windowEvent.xkey.keycode) &&
                             (m_lastKeyReleaseEvent.xkey.time == windowEvent.xkey.time);
            bool heldDown = keys[windowEvent.xkey.keycode / 8] & (1 << (windowEvent.xkey.keycode % 8));

            // KeyRelease event + key down = repeated event --> discard
            if (windowEvent.type == KeyRelease)
            {
                m_lastKeyReleaseEvent = windowEvent;
                if (heldDown || duplicate)
                    return false;
            }

            // KeyPress event + key repeat disabled + matching KeyRelease event = repeated event --> discard
            if ((windowEvent.type == KeyPress) && !m_keyRepeat && duplicate)
            {
               return false;
            }

It seems to work so far, but there is one situation where duplicates still occur: If the timestamp is minimally different. It happens sometimes that the time differs by 1 or 2 milliseconds. You could use an inequation for the duplicate expression, just to make code even uglier :D

By the way, you should really split your processEvent() function up :P
Also, the first two if statements in the original code can be merged using operator&&.
« Last Edit: January 02, 2013, 09:23:28 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #5 on: January 02, 2013, 11:02:57 pm »
Ok, I see. I'll rewrite that to include your fix (including the 2 ms lag in timestamps -- SDL does it too so it's not that bad) and clean this function a little bit :P

Thanks a lot for your help.
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #6 on: January 02, 2013, 11:57:39 pm »
No problem, I'm always glad if I can contribute to bugfixes :)

About the 2ms, I don't know how high the delay may be, it's just the highest one I have seen with a few tests. But SDL should know it ;)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #7 on: January 03, 2013, 07:56:54 am »
Since you already have a test environment ready to work, could you quickly test this new code and tell me if it works as expected?

    // Detect repeated key events
    if (((windowEvent.type == KeyPress) || (windowEvent.type == KeyRelease)) && (windowEvent.xkey.keycode < 256))
    {
        // To detect if it is a repeated key event, we check the current state of the key:
        // - If the state is "down", KeyReleased events must obviously be discarded
        // - KeyPress events are a little bit harder to handle: they depend on the KeyRepeatEnabled state,
        //   and we need to properly forward the first one
       
        // Check if the key is currently down
        char keys[32];
        XQueryKeymap(m_display, keys);
        bool isDown = keys[windowEvent.xkey.keycode / 8] & (1 << (windowEvent.xkey.keycode % 8));

        // Check if it's a duplicate event
        bool isDuplicate = (windowEvent.xkey.keycode == m_lastKeyReleaseEvent.xkey.keycode) &&
                           (windowEvent.xkey.time - m_lastKeyReleaseEvent.xkey.time <= 2);

        // KeyRelease event + key down or duplicate event = repeated event --> discard
        if ((windowEvent.type == KeyRelease) && (isDown || isDuplicate))
            return false;

        // KeyPress event + matching KeyRelease event = repeated event --> discard if key repeat is disabled
        if ((windowEvent.type == KeyPress) && isDuplicate && !m_keyRepeat)
            return false;

        // Keep track of the last KeyRelease event
        if (windowEvent.type == KeyRelease)
            m_lastKeyReleaseEvent = windowEvent;
    }
I still don't get how a duplicate KeyRelease event can occur when the key is not down, but if it solves the problem, wonderful ;D

Quote
About the 2ms, I don't know how high the delay may be, it's just the highest one I have seen with a few tests. But SDL should know it
SDL uses < 2, so they may not know better than us ;D
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #8 on: January 03, 2013, 11:44:40 am »
The code doesn't work; it fires repeated events, when m_keyRepeat is false.

You need to store the last release event also if it is discarded, i.e. reorder the if statements:
        // Keep track of the last KeyRelease event
        if (windowEvent.type == KeyRelease)
            m_lastKeyReleaseEvent = windowEvent;

        // KeyRelease event + key down or duplicate event = repeated event --> discard
        if ((windowEvent.type == KeyRelease) && (isDown || isDuplicate))
            return false;

        // KeyPress event + matching KeyRelease event = repeated event --> discard if key repeat is disabled
        if ((windowEvent.type == KeyPress) && isDuplicate && !m_keyRepeat)
            return false;

This code works as expected. I have however managed to have a delay of 3 or even 4 ms ;D
(very rarely, and that computer is already half a decade old...)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #9 on: January 03, 2013, 11:54:29 am »
Oops! I forgot that the function returned immediately when it found an event to discard.

Quote
I have however managed to have a delay of 3 or even 4 ms
Let's use 5 ms then, it is very unlikely (impossible?) that a human can trigger two events that close.
In case you have extra free time (and I know you don't ;D), it would be interesting to test SDL (latest 1.3 sources) on your PC and see if you can break their code with your 4 ms delay.
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: KeyPressed and KeyReleased events fire twice on Linux
« Reply #10 on: January 03, 2013, 12:02:38 pm »
In case you have extra free time (and I know you don't ;D), it would be interesting to test SDL (latest 1.3 sources) on your PC and see if you can break their code with your 4 ms delay.
It would indeed be interesting, but at the moment I really don't have the time, already this took a lot of time (especially setting everything up and debugging on Linux).

Maybe in February ;)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

 

anything