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

feat(windows): Investigate if VKContextReset is needed in for core on kmx processor #10227

Closed
2 tasks
rc-swag opened this issue Dec 12, 2023 · 3 comments · Fixed by #11172
Closed
2 tasks

feat(windows): Investigate if VKContextReset is needed in for core on kmx processor #10227

rc-swag opened this issue Dec 12, 2023 · 3 comments · Fixed by #11172
Assignees
Milestone

Comments

@rc-swag
Copy link
Contributor

rc-swag commented Dec 12, 2023

In reviewing #10090 it become apparent that the call to reset the context in the windows engine base upon the table VKContextReset was no longer used. This ticket is to investigate if it is still needed if so integrated into the core. Also remove the Array from the Windows engine.

  • Remove extern const int VKContextReset[256]; from the Windows Engine

  • Store palce const int VKContextReset[256]; in the Core if it is needed

@rc-swag
Copy link
Contributor Author

rc-swag commented Dec 12, 2023

I have tested some of the keys by running keyman. However, it is hard to test when the context would be reset anyway (focus changing). The pause key seemed to follow and the escape key but others I couldn't be sure. It might be best to have a core level unit test.

@rc-swag rc-swag added this to the B17S2 milestone Jan 8, 2024
@mcdurdin mcdurdin modified the milestones: B17S2, B17S3 Mar 3, 2024
@darcywong00 darcywong00 modified the milestones: B17S3, B17S4 Mar 16, 2024
@rc-swag
Copy link
Contributor Author

rc-swag commented Mar 25, 2024

I have run the test host program and tested the keys available to me on my keyboard. There are a number of keys that reset the context that had a value of '0' in the VKContextReset array. See --- Reset Context when not expected by VKContextReset. I tested using the Euro Latin keyboard therefore using the kmx keyboard processor.
The key to be determined whether we reset the context for the KMX processor seems to be line 262 of kmx_processevent.cpp if(!gp || (m_state.charCode == 0 && gp->fUsingKeys)) plus it not being a backspace. This essentially means it is reset if the key is not a charCode according to this struct char_to_vkey

Note: This table is replicated in the core vkey_to_contextreset kmx_processevent.h It is never called now even by the ldml_processor.

1,   //L"K_BKSP",              // &H8
1,   //L"K_TAB",               // &H9

1,   //L"K_ENTER",             // &HD

0,   //L"K_SHIFT",           // &H10
0,   //L"K_CONTROL",    // &H11
0,   //L"K_ALT",                // &H12
1,   //L"K_PAUSE",              // &H13
0,   //L"K_CAPS",               // &H14

1,   //L"K_ESC",                // &H1B
0,   //L"K_SPACE",              // &H20
1,   //L"K_PGUP",               // &H21
1,   //L"K_PGDN",               // &H22
1,   //L"K_END",                // &H23
1,   //L"K_HOME",               // &H24
1,   //L"K_LEFT",               // &H25
1,   //L"K_UP",                 // &H26
1,   //L"K_RIGHT",              // &H27
1,   //L"K_DOWN",               // &H28

0,   //L"K_INS",                // &H2D                     ----- RESET Context
1,   //L"K_DEL",                // &H2E
0,   //L"K_0",                  // &H30
0,   //L"K_1",                  // &H31
0,   //L"K_2",                  // &H32
0,   //L"K_3",                  // &H33
0,   //L"K_4",                  // &H34
0,   //L"K_5",                  // &H35
0,   //L"K_6",                  // &H36
0,   //L"K_7",                  // &H37
0,   //L"K_8",                  // &H38
0,   //L"K_9",                  // &H39


0,   //L"K_NPSTAR",             // &H6A            ----- RESET Context
0,   //L"K_NPPLUS",             // &H6B             ----- RESET Context
0,   //L"K_SEPARATOR",          // &H6C         -------couldn't test
0,   //L"K_NPMINUS",            // &H6D           ----- RESET Context
0,   //L"K_NPDOT",              // &H6E           ----- RESET Context
0,   //L"K_NPSLASH",            // &H6F           ----- RESET Context
1,   //L"K_F1",                 // &H70
1,   //L"K_F2",                 // &H71
1,   //L"K_F3",                 // &H72
1,   //L"K_F4",                 // &H73
1,   //L"K_F5",                 // &H74
1,   //L"K_F6",                 // &H75
1,   //L"K_F7",                 // &H76
1,   //L"K_F8",                 // &H77
1,   //L"K_F9",                 // &H78
1,   //L"K_F10",                // &H79
1,   //L"K_F11",                // &H7A
1,   //L"K_F12",                // &H7B


0,   //L"K_NUMLOCK",            // &H90        ----- Didn't even go to engine
0,   //L"K_SCROLL",             // &H91         ----- RESET Context

0,   //L"K_COLON",              // &HBA
0,   //L"K_EQUAL",              // &HBB
0,   //L"K_COMMA",              // &HBC
0,   //L"K_HYPHEN",             // &HBD
0,   //L"K_PERIOD",             // &HBE
0,   //L"K_SLASH",              // &HBF
0,   //L"K_BKQUOTE",            // &HC0

@rc-swag
Copy link
Contributor Author

rc-swag commented Mar 25, 2024

I ended up adding logging to test the LDML processor. For non-compliant apps (non-context aware), it is never resetting context for a key press from the list above (well for any key). I realise now this has occurred because the action struct that the LDML processor is giving to the core does not have the option to invalidate the context.

Both LDML and KMX never inserted the \ - * keys from the number pad into the context, I am pretty sure they both are hitting the emit keystroke code. However, KMX resets context where as LDML does not.

We have 2 issues.

  1. KMX does resets context if the key is not a char code according to char_to_vkey this maybe acceptable the differences are the numlock * - \ keys and the scroll lock, and ins key.
  2. The action struct from processor to core does not provide a mechanism for invalidating/resetting the context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants