Repository: microsoft/terminal (language: cpp)

Issue:
Support exe/dll paths in the 'icon' setting
# Support exe/icon paths in the 'icon' setting of a profile

The icon setting of a profile should allow the path to an EXE or DLL to be specified, along with an optional ID for an icon resource in the file.

For example, I should be able to create a profile using icons from SHELL32:
```
    ...
    "icon": "c:\Windows\System32\SHELL32.dll,210",
    ...
```

Optionally, if the icon is missing and the executable from the command line has icons, pick the default icon, much like how the shell does.


Discussion / hints:
I agree. or things like "icon": "C:\Program Files\PowerShell\6-preview\pwsh.exe",

Yeah, it would be pretty cool if we did this.
Notes: Right now we're using the XAML resource loader to get at our icons. If we want to support EXE/other PE resources we'll need to fall back to a more win32-like resource loader. There are real benefits to the XAML one, such as _it choosing the right icon scale_ and supporting vector graphics and stuff like that. I've tagged it `help-wanted`.
@DHowett-MSFT afaik not even the settings app in Windows uses SVG graphics but PNGs. I stumbled across them once. I see the point of having this more modern and getting rid of old ico restrictions.
This is highly related to #705 (but definitely different)
> Notes: (…) There are real benefits to the XAML one, such as _it choosing the right icon scale_ and supporting vector graphics and stuff like that. I've tagged it `help-wanted`.

Just FYI: technically single icon inside PE resources can contain multiple icons (with various color depths, sizes - up to 256x256 etc.), that is how you could have same but actually different icon for title bar and for application itself. (that is, smaller one was used for title bar).

ofc, afaik vector graphics is/was not an option.
Notes from some tabs I had open for the last few weeks:
* dotnet uses `System.Windows.Interop.Imaging.CreateBitmapSourceFromHIcon`, which obv we can't use. 
* https://github.com/dotnet/wpf/blob/89d172db0b7a192de720c6cfba5e28a1e7d46123/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/InterOp/Imaging.cs#L77
* https://github.com/dotnet/wpf/blob/89d172db0b7a192de720c6cfba5e28a1e7d46123/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Imaging/InteropBitmapSource.cs#L91-L96
* [`IWICImagingFactory_CreateBitmapFromHICON_Proxy`](https://docs.microsoft.com/en-us/windows/win32/wic/-wic-codec-iwicimagingfactory-createbitmapfromhicon-proxy)
* Data from that is accessed as in [`IWICBitmapLock::GetDataPointer`](https://docs.microsoft.com/en-us/windows/win32/api/wincodec/nf-wincodec-iwicbitmaplock-getdatapointer)

Still not sure if that connects up to a `BitmapImage`. Maybe I should look in the BitmapImage source itself - it can apparently read from `.ico` files, so there's probably some overlap between reading an `.ico` and reading an `HICON`.



More notes:
* `Window::s_RegisterWindowClass` is where we set the HICON for the console window.
* This is unpacked in `Icon::GetIcons`, after a call to:
* `Icon::LoadIconsFromPath`, in `win32/SystemConfigurationProvider::GetSettingsFromLink` (see also #11759)
  * first check: `ShortcutSerialization::s_GetLinkValues` 
    * `ShortcutSerialization::s_GetLoadedShellLinkForShortcut` to load the `IShellLink` for the lnk
    * `psl->GetIconLocation` to load the icon from the `IShellLink`. 
  * Then, we fall back to the title?
  * `SystemConfigurationProvider::GetSettingsFromLink` sets the icon GLOBALLY. It probably initializes other state. If we want to reuse that to pull the icon out of an lnk before handoff, the we should be SURE it's not setting any other magic global state. We can use a dummy console `Settings` to hold data if need be. 
* see 1dd6e9934

Fix patch:
diff --git a/src/cascadia/LocalTests_SettingsModel/pch.h b/src/cascadia/LocalTests_SettingsModel/pch.h
index e93c7f6fc0e..b018bdaa4a6 100644
--- a/src/cascadia/LocalTests_SettingsModel/pch.h
+++ b/src/cascadia/LocalTests_SettingsModel/pch.h
@@ -41,11 +41,13 @@ Author(s):
 #include <winrt/Windows.system.h>
 #include <winrt/Windows.Foundation.h>
 #include <winrt/Windows.Foundation.Collections.h>
+#include <Windows.Graphics.Imaging.Interop.h>
 #include <winrt/windows.ui.core.h>
 #include <winrt/Windows.ui.input.h>
 #include <winrt/Windows.UI.Xaml.Controls.h>
 #include <winrt/Windows.UI.Xaml.Controls.Primitives.h>
 #include <winrt/Windows.ui.xaml.media.h>
+#include <winrt/Windows.UI.Xaml.Media.Imaging.h>
 #include <winrt/Windows.ui.xaml.input.h>
 #include <winrt/Windows.UI.Xaml.Markup.h>
 #include <winrt/Windows.UI.Xaml.Documents.h>
diff --git a/src/cascadia/TerminalApp/CommandPalette.xaml b/src/cascadia/TerminalApp/CommandPalette.xaml
index 476cf029b30..368de8e8250 100644
--- a/src/cascadia/TerminalApp/CommandPalette.xaml
+++ b/src/cascadia/TerminalApp/CommandPalette.xaml
@@ -51,10 +51,10 @@
                         <!--  gutter for scrollbar  -->
                     </Grid.ColumnDefinitions>
 
-                    <IconSourceElement Grid.Column="0"
-                                       Width="16"
-                                       Height="16"
-                                       IconSource="{x:Bind Item.Icon, Mode=OneWay, Converter={StaticResource IconSourceConverter}}" />
+                    <ContentPresenter Grid.Column="0"
+                                      Width="16"
+                                      Height="16"
+                                      Content="{x:Bind Item.ResolvedIcon, Mode=OneWay}" />
 
                     <local:HighlightedTextControl Grid.Column="1"
                                                   HorizontalAlignment="Left"
@@ -101,10 +101,10 @@
                         <!--  gutter for scrollbar  -->
                     </Grid.ColumnDefinitions>
 
-                    <IconSourceElement Grid.Column="0"
-                                       Width="16"
-                                       Height="16"
-                                       IconSource="{x:Bind Item.Icon, Mode=OneWay, Converter={StaticResource IconSourceConverter}}" />
+                    <ContentPresenter Grid.Column="0"
+                                      Width="16"
+                                      Height="16"
+                                      Content="{x:Bind Item.ResolvedIcon, Mode=OneWay}" />
 
                     <local:HighlightedTextControl Grid.Column="1"
                                                   HorizontalAlignment="Left"
@@ -171,10 +171,10 @@
                                       Visibility="{x:Bind Item.(local:TabPaletteItem.TabStatus).IsProgressRingActive, Mode=OneWay}"
                                       Value="{x:Bind Item.(local:TabPaletteItem.TabStatus).ProgressValue, Mode=OneWay}" />
 
-                    <IconSourceElement Grid.Column="0"
-                                       Width="16"
-                                       Height="16"
-                                       IconSource="{x:Bind Item.Icon, Mode=OneWay, Converter={StaticResource IconSourceConverter}}" />
+                    <ContentPresenter Grid.Column="0"
+                                      Width="16"
+                                      Height="16"
+                                      Content="{x:Bind Item.ResolvedIcon, Mode=OneWay}" />
 
                     <local:HighlightedTextControl Grid.Column="1"
                                                   HorizontalAlignment="Left"
diff --git a/src/cascadia/TerminalApp/PaletteItem.cpp b/src/cascadia/TerminalApp/PaletteItem.cpp
index 697898dbcb0..443f26f0ad2 100644
--- a/src/cascadia/TerminalApp/PaletteItem.cpp
+++ b/src/cascadia/TerminalApp/PaletteItem.cpp
@@ -15,4 +15,11 @@ using namespace winrt::Windows::System;
 
 namespace winrt::TerminalApp::implementation
 {
+    Controls::IconElement PaletteItem::ResolvedIcon()
+    {
+        const auto icon = IconPathConverter::IconWUX(Icon());
+        icon.Width(16);
+        icon.Height(16);
+        return icon;
+    }
 }
diff --git a/src/cascadia/TerminalApp/PaletteItem.h b/src/cascadia/TerminalApp/PaletteItem.h
index 973de7590bb..ca265307fe0 100644
--- a/src/cascadia/TerminalApp/PaletteItem.h
+++ b/src/cascadia/TerminalApp/PaletteItem.h
@@ -9,6 +9,8 @@ namespace winrt::TerminalApp::implementation
     struct PaletteItem : PaletteItemT<PaletteItem>
     {
     public:
+        Windows::UI::Xaml::Controls::IconElement ResolvedIcon();
+
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
 
         WINRT_OBSERVABLE_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers);
diff --git a/src/cascadia/TerminalApp/PaletteItem.idl b/src/cascadia/TerminalApp/PaletteItem.idl
index a993dcd5c05..c4273de5d1c 100644
--- a/src/cascadia/TerminalApp/PaletteItem.idl
+++ b/src/cascadia/TerminalApp/PaletteItem.idl
@@ -8,5 +8,6 @@ namespace TerminalApp
         String Name;
         String KeyChordText;
         String Icon;
+        Windows.UI.Xaml.Controls.IconElement ResolvedIcon { get; };
     }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index b29b1489c3d..29828dbfcf6 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -853,12 +853,9 @@ namespace winrt::TerminalApp::implementation
             // this flyout item.
             if (!profile.Icon().empty())
             {
-                const auto iconSource{ IconPathConverter().IconSourceWUX(profile.Icon()) };
-
-                WUX::Controls::IconSourceElement iconElement;
-                iconElement.IconSource(iconSource);
-                profileMenuItem.Icon(iconElement);
-                Automation::AutomationProperties::SetAccessibilityView(iconElement, Automation::Peers::AccessibilityView::Raw);
+                auto icon = IconPathConverter::IconWUX(profile.Icon());
+                Automation::AutomationProperties::SetAccessibilityView(icon, Automation::Peers::AccessibilityView::Raw);
+                profileMenuItem.Icon(icon);
             }
 
             if (profile.Guid() == defaultProfileGuid)
diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp
index 26629625aa4..ef79f1db547 100644
--- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp
+++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp
@@ -568,11 +568,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         MUX::Controls::NavigationViewItem profileNavItem;
         profileNavItem.Content(box_value(profile.Name()));
         profileNavItem.Tag(box_value<Editor::ProfileViewModel>(profile));
-
-        const auto iconSource{ IconPathConverter::IconSourceWUX(profile.Icon()) };
-        WUX::Controls::IconSourceElement icon;
-        icon.IconSource(iconSource);
-        profileNavItem.Icon(icon);
+        profileNavItem.Icon(IconPathConverter::IconWUX(profile.Icon()));
 
         // Update the menu item when the icon/name changes
         auto weakMenuItem{ make_weak(profileNavItem) };
@@ -582,10 +578,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                 const auto& tag{ menuItem.Tag().as<Editor::ProfileViewModel>() };
                 if (args.PropertyName() == L"Icon")
                 {
-                    const auto iconSource{ IconPathConverter::IconSourceWUX(tag.Icon()) };
-                    WUX::Controls::IconSourceElement icon;
-                    icon.IconSource(iconSource);
-                    menuItem.Icon(icon);
+                    menuItem.Icon(IconPathConverter::IconWUX(tag.Icon()));
                 }
                 else if (args.PropertyName() == L"Name")
                 {
diff --git a/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp b/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
index 04fff1537aa..e926efb7c70 100644
--- a/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
+++ b/src/cascadia/TerminalSettingsModel/IconPathConverter.cpp
@@ -4,9 +4,21 @@
 
 #include "Utils.h"
 
+#include <Shlobj.h>
+#include <Shlobj_core.h>
+#include <wincodec.h>
+
+namespace winrt
+{
+    namespace MUX = Microsoft::UI::Xaml;
+}
+
 using namespace winrt::Windows;
 using namespace winrt::Windows::UI::Xaml;
 
+using namespace winrt::Windows::Graphics::Imaging;
+using namespace winrt::Windows::Storage::Streams;
+
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 {
 // These are templates that help us figure out which BitmapIconSource/FontIconSource to use for a given IconSource.
@@ -197,13 +209,161 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         throw hresult_not_implemented();
     }
 
-    Windows::UI::Xaml::Controls::IconSource IconPathConverter::IconSourceWUX(hstring path)
+    Windows::UI::Xaml::Controls::IconSource _IconSourceWUX(hstring path)
     {
         return _getIconSource<Windows::UI::Xaml::Controls::IconSource>(path);
     }
 
-    Microsoft::UI::Xaml::Controls::IconSource IconPathConverter::IconSourceMUX(hstring path)
+    Microsoft::UI::Xaml::Controls::IconSource _IconSourceMUX(hstring path)
     {
         return _getIconSource<Microsoft::UI::Xaml::Controls::IconSource>(path);
     }
+
+    SoftwareBitmap _convertToSoftwareBitmap(HICON hicon,
+                                            BitmapPixelFormat pixelFormat,
+                                            BitmapAlphaMode alphaMode,
+                                            IWICImagingFactory* imagingFactory)
+    {
+        // Load the icon into an IWICBitmap
+        wil::com_ptr<IWICBitmap> iconBitmap;
+        THROW_IF_FAILED(imagingFactory->CreateBitmapFromHICON(hicon, iconBitmap.put()));
+
+        // Put the IWICBitmap into a SoftwareBitmap. This may fail if WICBitmap's format is not supported by
+        // SoftwareBitmap. CreateBitmapFromHICON always creates RGBA8 so we're ok.
+        auto softwareBitmap = winrt::capture<SoftwareBitmap>(
+            winrt::create_instance<ISoftwareBitmapNativeFactory>(CLSID_SoftwareBitmapNativeFactory),
+            &ISoftwareBitmapNativeFactory::CreateFromWICBitmap,
+            iconBitmap.get(),
+            false);
+
+        // Convert the pixel format and alpha mode if necessary
+        if (softwareBitmap.BitmapPixelFormat() != pixelFormat || softwareBitmap.BitmapAlphaMode() != alphaMode)
+        {
+            softwareBitmap = SoftwareBitmap::Convert(softwareBitmap, pixelFormat, alphaMode);
+        }
+
+        return softwareBitmap;
+    }
+
+    SoftwareBitmap _getBitmapFromIconFileAsync(const winrt::hstring& iconPath,
+                                               int32_t iconIndex,
+                                               uint32_t iconSize)
+    {
+        wil::unique_hicon hicon;
+        LOG_IF_FAILED(SHDefExtractIcon(iconPath.c_str(), iconIndex, 0, &hicon, nullptr, iconSize));
+
+        if (!hicon)
+        {
+            return nullptr;
+        }
+
+        wil::com_ptr<IWICImagingFactory> wicImagingFactory;
+        THROW_IF_FAILED(CoCreateInstance(CLSID_WICImagingFactory, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&wicImagingFactory)));
+
+        return _convertToSoftwareBitmap(hicon.get(),
+                                        BitmapPixelFormat::Bgra8,
+                                        BitmapAlphaMode::Premultiplied,
+                                        wicImagingFactory.get());
+    }
+
+    // Method Description:
+    // - Attempt to get the icon index from the icon path provided
+    // Arguments:
+    // - iconPath: the full icon path, including the index if present
+    // - iconPathWithoutIndex: the place to store the icon path, sans the index if present
+    // Return Value:
+    // - nullopt if the iconPath is not an exe/dll/lnk file in the first place
+    // - 0 if the iconPath is an exe/dll/lnk file but does not contain an index (i.e. we default
+    //   to the first icon in the file)
+    // - the icon index if the iconPath is an exe/dll/lnk file and contains an index
+    std::optional<int> _getIconIndex(const winrt::hstring& iconPath, std::wstring_view& iconPathWithoutIndex)
+    {
+        const auto pathView = std::wstring_view{ iconPath };
+        // Does iconPath have a comma in it? If so, split the string on the
+        // comma and look for the index and extension.
+        const auto commaIndex = pathView.find(L',');
+
+        // split the path on the comma
+        iconPathWithoutIndex = pathView.substr(0, commaIndex);
+
+        // It's an exe, dll, or lnk, so we need to extract the icon from the file.
+        if (!til::ends_with(iconPathWithoutIndex, L".exe") &&
+            !til::ends_with(iconPathWithoutIndex, L".dll") &&
+            !til::ends_with(iconPathWithoutIndex, L".lnk"))
+        {
+            return std::nullopt;
+        }
+
+        if (commaIndex != std::wstring::npos)
+        {
+            // Convert the string iconIndex to an int
+            const auto index = til::to_ulong(pathView.substr(commaIndex + 1));
+            if (index == til::to_ulong_error)
+            {
+                return std::nullopt;
+            }
+            return static_cast<int>(index);
+        }
+
+        // We had a binary path, but no index. Default to 0.
+        return 0;
+    }
+
+    winrt::Windows::UI::Xaml::Media::Imaging::SoftwareBitmapSource _getImageIconSourceForBinary(std::wstring_view iconPathWithoutIndex,
+                                                                                                int index)
+    {
+        // Try:
+        // * c:\Windows\System32\SHELL32.dll, 210
+        // * c:\Windows\System32\notepad.exe, 0
+        // * C:\Program Files\PowerShell\6-preview\pwsh.exe, 0 (this doesn't exist for me)
+        // * C:\Program Files\PowerShell\7\pwsh.exe, 0
+
+        const auto swBitmap{ _getBitmapFromIconFileAsync(winrt::hstring{ iconPathWithoutIndex }, index, 32) };
+        if (swBitmap == nullptr)
+        {
+            return nullptr;
+        }
+
+        winrt::Windows::UI::Xaml::Media::Imaging::SoftwareBitmapSource bitmapSource{};
+        bitmapSource.SetBitmapAsync(swBitmap);
+        return bitmapSource;
+    }
+
+    MUX::Controls::IconSource IconPathConverter::IconSourceMUX(const winrt::hstring& iconPath)
+    {
+        std::wstring_view iconPathWithoutIndex;
+        const auto indexOpt = _getIconIndex(iconPath, iconPathWithoutIndex);
+        if (!indexOpt.has_value())
+        {
+            return _IconSourceMUX(iconPath);
+        }
+
+        const auto bitmapSource = _getImageIconSourceForBinary(iconPathWithoutIndex, indexOpt.value());
+
+        MUX::Controls::ImageIconSource imageIconSource{};
+        imageIconSource.ImageSource(bitmapSource);
+
+        return imageIconSource;
+    }
+
+    Windows::UI::Xaml::Controls::IconElement IconPathConverter::IconWUX(const winrt::hstring& iconPath)
+    {
+        std::wstring_view iconPathWithoutIndex;
+        const auto indexOpt = _getIconIndex(iconPath, iconPathWithoutIndex);
+        if (!indexOpt.has_value())
+        {
+            auto source = _IconSourceWUX(iconPath);
+            Controls::IconSourceElement icon;
+            icon.IconSource(source);
+            return icon;
+        }
+
+        const auto bitmapSource = _getImageIconSourceForBinary(iconPathWithoutIndex, indexOpt.value());
+
+        winrt::Microsoft::UI::Xaml::Controls::ImageIcon icon{};
+        icon.Source(bitmapSource);
+        icon.Width(32);
+        icon.Height(32);
+        return icon;
+    }
 }
diff --git a/src/cascadia/TerminalSettingsModel/IconPathConverter.h b/src/cascadia/TerminalSettingsModel/IconPathConverter.h
index 51604ce1147..a9eee104b94 100644
--- a/src/cascadia/TerminalSettingsModel/IconPathConverter.h
+++ b/src/cascadia/TerminalSettingsModel/IconPathConverter.h
@@ -18,8 +18,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
                                                       const Windows::Foundation::IInspectable& parameter,
                                                       const hstring& language);
 
-        static Windows::UI::Xaml::Controls::IconSource IconSourceWUX(hstring path);
-        static Microsoft::UI::Xaml::Controls::IconSource IconSourceMUX(hstring path);
+        static Windows::UI::Xaml::Controls::IconElement IconWUX(const winrt::hstring& iconPath);
+        static Microsoft::UI::Xaml::Controls::IconSource IconSourceMUX(const winrt::hstring& iconPath);
     };
 }
 
diff --git a/src/cascadia/TerminalSettingsModel/IconPathConverter.idl b/src/cascadia/TerminalSettingsModel/IconPathConverter.idl
index 6a1d776a43b..bbe1e055b8c 100644
--- a/src/cascadia/TerminalSettingsModel/IconPathConverter.idl
+++ b/src/cascadia/TerminalSettingsModel/IconPathConverter.idl
@@ -15,7 +15,7 @@ namespace Microsoft.Terminal.Settings.Model
     {
         IconPathConverter();
 
-        static Windows.UI.Xaml.Controls.IconSource IconSourceWUX(String path);
+        static Windows.UI.Xaml.Controls.IconElement IconWUX(String path);
         static Microsoft.UI.Xaml.Controls.IconSource IconSourceMUX(String path);
     };
 
diff --git a/src/cascadia/TerminalSettingsModel/pch.h b/src/cascadia/TerminalSettingsModel/pch.h
index 3f0220ef1a4..f8266b66fd1 100644
--- a/src/cascadia/TerminalSettingsModel/pch.h
+++ b/src/cascadia/TerminalSettingsModel/pch.h
@@ -29,11 +29,15 @@
 #include <winrt/Windows.ApplicationModel.h>
 #include <winrt/Windows.Foundation.Collections.h>
 #include <winrt/Windows.Foundation.h>
+#include <winrt/Windows.Graphics.Imaging.h>
+#include <Windows.Graphics.Imaging.Interop.h>
 #include <winrt/Windows.Storage.h>
+#include <winrt/Windows.Storage.Streams.h>
 #include <winrt/Windows.System.h>
 #include <winrt/Windows.UI.Core.h>
 #include <winrt/Windows.UI.Xaml.Controls.h>
 #include <winrt/Windows.UI.Xaml.Media.h>
+#include <winrt/Windows.UI.Xaml.Media.Imaging.h>
 
 #include <winrt/Microsoft.UI.Xaml.Controls.h>
 


Test patch: