[GEOT-6127] LabelSplitter start-index bug.

[wcmatthysen]

• Fixed a bug in the LabelSplitter class where the start index would be
  incorrectly advanced at the end of the buildFontRanges() method. This
  happened when a font could not be found to display the character at
  the current index, the start-index would then be incremented in the
  else. This can lead to an incorrect font-range from being created, or
  at worst a StringIndexOutOfBoundsException.
  The fix for this is to not increment the start-index inside the
  for-loop, but to keep it fixed so that all fonts can be checked
  whether or not they can render the current character. Only after the
  loop has iterated through all fonts; and no font has been found; do we
  increment the start-index.
• A new unit test class called LabelSplitterTest was added. These tests
  were written to illustrate the abovementioned bug in different edge
  cases. With the first test, we have a string with some special
  ethiopic and arabic characters in the middle that cannot be rendered
  by the given fonts. If the bug-fix is not applied to the LabelSplitter
  the ranges will be completely messed up.
  The second test has a composite chinese character at the start that
  cannot be rendered by the given fonts. However, the rest of the
  characters can be rendered by the "Droid Sans Fallback" font. If the
  bug-fix is not applied the LabelSplitter will create a font range that
  will cause a StringIndexOutOfBoundsException.
• The DroidSansFallback font had to be added as it is needed in the
  second test method.

[wcmatthysen]

The Travis build stalled for some reason.

[bradh]

I've restarted the build.

[wcmatthysen]

Great, the Travis build was successful. However, looks like the AppVeyor one failed. But, it failed on the Web Map Tile Service Client and not the Renderer, so that's ok.

A quick test to see what I'm trying to fix is, if you remove the start-index fix that I made to LabelSplitter and run the two tests, you'll see the font-ranges for the first test being messed up. The second test will break with a StringIndexOutOfBoundsException.

I encountered the StringIndexOutOfBoundsException when I tried to render the first couple of layers of OpenStreetMap tiles in GeoServer for the entire world. The exception causes the entire tile-rendering process to just bomb out and you cannot continue to render tiles past this point. However, this is quite rare as I only encountered this once, and only for a single label somewhere in China. So, this is most probably a very rare edge case that causes this exception to pop up.

[aaime]

The AppVeyor build failed on an online test that should not run on a normal build, I've tried to get the module maintainers to amend it (@etj @ianturton) so far with little success ;-)

testDefaultStyleRequired(org.geotools.data.wmts.client.WMTSServiceTest)

[ianturton]

None of those tests should be running unless there is an online fixture set up, I'll try to check tonight.

[bradh]

https://osgeo-org.atlassian.net/projects/GEOT/issues/GEOT-6114 tracks this issue

[aaime]

@wcmatthysen good fix and good test. What we miss now is a ticket in Jira, and the commit message should refer to it, for tracking purposes, e.g., "[GEOT-XYZW] Title of the ticket here".

[wcmatthysen]

@aaime I opened a ticket in Jira: https://osgeo-org.atlassian.net/projects/GEOT/issues/GEOT-6127?filter=allopenissues

I also changed the commit message to reflect this.

[aaime]

Build failure unrelated, merging. Do you want the fix to be available on 20.x and 19.x too?
If so, could do you backport pull requests?

[wcmatthysen]

Jip. I'm using version 19 at the moment. That is where I initially picked up the bug. So a backport to 19 would be great for me.

I created the pull requests.