Only update the icon of a tab it the icon actually _changed_

* fixes #1333 * Also tangentially fixes #2339
microsoft · Aug 9, 2019 · b94c767 · b94c767
1 parent 1e4e125
commit b94c767
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 24 deletions.
diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp
@@ -686,16 +686,18 @@ namespace winrt::TerminalApp::implementation
         if (lastFocusedProfileOpt.has_value())
         {
             const auto lastFocusedProfile = lastFocusedProfileOpt.value();
-
-            auto tabViewItem = tab->GetTabViewItem();
-            tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this, lastFocusedProfile, tabViewItem]() {
-                // _GetIconFromProfile has to run on the main thread
-                const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile);
-                if (matchingProfile)
-                {
-                    tabViewItem.Icon(App::_GetIconFromProfile(*matchingProfile));
-                }
-            });
+            const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile);
+            if (matchingProfile)
+            {
+                std::wstring path{ matchingProfile->GetIconPath() };
+                const auto envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(path.data()) };
+                winrt::hstring iconPath{ envExpandedPath };
+                tab->UpdateIcon(iconPath);
+            }
+            else
+            {
+                tab->UpdateIcon({});
+            }
         }
     }
 
@@ -928,7 +930,10 @@ namespace winrt::TerminalApp::implementation
         // Set this profile's tab to the icon the user specified
         if (profile != nullptr && profile->HasIcon())
         {
-            tabViewItem.Icon(_GetIconFromProfile(*profile));
+            std::wstring path{ profile->GetIconPath() };
+            const auto envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(path.data()) };
+            winrt::hstring iconPath{ envExpandedPath };
+            newTab->UpdateIcon(iconPath);
         }
 
         tabViewItem.PointerPressed({ this, &App::_OnTabClick });
@@ -1252,19 +1257,24 @@ namespace winrt::TerminalApp::implementation
     {
         if (profile.HasIcon())
         {
-            std::wstring path{ profile.GetIconPath() };
-            const auto envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(path.data()) };
-            winrt::hstring iconPath{ envExpandedPath };
-            winrt::Windows::Foundation::Uri iconUri{ iconPath };
-            Controls::BitmapIconSource iconSource;
-            // Make sure to set this to false, so we keep the RGB data of the
-            // image. Otherwise, the icon will be white for all the
-            // non-transparent pixels in the image.
-            iconSource.ShowAsMonochrome(false);
-            iconSource.UriSource(iconUri);
-            Controls::IconSourceElement elem;
-            elem.IconSource(iconSource);
-            return elem;
+            try
+            {
+                std::wstring path{ profile.GetIconPath() };
+                const auto envExpandedPath{ wil::ExpandEnvironmentStringsW<std::wstring>(path.data()) };
+                winrt::hstring iconPath{ envExpandedPath };
+                winrt::Windows::Foundation::Uri iconUri{ iconPath };
+                Controls::BitmapIconSource iconSource;
+                // Make sure to set this to false, so we keep the RGB data of the
+                // image. Otherwise, the icon will be white for all the
+                // non-transparent pixels in the image.
+                iconSource.ShowAsMonochrome(false);
+                iconSource.UriSource(iconUri);
+                Controls::IconSourceElement elem;
+                elem.IconSource(iconSource);
+                return elem;
+            }
+            CATCH_LOG();
+            return { nullptr };
         }
         else
         {

diff --git a/src/cascadia/TerminalApp/Tab.cpp b/src/cascadia/TerminalApp/Tab.cpp
@@ -142,6 +142,39 @@ void Tab::UpdateFocus()
     _rootPane->UpdateFocus();
 }
 
+void Tab::UpdateIcon(const winrt::hstring iconPath)
+{
+    // Don't reload our icon if it hasn't changed.
+    if (iconPath == _lastIconPath)
+    {
+        return;
+    }
+
+    _lastIconPath = iconPath;
+
+    _tabViewItem.Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [this]() {
+        Controls::IconSourceElement elem{};
+
+        if (!_lastIconPath.empty())
+        {
+            try
+            {
+                winrt::Windows::Foundation::Uri iconUri{ _lastIconPath };
+                Controls::BitmapIconSource iconSource;
+                // Make sure to set this to false, so we keep the RGB data of the
+                // image. Otherwise, the icon will be white for all the
+                // non-transparent pixels in the image.
+                iconSource.ShowAsMonochrome(false);
+                iconSource.UriSource(iconUri);
+                elem.IconSource(iconSource);
+            }
+            CATCH_LOG();
+        }
+
+        _tabViewItem.Icon(elem);
+    });
+}
+
 // Method Description:
 // - Gets the title string of the last focused terminal control in our tree.
 //   Returns the empty string if there is no such control.

diff --git a/src/cascadia/TerminalApp/Tab.h b/src/cascadia/TerminalApp/Tab.h
@@ -23,6 +23,8 @@ class Tab
     void AddHorizontalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
 
     void UpdateFocus();
+    void UpdateIcon(const winrt::hstring iconPath);
+
     void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
     void ResizePane(const winrt::TerminalApp::Direction& direction);
     void NavigateFocus(const winrt::TerminalApp::Direction& direction);
@@ -37,6 +39,7 @@ class Tab
 
 private:
     std::shared_ptr<Pane> _rootPane{ nullptr };
+    winrt::hstring _lastIconPath{};
 
     bool _focused{ false };
     winrt::Microsoft::UI::Xaml::Controls::TabViewItem _tabViewItem{ nullptr };