sf::Font Change & sf::Text Notification

[FRex]

Done...
As for sf::Texture: In theory there is no reason why it's private but I don't care about it, since it's possible use (caching in own GL code while using sf::Texture) is waaay more niche than this (own text layout class).

[select_this]

I hate to nitpick, but it should be 'dirtiness', not 'dirtyness'

[FRex]

It should but that is a common misspelling, including in "buffer dirtyness" context.
So.. we will see what Laurent says.

[Laurent]

It should but that is a common misspelling

Hmm... I expected more rigor from you

If the correct spelling is "dirtiness" then "dirtiness" must be used.

[FRex]

Done too.
Have you looked at the if I mentioned?
https://github.com/FRex/SFML/blob/master/src/SFML/Graphics/Text.cpp#L245
Because of the way it is now, every time text gets drawn without a font set, it will reset it's vertices and bounds but I'd say it's OK enough.

[Laurent]

Yes of course, it's more than ok.

[FRex]

Do you want a pull request for this?

[Laurent]

I'd like to have other opinions, but since there's none, let's do this PR, yes.

[eXpl0it3r]

If there's no other way, I'm okay with it.
What's the opinion of the others?

[binary1248]

So... After re-reading this whole thread again, I still can't see why nobody considered just making sf::Text (yet another) friend of sf::Texture so that it can check m_cacheId itself. Any time font texture data is updated, the sf::Text would check the cache ID and make sure that the geometry is also updated. Yes... it is a bit more conservative (i.e. might update more) than what FRex has proposed, but it is much simpler if you ask me, and more importantly, it doesn't require changes to the public API.

[Laurent]

Because:

There is no reason to make it 100% private because that'd harm any new text layout class that isn't sf::Text because they couldn't check for that.

[binary1248]

But the question now would be: If exposing it through the API is only important for custom text classes, wouldn't the user be able to take care of this themselves? I figured that the original issue was that the SFML classes did not communicate among themselves enough, thus leading to the garbage text. If the user knows when they load a new font, surely they can also notify their own text class to update its geometry as well no?

[Laurent]

Hmm... this is a valid point. Although it might be really complicated for the user to do it externally, instead of relying on a direct feature in sf::Font. But not impossible.

[FRex]

In theory, you can require users to notify sf::Text too... but it's super tedious.
IMO: There is no reason to make sf::Font a friend of sf::Text for this, SFML is supposed to not be an all inclusive toolset so making use of class harder for users (who can't add a friend declaration) seems really counterproductive.
This is not a change BTW, it's a 100% backwards compatible addition, nothing is gone or altered so any 2.x user code will still work.

[binary1248]

Well... I still don't know. What sf::Font does now is basically "caching". The problem is that sf::Text assumes that the cache stays valid forever hence it never checked for it's validity (however that might have been implemented). Caching, be it in sf::Texture via the cache ID or now in sf::Font is really an internal detail if you ask me. Having to expose it through the public API just so non-SFML classes can be built on top of this specific implementation doesn't feel right.

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 .