try to cache icons for differetnt DPIs I guess?

src/interactivity/win32/icon.cpp

@@ -8,6 +8,7 @@
 #include "window.hpp"
 
 #include "../inc/ServiceLocator.hpp"
+#include "windowdpiapi.hpp"
 
 using namespace Microsoft::Console::Interactivity::Win32;
 
@@ -21,7 +22,7 @@ static constexpr uint32_t LR_EXACTSIZEONLY{ 0x10000 };
 // larger than 256 returns the input value, which would result in the 256px icon being used.
 static int SnapIconSize(int cx)
 {
-    static constexpr int rgSizes[] = { 16, 32, 48, 256 };
+    static constexpr int rgSizes[] = { 16, 24, 32, 48, 64, 256 };
     for (auto sz : rgSizes)
     {
         if (cx <= sz)
@@ -240,14 +241,14 @@ static UINT ConExtractIcons(PCWSTR szFileName, int nIconIndex, int cxIcon, int c
     return result;
 }
 
-static UINT ConExtractIconInBothSizesW(PCWSTR szFileName, int nIconIndex, HICON* phiconLarge, HICON* phiconSmall)
+static UINT ConExtractIconInBothSizesW(int dpi, PCWSTR szFileName, int nIconIndex, HICON* phiconLarge, HICON* phiconSmall)
 {
     HICON ahicon[2] = { nullptr, nullptr };
     auto result = ConExtractIcons(
         szFileName,
         nIconIndex,
-        MAKELONG(GetSystemMetrics(SM_CXICON), GetSystemMetrics(SM_CXSMICON)),
-        MAKELONG(GetSystemMetrics(SM_CYICON), GetSystemMetrics(SM_CYSMICON)),
+        MAKELONG(GetSystemMetricsForDpi(dpi, SM_CXICON), GetSystemMetricsForDpi(dpi, SM_CXSMICON)),
+        MAKELONG(GetSystemMetricsForDpi(dpi, SM_CYICON), GetSystemMetricsForDpi(dpi, SM_CYSMICON)),
         ahicon,
         2,
         0);
@@ -259,23 +260,18 @@ static UINT ConExtractIconInBothSizesW(PCWSTR szFileName, int nIconIndex, HICON*
 }
 // Excerpted Region Ends
 
-Icon::Icon() :
-    _fInitialized(false),
-    _hDefaultIcon(nullptr),
-    _hDefaultSmIcon(nullptr),
-    _hIcon(nullptr),
-    _hSmIcon(nullptr)
+Icon::Icon()
 {
-}
-
-Icon::~Icon()
-{
-    // Do not destroy icon handles. They're shared icons as they were loaded from LoadIcon/LoadImage.
-    // http://msdn.microsoft.com/en-us/library/windows/desktop/ms648063(v=vs.85).aspx
-
-    // Do destroy icons from ExtractIconEx. They're not shared.
-    // http://msdn.microsoft.com/en-us/library/windows/desktop/ms648069(v=vs.85).aspx
-    _DestroyNonDefaultIcons();
+#pragma warning(push)
+#pragma warning(disable : 4302) // typecast warning from MAKEINTRESOURCE
+    _hDefaultIcon = LoadIconW(nullptr, MAKEINTRESOURCE(IDI_APPLICATION));
+    _hDefaultSmIcon = (HICON)LoadImageW(nullptr,
+                                        MAKEINTRESOURCE(IDI_APPLICATION),
+                                        IMAGE_ICON,
+                                        GetSystemMetrics(SM_CXSMICON),
+                                        GetSystemMetrics(SM_CYSMICON),
+                                        LR_SHARED);
+#pragma warning(pop)
 }
 
 // Routine Description:
@@ -297,64 +293,48 @@ Icon& Icon::Instance()
 // - phSmIcon - The small icon representation.
 // Return Value:
 // - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::GetIcons(_Out_opt_ HICON* const phIcon, _Out_opt_ HICON* const phSmIcon)
+[[nodiscard]] HRESULT Icon::GetIcons(int dpi, _Out_opt_ HICON* const phIcon, _Out_opt_ HICON* const phSmIcon)
 {
-    auto hr = S_OK;
-
-    if (nullptr != phIcon)
+    auto found{ _iconHandlesPerDpi.find(dpi) };
+    if (found == _iconHandlesPerDpi.end())
     {
-        hr = _GetAvailableIconFromReference(_hIcon, _hDefaultIcon, phIcon);
+        std::ignore = LoadIconsForDpi(dpi);
+        found = _iconHandlesPerDpi.find(dpi);
     }
 
-    if (SUCCEEDED(hr))
+    if (phIcon)
     {
-        if (nullptr != phSmIcon)
+        if (found != _iconHandlesPerDpi.end())
         {
-            hr = _GetAvailableIconFromReference(_hSmIcon, _hDefaultSmIcon, phSmIcon);
+            *phIcon = found->second.first.get();
+        }
+        if (!*phIcon)
+        {
+            *phIcon = _hDefaultIcon;
+        }
+        if (!*phIcon)
+        {
+            return E_FAIL;
         }
     }
 
-    return hr;
-}
-
-// Routine Description:
-// - Sets custom icons onto the class or resets the icons to defaults. Use a null handle to reset an icon to its default value.
-// Arguments:
-// - hIcon - The large icon handle or null to reset to default
-// - hSmIcon - The small icon handle or null to reset to default
-// Return Value:
-// - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::SetIcons(const HICON hIcon, const HICON hSmIcon)
-{
-    auto hr = _SetIconFromReference(_hIcon, hIcon);
-
-    if (SUCCEEDED(hr))
+    if (phSmIcon)
     {
-        hr = _SetIconFromReference(_hSmIcon, hSmIcon);
-    }
-
-    if (SUCCEEDED(hr))
-    {
-        HICON hNewIcon;
-        HICON hNewSmIcon;
-
-        hr = GetIcons(&hNewIcon, &hNewSmIcon);
-
-        if (SUCCEEDED(hr))
+        if (found != _iconHandlesPerDpi.end())
         {
-            // Special case. If we had a non-default big icon and a default small icon, set the small icon to null when updating the window.
-            // This will cause the large one to be stretched and used as the small one.
-            if (hNewIcon != _hDefaultIcon && hNewSmIcon == _hDefaultSmIcon)
-            {
-                hNewSmIcon = nullptr;
-            }
-
-            PostMessageW(ServiceLocator::LocateConsoleWindow()->GetWindowHandle(), WM_SETICON, ICON_BIG, (LPARAM)hNewIcon);
-            PostMessageW(ServiceLocator::LocateConsoleWindow()->GetWindowHandle(), WM_SETICON, ICON_SMALL, (LPARAM)hNewSmIcon);
+            *phSmIcon = found->second.second.get();
+        }
+        if (!*phSmIcon)
+        {
+            *phSmIcon = _hDefaultSmIcon;
+        }
+        if (!*phSmIcon)
+        {
+            return E_FAIL;
         }
     }
 
-    return hr;
+    return S_OK;
 }
 
 // Routine Description:
@@ -367,20 +347,22 @@ Icon& Icon::Instance()
 // - S_OK or HRESULT failure code.
 [[nodiscard]] HRESULT Icon::LoadIconsFromPath(_In_ PCWSTR pwszIconLocation, const int nIconIndex)
 {
-    auto hr = S_OK;
+    _iconPathAndIndex = { std::wstring{ pwszIconLocation }, nIconIndex };
+    _iconHandlesPerDpi.clear();
+    return LoadIconsForDpi(96);
+}
 
-    // Return value is count of icons extracted, which is redundant with filling the pointers.
-    // http://msdn.microsoft.com/en-us/library/windows/desktop/ms648069(v=vs.85).aspx
-    ConExtractIconInBothSizesW(pwszIconLocation, nIconIndex, &_hIcon, &_hSmIcon);
-
-    // If the large icon failed, then ensure that we use the defaults.
-    if (_hIcon == nullptr)
+[[nodiscard]] HRESULT Icon::LoadIconsForDpi(int dpi)
+{
+    wil::unique_hicon icon, smIcon;
+    ConExtractIconInBothSizesW(dpi, _iconPathAndIndex.first.c_str(), _iconPathAndIndex.second, &icon, &smIcon);
+    if (!icon)
     {
-        _DestroyNonDefaultIcons(); // ensure handles are destroyed/null
-        hr = E_FAIL;
+        return E_FAIL;
     }
 
-    return hr;
+    _iconHandlesPerDpi.try_emplace(dpi, std::pair<wil::unique_hicon, wil::unique_hicon>{ std::move(icon), std::move(smIcon) });
+    return S_OK;
 }
 
 // Routine Description:
@@ -392,184 +374,15 @@ Icon& Icon::Instance()
 // - hwnd - Handle to apply message workaround to.
 // Return Value:
 // - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::ApplyWindowMessageWorkaround(const HWND hwnd)
+[[nodiscard]] HRESULT Icon::ApplyIconsToWindow(const HWND hwnd)
 {
-    HICON hIcon;
-    HICON hSmIcon;
+    HICON hIcon, hSmIcon;
 
-    auto hr = GetIcons(&hIcon, &hSmIcon);
+    const auto dpi = ServiceLocator::LocateHighDpiApi<WindowDpiApi>()->GetDpiForWindow(hwnd);
+    RETURN_IF_FAILED(GetIcons(dpi, &hIcon, &hSmIcon));
 
-    if (SUCCEEDED(hr))
-    {
-        SendMessageW(hwnd, WM_SETICON, ICON_BIG, (LPARAM)hIcon);
-        SendMessageW(hwnd, WM_SETICON, ICON_SMALL, (LPARAM)hSmIcon);
-    }
+    SendMessageW(hwnd, WM_SETICON, ICON_BIG, (LPARAM)hIcon);
+    SendMessageW(hwnd, WM_SETICON, ICON_SMALL, (LPARAM)hSmIcon);
 
-    return hr;
-}
-
-// Routine Description:
-// - Initializes the icon class by loading the default icons.
-// Arguments:
-// - <none>
-// Return Value:
-// - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::_Initialize()
-{
-    auto hr = S_OK;
-
-    if (!_fInitialized)
-    {
-#pragma warning(push)
-#pragma warning(disable : 4302) // typecast warning from MAKEINTRESOURCE
-        _hDefaultIcon = LoadIconW(nullptr, MAKEINTRESOURCE(IDI_APPLICATION));
-#pragma warning(pop)
-
-        if (_hDefaultIcon == nullptr)
-        {
-            hr = HRESULT_FROM_WIN32(GetLastError());
-        }
-
-        if (SUCCEEDED(hr))
-        {
-#pragma warning(push)
-#pragma warning(disable : 4302) // typecast warning from MAKEINTRESOURCE
-            _hDefaultSmIcon = (HICON)LoadImageW(nullptr,
-                                                MAKEINTRESOURCE(IDI_APPLICATION),
-                                                IMAGE_ICON,
-                                                GetSystemMetrics(SM_CXSMICON),
-                                                GetSystemMetrics(SM_CYSMICON),
-                                                LR_SHARED);
-#pragma warning(pop)
-
-            if (_hDefaultSmIcon == nullptr)
-            {
-                hr = HRESULT_FROM_WIN32(GetLastError());
-            }
-        }
-
-        if (SUCCEEDED(hr))
-        {
-            _fInitialized = true;
-        }
-    }
-
-    return hr;
-}
-
-// Routine Description:
-// - Frees any non-default icon handles we may have loaded from a path on the file system
-// Arguments:
-// - <none>
-// Return Value:
-// - <none>
-void Icon::_DestroyNonDefaultIcons()
-{
-    _FreeIconFromReference(_hIcon);
-    _FreeIconFromReference(_hSmIcon);
-}
-
-// Routine Description:
-// - Helper method to choose one of the two given references to fill the pointer with.
-// - It will choose the specific icon if it is available and otherwise fall back to the default icon.
-// Arguments:
-// - hIconRef - reference to the specific icon handle inside this class
-// - hDefaultIconRef - reference to the default icon handle inside this class
-// - phIcon - pointer to receive the chosen icon handle
-// Return Value:
-// - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::_GetAvailableIconFromReference(_In_ HICON& hIconRef, _In_ HICON& hDefaultIconRef, _Out_ HICON* const phIcon)
-{
-    auto hr = S_OK;
-
-    // expecting hIconRef to be pointing to either the regular or small custom handles
-    FAIL_FAST_IF(!(&hIconRef == &_hIcon || &hIconRef == &_hSmIcon));
-
-    // expecting hDefaultIconRef to be pointing to either the regular or small default handles
-    FAIL_FAST_IF(!(&hDefaultIconRef == &_hDefaultIcon || &hDefaultIconRef == &_hDefaultSmIcon));
-
-    if (hIconRef != nullptr)
-    {
-        *phIcon = hIconRef;
-    }
-    else
-    {
-        hr = _GetDefaultIconFromReference(hDefaultIconRef, phIcon);
-    }
-
-    return hr;
-}
-
-// Routine Description:
-// - Helper method to initialize and retrieve a default icon.
-// Arguments:
-// - hIconRef - Either the small or large icon handle references within this class
-// - phIcon - The pointer to fill with the icon if it is available.
-// Return Value:
-// - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::_GetDefaultIconFromReference(_In_ HICON& hIconRef, _Out_ HICON* const phIcon)
-{
-    // expecting hIconRef to be pointing to either the regular or small default handles
-    FAIL_FAST_IF(!(&hIconRef == &_hDefaultIcon || &hIconRef == &_hDefaultSmIcon));
-
-    auto hr = _Initialize();
-
-    if (SUCCEEDED(hr))
-    {
-        *phIcon = hIconRef;
-    }
-
-    return hr;
-}
-
-// Routine Description:
-// - Helper method to set an icon handle into the given reference to an icon within this class.
-// - This will appropriately call to free existing custom icons.
-// Arguments:
-// - hIconRef - Either the small or large icon handle references within this class
-// - hNewIcon - The new icon handle to replace the reference with.
-// Return Value:
-// - S_OK or HRESULT failure code.
-[[nodiscard]] HRESULT Icon::_SetIconFromReference(_In_ HICON& hIconRef, const HICON hNewIcon)
-{
-    // expecting hIconRef to be pointing to either the regular or small custom handles
-    FAIL_FAST_IF(!(&hIconRef == &_hIcon || &hIconRef == &_hSmIcon));
-
-    auto hr = S_OK;
-
-    // Only set icon if something changed
-    if (hNewIcon != hIconRef)
-    {
-        // If we had an existing custom icon, free it.
-        _FreeIconFromReference(hIconRef);
-
-        // If we were given a non-null icon, store it.
-        if (hNewIcon != nullptr)
-        {
-            hIconRef = hNewIcon;
-        }
-
-        // Otherwise, we'll default back to using the default icon. Get method will handle this.
-    }
-
-    return hr;
-}
-
-// Routine Description:
-// - Helper method to free a specific icon reference to a specific icon within this class.
-// - WARNING: Do not use with the default icons. They do not need to be released.
-// Arguments:
-// - hIconRef - Either the small or large specific icon handle references within this class
-// Return Value:
-// - None
-void Icon::_FreeIconFromReference(_In_ HICON& hIconRef)
-{
-    // expecting hIconRef to be pointing to either the regular or small custom handles
-    FAIL_FAST_IF(!(&hIconRef == &_hIcon || &hIconRef == &_hSmIcon));
-
-    if (hIconRef != nullptr)
-    {
-        DestroyIcon(hIconRef);
-        hIconRef = nullptr;
-    }
+    return S_OK;
 }

src/interactivity/win32/icon.hpp

@@ -22,34 +22,23 @@ namespace Microsoft::Console::Interactivity::Win32
     public:
         static Icon& Instance();
 
-        [[nodiscard]] HRESULT GetIcons(_Out_opt_ HICON* const phIcon, _Out_opt_ HICON* const phSmIcon);
-        [[nodiscard]] HRESULT SetIcons(const HICON hIcon, const HICON hSmIcon);
-
+        [[nodiscard]] HRESULT GetIcons(int dpi, _Out_opt_ HICON* const phIcon, _Out_opt_ HICON* const phSmIcon);
         [[nodiscard]] HRESULT LoadIconsFromPath(_In_ PCWSTR pwszIconLocation, const int nIconIndex);
-
-        [[nodiscard]] HRESULT ApplyWindowMessageWorkaround(const HWND hwnd);
+        [[nodiscard]] HRESULT ApplyIconsToWindow(const HWND hwnd);
 
     protected:
         Icon();
-        ~Icon();
+        ~Icon() = default;
         Icon(const Icon&) = delete;
         void operator=(const Icon&) = delete;
 
     private:
-        [[nodiscard]] HRESULT _Initialize();
+        [[nodiscard]] HRESULT LoadIconsForDpi(int dpi);
+        // We are not using unique_hicon for these, as they are loaded from our mapped executable image.
+        HICON _hDefaultIcon{ nullptr };
+        HICON _hDefaultSmIcon{ nullptr };
 
-        void _DestroyNonDefaultIcons();
-
-        // Helper methods
-        [[nodiscard]] HRESULT _GetAvailableIconFromReference(_In_ HICON& hIconRef, _In_ HICON& hDefaultIconRef, _Out_ HICON* const phIcon);
-        [[nodiscard]] HRESULT _GetDefaultIconFromReference(_In_ HICON& hIconRef, _Out_ HICON* const phIcon);
-        [[nodiscard]] HRESULT _SetIconFromReference(_In_ HICON& hIconRef, const HICON hNewIcon);
-        void _FreeIconFromReference(_In_ HICON& hIconRef);
-
-        bool _fInitialized;
-        HICON _hDefaultIcon;
-        HICON _hDefaultSmIcon;
-        HICON _hIcon;
-        HICON _hSmIcon;
+        std::unordered_map<int, std::pair<wil::unique_hicon, wil::unique_hicon>> _iconHandlesPerDpi;
+        std::pair<std::wstring, int> _iconPathAndIndex;
     };
 }

src/interactivity/win32/menu.cpp

@@ -316,7 +316,7 @@ void Menu::s_ShowPropertiesDialog(HWND const hwnd, BOOL const Defaults)
     pStateInfo->CursorType = static_cast<unsigned int>(cursor.GetType());
 
     // Retrieve small icon for use in displaying the dialog
-    LOG_IF_FAILED(Icon::Instance().GetIcons(nullptr, &pStateInfo->hIcon));
+    LOG_IF_FAILED(Icon::Instance().GetIcons(96, nullptr, &pStateInfo->hIcon));
 
     pStateInfo->QuickEdit = !!(gci.Flags & CONSOLE_QUICK_EDIT_MODE);
     pStateInfo->AutoPosition = !!(gci.Flags & CONSOLE_AUTO_POSITION);

src/interactivity/win32/window.cpp

@@ -138,7 +138,7 @@ Window::~Window()
         wc.lpszClassName = CONSOLE_WINDOW_CLASS;
 
         // Load icons
-        status = Icon::Instance().GetIcons(&wc.hIcon, &wc.hIconSm);
+        status = Icon::Instance().GetIcons(96, &wc.hIcon, &wc.hIconSm);
 
         if (SUCCEEDED_NTSTATUS(status))
         {
@@ -335,7 +335,7 @@ void Window::_UpdateSystemMetrics() const
                 if (SUCCEEDED_NTSTATUS(status))
                 {
                     // Do WM_GETICON workaround. Must call WM_SETICON once or apps calling WM_GETICON will get null.
-                    LOG_IF_FAILED(Icon::Instance().ApplyWindowMessageWorkaround(hWnd));
+                    LOG_IF_FAILED(Icon::Instance().ApplyIconsToWindow(hWnd));
 
                     // Set up the hot key for this window.
                     if (gci.GetHotKey() != 0)

src/interactivity/win32/windowproc.cpp

@@ -7,6 +7,7 @@
 #include "clipboard.hpp"
 #include "find.h"
 #include "menu.hpp"
+#include "icon.hpp"
 #include "windowdpiapi.hpp"
 #include "windowio.hpp"
 #include "windowmetrics.hpp"
@@ -267,6 +268,8 @@ static constexpr TsfDataProvider s_tsfDataProvider;
                      RECT_HEIGHT(prcNewScale),
                      SWP_NOZORDER | SWP_NOACTIVATE);
 
+        LOG_IF_FAILED(Icon::Instance().ApplyIconsToWindow(hWnd));
+
         _fInDPIChange = false;
 
         break;