sf::Text bounds are incorrect in case of underline and strikethrough

[FRex]

Subject of the issue

Local (and thus global) bounds of SFML text do not take underline and strikethrough into account.

Your environment

• Windows 10, but the issue is OS independent.
• SFML from git commit 3f6403e (issue is present in 2.4.1 and 2.5.0 too)
• MSVC 19.37.32822 via Visual Studio 2022 but issue is compiler independent.
• No special compiler flags.

Steps to reproduce

Make an sf::Text instance with style underline set on it, get bounds, try display that bounding box using sf::RectangleShape or similar, see that underline is outside the rectangle drawn.

A more through example with interactive changing of styles, text and outline size, and my (bad - I don't recommend it) attempt at a fix is available at https://github.com/FRex/sfml-text-bounds-with-underline

#include <SFML/Graphics.hpp>

int main()
{
    sf::RenderWindow app(sf::VideoMode(640u, 480u), "Test");

    sf::Font font;
    font.loadFromFile("DejaVuSans.ttf");

    sf::Text txt("Test", font, 200);
    txt.setStyle(sf::Text::Underlined);
    const auto bounds = txt.getLocalBounds();

    sf::RectangleShape sha;
    sha.setFillColor(sf::Color::Red);
    sha.setPosition(bounds.left, bounds.top);
    sha.setSize(sf::Vector2f(bounds.width, bounds.height));

    txt.move(100.f, 100.f);
    sha.move(100.f, 100.f);

    while(app.isOpen())
    {
        sf::Event eve;
        while(app.pollEvent(eve))
            if(eve.type == sf::Event::Closed)
                app.close();

        app.clear();
        app.draw(sha);
        app.draw(txt);
        app.display();
    }
}

Expected behavior

Underline should be included in the bounding box, so it should fall inside the drawn rectangle shape. Use case I have is having a background box drawn under sf::Text using these bounds. There is no way to know how big the box should be now, due to missing underline.

Actual behavior

Underline falls outside the drawn shape, because the bounds don't take underline (and strikethrough) into account. They do take bold and italic styles, and outline thickness (including negative) into account. Also scree screenshot below (result of above program) and repo linked above for more examples.

[FRex]

@eXpl0it3r @ChrisThrasher Take a look at this above and at the repo with example program and fix, and decide if this is a problem that should be fixed. Maybe binary1248 can comment if he is available since he worked on this?

The fix I tried in the repo isn't a good one since it changes bounds even in normal cases, and it needlessly looks at all vertices again, but it's a starting point to consider what 'nicer' bounds can look like.

[ChrisThrasher]

What would a unit test look like to catch this? Maybe we check the local bounds of "Test" then add an underline and observe that the local bounds do not change? Would that be a valid test for this bug?

[FRex]

That test sounds okay, but more strings and fonts can be tested.

Not all texts will change bounds with underline added, e.g. "AgA" due to wide As and long tail of g:

"Test" might be a good string to test with.

Any string that is very "small" naturally like: all spaces, all dots, all carets, all small letters (except small g since it has the tail), etc. will work to show the change.

Anything long (small g) or wide (capital letters like A) will push the bounds out so far that underline won't change them.

[FRex]

There was once a discussion on the forum about text bounds: https://en.sfml-dev.org/forums/index.php?topic=24985.0

It was me, exploiter and Laurent. It was only about trailing and leading spaces in text making the bounds bigger - I disagreed with that approach (but looking at it today after working with positioning texts next to each other - I think it's correct after all).

In last post there is a weak comment by exploiter implying that he thought that underline and strikethrough should change bounds: "take the underline or strike-through decorations, should the bounding box not be adjusted for the now visible space character or visible decorations?"

[ChrisThrasher]

master...test_text

The existing sf::Text::getLocalBounds and sf::Text::getGlobalBounds tests were pretty basic and not terribly useful so I wrote some more that should do a better job actually testing their logic. These will come in handy when we're making changes to the underlying algorithms.

EDIT: I opened #2712 to give us a better foundation for testing changes to getLocalBounds.