clintbellanger / flare

Free Libre Action Roleplaying Engine
http://clintbellanger.net/rpg/
GNU General Public License v3.0
166 stars 41 forks source link

String surface cache for FontEngine #967

Closed hean01 closed 12 years ago

hean01 commented 12 years ago

Simple cache of rendered strings so it can be reused on each viewport.render() and not regenerated on each text string that is going to be displayed..

igorko commented 12 years ago

@stefanbeller can you check speed up?

stefanbeller commented 12 years ago

@hean01,

I tried to benchmark the performance improvements of the font caching. So I started flare and measured cpuload/memory in the character selection screen. But I could not find a difference. So I fired up perf tools and started the game with it. In the game I opened the inventory and moved mouse over a few (< 20) items rapidly. With you patch applied I "felt" it was faster and smoother, but it was just an impression of mine. (if you know, it should be faster, it feels so... humans tend to be biased there)

Once introducing such a cache we need to think about the tradeoff of strings cached. The more we are caching, the more memory we need. (Also we need longer to search that list, though rendering fonts usually takes way longer, so it's only a little counter argument.)

How did you find out about fonts needing a cache, i.e. where did you notice lag/cpu spikes or such? Personally I would really like to measure the improvement, instead on relying on feelings.

Maybe we could measure the TurnAroundTime, the time which is needed to render one frame. Maybe it would be better to measure the inverse, so the time to wait between each frame. And if the waiting time jitters strong at getting new fonts rendered, we'll have found the why it feels smoother?

clintbellanger commented 12 years ago

Note that WidgetLabels keep a cache of their own font rendering. Most Widgets with text should have an internal WidgetLabel or some specific render buffer. Almost all text rendered to screen should be in a Widget (if not, that should be fixed).

So this cache is likely an unnecessary duplication.

As always, we can eliminate unnecessary code work by discussing ideas before implementing them.

hean01 commented 12 years ago

@clintbellanger ah ok, the main reason to add this was that i read some issue mention the the load scene and where the label was reused and that i needed some text rendering in the cutscenes for caption.. i assumed that cache wasn't done but apperently it is, imho on the wrong level.. the cache in place is only used per label..

I looked into tooltip which can draw benefit of this level of cache regarding rendering times, labels will have benefit if there are more then one label with same text, the data allocation (surface) will be shared.

Back to my cutscene impl i guess i should just use a WidgetLabel..