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

Compressed godot.ico brakes Godot Icon tool #73497

Open
pkowal1982 opened this issue Feb 17, 2023 · 12 comments · May be fixed by #75950
Open

Compressed godot.ico brakes Godot Icon tool #73497

pkowal1982 opened this issue Feb 17, 2023 · 12 comments · May be fixed by #75950

Comments

@pkowal1982
Copy link
Contributor

Godot version

4.0 rc 2

System information

All operating systems

Issue description

@bruvzg This commit brakes compability with Godot Icon tool.
42c2c02

As agreed before in this commit:
d469cfb

having a little bigger uncompressed Windows icon is ok when it enables users to replace it without rcedit usage.
Godot Icon tool is especially helpful on Linux systems.

Also Godot Icon is mentioned in official docs (at least for 3.x) so it would be nice to leave the uncompressed icon.

Steps to reproduce

No steps needed to reproduce.

Minimal reproduction project

As archive is required I'm attaching empty one.
empty.zip

@bruvzg
Copy link
Member

bruvzg commented Feb 17, 2023

Uncompressed icon was causing issues - see #64073, also we have two executable, and two icons (and in case of the console wrapper, uncompressed icon is lately 3 times larger that executable itself).

Also, note that now Godot supports generating windows icon from the project icon, and will try to fix icon order on the export, so any executable that will rcedit will have compressed icon regardless of the source icon or default icon compression.

@pkowal1982
Copy link
Contributor Author

I would assume that the only icon changed by user will be godot.ico not the console wrapper one so this excess file size would be acceptable.

As I understand #64073 was caused by rcedit bug. It's strange to me that we try to fit our software to this buggy behaviour.
Remember that some people started to use Godot Icon and they liked it, especially for cross system compatibility and small download size.

If the problem lies in rcedit wrongly sorting images we should state it to the people so they know why their icons are blurred and how to prevent it. If rcedit produces wrong output they can still use icon created with Godot Icon tool? Probably it would mitigate the problem.
Does changing icon with rcedit stopped braking executables with embedded pck?

I would restore uncompressed godot.ico and educate people how to prevent icon blurring on their exports.

@bruvzg
Copy link
Member

bruvzg commented Feb 17, 2023

I would assume that the only icon changed by user will be godot.ico not the console wrapper one so this excess file size would be acceptable.

Both icons can be changed by user.

Does changing icon with rcedit stopped braking executables with embedded pck?

Yes, current export process can be used with embedded PCK, rcedit modification and signed executable. See #56093.

and educate people how to prevent icon blurring on their exports.

Most users won't read docs, and rcedit is basically a standard tool everyone is using, so bug or not we definitely should support it.

As long as sorting orders used by rcedit, Godot export plugin (see #68828, it's trying to reorder sizes in the user provided icon), default icon are matching (and not causing any issues).

Personally, I won't oppose uncompressed icon as such, but I'm not sure if support for the external tool that is less versatile than rcedit is convincing reason to have it.

@pkowal1982
Copy link
Contributor Author

If we invert the order of images in uncompressed godot.ico it will fix the issues users had?
Cause this way we can also support Linux users and everyone will be content.

@akien-mga
Copy link
Member

Linux can use rcedit just fine with Wine.

@pkowal1982
Copy link
Contributor Author

Ok, looks like Wine should work, just this 1 161 MB of used space seems a little overshot to change the icon. :)

@bruvzg
Copy link
Member

bruvzg commented Feb 17, 2023

Ideally, we want to be able to do it without rcedit, but we need to change other stuff, not just icons. What's status of https://github.com/hpvb/ppelib? AFAIK it was created for this purpose, but I'm not sure if it's usable.

@akien-mga
Copy link
Member

Ideally, we want to be able to do it without rcedit, but we need to change other stuff, not just icons. What's status of hpvb/ppelib? AFAIK it was created for this purpose, but I'm not sure if it's usable.

It was fairly advanced as I remember, though @hpvb was maybe being a bit perfectionist with it and didn't get to finish before life caught on. It's probably not far from what we'd need in Godot if you want to take a look / poke hp on RocketChat to discuss it further.

@hpvb
Copy link
Member

hpvb commented Feb 17, 2023

I made a different one here; https://github.com/hpvb/pperesource which is based on ppelib but tries to be less "perfectionist" :P I'll add resource saving in it over the next week or two and then it can be integrated.

I kind of forgot about this to be entirely honest.

@Calinou Calinou added this to the 4.x milestone Feb 17, 2023
@pkowal1982
Copy link
Contributor Author

@hpvb Any news on the topic? Maybe I can help with your library?

@ghost
Copy link

ghost commented Mar 10, 2023

I am on a Mac and having a plugin or native support for adding/changing the icon for Windows would be most helpful.

@pkowal1982
Copy link
Contributor Author

I've created PR with POC how we could remove rcedit completely.

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

Successfully merging a pull request may close this issue.

5 participants