Fix incorrect glyph rect for text outline

[cschreib]

• Has this change been discussed on the forum or in an issue before?
• Does the code follow the SFML Code Style Guide?
• Have you provided some example/test code for your changes?
• If you have additional steps which need to be performed list them as tasks!

---

Description

This PR fixes the glyph rect calculation in sf::Font to use the most accurate information from the FreeType rendered bitmap data, rather than the glyph metrics. The returned rect is guaranteed to contain the full rendered glyph with no stretching. It also simplifies the positioning in sf::Text, since the offsetting from the outline is now correctly taken into account in the returned rect from sf::Font.

As a side effect, it seems to have "fixed" an unintended stretching for bold fonts:

This PR is related to the issue #1826

Tasks

• Tested on Linux
• Tested on Windows
• Tested on macOS
• Tested on iOS
• Tested on Android

How to test this PR?

Describe how to best test these changes. Please provide a minimal, complete and verifiable example if possible, you can use the follow template as a start:

• Font I use for testing: noto.ttf.zip
• Code:

#include <SFML/Graphics.hpp>
#include <cmath>

int main() {
    sf::RenderWindow window(sf::VideoMode(768, 512), "Minimal, complete and verifiable example");
    window.setFramerateLimit(60);

    sf::Font fnt;
    fnt.loadFromFile("noto.ttf");

    while (window.isOpen())
    {
        sf::Event event;
        while (window.pollEvent(event))
        {
            if (event.type == sf::Event::Closed)
                window.close();
        }

        window.clear(sf::Color(128, 128, 128));

        float y = 0.0;
        for (std::size_t i = 5; i < 20; ++i) {
            sf::Text txt("Language: C++", fnt, i);
            txt.setFillColor(sf::Color::White);
            txt.setOutlineColor(sf::Color::Black);
            txt.setOutlineThickness(2.0f);
            txt.setPosition(0.0, std::round(y));
            window.draw(txt);

            y += 1.5f*i;
        }

        y = 0.0;
        for (std::size_t i = 5; i < 20; ++i) {
            sf::Text txt("Language: C++", fnt, i);
            txt.setFillColor(sf::Color::White);
            txt.setStyle(sf::Text::Bold);
            txt.setOutlineColor(sf::Color::Black);
            txt.setOutlineThickness(2.0f);
            txt.setPosition(256.0, std::round(y));
            window.draw(txt);

            y += 1.5f*i;
        }

        y = 0.0;
        for (std::size_t i = 5; i < 20; ++i) {
            sf::Text txt("Language: C++", fnt, i);
            txt.setFillColor(sf::Color::White);
            txt.setStyle(sf::Text::Italic);
            txt.setOutlineColor(sf::Color::Black);
            txt.setOutlineThickness(2.0f);
            txt.setPosition(512.0, std::round(y));
            window.draw(txt);

            y += 1.5f*i;
        }

        window.display();
    }
}

[Bromeon]

Thanks a lot for the issue with immediate PR!
Did you happen to see if it also improves the outlines of other fonts?

I'll try to have a look at it on Windows this week.

[cschreib]

It is perhaps not as striking on other fonts, but you can see an effect too, particularly in italic style.

Cantarell Regular (OTF)

DejaVu Sans (TTF)

Vera Mono (TTF)

Vera Sans (TTF)

[eXpl0it3r]

Summoning our fonts expert, @oomek 😄
Can you take a peek at this?

[oomek]

Sure, on it.

[oomek]

There was indeed funny business going on with outline and style flags applied. @cschreib commit seems to fix the rendering with every font, style flag and outline combination I tested so far. Forced autohint advance compensation is still working as before. Thanks for spotting it and fixing.
Just one thing I would change is the line:

SFML/src/SFML/Graphics/Font.cpp

Line 634 in 40a0584

glyph.advance = std::round(static_cast<float>(bitmapGlyph->root.advance.x) / static_cast<float>(1 << 16));

From what I see the root.advance.x is now of long type and a multiplier of 65536 so you could replace it with just:

glyph.advance = static_cast<float>(bitmapGlyph->root.advance.x >> 16);

Update: a little explanation why we can just >> 16 Sine we use forced hinting flag the advance will always be integer and since it's in 16.16 fixed point we can just skip the fractional part. The actual fractional advance is handled by bearings.

[oomek]

I've just noticed some new odd artifact when outline is large enough to make internal circles collapse and turn inside-out

Chaging

FT_Glyph_Stroke(&glyphDesc, stroker, true);

to

FT_Glyph_StrokeBorder(&glyphDesc, stroker, false, true);

fixes i and : but o and p is still affected.

[cschreib]

Just one thing I would change is the line:

SFML/src/SFML/Graphics/Font.cpp

Line 634 in 40a0584

glyph.advance = std::round(static_cast<float>(bitmapGlyph->root.advance.x) / static_cast<float>(1 << 16));

From what I see the root.advance.x is now of long type and a multiplier of 65536 so you could replace it with just:

glyph.advance = static_cast<float>(bitmapGlyph->root.advance.x >> 16);

Update: a little explanation why we can just >> 16 Sine we use forced hinting flag the advance will always be integer and since it's in 16.16 fixed point we can just skip the fractional part. The actual fractional advance is handled by bearings.

Thanks for the explanation, that makes sense. I work in parallel on my own FreeType+OpenGL renderer, which does not use hinting, and it needed round() to get the right advance. Now I know why :) I thought it was a general thing, that the advance had to be rounded, so I ported it to SFML just in case. But you are right, there is absolutely no difference in output from SFML whether I use round() or floor().

I have pushed a commit to apply this suggestion.

I've just noticed some new odd artifact when outline is large enough to make internal circles collapse and turn inside-out

It sounds like it may be a fundamental problem with the Freetype stroker:
https://lists.nongnu.org/archive/html/freetype/2017-05/msg00017.html

When the outline thickness becomes large enough compared to the font size, it is probably best to simply convolve the non-outlined glyph with a circular mask (of radius = outline thickness). This could quickly become a can of worms, so I'm not sure this should be done in this PR.

[eXpl0it3r]

If the two issues aren't directly related, then I too suggest to separate these. If it's really something that SFML can fix/improve then feel free to create another issue, so it can be tracked.

[oomek]

If the two issues aren't directly related, then I too suggest to separate these. If it's really something that SFML can fix/improve then feel free to create another issue, so it can be tracked.

I agree, the guy from freetype repo just confirmed that limitation of the stroker algorithm, I'll keep researching the subject.

[eXpl0it3r]

Thanks a lot for finding this, reporting and fixing it, highly appreciated! 🙂

[texus]

After porting this change to my own code, I noticed that it removes the outline around the underline when sf::Text::Underlined is included in the text style. The same is probably true for strikethrough.
I think outlineThickness should have only been removed in addGlyphQuad and not in addLine.

[eXpl0it3r]

Oh oh 😅

So if only addGlyphQuad is adjusted, does it then still fix the original issue, @cschreib?

[eXpl0it3r]

Anyone willing to provide a fix for the fix, otherwise I'll try to pick it up when I get around to it...

[cschreib]

Sorry, I didn't test strikethrough or underline when making the original changes... I will have a look tomorrow morning, or later this week.

[cschreib]

Yep my bad, I did not look closely enough at the code. I thought addLine meant "add a line of text" rather than "add a straight line for underline or strike-through" (despite the comment above the function being pretty explicit, doh), and blindly removed anything related to outline. I'll open a PR now with the fix for the fix...

[eXpl0it3r]

@texus assuming this fixed it for you as well?

[texus]

@eXpl0it3r Yes, #1836 fixed it.