fallahn / xygine

2D engine / framework built around SFML
218 stars 19 forks source link

Generate Text local bounds if not already generated #96

Closed JonnyPtn closed 6 years ago

JonnyPtn commented 6 years ago

Means you can query text bounds immediately without waiting for the scene to update.

Ideally I'd prefer not to just duplicate what text renderer does, but I couldn't work out a tidy way of avoiding it, so this was the path of least resistance!

Also fixed what I presume was a typo in TextRenderer? let me know if I'm misunderstanding that

fallahn commented 6 years ago

Thanks! This was the exact reason I hadn't done it originally - although now I have an idea. How about moving the bounds update into a private function in the component and have the text system call it on components that are dirty? This would remove the code duplication, while still allowing the component to update itself immediately. Oh and good catch on the typo - I can't believe that hadn't caused any weird bugs so far :D

JonnyPtn commented 6 years ago

I thought of that too, and it's a viable option but would still result in some duplication (and a decrease in performance, albeit negligible):

TextRenderer::update also does the recreation of the quads when it updates a dirty text:

//create the quads.
const auto& glyph = font->getGlyph(currChar, text.m_charSize, false);
addQuad(text, sf::Vector2f(x, y), glyph);

So If we were to add an Text::updateLocalBounds function it would have to iterate over the text string once, and then TextRenderer::update would iterate over the string again, so it would iterate over the string twice every time it was made dirty, unless I'm missing some other solution.

I don't feel terribly strongly either way, as it will probably only cause a noticeable difference if someone is regularly modifying a particularly long text, so I'll leave it up to you to decide which solution you prefer

fallahn commented 6 years ago

Oh, hm I forgot about that. Well in that case, as you say, let's take the path of least resistance. My only real worry about code duplication is any future updates not making it from one instance to another - but I think the text system is pretty much set in stone now (maybe underline/strikethough/outlines would get added? I don't think I'd personally have a use for those features though)