Reuse Bitmap, Paint & Canvas instances in Android LocalGlyphRasterizer

[themics]

Mapbox SDK versions: 6.3.0-beta1

Since LocalGlyphRasterizer.drawGlyphBitmap() only called in a GL thread, couldn't we store these instances(per GL thread, perhaps ThreadLocal?) and reuse?

In this case of course we should clear the canvas by drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR) before drawing, but even with that, reusing is much more fast.

I tested a bit by rasterizing 1,000 korean characters with my Pixel 2, here's the result:

• Current implementation
  • 1st attempt: 158ms
  • 2nd~ attempt with same characters: 32ms(word layout cached by Android system)
• Reusing implementation
  • 1st: 102ms (1.5x faster)
  • 2nd~: 10ms (3x faster)

[ChrisLoer]

Hi @themics, that sounds like a good suggestion! You're right, drawGlyphBitmap() should only be called from the "renderer" thread. I implemented LocalGlyphRasterizer, but I'm not normally a Java developer -- would you be interested in submitting a PR with your suggested changes?

Also I'm curious, did the glyph rasterization cost show up as a significant expense in part of your profiling? A 1.5x-3x speed-up is great, although I had the assumption that even on maps with lots of CJK characters, the rasterization cost would be a pretty small part of overall map loading time.

[themics]

@ChrisLoer I'll try if you want, but it will takes time because I should rework. My test was kind of proof of concept, so I just stored them in the static field.

public class LocalGlyphRasterizer {
    private static Bitmap bitmap;
    private static Paint paint;
    private static Canvas canvas;

    static {
        bitmap = Bitmap.createBitmap(35, 35, Bitmap.Config.ARGB_8888);

        paint = new Paint();
        paint.setAntiAlias(true);
        paint.setTextSize(24);

        canvas = new Canvas();
        canvas.setBitmap(bitmap);
    }

    @WorkerThread
    public static Bitmap drawGlyphBitmap(String fontFamily, boolean bold, char glyphID) {
        paint.setTypeface(Typeface.create(fontFamily, bold ? Typeface.BOLD : Typeface.NORMAL));
        canvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.CLEAR);
        canvas.drawText(String.valueOf(glyphID), 0, 20, paint);
        return bitmap;
    }
}

As you can see it's not usable when 2 or more MapViews running simultaneously. I think storing them in the non-static field and instantiating LocalGlyphRasterizer per renderer will be proper solution but it means more coding! 😂

About overall loading time, you're right. It's not much significant but still effective especially on the large screen and/or maximum tilt. I measured time between DID_FINISH_LOADING_STYLE ~ DID_FINISH_RENDERING_FRAME_FULLY_RENDERED on Shield tablet K1 with CameraPosition of:

new CameraPosition.Builder().target(new LatLng(37.56738, 126.99655))
      .zoom(13).tilt(60).bearing(90)
      .build()

The result is:

• current implementation: 1425ms
• when reusing: 1313ms

Screenshot:

[ChrisLoer]

Ah I see, yes, it probably most makes sense for the re-used items to be members of LocalGlyphRasterizer, and then drawGlyphBitmap should become a member function instead of a static function. android/src/text/local_glyph_rasterizer.cpp will have to be updated to instantiate the java LocalGlyphRasterizer and hold onto it over the lifetime of LocalGlyphRasterizer::Impl. Are you familiar with JNI? I am no expert on JNI, but I recently used jni::UniqueObject to hold onto a long-lived JNI object in android/src/text/collator.cpp, you could try following that pattern.

Sounds like almost 9% improvement in time to render, or >100ms -- that's not huge, but it definitely sounds worth the effort. I'm really glad to hear you're getting use out of the local glyph generation feature!

If you got really ambitious and wanted to further optimize Android glyph generation, I suspect a good approach would be to batch up the drawing request so that all of the glyphs in a batch were drawn into a single bitmap, in order to minimize the JNI overhead.

[themics]

@ChrisLoer Yup turns out it was simple job, native LocalGlyphRasterizer was already in 1:1 relation with a renderer. I'll send a PR soon. Batching sounds really good solution - but I'm out of ambition for now(haha). Maybe I can take a look next time.