[macOS, Keyboard] Enqueue event processing #32120
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
dkwingsmt
force-pushed
the
mac-key-full-queue
branch
from
ad05fd8 to
ad8b644
Compare
| uint64_t responseId = _responseId; | ||
| FlutterKeyPendingResponse* pending = | ||
| [[FlutterKeyPendingResponse alloc] initWithHandler:self responseId:responseId]; | ||
| // The `pending` is released in `HandleResponse`. |
There was a problem hiding this comment.
Don't need the backquotes (not a markdown comment)
| } | ||
|
|
||
| - (void)handleEvent:(nonnull NSEvent*)event { | ||
| // The `handleEvent` does not process the event immediately, but instead put |
There was a problem hiding this comment.
| // The `handleEvent` does not process the event immediately, but instead put | |
| // The `handleEvent` does not process the event immediately, but instead puts |
Also don't need the backticks (not markdown).
shell/platform/darwin/macos/framework/Source/FlutterKeyboardManagerUnittests.mm
Outdated
Show resolved
Hide resolved
shell/platform/darwin/macos/framework/Source/FlutterKeyboardManagerUnittests.mm
Outdated
Show resolved
Hide resolved
|
|
||
| // At the start of any kind of call, record the call type to the given storage. | ||
| // | ||
| // Only calls that are included in `typeMask` will be added. Options are |
There was a problem hiding this comment.
Don't need the backticks
There was a problem hiding this comment.
Yeah but I thought having backticks made it easier to recognize they are variables.
Co-authored-by: Greg Spencer <[email protected]>
3 participants
This PR puts the procedure of handling a key event into a queue, so that before the last event is done processing (possibly handled by
TextInputPluginand sent to the next responder), the next event will not be started at all.This PR fixes the macOS part of flutter/flutter#82673, which is caused by the racing condition between the two steps of key handling: key events, and text events. The complete analysis is posted at flutter/flutter#82673 (comment).
However, by reducing (the problematic) parallelization, this fix unsurprisingly comes at a cost of slowing down the overall key event processing. This should not be a problem in other languages, but in Vienamese Telex IME, where one key press can result in as many as 5 messages, the result is noticeable, as shown below.
1.mov
Mitigating this problem might need bigger changes in the future. For now, let's land this PR to correct the behavior first.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.