Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jul 22, 2023

Conversation

zedseven
Copy link
Contributor

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

Relevant documentation:

Closes #5.

zedseven added 3 commits July 19, 2023 21:07
This commit is identical to the one originally suggested for the `winres` project.
…ved the pertinent section from the old link.
@BenjaminRi
Copy link
Owner

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
Copy link
Contributor Author

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 BenjaminRi merged commit 6eeadea into BenjaminRi:master Jul 22, 2023
@BenjaminRi
Copy link
Owner

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

@Korne127
Copy link

Korne127 commented May 9, 2024

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
Copy link

Korne127 commented May 9, 2024

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
Copy link

Korne127 commented May 9, 2024

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
Copy link
Owner

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.

@oldnewthing
Copy link

The ID should be 1. The value 32512 is not "the value you should use for your application icon." It is "the value you should pass to LoadIcon to ask the system to give you a copy of the default application."

@BenjaminRi
Copy link
Owner

@oldnewthing Thank you for the reply!

Raymond Chen wrote the following additional details in an email, which I find worth adding to this discussion thread:

The reason you see varying numbers is that Windows uses the lowest-numbered icon. The easiest way to get the lowest-numbered icon is to use the number 1, but you could use any number, as long as you promise not to use any smaller numbers. So you can use 2 if you promise not to use 1. You can use 32512 if you promise not to use 1 through 32511. But the cleanest solution is to just use 1. That way you don’t have to worry about somebody later attaching some other icon that sneaks below yours.

[If you have named icons, then the alphabetically first named icon is used.]

That settles it, then. We will set the ID to 1 by default.

@Korne127
Copy link

I mean that's literally what I wrote (researched and tested) half a year ago…
But I'm glad that this got finally fixed :)

@BenjaminRi
Copy link
Owner

I mean that's literally what I wrote (researched and tested) half a year ago…

Because both sides raised convincing arguments, I was not going to change it again until an authoritative or trusted source appeared. But yes, you were indeed correct. By the way, the crate is released, 0.1.19 fixes the issue.

@Korne127
Copy link

Ah, I'm sorry. I missed that they were from Microsoft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default icon id should be 32512
4 participants