diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 0d4a245fd2..7e74aa9ecd 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4042,10 +4042,11 @@ namespace winrt::TerminalApp::implementation _UpdateTabWidthMode(); _CreateNewTabFlyout(); - // Reload the current value of alwaysOnTop from the settings file. This - // will let the user hot-reload this setting, but any runtime changes to - // the alwaysOnTop setting will be lost. - _isAlwaysOnTop = _settings.GlobalSettings().AlwaysOnTop(); + // Reload the current value of alwaysOnTop from the settings file, but let + // a runtime toggle (ToggleAlwaysOnTop) win so a settings reload doesn't + // stomp it (GH#12424). This still lets the user hot-reload the setting when + // they haven't overridden it at runtime. + _isAlwaysOnTop = _runtimeAlwaysOnTop.value_or(_settings.GlobalSettings().AlwaysOnTop()); AlwaysOnTopChanged.raise(*this, nullptr); _showTabsFullscreen = _settings.GlobalSettings().ShowTabsFullscreen(); @@ -4266,6 +4267,8 @@ namespace winrt::TerminalApp::implementation void TerminalPage::ToggleAlwaysOnTop() { _isAlwaysOnTop = !_isAlwaysOnTop; + // Record the runtime override so a settings reload preserves it (GH#12424). + _runtimeAlwaysOnTop = _isAlwaysOnTop; AlwaysOnTopChanged.raise(*this, nullptr); } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 72f13bf43d..e25641356e 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -253,6 +253,10 @@ namespace winrt::TerminalApp::implementation bool _isFullscreen{ false }; bool _isMaximized{ false }; bool _isAlwaysOnTop{ false }; + // Runtime override for always-on-top (toggled via ToggleAlwaysOnTop). When + // set, it wins over GlobalSettings().AlwaysOnTop() so a settings reload + // doesn't stomp the user's runtime toggle (GH#12424). + std::optional _runtimeAlwaysOnTop{ std::nullopt }; bool _showTabsFullscreen{ false }; std::optional _loadFromPersistedLayoutIdx{}; diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 3ca144bb4d..1871ad3b1f 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -912,6 +912,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - INVARIANT: This method can only be called if the caller DOES NOT HAVE writing lock on the terminal. void ControlCore::UpdateSettings(const IControlSettings& settings, const IControlAppearance& newAppearance) { + // Capture any runtime opacity adjustment (e.g. Ctrl+Shift+scroll, the + // adjustOpacity action) as a delta over the *old* settings opacity before + // _settings is replaced. On reload we re-apply that delta over the new + // settings opacity instead of snapping back to the configured value, so a + // settings reload doesn't stomp the user's runtime adjustment (GH#12424). + // This mirrors the font-size delta below, but is derived from the live + // runtime value rather than stored, because opacity is clamped to [0,1] on + // every adjustment -- a separately accumulated delta would drift out of + // sync with the (clamped) runtime opacity. + std::optional runtimeOpacityDelta; + if (_runtimeOpacity) + { + runtimeOpacityDelta = *_runtimeOpacity - _settings.Opacity(); + } + _settings = settings; _hasUnfocusedAppearance = static_cast(newAppearance); _unfocusedAppearance = _hasUnfocusedAppearance ? newAppearance : settings; @@ -922,11 +937,26 @@ namespace winrt::Microsoft::Terminal::Control::implementation _colorGlyphs = _settings.EnableColorGlyphs(); _cellWidth = CSSLengthPercentage::FromString(_settings.CellWidth().c_str()); _cellHeight = CSSLengthPercentage::FromString(_settings.CellHeight().c_str()); - _runtimeOpacity = std::nullopt; - _runtimeFocusedOpacity = std::nullopt; - // Manually turn off acrylic if they turn off transparency. - _runtimeUseAcrylic = _settings.Opacity() < 1.0 && _settings.UseAcrylic(); + // Preserve a runtime opacity adjustment across the reload by re-applying + // the captured delta over the new settings opacity. FocusedAppearance() is + // _settings, so _runtimeFocusedOpacity shares the same base and resolves to + // the same value. + if (runtimeOpacityDelta) + { + const auto resolvedOpacity = std::clamp(_settings.Opacity() + *runtimeOpacityDelta, 0.0f, 1.0f); + _runtimeOpacity = resolvedOpacity; + _runtimeFocusedOpacity = resolvedOpacity; + } + else + { + _runtimeOpacity = std::nullopt; + _runtimeFocusedOpacity = std::nullopt; + } + + // Manually turn off acrylic if they turn off transparency. Use the resolved + // (runtime-preserved) opacity so acrylic follows the runtime adjustment. + _runtimeUseAcrylic = Opacity() < 1.0 && _settings.UseAcrylic(); const auto sizeChanged = _setFontSizeUnderLock(_settings.FontSize() + _accumulatedFontSizeDelta); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 0746066b75..688eb61aba 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -91,6 +91,18 @@ void Terminal::CreateFromSettings(ICoreSettings settings, // - settings: an ICoreSettings with new settings values for us to use. void Terminal::UpdateSettings(ICoreSettings settings) { + // Capture any color table entries that were overridden at runtime by VT + // sequences (OSC 4/10/11/12) so the color-scheme refresh below doesn't stomp + // them on a settings reload (GH#12424). They're re-applied after the new + // scheme is saved as the default. Skipped on the very first load, where the + // constructor's default fg/bg setup would otherwise look like a runtime + // override (the scheme baseline isn't established until the first save below). + std::vector> runtimeColorOverrides; + if (_colorSchemeInitialized) + { + runtimeColorOverrides = GetRenderSettings().GetRuntimeColorTableOverrides(); + } + UpdateAppearance(settings); _snapOnInput = settings.SnapOnInput(); @@ -123,6 +135,15 @@ void Terminal::UpdateSettings(ICoreSettings settings) // Save the changes made above and in UpdateAppearance as the new default render settings. GetRenderSettings().SaveDefaultSettings(); + // Re-apply any runtime VT color overrides captured above on top of the + // refreshed scheme baseline, so a settings reload preserves them (GH#12424). + // A hard reset (RIS) still restores the new scheme via RestoreDefaultSettings. + for (const auto& [index, color] : runtimeColorOverrides) + { + GetRenderSettings().SetColorTableEntry(index, color); + } + _colorSchemeInitialized = true; + if (!_startingTitle) { _startingTitle = settings.StartingTitle(); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 8f4ca627a2..3433f5f54b 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -356,6 +356,11 @@ private: std::optional _startingTitle; std::optional _startingTabColor; + // Tracks whether UpdateSettings has run at least once, so the first load + // doesn't mistake the constructor's default fg/bg color setup for runtime VT + // color overrides when preserving them across reloads (GH#12424). + bool _colorSchemeInitialized{ false }; + std::vector _searchHighlights; size_t _searchHighlightFocused = 0; diff --git a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp index 7a0ef5134f..25ddb0c5fe 100644 --- a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp @@ -28,6 +28,8 @@ namespace ControlUnitTests TEST_METHOD(InstantiateCore); TEST_METHOD(TestInitialize); TEST_METHOD(TestAdjustAcrylic); + TEST_METHOD(TestRuntimeOpacitySurvivesSettingsReload); + TEST_METHOD(TestReadOnlySurvivesSettingsReload); TEST_METHOD(TestFreeAfterClose); @@ -206,6 +208,66 @@ namespace ControlUnitTests core->AdjustOpacity(-0.25f); } + void ControlCoreTests::TestRuntimeOpacitySurvivesSettingsReload() + { + // GH#12424: a runtime opacity adjustment (Ctrl+Shift+scroll / adjustOpacity) + // must survive a settings reload as a delta over the new settings opacity, + // rather than snapping back to the configured value. + auto [settings, conn] = _createSettingsAndConnection(); + + settings->UseAcrylic(true); + settings->Opacity(0.5f); + + auto core = createCore(*settings, *conn); + VERIFY_IS_NOT_NULL(core); + core->Initialize(270, 380, 1.0); + VERIFY_IS_TRUE(core->_initializedTerminal); + + Log::Comment(L"Adjust opacity at runtime: 0.5 -> 0.7 (delta +0.2)"); + core->AdjustOpacity(0.2f); + VERIFY_ARE_EQUAL(0.7f, core->Opacity()); + + Log::Comment(L"Reload settings with a different opacity (0.4). A reload builds a " + L"fresh settings object, so use a separate mock here."); + auto reloaded = winrt::make_self(); + reloaded->UseAcrylic(true); + reloaded->Opacity(0.4f); + core->UpdateSettings(*reloaded, *reloaded); + + Log::Comment(L"The runtime delta (+0.2) rides on top of the new settings opacity " + L"(0.4) -> 0.6, instead of being stomped back to 0.4."); + VERIFY_ARE_EQUAL(0.6f, core->Opacity()); + // Acrylic follows the resolved (runtime-preserved) opacity, which is < 1.0. + VERIFY_IS_TRUE(core->UseAcrylic()); + + Log::Comment(L"With no runtime adjustment, a reload follows the settings opacity."); + auto core2 = createCore(*settings, *conn); + core2->Initialize(270, 380, 1.0); + VERIFY_ARE_EQUAL(0.5f, core2->Opacity()); + auto reloaded2 = winrt::make_self(); + reloaded2->UseAcrylic(true); + reloaded2->Opacity(0.3f); + core2->UpdateSettings(*reloaded2, *reloaded2); + VERIFY_ARE_EQUAL(0.3f, core2->Opacity()); + } + + void ControlCoreTests::TestReadOnlySurvivesSettingsReload() + { + // GH#12424: a runtime read-only toggle must not be reset by a settings reload. + auto [settings, conn] = _createSettingsAndConnection(); + auto core = createCore(*settings, *conn); + VERIFY_IS_NOT_NULL(core); + core->Initialize(270, 380, 1.0); + + VERIFY_IS_FALSE(core->IsInReadOnlyMode()); + core->SetReadOnlyMode(true); + VERIFY_IS_TRUE(core->IsInReadOnlyMode()); + + auto reloaded = winrt::make_self(); + core->UpdateSettings(*reloaded, *reloaded); + VERIFY_IS_TRUE(core->IsInReadOnlyMode()); + } + void ControlCoreTests::TestFreeAfterClose() { { diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp index 51e53f1738..18e4e47f22 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp @@ -25,6 +25,7 @@ namespace TerminalCoreUnitTests TEST_CLASS(TerminalApiTest); TEST_METHOD(SetColorTableEntry); + TEST_METHOD(PreserveRuntimeColorAcrossSettingsReload); TEST_METHOD(CursorVisibilityViaStateMachine); @@ -60,6 +61,47 @@ void TerminalApiTest::SetColorTableEntry() VERIFY_THROWS(term._renderSettings.SetColorTableEntry(512, 100), std::exception); } +void TerminalApiTest::PreserveRuntimeColorAcrossSettingsReload() +{ + // GH#12424: a color table entry changed at runtime by a VT sequence (e.g. + // OSC 4) must survive a settings reload -- the new color scheme updates the + // baseline, but the runtime override is preserved. A non-overridden entry + // still picks up the new scheme. + Terminal term{ Terminal::TestDummyMarker{} }; + DummyRenderer renderer{ &term }; + term.Create({ 100, 100 }, 0, renderer); + + const auto makeUniformTable = [](uint8_t v) { + std::array table; + table.fill(winrt::Microsoft::Terminal::Core::Color{ v, v, v, 255 }); + return table; + }; + + // Initial load with scheme A (all entries 0x111111). + auto settingsA = winrt::make_self(100, 100, 100); + settingsA->SetColorTable(makeUniformTable(0x11)); + term.UpdateSettings(*settingsA); + + // Simulate an OSC color override at index 5. + const auto runtimeColor = static_cast(RGB(0xAB, 0xCD, 0xEF)); + term._renderSettings.SetColorTableEntry(5, runtimeColor); + VERIFY_ARE_EQUAL(runtimeColor, term._renderSettings.GetColorTableEntry(5)); + + // Reload with scheme B (all entries 0x222222). A reload builds a fresh + // settings object, so use a separate mock. + auto settingsB = winrt::make_self(100, 100, 100); + settingsB->SetColorTable(makeUniformTable(0x22)); + term.UpdateSettings(*settingsB); + + const auto schemeBColor = static_cast(til::color{ winrt::Microsoft::Terminal::Core::Color{ 0x22, 0x22, 0x22, 255 } }); + + Log::Comment(L"The runtime-overridden entry keeps its value across the reload."); + VERIFY_ARE_EQUAL(runtimeColor, term._renderSettings.GetColorTableEntry(5)); + + Log::Comment(L"A non-overridden entry picks up the new scheme."); + VERIFY_ARE_EQUAL(schemeBColor, term._renderSettings.GetColorTableEntry(3)); +} + // Terminal::_WriteBuffer used to enter infinite loops under certain conditions. // This test ensures that Terminal::_WriteBuffer doesn't get stuck when // PrintString() is called with more code units than the buffer width. diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp index 292f86faf6..b4e0f420a9 100644 --- a/src/renderer/base/RenderSettings.cpp +++ b/src/renderer/base/RenderSettings.cpp @@ -79,6 +79,31 @@ const std::array& RenderSettings::GetColorTable return _colorTable; } +// Routine Description: +// - Returns the color table entries that have diverged from the saved defaults +// since the last SaveDefaultSettings() -- i.e. entries that were changed at +// runtime, typically by OSC color sequences (OSC 4/10/11/12). A settings +// reload uses this to refresh the color-scheme baseline (the default table) +// without stomping these runtime overrides (GH#12424). +// - The frame colors (tab fg/bg) are intentionally excluded: they're driven by +// the separate tab-color settings/runtime path, not treated as VT overrides. +std::vector> RenderSettings::GetRuntimeColorTableOverrides() const +{ + std::vector> overrides; + for (size_t i = 0; i < TextColor::TABLE_SIZE; i++) + { + if (i == TextColor::FRAME_FOREGROUND || i == TextColor::FRAME_BACKGROUND) + { + continue; + } + if (_colorTable.at(i) != _defaultColorTable.at(i)) + { + overrides.emplace_back(i, _colorTable.at(i)); + } + } + return overrides; +} + // Routine Description: // - Resets the first 16 color table entries with default values. void RenderSettings::ResetColorTable() noexcept diff --git a/src/renderer/inc/RenderSettings.hpp b/src/renderer/inc/RenderSettings.hpp index 2b25f01e0a..ba012efd56 100644 --- a/src/renderer/inc/RenderSettings.hpp +++ b/src/renderer/inc/RenderSettings.hpp @@ -35,6 +35,7 @@ namespace Microsoft::Console::Render void SetRenderMode(const Mode mode, const bool enabled) noexcept; bool GetRenderMode(const Mode mode) const noexcept; const std::array& GetColorTable() const noexcept; + std::vector> GetRuntimeColorTableOverrides() const; void ResetColorTable() noexcept; void SetColorTableEntry(const size_t tableIndex, const COLORREF color); COLORREF GetColorTableEntry(const size_t tableIndex) const;