From ae91734e3f73bc87a84d6ddbd766afc8828b4ed3 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Thu, 2 May 2024 13:01:40 +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 | 157 ++++++++++++------ .../TerminalSettingsEditor/Appearances.h | 20 +-- .../TerminalSettingsEditor/Appearances.xaml | 4 +- .../Profiles_Appearance.cpp | 24 ++- .../Profiles_Appearance.h | 10 +- 6 files changed, 142 insertions(+), 78 deletions(-) diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index 775694924d3..5c0c14b0df9 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 c2823b955bc..2ed71afc5a3 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,47 +761,13 @@ 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(); - } - - void AppearanceViewModel::UpdateFontSetting(const FontKeyValuePair* kvImpl) - { - const auto tag = kvImpl->Key(); - const auto value = kvImpl->Value(); - const auto tagString = tagToString(tag); - const auto fontInfo = _appearance.SourceProfile().FontInfo(); - auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes(); - - if (!fontSettingsUser) - { - fontSettingsUser = winrt::single_threaded_map(); - if (kvImpl->IsFontFeature()) - { - fontInfo.FontFeatures(fontSettingsUser); - } - else - { - fontInfo.FontAxes(fontSettingsUser); - } - } - - std::ignore = fontSettingsUser.Insert(std::wstring_view{ tagString }, value); + _notifyChangesForFontSettingsReactive(fontSettingsIndex); } - void AppearanceViewModel::_deleteAllFontSettings(FontSettingIndex fontSettingsIndex) + void AppearanceViewModel::_deleteAllFontKeyValuePairs(FontSettingIndex fontSettingsIndex) { const auto fontInfo = _appearance.SourceProfile().FontInfo(); if (fontSettingsIndex == FontFeaturesIndex) @@ -778,16 +786,61 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation auto& d = *_fontFaceDependents; auto& used = d.fontSettingsUsed[fontSettingsIndex]; - auto& unused = d.fontSettingsUnused[fontSettingsIndex]; for (const auto& kv : used) { - unused.emplace_back(_createFontSettingMenuItem(kv)); + _addMenuFlyoutItemToUnused(fontSettingsIndex, _createFontSettingMenuItem(kv)); } used.Clear(); - _notifyChangesForFontSettings(); + _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) + { + const auto tag = kvImpl->Key(); + const auto value = kvImpl->Value(); + const auto tagString = tagToString(tag); + const auto fontInfo = _appearance.SourceProfile().FontInfo(); + auto fontSettingsUser = kvImpl->IsFontFeature() ? fontInfo.FontFeatures() : fontInfo.FontAxes(); + + if (!fontSettingsUser) + { + fontSettingsUser = winrt::single_threaded_map(); + if (kvImpl->IsFontFeature()) + { + fontInfo.FontFeatures(fontSettingsUser); + } + else + { + fontInfo.FontAxes(fontSettingsUser); + } + } + + std::ignore = fontSettingsUser.Insert(std::wstring_view{ tagString }, value); + // 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 572b6240984..7ad8a03ce47 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 2f94cf4473b..a3f643c53c3 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 5295d3042bb..cdf1e851ae2 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 ab9ebf66fb0..27c8e455a52 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;