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

feat: generate icons as 16×16 #2837

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Jan 22, 2025

Description

Converts all generated icons to a 16×16 format with pixel-perfect points, matching the updated contribution guidelines.

Contribution Guidelines

@Repiteo
Copy link
Contributor Author

Repiteo commented Jan 23, 2025

The CL tools can't recognize generated files, so here's the svg outputs:

file

file

folder

folder

folder-open

folder-open

folder-root

folder-root

folder-root-open

folder-root-open

@Repiteo Repiteo mentioned this pull request Jan 23, 2025
2 tasks
Copy link
Member

Choose a reason for hiding this comment

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

I could make out some differences between the normal folder icon and some specific folder icons (in this case the folder-dist icon). Then I put both of them together and found out that there's a different design. I'd highly appreciate if we could use the same folder shape for the default and the specific folder icons:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with them being synced, but I'm not certain if that uniformity is actually enforced atm. It'd be nice if there was some way to explicitly sync these icons, but that might be outside the scope of this PR. I'll still look into it later today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resynced the upper inconsistency for folders. However, I left the lower inconsitencies with the open folder deliberately, as it was previously lopsided so this is a "fix". However, if you want an exact match with (most) existing folders, then I think I'll just not bother with the folders at all — they couldn't really get anywhere close to pixel-perfect with their existing design
25-01-25 11-39-39 godot windows editor dev x86_64

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer if all folder icons have the exact same shape. The rendering of the pixels will not work properly for these areas anyway. If only 16x16 pixels are available for rendering in low resolution, it only makes a difference if we fill the pixels completely. This can be clearly seen in our example from the Contribution Guidelines. Here you can clearly see that the lines that end directly with a pixel and thus fill an entire pixel have a very sharp edge.

image

"Consequently, when rendering a curve, we're essentially asking the display to render a fraction of a pixel, which is impossible. As a result, curves tend to appear blurry. This is normal. However, it's perfectly fine to use curves, circles, and rounded edges in your icons. Just keep in mind these limitations if you're wondering why your icon doesn't look as sharp as you'd like." (bullet point "Curves vs straight lines" in the contribution guidelines).

This is also the reason why the red areas in our folder will also never be perfect, regardless how the corners will be formed:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll roll with full parity then. The new commit is the original version downscaled by 50%, so they should match up perfectly with existing folders. I am interested in eventually revisiting my pixel-perfect implementation, but only if that could somehow be applied to all folders.

@Repiteo Repiteo force-pushed the 16×16-generated branch 3 times, most recently from 3c70591 to 03d947d Compare January 26, 2025 16:18
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.

2 participants