[thomaskistler]

Fixes: N/A (feature; no linked issue).

Summary of changes

This extends the Lua LVGL image API to accept existing Bitmap objects. Widgets can then load, retain and release decoded images themselves, rather than relying entirely on LVGL's filename cache.

The API remains lvgl.image([parent, ]params), with a new bitmap source in params:

local bitmap = Bitmap.open("/IMAGES/tile.jpg")
image = lvgl.image({ bitmap = bitmap, x = x, y = y, w = w, h = h })
-- Or attach the image to an explicit parent:
image = lvgl.image(parent, { bitmap = bitmap, x = x, y = y, w = w, h = h })

image:set({ bitmap = bitmap })
image:set({ bitmap = false }) -- Clear the source and release the image's reference.

The existing file source remains supported; specify either file or bitmap, not both.

Motivation

While I was adding a map panel to the AeroGrid widget, I ran into performance problems with map tiles related to how LVGL manages its image cache. A map view made up of multiple tiles can easily exceed the default eight-entry cache when panning and zooming. This can cause the same JPEGs to be decoded repeatedly, even after all the tiles needed for the current view have already been loaded.

To get better performance, we need a way to manage the decoded tile cache from the widget. For example, as we zoom in and out, tiles from the old zoom level should be released, newly needed tiles loaded, and useful tiles kept around. Increasing the shared cache size alone doesn't give the widget that control.

LVGL 9.2, for example, lets applications size the image cache in bytes and choose whether resizing immediately evicts entries with lv_image_cache_resize(new_size, evict_now). EdgeTX currently uses LVGL 8.4.1, and I don't know of a planned upgrade to those APIs. This provides a way to manage image lifetimes with EdgeTX's existing LVGL version, without making that upgrade a prerequisite.

Details

• lvgl.image({bitmap=b}) and image:set({bitmap=b}) display an existing Bitmap. Bitmap support is documented as added in 3.0.0, so widgets can detect it with a firmware version check and keep a filename-based fallback for older firmware.
• The image holds a reference to the Bitmap while it uses the pixels. Replacing the source, deleting the image or tearing down the widget releases that reference. image:set({bitmap=false}) clears it immediately, even while hidden. Setting bitmap=nil does not clear it, since nil fields are absent from Lua tables.
• RGB565 pixels are borrowed without copying. ARGB4444 images need conversion to LVGL's RGB565-plus-alpha layout; that allocation counts against the existing Lua bitmap memory budget. Invalid Bitmaps and conversion allocation/budget failures raise a Lua error without replacing the current image.
• Filename strings and callbacks remain supported. Bitmap sources are assigned directly, not through callbacks. Switching between filename and Bitmap sources is handled.
• Behavior change for existing scripts: image:set({file = "..."}) now changes the displayed image and replaces any previous filename callback. Previously the new path was stored but never applied, and an existing callback kept overriding it.

The binding is generic: there is no map-specific API, change to LVGL, larger image cache or increased bitmap memory budget. The API documentation and 13 focused tests are included.

Performance experiments

I used a test widget to load a tiled map, drag it, zoom in and drag again, then zoom out and drag once more. Using the same v2.12.0-based native TX16S simulator build for both modes, the existing file API produced on the order of 100 JPEG decode events during those drag phases. With bitmap sources, those drag-time decodes were completely eliminated; initial tile loading still required decoding.

Validation

Validated on actual hardware using a custom v2.12.0-based build on my TX16S Mk2. Both the standalone Bitmap test and the AeroGrid map were responsive. The main-based revision also passes all 34 targeted native tests, but has not yet been tested on hardware.

[pfeerick]

Thanks for this, Bitmap sources for lvgl.image will be really useful, and nice performance improvement! I asked Claude to review the lifetime and parsing paths and it suggests there are a few things to sort out before merging, mainly around the new LV_EVENT_DELETE handler in LvglWidgetImage. Line numbers below are against the current PR head.

One behaviour change for existing scripts is worth calling out in the PR description: image:set({file = "..."}) now actually changes the displayed image. Previously the new path was stored but never applied, and a function source kept overriding it.

1. windowDeleted frees the image's Lua ref while the parent still lists it

radio/src/lua/lua_lvgl_widget.cpp:2096

windowDeleted calls clearRefs(), which luaL_unrefs luaRef, and then delete self. But the parent box's lvglObjectRefs (or LuaScriptManager::lvglObjectRefs for a top-level image) still holds that integer and is never updated.

Lua 5.3's luaL_ref reuses the most recently freed slot first, so this sequence breaks:

box:clear()                        -- image window deleted -> windowDeleted frees ref R
local lbl = lvgl.label({...})      -- under another parent: luaL_ref hands out R again
-- next callRefs: box takes the clearRequest path -> clearChildRefs
-- -> lua_rawgeti(R) resolves to lbl -> lbl:clearRefs()
lbl:set({text = "x"})              -- "Invalid object (it has been probably been cleared)"

The same applies to native deletions where the parent list isn't cleared (page or dialog close). A top-level lvgl.clear() is fine, because LuaWidget::clear() / StandaloneLuaWindow::clear() clear the refs before the windows are destroyed.

This also makes images the only widget type that becomes invalid immediately on window deletion; every other LvglWidgetObject stays valid until the next clearChildRefs. Since build() registers the handler on every image, this affects existing file-only scripts too, not just the new bitmap path.

Suggestion: have windowDeleted only release the bitmap and drop the window pointer, and leave luaRef, the userdata and the C++ object to the existing clearChildRefs path (it already guards if (window)):

static void windowDeleted(lv_event_t* e)
{
  auto self = (LvglWidgetImage*)lv_event_get_user_data(e);
  self->clearBitmap(self->bitmapState);
  self->window = nullptr;
  self->bitmapState = nullptr;
}

That keeps the "Bitmap is released when the image's window is deleted" guarantee from the docs, while removing the stale-ref problem and keeping image lifetime consistent with the other widgets. Because window is then null until clearChildRefs runs, LvglWidgetImage::refresh() / callRefs() and LvglWidgetObject::isVisible() (lua_lvgl_widget.h:482) need if (!window) guards; otherwise img:set(...) between box:clear() and the next update dereferences null. NativeDeletionReleasesStrongReference would need adjusting if it relies on the handle becoming invalid immediately.

While here, build() (:2133) stores the caller's L in bitmapState, which could be a script-created coroutine (e.g. coroutine.wrap(function() img = lvgl.image({bitmap = b}) end)()). If that coroutine is collected before the window is deleted, windowDeleted would luaL_unref on a freed lua_State. It's an unlikely pattern, but storing the main thread avoids it:

lua_rawgeti(L, LUA_REGISTRYINDEX, LUA_RIDX_MAINTHREAD);
bitmapState = lua_tothread(L, -1);
lua_pop(L, 1);

2. A failed set() can leave the image blank

radio/src/lua/lua_lvgl_widget.cpp:2051

The documented failures (invalid or unloaded Bitmap, allocation or budget failure) are all checked before the old source is cleared, so those keep the previous image as promised. Two other errors don't:

• image:set({file = {}}): clearBitmap(L) runs (deleting the displayed lv_img) before filename.parse(L) hits luaL_checkstring and errors.
• image:set({bitmap = b2, bogus = 1}): lua_next order isn't fixed, so if bitmap is handled first the old image is cleared and bitmapRef swapped, then "Invalid property" aborts before refresh().

In both cases the image is blank until the next successful set(). Before this PR, the file branch had no side effects before the error.

Suggestion: stage the new source in parseParam (validate, take the ref) and do the clearBitmap/clearSource swap in refresh() once parsing has succeeded. A minimal fix for the file case is to check lua_isstring before calling clearBitmap.

3. A failed constructor leaks the bitmap and its memory-budget charge

radio/src/lua/lua_lvgl_widget.cpp:2045

On the constructor path, lvgl.image({bitmap = b, badkey = 1}) errors inside obj->create() before the object is registered or pushed (luaLvglObjEx). At that point bitmapRef is held, the ARGB4444 conversion buffer is allocated and luaExtraMemoryUsage has been charged, and nothing ever releases them. Callback refs already leak this way on a failed constructor, but the budget charge is new: every leak shrinks what's left for Bitmap.open for the rest of the session.

Suggestion: do the conversion/charge only after all params have parsed (same staging as in 2), or release the bitmap on the error path in the constructor.

4. Drop lvgl.IMAGE_BITMAP_SUPPORTED

radio/src/lua/api_colorlcd_lvgl.cpp:622

Scripts should detect this with a firmware version check, so the extra capability constant (and the matching @notice in the docs) isn't needed. Also the @status line needs to be updated. Since it seems it will not significantly change the API for existing scripts, it can be considered for the next 2.12 release.

5. Nit: {file = ..., bitmap = false} is rejected

radio/src/lua/lua_lvgl_widget.cpp:1990

conflicting = !lua_isnil(L, -1) treats bitmap = false as set, so image:set({file = "/IMAGES/a.png", bitmap = false}) errors with "Specify either file or bitmap, not both", even though false means no bitmap. Since set({file = ...}) already clears the bitmap this is minor, but the error is misleading. Treating boolean false as absent in the check would fix it.

[thomaskistler]

@pfeerick thanks for the thorough review! I've pushed seven follow-up commits to this PR. Commits 1–5 address your five points. Commits 6 and 7 fix two further issues found while re-reviewing the lifetime and parsing paths. Each commit includes regression tests. The PR description now calls out the image:set({file=...}) behavior change and no longer mentions lvgl.IMAGE_BITMAP_SUPPORTED.

1. Native window deletion freed the image's Lua handle early (047256d, review point 1)

Issue: The image's LV_EVENT_DELETE handler released the image's registry ref and deleted the wrapper, but the parent's child list still held that ref number. Lua reuses freed registry slots, so after box:clear() a newly created object could get the same slot. The parent's next cleanup then cleared that unrelated object instead. The same happened after native page or dialog close. The handler also stored the caller's lua_State, which could be a coroutine that has since been garbage-collected.

Fix: The delete handler now only releases the Bitmap (and any conversion buffer) and forgets the window. The handle and its registry slot stay valid until the parent's normal cleanup, which then deletes the wrapper exactly once. Image methods, callbacks and visibility checks are guarded while the window is gone. set() on such an image raises "Image window has been deleted". The handler uses the main Lua thread rather than the creating coroutine.

The new deleted-parent check for lvgl.image(parent, ...) and lvgl.build(parent, ...) asks the object whether its window was deleted. Only images track that state, so labels, lines and triangles, which have no window, can still be used as parents and fall back to the script window as before.

2. A failed set() could leave the image blank (16a8e27, review point 2)

Issue: image:set({file = {}}) cleared the displayed Bitmap before the file value was validated. image:set({bitmap = b2, bogus = 1}) could swap the source before the invalid key raised, depending on table iteration order. Either way the image stayed blank until the next successful set().

Fix: The file/bitmap source is parsed after all other parameters, so an invalid key raises before the source is touched. The new source is fully validated first: its type, unloaded or oversized Bitmaps, the pixel format and the memory budget. Its ref and conversion buffer are acquired next, and only then is the old source released and replaced. Any error therefore leaves the current image intact.

file and bitmap are read with raw lookups, so an __index metamethod on the params table can't run Lua during the update.

Other parameters in a rejected set() behave as for every other widget: keys processed before the error stay applied. Only the image source is guaranteed to stay unchanged.

3. Failed constructors leaked the Bitmap and its memory-budget charge (864507a, review point 3)

Issue: lvgl.image({bitmap = b, badkey = 1}) raised after taking the Bitmap ref, allocating the ARGB4444 conversion buffer and charging the Lua bitmap memory budget. Nothing released them, so each failure permanently shrank the budget available to Bitmap.open().

Fix: Image construction runs in a protected call. On failure the image's refs, conversion buffer, budget charge, native window and wrapper are all released. Failures while creating children release the partially built object, and the previous temporary parent is restored.

4. Remove lvgl.IMAGE_BITMAP_SUPPORTED (717342f, review point 4)

Issue: Scripts should detect the feature with a firmware version check, so the extra capability constant and its @notice aren't needed. The @status line also needed updating.

Fix: Removed the constant and its documentation. @status now says Bitmap support was added in 3.0.0, main's current version. If this is backported to 2.12, the backport needs the matching 2.12.x version.

5. {file = ..., bitmap = false} was rejected (13a1c6b, review point 5)

Issue: The conflict check treated bitmap = false as a Bitmap, so image:set({file = "/IMAGES/a.png", bitmap = false}) failed with "Specify either file or bitmap, not both". The result also depended on table iteration order.

Fix: file and bitmap are read once, after the other parameters, so field order doesn't matter. false means "no Bitmap": with file it selects the file, alone it clears the image. A real Bitmap together with file is still rejected.

6. lvgl.clear(image) left a dangling image object (46c307b)

Issue: lvgl.clear(image) cleans the StaticImage window, which deletes its internal LVGL image object, but StaticImage kept the pointer. The next source change, image:set({bitmap=false}) or teardown called lv_obj_del() on freed memory and crashed.

Fix: StaticImage now overrides clear() and resets its image pointer after the window is cleaned. Later source changes create a new image object.

7. A failed child build left the wrong parent active (4ffde23)

Issue: lvgl.build() and the children of page/dialog constructors set a temporary parent while building nested objects but never restored it when a child raised. Objects created later were attached to that parent. With deferred image deletion, that parent could already be a deleted image wrapper.

Fix: Nested objects are built in a protected call, and the previous parent is restored before the error is re-raised.

[pfeerick]

Might be a good idea to ask your AI assistant to do a code-review on the PR (i.e. on it's own work), as Claude is still griping about a few things... I can continue to give you the feedback if you want me to but it would be quicker for you to get it to self-assess and loop through fixing any issues it finds.

Code review feedback

These are the problems PR #7947 introduces or makes worse. The others either already existed on main, are improved by the PR, or aren't bugs.

1. Use-after-free when lvgl.clear() is called from an image callback (blocking)
   How it happens: an image's property callback (visible, color, pos, size, opacity, file, ...) calls lvgl.clear() with no argument. That runs LuaWidget::clear() → LuaScriptManager::clearRefs → LvglWidgetImage::release(), which does clearRefs(L); delete this; straight away. Control then returns into pcallUpdate* and LvglWidgetImage::callRefs, which use the freed object: the update lambda captures this, and the code reads isVisible(), iterates lvglObjectRefs, and touches filename.function/changedText. The standalone path (StandaloneLuaWindow::clear) has the same problem.
   Why it's new: before the PR, clearRefs never deleted anything. The object leaked, and because its window was closed, isVisible() returned false, so callRefs skipped it. This PR is what turns that harmless leak into a crash.
   Fix: defer the delete. LuaScriptManager::clearRefs could queue the released objects, or set a flag, and the queue would be emptied after callRefs returns or at the start of the next tick. Per-object clearRequest already works this way.

2. lvgl.clear(image) leaves the image blank and keeps the bitmap memory counted (low)
   How it happens: StaticImage::clear() deletes the lv_img and nulls image. LvglWidgetImage can't step in because LvglWidgetObjectBase::clear() is non-virtual. It keeps bitmapRef, the ARGB conversion buffer and the charge against luaExtraMemoryUsage. sourceChanged stays false and filename's hash doesn't change, so a constant file callback never calls setSource again and the image stays blank. A bitmap source is never re-applied either.
   What the PR is responsible for: the blank-forever behaviour already existed. The memory left counted is new, because bitmap sources are new in this PR. The PR also fixes an older problem where lv_img dangled after lv_obj_clean.
   Severity: an edge case. lvgl.clear(image) means "delete the image's children", and images normally have none.
   Fix: in LvglWidgetImage::callRefs, when clearRequest is set, force the filename refresh and set sourceChanged = true. Releasing the source there would also work.

3. Replacing an ARGB4444 bitmap counts the old buffer against the limit (minor)
   How it happens: the check at lua_lvgl_widget.cpp:2047 (luaExtraMemoryUsage > LUA_MEM_EXTRA_MAX - size) runs while the current image's bitmapPixelsSize is still counted. Close to the limit, swapping in a bitmap of the same size is rejected even though it would fit once the old one is freed. The workaround is set{bitmap=false} first.
   Context: the code deliberately allocates the new buffer before releasing the old one, and the check matches the real transient peak, so this is arguably a trade-off. Neither ConversionCannotBypassLegacyBudget nor RepeatedVisibleSetsStayWithinExistingBudget tests a same-size replacement.
   Fix, if wanted: compare against luaExtraMemoryUsage - bitmapPixelsSize. That only holds if the old buffer really is freed before the new one goes live; otherwise, document that the limit counts the peak during a swap.

The agent also found two minor issues it attributes to the PR:

• An image that has children stops processing clearRequest and its children's callbacks once its window is deleted, because callRefs returns early when window == nullptr.
• Switching file from a string to a function with set{file=fn} shows a blank image for one tick.

[thomaskistler]

Might be a good idea to ask your AI assistant to do a code-review on the PR (i.e. on it's own work), as Claude is still griping about a few things... I can continue to give you the feedback if you want me to but it would be quicker for you to get it to self-assess and loop through fixing any issues it finds.

Code review feedback

Ok. Ironically, I did review it with Claude Opus 5.5 at max effort. That's how it found issues 6 and 7 above. But I will run it one more time.

[thomaskistler]

@pfeerick I pushed seven more commits that address the five points:

1. lvgl.clear() from an image callback (use-after-free): released image wrappers are no longer deleted immediately. They are freed from the main loop's existing trash, the same place closed windows are freed, so callback code still running can't touch freed memory. Pixels and refs are still released right away, and the image is detached from its window. I also guarded the file callback path against a callback that closes the image's window (e.g. its dialog) (1eb93e5).
2. lvgl.clear(image): clearing an image now removes only its children. Its own source and pixels stay displayed, so the Bitmap memory that stays counted is still in use (de8df18).
3. ARGB4444 swap headroom: I kept this as is on purpose. The new conversion buffer is allocated before the old one is freed, so a failed swap keeps the current image, and the budget check matches that real peak. The docs now say so, and that near the limit set({bitmap=false}) first is the workaround (98e6aec).
4. Deleted image with children: a natively deleted image still runs no callbacks for its children, but it now releases the children of a pending lvgl.clear(image) instead of keeping them referenced (ca94527).
5. file string to function: the old image now stays up until the new callback's first result is applied, so there's no blank tick. Switching from a Bitmap still releases the Bitmap immediately (014da21).

Another review pass found two related edge cases, which are fixed too:

• A __gc finalizer that clears the UI in the middle of image:set() could leave a Bitmap ref and conversion charge behind (086f504).
• A file callback that replaced the image source itself could have its returned path override the new source (a652b53).

All 286 native gtests pass, and an independent repro-based review of the latest head found no further issues. I haven't tested this revision on hardware yet.

[philmoz]

Not keen on overloading the exiting image class this way to handle bitmaps (which could eventually be deprecated and removed).

[thomaskistler]

Not keen on overloading the exiting image class this way to handle bitmaps (which could eventually be deprecated and removed).

@philmoz That’s fair. The goal is to let Lua widgets control image caching to avoid repeated JPEG decoding. In AeroGrid’s map panel, the shared cache can evict tiles that are still visible, making panning very slow.

I used Bitmap because it already provides Lua ownership and memory accounting, with no extra pixel copy for RGB565. That addressed the problem without changing LVGL or increasing the global cache.

I’m not attached to that API or extending the existing image class. An LVGL-native alternative would work too. What approach would you recommend for keeping widgets' active images decoded and releasing them when they’re no longer needed?

[philmoz]

lvgl.image is a wrapper for the StaticImage class.
Why not just add a new 'lvgl.bitmap' object that is a wrapper for the StaticBitmap class?