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

Update Windows icon and NSIS banner #32054

Merged
merged 1 commit into from
May 21, 2019
Merged

Update Windows icon and NSIS banner #32054

merged 1 commit into from
May 21, 2019

Conversation

musm
Copy link
Contributor

@musm musm commented May 16, 2019

part of #32038

It looks like the old icon had a smaller size, updating it now looks much better.

@musm
Copy link
Contributor Author

musm commented May 16, 2019

I'm not sure why but the julia exe files are still using the old icons even after updating them. It might have something to do with the 7zS.sfx ? the julia.rc file specifies the icon as julia.ico which is the same folder as the julia.rc file so I don't know why it is not picking up the refreshed icons.

@StefanKarpinski
Copy link
Member

Thanks for polishing this!

@musm
Copy link
Contributor Author

musm commented May 16, 2019

Fixed up the actual 7z stage of the installer icon, it needed manual modification using resource hacker.

@vtjnash do you by any chance know why the actual julia.exe is not picking up the julia.ico changes? My understanding was that julia.rc is involved. https://github.com/JuliaLang/julia/blob/master/contrib/windows/julia.rc#L34

@musm
Copy link
Contributor Author

musm commented May 17, 2019

So what I think was happening was that the resource file defined the same number for VERSIONINFO and ICON

1 VERSIONINFO
101 ICON "julia.ico"

changing the resource number for ICON to 101 (the number is arbitrary and doesn't matter unless it is referenced), seems to fix the clash and now builds properly update the icons to the new ones.

I'm not sure if it was some strange luck that the current version works. It could be related to the sizes embedded in the ico file on master are smaller than the updated ico file in this PR (which have embedded more icon sizes)

Compare the new icon (red arrow) to the old icon on the right in the figure below
image

image

@vtjnash
Copy link
Member

vtjnash commented May 17, 2019

I assume it's just because you modified the file so it rebuilt. There's no conflict, so it'll just pick up the lowest number in each group (1, 24, and 101; or it could have been 1, 1, and 1)

@musm
Copy link
Contributor Author

musm commented May 17, 2019

I don't think so? I deleted my icon cache and I made sure to distclean and built from scratch, but it still persisted, before renumbering.

@musm
Copy link
Contributor Author

musm commented May 17, 2019

Seems like you were right.... It's pretty annoying to mess around with icons since things don't always refresh properly...

@musm
Copy link
Contributor Author

musm commented May 17, 2019

I fixed up a very subtle scaling issue (@cormullion the julia-dots.svg isn't perfectly square and there is some space even around the widest dots, I trimmed that space and made it a perfect square, which fixed the scaling in the creation of the ico files)

@musm
Copy link
Contributor Author

musm commented May 18, 2019

FYI this is good to go on my end

@musm
Copy link
Contributor Author

musm commented May 20, 2019

merge?

@KristofferC
Copy link
Member

Is the Title change and the ShowUninstDetails change part of the icon change?

@ViralBShah
Copy link
Member

I believe they are part of the NSIS banner change which we discussed elsewhere. Let's go ahead with this.

@ViralBShah ViralBShah merged commit a43c46b into JuliaLang:master May 21, 2019
@ViralBShah ViralBShah added system:32-bit Affects only 32-bit systems building Build system, or building Julia or its dependencies system:windows Affects only Windows and removed system:32-bit Affects only 32-bit systems labels May 21, 2019
@musm
Copy link
Contributor Author

musm commented May 22, 2019

Yeah it's not entirely related to the branding update. The showuninst improves consistency with the installer and the name change actually has no impact due to the 7zSFX module used is outdated, but I changed it anyways if it gets updated.

@musm musm deleted the ico branch May 22, 2019 16:23
@musm musm mentioned this pull request Jul 16, 2019
14 tasks
musm added a commit to musm/julia that referenced this pull request Aug 7, 2019
musm added a commit to musm/julia that referenced this pull request Aug 7, 2019
musm added a commit to musm/julia that referenced this pull request Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants