sf::Font Change & sf::Text Notification

[FRex]

I don't know either, I more often than not take "this cements the stupid stuff away so no one has to deal with it ever again" and "don't be annoying" over "feels right (to make users do tedious stuff)". But then again, I write mostly for myself.

You see that I let this thread sit for a year unnoticed, it's because for me it's just trivia about sf::Text and sf::Font - I don't care because I never reuse sf::Texture and sf::Font instances myself.
Exploiter noticed it because I pointed to it in pixelated text thread, saying that dirtyness counter would fix this issue as well. Your fix would handle both too but it'd be a fair bit more... obnoxious to users who reuse sf::Font instances (fortunately not me ).

[eXpl0it3r]

Strictly speaking, one could even argue that when you set an sf::Text's font, you promise not to reload it without setting it again. Reloading the font without re-setting it would break that promise since you are in effect "changing the font" without informing (re-setting the font) the sf::Text. This implies that the (m_font != &font) check would have to be removed though, but I honestly can't think of a reasonable use for that check anyway .

Well in my example I did reload the font and did reset the font on the text object and yet it still didn't render, which in my opinion really shouldn't happen...

Anyways what the general verdict on the open PR?

[Laurent]

Well in my example I did reload the font and did reset the font on the text object and yet it still didn't render

-->

This implies that the (m_font != &font) check would have to be removed

[eXpl0it3r]

Ah didn't realize why it should get removed.

[eXpl0it3r]

Strictly speaking, one could even argue that when you set an sf::Text's font, you promise not to reload it without setting it again. Reloading the font without re-setting it would break that promise since you are in effect "changing the font" without informing (re-setting the font) the sf::Text.

I kind of agree with you, but shouldn't we make things consistent? Like if you set a texture and you "reload" as much as you want without having to resetting it.

Should we add a vote to the thread?

[binary1248]

I agree that something should probably be done about it. I just don't have a good feeling about adding something to the public interface just because of SFML's internal optimization. The interface guarantees behaviour between the library and user code, independent of implementation. If the implementation doesn't fulfil those guarantees then it should be fixed without adding even more to the public interface which means even more guarantees. In this case the new guarantee would mean that we won't be able to abstract from the optimization because it has been made public.

If a user wants to create their own text class then they are free to implement their own optimizations as well. But they will have to bear in mind that they are not able to reuse something that is internal to SFML. They will have to make it work independent of SFML, which in the end is a better choice anyway if you want to build something that should be reusable.

[Hiura]

Just a quick note on the status of this PR (https://github.com/SFML/SFML/pull/769) since no consensus was found since January on whether we should:
 * "help" users who reuse `sf::Font` instances by adding a public versioning index (aka `dirtiness`) to "notify" users that the font has changed [this PR],
 * do the same but privately (by making `sf::Font` and `sf::Text` friends) and recommend users who implement some utility like `sf::Text` to not rely on this feature [highly debatable IMO],
 * do nothing and let the users manually notify `sf::Text` when they reload a font. [opposite direction of this PR]

Applying this PR would make things more consistent with `sf::Texture`. I think we should do it because if the user has to keep track of which `sf::Font` a `sf::Text` instance uses then it will become quite a mess very quickly (you'll need some very clever resource manager). Also, `sf::Text` takes a reference to a font and the user has to guarantee the font will live long enough, so why wouldn't it make sure it's displaying the right thing?

[Nexus]

This topic is still open for discussion. The problem summarized: Reloading sf::Font while it is used by sf::Text instances invalidates the glyph resources, and sf::Text does currently not recognize this, leading to bad rendering. Hiura summarized possible solutions just above.


The question is also whether a sf::Font instance can be considered "the same" after it has been reloaded, or after it has been re-assigned using operator= (which is another case to consider). If so, we would use the object's address to identify it, i.e. any font constructed in the same place would be considered identical. In an extreme case, even manual destruction + placement new should not break this.

The other option is that any reload should be considered as a fresh instance, and texts referring to it become invalid. This is probably less intuitive for the user. But we should also consider that in SFML 3, the bool Font::loadFromXy() methods will probably vanish in favor of static Font Font::fromXy() factories that can throw exceptions. Reloading then effectively equals creating a new instance (even if the same address is being reused).


I'm not sure whether we should enlarge the public interface with a "dirtyness counter" (or "generation", which is a better name in my opinion). Consequently, it would have to be done for other classes relying on some cache as well. And as binary1248 states, it exposes an implementation detail (namely the fact that SFML uses a cache), and in case that changes one day, the API has to change as well.

So I'm personally rather in favor of implementing this privately. It can still be made public if we see that there are many people using another front-end to sf::Font so that this becomes an actual issue; but let's not do it prematurely and limit ourselves in the future.

[Hiura]

So I'm personally rather in favor of implementing this privately. It can still be made public if we see that there are many people using another front-end to sf::Font so that this becomes an actual issue; but let's not do it prematurely and limit ourselves in the future.

That could indeed be a good starting point and make things evolve a bit. After all, few are the people who reload a font, aren't they?

[eXpl0it3r]

So I'm personally rather in favor of implementing this privately. It can still be made public if we see that there are many people using another front-end to sf::Font so that this becomes an actual issue; but let's not do it prematurely and limit ourselves in the future.

I thought the whole problem was that we couldn't implement it privately? Or what exactly did you mean with that?

[Nexus]

It is possible, the question was whether we should do it.

The argument against making the validity/generation/dirty flag private was that sf::Font clients other than sf::Text cannot query the font whether its values are up-to-date, instead they need to be notified manually when a font is reloaded.

The argument for making it private was that we wouldn't expose implementation details of an internal optimization (that might possibly change in the future), and that other sf::Font clients combined with font reloading is an extremely rarely occurring situation, not justifying the price.

[eXpl0it3r]

Bump!

Do we need a poll?

[Hiura]

Maybe.. Personally I'd say we could start with what Nexus said:

So I'm personally rather in favor of implementing this privately. It can still be made public if we see that there are many people using another front-end to sf::Font so that this becomes an actual issue; but let's not do it prematurely and limit ourselves in the future.

[eXpl0it3r]

A few years later...

Digging this older thread out, as we've once again did indeed run into someone wanting build their on text implementation.
Quick re-cap, FRex's PR was eventually superseded by Foaly's PR, which is what we're having these days, where sf::Font friends sf::Text and then we can access the texture cache ID owned by the font from the text class.
This works great for our own text class, but anyone wanting to implement their own text class, won't be able to figure out if the font has changed.

We decided against resolving this limitation for the time being and see if there are people who actually want to take advantage of.
Today, someone Discord (JSpirit) asked about having the cache ID public, but they are not the first one to ask. Over the past few years, we've seen this request not very regularly, but still more than enough, that we shouldn't ignore it.

But now, standing in the middle of the SFML 3 development, we also have a new option, that is to not only add to the API, but to change it, if it helps with this problem.

Anyone got an idea that's better than having a public dirtiness flag?

[eXpl0it3r]

Two ideas that we briefly discussed on Discord where

• Callbacks
• Observer Pattern

kimci86 wrote a quick code mock up to understand what the observer pattern could look like:

class FontObserver
{
public:
    virtual void onFontUpdate() = 0;
};

class Font
{
public:
    void addObserver(FontObserver&);
    void removeObserver(FontObserver&);
private:
    void notifyObservers()
    {
        for(auto* obs : m_observers)
            obs->onFontUpdate();
    }
    std::vector<FontObserver*> m_observers;
};