Skip to content

Commit

Permalink
Replace IInputEvent with INPUT_RECORD (#15673)
Browse files Browse the repository at this point in the history
`IInputEvent` makes adding Unicode support to `InputBuffer` more
difficult than necessary as the abstract class makes downcasting
as well as copying quite verbose. I found that using `INPUT_RECORD`s
directly leads to a significantly simplified implementation.

In addition, this commit fixes at least one bug: The previous approach
to detect the null key via `DoActiveModifierKeysMatch` didn't work.
As it compared the modifier keys as a bitset with `==` it failed to
match whenever the numpad key was set, which it usually is.

## Validation Steps Performed
* Unit and feature tests are ✅
  • Loading branch information
lhecker committed Aug 11, 2023
1 parent e9c8391 commit 5b44476
Show file tree
Hide file tree
Showing 54 changed files with 786 additions and 2,278 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.cpp
Expand Up @@ -511,7 +511,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// modifier key. We'll wait for a real keystroke to dismiss the
// GH #7395 - don't update selection when taking PrintScreen
// selection.
return HasSelection() && !KeyEvent::IsModifierKey(vkey) && vkey != VK_SNAPSHOT;
return HasSelection() && ::Microsoft::Terminal::Core::Terminal::IsInputKey(vkey);
}

bool ControlCore::TryMarkModeKeybinding(const WORD vkey,
Expand Down
10 changes: 5 additions & 5 deletions src/cascadia/TerminalCore/Terminal.cpp
Expand Up @@ -661,7 +661,7 @@ bool Terminal::SendKeyEvent(const WORD vkey,
// modifier key. We'll wait for a real keystroke to snap to the bottom.
// GH#6481 - Additionally, make sure the key was actually pressed. This
// check will make sure we behave the same as before GH#6309
if (!KeyEvent::IsModifierKey(vkey) && keyDown)
if (IsInputKey(vkey) && keyDown)
{
TrySnapOnInput();
}
Expand Down Expand Up @@ -714,8 +714,8 @@ bool Terminal::SendKeyEvent(const WORD vkey,
return false;
}

const KeyEvent keyEv{ keyDown, 1, vkey, sc, ch, states.Value() };
return _handleTerminalInputResult(_terminalInput.HandleKey(&keyEv));
const auto keyEv = SynthesizeKeyEvent(keyDown, 1, vkey, sc, ch, states.Value());
return _handleTerminalInputResult(_terminalInput.HandleKey(keyEv));
}

// Method Description:
Expand Down Expand Up @@ -791,8 +791,8 @@ bool Terminal::SendCharEvent(const wchar_t ch, const WORD scanCode, const Contro
MarkOutputStart();
}

const KeyEvent keyDown{ true, 1, vkey, scanCode, ch, states.Value() };
return _handleTerminalInputResult(_terminalInput.HandleKey(&keyDown));
const auto keyDown = SynthesizeKeyEvent(true, 1, vkey, scanCode, ch, states.Value());
return _handleTerminalInputResult(_terminalInput.HandleKey(keyDown));
}

// Method Description:
Expand Down
16 changes: 16 additions & 0 deletions src/cascadia/TerminalCore/Terminal.hpp
Expand Up @@ -60,6 +60,22 @@ class Microsoft::Terminal::Core::Terminal final :
using RenderSettings = Microsoft::Console::Render::RenderSettings;

public:
static constexpr bool IsInputKey(WORD vkey)
{
return vkey != VK_CONTROL &&
vkey != VK_LCONTROL &&
vkey != VK_RCONTROL &&
vkey != VK_MENU &&
vkey != VK_LMENU &&
vkey != VK_RMENU &&
vkey != VK_SHIFT &&
vkey != VK_LSHIFT &&
vkey != VK_RSHIFT &&
vkey != VK_LWIN &&
vkey != VK_RWIN &&
vkey != VK_SNAPSHOT;
}

Terminal();

void Create(til::size viewportSize,
Expand Down
2 changes: 2 additions & 0 deletions src/host/cmdline.h
Expand Up @@ -87,4 +87,6 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData);
bool IsWordDelim(const wchar_t wch);
bool IsWordDelim(const std::wstring_view charData);

bool IsValidStringBuffer(_In_ bool Unicode, _In_reads_bytes_(Size) PVOID Buffer, _In_ ULONG Size, _In_ ULONG Count, ...);

void SetCurrentCommandLine(COOKED_READ_DATA& cookedReadData, _In_ CommandHistory::Index Index);
13 changes: 4 additions & 9 deletions src/host/conimeinfo.cpp
Expand Up @@ -462,18 +462,13 @@ void ConsoleImeInfo::_InsertConvertedString(const std::wstring_view text)
}

const auto dwControlKeyState = GetControlKeyState(0);
std::deque<std::unique_ptr<IInputEvent>> inEvents;
KeyEvent keyEvent{ TRUE, // keydown
1, // repeatCount
0, // virtualKeyCode
0, // virtualScanCode
0, // charData
dwControlKeyState }; // activeModifierKeys
InputEventQueue inEvents;
auto keyEvent = SynthesizeKeyEvent(true, 1, 0, 0, 0, dwControlKeyState);

for (const auto& ch : text)
{
keyEvent.SetCharData(ch);
inEvents.push_back(std::make_unique<KeyEvent>(keyEvent));
keyEvent.Event.KeyEvent.uChar.UnicodeChar = ch;
inEvents.push_back(keyEvent);
}

gci.pInputBuffer->Write(inEvents);
Expand Down
15 changes: 6 additions & 9 deletions src/host/directio.cpp
Expand Up @@ -101,7 +101,7 @@ using Microsoft::Console::Interactivity::ServiceLocator;
// Return Value:
// - HRESULT indicating success or failure
[[nodiscard]] static HRESULT _WriteConsoleInputWImplHelper(InputBuffer& context,
std::deque<std::unique_ptr<IInputEvent>>& events,
const std::span<const INPUT_RECORD>& events,
size_t& written,
const bool append) noexcept
{
Expand Down Expand Up @@ -151,7 +151,7 @@ try
auto Unlock = wil::scope_exit([&] { UnlockConsole(); });

const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
til::small_vector<INPUT_RECORD, 16> events;
InputEventQueue events;

auto it = buffer.begin();
const auto end = buffer.end();
Expand All @@ -160,7 +160,7 @@ try
// the next call to WriteConsoleInputAImpl to join it with the now available trailing DBCS.
if (context.IsWritePartialByteSequenceAvailable())
{
auto lead = context.FetchWritePartialByteSequence(false)->ToInputRecord();
auto lead = context.FetchWritePartialByteSequence();
const auto& trail = *it;

if (trail.EventType == KEY_EVENT)
Expand Down Expand Up @@ -200,7 +200,7 @@ try
if (it == end)
{
// Missing trailing DBCS -> Store the lead for the next call to WriteConsoleInputAImpl.
context.StoreWritePartialByteSequence(IInputEvent::Create(lead));
context.StoreWritePartialByteSequence(lead);
break;
}

Expand All @@ -225,8 +225,7 @@ try
}
}

auto result = IInputEvent::Create(std::span{ events.data(), events.size() });
return _WriteConsoleInputWImplHelper(context, result, written, append);
return _WriteConsoleInputWImplHelper(context, events, written, append);
}
CATCH_RETURN();

Expand All @@ -252,9 +251,7 @@ CATCH_RETURN();

try
{
auto events = IInputEvent::Create(buffer);

return _WriteConsoleInputWImplHelper(context, events, written, append);
return _WriteConsoleInputWImplHelper(context, buffer, written, append);
}
CATCH_RETURN();
}
Expand Down
6 changes: 3 additions & 3 deletions src/host/ft_host/CJK_DbcsTests.cpp
Expand Up @@ -1919,11 +1919,11 @@ void DbcsTests::TestMultibyteInputCoalescing()

DWORD count;
{
const auto record = KeyEvent{ true, 1, 123, 456, 0x82, 789 }.ToInputRecord();
const auto record = SynthesizeKeyEvent(true, 1, 123, 456, 0x82, 789);
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleInputA(in, &record, 1, &count));
}
{
const auto record = KeyEvent{ true, 1, 234, 567, 0xA2, 890 }.ToInputRecord();
const auto record = SynthesizeKeyEvent(true, 1, 234, 567, 0xA2, 890);
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleInputA(in, &record, 1, &count));
}

Expand All @@ -1933,7 +1933,7 @@ void DbcsTests::TestMultibyteInputCoalescing()
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleInputW(in, &actual[0], 2, &count));
VERIFY_ARE_EQUAL(1u, count);

const auto expected = KeyEvent{ true, 1, 123, 456, L'', 789 }.ToInputRecord();
const auto expected = SynthesizeKeyEvent(true, 1, 123, 456, L'', 789);
VERIFY_ARE_EQUAL(expected, actual[0]);
}

Expand Down
1 change: 0 additions & 1 deletion src/host/ft_host/Message_KeyPressTests.cpp
Expand Up @@ -86,7 +86,6 @@ class KeyPressTests
expectedRecord.Event.KeyEvent.uChar.UnicodeChar = 0x0;
expectedRecord.Event.KeyEvent.bKeyDown = true;
expectedRecord.Event.KeyEvent.dwControlKeyState = ENHANCED_KEY;
expectedRecord.Event.KeyEvent.dwControlKeyState |= (GetKeyState(VK_NUMLOCK) & KEY_STATE_TOGGLED) ? NUMLOCK_ON : 0;
expectedRecord.Event.KeyEvent.wRepeatCount = SINGLE_KEY_REPEAT;
expectedRecord.Event.KeyEvent.wVirtualKeyCode = VK_APPS;
expectedRecord.Event.KeyEvent.wVirtualScanCode = (WORD)scanCode;
Expand Down
39 changes: 16 additions & 23 deletions src/host/input.cpp
Expand Up @@ -105,17 +105,18 @@ bool ShouldTakeOverKeyboardShortcuts()

// Routine Description:
// - handles key events without reference to Win32 elements.
void HandleGenericKeyEvent(_In_ KeyEvent keyEvent, const bool generateBreak)
void HandleGenericKeyEvent(INPUT_RECORD event, const bool generateBreak)
{
auto& keyEvent = event.Event.KeyEvent;
const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto ContinueProcessing = true;

if (keyEvent.IsCtrlPressed() &&
!keyEvent.IsAltPressed() &&
keyEvent.IsKeyDown())
if (WI_IsAnyFlagSet(keyEvent.dwControlKeyState, CTRL_PRESSED) &&
WI_AreAllFlagsClear(keyEvent.dwControlKeyState, ALT_PRESSED) &&
keyEvent.bKeyDown)
{
// check for ctrl-c, if in line input mode.
if (keyEvent.GetVirtualKeyCode() == 'C' && IsInProcessedInputMode())
if (keyEvent.wVirtualKeyCode == 'C' && IsInProcessedInputMode())
{
HandleCtrlEvent(CTRL_C_EVENT);
if (gci.PopupCount == 0)
Expand All @@ -130,7 +131,7 @@ void HandleGenericKeyEvent(_In_ KeyEvent keyEvent, const bool generateBreak)
}

// Check for ctrl-break.
else if (keyEvent.GetVirtualKeyCode() == VK_CANCEL)
else if (keyEvent.wVirtualKeyCode == VK_CANCEL)
{
gci.pInputBuffer->Flush();
HandleCtrlEvent(CTRL_BREAK_EVENT);
Expand All @@ -146,33 +147,25 @@ void HandleGenericKeyEvent(_In_ KeyEvent keyEvent, const bool generateBreak)
}

// don't write ctrl-esc to the input buffer
else if (keyEvent.GetVirtualKeyCode() == VK_ESCAPE)
else if (keyEvent.wVirtualKeyCode == VK_ESCAPE)
{
ContinueProcessing = false;
}
}
else if (keyEvent.IsAltPressed() &&
keyEvent.IsKeyDown() &&
keyEvent.GetVirtualKeyCode() == VK_ESCAPE)
else if (WI_IsAnyFlagSet(keyEvent.dwControlKeyState, ALT_PRESSED) &&
keyEvent.bKeyDown &&
keyEvent.wVirtualKeyCode == VK_ESCAPE)
{
ContinueProcessing = false;
}

if (ContinueProcessing)
{
size_t EventsWritten = 0;
try
gci.pInputBuffer->Write(event);
if (generateBreak)
{
EventsWritten = gci.pInputBuffer->Write(std::make_unique<KeyEvent>(keyEvent));
if (EventsWritten && generateBreak)
{
keyEvent.SetKeyDown(false);
EventsWritten = gci.pInputBuffer->Write(std::make_unique<KeyEvent>(keyEvent));
}
}
catch (...)
{
LOG_HR(wil::ResultFromCaughtException());
keyEvent.bKeyDown = false;
gci.pInputBuffer->Write(event);
}
}
}
Expand Down Expand Up @@ -210,7 +203,7 @@ void HandleMenuEvent(const DWORD wParam)
size_t EventsWritten = 0;
try
{
EventsWritten = gci.pInputBuffer->Write(std::make_unique<MenuEvent>(wParam));
EventsWritten = gci.pInputBuffer->Write(SynthesizeMenuEvent(wParam));
if (EventsWritten != 1)
{
RIPMSG0(RIP_WARNING, "PutInputInBuffer: EventsWritten != 1, 1 expected");
Expand Down
2 changes: 1 addition & 1 deletion src/host/input.h
Expand Up @@ -78,7 +78,7 @@ bool ShouldTakeOverKeyboardShortcuts();
void HandleMenuEvent(const DWORD wParam);
void HandleFocusEvent(const BOOL fSetFocus);
void HandleCtrlEvent(const DWORD EventType);
void HandleGenericKeyEvent(_In_ KeyEvent keyEvent, const bool generateBreak);
void HandleGenericKeyEvent(INPUT_RECORD event, const bool generateBreak);

void ProcessCtrlEvents();

Expand Down

0 comments on commit 5b44476

Please sign in to comment.