From 475b3878f675431b47aa7c11b8af403aa200b2b2 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 2 May 2024 18:12:34 +0200 Subject: [PATCH] Fix font axis/feature SUI issues (#17173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Something something code changes, fixes issue. Events need throwing, some events don't need throwing, some events need throttling to not overwhelm the flurble function within the gumbies component. This is all boilerplate and it works now. Closes #17171 Closes #17172 ## Validation Steps Performed * Resetting the font axis/feature expander keeps the add new button flyout sorted ✅ * Changing a font axis/feature value updates the preview ✅ * After changing the font, features/axes can still be edited ✅ --- .github/actions/spelling/expect/expect.txt | 5 + .../TerminalSettingsEditor/Appearances.cpp | 163 ++++++++++++------ .../TerminalSettingsEditor/Appearances.h | 20 +-- .../TerminalSettingsEditor/Appearances.xaml | 4 +- .../Profiles_Appearance.cpp | 24 ++- .../Profiles_Appearance.h | 10 +- 6 files changed, 145 insertions(+), 81 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 775694924d..5c0c14b0df 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -245,6 +245,7 @@ CONKBD conlibk conmsgl CONNECTINFO +connyection CONOUT conprops conpropsp @@ -1420,6 +1421,8 @@ PUCHAR pvar pwch PWDDMCONSOLECONTEXT +Pwease +pweview pws pwstr pwsz @@ -1898,6 +1901,7 @@ UVWXY UVWXYZ uwa uwp +uwu uxtheme Vanara vararg @@ -1971,6 +1975,7 @@ wdm webpage websites wekyb +wewoad wex wextest wextestclass diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.cpp b/src/cascadia/TerminalSettingsEditor/Appearances.cpp index c2823b955b..2ed71afc5a 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.cpp +++ b/src/cascadia/TerminalSettingsEditor/Appearances.cpp @@ -544,14 +544,33 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation return item; } + // Call this when all the _fontFaceDependents members have changed. void AppearanceViewModel::_notifyChangesForFontSettings() { - _NotifyChanges( - L"FontFaceDependents", - L"FontAxes", - L"FontFeatures", - L"HasFontAxes", - L"HasFontFeatures"); + _NotifyChanges(L"FontFaceDependents"); + _NotifyChanges(L"FontAxes"); + _NotifyChanges(L"FontFeatures"); + _NotifyChanges(L"HasFontAxes"); + _NotifyChanges(L"HasFontFeatures"); + } + + // Call this when used items moved into unused and vice versa. + // Because this doesn't recreate the IObservableVector instances, + // we don't need to notify the UI about changes to the "FontAxes" property. + void AppearanceViewModel::_notifyChangesForFontSettingsReactive(FontSettingIndex fontSettingsIndex) + { + _NotifyChanges(L"FontFaceDependents"); + switch (fontSettingsIndex) + { + case FontAxesIndex: + _NotifyChanges(L"HasFontAxes"); + break; + case FontFeaturesIndex: + _NotifyChanges(L"HasFontFeatures"); + break; + default: + break; + } } double AppearanceViewModel::LineHeight() const @@ -616,6 +635,30 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation FontWeight(winrt::Microsoft::Terminal::UI::Converters::DoubleToFontWeight(fontWeight)); } + const AppearanceViewModel::FontFaceDependentsData& AppearanceViewModel::FontFaceDependents() + { + if (!_fontFaceDependents) + { + _refreshFontFaceDependents(); + } + return *_fontFaceDependents; + } + + winrt::hstring AppearanceViewModel::MissingFontFaces() + { + return FontFaceDependents().missingFontFaces; + } + + winrt::hstring AppearanceViewModel::ProportionalFontFaces() + { + return FontFaceDependents().proportionalFontFaces; + } + + bool AppearanceViewModel::HasPowerlineCharacters() + { + return FontFaceDependents().hasPowerlineCharacters; + } + IObservableVector AppearanceViewModel::FontAxes() { return FontFaceDependents().fontSettingsUsed[FontAxesIndex]; @@ -628,7 +671,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void AppearanceViewModel::ClearFontAxes() { - _deleteAllFontSettings(FontAxesIndex); + _deleteAllFontKeyValuePairs(FontAxesIndex); } Model::FontConfig AppearanceViewModel::FontAxesOverrideSource() const @@ -648,7 +691,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void AppearanceViewModel::ClearFontFeatures() { - _deleteAllFontSettings(FontFeaturesIndex); + _deleteAllFontKeyValuePairs(FontFeaturesIndex); } Model::FontConfig AppearanceViewModel::FontFeaturesOverrideSource() const @@ -664,7 +707,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } const auto kvImpl = winrt::get_self(kv); - const auto fontSettingsIndex = kvImpl->IsFontFeature() ? 1 : 0; + const auto fontSettingsIndex = kvImpl->IsFontFeature() ? FontFeaturesIndex : FontAxesIndex; auto& d = *_fontFaceDependents; auto& used = d.fontSettingsUsed[fontSettingsIndex]; auto& unused = d.fontSettingsUnused[fontSettingsIndex]; @@ -686,7 +729,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation unused.erase(it); - _notifyChangesForFontSettings(); + _notifyChangesForFontSettingsReactive(fontSettingsIndex); } void AppearanceViewModel::DeleteFontKeyValuePair(const Editor::FontKeyValuePair& kv) @@ -699,10 +742,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation const auto kvImpl = winrt::get_self(kv); const auto tag = kvImpl->Key(); const auto tagString = tagToString(tag); - const auto fontSettingsIndex = kvImpl->IsFontFeature() ? 1 : 0; + const auto fontSettingsIndex = kvImpl->IsFontFeature() ? FontFeaturesIndex : FontAxesIndex; auto& d = *_fontFaceDependents; auto& used = d.fontSettingsUsed[fontSettingsIndex]; - auto& unused = d.fontSettingsUnused[fontSettingsIndex]; const auto fontInfo = _appearance.SourceProfile().FontInfo(); auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes(); @@ -719,20 +761,59 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation fontSettingsUser.Remove(std::wstring_view{ tagString }); - // Insert the item into the unused list, keeping it sorted by the display text. - { - const auto item = _createFontSettingMenuItem(*it); - const auto it = std::lower_bound(unused.begin(), unused.end(), item, [](const MenuFlyoutItemBase& lhs, const MenuFlyoutItemBase& rhs) { - const auto& a = lhs.as().Text(); - const auto& b = rhs.as().Text(); - return til::compare_linguistic_insensitive(a, b) < 0; - }); - unused.insert(it, item); - } - + _addMenuFlyoutItemToUnused(fontSettingsIndex, _createFontSettingMenuItem(*it)); used.RemoveAt(gsl::narrow(it - used.begin())); - _notifyChangesForFontSettings(); + _notifyChangesForFontSettingsReactive(fontSettingsIndex); + } + + void AppearanceViewModel::_deleteAllFontKeyValuePairs(FontSettingIndex fontSettingsIndex) + { + const auto fontInfo = _appearance.SourceProfile().FontInfo(); + if (fontSettingsIndex == FontFeaturesIndex) + { + fontInfo.ClearFontFeatures(); + } + else + { + fontInfo.ClearFontAxes(); + } + + if (!_fontFaceDependents) + { + return; + } + + auto& d = *_fontFaceDependents; + auto& used = d.fontSettingsUsed[fontSettingsIndex]; + + for (const auto& kv : used) + { + _addMenuFlyoutItemToUnused(fontSettingsIndex, _createFontSettingMenuItem(kv)); + } + + used.Clear(); + + _notifyChangesForFontSettingsReactive(fontSettingsIndex); + } + + // Inserts the given menu item into the unused list, while keeping it sorted by the display text. + void AppearanceViewModel::_addMenuFlyoutItemToUnused(FontSettingIndex index, MenuFlyoutItemBase item) + { + if (!_fontFaceDependents) + { + return; + } + + auto& d = *_fontFaceDependents; + auto& unused = d.fontSettingsUnused[index]; + + const auto it = std::lower_bound(unused.begin(), unused.end(), item, [](const MenuFlyoutItemBase& lhs, const MenuFlyoutItemBase& rhs) { + const auto& a = lhs.as().Text(); + const auto& b = rhs.as().Text(); + return til::compare_linguistic_insensitive(a, b) < 0; + }); + unused.insert(it, std::move(item)); } void AppearanceViewModel::UpdateFontSetting(const FontKeyValuePair* kvImpl) @@ -757,37 +838,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } std::ignore = fontSettingsUser.Insert(std::wstring_view{ tagString }, value); - } - - void AppearanceViewModel::_deleteAllFontSettings(FontSettingIndex fontSettingsIndex) - { - const auto fontInfo = _appearance.SourceProfile().FontInfo(); - if (fontSettingsIndex == FontFeaturesIndex) - { - fontInfo.ClearFontFeatures(); - } - else - { - fontInfo.ClearFontAxes(); - } - - if (!_fontFaceDependents) - { - return; - } - - auto& d = *_fontFaceDependents; - auto& used = d.fontSettingsUsed[fontSettingsIndex]; - auto& unused = d.fontSettingsUnused[fontSettingsIndex]; - - for (const auto& kv : used) - { - unused.emplace_back(_createFontSettingMenuItem(kv)); - } - - used.Clear(); - - _notifyChangesForFontSettings(); + // Pwease call Profiles_Appearance::_onProfilePropertyChanged to make the pweview connyection wewoad. Thanks!! uwu + // ...I hate this. + _NotifyChanges(L"uwu"); } void AppearanceViewModel::SetBackgroundImageOpacityFromPercentageValue(double percentageValue) diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.h b/src/cascadia/TerminalSettingsEditor/Appearances.h index 572b624098..7ad8a03ce4 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.h +++ b/src/cascadia/TerminalSettingsEditor/Appearances.h @@ -94,18 +94,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void SetFontWeightFromDouble(double fontWeight); - const FontFaceDependentsData& FontFaceDependents() - { - if (!_fontFaceDependents) - { - _refreshFontFaceDependents(); - } - return *_fontFaceDependents; - } - - winrt::hstring MissingFontFaces() { return FontFaceDependents().missingFontFaces; } - winrt::hstring ProportionalFontFaces() { return FontFaceDependents().proportionalFontFaces; } - bool HasPowerlineCharacters() { return FontFaceDependents().hasPowerlineCharacters; } + const FontFaceDependentsData& FontFaceDependents(); + winrt::hstring MissingFontFaces(); + winrt::hstring ProportionalFontFaces(); + bool HasPowerlineCharacters(); Windows::Foundation::Collections::IObservableVector FontAxes(); bool HasFontAxes() const; @@ -165,7 +157,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void _generateFontFeatures(IDWriteFontFace* fontFace, std::vector& list); Windows::UI::Xaml::Controls::MenuFlyoutItemBase _createFontSettingMenuItem(const Editor::FontKeyValuePair& kv); void _notifyChangesForFontSettings(); - void _deleteAllFontSettings(FontSettingIndex index); + void _notifyChangesForFontSettingsReactive(FontSettingIndex fontSettingsIndex); + void _deleteAllFontKeyValuePairs(FontSettingIndex index); + void _addMenuFlyoutItemToUnused(FontSettingIndex index, Windows::UI::Xaml::Controls::MenuFlyoutItemBase item); Model::AppearanceConfig _appearance; winrt::hstring _lastBgImagePath; diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.xaml b/src/cascadia/TerminalSettingsEditor/Appearances.xaml index 2f94cf4473..a3f643c53c 100644 --- a/src/cascadia/TerminalSettingsEditor/Appearances.xaml +++ b/src/cascadia/TerminalSettingsEditor/Appearances.xaml @@ -309,7 +309,7 @@ @@ -336,7 +336,7 @@ diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp index 5295d3042b..cdf1e851ae 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.cpp @@ -3,13 +3,11 @@ #include "pch.h" #include "Profiles_Appearance.h" -#include "Profiles_Appearance.g.cpp" + #include "ProfileViewModel.h" #include "PreviewConnection.h" -#include "EnumEntry.h" -#include -#include "..\WinRTUtils\inc\Utils.h" +#include "Profiles_Appearance.g.cpp" using namespace winrt::Windows::UI::Xaml; using namespace winrt::Windows::UI::Xaml::Navigation; @@ -64,10 +62,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation _Profile.DeleteUnfocusedAppearance(); } - void Profiles_Appearance::_onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const + void Profiles_Appearance::_onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) { - const auto settings = _Profile.TermSettings(); - _previewConnection->DisplayPowerlineGlyphs(_Profile.DefaultAppearance().HasPowerlineCharacters()); - _previewControl.UpdateControlSettings(settings, settings); + if (!_updatePreviewControl) + { + _updatePreviewControl = std::make_shared>( + winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), + std::chrono::milliseconds{ 100 }, + [this]() { + const auto settings = _Profile.TermSettings(); + _previewConnection->DisplayPowerlineGlyphs(_Profile.DefaultAppearance().HasPowerlineCharacters()); + _previewControl.UpdateControlSettings(settings, settings); + }); + } + + _updatePreviewControl->Run(); } } diff --git a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h index ab9ebf66fb..27c8e455a5 100644 --- a/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h +++ b/src/cascadia/TerminalSettingsEditor/Profiles_Appearance.h @@ -3,9 +3,12 @@ #pragma once -#include "Profiles_Appearance.g.h" -#include "Utils.h" +#include + #include "PreviewConnection.h" +#include "Utils.h" + +#include "Profiles_Appearance.g.h" namespace winrt::Microsoft::Terminal::Settings::Editor::implementation { @@ -26,10 +29,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation WINRT_PROPERTY(Editor::ProfileViewModel, Profile, nullptr); private: - void _onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&) const; + void _onProfilePropertyChanged(const IInspectable&, const PropertyChangedEventArgs&); winrt::com_ptr _previewConnection{ nullptr }; Microsoft::Terminal::Control::TermControl _previewControl{ nullptr }; + std::shared_ptr> _updatePreviewControl; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _ViewModelChangedRevoker; Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker _AppearanceViewModelChangedRevoker; Editor::IHostedInWindow _windowRoot;