tdewolff / canvas

Cairo in Go: vector to raster, SVG, PDF, EPS, WASM, OpenGL, Gio, etc.
MIT License
1.46k stars 98 forks source link

Vertical alignment inside a text box using some fonts is misaligned #247

Closed mufty closed 10 months ago

mufty commented 10 months ago

canvas_test.zip

Attached example project where the issue is visible. There is an example using "Omnes" font that one is aligned correctly doesn't matter the setup. The "Helvetica" font isn't it is more visible the bigger the text is. While the text is small it's sort of OK though still not in the center like it should be, it's just a little off.

helvetica_small

But the bigger the text is the more visible it is how off it is.

helvetica

Compared to the font that just sits in the middle like it should.

omnes

tdewolff commented 10 months ago

Yes there seems to be something off, thanks for raising the issue. I'll look into it and get back to you.

tdewolff commented 10 months ago

Should be fixed now, please let me know how it goes!

mufty commented 10 months ago

nice will give it a try thanks

mufty commented 10 months ago

@tdewolff the last change did help a little it did move the text down but it's still not in the middle.

helvetica

mufty commented 10 months ago

In some cases we can fix this on our end with a workaround by recalculating the position again. Something like:

dy = (actualTextboxHeight - textBoundsHeight) * verticalAlign
context.Translate(0, dy)

This tho is big ugly because it's using bounds so we created a fake textbox just to get bounds using glyphs and then just set the position to the difference of it. So this approach is kinda completely ignoring the positioning on your end and applying a fix after the library calculated position.

This workaround is only needed for some fonts because some of them are positioned correctly so that also doesn't help the case to keep doing that just for the couple fonts that need it.

tdewolff commented 10 months ago

Canvas (and browsers) vertically center text based on its ascender+descender, which are the maximum height above and below the baseline respectively. I guess when you add a character like g or q it will look centered. You could in your case move the text half the descender down (see FontFace.Metrics()).

On Mon, Sep 11, 2023, 03:01 Michal Gubriansky @.***> wrote:

In some cases we can fix this on our end with a workaround by recalculating the position again. Something like:

dy = (actualTextboxHeight - textBoundsHeight) * verticalAlign context.Translate(0, dy)

This tho is big ugly because it's using bounds so we created a fake textbox just to get bounds using glyphs and then just set the position to the difference of it. So this approach is kinda completely ignoring the positioning on your end and applying a fix after the library calculated position.

— Reply to this email directly, view it on GitHub https://github.com/tdewolff/canvas/issues/247#issuecomment-1713219452, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKOGHQBTXFIMKP76P6AMB3XZ2SMTANCNFSM6AAAAAA4P36RPE . You are receiving this because you were mentioned.Message ID: @.***>

mufty commented 10 months ago

The problem with that approach is that it isn't universal and won't work correctly when applied to all fonts like that. Because some of them are centered and some aren't and I don't see a way to identify when to apply it or not. It's a font issue really because the font doesn't have correct values set in its properties like you say and thus isn't positioned correctly when using just data from the font and not glyphs.

So yeah I need a solution that'll work for multiple types of fonts and for that I only see using glyphs as the one "blanket" fix.

Canvas (and browsers) vertically center text based on its ascender+descender, which are the maximum height above and below the baseline respectively. I guess when you add a character like g or q it will look centered. You could in your case move the text half the descender down (see FontFace.Metrics()).

mufty commented 10 months ago

image Made a simple test using the exact same font in HTML and CSS not sure if that proves anything just adding it here for more context.

tdewolff commented 10 months ago

It is a font problem but not entirely. The ascender+descender are part of what is vertically centered, since your characters are all above the baseline (and not into the descender), it looks unbalanced. But it's not unbalanced when using those characters: Screenshot from 2023-09-14 13-12-37

However, I can't replicate your last example. When using Firefox it is shown as: Screenshot from 2023-09-14 13-10-49

mufty commented 10 months ago

hehe I can't get it to show up like you did either :) for me it's always in the middle and the characters that are in the descender they aren't centered.

<!doctype html>
<html>
  <head>
    <style>
        @font-face {
            font-family: customfont;
            src: url('HelveticaLTStd-BlkCond.ttf') format('truetype');
        }

        div {
            display: table-cell;
            background-color: red;
            font-size: 150px;
            font-family: customfont;
            height: 200px;
            vertical-align: middle;
        }
    </style>
  </head>
  <body>
      <div>
        LOREM IPSUM gjq;,|@
      </div>
  </body>
</html>

image

Tried in FF, Edge and Chrome they all look the same like this.

mufty commented 10 months ago

I'm not trying to prove here that you are wrong but rather to understand why it is like this to come up with some blanket fix. Cuz we use all sorts of fonts and 9/10 of them are fine. I know this is a font issue but I need a solution that works for all fonts so what do the browsers do that's not done here then? We'd like it to show up like my tests in the browser that's our expected result.

mufty commented 10 months ago

Oh this thing might be an OS thing. Thus why I can't replicate it in the browser either like you did. What OS do you use? I'm on Windows most of the time. OS might have something to do with this cuz there is different metrics on fonts for different OS. I've seen you mostly use HHead values from sfnt.

For ref this is the font in question in fontforge: image

mufty commented 10 months ago

Yeah it's the OS metrics. Tried playing with it by changing it here: https://github.com/tdewolff/canvas/blob/e6ea538e3e50a543f25779eb1d6d06f8f9974e56/font.go#L586

And it does get better for that one font I have that has the issue. But that breaks the other fonts. So now the question is how to fix this?

mufty commented 10 months ago

canvas_test.zip

Updated the test application a little with what I had to do to make it kinda work for both fonts. The difference is that I had to move the text by half of descender in both cases at the end.

And I had changed the font.go to use win metrics like this:

func (face *FontFace) Metrics() FontMetrics {
    sfnt := face.Font.SFNT
    ascender := int16(sfnt.OS2.UsWinAscent)
    descender := int16(sfnt.OS2.UsWinDescent)
    return FontMetrics{
        LineHeight: face.mmPerEm * float64(ascender-descender+sfnt.Hhea.LineGap),
        Ascent:     face.mmPerEm * float64(ascender),
        Descent:    face.mmPerEm * float64(-descender),
        LineGap:    face.mmPerEm * float64(sfnt.Hhea.LineGap),
        XHeight:    face.mmPerEm * float64(sfnt.OS2.SxHeight),
        CapHeight:  face.mmPerEm * float64(sfnt.OS2.SCapHeight),
        XMin:       face.mmPerEm * float64(sfnt.Head.XMin),
        YMin:       face.mmPerEm * float64(sfnt.Head.YMin),
        XMax:       face.mmPerEm * float64(sfnt.Head.XMax),
        YMax:       face.mmPerEm * float64(sfnt.Head.YMax),
    }
}

doing this both fonts are in the center: image image

Not sure why the manual update to move the text by half descender was needed in this case.

mufty commented 10 months ago

This would be what's expected on our end (without the descender adjustment). I did have a look at some other libraries we used in the past and they seem to use the Win metrics by default and do adjustments based on that.

Prior to swapping to go and using your library, we used nodejs with opentypejs + node-canvas + fabric. Opentypejs is using win metrics all the time thus why the expected result doesn't match what we need after the swap.

Not sure what other impact if any would it have to swap to win metrics on your end and make adjustments accounting for the descender when aligning text vertically.

But yeah I think I found what the issue is just not sure about the fix without your input. Spend over a week looking into this thing every day my head started to hurt but I got there somehow :D

tdewolff commented 10 months ago

Thanks for the in-depth analysis. I believe the current implementation is good for MacOS and Linux/Unix, but perhaps we should use different metrics depending on the platform? Specifically, if we're on Windows, use the Win or Typo metrics, and otherwise the Hhead metrics. Would that solve your problem?

See https://github.com/googlefonts/gf-docs/blob/main/VerticalMetrics/README.md and https://glyphsapp.com/learn/vertical-metrics for more information.

mufty commented 10 months ago

Thanks for the links those were helpful for adding more information about how it works. However based on that new info and what I know already it looks to me that detecting OS and using different metrics based on that won't help. Even though I'm mostly on Windows our application runs on Linux and so is the test application attached. The images produced are from it running on Ubuntu so if you did a platform detection and swapped metrics based on that it wouldn't help because on Linux it would still use the metrics like it is now thus produce the same incorrect result.

It looks to me that most of the current applications still use one of the legacy strategies for metrics and some font authors account for that. That's also why our prior nodejs approach worked the way it did (with opentypejs using win metrics no matter the platform).

IMO the way to handle this should be to use the "Use typo metrics" flag. If that's true only then use the metrics as is defined by the font itself. Otherwise use the legacy MS system (even on Mac and Linux). There even is a hint in fontforge as to why the flag has been added to fonts. image

I think that would be what should be done about it. I couldn't find if sfnt parses that flag from the fonts or if you can add it to your parser as you do wrap sfnt anyway.

It's a double edged sword and people might play ye ol "spiderman pointing fingers" game, it's still something applications need to account for anyway. If you don't you get these inconsistencies with some fonts that weren't done very well because they only test on one platform (or are intended only for one of them). Nowadays people download fonts from the internet left and right from all sorts of shady websites and we as application developers still have to make sure that these shady fonts are displayed correctly. Unfortunately such is life (ಥ﹏ಥ)

tdewolff commented 10 months ago

Just adding some information about uses in other software:

Freetype2: https://github.com/mozilla/gecko-dev/blob/36a0e62e4484f707e1eb88a6f3968c9ed72d0795/modules/freetype2/src/sfnt/sfobjs.c#L1329 Thebes: https://github.com/mozilla/gecko-dev/blob/36a0e62e4484f707e1eb88a6f3968c9ed72d0795/gfx/thebes/gfxFT2FontBase.cpp#L385 Qt: https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/text/qfontengine.cpp?h=dev#n387

I've pushed a fix that uses ascender, descender, and line gap values like other engines do. Both fonts now align vertically centered, even the Omnes looks better. Please let me know if this works for all your fonts!

mufty commented 10 months ago

Nice that's exactly what I had in mind you only use the typo metric when the font explicitly says so. Will have to run more tests but first pass looks ok to me. If we find something else to do I'll do a new ticket won't beat this dead horse anymore :)

Thank you ;)