Fix garbled glyphs: bounce buffer for AndroidBitmap_lockPixels

Shim_AndroidBitmap_lockPixels handed the guest a NULL pixel pointer
whenever the real buffer lived outside the guest region, on the
documented assumption that "texture decode FROM a Bitmap ... reads
pixels via other means, not by writing through this specific pointer".

Measurement disproved that assumption. The glyph atlas path is exactly
a read through this pointer:

  AndroidBitmap_lockPixels rc=0 hostAddr=0x... -> guest=0x0
  bitmap 1024x1024 stride=4096 format=1   (RGBA_8888)

1024x1024 is precisely the atlas size independently derived from the
glyph UVs (u1 = 35/1024 for a 35px glyph), which is why glyph
positions, sizes, pen advance and UVs all measured correct while the
pixels were coloured noise: BitmapGraphics rendered the glyphs fine,
the guest asked for them, got nothing, and uploaded guest address 0
as the font texture.

Fixed with a bounce buffer - real pixels copied into a guest-visible
buffer on lock, copied back and freed on unlock. The copy-back also
closes the write direction the original comment called a known gap.
Costs one height*stride copy each way, a handful of times per session.

Verified in both directions on the Pixel 6a: text renders correctly,
and the added "no bounce buffer was made" warning fired zero times,
so every lock genuinely got a buffer rather than silently falling
back to the old NULL path.

Co-Authored-By: Claude <noreply@anthropic.com>

mpcore/src/main/cpp/emu/gles_shim.cpp

@@ -183,26 +183,100 @@ uint32_t Shim_AndroidBitmap_getInfo(GuestEngine& eng, uint32_t, uint32_t r1, uin
     if (r2) memcpy(eng.G2H(r2), &info, sizeof(AndroidBitmapInfo));
     return (uint32_t)rc;
 }
+// FIX (2026-09-19, task #41 - garbled glyphs). The version below used to
+// hand the guest a NULL pixel pointer whenever the real buffer lived outside
+// the guest region, on the documented assumption that "texture decode FROM a
+// Bitmap ... reads pixels via other means, not by writing through this
+// specific pointer". Measurement disproved that assumption: the glyph atlas
+// path is exactly a read through this pointer. Live log -
+//   AndroidBitmap_lockPixels rc=0 hostAddr=0x... -> guest=0x0
+//   bitmap 1024x1024 stride=4096 format=1   (RGBA_8888)
+// - 1024x1024 being precisely the atlas size independently derived from the
+// glyph UVs (u1 = 35/1024 for a 35px glyph). So BitmapGraphics rendered the
+// glyphs correctly, the guest asked for the pixels, got nothing, and
+// uploaded whatever was at guest address 0 as the font texture. That is the
+// coloured noise on screen.
+//
+// Fixed with a bounce buffer: the real pixels are copied into a guest-visible
+// buffer on lock, and copied back on unlock (so a guest that WRITES pixels -
+// the case the original comment worried about - works too). Costs one
+// height*stride copy each way, but this fires a handful of times per session,
+// not per frame.
+namespace {
+struct BouncedBitmap {
+    GuestAddr guestBuf = 0;
+    uint32_t bytes = 0;
+    void* hostPixels = nullptr;
+};
+std::mutex g_bouncedBitmapsMutex;
+std::unordered_map<uint32_t, BouncedBitmap> g_bouncedBitmaps; // keyed by the guest bitmap handle
+} // namespace
+
 uint32_t Shim_AndroidBitmap_lockPixels(GuestEngine& eng, uint32_t, uint32_t r1, uint32_t r2, uint32_t, uint32_t) {
     jobject bitmap = (jobject)JniShim::Instance().handles().Resolve(r1);
     void* addr = nullptr;
     int rc = AndroidBitmap_lockPixels(JniShim::Instance().RealEnv(), bitmap, &addr);
+
+    // Preferred path: give the guest a real, readable copy.
+    if (rc == 0 && addr && r2 && !eng.IsHostPointerInRegion(addr)) {
+        AndroidBitmapInfo info{};
+        if (AndroidBitmap_getInfo(JniShim::Instance().RealEnv(), bitmap, &info) == 0 &&
+            info.stride && info.height) {
+            uint32_t bytes = info.stride * info.height;
+            GuestAddr buf = eng.heap().Alloc(bytes);
+            if (buf) {
+                memcpy(eng.G2H(buf), addr, bytes);
+                memcpy(eng.G2H(r2), &buf, 4);
+                {
+                    std::lock_guard<std::mutex> lock(g_bouncedBitmapsMutex);
+                    g_bouncedBitmaps[r1] = BouncedBitmap{buf, bytes, addr};
+                }
+                return (uint32_t)rc;
+            }
+            // Allocation failed - fall through to the old NULL behaviour
+            // rather than handing out a host pointer the guest would
+            // dereference into its own region.
+            Log("gles_shim: AndroidBitmap_lockPixels: could not allocate a %u-byte guest bounce "
+                "buffer for a %ux%u bitmap - guest will see NULL pixels", bytes, info.width,
+                info.height);
+        }
+    }
+
     if (r2) {
-        // Real pixel buffer lives in host (driver-owned) memory, not our
-        // guest region - H2G on it would silently produce a garbage/
-        // wraparound "guest address" (see IsHostPointerInRegion's own
-        // comment), so explicitly return NULL instead of a dangerous
-        // almost-valid-looking pointer. Guest code that wants to directly
-        // WRITE pixel data through this pointer can't do so under this
-        // design (documented gap, low priority - texture decode FROM a
-        // Bitmap is the main real use, which reads pixels via other means,
-        // not by writing through this specific pointer).
+        // Remaining cases: the lock itself failed, or the buffer already lives
+        // inside the guest region (then H2G is exact and no bounce is needed).
+        // A host pointer outside the region is never handed over as-is - H2G
+        // on it would silently produce a garbage/wraparound "guest address"
+        // (see IsHostPointerInRegion's own comment), so NULL is the honest
+        // answer.
         uint32_t g = (addr && eng.IsHostPointerInRegion(addr)) ? eng.H2G(addr) : 0;
         memcpy(eng.G2H(r2), &g, 4);
+        if (rc == 0 && !g) {
+            Log("gles_shim: AndroidBitmap_lockPixels succeeded (hostAddr=%p) but the guest gets "
+                "NULL - no bounce buffer was made, pixels read through this pointer will be "
+                "garbage", addr);
+        }
     }
     return (uint32_t)rc;
 }
-uint32_t Shim_AndroidBitmap_unlockPixels(GuestEngine&, uint32_t, uint32_t r1, uint32_t, uint32_t, uint32_t) {
+
+uint32_t Shim_AndroidBitmap_unlockPixels(GuestEngine& eng, uint32_t, uint32_t r1, uint32_t, uint32_t, uint32_t) {
+    // Flush a bounce buffer back into the real bitmap before unlocking, so
+    // guest-side writes (the direction the old NULL-returning code could not
+    // support at all) land where Java will see them.
+    BouncedBitmap bounced{};
+    {
+        std::lock_guard<std::mutex> lock(g_bouncedBitmapsMutex);
+        auto it = g_bouncedBitmaps.find(r1);
+        if (it != g_bouncedBitmaps.end()) {
+            bounced = it->second;
+            g_bouncedBitmaps.erase(it);
+        }
+    }
+    if (bounced.guestBuf) {
+        memcpy(bounced.hostPixels, eng.G2H(bounced.guestBuf), bounced.bytes);
+        eng.heap().Free(bounced.guestBuf);
+    }
     jobject bitmap = (jobject)JniShim::Instance().handles().Resolve(r1);
     return (uint32_t)AndroidBitmap_unlockPixels(JniShim::Instance().RealEnv(), bitmap);
 }