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

modify_font strikethrough_position isn't respected on macOS #5946

Closed
fenetikm opened this issue Jan 30, 2023 · 8 comments
Closed

modify_font strikethrough_position isn't respected on macOS #5946

fenetikm opened this issue Jan 30, 2023 · 8 comments
Labels

Comments

@fenetikm
Copy link

fenetikm commented Jan 30, 2023

Describe the bug
The modify_font strikethrough_position config option isn't respected - it's just hardcoded.

To Reproduce
Steps to reproduce the behavior:
(on macOS)
1.Set modify_font strikethrough_position 3
2. Have a look at the strikethrough position using, say, echo -e "\e[9mstrikethrough\e[0m"
3. Set modify_font strikethrough_position -3
4. Have a look at the strikethrough position, it will be the same as step 2

Environment details

aths:
  kitty: /Applications/kitty.app/Contents/MacOS/kitty
  base dir: /Applications/kitty.app/Contents/Resources/kitty
  extensions dir: /Applications/kitty.app/Contents/Resources/Python/lib/kitty-extensions
  system shell: /bin/zsh
Loaded config files:
  /Users/michael/.config/kitty/kitty.conf

Config options different from defaults:
bold_font                    Fira Code SemiBold
bold_italic_font             Hack Bold Italic
box_drawing_scale            (0.1, 1.0, 1.5, 2.0)
copy_on_select               clipboard
detect_urls                  False
enable_audio_bell            False
font_family                  Fira Code Regular
font_features:
{'FiraCode-Regular': ('+ss01', '+ss03', '+ss04', '+ss02', '+ss10'),
 'FiraCode-Retina': ('+ss01', '+ss03', '+ss04', '+ss02', '+ss10'),
 'FiraCode-SemiBold': ('+ss01', '+ss03', '+ss04', '+ss02', '+ss10')}
font_size                    15.0
hide_window_decorations      2
input_delay                  2
italic_font                  Hack Italic
macos_option_as_alt          3
macos_thicken_font           0.4
macos_traditional_fullscreen True
modify_font:
    baseline 2
    cell_height -2
    cell_width -1
    strikethrough_position -7
    strikethrough_thickness 200%
    underline_position 1
    underline_thickness 200%
mouse_hide_wait              3.0
remember_window_size         False
single_window_margin_width   FloatEdges(left=2.0, top=2.0, right=2.0, bottom=2.0)
symbol_map:
        U+23fb - U+23fe → Symbols Nerd Font
        U+2665 - U+2665 → Symbols Nerd Font
        U+26a1 - U+26a1 → Symbols Nerd Font
        U+2b58 - U+2b58 → Symbols Nerd Font
        U+e000 - U+e00a → Symbols Nerd Font
        U+e0a0 - U+e0a3 → Symbols Nerd Font
        U+e0b0 - U+e0c8 → Symbols Nerd Font
        U+e0ca - U+e0ca → Symbols Nerd Font
        U+e0cc - U+e0d2 → Symbols Nerd Font
        U+e0d4 - U+e0d4 → Symbols Nerd Font
        U+e200 - U+e2a9 → Symbols Nerd Font
        U+e300 - U+e3e3 → Symbols Nerd Font

... I can see in core_text.m it's just hardcoded as compared with freetype.c. (I also note that strikethrough_thickness just comes from the underline thickness and can't be independently set, again in comparison to the freetype implementation - let me know if you want a separate ticket for that)

Thanks.

@fenetikm fenetikm added the bug label Jan 30, 2023
@kovidgoyal
Copy link
Owner

The code to adjust these is backend independent so I dont see how it
could not work of macoS alone. Try using a larger value like say 20%.

@fenetikm
Copy link
Author

Here it is with 20%:
CleanShot 2023-01-30 at 19 49 30@2x

50%:
CleanShot 2023-01-30 at 19 50 24@2x

-50%:
CleanShot 2023-01-30 at 19 52 02@2x

Is there something else I could try?

@kovidgoyal
Copy link
Owner

The code for modifying the positions is all in fonts.c see the calc_cell_metrics() it has nothing platform specific in it.

@fenetikm
Copy link
Author

Hmmm doesn't it call cell_metrics to set the initial values which get adjusted? And cell_metrics has a free type version and a coretext version? (I'm not familiar with any of this, just poking around)

freetype.c has this fragment of code in cell_metrics:

    if (self->strikethrough_position != 0) {
      *strikethrough_position = MIN(*cell_height - 1, (unsigned int)font_units_to_pixels_y(self, MAX(0, self->ascender - self->strikethrough_position)));
    } else {
      *strikethrough_position = (unsigned int)floor(*baseline * 0.65);
    }
    if (self->strikethrough_thickness > 0) {
      *strikethrough_thickness = MAX(1, font_units_to_pixels_y(self, self->strikethrough_thickness));
    } else {
      *strikethrough_thickness = *underline_thickness;
    }

and core_text.m just has:

    *strikethrough_position = (unsigned int)floor(*baseline * 0.65);

and:

    *strikethrough_thickness = *underline_thickness;

@kovidgoyal
Copy link
Owner

Yes, that sets the initial position, which comes from the font file. The
adjustment is applied after.

@fenetikm
Copy link
Author

ok thanks - I'll see if I can find the actual issue then.

@fenetikm
Copy link
Author

Hi @kovidgoyal - I believe it's the strikethrough_position line in fonts.c:

    if (baseline_before != baseline) {
        int adjustment = baseline - baseline_before;
        baseline = adjust_ypos(baseline_before, cell_height, adjustment);
        underline_position = adjust_ypos(underline_position, cell_height, adjustment);
        strikethrough_position = adjust_ypos(underline_position, cell_height, adjustment);
    }

instead it should be:

        strikethrough_position = adjust_ypos(strikethrough_position, cell_height, adjustment);

I was incorrect re strikethrough thickness, that's working ok.

@kovidgoyal kovidgoyal reopened this Feb 1, 2023
@fenetikm
Copy link
Author

fenetikm commented Feb 1, 2023

Great, thanks 👍

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

No branches or pull requests

2 participants