diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 7ee396a3c80..d498bd6901b 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -104,6 +104,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())); + } + // Get our dispatcher. If we're hosted in-proc with XAML, this will get // us the same dispatcher as TermControl::Dispatcher(). If we're out of // proc, this'll return null. We'll need to instead make a new @@ -196,27 +222,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()); @@ -248,7 +253,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 @@ -1465,10 +1470,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine) { - if (_renderer) - { - _renderer->AddRenderEngine(pEngine); - } + // _renderer will always exist since it's introduced in the ctor + _renderer->AddRenderEngine(pEngine); } bool ControlCore::IsInReadOnlyMode() const diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index dc3f61490f0..55d38986966 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -614,7 +614,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation Control::InteractivityAutomationPeer ControlInteractivity::OnCreateAutomationPeer() try { - auto autoPeer = winrt::make_self(this); + const auto autoPeer = winrt::make_self(this); _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get()); _core->AttachUiaEngine(_uiaEngine.get()); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 8d477fadfaf..e54515bcd11 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -377,7 +377,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _changeBackgroundColor(bg); // Apply padding as swapChainPanel's margin - auto newMargin = ParseThicknessFromPadding(newSettings.Padding()); + const auto newMargin = ParseThicknessFromPadding(newSettings.Padding()); SwapChainPanel().Margin(newMargin); TSFInputControl().Margin(newMargin); @@ -511,18 +511,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - The automation peer for our control Windows::UI::Xaml::Automation::Peers::AutomationPeer TermControl::OnCreateAutomationPeer() { - if (_initializedTerminal && !_IsClosing()) // only set up the automation peer if we're ready to go live + // MSFT 33353327: We're purposefully not using _initializedTerminal to ensure we're fully initialized. + // Doing so makes us return nullptr when XAML requests an automation peer. + // Instead, we need to give XAML an automation peer, then fix it later. + if (!_IsClosing()) { // create a custom automation peer with this code pattern: // (https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers) if (const auto& interactivityAutoPeer{ _interactivity.OnCreateAutomationPeer() }) { - auto margins{ SwapChainPanel().Margin() }; - - Core::Padding padding{ margins.Left, - margins.Top, - margins.Right, - margins.Bottom }; + const auto margins{ SwapChainPanel().Margin() }; + const Core::Padding padding{ margins.Left, + margins.Top, + margins.Right, + margins.Bottom }; _automationPeer = winrt::make(this, padding, interactivityAutoPeer); return _automationPeer; } @@ -717,6 +719,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation _initializedTerminal = true; + // MSFT 33353327: If the AutomationPeer was created before we were done initializing, + // make sure it's properly set up now. + if (_automationPeer) + { + _automationPeer.UpdateControlBounds(); + const auto margins{ GetPadding() }; + _automationPeer.SetControlPadding(Core::Padding{ margins.Left, + margins.Top, + margins.Right, + margins.Bottom }); + } + // Likewise, run the event handlers outside of lock (they could // be reentrant) _InitializedHandlers(*this, nullptr); diff --git a/src/types/UiaTextRangeBase.cpp b/src/types/UiaTextRangeBase.cpp index 02e8abdb639..d6aac19cd00 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()) {