Windows icon does not scale correctly

[charlielee]

When my app is exported using the win-ico option the resulting executable has an icon that does not scale. This means that it appears pixelated when set as a desktop icon, pinned to the Windows 10 start screen or simply zoomed into in the File Explorer.

Using Resource Hacker I can see that an icon group with different sizes has been generated; it just does not seem to be using them correctly.

[evshiron]

Thanks for the report. I know there are some limitations with the integrated rcedit.exe, but I only have a Windows 7 VM and indicated from the "nwb-test" (the testing application built in the testing procedure) it looks acceptable (high resolution in File Explorer and Taskbar). Would you mind helping me run the tests on your Windows 10 machine and see if the "nwb-test" application (located in the temp/ directory) looks good? Also recommended command line tools to solve this issue are appreciated too. See CONTRIBUTING for a brief procedure to run the tests.

[charlielee]

I've just run npm test and it's failing with: Uncaught Error: ERROR_DONE_FILE_EXISTS (I assume this is related to #31).

The test application located at temp\build\nwb-test-win-ia32 unfortunately has the low res icon. If file explorer is set to the large icon view this becomes very clear:

I've noticed the same icon issue has actually been reported as atom/node-rcedit/issues/4.

Some alternative tools I've found:

• https://github.com/felicienfrancois/node-resourcehacker
• https://github.com/felicienfrancois/node-winresourcer

[evshiron]

@BoatsAreRockable Cool and thanks a lot! In #31 I forget the separator difference of Windows, and the alternatives look promising. node-winresourcer should be used in nw-builder and I think it doesn't support editing stuff other than icons, so I am going to try node-resourcehacker. BTW, did you manage to use Resource Hacker to get around this?

[charlielee]

I've just tested node-resourcehacker in my build process and it does indeed fix this :)

[evshiron]

Excellent. I have read https://github.com/nwjs/nw-builder/issues/44 and there are wrapper modules like winresourcer and resourcehacker. Because winresourcer requires .NET Framework 2.0, which will be much complicated on Linux/Mac OS X, I decide to use resourcehacker. But the resourcehacker wrapper seems to split the arguments by space, which I think will lead to problems when there are spaces in the path. I am thinking about wrapping it myself, but it will take some time.

[evshiron]

This issue should be fixed in nwjs-builder 1.12.2, as well as #31, but I have only tested it on Linux (Travis CI) and Mac OS X. Feel free to try it out :)

[evshiron]

It's a pity I can't include a copy of Resource Hacker because of it's license. I have to download it in the post-install step and it's extremely slow in my case. Tomorrow I will replace it with a "download when called" strategy.

[charlielee]

This issue has not been resolved in 1.12.2 - now the icon is not changed at all from the default.

I've looked at src/lib/build/win32 and it appears you are using node-resourcehacker but with a method from node-winresourcer!

[evshiron]

Hmm, not an expected behavior. Because I thought resourcehacker might have some problems with spaces, I made a wrapper and published it as node-resourcehacker (but I suspect it use commas as separator), but there should not be a problem with the function call. I will test again on a Windows 7 machine myself.

[evshiron]

BTW there is a ResourceHacker.log in node_modules/node-resourcehacker/ and when I run the tests on Windows it says 'Error: "assets/nwb-test.ico" does not exist', which is quite strange. EDIT: If the path starts with "./", building on Mac OS X will fail. However, on Windows, it seems like a relative path for .ico, starting with ".\", is a must. But it doesn't make it work, either. EDIT: Da*n, you have to use Unix separator with Resource Hacker, even on Windows.

[evshiron]

@BoatsAreRockable As node-resourcehacker 1.0.1 published, the tests should generate correct icons on Windows. Please help test it and see if everything works as expected, thanks.

[charlielee]

@evshiron the tests fail with: Uncaught TypeError: Cannot read property 'normalize' of undefined

However, I think I've found the issue and it is quite simple to fix. Please see my comment on your node-resourcehacker commit.

[evshiron]

@BoatsAreRockable Ouch... Haven't been writing ECMAScript 5 for a long time and forget the naming stuff. Should be fixed in node-resourcehacker 1.0.3 now.

[charlielee]

1.0.3 fixes this :+1:

Thank you for fixing this so quickly!