-
Notifications
You must be signed in to change notification settings - Fork 6k
Make pending event handling more lenient to allow out of order responses #23504
Conversation
06624b2 to
162534b
Compare
162534b to
cd2bd95
Compare
dkwingsmt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Does this happen to other platforms as well? |
|
Yes, Windows did (but I already fixed that in my pending PR), and Android needs a similar fix, which I'll work on today. |
|
@dkwingsmt I mean, actually, I haven't seen this same problem on other platforms, but I'm going to proactively fix the algorithm that the others are using (macOS doesn't use the same algorithm) to not assume that order is always preserved. |
| for (guint i = 0; i <= self->pending_events->len; ++i) { | ||
| if (FL_KEY_EVENT_PAIR(g_ptr_array_index(self->pending_events, i))->id == | ||
| id) { | ||
| return FL_KEY_EVENT_PAIR(g_ptr_array_index(self->pending_events, 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're testing against index i, but then still returning index 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding that. I'll submit a fix today.
Description
This PR makes the Linux key handling code a little more lenient when it comes to the order in which it receives responses to key events from the framework. I had assumed that there wasn't a case where responses could get out of order, but it seems that it is possible, given that you can mash on the keyboard and eventually get one out of order.
This changes the code so that instead of just looking at the first entry in the pending event deque, it searches the deque starting at the beginning to find the event, and remove it.
Related Issues
Tests