Skip to content

Build/Windows: Update the Windows icon as svg and build derivatives from it #14964

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

Closed
wants to merge 1 commit into from

Conversation

Ayesh
Copy link
Member

@Ayesh Ayesh commented Jul 15, 2024

This is a follow-up to #12016. The author unfortunately deleted their php-src fork, which closed the PR. As mentioned in the previous PR, we have not updated the Windows icon in 21 years, and it look really dated:


image


This takes the work from that PR, and the feedback it received to optimize the SVG.

In summary, this PR:

  1. Takes the source SVG from php.net and adds and centers it into a 512px viewbox SVG with #777BB3 background. The source "php" text fill is then changed to #fff.
  2. Adds a mkico.sh and an mkico.bat file to generate the .ico files from the source svg file. The icon file embeds pixel sizes from 16,24,32,48,64,128, and 256 squares. That's why the new ico file is larger than the existing one. In Update icon after 20 years #12016, it uses magick command, but this one uses the convert command which should be available in ImageMagick 6 in latest Ubuntu LTS version.
  3. The updated ico file is white text on the PHP purple background, and not the old black text with white outline in the PHP purple background.

I did not copy the SVG files from php/web-php because it has rounded edges, and may only work out best for Windows app icons.

@cmb69
Copy link
Member

cmb69 commented Jul 15, 2024

Thank you so much!

@cmb69 cmb69 self-requested a review July 15, 2024 19:09
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is long overdue, and the PR looks good to me. Thank you @Ayesh and @imshvc.

…rom it

Co-Authored-By: Nurudin Imsirovic <realnurudinimsirovic@gmail.com>
@Ayesh Ayesh force-pushed the update-win-ico-from-svg branch from b0872eb to b56efb7 Compare July 16, 2024 10:28
@Ayesh
Copy link
Member Author

Ayesh commented Jul 16, 2024

Thank you for reviewing this.

Winget seems to have officially sourced imagemagick on version 7, and Ubuntu 24.04 with version 6. I adjusted the scripts to reflect it; Imagemagick 6 apparently deprecates using the convert command in favor of magick. Because we already have the script, I think we can keep it.

@cmb69
Copy link
Member

cmb69 commented Jul 16, 2024

Imagemagick 6 apparently deprecates using the convert command in favor of magick

I wasn't sure about that regarding other platform than Windows. On Windows, it was long time overdue, because of the system command convert, so this could confusion, and maybe worse.

Because we already have the script, I think we can keep it.

Agreed.

I'm going to merge this PR soon, so we can have the nice icons already for PHP 8.4.0alpha2.

@Ayesh
Copy link
Member Author

Ayesh commented Jul 16, 2024

I tested on Fedora 40 that comes with ImageMagic v7. convert still works but deprecated, and magick works. Shall we replace convert with magick?

@cmb69
Copy link
Member

cmb69 commented Jul 16, 2024

I have no idea about the usage of ImageMagick v6 vs. v7. Maybe stick with convert for now, but add a comment that magick convert might be needed for newer ImageMagick?

@Ayesh
Copy link
Member Author

Ayesh commented Jul 16, 2024

I think you are right, it makes sense to stick to convert for now. On Windows, IMv7 seems to be the default version.

So for the state this PR is in now, it should work on the current IM version (v7) on Windows, as well as current IM version (v6) on latest Debian/Ubuntu/etc plus on Fedora too, albeit with the deprecation notice. This probably won't matter as much after all, we might need to update the scripts anyway by the time we come across to change the icons :)

@cmb69 cmb69 closed this in e7c16d2 Jul 16, 2024
@Ayesh
Copy link
Member Author

Ayesh commented Jul 16, 2024

image

image

image

Yay, thanks for merging. Here are the comparison against the latest master branch and before it.

@Ayesh Ayesh deleted the update-win-ico-from-svg branch July 16, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants