From 6e3be8f42add4e3d12d4964a407ec5c8fb112439 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 26 Aug 2021 08:38:45 -0700 Subject: [PATCH] [1.10] Ensure auto peer is created regardless of term init (#11042) ## Summary of the Pull Request This patch accomplishes what #10971 does, but for the release-1.10 branch. Some things couldn't be carried over because the TermControl split wasn't the complete (or in the same state as it is currently on main). The bug was that Narrator would still read the content of the old tab/pane although a new tab/pane was introduced. This is caused by the automation peer not being created when XAML requests it. Normally, we would prevent the automation peer from being created if the terminal was not fully initialized. This change allows the automation peer to be created regardless of the terminal being fully initialized by... - `ControlCore`: initialize the `_renderer` in the ctor so that we can attach the UIA Engine before `ControlCore::Initialize()` is called (dependent on `SwapChainPanel` loading) As a bonus, this also fixes a locking issue where logging would attempt to get the text range's text and lock twice. The locking fix is very similar to #10937. ## Differences from #10971 This was prior to #10874. _Thankfully_, we don't need to worry about the padding because that bug was introduced as a part of #10051. ## Other references MSFT-33353327 ## Validation Steps Performed - New pane from key binding is announced by Narrator - New tab from key binding is announced by Narrator - Bounding rects are placed on the screen accurately (even when the padding changes) --- src/cascadia/TerminalControl/ControlCore.cpp | 54 ++++++++++---------- src/cascadia/TerminalControl/TermControl.cpp | 2 +- src/types/UiaTextRangeBase.cpp | 10 ++-- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 100fdc12e8b..743bfd2ba38 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -94,6 +94,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation auto pfnTerminalTaskbarProgressChanged = std::bind(&ControlCore::_terminalTaskbarProgressChanged, this); _terminal->TaskbarProgressChangedCallback(pfnTerminalTaskbarProgressChanged); + // MSFT 33353327: Initialize the renderer in the ctor instead of Initialize(). + // We need the renderer to be ready to accept new engines before the SwapChainPanel is ready to go. + // If we wait, a screen reader may try to get the AutomationPeer (aka the UIA Engine), and we won't be able to attach + // the UIA Engine to the renderer. This prevents us from signaling changes to the cursor or buffer. + { + // First create the render thread. + // Then stash a local pointer to the render thread so we can initialize it and enable it + // to paint itself *after* we hand off its ownership to the renderer. + // We split up construction and initialization of the render thread object this way + // because the renderer and render thread have circular references to each other. + auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>(); + auto* const localPointerToThread = renderThread.get(); + + // Now create the renderer and initialize the render thread. + _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(_terminal.get(), nullptr, 0, std::move(renderThread)); + + _renderer->SetRendererEnteredErrorStateCallback([weakThis = get_weak()]() { + if (auto strongThis{ weakThis.get() }) + { + strongThis->_RendererEnteredErrorStateHandlers(*strongThis, nullptr); + } + }); + + THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get())); + } + UpdateSettings(settings); } @@ -131,27 +157,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation return false; } - // First create the render thread. - // Then stash a local pointer to the render thread so we can initialize it and enable it - // to paint itself *after* we hand off its ownership to the renderer. - // We split up construction and initialization of the render thread object this way - // because the renderer and render thread have circular references to each other. - auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>(); - auto* const localPointerToThread = renderThread.get(); - - // Now create the renderer and initialize the render thread. - _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(_terminal.get(), nullptr, 0, std::move(renderThread)); - ::Microsoft::Console::Render::IRenderTarget& renderTarget = *_renderer; - - _renderer->SetRendererEnteredErrorStateCallback([weakThis = get_weak()]() { - if (auto strongThis{ weakThis.get() }) - { - strongThis->_RendererEnteredErrorStateHandlers(*strongThis, nullptr); - } - }); - - THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get())); - // Set up the DX Engine auto dxEngine = std::make_unique<::Microsoft::Console::Render::DxEngine>(); _renderer->AddRenderEngine(dxEngine.get()); @@ -182,7 +187,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _settings.InitialCols(width); _settings.InitialRows(height); - _terminal->CreateFromSettings(_settings, renderTarget); + _terminal->CreateFromSettings(_settings, *_renderer); // IMPORTANT! Set this callback up sooner than later. If we do it // after Enable, then it'll be possible to paint the frame once @@ -1325,10 +1330,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine) { - if (_renderer) - { - _renderer->AddRenderEngine(pEngine); - } + _renderer->AddRenderEngine(pEngine); } bool ControlCore::IsInReadOnlyMode() const diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 01a70fc773c..cf29e402c44 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -529,7 +529,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation Windows::UI::Xaml::Automation::Peers::AutomationPeer TermControl::OnCreateAutomationPeer() try { - if (_initializedTerminal && !_IsClosing()) // only set up the automation peer if we're ready to go live + if (!_IsClosing()) { // create a custom automation peer with this code pattern: // (https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers) diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index e837f4122d9..5c73d06e36f 100644 --- a/src/types/UiaTextRangeBase.cpp +++ b/src/types/UiaTextRangeBase.cpp @@ -938,7 +938,12 @@ try const auto maxLengthOpt = (maxLength == -1) ? std::nullopt : std::optional{ maxLength }; + _pData->LockConsole(); + auto Unlock = wil::scope_exit([this]() noexcept { + _pData->UnlockConsole(); + }); const auto text = _getTextValue(maxLengthOpt); + Unlock.reset(); *pRetVal = SysAllocString(text.c_str()); RETURN_HR_IF_NULL(E_OUTOFMEMORY, *pRetVal); @@ -958,11 +963,6 @@ CATCH_RETURN(); #pragma warning(disable : 26447) // compiler isn't filtering throws inside the try/catch std::wstring UiaTextRangeBase::_getTextValue(std::optional maxLength) const { - _pData->LockConsole(); - auto Unlock = wil::scope_exit([&]() noexcept { - _pData->UnlockConsole(); - }); - std::wstring textData{}; if (!IsDegenerate()) {