Skip to content

Keyboard: don't auto-toggle CODE/KANA lock while that key is held down - #7

Closed
sndpl wants to merge 119 commits into
masterfrom
keyboard-code-kana-lock-desync
Closed

sndpl wants to merge 119 commits into
masterfrom
keyboard-code-kana-lock-desync

Conversation

@sndpl

@sndpl sndpl commented Jul 25, 2026 •

Copy link
Copy Markdown
Owner

On a machine where CODE/KANA locks (e.g. a Japanese MSX), the lock can get
permanently out of sync with what openMSX thinks it is, after which it can no
longer be switched off:

  • tap the host CODE/KANA key -> the KANA LED goes off, but
  • the very next ordinary keystroke turns it straight back on again.

The only way out is restarting openMSX; a machine reset does not help, because
locksOn is never reset while the MSX does clear its own state. This is easy
to run into on host layouts where AltGr has to be held down to type common
characters such as @, # or ~, because the host CODE/KANA key defaults to
right-Alt.

Cause

Keyboard::pressUnicodeByUser() auto-toggles the CODE/KANA lock by flipping
locksOn and calling pressKeyMatrixEvent() for the CODE key. That call is a
no-op when the key is already down (it deliberately bails out when pressing
would not change the matrix).

So while the user physically holds the host CODE/KANA key, the MSX never sees
a new key-press edge and never toggles its lock, but openMSX flips locksOn
anyway. From then on the two are inverted, and because needsLockToggle() now
reports the wrong direction, every following keystroke auto-toggles the real
lock the wrong way.

Fix

Only take the auto-toggle path when the CODE key is currently released, so the
press really produces an edge the MSX can act on. When the user is holding the
key, just type the character with CODE/KANA held down, which is what a real
MSX does.

The "already pressed" test is extracted from pressKeyMatrixEvent() into
isKeyMatrixPressed() so both call sites share it. When the CODE key is not
held - the common case - behaviour is unchanged.

How it was verified

Driven against a real emulator run (-control stdio) with synthetic host key
events posted through the OS, observing $led_kana, i.e. the actual KANA LED
as driven by the emulated MSX via PSG register 15. Every injected press and
release is confirmed against debug read keymatrix <row> before the next step,
so a dropped host event cannot silently produce a wrong trace.

Note: on macOS right-Alt is Option, which mangles the unicode of the following
key, so the character never reaches the unicode mapping path. Binding
kbd_code_kana_host_key to END hits exactly the same code path with an
unmangled character, which is what AltGr+3=# does on a Windows/Spanish
keyboard.

Machine Canon_V-20_JP, kbd_code_kana_host_key END,
kbd_auto_toggle_code_kana_lock on. Steps:

1 tap END               6 tap END
2 tap END               7 type 'a'
3 type 'a'              8 type 'a'
4 hold END, 'a', release 9 type 'a'
5 type 'a'

KANA LED after each step, for all three mapping modes:

                init   1    2    3    4    5    6    7    8    9
before
  CHARACTER     off    on   off  off  on   on   off  on   on   on
  KEY           off    on   off  off  on   on   off  off  off  off
  POSITIONAL    off    on   off  off  on   on   off  off  off  off
after
  CHARACTER     off    on   off  off  on   off  on   off  off  off
  KEY           off    on   off  off  on   on   off  off  off  off
  POSITIONAL    off    on   off  off  on   on   off  off  off  off

In CHARACTER mode before the fix, steps 7-9 are the bug: a plain letter
switches the KANA lock back on, and it stays stuck that way. Step 5 shows the
same desync from the other side - the lock is on and typing a plain letter
should auto-toggle it off, but openMSX already believes it is off so nothing
happens. After the fix step 5 auto-toggles correctly and steps 7-9 leave the
lock off.

KEY and POSITIONAL are byte-identical before and after, as expected:
processKeyEvent() forces unicode = 0 for those two modes, so
pressUnicodeByUser() - and with it the changed branch - is unreachable there.
Those traces do exercise the isKeyMatrixPressed() extraction heavily though,
since every keystroke in those modes goes through pressKeyMatrixEvent() via
processSdlKey().

Also checked to be unchanged:

  • default right-Alt binding behaves exactly as before;
  • the auto-toggle still does nothing when kbd_auto_toggle_code_kana_lock
    is off;
  • plain, shifted and CODE-held typing on screen: helo, HELO, and kana
    glyphs while CODE/KANA is held (which is correct MSX behaviour).

Not covered here

Two separate things that share the same trigger but have different causes, and
that I cannot test from macOS:

  • on Windows, AltGr additionally emits a synthetic left-Ctrl, which openMSX
    maps to the MSX CTRL key (so right-Alt+G beeps). That needs a fix in the
    Windows event path.
  • on macOS, Option acts as a dead-key/compose modifier, so the character
    following it is altered by the host before openMSX sees it.

sndpl added 30 commits January 11, 2026 14:02
Vampier and others added 25 commits June 16, 2026 19:54
…s+additions.

Signed-off-by: Vampier <vampiermsx@gmail.com>
With a <level_2_via> tag. According to the discussion in:
  openMSX#2138

I didn't do _any_ testing yet. I ran out of time and did already want to
push this experiment. I'll continue later, and then ask for help with
adjusting all the machine/extension configs.

Summary: added a new tag <level_2_via> with possible values:
* read   (=default)
* write
* interlocked_write_read
* only_level_1
"read" implies "shared_address=false", "shared_counter=false", the other
modes have "true". Open question, maybe this "read" mode can be dropped?

I only adjusted the machine configs
* Sanyo_PHC-70FD2.xml
* Sony_HB-F1XDJ.xml
* Yamaha_YIS-805-256.xml
See again: openMSX#2138

Apparently also 'level_2_by=read' must have a shared address/counter
(because of measurements on 'Sony HB-F1XDJ').

I realized that we can greatly simplify our implementation by always
assuming a shared address/counter. As there are no known non-emulator
implementations anymore without sharing. And if we ever do need an
unshared version again, then we can instantiate the MSXKanji device
twice: once on ports 0xd8/0xd9 and once on ports 0xda/0xdb.
(After IRC discussion)

Reading from the level2 ports when only level1 ROM is present (128kB
ROM, not 256kB) should return 0xff. Before we mistakenly thought it
would mirror level1 content.
Includes the used test program and explanation.

Commit for bsittler. Thank you so much!
Again a commit for bsittler. Thanks again!
Background info (at least how I understand it currently):
* Historically (openGL 1 and 2) textures object contain both the image
  data and sampling properties like filtering (nearest neighbour,
  bilinear interpolation, ...) or wrapping mode (tile, clamp to edge,
  ...), ...
* From openGL 3 onwards it became possible to separate the texture data
  (the image itself) from the sampling-parameters. The latter is a then
  a "sampler object".

OpenMSX is mostly still only using openGL2 features, so we still set the
sampling parameters on the texture objects.

Recently Dear ImGui started using openGL sampler objects (we got this
change since 4644985 when we update our Dear ImGui version).
Unfortunately this broke a few things:

For example the "Bitmap Viewer" debug window explicitly uses "nearest
neighbour" filtering to show the zoomed-in image. Before that worked
because we set the filtering mode on our texture (which we then draw
with ImGui::Image()). But the new ImGui version has a sampler object
active which overwrites the filtering mode to 'bilinear interpolation',
resuling in a blurry zoomed image.

Another example is drawing the grid on top of the zoomed image. We did
that via a texture containing a single grid-cell, and then using the
tile-wrapping mode on that texture. Though again because there's now a
sampler active it overrules that mode and uses clamp-to-edge.

This patch (hopefully) temporarily disables the new Dear ImGui behavior
(it forces the openGL2 fallback path where openGL sampler objects didn't
exist yet). Though there's probably a good reason why Dear ImGui want to
use sampler objects (and we should try to avoid patching the Dear ImGui
code .. makes upgrading to newer versions harder). So we should find
better solutions. ATM I have some ideas, but I need more time to work
them out.
)

A Hit Bit U owner has run the test and provided me with screen photos that show this behavior.

Co-authored-by: Benjamin C. Wiley Sittler <bsittler@gmail.com>
Due to incomplete constexpr support for std::string in the current C++ code I need to through some hoops here (thanks to Gemini) to get this constexpr. But I think I made it. If someone has a better idea on how to solve this, please change the code :)

Fixes openMSX#2149.
Tiny improvement on previous commit. More lines, but overall cleaner...
… older versions to reduce confusion.

This version newly incorporates the `BACONLDR.BIN` loader+runtime library for the (MIT-licensed) [MSX-BACON](https://github.com/hra1129/msx_basic_compiler) BASIC compiler, which is used to build a precompiled version of the test suite. This precompiled version loads and runs quite a bit faster on machines with disks and sufficient RAM (64K or more). This precompiled version of the test suite is assembled using [zma](https://github.com/hra1129/zma). To rebuild the precompiled test suite requires an MSX-BACON compiler bugfix from hra1129/msx_basic_compiler#32 to make `INP(`...`)` usable. Prebuilt binaries are included to ease testing on real hardware.

A small, standalone public domain Python CAS-to-WAV converter generated by Google's Gemini search assistant is also included, and was used to generate the WAV version of the cassette test suite from the CAS image.

The cassette version of the test suite excludes the precompiled MSX-BACON version as it would complicate cassette loading.

The X-BASIC / BASIC'n / MSXべーしっ君 `_TURBO` compiler is still recommended when loading from cassette, and for loading from disk in machines with 32KB of Z80 RAM. Machines with only 16KB of Z80 RAM are stuck with slow interpreted MSX-BASIC. 8KB of Z80 RAM isn't enough to run these tests.

This is related to openMSX#2138
Fix KEYPAD_COMMA input bug introduced in d24d37d and add POSITIONAL mode support

Use a dummy value for SCANCODE as it is not used in unit tests.
Changed the mouse speed correction from a fixed 1/2 multiplier to a ratio based on the display size and the MSX screen internal resolution (240).
Thanks uniskie for reporting!
    openMSX#2154

We added backwards compatibility serialization code in MSXKanji, but
then forgot to bump the version numberr, oops.
…penMSX into uniskie-Mouse_speed_adjustment_feature
National FS-4000 I/O port kanji behavior is confirmed by Jun Nishikawa using kanji test v3.1

see openMSX#2138 (comment)

this already happened to be how openMSX emulated this machine, but this makes it explicit and documents the behavior as matching test results
For discussion and background info see:
    openMSX#2152

This commit only changes the implementation of the `getUserHomeDir()`
function (which is used by tilde expansion). It does not yet do a full
audit of all openMSX commands that use filenames. That's for follow-up
patch(es).
See:
    openMSX#2157

In 'fast blink mode' (undocumented bit 2 in R#2) the blink status
changes per line instead of per frame. This was already implemented for
the bitmap modes. This commit also adds support for the TEXT2 mode
(screen 0, width 80).
Thanks to:
  openMSX#2057   (measurement tools by bengalack)
  #5          (analysis done by Claude (Opus), an AI assistant)
See comments in code for details. Read the discussion in the above
links for even more details.

@MBilderbeek MBilderbeek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good, but I think it's better to move the comment to the header file and put it in the right (Doxygen) format.

Comment thread src/input/Keyboard.cc Outdated

/*
* Is the given key currently held down in the MSX keyboard matrix?
* If it is, pressing it again won't produce a new key-press edge for the MSX.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe better to put method documentation in the header file? In Doxygen format, like we do for all methods?

On a machine where CODE/KANA locks, pressUnicodeByUser() can auto-toggle the
lock by flipping 'locksOn' and pressing the CODE key in the matrix. But
pressKeyMatrixEvent() is a no-op when the key is already down, so when the
user is physically holding the host CODE/KANA key (by default right-Alt,
which on many host layouts is AltGr and must be held to type '@', '#', ...)
the MSX never sees a new key-press edge and never toggles its lock, while
openMSX did flip 'locksOn'.

From that moment on the two are inverted and stay inverted: tapping the
host key appears to switch the lock off, but the next ordinary keystroke
auto-toggles it straight back on, and there is no way to get out of it
short of restarting openMSX. A reset does not help, since 'locksOn' is
not reset while the MSX does clear its own state.

Only take the auto-toggle path when the CODE key is currently released, so
that the press really produces an edge for the MSX. When the user is holding
the key, just type the character with CODE/KANA held, like a real MSX does.

Extract the "already pressed" test from pressKeyMatrixEvent() into
isKeyMatrixPressed() so both places share it.
@sndpl
sndpl force-pushed the keyboard-code-kana-lock-desync branch from c7deb74 to 8c07a58 Compare July 25, 2026 21:31
@sndpl

sndpl commented Jul 25, 2026

Copy link
Copy Markdown
Owner Author

Superseded by openMSX#2158, which targets the upstream repo. The documentation comment has been moved to the header in Doxygen format there, and both commits are squashed into one.

@sndpl sndpl closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants