Author Topic: DynamicFont fix, again  (Read 9186 times)

mdeletrain

  • Jr. Member
  • **
  • Thank You
  • -Given: 0
  • -Receive: 1
  • Posts: 71
    • View Profile
DynamicFont fix, again
« on: January 25, 2017, 07:27:15 AM »
Hi !

It has been a very long time since I needed to use dynamic fonts but here it is again.
Doing so, I see there is still an issue in current NGUI's version when new characters are added that overflow current font’s texture size : during one frame, garbage can be seen.
I already proposed a patch some years ago that tried to fix that issue, and here I am with a new version based on latest NGUI, which seems to work well, at least in my case :p.

This time, I'll explain all the changes :
 - Label that were marked dirty with MarkAsChanged where not actually updated geometry wise. that's why the issue is still visible in current NGUI -> I added a call to label.UpdateGeometry.
 - Updating labels can lead to new OnFontChanged calls to be done recursively, which would invalidate previous geometry updates, so instead of updating geometry immediately I keep all labels that needs to be updated and only do it once, when we actually exit recursion.
 - For the same reason a label may be updated several times, which is useless. So instead of storing draw calls to be updated, I store labels to be updated, and only do so once for each when exiting recursion.
 - Updating a label’s geometry can cause a OnFontChanged to be triggered because of the UpdateNGUIText call it mades which tries to get font metrics. To prevent that to append when exiting recursion I manually call UpdateNGUIText of labels when I store them for future update.
 - I use HashSets instead of Lists : iterating over them is slower than with Lists but lookups are faster. Since we iterate over them once per recursion exit, and we do lookups once for each label of updated font, it should be better.
 - I pre-initialized them to avoid runtime null check. In the case where we uselessly created them, memory impact is really negligible.


You can try this with the project I attached to this message by following this protocol :
 - before each test, reimport "res/eurofighter/Font Texture" asset, so that all cached data is cleared (sometimes need to do it twice, just check the texture is empty once done)
 - start the application
 - click somewhere in the screen
 - with current NGUI, see there is a glitchy frame (the project sets frame rate to 2FPS when it starts so that you have a chance to see it)
 - with patched NGUI, see there is no more glitchy frame

ArenMook

  • Administrator
  • Hero Member
  • *****
  • Thank You
  • -Given: 337
  • -Receive: 1171
  • Posts: 22,128
  • Toronto, Canada
    • View Profile
Re: DynamicFont fix, again
« Reply #1 on: January 25, 2017, 02:14:11 PM »
Thanks! Adding it to my list of things to consider integrating for the next update.

mdeletrain

  • Jr. Member
  • **
  • Thank You
  • -Given: 0
  • -Receive: 1
  • Posts: 71
    • View Profile
Re: DynamicFont fix, again
« Reply #2 on: January 26, 2017, 05:04:04 AM »
Nice!

As a side note, maybe replacing lbl.MarkAsChanged() by lbl.Invalidate(false) would be better, because it would avoid the cost of processing the text again while that's useless since text did not actually changed shape.

I did not tried on Unity 5 yet. Will do soon and report back once confirmed working there too.

mdeletrain

  • Jr. Member
  • **
  • Thank You
  • -Given: 0
  • -Receive: 1
  • Posts: 71
    • View Profile
Re: DynamicFont fix, again
« Reply #3 on: February 15, 2017, 11:21:17 AM »
I see you did add a modified version of my patch to NGUI's sources, and I have a question : what benefit do you see using List instead of HashSet ?

From my point of view you should avoid them in that case because each time you add a value you also do a Contains() call to shield against duplicates, so Add() complexity becomes an O(n) operation, whereas while using HashSet everything remains O(1).
Since a scene could have a high number of labels that may impact performances quite a bit.

mdeletrain

  • Jr. Member
  • **
  • Thank You
  • -Given: 0
  • -Receive: 1
  • Posts: 71
    • View Profile
Re: DynamicFont fix, again
« Reply #4 on: February 15, 2017, 11:26:24 AM »
Ho! and by the way, thank you including my patch ;D !

ArenMook

  • Administrator
  • Hero Member
  • *****
  • Thank You
  • -Given: 337
  • -Receive: 1171
  • Posts: 22,128
  • Toronto, Canada
    • View Profile
Re: DynamicFont fix, again
« Reply #5 on: February 18, 2017, 06:47:38 PM »
HashSet is better at checking contains / adding items, but it's worse for iteration. I also always avoid 'foreach' as it used to leak memory on iOS. I don't know if that was ever fixed. I doubt there will be enough labels affected at a time for it to matter either way though.

mdeletrain

  • Jr. Member
  • **
  • Thank You
  • -Given: 0
  • -Receive: 1
  • Posts: 71
    • View Profile
Re: DynamicFont fix, again
« Reply #6 on: February 27, 2017, 07:15:01 AM »
HashSet is better at checking contains / adding items, but it's worse for iteration.

True. Here you are going to have a lot of checks and a few iterations at the end, why I used HashSet.
Plus, foreach's cost over a for loop is just an overhead (so grows linearly with collection size), while check cost on a list is O(n)...

I also always avoid 'foreach' as it used to leak memory on iOS. I don't know if that was ever fixed.

True, foreach used to leak on iOS. IIRC this is a bug in mono's AOT compiler, so might not be true anymore because of IL2CPP.
But, if it was to be still true, I doubt it would make any big difference here, because that's something that is to be called very scarcely.
On my side, now that I am mostly writing tools, I usually write code using language's syntactic sugar and only revert to more low level constructs on hot spots, but yeah I was found of micro optimisations back in the J2ME era :) (and I've done quite some tricky ones :p ).

I doubt there will be enough labels affected at a time for it to matter either way though.

I guess you are right for most cases, and most people must be using bitmap fonts anyway by now.
But I recall seeing some people here complaining about some issues when they were using lots of labels, so I guess that might still be an issue.

Anyway, forget about all that, I'm just nit-picking :p !

Thanks for your great support.