Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork851
Allow Windows to search and load Icons and Cursors from VFS.#1781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
RegDogg commentedNov 26, 2025
Finished, awaiting final review. |
rdb left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This sounds like a useful feature to have, thanks for working on it! Comments inline.
| // The filename doesn't exist along the search path. | ||
| if (resolved.is_fully_qualified() && resolved.exists()) { | ||
| // But it does exist locally, so accept it. | ||
| if ((resolved.resolve_filename(get_model_path())) || (resolved.is_fully_qualified() && resolved.exists())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It may still be in the VFS ifexists() returns true if there is an overlapping mount.
Here's how to get the system filename from a VFS filename:
PT(VirtualFile)vfile= ...;//checkvfileSubfileInfoinfo;if (vfile->get_system_info(info)&&info.get_start()==0) {Filenamefn=info.get_filename();//dosomethingwithfn.to_os_specific()
| return (HICON)((*fi).second); | ||
| } | ||
| Filename os = resolved.to_os_specific(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
to_os_specific() returns anstd::string, not a Filename (as Filename is a container for OS-independent paths).
| VirtualFileSystem *vfs =VirtualFileSystem::get_global_ptr(); | ||
| PT(VirtualFile) vfile = vfs->get_file(filename); | ||
| if (vfile !=nullptr) { | ||
| std::string vfile_data = vfile->read_file(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
More idiomatic to use the vector_uchar version:
vector_uchar vfile_data;if (!vfile->read_file(vfile_data,false)) {// error handle}
| int offset =LookupIconIdFromDirectoryEx((PBYTE)vfile_data.data(),TRUE,0,0, LR_DEFAULTCOLOR); | ||
| if (offset !=0) { | ||
| HICON hIcon =CreateIconFromResourceEx((PBYTE)vfile_data.data() + offset, fsize - offset,TRUE,0x30000,0,0, LR_DEFAULTCOLOR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm wondering if there is a lifetime issue here. I haven't read the documentation, but does the icon created by CreateIconFromResourceEx copy the data passed in, or does it continue to refer to the same data in memory? If so, that might be an issue since thestd::string will be deallocated at the end of the function.
Secondly, you're passing in the remaining size of the whole data structure, not the size of the icon in it; is that okay?
| std::string vfile_data = vfile->read_file(0); | ||
| long fsize = vfile_data.length(); | ||
| int offset =LookupIconIdFromDirectoryEx((PBYTE)vfile_data.data(),TRUE,0,0, LR_DEFAULTCOLOR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm really confused by this; the "icon id" that this function returns is actually a byte offset in some cases? Is that an undocumented feature of this call?
| _cursor_filenames[resolved] = h; | ||
| return (HCURSOR)h; | ||
| HANDLE h =LoadImage(nullptr, os.c_str(), | ||
| IMAGE_CURSOR,0,0, LR_LOADFROMFILE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a lot of code to duplicate; is it conceivable to have the shared logic put in a private method that takes parameters as necessary to distinguish cursor from icon? It would do the actual loading but not the map storage/lookup.
Uh oh!
There was an error while loading.Please reload this page.
Issue description
This change will allow Windows to search Multifiles mounted with VFS for icons and cursors
Solution description
By utilizing functions
LookupIconIdFromDirectoryExandCreateIconFromResourceExwe can read filedata in the VFS fromicon-filename/cursor-filenamefilepathsChecklist
I have done my best to ensure that…