domtronn / all-the-icons.el

A utility package to collect various Icon Fonts and propertize them within Emacs.
MIT License
1.48k stars 177 forks source link

[SVG] Use the correct font size and make the size customizable #391

Closed Stebalien closed 1 month ago

Stebalien commented 3 months ago

I have two icon sizing issues with the svg branch:

  1. I use a smaller font in my mode-line and tab-line, but also want to insert icons. This means I need a way to specify a size other than the height of the default face.
  2. The font "height" (ascenders + descenders inclusive) of my default font ("JetBrains Mono") is 30 while the "size" (whatever that means) is 22. Unfortunately, inserting a 30px icon causes the line height to increase, making lines with icons taller. Setting the icon size to 22 fixes this.

So, I made a few changes in this patch:

  1. Derive the icon size from the font size (where known), not the text height. Using the text height to make the line too large.
  2. Make the :size configurable (to deal with buffers with multiple fonts/line heights).
  3. Export the function that returns the correct font size of a face to make it easier to specify the correct icon size.
Stebalien commented 1 month ago

What's needed is to extract the logic that adjusts the size of the SVG image to a separate function so you can pass the propertized string to it, it then extracts the various attributes from the image and the faces from the text properties set on the string, make your adjustments and then set them back to the string as text and image properties.

I'm not understanding this at all. Usually you'd create an icon as follows:

(all-the-icons-fluentui-system-icons "document_lock"
                                               :padding 2
                                               :face 'all-the-icons-orange)

With this change, you can explicitly set a size as follows:

(all-the-icons-fluentui-system-icons "document_lock"
                                               :padding 2
                                               :size 18
                                               :face 'all-the-icons-orange)

Where am I getting this propertized string?

wyuenho commented 1 month ago

Ah right sorry, I forgot that macro generates individual functions for each icon set. I was referring to the string returned. All SVG icons are just propertized strings with an image embedded in them.

Stebalien commented 1 month ago

Ah right sorry, I forgot that macro generates individual functions for each icon set. I was referring to the string returned. All SVG icons are just propertized strings with an image embedded in them.

Is the current approach (leaving the font-size questions aside) correct in this case? Or are you looking for a function that takes an icon and changes its size?

wyuenho commented 1 month ago

I'm looking for a solution that will solve this problem and #368, I still don't think this is it, tho we are close.

Stebalien commented 1 month ago

That lets me set the size manually but the default size is still wrong. In my setup at least, I need the size to be the "size" (aref 2) instead of the height (aref 3). Is that not the case in your setup?

wyuenho commented 1 month ago

I use the same font as you. aref 2 is the pixel size, which is the aref 3 - ascent and descent, that means the icon size will be smaller than the letter Q by default, why would you want this? It may help if you show me some screenshots to show me how it is on your screen now and how you want it to be.

Stebalien commented 1 month ago

Size-based (aref 2):

dired-1

Height-based (aref 3):

dired-2

If you look at them side-by-side, you'll notice the lines in the second screenshot have extra spacing between them.

Stebalien commented 1 month ago

This is more noticeable in, e.g., vertico:

c-2

c-1

wyuenho commented 1 month ago

If you look at them side-by-side, you'll notice the lines in the second screenshot have extra spacing between them.

What do you mean? You mean the icon paths protrudes above the x-height and below the baseline, meaning the icon taking up the whole line-height? What's wrong with that? It's an icon, not a font. If it bothers you because you have larger default font sizes, just add more padding. Most people don't use a font size that large, setting the icon size to x-height is too small to see clearly.

Stebalien commented 1 month ago

The part that bothers me is that it increases the space between the lines, it's not just taking up the whole line it's increasing the line height. This by itself wouldn't be a huge issue but if some lines have icons and some don't it looks pretty strange (different line heights).

wyuenho commented 1 month ago

if some lines have icons and some don't it looks pretty strange (different line heights)

This is a fair point. I do notice the some of the icons protrudes below the baseline even in your first vertico screenshot tho, I don't understand why. Let me know if you have any ideas, or you can file a separate bug unrelated to this issue.

Stebalien commented 1 month ago

I'm guessing it might be a build thing if you're not seeing this. I'll file an issue to discuss.

Stebalien commented 1 month ago

Filed here: https://github.com/domtronn/all-the-icons.el/issues/392