scan directories recursively for UWP app icon#107
scan directories recursively for UWP app icon#107its-bubble-gum wants to merge 1 commit intohdlx:masterfrom
Conversation
There was a problem hiding this comment.
Thanks for the PR :)
It breaks load of icon for some apps on my side. Ex: spotify or windows "settings".
Not fully clang formated, you can run python.exe AAS.py Format
The "depth" count solution is not bad but maybe we are missing something. Like "assets" directory is some kind of standard and must always be searched, as well as the intermediate language directory.
Maybe AppxManifestResourcesEnumerator can help. Not sure, I have probably tried this before.
| return found; | ||
| } | ||
|
|
||
| static void FindLogoInDir(wchar_t parentDir[MAX_PATH], wchar_t* logoNoExt, wchar_t* outIconPath, bool lightTheme, uint32_t* maxSize) { |
There was a problem hiding this comment.
Why not ptr declaration for parentDir? Sized array is misleading as there is not more size information inside the function.
logoNoExt can be const.
| } | ||
|
|
||
| static void ScanDirForAppLogoRecursive(wchar_t dir[MAX_PATH], wchar_t* logoPath, uint32_t logoPathDepth, wchar_t* logoName, bool lightTheme, uint32_t* maxSize, wchar_t* outIconPath) { | ||
| if (logoPathDepth == 0) { |
There was a problem hiding this comment.
Not sure I understand the "depth" logic. Why is it needed ? Can't we just treat each recursion the same way, and have only one while loop, either recursing of processing the file (checking size etc) depending on the type (image vs directory). That seems easier to understand to me :)
There was a problem hiding this comment.
the depth follows the idea of fully matching the path for the icon.
Take iCloud, for instance: logo prop == "iCloud/Square44x44.png", actual path: "Assets/iCloud/en_US/Square44x44.png"
depth allows us to split the path into directories which are not guaranteed to directly follow one another, but rather follow the logical path.
This way we ignore icons in "Assets/iCloudPhotos/*", because the path doesn't match
|
|
||
| wchar_t* nextLogoPath = logoPath; | ||
| uint32_t nextLogoPathDepth = logoPathDepth; | ||
| if (logoPathDepth && !wcscmp(findData.cFileName, logoPath)) { |
There was a problem hiding this comment.
Ok I think I get the depth logic. It "counts" down to zero, each time we met an element of the path. This means we need every subdirectory of the path but we don't mind extra intermediate directory.
We could still have only one while (FindNextFileW(h) loop.
There was a problem hiding this comment.
yeahh.. wanted to split the logic in 2 separate sections. could be merged, why not
| wcscat(outIconPath, findData.cFileName); | ||
| } | ||
| } | ||
| FindLogoFromProp(packagePath, logoProp, logoNoExt, lightTheme, outIconPath); |
There was a problem hiding this comment.
logoProp is not logo property (the data from the manifest) anymore, since a terminating char has been added.
| } | ||
|
|
||
| static void FindLogoFromProp(wchar_t packagePath[MAX_PATH], wchar_t logoPath[MAX_PATH], wchar_t* logoName, bool lightTheme, wchar_t* outIconPath) { | ||
| uint32_t logoPathDepth = 1; |
There was a problem hiding this comment.
Why starting at one? Can we guarantee an icon property can't be just a file without any parent path?
There was a problem hiding this comment.
not really guarantee, but this would be highly unconventional and rare. makes sense to start at 0, i guess
|
@hdlx thanks for the response! From what I read, Assets directory is conventional but not mandatory. And the logo paths define the logical, not actually path, so, PS. i know it breaks some apps, wanted to just introduce the idea and get some thoughts about it refs: https://learn.microsoft.com/en-us/windows/win32/menurc/pri-indexing-reference |
|
Oh ok, then I like the idea, this seems rather resilient to path variation. Maybe simpler than trying to guess standards. When I first wrote this I remember trying a bunch of stuff, hoping some higher level api would give me the full path from the property, with no success. |
|
lol, go this somehow. zen browser has arch linux icon. which i don't mind personally, but... |
2 participants
Played around with it, made this.
It loads correct icon for iCloud suite now ((: