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

Properly interpret modifiers on GLFW key events #44844

Merged
merged 3 commits into from
Nov 18, 2019

Conversation

gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Nov 13, 2019

Description

GLFW key events set modifier flags based on what the modifier state was before the event, unlike every other platform. This modifies the GLFW key support to take that into account.

As a small cleanup, I fixed a documentation macro reference for the modifier flags that was duplicated.

Tests

  • Updated GLFW tests to simulate what actually happens on GLFW.

Breaking Change

  • No, this is not a breaking change.

@gspencergoog gspencergoog added platform-linux Building on or for Linux specifically a: desktop Running on desktop labels Nov 13, 2019
@fluttergithubbot fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Nov 13, 2019
static const int modifierNumericPad = 0x0020;

int _mergeModifiers({int modifiers, int keyCode, bool isDown}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So my understanding is correct: The flutter keyboard code treats the modifiers as containing the current modifier key info, like if you press left control. But GLFW doesn't include this info, so it would send a modifier without the left control info. Here you are normalizing it by updating the modifier with the current key info?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, more or less. More accurately, GLFW just sends the value of modifiers as the state before the current event, so it lags. All the other platforms set the state of the modifier including the effect of the current event.


int modifierChange = 0;
switch (keyCode) {
case 341: // controlLeft
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these keycodes have constants that could be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gspencergoog gspencergoog merged commit d026f2d into flutter:master Nov 18, 2019
@gspencergoog gspencergoog deleted the fix_glfw_modifiers branch March 13, 2020 16:10
@lock
Copy link

lock bot commented Apr 2, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@lock lock bot locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop framework flutter/packages/flutter repository. See also f: labels. platform-linux Building on or for Linux specifically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants