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

Improved method used to filter KeyRelease in X11 #1230

Closed
wants to merge 1 commit into from

Conversation

texus
Copy link
Contributor

@texus texus commented May 8, 2017

This should fix #1223.

The event behind KeyRelease is now only retrieved from the same window. Because peeking did not work well, the event is instead removed from the queue and put back afterwards (unless the event is KeyPress and key repeating is disabled).

@mantognini
Copy link
Member

mantognini commented Jun 1, 2017

I quickly tested this, it seems to indeed fix the issue. @naezith, can you confirm it works as well on your system?

@naezith
Copy link

naezith commented Jun 1, 2017

@mantognini It's working exactly the same for me, Release & Press repeats after a new window.

@mantognini
Copy link
Member

@naezith Strange.. I did check again in a Ubuntu VM (at work I'm under Mint), this PR fixed the issue. Did you make sure to remove the old binaries and do a clean build?

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.5.0 Jun 3, 2017
@eXpl0it3r
Copy link
Member

Did you make sure to remove the old binaries and do a clean build?

@naezith

@naezith
Copy link

naezith commented Jun 5, 2017

Yes.

@texus
Copy link
Contributor Author

texus commented Jun 10, 2017

There seems to be a second issue. My fix should solve the issue caused by multiple windows (tested on both manjaro and on ubuntu). However, even with a single window I sometimes get the Release & Press repeats (but only on my pc with ubuntu), just at a much slower rate (e.g. once every second instead of many events per second which happened with multiple windows before my fix).

While debugging I noticed that the repeats stop entirely when there is a cout on top of the function. I added a short sleep now when a KeyRelease event occurs to allow some time for the KeyPress to arrive. Now all key pressed and released events always seem to occur like they should.

@naezith Could you test this new version?

@naezith
Copy link

naezith commented Jul 14, 2017

I think sleep call should not be there no matter what.

@texus
Copy link
Contributor Author

texus commented Jul 14, 2017

I agree with that. The behavior was a bit inconsistent on my ubuntu pc without that sleep call, but that commit was more for testing than to really be included in SFML.

@eXpl0it3r eXpl0it3r moved this from Discussion to WIP in SFML 2.5.0 Jul 18, 2017
@eXpl0it3r eXpl0it3r moved this from WIP to Review & Testing in SFML 2.5.0 Jul 18, 2017
@eXpl0it3r
Copy link
Member

So what's the status of this? Is the first commit okay? Did you manage to find out why there are additional repeats?

@eXpl0it3r eXpl0it3r moved this from Review & Testing to Requires Adjustments in SFML 2.5.0 Jul 18, 2017
@texus
Copy link
Contributor Author

texus commented Jul 18, 2017

The first commit should solve the issue. I tested it on two computers and mantognini also confirmed that it worked.
Only @naezith claims that it didn't fix the issue for him. So he probably has to figure out why it isn't working for him (if absolutely certain the code with the fix is used, try outputting some variables inside WindowImplX11::processEvent and see if anything weird shows).

I have no idea why on my ubuntu pc there were additional repeats unrelated to the first issue (they were much less often and more irregular, so the distinction with the original issue was very clear). But considering they only happen on that pc and the repeats didn't always occur (e.g. plugging in a keyboard or creating a new window or doing some other things could cause the repeats to stop), I'd suggest to just forget about this ever happening until someone else ever faces a similar issue. I'll remove the second commit later today.

@eXpl0it3r eXpl0it3r moved this from Requires Adjustments to Review & Testing in SFML 2.5.0 Aug 1, 2017
@eXpl0it3r
Copy link
Member

@binary1248 mentioned that this isn't really a good fix. Can you elaborate on a better approach to fix this issue, @binary1248?

@binary1248
Copy link
Member

I submitted an alternative patch in #1291.

@eXpl0it3r
Copy link
Member

There's a better implementation by @binary1248 in #1291

@eXpl0it3r eXpl0it3r closed this Sep 25, 2017
@eXpl0it3r eXpl0it3r moved this from Review & Testing to Ready in SFML 2.5.0 Sep 25, 2017
@eXpl0it3r eXpl0it3r moved this from Ready to Merged / Superseded in SFML 2.5.0 Sep 25, 2017
@SFML SFML deleted a comment from mantognini Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.5.0
  
Merged / Superseded
Development

Successfully merging this pull request may close these issues.

setKeyRepeatEnabled(false) doesn't work after second window.create() call on Linux
5 participants