Hardware keyboard: codegen #73440
Hardware keyboard: codegen #73440
Conversation
flutter-dashboard
bot
added
the
c: contributor-productivity
|
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. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@gspencergoog My apology for submitting this giant PR altogether. I want to avoid it but I can't find a way to split. |
gspencergoog
left a comment
gspencergoog
left a comment
There was a problem hiding this comment.
I've made a first pass. Whew that's a lot of code! :-)
Do you have any idea of the key mappings that were lost during this conversion (if any) that you mentioned in the PR description? It would be good to be able to quantify that for the people who were depending on those keys.
| import 'utils.dart'; | ||
|
|
||
|
|
||
| /// Generates the key mapping of Android, based on the information in the key |
There was a problem hiding this comment.
| /// Generates the key mapping of Android, based on the information in the key | |
| /// Generates the key mapping for Android, based on the information in the key |
There was a problem hiding this comment.
I think you missed this one (I know, it's minor).
| import 'physical_key_data.dart'; | ||
| import 'utils.dart'; | ||
|
|
||
| bool _isLetter(String? char) { |
There was a problem hiding this comment.
Should probably call this _isAsciiLetter, since it doesn't include most of the letters in Unicode.
There was a problem hiding this comment.
I don't think you addressed this in the last pass.
| ]; | ||
|
|
||
| // The name of keys that require special attention. | ||
| const List<String> kSpecialPhysicalKeys = <String>['CapsLock']; |
There was a problem hiding this comment.
Why doesn't this include NumLock?
There was a problem hiding this comment.
The capslock is special because of macOS's weird key model for it (every key tap is counted as either keydown or keyup, but not keydown and keyup). The macOS embedding therefore has a function only handles CapsLock. This variable is generated so that the macOS embedding can use if (key == kCapsLock) { handleCapsLock(); }
There was a problem hiding this comment.
Ahh, I thought NumLock also followed that same weird convention. That's super odd that it's only CapsLock, but OK.
There was a problem hiding this comment.
Well the fact is that I've not seen a macOS that supports NumLock. The macOS embedding's KeyboardManager doesn't even support locks other than CapsLock.
There was a problem hiding this comment.
True, Apple branded keyboards don't use num lock, but you can plug in a non-apple keyboard that has numlock, and it works, so I assumed there were similar events.
|
@gspencergoog I think I've taken care of all comments. Can you take another look? |
| /// /** Space key. */ | ||
| /// #define GDK_KEY_space 0x020 | ||
| static void _readGtkKeyCodes(Map<String, LogicalKeyEntry> data, String headerFile, Map<String, List<String>> nameToGtkName) { | ||
| final RegExp definedCodes = RegExp(r'#define GDK_KEY_([a-zA-Z0-9_]+)\s*0x([0-9a-f]+),?'); |
There was a problem hiding this comment.
This seems like another good place to use named groups, and for the other platforms.
| ]; | ||
|
|
||
| // The name of keys that require special attention. | ||
| const List<String> kSpecialPhysicalKeys = <String>['CapsLock']; |
There was a problem hiding this comment.
Ahh, I thought NumLock also followed that same weird convention. That's super odd that it's only CapsLock, but OK.
|
@gspencergoog I should have addressed all issues (hopefully) |
| import 'utils.dart'; | ||
|
|
||
|
|
||
| /// Generates the key mapping of Android, based on the information in the key |
There was a problem hiding this comment.
I think you missed this one (I know, it's minor).
|
GitHub seems to be acting weird, so here's what I think still needs to be fixed still before landing:
|
|
|
List of physical keys is unchanged. List of logical keys is changed as follows: @@ -1,26 +1,47 @@
abort
+accel
+accept
+add
again
+allCandidates
+alphanumeric
alt
+altGraph
altLeft
altRight
+ampersand
+appSwitch
arrowDown
arrowLeft
arrowRight
arrowUp
+asterisk
+at
+attn
+audioBalanceLeft
+audioBalanceRight
+audioBassBoostDown
+audioBassBoostToggle
+audioBassBoostUp
+audioFaderFront
+audioFaderRear
+audioSurroundModeNext
+audioTrebleDown
+audioTrebleUp
audioVolumeDown
audioVolumeMute
audioVolumeUp
+avrInput
+avrPower
backquote
backslash
backspace
-bassBoost
+bar
+braceLeft
+braceRight
bracketLeft
bracketRight
-brightnessAuto
brightnessDown
-brightnessMaximum
-brightnessMinimum
-brightnessToggle
brightnessUp
browserBack
browserFavorites
@@ -29,18 +50,34 @@ browserHome
browserRefresh
browserSearch
browserStop
+call
+camera
+cameraFocus
+cancel
capsLock
+caret
channelDown
channelUp
+clear
close
closedCaptionToggle
+codeInput
+colon
+colorF0Red
+colorF1Green
+colorF2Yellow
+colorF3Blue
+colorF4Grey
+colorF5Brown
comma
+compose
contextMenu
control
controlLeft
controlRight
convert
copy
+crSel
cut
delete
digit0
@@ -53,12 +90,21 @@ digit6
digit7
digit8
digit9
-displayToggleIntExt
+dimmer
+displaySwap
+dollar
+dvr
+eisu
eject
end
+endCall
enter
equal
+eraseEof
escape
+exSel
+exclamation
+execute
exit
f1
f10
@@ -84,6 +130,19 @@ f6
f7
f8
f9
+favoriteClear0
+favoriteClear1
+favoriteClear2
+favoriteClear3
+favoriteRecall0
+favoriteRecall1
+favoriteRecall2
+favoriteRecall3
+favoriteStore0
+favoriteStore1
+favoriteStore2
+favoriteStore3
+finalMode
find
fn
fnLock
@@ -118,17 +177,38 @@ gameButtonThumbRight
gameButtonX
gameButtonY
gameButtonZ
+goBack
+goHome
+greater
+groupFirst
+groupLast
+groupNext
+groupPrevious
+guide
+guideNextDay
+guidePreviousDay
+hangulMode
+hanjaMode
+hankaku
+headsetHook
help
+hibernate
+hiragana
+hiraganaKatakana
home
hyper
info
insert
+instantReplay
intlBackslash
intlRo
intlYen
+junjaMode
kanaMode
-kbdIllumDown
-kbdIllumUp
+kanjiMode
+katakana
+key11
+key12
keyA
keyB
keyC
@@ -155,32 +235,40 @@ keyW
keyX
keyY
keyZ
-keyboardLayoutSelect
lang1
lang2
lang3
lang4
lang5
-launchApp1
-launchApp2
+lastNumberRedial
+launchApplication1
+launchApplication2
launchAssistant
-launchAudioBrowser
launchCalendar
launchContacts
launchControlPanel
-launchDocuments
-launchInternetBrowser
-launchKeyboardLayout
launchMail
+launchMediaPlayer
+launchMusicPlayer
launchPhone
launchScreenSaver
launchSpreadsheet
+launchWebBrowser
+launchWebCam
launchWordProcessor
-lockScreen
+less
+link
+listProgram
+liveContent
+lock
logOff
mailForward
mailReply
mailSend
+mannerMode
+mediaApps
+mediaAudioTrack
+mediaClose
mediaFastForward
mediaLast
mediaPause
@@ -188,18 +276,37 @@ mediaPlay
mediaPlayPause
mediaRecord
mediaRewind
-mediaSelect
+mediaSkip
+mediaSkipBackward
+mediaSkipForward
+mediaStepBackward
+mediaStepForward
mediaStop
+mediaTopMenu
mediaTrackNext
mediaTrackPrevious
meta
metaLeft
metaRight
+microphoneToggle
+microphoneVolumeDown
+microphoneVolumeMute
+microphoneVolumeUp
minus
+modeChange
+navigateIn
+navigateNext
+navigateOut
+navigatePrevious
newKey
+nextCandidate
+nextFavoriteChannel
+nextUserProfile
nonConvert
none
+notification
numLock
+numberSign
numpad0
numpad1
numpad2
@@ -211,63 +318,128 @@ numpad7
numpad8
numpad9
numpadAdd
-numpadBackspace
-numpadClear
-numpadClearEntry
numpadComma
numpadDecimal
numpadDivide
numpadEnter
numpadEqual
-numpadMemoryAdd
-numpadMemoryClear
-numpadMemoryRecall
-numpadMemoryStore
-numpadMemorySubtract
numpadMultiply
numpadParenLeft
numpadParenRight
-numpadSignChange
numpadSubtract
+onDemand
open
+pInPDown
+pInPMove
+pInPToggle
+pInPUp
pageDown
pageUp
+pairing
+parenthesisLeft
+parenthesisRight
paste
pause
+percent
period
+play
+playSpeedDown
+playSpeedReset
+playSpeedUp
power
+powerOff
+previousCandidate
print
printScreen
-privacyScreenToggle
-programGuide
+process
props
+question
quote
+quoteSingle
+randomToggle
+rcLowBattery
+recordSpeedNext
redo
resume
+rfBypass
+romaji
save
+scanChannelsToggle
+screenModeNext
scrollLock
select
-selectTask
semicolon
+settings
shift
shiftLeft
+shiftLevel5
shiftRight
-showAllWindows
+singleCandidate
slash
sleep
+soft1
+soft2
+soft3
+soft4
+soft5
+soft6
+soft7
+soft8
space
+speechCorrectionList
speechInputToggle
spellCheck
+splitScreenToggle
+standby
+stbInput
+stbPower
+subtitle
superKey
suspend
+symbol
+symbolLock
tab
-turbo
+teletext
+tilde
+tv
+tv3DMode
+tvAntennaCable
+tvAudioDescription
+tvAudioDescriptionMixDown
+tvAudioDescriptionMixUp
+tvContentsMenu
+tvDataService
+tvInput
+tvInputComponent1
+tvInputComponent2
+tvInputComposite1
+tvInputComposite2
+tvInputHDMI1
+tvInputHDMI2
+tvInputHDMI3
+tvInputHDMI4
+tvInputVGA1
+tvMediaContext
+tvNetwork
+tvNumberEntry
+tvPower
+tvRadioService
+tvSatellite
+tvSatelliteBS
+tvSatelliteCS
+tvSatelliteToggle
+tvTerrestrialAnalog
+tvTerrestrialDigital
+tvTimer
+underscore
undo
-usbErrorRollOver
-usbErrorUndefined
-usbPostFail
-usbReserved
+unidentified
+videoModeNext
+voiceDial
wakeUp
+wink
+zenkaku
+zenkakuHankaku
zoomIn
zoomOut
zoomToggle |
I noticed that tab traversal stopped working on iOS when #73440 was landed. It seems to be that the control characters were excluded from the list of logical keys, and so they ended up being generated values. This PR just adds "Enter" and "Tab" to the list for both macOS and iOS.
dkwingsmt
added
the
a: text input
2 participants
Description
This PR rewrites
tools/gen_keycodes, the script that generates key mappings across the framework and the engine.For framework
Regarding framework, this PR:
The reasons for this change will be state below.
For the codegen script
Regarding the script, some major changes include:
Split physical keys and logical keys. This is the major reason why this PR needs to be a rewrite. Previously the list of physical keys and logical keys are identical, both generated from Chromium's
dom_code_data.inc. This is incorrect since physical keys are merely mnemonics of USB HID usages, while logical keys represent the actual effect of a key. They are fundamentally independent, both their values and their mappings.With this PR, the logical keys are now collected from
dom_key_data.incinstead. To accomplish this,KeyDataandKeyEntryare split into physical and logical versions, and the values of all logical keys (which were calculated from USB HID code) are redefined.Implemented engine mappings. The code used to generate engine mappings for various platforms has been added. The result of these code has been submitted in various engine PRs including flutter/engine#23466 (Web), flutter/engine#23465 (Windows), flutter/engine#23469 (macOS) and flutter/engine#23467 (Linux GTK). This PR publishes their generation logic.
Engineering enhances. This PR converts the entire script into null-safety; converts
KeyEntryandKeyDatainto immutable classes; enables assertions and applies assertions across the script; document all input files indata/README.md.Other notable changes include:
Removed most file-specifying options. Most options to specify input files are removed. I don't find them useful at all, and they greatly increases the difficulty of maintenance, especially with a lot more input files after this PR.
Modified logical value masks. A few masks are added, such as "leftModifier" "rightModifier" "keyPad". The "synonym" bit is changed.
Fixed RawKeyboard logic. How
RawKeyboard.logicalKeyis derived on iOS and macOS is fixed to query the mapping table first.Is it breaking?
This PR is written with minimizing breakage in mind. The most likely breakages are that some rarely used logical keys are removed. The change to the list of logical keys can be seen at #73440 (comment).
Although the values of logical keys are redesigned, since their mappings are rewritten accordingly, no actual breakages should occur unless some code is using hard-coded hex values.
Related Issues
Tests
I added the following tests:
This PR doesn't needs tests.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.