BenjaminRi / winresource

Create and set windows icons and metadata for executables with a rust build script
MIT License
50 stars 10 forks source link

Change the default icon ID to `32512` to be in line with the Windows default. #12

Closed zedseven closed 1 year ago

zedseven commented 1 year ago

I originally submitted a PR for this to the winres repo (https://github.com/mxre/winres/pull/39), but it was ignored. I'm hoping here I can finally get the change made.

Relevant documentation:

Closes #5.

BenjaminRi commented 1 year ago

I looked into it and even Microsoft binaries themselves are all over the place regarding icon IDs. However, I do commend your persistence and it seems like it is indeed the preferred icon ID. If it does not cause any breakage - which it doesn't seem to - I will merge it shortly and release a new version.

zedseven commented 1 year ago

Haha, thank you.

As with everything else related to Windows, it's a mess, but I think it makes sense to match what Visual Studio uses and what the MSDN claims is the default application icon ID.

BenjaminRi commented 1 year ago

It is done, I just released a new version: winresource = "0.1.16"

Korne127 commented 6 months ago

Just stumbled across this. To me, the Microsoft link you referred to looks like 32512 is supposed to load the general Windows default application icon:

The operating system provides a set of standard icons that are available for any application to use at any time. The software development kit (SDK) header files contain identifiers for the system icons — the identifiers begin with the IDI_ prefix.

To me, the documentation reads like you should use that number in case your application wants to load this default icon, such as it might want to load the other icons like the question mark, and not to use it for your own custom application icon.

However, if Visual Studio itself uses this icon number, the documentation might just be bad / misleading (as the same-looking icon also has the id 32517). Probably this should be mentioned in the documentation (that it's the number used by VSCode itself) as the documentation doesn't look like it supports this change to me.

@zedseven Have you tried adding several icons in a Visual Studio project and seeing if the main icon still has the ID? I might try setting one icon to 32512 and one to 1 and see which Windows then displays as application icon.

Korne127 commented 6 months ago

Update: I just found a Microsoft documentation from 1995 about which icon ID to use.

It states:

If more than one such group resource exists in the file, Windows must decide which one to use. Windows NT simply chooses the first resource listed in the application's RC script. On the other hand, Windows 95's algorithm is to choose the alphabetically first named group icon if one exists. If one such group resource does not exist, Windows chooses the icon with the numerically lowest identifier. So, to be sure that a particular icon is used for an application, the developer should insure that both of the following criteria are met: The icon is placed before all other icons in the RC file. If the icon is named, its name is alphabetically before any other named icon, otherwise its resource identifier is numerically smaller than any other icon.

I don't know if this is still true for modern operating systems, but I think this should be tested (and if icon 1 takes precedence over 32512, changed back).

(Also I found this in this in the documentation to set_icon_with_id of this repository, which still recommends on the base of this link to use the ID 1 for the application icon. It looks like that documentation hasn't been updated after this PR. That might be the case with some other changes too (e.g. the documentation still says to use cfg!(target_os = "windows"), which doesn't work). I can look into this at some point and maybe do a PR updating it.)

Korne127 commented 6 months ago

Update: I tested it myself and compiled two versions with Icons with both ID 1 and 32512. One version had the icon 1 in front of the icon 32512, one had the icon 32512 in front of the icon 1. But both had the icon with the id 1 as the application icon :/ image

So yeah, this change isn't correct, and it might break some projects that use another icon with a lower id :/ @zedseven Can you maybe create a test project in Visual Studio with several icons and look at how they're named? Then we could change the behaviour of winresource to match that.

BenjaminRi commented 6 months ago

Thank you for your thorough analysis. I feel like Microsoft really dropped the ball here, they should be clearer about it. Various people spent a considerable amount of time trying to figure it out and it's just confusing. Maybe in a few years, we'll see a Raymond Chen post about the mess that is resource IDs, and an in-depth exploration of the ancient and convoluted history of Windows resources, haha.