Opened 14 years ago
Closed 14 years ago
#1013 closed enhancement (fixed)
Make BIDI an option via serverconfig.ini
|Reported by:||zspitzer||Owned by:||waltweltonlair|
#271 adds support for BIDI text rendering, however, there is a circa 5% overhead for this support for all maps with labels.
As the majority of users won't ever need this, it should be an option
Change History (5)
comment:1 by , 14 years ago
comment:2 by , 14 years ago
|Status:||new → assigned|
Well I'm still awake, and I've put together an initial fix for this. I've updated the BIDIConverter class to first check if the input string contains right-to-left characters, and only if it does will the conversion algorithm be called.
Initial results for 100 iterations of the dense labeled map (test 2 in #217 discussion)
- time without any BIDI checks: 58.8 seconds
- time with unoptimized BIDI checks: 63.0 seconds
- time with optimized BIDI checks: 59.0 seconds
So with the optimized check it's now only a 0.3% performance impact for this map.
I will do more testing tomorrow and then get this submitted.
comment:3 by , 14 years ago
I meant test 2 in #271 discussion...
comment:4 by , 14 years ago
Fix #1013 (Make BIDI an option via serverconfig.ini)
The motivation for the ticket was to give users the option of avoiding the performance impact of BIDI conversion if they know none of their maps need it.
I solved this by adding a check to the converter which determines if conversion should be performed. It simply checks if any character in the string is outside the range [0..255]. A more thorough check could be implemented, but the simple check is sufficient for the vast majority of cases (and it's faster). If no characters are outside the range then the conversion method simply returns the passed-in string.
With this update the performance impact of the fix for #271 on maps containing English-only labels is negligible. I reran the test where I did 100 iterations of the dense labeled map (test 2 in #271 discussion) and could not measure a statistically significant performance change.
comment:5 by , 14 years ago
|Status:||assigned → closed|
Comment from Traian:
I've asked Traian if he still has that code laying around, and if so we could consider adding it. If it reduces the overhead to near 0 then that's better than another serverconfig option.