From 7766c719cd3dde437d95fd521939279463c8d1e9 Mon Sep 17 00:00:00 2001 From: Venkata Sharath Chandra Manchala Date: Wed, 30 Mar 2022 17:03:15 -0700 Subject: [PATCH] Address comments --- .../AppNotificationUtility.cpp | 63 +++---------------- dev/AppNotifications/WICUtility.cpp | 12 ++-- .../ToastNotificationsDemoApp.vcxproj | 7 --- .../ToastNotificationsDemoApp.vcxproj.filters | 13 ---- 4 files changed, 14 insertions(+), 81 deletions(-) diff --git a/dev/AppNotifications/AppNotificationUtility.cpp b/dev/AppNotifications/AppNotificationUtility.cpp index 4bac93d0104..28e4b216410 100644 --- a/dev/AppNotifications/AppNotificationUtility.cpp +++ b/dev/AppNotifications/AppNotificationUtility.cpp @@ -240,65 +240,19 @@ std::wstring Microsoft::Windows::AppNotifications::Helpers::GetDisplayNameBasedO return displayName; } -#define GIL_EXACTSIZEONLY 0x0100 -// Looks at the passed in icon and tries loading a better sized icon from the resource file. -// If there is a larger icon available, use that icon instead. This is because the HICON passed -// in to the legacy tray item is either a SM_CXSMICON x SM_CYSMICON (16x16) icon, or if NIIF_LARGE_ICON -// is specified in the dwInfoFlags then it should be a SM_CXICON x SM_CYICON (32x32) icon. For toast messages, -// an image of 150x150 is expected. To allow a larger icon to be used we will check the icon sizes which are available. -// When we take DPI into account, we always want to pick the next larger icon size and scale down. For example, -// if we are running 200%, we are trying to load an image of 300x300 px. The next largest snap icon size is 512px, -// but icon file format requires this to be available. So the largest possible size in an icon is 256px, which means -// we should just load that image file size and ignore DPI. -inline HICON LoadBetterIconFromResource(_In_ HICON hIcon, int cx, int cy, UINT uFlags /*= 0*/) -{ - // TODO: Use wil handle - HICON hIconNew = nullptr; - - // TODO: Wil wrapper for this? - ICONINFOEX iconInfoEx = {}; - iconInfoEx.cbSize = sizeof(iconInfoEx); - if (GetIconInfoEx(hIcon, &iconInfoEx)) - { - if (S_OK != SHDefExtractIcon(iconInfoEx.szModName, -iconInfoEx.wResID, uFlags, &hIconNew, nullptr, MAKELONG(cx, cy))) - { - DestroyIcon(hIconNew); - hIconNew = nullptr; - } - - THROW_IF_WIN32_BOOL_FALSE(DeleteObject(iconInfoEx.hbmColor)); - THROW_IF_WIN32_BOOL_FALSE(DeleteObject(iconInfoEx.hbmMask)); - } - return hIconNew; -} - -inline HICON RetrieveIconFromProcess() +inline wil::unique_hicon RetrieveIconFromProcess() { std::wstring processPath{}; THROW_IF_FAILED(wil::GetModuleFileNameExW(GetCurrentProcess(), nullptr, processPath)); - HICON hIcons[1]; - // TODO: Add a check for HICON here. - ExtractIconExW(processPath.c_str(), 0, hIcons, nullptr, 1); - - int iconSizes{ 0 }; - HICON hIconNew{ nullptr }; - int const rgSnapSize[] = { 256, 196, 128, 64, 48, 32, 24, 16 }; - do - { - // Load a better image, looking for an exact match above 256x256, or something that is close once we get below 48px (unscaled). - hIconNew = LoadBetterIconFromResource(hIcons[0], rgSnapSize[iconSizes], rgSnapSize[iconSizes], (rgSnapSize[iconSizes] > 48) ? GIL_EXACTSIZEONLY : 0); - iconSizes++; - } while ((hIconNew == nullptr) && (iconSizes < ARRAYSIZE(rgSnapSize))); - - if (hIconNew != nullptr) + // Extract the small icon from the first .ico, if failed extract the large icon. + wil::unique_hicon hIcon{}; + if (!ExtractIconExW(processPath.c_str(), 0 /* index */, nullptr, &hIcon, 1)) { - // TODO: Clear the NIIF_USER flag since we need to now destroy the newly loaded icon. - //WI_ClearFlag(dwInfoFlags, NIIF_USER); - hIcons[0] = hIconNew; + THROW_HR_IF(E_FAIL, ExtractIconExW(processPath.c_str(), 0, &hIcon, nullptr, 1) == 0); } - return hIcons[0]; + return hIcon; } inline std::wstring RetrieveLocalFolderPath() @@ -323,7 +277,6 @@ inline std::wstring RetrieveLocalFolderPath() HRESULT Microsoft::Windows::AppNotifications::Helpers::RetrieveAssetsFromProcess(_Out_ Microsoft::Windows::AppNotifications::Helpers::AppNotificationAssets& assets) noexcept try { wil::unique_hicon hIcon{ RetrieveIconFromProcess() }; - THROW_HR_IF(E_UNEXPECTED, hIcon == nullptr); std::wstring notificationAppId{ RetrieveNotificationAppId() }; @@ -393,8 +346,8 @@ void Microsoft::Windows::AppNotifications::Helpers::RegisterAssets(std::wstring // 3. Use the default assets. Microsoft::Windows::AppNotifications::Helpers::AppNotificationAssets assets{}; - if (FAILED(RetrieveAssetsFromProcess(assets)) && - FAILED(ToastNotifications_RetrieveAssets_Stub(assets))) + if (FAILED(ToastNotifications_RetrieveAssets_Stub(assets)) && + FAILED(RetrieveAssetsFromProcess(assets))) { THROW_IF_FAILED(RetrieveDefaultAssets(assets)); } diff --git a/dev/AppNotifications/WICUtility.cpp b/dev/AppNotifications/WICUtility.cpp index e21334c9db3..267fed6754d 100644 --- a/dev/AppNotifications/WICUtility.cpp +++ b/dev/AppNotifications/WICUtility.cpp @@ -94,7 +94,7 @@ winrt::com_ptr GetStreamOfWICBitmapSource( _In_ BITMAP_VERSION bmpv) { winrt::com_ptr spImageStream; - THROW_IF_FAILED(CreateStreamOnHGlobal(nullptr, true, spImageStream.put())); + THROW_IF_FAILED(CreateStreamOnHGlobal(nullptr /* handle */, true /* delete on release */, spImageStream.put())); // Create encoder and initialize it winrt::com_ptr wicEncoder; @@ -109,7 +109,7 @@ winrt::com_ptr GetStreamOfWICBitmapSource( // Seek the stream to the beginning and transfer static const LARGE_INTEGER lnBeginning = {}; - THROW_IF_FAILED(spImageStream->Seek(lnBeginning, STREAM_SEEK_SET, nullptr)); + THROW_IF_FAILED(spImageStream->Seek(lnBeginning, STREAM_SEEK_SET, nullptr /* new seek pointer */)); return spImageStream; } @@ -125,10 +125,10 @@ void SaveImageWithWIC( // Seek the stream to the beginning and transfer static LARGE_INTEGER const lnBeginning{ 0 }; - THROW_IF_FAILED(spImageStream->Seek(lnBeginning, STREAM_SEEK_SET, nullptr)); + THROW_IF_FAILED(spImageStream->Seek(lnBeginning, STREAM_SEEK_SET, nullptr /* new seek pointer */)); static ULARGE_INTEGER lnbuffer{ INT_MAX }; - THROW_IF_FAILED(spImageStream->CopyTo(pStream.get(), lnbuffer, nullptr, nullptr)); + THROW_IF_FAILED(spImageStream->CopyTo(pStream.get(), lnbuffer, nullptr /* pointer to number of bytes read */, nullptr /* pointer to number of bytes written */)); } void Microsoft::Windows::AppNotifications::WICHelpers::WriteHIconToPngFile(wil::unique_hicon const& hIcon, _In_ PCWSTR pszFileName) @@ -140,7 +140,7 @@ void Microsoft::Windows::AppNotifications::WICHelpers::WriteHIconToPngFile(wil:: // Create stream to save the HICON to. winrt::com_ptr spStream; - THROW_IF_FAILED(CreateStreamOnHGlobal(nullptr, TRUE, spStream.put())); + THROW_IF_FAILED(CreateStreamOnHGlobal(nullptr /* handle */, TRUE, spStream.put())); winrt::com_ptr wicBitmapSource; wicBitmapSource = wicBitmap; @@ -159,7 +159,7 @@ void Microsoft::Windows::AppNotifications::WICHelpers::WriteHIconToPngFile(wil:: THROW_IF_FAILED(spStreamOut->SetSize(statstg.cbSize)); // TODO: Comments need to be added - THROW_IF_FAILED(spStream->CopyTo(spStreamOut.get(), statstg.cbSize, nullptr, nullptr)); + THROW_IF_FAILED(spStream->CopyTo(spStreamOut.get(), statstg.cbSize, nullptr /* pointer to number of bytes read */, nullptr /* pointer to number of bytes written */)); THROW_IF_FAILED(spStreamOut->Commit(STGC_DANGEROUSLYCOMMITMERELYTODISKCACHE)); } diff --git a/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj b/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj index a503352b2b6..b28e4efa32b 100644 --- a/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj +++ b/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj @@ -241,7 +241,6 @@ - @@ -268,12 +267,6 @@ {f76b776e-86f5-48c5-8fc7-d2795ecc9746} - - - - - - diff --git a/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj.filters b/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj.filters index f8de0a3b86a..628b51e4d68 100644 --- a/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj.filters +++ b/test/TestApps/ToastNotificationsDemoApp/ToastNotificationsDemoApp.vcxproj.filters @@ -26,21 +26,8 @@ Header Files - - Header Files - - - - Resource Files - - - - - Resource Files - - \ No newline at end of file