From a2d80682c96eb85d6537c3f9371ba64bcb3814c4 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 24 Apr 2025 19:22:30 +0200 Subject: [PATCH] Use RenderSettings for DECSET 2026 - Synchronized Output (#18833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RenderSettings` already stores `DECSCNM` (reversed screen), so it only makes sense to also store DECSET 2026 there. ## Validation Steps Performed * Same as in #18826 ✅ --- src/cascadia/TerminalControl/ControlCore.cpp | 2 +- src/renderer/base/RenderSettings.cpp | 6 +-- src/renderer/base/renderer.cpp | 52 +++++++++++--------- src/renderer/base/renderer.hpp | 7 ++- src/renderer/inc/RenderSettings.hpp | 3 +- src/terminal/adapter/adaptDispatch.cpp | 14 +++--- 6 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 4bd41d58d0..a96adb7a1d 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -143,7 +143,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // the UIA Engine to the renderer. This prevents us from signaling changes to the cursor or buffer. { // Now create the renderer and initialize the render thread. - const auto& renderSettings = _terminal->GetRenderSettings(); + auto& renderSettings = _terminal->GetRenderSettings(); _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get()); _renderer->SetBackgroundColorChangedCallback([this]() { _rendererBackgroundColorChanged(); }); diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 371b591b32..c587c60128 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -46,9 +46,9 @@ void RenderSettings::RestoreDefaultSettings() noexcept { _colorTable = _defaultColorTable; _colorAliasIndices = _defaultColorAliasIndices; - // For now, DECSCNM is the only render mode we need to reset. The others are - // all user preferences that can't be changed programmatically. - _renderMode.reset(Mode::ScreenReversed); + // DECSCNM and Synchronized Output are the only render mode we need to reset. + // The others are all user preferences that can't be changed programmatically. + _renderMode.reset(Mode::ScreenReversed, Mode::SynchronizedOutput); } // Routine Description: diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 5824bcb2f2..c923ef8e3f 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -25,7 +25,7 @@ static constexpr auto renderBackoffBaseTimeMilliseconds{ 150 }; // - pData - The interface to console data structures required for rendering // Return Value: // - An instance of a Renderer. -Renderer::Renderer(const RenderSettings& renderSettings, IRenderData* pData) : +Renderer::Renderer(RenderSettings& renderSettings, IRenderData* pData) : _renderSettings(renderSettings), _pData(pData) { @@ -187,31 +187,36 @@ void Renderer::NotifyPaintFrame() noexcept } // NOTE: You must be holding the console lock when calling this function. -void Renderer::SynchronizedOutputBegin() noexcept +void Renderer::SynchronizedOutputChanged() noexcept { - // Kick the render thread into calling `_synchronizeWithOutput()`. - _isSynchronizingOutput = true; -} + const auto so = _renderSettings.GetRenderMode(RenderSettings::Mode::SynchronizedOutput); + if (_isSynchronizingOutput == so) + { + return; + } -// NOTE: You must be holding the console lock when calling this function. -void Renderer::SynchronizedOutputEnd() noexcept -{ - // Unblock `_synchronizeWithOutput()` from the `WaitOnAddress` call. - _isSynchronizingOutput = false; - WakeByAddressSingle(&_isSynchronizingOutput); + // If `_isSynchronizingOutput` is true, it'll kick the + // render thread into calling `_synchronizeWithOutput()`... + _isSynchronizingOutput = so; - // It's crucial to give the render thread at least a chance to gain the lock. - // Otherwise, a VT application could continuously spam DECSET 2026 (Synchronized Output) and - // essentially drop our renderer to 10 FPS, because `_isSynchronizingOutput` is always true. - // - // Obviously calling LockConsole/UnlockConsole here is an awful, ugly hack, - // since there's no guarantee that this is the same lock as the one the VT parser uses. - // But the alternative is Denial-Of-Service of the render thread. - // - // Note that this causes raw throughput of DECSET 2026 to be comparatively low, but that's fine. - // Apps that use DECSET 2026 don't produce that sequence continuously, but rather at a fixed rate. - _pData->UnlockConsole(); - _pData->LockConsole(); + if (!_isSynchronizingOutput) + { + // ...otherwise, unblock `_synchronizeWithOutput()` from the `WaitOnAddress` call. + WakeByAddressSingle(&_isSynchronizingOutput); + + // It's crucial to give the render thread at least a chance to gain the lock. + // Otherwise, a VT application could continuously spam DECSET 2026 (Synchronized Output) and + // essentially drop our renderer to 10 FPS, because `_isSynchronizingOutput` is always true. + // + // Obviously calling LockConsole/UnlockConsole here is an awful, ugly hack, + // since there's no guarantee that this is the same lock as the one the VT parser uses. + // But the alternative is Denial-Of-Service of the render thread. + // + // Note that this causes raw throughput of DECSET 2026 to be comparatively low, but that's fine. + // Apps that use DECSET 2026 don't produce that sequence continuously, but rather at a fixed rate. + _pData->UnlockConsole(); + _pData->LockConsole(); + } } void Renderer::_synchronizeWithOutput() noexcept @@ -249,6 +254,7 @@ void Renderer::_synchronizeWithOutput() noexcept // If a timeout occurred, `_isSynchronizingOutput` may still be true. // Set it to false now to skip calling `_synchronizeWithOutput()` on the next frame. _isSynchronizingOutput = false; + _renderSettings.SetRenderMode(RenderSettings::Mode::SynchronizedOutput, false); } // Routine Description: diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index c8c3045760..31acc72860 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -28,15 +28,14 @@ namespace Microsoft::Console::Render class Renderer { public: - Renderer(const RenderSettings& renderSettings, IRenderData* pData); + Renderer(RenderSettings& renderSettings, IRenderData* pData); IRenderData* GetRenderData() const noexcept; [[nodiscard]] HRESULT PaintFrame(); void NotifyPaintFrame() noexcept; - void SynchronizedOutputBegin() noexcept; - void SynchronizedOutputEnd() noexcept; + void SynchronizedOutputChanged() noexcept; void TriggerSystemRedraw(const til::rect* const prcDirtyClient); void TriggerRedraw(const Microsoft::Console::Types::Viewport& region); void TriggerRedraw(const til::point* const pcoord); @@ -113,7 +112,7 @@ namespace Microsoft::Console::Render void _prepareNewComposition(); [[nodiscard]] HRESULT _PrepareRenderInfo(_In_ IRenderEngine* const pEngine); - const RenderSettings& _renderSettings; + RenderSettings& _renderSettings; std::array _engines{}; IRenderData* _pData = nullptr; // Non-ownership pointer static constexpr size_t _firstSoftFontChar = 0xEF20; diff --git a/src/renderer/inc/RenderSettings.hpp b/src/renderer/inc/RenderSettings.hpp index 84443889bd..8b0d64379f 100644 --- a/src/renderer/inc/RenderSettings.hpp +++ b/src/renderer/inc/RenderSettings.hpp @@ -25,7 +25,8 @@ namespace Microsoft::Console::Render AlwaysDistinguishableColors, IntenseIsBold, IntenseIsBright, - ScreenReversed + ScreenReversed, + SynchronizedOutput, }; RenderSettings() noexcept; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index c8f4c5eb85..12fe6854e0 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -1915,16 +1915,10 @@ void AdaptDispatch::_ModeParamsHelper(const DispatchTypes::ModeParams param, con _api.SetSystemMode(ITerminalApi::Mode::BracketedPaste, enable); break; case DispatchTypes::ModeParams::SO_SynchronizedOutput: + _renderSettings.SetRenderMode(RenderSettings::Mode::SynchronizedOutput, enable); if (_renderer) { - if (enable) - { - _renderer->SynchronizedOutputBegin(); - } - else - { - _renderer->SynchronizedOutputEnd(); - } + _renderer->SynchronizedOutputChanged(); } break; case DispatchTypes::ModeParams::GCM_GraphemeClusterMode: @@ -2065,6 +2059,9 @@ void AdaptDispatch::RequestMode(const DispatchTypes::ModeParams param) case DispatchTypes::ModeParams::XTERM_BracketedPasteMode: state = mapTemp(_api.GetSystemMode(ITerminalApi::Mode::BracketedPaste)); break; + case DispatchTypes::ModeParams::SO_SynchronizedOutput: + state = mapTemp(_renderSettings.GetRenderMode(RenderSettings::Mode::SynchronizedOutput)); + break; case DispatchTypes::ModeParams::GCM_GraphemeClusterMode: state = mapPerm(CodepointWidthDetector::Singleton().GetMode() == TextMeasurementMode::Graphemes); break; @@ -3050,6 +3047,7 @@ void AdaptDispatch::HardReset() if (_renderer) { _renderer->TriggerRedrawAll(true, true); + _renderer->SynchronizedOutputChanged(); } // Cursor to 1,1 - the Soft Reset guarantees this is absolute