dfrg / swash

Font introspection, complex text shaping and glyph rendering.
Apache License 2.0
636 stars 36 forks source link

Some ligatures are incorrect #15

Closed Kethku closed 2 years ago

Kethku commented 3 years ago

https://github.com/be5invis/Iosevka

The demo shows: image

Looks like something about the way iosevka renders ligatures doesn't play well with swash

Kethku commented 3 years ago

https://github.com/tonsky/FiraCode

The demo shows: image

Looks like there are issues with fira code's ligatures as well

zoomlogo commented 3 years ago

This is only with length > 2 ligatures for Fira Code

dfrg commented 3 years ago

Thanks for the heads up. I'll trace the code and see what's going on.

dfrg commented 3 years ago

I pushed a fix for Iosevka and published 0.1.3 to crates.io. I'm hoping this will fix the Fira Code issues as well, but I'll test it in a bit.

zoomlogo commented 3 years ago

I'm hoping this will fix the Fira Code issues

Iosevka works now!

Just above length 2 ligatures in Fira Code dont.

dfrg commented 3 years ago

One out of two ain’t bad! Glad to hear that it’s working. I’ll have to look at Fira Code later this evening.

dustinroepsch commented 3 years ago

https://fsd.it/shop/fonts/pragmatapro/ The ligatures in PragmataPro don't render at all,

Screen Shot 2021-07-28 at 11 56 27 AM

The text was:

    db.enter_span(&[S::family_list("PragmataProMonoLiga Nerd Font"), S::LineSpacing(1.)]);
    db.add_text("This test is PragmataProMonoLiga Nerd Font -> != <= == ==>");

In this example I'm using a version of PragmataPro that was patched with nerd fonts, but using a non patched version has the same issue.

PragmataPro isn't a free font, but if there is any info I can give you to help debug this, let me know! Thanks for the awesome project

dfrg commented 3 years ago

This bug seems like it has to do with rendering rather than shaping. Do you know if Pragmata has embedded bitmaps? The renderer in the demo only has support for the color variety to accommodate emojis.

I added a hotkey to the demo that will dump the glyph clusters. You can reduce the text in the build_document() function to just the desired ligatures, run the demo, and press F7 to print the layout info to the console. That might at least give some indication as to whether or not the shaper is assigning reasonable glyph indices.

You can also try disabling the calt feature to see if the non-ligated versions work. You can do that with this code:

db.enter_span(&[
    S::family_list("PragmataProMonoLiga Nerd Font"), 
    S::LineSpacing(1.),
    S::features(&[("calt", 0).into()][..])
]);
dustinroepsch commented 3 years ago

Thanks, I'll find time tomorrow to do that and post back here :)

dfrg commented 3 years ago

Fira Code should be fixed now (-> 0.1.4). These fonts really abuse the OpenType layout mechanisms, so thanks for the stress test :)

I should note that these are technically not ligatures from the OpenType perspective which explains why kitty uses a weird set of glyph name hacks to identify them.

dustinroepsch commented 3 years ago

I got that debug info for you, heres my build document function

fn build_document() -> doc::Document {
    use layout::*;
    let mut db = doc::Document::builder();

    use SpanStyle as S;

    db.enter_span(&[
        S::family_list("PragmataProLiga Nerd Font")
    ]);
    db.add_text("-> <= >= ==");
    db.leave_span();
    db.build()
}

A screenshot of the window:

Screen Shot 2021-07-29 at 1 22 02 PM

And the output after hitting F7:

Pixel format of the window's GL context: PixelFormat { hardware_accelerated: true, color_bits: 24, alpha_bits: 8, depth_bits: 24, stencil_bits: 8, stereoscopy: false, double_buffer: true, multisampling: None, srgb: true }
OpenGL version 4.1 ATI-4.6.20
Evicted 0
| 0 | 0 | - | 5696 | 0.00, 29.00 |
| 1 | 1 | > | 5696 | 16.00, 29.00 |
| 2 | 2 |   | 4 | 32.00, 29.00 |
| 3 | 3 | < | 5696 | 48.00, 29.00 |
| 4 | 4 | = | 5696 | 64.00, 29.00 |
| 5 | 5 |   | 4 | 80.00, 29.00 |
| 6 | 6 | > | 5696 | 96.00, 29.00 |
| 7 | 7 | = | 5696 | 112.00, 29.00 |
| 8 | 8 |   | 4 | 128.00, 29.00 |
| 9 | 9 | = | 5696 | 144.00, 29.00 |
| 10 | 10 | = | 5696 | 160.00, 29.00 |

Thanks for taking the time to look into this!

dustinroepsch commented 3 years ago

Also tested disabling ligatures using S::features(&[("calt", 0).into()][..], with that setting everything renders just fine:

Screen Shot 2021-07-29 at 1 26 49 PM
dustinroepsch commented 3 years ago

I don't know if pragmata uses bitmaps for ligatures, is there a tool I could use to check that? Thanks again and sorry about the spam!

dustinroepsch commented 3 years ago

UPDATE: I totally forgot to run cargo update to update swash before posting those results. After a cargo update, ligatures are working!! :)

Screen Shot 2021-07-29 at 1 51 36 PM

Thanks for the help!

Kethku commented 3 years ago

This is fixed! @dfrg thanks for taking a look at this!

msiglreith commented 3 years ago

Hi! Found one additional issue with Fira Code I think (rev 437578f31c42d511e119392dcd1868091db53433):

Output: grafik Expected: (Google Fonts) grafik

The last 3 characters are interpreted as : :< instead of :: <.

dfrg commented 3 years ago

Thanks for catching this! I'll look into it and report back here when I figure out what's going on.

dfrg commented 2 years ago

This was a bit of a head scratcher as HarfBuzz, Uniscribe and swash were all producing identical results. It turns out that the version of Fira Code vended by Google Fonts is simply different from the one available on GitHub. If you grab the Google Fonts copy, swash will produce the expected output: fira_code2

msiglreith commented 2 years ago

oh no - thanks a lot for tracking this down! It looks indeed much better with the Google Fonts version (also locally).

dfrg commented 2 years ago

I'd be lying if I said I hadn't made the same mistake a few times while working on this :) I'm going to go ahead and close, but feel free to reopen if you encounter any further inconsistencies!