Minor BitmapFont code architecture issues

[tonihele]

Minor BitmapFont code architecture issues. Functionally it works just fine :)

1. https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/font/BitmapTextPage.java constructor takes in parameter arrayBased. It never uses it, it is always true. Some of these have been deprecated already in the hierarchy, maybe this one should also be. Now it is just confusing

2. jmonkeyengine/jme3-core/src/main/java/com/jme3/font/BitmapTextPage.java

   Line 153 in ea715d8

   void assemble(Letters quads) {

   is called every frame your text changes. This might be all the time, like with the jME FPS counter. pageQuads is LinkedList, and the said method heavily uses get(int) on it. This is not great, n^2 problem, right?

3. jmonkeyengine/jme3-core/src/main/java/com/jme3/font/ColorTags.java

   Line 68 in ea715d8

   LinkedList<Range> getTags() {

   , same with all the implementations that use this in jME code. Go through the collection using get(int).

LinkedList is probably utilized since its steady add performance and that it doesn't leave potentially big arrays behind (if your text goes from being 2,147,483,647 to 1 character... you'll never recover). At least point 2 could be covered by a for each loop on the LinkedList and having a manual counter for the index as it is needed elsewhere.

[pspeed42]

The bitmap font/text code is a great example of bad code. It does back flips and hand springs to seemingly avoid creating "garbage"... but then ends up creating hidden garbage instead, being overly complicated, and hurting its own performance. It also confounds usability improvements like easily being able to detect 'clicked character' and stuff like that.

Some of the problems are even more fundamental than what you point out. The whole thing is in need of a rewrite. I started one once but it's a big job.

[jaime-jmebot]

Opened #2963 with the first slice of this.

1. The arrayBased parameter that does nothing (BitmapTextPage)
Gone. A new BitmapTextPage(BitmapFont font, int page) constructor always allocates the quad arrays, the two constructors that take the flag are deprecated, and the dead branch that used to leave the arrays null was deleted.

2. assemble() walking the pageQuads LinkedList with get(int)
The per-quad loop now iterates the list once with a for-each loop and keeps a manual quad index, so refreshing the text is O(n) instead of O(n²) — which is the case that matters for something like the FPS counter.

To be sure nothing shifted, I compared the assembled mesh data (positions, colors, indices) of the old and the new code for plain, colour-tagged, multi-line, word-wrapped/aligned and empty texts: the buffers come out byte-for-byte identical. jme3-core still compiles as Java 8 with -Xlint:unchecked -Werror.

Still to do: the third item — the same index-walking pattern on the colour-tag ranges in Letters (used by setText() and setBaseAlpha()) — is not part of that change and remains open.