Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String surface cache for FontEngine #967

Closed
wants to merge 1 commit into from

Conversation

hean01
Copy link
Contributor

@hean01 hean01 commented Sep 13, 2012

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..

and prevents of deep font rendering each string in each frame.
@igorko
Copy link
Collaborator

igorko commented Sep 13, 2012

@stefanbeller can you check speed up?

@stefanbeller
Copy link
Collaborator

@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
Copy link
Owner

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
Copy link
Contributor Author

hean01 commented Sep 13, 2012

@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..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants