Skip to content

Commit

Permalink
PR 7101178: Remove the OneCore redirects for MapVkey/VkeyScan/GetKeyS…
Browse files Browse the repository at this point in the history
…tate

In !1806141, MapVirtualKey and VkKeyScan became part of OneCore (yeah,
in 2018!). GetKeyState I can't quite figure out... but it looks like
the places we would use it (Win32 Window and Selection) already either
don't exist (window) or don't work (selection) in OneCore conhost.

Removing these redirects reduces our maintenance burden quite a bit.
Because we had to run all keyboard code through the service locator,
anything that had a dependency on key translation needed to link the
entire console host. Therefore, for anything that conhost depended upon,
so did the unit test binaries. Ugh.

I chose to keep TranslateCharsetInfo, even though it looks like gdi32 is
hosted in OneCore and the code appears as though it would work. It was
not in my critical path, and is a very basic local lookup that only powers
whether gridlines are available and the console's "Lang ID".

TEST RESULTS FROM ONECORE

Microsoft.Console.Host.FeatureTests.dll
Summary: Total=408, Passed=140, Failed=266, Blocked=0, Not Run=0, Skipped=2

I do not know how to make sure that the feature tests run properly on
OneCore. It looks like ModuleSetup is failing, which is unlikely to be
related to this change (especially given that conhost *does* launch.)

If I re-run the categories individually, they either pass or get marked
as skipped intentionally.

Microsoft.Console.Host.IntegrityTests.dll
Summary: Total=5, Passed=0, Failed=5, Blocked=0, Not Run=0, Skipped=0

Same.

Microsoft.Console.Interactivity.Win32.UnitTests.dll
Summary: Total=392, Passed=392, Failed=0, Blocked=0, Not Run=0, Skipped=0

Microsoft.Console.TextBuffer.UnitTests.dll
Summary: Total=27, Passed=27, Failed=0, Blocked=0, Not Run=0, Skipped=0

Microsoft.Console.Host.UIAutomationTests.dll
Summary: Total=6, Passed=0, Failed=0, Blocked=6, Not Run=0, Skipped=0

Microsoft.Console.Types.UnitTests.dll
Summary: Total=8, Passed=8, Failed=0, Blocked=0, Not Run=0, Skipped=0

Microsoft.Terminal.Til.UnitTests.dll
Summary: Total=290, Passed=290, Failed=0, Blocked=0, Not Run=0, Skipped=0

Microsoft.Console.VirtualTerminal.Parser.UnitTests.dll
Summary: Total=748, Passed=747, Failed=0, Blocked=0, Not Run=0, Skipped=1

Microsoft.Console.VirtualTerminal.Adapter.UnitTests.dll
Summary: Total=222, Passed=222, Failed=0, Blocked=0, Not Run=0, Skipped=0

Microsoft.Console.Host.UnitTests.dll
Summary: Total=5235, Passed=5222, Failed=0, Blocked=12, Not Run=0, Skipped=1

Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev c4c0267e1139d8d205bc9995da7906e5b09f03db

Related work items: MSFT-38632962
  • Loading branch information
DHowett committed Mar 23, 2022
1 parent 8f180c2 commit fd4fa66
Show file tree
Hide file tree
Showing 23 changed files with 23 additions and 458 deletions.
16 changes: 8 additions & 8 deletions src/host/input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,35 +52,35 @@ ULONG GetControlKeyState(const LPARAM lParam)
{
ULONG ControlKeyState = 0;

if (ServiceLocator::LocateInputServices()->GetKeyState(VK_LMENU) & KEY_PRESSED)
if (GetKeyState(VK_LMENU) & KEY_PRESSED)
{
ControlKeyState |= LEFT_ALT_PRESSED;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_RMENU) & KEY_PRESSED)
if (GetKeyState(VK_RMENU) & KEY_PRESSED)
{
ControlKeyState |= RIGHT_ALT_PRESSED;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_LCONTROL) & KEY_PRESSED)
if (GetKeyState(VK_LCONTROL) & KEY_PRESSED)
{
ControlKeyState |= LEFT_CTRL_PRESSED;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_RCONTROL) & KEY_PRESSED)
if (GetKeyState(VK_RCONTROL) & KEY_PRESSED)
{
ControlKeyState |= RIGHT_CTRL_PRESSED;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_SHIFT) & KEY_PRESSED)
if (GetKeyState(VK_SHIFT) & KEY_PRESSED)
{
ControlKeyState |= SHIFT_PRESSED;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_NUMLOCK) & KEY_TOGGLED)
if (GetKeyState(VK_NUMLOCK) & KEY_TOGGLED)
{
ControlKeyState |= NUMLOCK_ON;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_SCROLL) & KEY_TOGGLED)
if (GetKeyState(VK_SCROLL) & KEY_TOGGLED)
{
ControlKeyState |= SCROLLLOCK_ON;
}
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_CAPITAL) & KEY_TOGGLED)
if (GetKeyState(VK_CAPITAL) & KEY_TOGGLED)
{
ControlKeyState |= CAPSLOCK_ON;
}
Expand Down
7 changes: 3 additions & 4 deletions src/host/selectionInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ using Microsoft::Console::Interactivity::ServiceLocator;
Selection::KeySelectionEventResult Selection::HandleKeySelectionEvent(const INPUT_KEY_INFO* const pInputKeyInfo)
{
const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const auto inputServices = ServiceLocator::LocateInputServices();
FAIL_FAST_IF(!IsInSelectingState());

const WORD wVirtualKeyCode = pInputKeyInfo->GetVirtualKey();
const bool ctrlPressed = WI_IsFlagSet(inputServices->GetKeyState(VK_CONTROL), KEY_PRESSED);
const bool ctrlPressed = WI_IsFlagSet(GetKeyState(VK_CONTROL), KEY_PRESSED);

// if escape or ctrl-c, cancel selection
if (!IsMouseButtonDown())
Expand Down Expand Up @@ -612,7 +611,7 @@ bool Selection::HandleKeyboardLineSelectionEvent(const INPUT_KEY_INFO* const pIn
// - <none>
void Selection::CheckAndSetAlternateSelection()
{
_fUseAlternateSelection = !!(ServiceLocator::LocateInputServices()->GetKeyState(VK_MENU) & KEY_PRESSED);
_fUseAlternateSelection = !!(GetKeyState(VK_MENU) & KEY_PRESSED);
}

// Routine Description:
Expand Down Expand Up @@ -905,7 +904,7 @@ bool Selection::_HandleMarkModeSelectionNav(const INPUT_KEY_INFO* const pInputKe
}

// see if shift is down. if so, we're extending the selection. otherwise, we're resetting the anchor
if (ServiceLocator::LocateInputServices()->GetKeyState(VK_SHIFT) & KEY_PRESSED)
if (GetKeyState(VK_SHIFT) & KEY_PRESSED)
{
// if we're just starting to "extend" our selection from moving around as a cursor
// then attempt to set the alternate selection state based on the ALT key right now
Expand Down
2 changes: 1 addition & 1 deletion src/host/stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ using Microsoft::Console::Interactivity::ServiceLocator;
}
else
{
const short zeroVkeyData = ServiceLocator::LocateInputServices()->VkKeyScanW(0);
const short zeroVkeyData = VkKeyScanW(0);
const byte zeroVKey = LOBYTE(zeroVkeyData);
const byte zeroControlKeyState = HIBYTE(zeroVkeyData);

Expand Down
19 changes: 6 additions & 13 deletions src/host/ut_host/ClipboardTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@

#include <cctype>

#ifdef BUILD_ONECORE_INTERACTIVITY
#include "../../interactivity/inc/VtApiRedirection.hpp"
#endif

#include "../../inc/consoletaeftemplates.hpp"

using namespace WEX::Common;
Expand Down Expand Up @@ -161,7 +157,6 @@ class ClipboardTests
std::deque<std::unique_ptr<IInputEvent>> events = Clipboard::Instance().TextToKeyEvents(wstr.c_str(),
wstr.size());
VERIFY_ARE_EQUAL(wstr.size() * 2, events.size());
IInputServices* pInputServices = ServiceLocator::LocateInputServices();
for (wchar_t wch : wstr)
{
std::deque<bool> keydownPattern{ true, false };
Expand All @@ -172,9 +167,9 @@ class ClipboardTests
keyEvent.reset(static_cast<KeyEvent* const>(events.front().release()));
events.pop_front();

const short keyState = pInputServices->VkKeyScanW(wch);
const short keyState = VkKeyScanW(wch);
VERIFY_ARE_NOT_EQUAL(-1, keyState);
const WORD virtualScanCode = static_cast<WORD>(pInputServices->MapVirtualKeyW(LOBYTE(keyState), MAPVK_VK_TO_VSC));
const WORD virtualScanCode = static_cast<WORD>(MapVirtualKeyW(LOBYTE(keyState), MAPVK_VK_TO_VSC));

VERIFY_ARE_EQUAL(wch, keyEvent->GetCharData());
VERIFY_ARE_EQUAL(isKeyDown, keyEvent->IsKeyDown());
Expand All @@ -198,8 +193,6 @@ class ClipboardTests
wstr.size());

VERIFY_ARE_EQUAL((wstr.size() + uppercaseCount) * 2, events.size());
IInputServices* pInputServices = ServiceLocator::LocateInputServices();
VERIFY_IS_NOT_NULL(pInputServices);
for (wchar_t wch : wstr)
{
std::deque<bool> keydownPattern{ true, false };
Expand All @@ -213,9 +206,9 @@ class ClipboardTests
events.pop_front();

const short keyScanError = -1;
const short keyState = pInputServices->VkKeyScanW(wch);
const short keyState = VkKeyScanW(wch);
VERIFY_ARE_NOT_EQUAL(keyScanError, keyState);
const WORD virtualScanCode = static_cast<WORD>(pInputServices->MapVirtualKeyW(LOBYTE(keyState), MAPVK_VK_TO_VSC));
const WORD virtualScanCode = static_cast<WORD>(MapVirtualKeyW(LOBYTE(keyState), MAPVK_VK_TO_VSC));

if (std::isupper(wch))
{
Expand All @@ -229,9 +222,9 @@ class ClipboardTests
keyEvent2.reset(static_cast<KeyEvent* const>(events.front().release()));
events.pop_front();

const short keyState2 = pInputServices->VkKeyScanW(wch);
const short keyState2 = VkKeyScanW(wch);
VERIFY_ARE_NOT_EQUAL(keyScanError, keyState2);
const WORD virtualScanCode2 = static_cast<WORD>(pInputServices->MapVirtualKeyW(LOBYTE(keyState2), MAPVK_VK_TO_VSC));
const WORD virtualScanCode2 = static_cast<WORD>(MapVirtualKeyW(LOBYTE(keyState2), MAPVK_VK_TO_VSC));

if (isKeyDown)
{
Expand Down
4 changes: 0 additions & 4 deletions src/interactivity/base/EventSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
#include "../inc/EventSynthesis.hpp"
#include "../../types/inc/convert.hpp"

#ifdef BUILD_ONECORE_INTERACTIVITY
#include "../inc/VtApiRedirection.hpp"
#endif

#pragma hdrstop

// TODO: MSFT 14150722 - can these const values be generated at
Expand Down
27 changes: 0 additions & 27 deletions src/interactivity/base/VtApiRedirection.cpp

This file was deleted.

3 changes: 1 addition & 2 deletions src/interactivity/base/lib/InteractivityBase.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
<ClInclude Include="..\..\inc\ISystemConfigurationProvider.hpp" />
<ClInclude Include="..\..\inc\IWindowMetrics.hpp" />
<ClInclude Include="..\..\inc\Module.hpp" />
<ClInclude Include="..\..\inc\VtApiRedirection.hpp" />
<ClInclude Include="..\..\inc\EventSynthesis.hpp" />
<ClInclude Include="..\ApiDetector.hpp" />
<ClInclude Include="..\HostSignalInputThread.hpp" />
Expand All @@ -52,4 +51,4 @@
</ItemGroup>
<!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->
<Import Project="$(SolutionDir)src\common.build.post.props" />
</Project>
</Project>
5 changes: 1 addition & 4 deletions src/interactivity/base/lib/InteractivityBase.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@
<ClInclude Include="..\..\inc\Module.hpp">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="..\..\inc\VtApiRedirection.hpp">
<Filter>Header Files</Filter>
</ClInclude>
<ClInclude Include="..\..\inc\EventSynthesis.hpp">
<Filter>Header Files</Filter>
</ClInclude>
Expand All @@ -96,4 +93,4 @@
<ItemGroup>
<Natvis Include="$(SolutionDir)tools\ConsoleTypes.natvis" />
</ItemGroup>
</Project>
</Project>
1 change: 0 additions & 1 deletion src/interactivity/base/sources.inc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ SOURCES = \
..\ApiDetector.cpp \
..\InteractivityFactory.cpp \
..\ServiceLocator.cpp \
..\VtApiRedirection.cpp \
..\EventSynthesis.cpp \
..\RemoteConsoleControl.cpp \
..\HostSignalInputThread.cpp \
Expand Down
3 changes: 0 additions & 3 deletions src/interactivity/inc/IInputServices.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ namespace Microsoft::Console::Interactivity
class IInputServices
{
public:
virtual UINT MapVirtualKeyW(_In_ UINT uCode, _In_ UINT uMapType) = 0;
virtual SHORT VkKeyScanW(_In_ WCHAR ch) = 0;
virtual SHORT GetKeyState(_In_ int nVirtKey) = 0;
virtual BOOL TranslateCharsetInfo(_Inout_ DWORD FAR* lpSrc, _Out_ LPCHARSETINFO lpCs, _In_ DWORD dwFlags) = 0;
virtual ~IInputServices() = 0;

Expand Down
29 changes: 0 additions & 29 deletions src/interactivity/inc/VtApiRedirection.hpp

This file was deleted.

112 changes: 0 additions & 112 deletions src/interactivity/onecore/ConIoSrvComm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,70 +532,6 @@ VOID ConIoSrvComm::CleanupForHeadless(const NTSTATUS status)
return Status;
}

[[nodiscard]] NTSTATUS ConIoSrvComm::RequestMapVirtualKey(_In_ UINT uCode, _In_ UINT uMapType, _Out_ UINT* puReturnValue)
{
NTSTATUS Status;

Status = EnsureConnection();
if (NT_SUCCESS(Status))
{
CIS_MSG Message = { 0 };
Message.Type = CIS_MSG_TYPE_MAPVIRTUALKEY;
Message.MapVirtualKeyParams.Code = uCode;
Message.MapVirtualKeyParams.MapType = uMapType;

Status = SendRequestReceiveReply(&Message);
if (NT_SUCCESS(Status))
{
*puReturnValue = Message.MapVirtualKeyParams.ReturnValue;
}
}

return Status;
}

[[nodiscard]] NTSTATUS ConIoSrvComm::RequestVkKeyScan(_In_ WCHAR wCharacter, _Out_ SHORT* psReturnValue)
{
NTSTATUS Status;

Status = EnsureConnection();
if (NT_SUCCESS(Status))
{
CIS_MSG Message = { 0 };
Message.Type = CIS_MSG_TYPE_VKKEYSCAN;
Message.VkKeyScanParams.Character = wCharacter;

Status = SendRequestReceiveReply(&Message);
if (NT_SUCCESS(Status))
{
*psReturnValue = Message.VkKeyScanParams.ReturnValue;
}
}

return Status;
}

[[nodiscard]] NTSTATUS ConIoSrvComm::RequestGetKeyState(_In_ int iVirtualKey, _Out_ SHORT* psReturnValue)
{
NTSTATUS Status;

Status = EnsureConnection();
if (NT_SUCCESS(Status))
{
CIS_MSG Message = { 0 };
Message.Type = CIS_MSG_TYPE_GETKEYSTATE;
Message.GetKeyStateParams.VirtualKey = iVirtualKey;

Status = SendRequestReceiveReply(&Message);
if (NT_SUCCESS(Status))
{
*psReturnValue = Message.GetKeyStateParams.ReturnValue;
}
}

return Status;
}

[[nodiscard]] USHORT ConIoSrvComm::GetDisplayMode() const
{
return _displayMode;
Expand All @@ -610,54 +546,6 @@ PVOID ConIoSrvComm::GetSharedViewBase() const

#pragma region IInputServices Members

UINT ConIoSrvComm::MapVirtualKeyW(UINT uCode, UINT uMapType)
{
NTSTATUS Status = STATUS_SUCCESS;

UINT ReturnValue;
Status = RequestMapVirtualKey(uCode, uMapType, &ReturnValue);

if (!NT_SUCCESS(Status))
{
ReturnValue = 0;
SetLastError(ERROR_PROC_NOT_FOUND);
}

return ReturnValue;
}

SHORT ConIoSrvComm::VkKeyScanW(WCHAR ch)
{
NTSTATUS Status = STATUS_SUCCESS;

SHORT ReturnValue;
Status = RequestVkKeyScan(ch, &ReturnValue);

if (!NT_SUCCESS(Status))
{
ReturnValue = 0;
SetLastError(ERROR_PROC_NOT_FOUND);
}

return ReturnValue;
}

SHORT ConIoSrvComm::GetKeyState(int nVirtKey)
{
NTSTATUS Status = STATUS_SUCCESS;

SHORT ReturnValue;
Status = RequestGetKeyState(nVirtKey, &ReturnValue);

if (!NT_SUCCESS(Status))
{
ReturnValue = 0;
SetLastError(ERROR_PROC_NOT_FOUND);
}

return ReturnValue;
}

BOOL ConIoSrvComm::TranslateCharsetInfo(DWORD* lpSrc, LPCHARSETINFO lpCs, DWORD dwFlags)
{
SetLastError(ERROR_SUCCESS);
Expand Down
Loading

0 comments on commit fd4fa66

Please sign in to comment.