Skip to content

Commit

Permalink
Skip accessibility notifier and all event calculations when we're in …
Browse files Browse the repository at this point in the history
…PTY mode (#10569)

Change accessibility notifier creation so we do not create one when we're in PTY mode. (Guard all call sites to skip math/event work when the notifier is null.) MSAA events are legacy events that are registered for globally and used by some screen readers to find content in the conhost window. The PTY mode is not responsible for hosting the display content or input window, so it makes sense for it to not broadcast these events and delegate the accessibility requirement to the connected terminal.

## References
- #10537 

## PR Checklist
* [x] Closes #10568
* [x] I work here
* [x] Manual test launches passed.
  • Loading branch information
miniksa authored Jul 9, 2021
1 parent 6409ab9 commit 91b454a
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 45 deletions.
9 changes: 5 additions & 4 deletions src/host/CursorBlinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,16 @@ void CursorBlinker::TimerRoutine(SCREEN_INFORMATION& ScreenInfo)
auto& buffer = ScreenInfo.GetTextBuffer();
auto& cursor = buffer.GetCursor();
const CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto* const _pAccessibilityNotifier = ServiceLocator::LocateAccessibilityNotifier();
auto* const pAccessibilityNotifier = ServiceLocator::LocateAccessibilityNotifier();

if (!WI_IsFlagSet(gci.Flags, CONSOLE_HAS_FOCUS))
{
goto DoScroll;
}

// Update the cursor pos in USER so accessibility will work.
if (cursor.HasMoved())
// Don't do all this work or send events if we don't have a notifier target.
if (pAccessibilityNotifier && cursor.HasMoved())
{
// Convert the buffer position to the equivalent screen coordinates
// required by the notifier, taking line rendition into account.
Expand All @@ -93,7 +94,7 @@ void CursorBlinker::TimerRoutine(SCREEN_INFORMATION& ScreenInfo)
rc.right = rc.left + fontSize.X;
rc.bottom = rc.top + fontSize.Y;

_pAccessibilityNotifier->NotifyConsoleCaretEvent(rc);
pAccessibilityNotifier->NotifyConsoleCaretEvent(rc);

// Send accessibility information
{
Expand All @@ -109,7 +110,7 @@ void CursorBlinker::TimerRoutine(SCREEN_INFORMATION& ScreenInfo)
flags = IAccessibilityNotifier::ConsoleCaretEventFlags::CaretVisible;
}

_pAccessibilityNotifier->NotifyConsoleCaretEvent(flags, MAKELONG(position.X, position.Y));
pAccessibilityNotifier->NotifyConsoleCaretEvent(flags, MAKELONG(position.X, position.Y));
}
}

Expand Down
20 changes: 13 additions & 7 deletions src/host/_output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,13 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)

cellsModified = done.GetCellDistance(it);

// Notify accessibility
auto endingCoordinate = startingCoordinate;
bufferSize.MoveInBounds(cellsModified, endingCoordinate);
screenBuffer.NotifyAccessibilityEventing(startingCoordinate.X, startingCoordinate.Y, endingCoordinate.X, endingCoordinate.Y);
if (screenBuffer.HasAccessibilityEventing())
{
// Notify accessibility
auto endingCoordinate = startingCoordinate;
bufferSize.MoveInBounds(cellsModified, endingCoordinate);
screenBuffer.NotifyAccessibilityEventing(startingCoordinate.X, startingCoordinate.Y, endingCoordinate.X, endingCoordinate.Y);
}
}
CATCH_RETURN();

Expand Down Expand Up @@ -284,9 +287,12 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
cellsModified = done.GetInputDistance(it);

// Notify accessibility
auto endingCoordinate = startingCoordinate;
bufferSize.MoveInBounds(cellsModified, endingCoordinate);
screenInfo.NotifyAccessibilityEventing(startingCoordinate.X, startingCoordinate.Y, endingCoordinate.X, endingCoordinate.Y);
if (screenInfo.HasAccessibilityEventing())
{
auto endingCoordinate = startingCoordinate;
bufferSize.MoveInBounds(cellsModified, endingCoordinate);
screenInfo.NotifyAccessibilityEventing(startingCoordinate.X, startingCoordinate.Y, endingCoordinate.X, endingCoordinate.Y);
}

// GH#3126 - This is a shim for powershell's `Clear-Host` function. In
// the vintage console, `Clear-Host` is supposed to clear the entire
Expand Down
5 changes: 4 additions & 1 deletion src/host/_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,10 @@ constexpr unsigned int LOCAL_BUFFER_SIZE = 100;
const auto itEnd = screenInfo.Write(it);

// Notify accessibility
screenInfo.NotifyAccessibilityEventing(CursorPosition.X, CursorPosition.Y, CursorPosition.X + gsl::narrow<SHORT>(i - 1), CursorPosition.Y);
if (screenInfo.HasAccessibilityEventing())
{
screenInfo.NotifyAccessibilityEventing(CursorPosition.X, CursorPosition.Y, CursorPosition.X + gsl::narrow<SHORT>(i - 1), CursorPosition.Y);
}

// The number of "spaces" or "cells" we have consumed needs to be reported and stored for later
// when/if we need to erase the command line.
Expand Down
5 changes: 4 additions & 1 deletion src/host/conimeinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ std::vector<OutputCell>::const_iterator ConsoleImeInfo::_WriteConversionArea(con
area.Paint();

// Notify accessibility that we have updated the text in this display region within the viewport.
screenInfo.NotifyAccessibilityEventing(insertionPos.X, insertionPos.Y, gsl::narrow<SHORT>(insertionPos.X + lineVec.size() - 1), insertionPos.Y);
if (screenInfo.HasAccessibilityEventing())
{
screenInfo.NotifyAccessibilityEventing(insertionPos.X, insertionPos.Y, gsl::narrow<SHORT>(insertionPos.X + lineVec.size() - 1), insertionPos.Y);
}

// Hand back the iterator representing the end of what we used to be fed into the beginning of the next call.
return lineEnd;
Expand Down
12 changes: 8 additions & 4 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2210,10 +2210,14 @@ void DoSrvPrivateMoveToBottom(SCREEN_INFORMATION& screenInfo)
screenInfo.Write(fillData, startPosition, false);

// Notify accessibility
auto endPosition = startPosition;
const auto bufferSize = screenInfo.GetBufferSize();
bufferSize.MoveInBounds(fillLength - 1, endPosition);
screenInfo.NotifyAccessibilityEventing(startPosition.X, startPosition.Y, endPosition.X, endPosition.Y);
if (screenInfo.HasAccessibilityEventing())
{
auto endPosition = startPosition;
const auto bufferSize = screenInfo.GetBufferSize();
bufferSize.MoveInBounds(fillLength - 1, endPosition);
screenInfo.NotifyAccessibilityEventing(startPosition.X, startPosition.Y, endPosition.X, endPosition.Y);
}

return S_OK;
}
CATCH_RETURN();
Expand Down
33 changes: 26 additions & 7 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ SCREEN_INFORMATION::~SCREEN_INFORMATION()
THROW_HR_IF_NULL(E_FAIL, pMetrics);

IAccessibilityNotifier* pNotifier = ServiceLocator::LocateAccessibilityNotifier();
THROW_HR_IF_NULL(E_FAIL, pNotifier);

// It is possible for pNotifier to be null and that's OK.
// For instance, the PTY doesn't need to send events. Just pass it along
// and be sure that `SCREEN_INFORMATION` bypasses all event work if it's not there.
SCREEN_INFORMATION* const pScreen = new SCREEN_INFORMATION(pMetrics, pNotifier, popupAttributes, fontInfo);

// Set up viewport
Expand Down Expand Up @@ -564,6 +565,19 @@ void SCREEN_INFORMATION::UpdateFont(const FontInfo* const pfiNewFont)
}
}

// Routine Description:
// - Informs clients whether we have accessibility eventing so they can
// save themselves the work of performing math or lookups before calling
// `NotifyAccessibilityEventing`.
// Arguments:
// - <none>
// Return Value:
// - True if we have an accessibility listener. False otherwise.
bool SCREEN_INFORMATION::HasAccessibilityEventing() const noexcept
{
return _pAccessibilityNotifier;
}

// NOTE: This method was historically used to notify accessibility apps AND
// to aggregate drawing metadata to determine whether or not to use PolyTextOut.
// After the Nov 2015 graphics refactor, the metadata drawing flag calculation is no longer necessary.
Expand All @@ -573,8 +587,7 @@ void SCREEN_INFORMATION::NotifyAccessibilityEventing(const short sStartX,
const short sEndX,
const short sEndY)
{
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
if (gci.IsInVtIoMode())
if (!_pAccessibilityNotifier)
{
return;
}
Expand Down Expand Up @@ -680,7 +693,10 @@ VOID SCREEN_INFORMATION::InternalUpdateScrollBars()
}

// Fire off an event to let accessibility apps know the layout has changed.
_pAccessibilityNotifier->NotifyConsoleLayoutEvent();
if (_pAccessibilityNotifier)
{
_pAccessibilityNotifier->NotifyConsoleLayoutEvent();
}

ResizingWindow--;
}
Expand Down Expand Up @@ -1526,7 +1542,10 @@ bool SCREEN_INFORMATION::IsMaximizedY() const

if (NT_SUCCESS(status))
{
NotifyAccessibilityEventing(0, 0, (SHORT)(coordNewScreenSize.X - 1), (SHORT)(coordNewScreenSize.Y - 1));
if (HasAccessibilityEventing())
{
NotifyAccessibilityEventing(0, 0, (SHORT)(coordNewScreenSize.X - 1), (SHORT)(coordNewScreenSize.Y - 1));
}

if ((!ConvScreenInfo))
{
Expand All @@ -1538,7 +1557,7 @@ bool SCREEN_INFORMATION::IsMaximizedY() const
}

// Fire off an event to let accessibility apps know the layout has changed.
if (IsActiveScreenBuffer())
if (_pAccessibilityNotifier && IsActiveScreenBuffer())
{
_pAccessibilityNotifier->NotifyConsoleLayoutEvent();
}
Expand Down
1 change: 1 addition & 0 deletions src/host/screenInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console

[[nodiscard]] NTSTATUS ResizeScreenBuffer(const COORD coordNewScreenSize, const bool fDoScrollBarUpdate);

bool HasAccessibilityEventing() const noexcept;
void NotifyAccessibilityEventing(const short sStartX, const short sStartY, const short sEndX, const short sEndY);

void UpdateScrollBars();
Expand Down
12 changes: 10 additions & 2 deletions src/host/selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ void Selection::InitializeMouseSelection(const COORD coordBufferPos)
}

// Fire off an event to let accessibility apps know the selection has changed.
ServiceLocator::LocateAccessibilityNotifier()->NotifyConsoleCaretEvent(IAccessibilityNotifier::ConsoleCaretEventFlags::CaretSelection, PACKCOORD(coordBufferPos));
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
if (pNotifier)
{
pNotifier->NotifyConsoleCaretEvent(IAccessibilityNotifier::ConsoleCaretEventFlags::CaretSelection, PACKCOORD(coordBufferPos));
}
}

// Routine Description:
Expand Down Expand Up @@ -258,7 +262,11 @@ void Selection::ExtendSelection(_In_ COORD coordBufferPos)
_PaintSelection();

// Fire off an event to let accessibility apps know the selection has changed.
ServiceLocator::LocateAccessibilityNotifier()->NotifyConsoleCaretEvent(IAccessibilityNotifier::ConsoleCaretEventFlags::CaretSelection, PACKCOORD(coordBufferPos));
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
if (pNotifier)
{
pNotifier->NotifyConsoleCaretEvent(IAccessibilityNotifier::ConsoleCaretEventFlags::CaretSelection, PACKCOORD(coordBufferPos));
}
LOG_IF_FAILED(ServiceLocator::LocateConsoleWindow()->SignalUia(UIA_Text_TextSelectionChangedEventId));
}

Expand Down
11 changes: 11 additions & 0 deletions src/host/srvinit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ try
}
}

// Create the accessibility notifier early in the startup process.
// Only create if we're not in PTY mode.
// The notifiers use expensive legacy MSAA events and the PTY isn't even responsible
// for the terminal user interface, so we should set ourselves up to skip all
// those notifications and the mathematical calculations required to send those events
// for performance reasons.
if (!args->InConptyMode())
{
RETURN_IF_FAILED(ServiceLocator::CreateAccessibilityNotifier());
}

// Removed allocation of scroll buffer here.
return S_OK;
}
Expand Down
35 changes: 18 additions & 17 deletions src/interactivity/base/ServiceLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,24 @@ void ServiceLocator::RundownAndExit(const HRESULT hr)
return status;
}

[[nodiscard]] HRESULT ServiceLocator::CreateAccessibilityNotifier()
{
// Can't create if we've already created.
if (s_accessibilityNotifier)
{
return E_UNEXPECTED;
}

if (!s_interactivityFactory)
{
RETURN_IF_NTSTATUS_FAILED(ServiceLocator::LoadInteractivityFactory());
}

RETURN_IF_NTSTATUS_FAILED(s_interactivityFactory->CreateAccessibilityNotifier(s_accessibilityNotifier));

return S_OK;
}

#pragma endregion

#pragma region Set Methods
Expand Down Expand Up @@ -224,23 +242,6 @@ IWindowMetrics* ServiceLocator::LocateWindowMetrics()

IAccessibilityNotifier* ServiceLocator::LocateAccessibilityNotifier()
{
NTSTATUS status = STATUS_SUCCESS;

if (!s_accessibilityNotifier)
{
if (s_interactivityFactory.get() == nullptr)
{
status = ServiceLocator::LoadInteractivityFactory();
}

if (NT_SUCCESS(status))
{
status = s_interactivityFactory->CreateAccessibilityNotifier(s_accessibilityNotifier);
}
}

LOG_IF_NTSTATUS_FAILED(status);

return s_accessibilityNotifier.get();
}

Expand Down
1 change: 1 addition & 0 deletions src/interactivity/inc/ServiceLocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ namespace Microsoft::Console::Interactivity
// In case the on-demand creation fails, the return value
// is nullptr and a message is logged.

[[nodiscard]] static HRESULT CreateAccessibilityNotifier();
static IAccessibilityNotifier* LocateAccessibilityNotifier();

[[nodiscard]] static NTSTATUS SetConsoleControlInstance(_In_ std::unique_ptr<IConsoleControl>&& control);
Expand Down
12 changes: 10 additions & 2 deletions src/server/IoDispatchers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,11 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleHandleConnectionRequest(_In_ PCONSOLE_API
LOG_IF_FAILED(ServiceLocator::LocateConsoleControl()->NotifyConsoleApplication(dwProcessId));
}

ServiceLocator::LocateAccessibilityNotifier()->NotifyConsoleStartApplicationEvent(dwProcessId);
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
if (pNotifier)
{
pNotifier->NotifyConsoleStartApplicationEvent(dwProcessId);
}

if (WI_IsFlagClear(gci.Flags, CONSOLE_INITIALIZED))
{
Expand Down Expand Up @@ -460,7 +464,11 @@ PCONSOLE_API_MSG IoDispatchers::ConsoleClientDisconnectRoutine(_In_ PCONSOLE_API

ConsoleProcessHandle* const pProcessData = pMessage->GetProcessHandle();

ServiceLocator::LocateAccessibilityNotifier()->NotifyConsoleEndApplicationEvent(pProcessData->dwProcessId);
auto pNotifier = ServiceLocator::LocateAccessibilityNotifier();
if (pNotifier)
{
pNotifier->NotifyConsoleEndApplicationEvent(pProcessData->dwProcessId);
}

LOG_IF_FAILED(RemoveConsole(pProcessData));

Expand Down

0 comments on commit 91b454a

Please sign in to comment.