Opened 9 years ago

Closed 9 years ago

#1013 closed enhancement (fixed)

Make BIDI an option via serverconfig.ini

Reported by: zspitzer Owned by: waltweltonlair
Priority: low Milestone: 2.1
Component: Rendering Service Version: 2.1.0
Severity: trivial Keywords:
Cc: External ID:

Description

#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 Changed 9 years ago by waltweltonlair

Comment from Traian:

When I prototyped this, I had code to detect if the string is plain ASCII, in which case I did not pass it to the reordering code. This would reduce your 5% overhead to nearly 0.

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.

comment:2 Changed 9 years ago by waltweltonlair

Owner: set to waltweltonlair
Status: newassigned

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 Changed 9 years ago by waltweltonlair

I meant test 2 in #271 discussion...

comment:4 Changed 9 years ago by waltweltonlair

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 Changed 9 years ago by waltweltonlair

Resolution: fixed
Status: assignedclosed
  • fixed in the trunk stream with submissions r3942
  • fixed in the MGOS 2.1 stream with submission r3943
  • fixed in the sandbox stream with submission r3944
Note: See TracTickets for help on using tickets.