jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.74k stars 1.12k forks source link

Minor BitmapFont code architecture issues #2221

Open tonihele opened 3 months ago

tonihele commented 3 months ago

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. https://github.com/jMonkeyEngine/jmonkeyengine/blob/ea715d80257c7c2c9a17d0e44ccd86176e5a0dc2/jme3-core/src/main/java/com/jme3/font/BitmapTextPage.java#L153 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. https://github.com/jMonkeyEngine/jmonkeyengine/blob/ea715d80257c7c2c9a17d0e44ccd86176e5a0dc2/jme3-core/src/main/java/com/jme3/font/ColorTags.java#L68, 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 commented 3 months ago

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.