From 25e7f126036c1b8f027f045dee8aacb2e25354e5 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 15:06:49 -0700 Subject: [PATCH 01/15] feat(lua): support Bitmap sources in LVGL image objects Let widgets manage decoded image lifetimes through direct Bitmap sources while preserving filename support. Retain Bitmap userdata while displayed, support explicit clearing, and account for alpha conversion within the existing bitmap memory budget. Document the API and cover source replacement, clearing, conversion, and lifecycle cleanup with focused tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/gui/colorlcd/libui/bitmapbuffer.h | 1 + radio/src/gui/colorlcd/libui/static.cpp | 9 + radio/src/gui/colorlcd/libui/static.h | 1 + radio/src/lua/api_colorlcd.cpp | 2 +- radio/src/lua/api_colorlcd.h | 5 + radio/src/lua/api_colorlcd_lvgl.cpp | 10 +- radio/src/lua/lua_lvgl_widget.cpp | 114 ++++++- radio/src/lua/lua_lvgl_widget.h | 9 + radio/src/tests/lua_lvgl.cpp | 328 ++++++++++++++++++++ 9 files changed, 474 insertions(+), 5 deletions(-) create mode 100644 radio/src/tests/lua_lvgl.cpp diff --git a/radio/src/gui/colorlcd/libui/bitmapbuffer.h b/radio/src/gui/colorlcd/libui/bitmapbuffer.h index 29d4ee04d23..d8e8de3e203 100644 --- a/radio/src/gui/colorlcd/libui/bitmapbuffer.h +++ b/radio/src/gui/colorlcd/libui/bitmapbuffer.h @@ -145,6 +145,7 @@ class BitmapBuffer inline uint16_t height() const { return _height; } inline pixel_t* getData() const { return data; } + inline uint8_t getFormat() const { return format; } uint32_t getDataSize() const { return _width * _height * sizeof(pixel_t); } diff --git a/radio/src/gui/colorlcd/libui/static.cpp b/radio/src/gui/colorlcd/libui/static.cpp index 1b6d6219e7b..15adc5a915e 100644 --- a/radio/src/gui/colorlcd/libui/static.cpp +++ b/radio/src/gui/colorlcd/libui/static.cpp @@ -244,6 +244,15 @@ void StaticImage::setSource(std::string filename) } } +void StaticImage::setSource(const lv_img_dsc_t* bitmap) +{ + if (!image) image = lv_img_create(lvobj); + lv_obj_set_size(image, LV_SIZE_CONTENT, LV_SIZE_CONTENT); + lv_img_set_src(image, bitmap); + lv_obj_center(image); + setZoom(); +} + void StaticImage::clearSource() { if (image) lv_obj_del(image); diff --git a/radio/src/gui/colorlcd/libui/static.h b/radio/src/gui/colorlcd/libui/static.h index 6627ca9193c..22f9ff6d37e 100644 --- a/radio/src/gui/colorlcd/libui/static.h +++ b/radio/src/gui/colorlcd/libui/static.h @@ -154,6 +154,7 @@ class StaticImage : public Window void setZoom(); void setSource(std::string filename); + void setSource(const lv_img_dsc_t* bitmap); void clearSource(); bool hasImage() const; diff --git a/radio/src/lua/api_colorlcd.cpp b/radio/src/lua/api_colorlcd.cpp index 2c6451c029e..b982f7617a2 100644 --- a/radio/src/lua/api_colorlcd.cpp +++ b/radio/src/lua/api_colorlcd.cpp @@ -540,7 +540,7 @@ static int luaOpenBitmap(lua_State *L) return 1; } -static BitmapBuffer * checkBitmap(lua_State * L, int index) +BitmapBuffer * checkBitmap(lua_State * L, int index) { BitmapBuffer ** b = (BitmapBuffer **)luaL_checkudata(L, index, BITMAP_METATABLE); return *b; diff --git a/radio/src/lua/api_colorlcd.h b/radio/src/lua/api_colorlcd.h index 8445eb86bdf..bc59dc4486b 100644 --- a/radio/src/lua/api_colorlcd.h +++ b/radio/src/lua/api_colorlcd.h @@ -24,5 +24,10 @@ #include "definitions.h" #include "lua_states.h" +#ifdef __cplusplus +class BitmapBuffer; +BitmapBuffer* checkBitmap(lua_State* L, int index); +#endif + EXTERN_C(LUALIB_API int luaopen_bitmap(lua_State * L)); EXTERN_C(LUALIB_API int luaopen_lvgl(lua_State * L)); diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 702aa88e28d..c3a5ff9085b 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -619,6 +619,9 @@ static int luaLvglGetPerf(lua_State *L) extern "C" { // lvgl functions LROT_BEGIN(lvgllib, NULL, 0) + // Capabilities + LROT_NUMENTRY(IMAGE_BITMAP_SUPPORTED, 1) + LROT_FUNCENTRY(clear, luaLvglClear) LROT_FUNCENTRY(build, luaLvglBuild) LROT_FUNCENTRY(isAppMode, luaLvglIsAppMode) @@ -823,12 +826,17 @@ LROT_BEGIN(lvgllib, NULL, 0) @param params (table): @commonparams lvgl_object - * `file` (string) full path to the image file on the SD card, defaults to an empty string + * `file` (string or function) full path to the image file on the SD card, or a callback returning that path; mutually exclusive with `bitmap`, defaults to an empty string + * `bitmap` (Bitmap or false) existing Bitmap object assigned directly, not a callback; false clears the source and releases the image's Bitmap reference, defaults to nil * `fill` (boolean) if true the image is scaled to completely fill the frame (may be cropped); if false it is scaled to fit entirely within the frame (may have empty borders), defaults to false @retval table LVGL object @notice `w` and `h` are required. + @notice Detect Bitmap support with `lvgl.IMAGE_BITMAP_SUPPORTED == 1`. + @notice The image keeps its Bitmap alive until replaced, cleared, or deleted. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. + @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. + @notice Invalid/unloaded Bitmaps and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. @status current Introduced in 2.11.0, floating added in 2.11.6 */ diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 68f779897d3..04227f20a59 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -19,14 +19,14 @@ * GNU General Public License for more details. */ -#include "lua_widget.h" - +#include "api_colorlcd.h" #include "color_picker.h" #include "dialog.h" #include "edgetx.h" #include "filechoice.h" #include "keyboard_base.h" #include "lua_event.h" +#include "lua_widget.h" #include "menu.h" #include "numberedit.h" #include "page.h" @@ -1986,8 +1986,72 @@ void LvglWidgetArc::build(lua_State *L) void LvglWidgetImage::parseParam(lua_State *L, const char *key) { - if (!strcmp(key, "file")) { + if (!strcmp(key, "bitmap") || !strcmp(key, "file")) { + lua_getfield(L, -3, !strcmp(key, "bitmap") ? "file" : "bitmap"); + bool conflicting = !lua_isnil(L, -1); + lua_pop(L, 1); + if (conflicting) luaL_error(L, "Specify either file or bitmap, not both"); + sourceChanged = true; + } + if (!strcmp(key, "bitmap")) { + if (lua_isboolean(L, -1) && !lua_toboolean(L, -1)) { + clearBitmap(L); + filename.clearRef(L); + filename.txt.clear(); + return; + } + + auto bitmap = checkBitmap(L, -1); + if (!bitmap || !bitmap->getData() || !bitmap->width() || !bitmap->height()) + luaL_error(L, "Cannot display an unloaded Bitmap"); + if (bitmap->width() > 2047 || bitmap->height() > 2047) + luaL_error(L, "Bitmap dimensions exceed the LVGL image header limit"); + + uint8_t* pixels = nullptr; + uint32_t size = 0; + if (bitmap->getFormat() == BMP_ARGB4444) { + size = bitmap->width() * bitmap->height() * 3u; + if (size > LUA_MEM_EXTRA_MAX || + luaExtraMemoryUsage > LUA_MEM_EXTRA_MAX - size) + luaL_error(L, + "Bitmap image conversion exceeds Lua bitmap memory limit"); + pixels = (uint8_t*)malloc(size); + if (!pixels) luaL_error(L, "Cannot allocate Bitmap image conversion"); + // LVGL needs RGB565 + alpha, not the legacy packed ARGB4444 layout. + const auto src = bitmap->getData(); + for (uint32_t i = 0; i < bitmap->width() * bitmap->height(); ++i) { + auto c = src[i]; + auto r = ((c >> 8) & 15) * 17; + auto g = ((c >> 4) & 15) * 17; + auto b = (c & 15) * 17; + uint16_t rgb = RGB(r, g, b); + pixels[i * 3] = rgb & 255; + pixels[i * 3 + 1] = rgb >> 8; + pixels[i * 3 + 2] = (c >> 12) * 17; + } + } else if (bitmap->getFormat() != BMP_RGB565) { + luaL_error(L, "Unsupported Bitmap pixel format"); + } + + clearBitmap(L); + filename.clearRef(L); + filename.txt.clear(); + lua_pushvalue(L, -1); + bitmapRef = ::getRef(L, LUA_REGISTRYINDEX); + bitmapPixels = pixels; + bitmapPixelsSize = size; + luaExtraMemoryUsage += size; + bitmapDescriptor.header.cf = + pixels ? LV_IMG_CF_TRUE_COLOR_ALPHA : LV_IMG_CF_TRUE_COLOR; + bitmapDescriptor.header.w = bitmap->width(); + bitmapDescriptor.header.h = bitmap->height(); + bitmapDescriptor.data_size = pixels ? size : bitmap->getDataSize(); + bitmapDescriptor.data = pixels ? pixels : (const uint8_t*)bitmap->getData(); + } else if (!strcmp(key, "file")) { + clearBitmap(L); + filename.clearRef(L); filename.parse(L); + filename.forceUpdate(); } else if (!strcmp(key, "fill")) { fillFrame = getLuaBool(L); } else { @@ -2019,14 +2083,58 @@ bool LvglWidgetImage::callRefs(lua_State *L) void LvglWidgetImage::clearRefs(lua_State *L) { + clearBitmap(L); + if (window) { + lv_obj_remove_event_cb_with_user_data(window->getLvObj(), windowDeleted, + this); + } + bitmapState = nullptr; filename.clearRef(L); LvglWidgetObject::clearRefs(L); } +void LvglWidgetImage::windowDeleted(lv_event_t* event) +{ + auto self = (LvglWidgetImage*)lv_event_get_user_data(event); + self->window = nullptr; + self->clearRefs(self->bitmapState); + delete self; +} + +void LvglWidgetImage::clearBitmap(lua_State* L) +{ + if (bitmapRef != LUA_REFNIL) { + if (window) ((StaticImage*)window)->clearSource(); + lv_img_cache_invalidate_src(&bitmapDescriptor); + clearRef(L, bitmapRef); + } + free(bitmapPixels); + bitmapPixels = nullptr; + luaExtraMemoryUsage -= bitmapPixelsSize; + bitmapPixelsSize = 0; + bitmapDescriptor = {}; +} + +void LvglWidgetImage::refresh() +{ + LvglWidgetObject::refresh(); + if (!sourceChanged) return; + sourceChanged = false; + if (bitmapRef != LUA_REFNIL) + ((StaticImage*)window)->setSource(&bitmapDescriptor); + else + ((StaticImage*)window)->setSource(filename.chars()); +} + void LvglWidgetImage::build(lua_State *L) { window = new StaticImage(lvglManager->getCurrentParent(), {x, y, w, h}, filename.chars(), fillFrame); + bitmapState = L; + lv_obj_add_event_cb(window->getLvObj(), windowDeleted, LV_EVENT_DELETE, this); + if (bitmapRef != LUA_REFNIL) + ((StaticImage*)window)->setSource(&bitmapDescriptor); + sourceChanged = false; setFloating(floating); } diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index a9951b9234f..c57ba2f3918 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -675,9 +675,18 @@ class LvglWidgetImage : public LvglWidgetObject protected: LvglParamFuncOrString filename = { .function = LUA_REFNIL, .txt = ""}; bool fillFrame = false; + int bitmapRef = LUA_REFNIL; + lv_img_dsc_t bitmapDescriptor = {}; + uint8_t* bitmapPixels = nullptr; + uint32_t bitmapPixelsSize = 0; + lua_State* bitmapState = nullptr; + bool sourceChanged = false; void build(lua_State *L) override; void parseParam(lua_State *L, const char *key) override; + void refresh() override; + void clearBitmap(lua_State* L); + static void windowDeleted(lv_event_t* event); }; //----------------------------------------------------------------------------- diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp new file mode 100644 index 00000000000..501dcc064a1 --- /dev/null +++ b/radio/src/tests/lua_lvgl.cpp @@ -0,0 +1,328 @@ +/* + * Copyright (C) EdgeTX + * + * Based on code named + * opentx - https://github.com/opentx/opentx + * th9x - http://code.google.com/p/th9x + * er9x - http://code.google.com/p/er9x + * gruvin9x - http://code.google.com/p/gruvin9x + * + * License GPLv2: http://www.gnu.org/licenses/gpl-2.0.html + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "gtests.h" + +#if defined(LUA) && defined(COLORLCD) + +#include "edgetx.h" +#include "lua/api_colorlcd.h" +#include "lua/lua_widget.h" +#include "mainwindow.h" + +extern void lv_stb_init(); + +namespace +{ + +class ImageScriptManager : public LuaScriptManager +{ + public: + Window* root = nullptr; + Window* getCurrentParent() const override + { + return tempParent ? tempParent->getWindow() : root; + } + void clear() override { root->clear(); } + bool useLvglLayout() const override { return true; } + bool isAppMode() const override { return false; } + bool isWidget() override { return true; } + bool isFullscreen() override { return true; } + void luaShowError() override { ADD_FAILURE() << "Lua LVGL callback failed"; } +}; + +class LuaBitmapImage : public ::testing::Test +{ + protected: + lua_State* L = nullptr; + LuaScriptManager* previousManager = nullptr; + ImageScriptManager manager; + uint32_t initialUsage = 0; + + static void SetUpTestSuite() { lv_stb_init(); } + + void SetUp() override + { + initialUsage = luaExtraMemoryUsage; + L = luaL_newstate(); + ASSERT_NE(L, nullptr); + luaRegisterLibraries(L); + manager.root = new Window(nullptr, {0, 0, 100, 100}); + previousManager = luaScriptManager; + luaScriptManager = &manager; + } + + void TearDown() override + { + manager.clearRefs(L); + manager.root->closeWindow(); + MainWindow::instance()->run(); + lua_close(L); + luaScriptManager = previousManager; + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + } + + ::testing::AssertionResult exec(const char* code) + { + if (luaL_dostring(L, code)) + return ::testing::AssertionFailure() << lua_tostring(L, -1); + return ::testing::AssertionSuccess(); + } + + BitmapBuffer* addBitmap(const char* name, uint8_t format = BMP_RGB565, + uint16_t w = 2, uint16_t h = 1) + { + auto slot = (BitmapBuffer**)lua_newuserdata(L, sizeof(BitmapBuffer*)); + auto bitmap = *slot = new BitmapBuffer(format, w, h); + bitmap->getData()[0] = format == BMP_RGB565 ? RGB(255, 0, 0) : 0x8f00; + bitmap->getData()[1] = format == BMP_RGB565 ? RGB(0, 255, 0) : 0xf0f0; + luaExtraMemoryUsage += bitmap->getDataSize(); + luaL_getmetatable(L, "BITMAP*"); + lua_setmetatable(L, -2); + lua_setglobal(L, name); + return bitmap; + } + + LvglWidgetObjectBase* image() + { + lua_getglobal(L, "image"); + auto obj = LvglWidgetObjectBase::checkLvgl(L, -1, true); + lua_pop(L, 1); + return obj; + } + + const lv_img_dsc_t* source() + { + auto child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); + return (const lv_img_dsc_t*)lv_img_get_src(child); + } +}; + +TEST_F(LuaBitmapImage, BorrowsRgb565AndKeepsUserdataAlive) +{ + auto bitmap = addBitmap("bitmap"); + ASSERT_TRUE( + exec("assert(lvgl.IMAGE_BITMAP_SUPPORTED == 1);" + "image = lvgl.image({bitmap=bitmap, w=2, h=1});" + "weak = setmetatable({bitmap}, {__mode='v'});" + "bitmap = nil; collectgarbage(); assert(weak[1])")); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + EXPECT_EQ(source()->header.cf, LV_IMG_CF_TRUE_COLOR); + EXPECT_EQ(source()->header.w, 2); + EXPECT_EQ(source()->header.h, 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE( + exec("image:set({bitmap=false}); collectgarbage();" + "assert(weak[1] == nil)")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0); +} + +TEST_F(LuaBitmapImage, ReplacesAndClearsImmediately) +{ + addBitmap("first"); + auto second = addBitmap("second"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=first, w=2, h=1});" + "first = nil; image:set({bitmap=second}); collectgarbage()")); + EXPECT_EQ(source()->data, (const uint8_t*)second->getData()); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE(exec("second = nil; image:set({bitmap=nil}); collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE(exec("image:set({bitmap=false}); collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, UsesBitmapOpenResultWithoutCopying) +{ + ASSERT_TRUE( + exec("bitmap=Bitmap.open('images/color/bitmap_480x272.png');" + "local w,h=Bitmap.getSize(bitmap); assert(w==480 and h==272);" + "image=lvgl.image({bitmap=bitmap,w=480,h=272})")); + lua_getglobal(L, "bitmap"); + auto bitmap = checkBitmap(L, -1); + lua_pop(L, 1); + ASSERT_NE(bitmap, nullptr); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 480 * 272 * 2); +} + +TEST_F(LuaBitmapImage, ConvertsArgb4444AndAccountsForPixels) +{ + auto bitmap = addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image = lvgl.image({bitmap=bitmap, w=2, h=1})")); + const auto descriptor = source(); + EXPECT_EQ(descriptor->header.cf, LV_IMG_CF_TRUE_COLOR_ALPHA); + EXPECT_EQ(descriptor->data_size, 6); + const uint8_t expected[] = {0x00, 0xf8, 0x88, 0xe0, 0x07, 0xff}; + EXPECT_EQ(memcmp(descriptor->data, expected, sizeof(expected)), 0); + EXPECT_EQ(bitmap->getData()[0], 0x8f00); + EXPECT_EQ(bitmap->getData()[1], 0xf0f0); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 10); + ASSERT_TRUE(exec("image:set({bitmap=false})")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); +} + +TEST_F(LuaBitmapImage, RejectsInvalidSourcesWithoutReplacingImage) +{ + auto bitmap = addBitmap("bitmap"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1});" + "failed = Bitmap.open('/missing-image.png');" + "local w,h = Bitmap.getSize(failed); assert(w==0 and h==0);" + "assert(not pcall(image.set, image, {bitmap=failed}));" + "assert(not pcall(image.set, image, {bitmap=function() return " + "bitmap end}));" + "assert(not pcall(image.set, image, {bitmap=true}));" + "assert(not pcall(image.set, image, {file='', bitmap=bitmap}))")); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + addBitmap("oversized", BMP_RGB565, 2048, 1); + ASSERT_TRUE(exec("assert(not pcall(image.set, image, {bitmap=oversized}))")); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); +} + +TEST_F(LuaBitmapImage, ConversionCannotBypassLegacyBudget) +{ + auto bitmap = addBitmap("bitmap"); + addBitmap("alpha", BMP_ARGB4444); + ASSERT_TRUE(exec("image = lvgl.image({bitmap=bitmap, w=2, h=1})")); + auto actualUsage = luaExtraMemoryUsage; + luaExtraMemoryUsage = LUA_MEM_EXTRA_MAX; + EXPECT_TRUE( + exec("local ok,err = pcall(image.set, image, {bitmap=alpha});" + "assert(not ok and string.find(err, 'memory limit'))")); + luaExtraMemoryUsage = actualUsage; + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); +} + +TEST_F(LuaBitmapImage, NativeDeletionReleasesStrongReference) +{ + addBitmap("bitmap"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1}); bitmap=nil")); + image()->getWindow()->closeWindow(); + ASSERT_TRUE( + exec("collectgarbage();" + "assert(not pcall(image.set, image, {bitmap=false}))")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + manager.clearRefs(L); + ASSERT_TRUE(exec("image=nil; collectgarbage()")); +} + +TEST_F(LuaBitmapImage, WidgetTeardownReleasesStrongReference) +{ + addBitmap("bitmap"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1}); bitmap=nil")); + manager.clearRefs(L); + ASSERT_TRUE(exec("collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, HiddenImagesCanBeCleared) +{ + addBitmap("bitmap"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1});" + "image:hide(); bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE(exec("image:set({bitmap=false}); collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, RepeatedVisibleSetsStayWithinExistingBudget) +{ + ASSERT_TRUE( + exec("images={}; for i=1,9 do " + "images[i]=lvgl.image({bitmap=false,w=256,h=256}) end")); + for (int set = 0; set < 20; ++set) { + for (int i = 1; i <= 9; ++i) { + addBitmap("bitmap", BMP_RGB565, 256, 256); + lua_pushinteger(L, i); + lua_setglobal(L, "index"); + ASSERT_TRUE(exec("images[index]:set({bitmap=bitmap}); bitmap=nil")); + } + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 9 * 256 * 256 * 2); + EXPECT_LE(luaExtraMemoryUsage, LUA_MEM_EXTRA_MAX); + ASSERT_TRUE( + exec("for i=1,9 do images[i]:set({bitmap=false}) end;" + "collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + } +} + +TEST_F(LuaBitmapImage, FileSourcesStillReplaceBitmapSources) +{ + addBitmap("bitmap"); + ASSERT_TRUE( + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1}); bitmap=nil;" + "image:set({file='images/color/edgetx.png'}); collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + auto child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); + ASSERT_NE(child, nullptr); + EXPECT_STREQ((const char*)lv_img_get_src(child), "A/images/color/edgetx.png"); + ASSERT_TRUE( + exec("image:set({file=function() return " + "'images/color/bitmap_320x240.png' end})")); + EXPECT_TRUE(manager.callRefs(L)); + child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); + ASSERT_NE(child, nullptr); + EXPECT_STREQ((const char*)lv_img_get_src(child), + "A/images/color/bitmap_320x240.png"); + addBitmap("bitmap"); + ASSERT_TRUE( + exec("image:set({bitmap=bitmap});" + "image:set({file=function() return " + "'images/color/bitmap_320x240.png' end})")); + EXPECT_TRUE(manager.callRefs(L)); + child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); + ASSERT_NE(child, nullptr); + EXPECT_STREQ((const char*)lv_img_get_src(child), + "A/images/color/bitmap_320x240.png"); +} + +TEST_F(LuaBitmapImage, FilenameSourcesCanBeReplacedAndCleared) +{ + ASSERT_TRUE( + exec("image=lvgl.image({file='images/color/edgetx.png',w=2,h=1});" + "image:set({file='images/color/bitmap_320x240.png'})")); + auto child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); + ASSERT_NE(child, nullptr); + EXPECT_STREQ((const char*)lv_img_get_src(child), + "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("image:set({file=''})")); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0); +} + +TEST_F(LuaBitmapImage, AlphaConversionIsReleasedOnNativeDeletion) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=2,h=1}); bitmap=nil")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 10); + image()->getWindow()->closeWindow(); + ASSERT_TRUE(exec("collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + manager.clearRefs(L); +} + +} // namespace + +#endif From 047256d0dc15cfea784ac1e61cc3b98ad7e447a0 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:10:29 -0700 Subject: [PATCH 02/15] fix(lua): preserve image ownership after native deletion Address review point 1. Release native Bitmap resources immediately but keep the parent-owned registry slot and handle until deferred cleanup. Delete image wrappers exactly once, guard deleted windows, and retain the main Lua thread. Reject only explicit parents whose image window was deleted, so windowless objects such as labels still fall back to the script window. Include lifecycle, ref-slot reuse, coroutine and windowless-parent regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 4 + radio/src/lua/lua_lvgl_widget.cpp | 19 ++- radio/src/lua/lua_lvgl_widget.h | 20 +++- radio/src/lua/lua_widget.cpp | 2 +- radio/src/tests/lua_lvgl.cpp | 179 +++++++++++++++++++++++++++- 5 files changed, 210 insertions(+), 14 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index c3a5ff9085b..4e696a19f12 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -427,6 +427,8 @@ static int luaLvglObjEx(lua_State *L, std::function cre LvglWidgetObjectBase* prevParent = nullptr; if (lua_gettop(L) == 2) { p = LvglWidgetObjectBase::checkLvgl(L, 1, true); + if (p && p->hasDeletedWindow()) + return luaL_error(L, "Parent window has been deleted"); if (p) { prevParent = luaScriptManager->getTempParent(); luaScriptManager->setTempParent(p); @@ -478,6 +480,8 @@ static int luaLvglBuild(lua_State *L) LvglWidgetObjectBase* prevParent = nullptr; if (lua_gettop(L) == 2) { p = LvglWidgetObjectBase::checkLvgl(L, 1, true); + if (p && p->hasDeletedWindow()) + return luaL_error(L, "Parent window has been deleted"); if (p) { prevParent = luaScriptManager->getTempParent(); luaScriptManager->setTempParent(p); diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 04227f20a59..41efaea61d5 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -720,7 +720,7 @@ void LvglWidgetObjectBase::clearChildRefs(lua_State *L) lua_rawgeti(L, LUA_REGISTRYINDEX, lvglObjectRefs[i]); auto p = LvglWidgetObjectBase::checkLvgl(L, -1); lua_pop(L, 1); - if (p) p->clearRefs(L); + if (p) p->release(L); } lvglObjectRefs.clear(); } @@ -1984,6 +1984,12 @@ void LvglWidgetArc::build(lua_State *L) //----------------------------------------------------------------------------- +void LvglWidgetImage::getParams(lua_State* L, int index) +{ + if (hasDeletedWindow()) luaL_error(L, "Image window has been deleted"); + LvglWidgetObjectBase::getParams(L, index); +} + void LvglWidgetImage::parseParam(lua_State *L, const char *key) { if (!strcmp(key, "bitmap") || !strcmp(key, "file")) { @@ -2061,6 +2067,7 @@ void LvglWidgetImage::parseParam(lua_State *L, const char *key) bool LvglWidgetImage::callRefs(lua_State *L) { + if (!window) return true; if (!LvglWidgetObject::callRefs(L)) return false; if (isVisible()) { @@ -2088,7 +2095,6 @@ void LvglWidgetImage::clearRefs(lua_State *L) lv_obj_remove_event_cb_with_user_data(window->getLvObj(), windowDeleted, this); } - bitmapState = nullptr; filename.clearRef(L); LvglWidgetObject::clearRefs(L); } @@ -2097,8 +2103,8 @@ void LvglWidgetImage::windowDeleted(lv_event_t* event) { auto self = (LvglWidgetImage*)lv_event_get_user_data(event); self->window = nullptr; - self->clearRefs(self->bitmapState); - delete self; + self->clearBitmap(self->bitmapState); + self->bitmapState = nullptr; } void LvglWidgetImage::clearBitmap(lua_State* L) @@ -2117,6 +2123,7 @@ void LvglWidgetImage::clearBitmap(lua_State* L) void LvglWidgetImage::refresh() { + if (!window) return; LvglWidgetObject::refresh(); if (!sourceChanged) return; sourceChanged = false; @@ -2130,7 +2137,9 @@ void LvglWidgetImage::build(lua_State *L) { window = new StaticImage(lvglManager->getCurrentParent(), {x, y, w, h}, filename.chars(), fillFrame); - bitmapState = L; + lua_rawgeti(L, LUA_REGISTRYINDEX, LUA_RIDX_MAINTHREAD); + bitmapState = lua_tothread(L, -1); + lua_pop(L, 1); lv_obj_add_event_cb(window->getLvObj(), windowDeleted, LV_EVENT_DELETE, this); if (bitmapRef != LUA_REFNIL) ((StaticImage*)window)->setSource(&bitmapDescriptor); diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index c57ba2f3918..13708e66c28 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -248,6 +248,7 @@ class LvglWidgetObjectBase virtual bool callRefs(lua_State *L); virtual void clearRefs(lua_State *L); + virtual void release(lua_State *L) { clearRefs(L); } void clearChildRefs(lua_State *L); virtual void show() = 0; @@ -270,6 +271,7 @@ class LvglWidgetObjectBase void update(lua_State *L); virtual Window *getWindow() const = 0; + virtual bool hasDeletedWindow() const { return false; } static LvglWidgetObjectBase *checkLvgl(lua_State *L, int index, bool required = false); @@ -294,7 +296,7 @@ class LvglWidgetObjectBase virtual void parseParam(lua_State *L, const char *key); - void getParams(lua_State *L, int index); + virtual void getParams(lua_State *L, int index); void pcallSimpleFunc(lua_State *L, int funcRef); bool pcallUpdateBool(lua_State *L, int getFuncRef, @@ -477,11 +479,11 @@ class LvglWidgetObject : public LvglWidgetObjectBase public: LvglWidgetObject(const char* meta = LVGL_METATABLE) : LvglWidgetObjectBase(meta) {} - void show() override { window->show(); } - void hide() override { window->hide(); } - bool isVisible() override { return window->isVisible(); } - void enable() override { window->enable(); } - void disable() override { window->disable(); } + void show() override { if (window) window->show(); } + void hide() override { if (window) window->hide(); } + bool isVisible() override { return window && window->isVisible(); } + void enable() override { if (window) window->enable(); } + void disable() override { if (window) window->disable(); } void setPos(coord_t x, coord_t y) override; void setSize(coord_t w, coord_t h) override; @@ -671,6 +673,11 @@ class LvglWidgetImage : public LvglWidgetObject bool callRefs(lua_State *L) override; void clearRefs(lua_State *L) override; + void release(lua_State *L) override { clearRefs(L); delete this; } + bool hasDeletedWindow() const override + { + return !window && luaRef != LUA_REFNIL; + } protected: LvglParamFuncOrString filename = { .function = LUA_REFNIL, .txt = ""}; @@ -683,6 +690,7 @@ class LvglWidgetImage : public LvglWidgetObject bool sourceChanged = false; void build(lua_State *L) override; + void getParams(lua_State *L, int index) override; void parseParam(lua_State *L, const char *key) override; void refresh() override; void clearBitmap(lua_State* L); diff --git a/radio/src/lua/lua_widget.cpp b/radio/src/lua/lua_widget.cpp index 2b2c0b301c5..ef21c545ffa 100644 --- a/radio/src/lua/lua_widget.cpp +++ b/radio/src/lua/lua_widget.cpp @@ -589,7 +589,7 @@ void LuaScriptManager::clearRefs(lua_State *L) lua_rawgeti(L, LUA_REGISTRYINDEX, lvglObjectRefs[i]); auto p = LvglWidgetObjectBase::checkLvgl(L, -1); lua_pop(L, 1); - if (p) p->clearRefs(L); + if (p) p->release(L); } lvglObjectRefs.clear(); } diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 501dcc064a1..5fa78a16e57 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -27,6 +27,7 @@ #include "lua/api_colorlcd.h" #include "lua/lua_widget.h" #include "mainwindow.h" +#include "view_main.h" extern void lv_stb_init(); @@ -39,7 +40,8 @@ class ImageScriptManager : public LuaScriptManager Window* root = nullptr; Window* getCurrentParent() const override { - return tempParent ? tempParent->getWindow() : root; + return (tempParent && tempParent->getWindow()) ? tempParent->getWindow() + : root; } void clear() override { root->clear(); } bool useLvglLayout() const override { return true; } @@ -56,6 +58,16 @@ class LuaBitmapImage : public ::testing::Test LuaScriptManager* previousManager = nullptr; ImageScriptManager manager; uint32_t initialUsage = 0; + int destroyedImages = 0; + + class TrackedImage : public LvglWidgetImage + { + public: + explicit TrackedImage(int* count) : count(count) {} + ~TrackedImage() override { ++*count; } + private: + int* count; + }; static void SetUpTestSuite() { lv_stb_init(); } @@ -68,6 +80,16 @@ class LuaBitmapImage : public ::testing::Test manager.root = new Window(nullptr, {0, 0, 100, 100}); previousManager = luaScriptManager; luaScriptManager = &manager; + lua_pushlightuserdata(L, &destroyedImages); + lua_pushcclosure(L, [](lua_State* L) { + auto count = (int*)lua_touserdata(L, lua_upvalueindex(1)); + auto obj = new TrackedImage(count); + obj->create(L, 1); + obj->push(L); + return 1; + }, 1); + lua_setglobal(L, "trackedImage"); + } void TearDown() override @@ -114,6 +136,22 @@ class LuaBitmapImage : public ::testing::Test auto child = lv_obj_get_child(image()->getWindow()->getLvObj(), 0); return (const lv_img_dsc_t*)lv_img_get_src(child); } + + int imageRef() + { + lua_getglobal(L, "image"); + int ref = LUA_REFNIL; + lua_pushnil(L); + while (lua_next(L, LUA_REGISTRYINDEX)) { + if (lua_isinteger(L, -2) && lua_rawequal(L, -1, -3)) + ref = lua_tointeger(L, -2); + lua_pop(L, 1); + } + lua_pop(L, 1); + return ref; + } + + }; TEST_F(LuaBitmapImage, BorrowsRgb565AndKeepsUserdataAlive) @@ -221,10 +259,15 @@ TEST_F(LuaBitmapImage, NativeDeletionReleasesStrongReference) image()->getWindow()->closeWindow(); ASSERT_TRUE( exec("collectgarbage();" + "image:show(); image:hide(); lvgl.enable(image); lvgl.disable(image);" "assert(not pcall(image.set, image, {bitmap=false}))")); + EXPECT_NE(image(), nullptr); + EXPECT_EQ(image()->getWindow(), nullptr); + EXPECT_TRUE(manager.callRefs(L)); EXPECT_EQ(luaExtraMemoryUsage, initialUsage); manager.clearRefs(L); - ASSERT_TRUE(exec("image=nil; collectgarbage()")); + ASSERT_TRUE(exec("assert(not pcall(image.show, image));" + "image=nil; collectgarbage()")); } TEST_F(LuaBitmapImage, WidgetTeardownReleasesStrongReference) @@ -323,6 +366,138 @@ TEST_F(LuaBitmapImage, AlphaConversionIsReleasedOnNativeDeletion) manager.clearRefs(L); } +TEST_F(LuaBitmapImage, ParentClearPreservesRegistrySlotUntilDeferredCleanup) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("box=lvgl.box({w=100,h=100})")); + lua_getglobal(L, "box"); + manager.setTempParent(LvglWidgetObjectBase::checkLvgl(L, -1, true)); + lua_pop(L, 1); + auto created = exec("image=trackedImage({bitmap=bitmap,w=2,h=1}); bitmap=nil"); + manager.setTempParent(nullptr); + ASSERT_TRUE(created); + int ref = imageRef(); + ASSERT_GT(ref, 0); + ASSERT_TRUE(exec("box:clear(); collectgarbage();" + "label=lvgl.label({text='unrelated'});" + "image:show(); image:hide()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(destroyedImages, 0); + EXPECT_EQ(imageRef(), ref); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_EQ(destroyedImages, 1); + ASSERT_TRUE(exec("label:set({text='still valid'});" + "assert(not pcall(image.show,image))")); + lua_pushboolean(L, true); + int reused = luaL_ref(L, LUA_REGISTRYINDEX); + EXPECT_EQ(reused, ref); + luaL_unref(L, LUA_REGISTRYINDEX, reused); +} + +TEST_F(LuaBitmapImage, FilenameOnlyNativeDeletionKeepsDeferredHandle) +{ + ASSERT_TRUE(exec("image=trackedImage({file='images/color/edgetx.png'," + "w=2,h=1})")); + int ref = imageRef(); + image()->getWindow()->closeWindow(); + EXPECT_EQ(imageRef(), ref); + EXPECT_EQ(destroyedImages, 0); + ASSERT_TRUE(exec("image:show(); image:hide(); lvgl.enable(image);" + "lvgl.disable(image);" + "assert(not pcall(image.set,image,{file=''}));" + "label=lvgl.label({text='unrelated'})")); + EXPECT_TRUE(manager.callRefs(L)); + manager.clearRefs(L); + EXPECT_EQ(destroyedImages, 1); + manager.clearRefs(L); + ASSERT_TRUE(exec("image=nil; collectgarbage(); collectgarbage()")); + EXPECT_EQ(destroyedImages, 1); +} + +TEST_F(LuaBitmapImage, NativeDeletedImageRejectsSourcesAndSkipsCallbacks) +{ + addBitmap("bitmap"); + ASSERT_TRUE(exec("calls=0; image=lvgl.image({bitmap=bitmap,w=2,h=1," + "visible=function() calls=calls+1; return true end});" + "calls=0")); + image()->getWindow()->closeWindow(); + ASSERT_TRUE(exec("assert(not pcall(image.set,image,{bitmap=bitmap}));" + "assert(not pcall(lvgl.image,image,{bitmap=bitmap}));" + "assert(not pcall(lvgl.build,image," + "{{type='image',bitmap=bitmap}}));" + "bitmap=nil; collectgarbage()")); + EXPECT_TRUE(manager.callRefs(L)); + ASSERT_TRUE(exec("assert(calls==0)")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, CollectedCreatorCoroutineDoesNotOwnDeletionState) +{ + addBitmap("bitmap", BMP_ARGB4444); + auto co = lua_newthread(L); + lua_setglobal(L, "creator"); + ASSERT_EQ(luaL_dostring(co, + "image=trackedImage({bitmap=bitmap,w=2,h=1})"), LUA_OK); + ASSERT_TRUE(exec("weak=setmetatable({creator},{__mode='v'});" + "creator=nil; bitmap=nil; collectgarbage(); collectgarbage();" + "assert(weak[1]==nil)")); + image()->getWindow()->closeWindow(); + ASSERT_TRUE(exec("collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + manager.clearRefs(L); + EXPECT_EQ(destroyedImages, 1); +} + +TEST_F(LuaBitmapImage, NativePageAndDialogCloseReleasePixelsBeforeHandles) +{ + ViewMain::instance(); + for (const char* constructor : {"page", "dialog"}) { + addBitmap("bitmap", BMP_ARGB4444); + lua_pushstring(L, constructor); + lua_setglobal(L, "constructor"); + ASSERT_TRUE(exec("container=lvgl[constructor]({title='Bitmap test'});" + "image=lvgl.image(container,{bitmap=bitmap,w=2,h=1});" + "bitmap=nil")); + int ref = imageRef(); + lua_getglobal(L, "container"); + auto container = LvglWidgetObjectBase::checkLvgl(L, -1, true); + lua_pop(L, 1); + container->getWindow()->getParent()->closeWindow(); + ASSERT_TRUE(exec("collectgarbage(); image:show(); image:hide();" + "assert(not pcall(image.set,image,{bitmap=false}))")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(imageRef(), ref); + EXPECT_EQ(image()->getWindow(), nullptr); + manager.clearRefs(L); + ASSERT_TRUE(exec("image=nil; container=nil; collectgarbage()")); + MainWindow::instance()->run(); + } +} + +TEST_F(LuaBitmapImage, ParentCleanupDeletesWrapperExactlyOnceBeforeNativeClose) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image=trackedImage({bitmap=bitmap,w=2,h=1}); bitmap=nil")); + auto window = image()->getWindow(); + manager.clearRefs(L); + EXPECT_EQ(destroyedImages, 1); + window->closeWindow(); + ASSERT_TRUE(exec("image=nil; collectgarbage(); collectgarbage()")); + manager.clearRefs(L); + EXPECT_EQ(destroyedImages, 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) +{ + ASSERT_TRUE(exec("parent=lvgl.label({text='parent'});" + "child=lvgl.label(parent,{text='child'});" + "built=lvgl.build(parent,{{type='label',name='label'," + "text='built'}}); assert(child and built.label)")); + EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), 3u); + EXPECT_EQ(manager.getTempParent(), nullptr); +} + } // namespace #endif From 16a8e27cf8234ce9dc914f2461a86072b9f55035 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:10:30 -0700 Subject: [PATCH 03/15] fix(lua): preserve image sources when parameter updates fail Address review point 2. Parse the file/bitmap source after all other image parameters, so a rejected property never replaces it. Validate the new source and acquire its ref and conversion buffer before releasing the old source, so validation and allocation errors leave the current image intact. Read source fields with raw lookups so params table __index metamethods cannot run during the update. Include rejected-update, metamethod and decoded JPEG zero-copy regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 2 +- radio/src/lua/lua_lvgl_widget.cpp | 98 ++++++++++++------ radio/src/lua/lua_lvgl_widget.h | 2 + radio/src/tests/images/color/bitmap_2x2.jpg | Bin 0 -> 777 bytes radio/src/tests/lua_lvgl.cpp | 105 +++++++++++++++++++- 5 files changed, 174 insertions(+), 33 deletions(-) create mode 100644 radio/src/tests/images/color/bitmap_2x2.jpg diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 4e696a19f12..69ce78fbb16 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -840,7 +840,7 @@ LROT_BEGIN(lvgllib, NULL, 0) @notice Detect Bitmap support with `lvgl.IMAGE_BITMAP_SUPPORTED == 1`. @notice The image keeps its Bitmap alive until replaced, cleared, or deleted. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. - @notice Invalid/unloaded Bitmaps and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. + @notice Invalid source values or parameters and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. @status current Introduced in 2.11.0, floating added in 2.11.6 */ diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 41efaea61d5..1520096ceef 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -1988,32 +1988,45 @@ void LvglWidgetImage::getParams(lua_State* L, int index) { if (hasDeletedWindow()) luaL_error(L, "Image window has been deleted"); LvglWidgetObjectBase::getParams(L, index); + // Parsed last, so a rejected property never replaces the source. + parseSource(L, index); } -void LvglWidgetImage::parseParam(lua_State *L, const char *key) +void LvglWidgetImage::releaseSource(lua_State* L) { - if (!strcmp(key, "bitmap") || !strcmp(key, "file")) { - lua_getfield(L, -3, !strcmp(key, "bitmap") ? "file" : "bitmap"); - bool conflicting = !lua_isnil(L, -1); - lua_pop(L, 1); - if (conflicting) luaL_error(L, "Specify either file or bitmap, not both"); - sourceChanged = true; - } - if (!strcmp(key, "bitmap")) { - if (lua_isboolean(L, -1) && !lua_toboolean(L, -1)) { - clearBitmap(L); - filename.clearRef(L); - filename.txt.clear(); - return; - } - + clearBitmap(L); + filename.clearRef(L); + filename.txt.clear(); +} + +void LvglWidgetImage::parseSource(lua_State* L, int index) +{ + // Resolve both fields once, independently of lua_next iteration order. + // Raw lookups keep __index metamethods from running during the update. + index = lua_absindex(L, index); + lua_pushliteral(L, "file"); + lua_rawget(L, index); + lua_pushliteral(L, "bitmap"); + lua_rawget(L, index); + bool hasFile = !lua_isnil(L, -2); + bool hasBitmap = !lua_isnil(L, -1); + bool clear = lua_isboolean(L, -1) && !lua_toboolean(L, -1); + if (hasFile && hasBitmap) + luaL_error(L, "Specify either file or bitmap, not both"); + if (!hasFile && !hasBitmap) { + lua_pop(L, 2); + return; + } + + // Validate and acquire the new source before releasing the old one, so + // errors leave the current image untouched. + if (hasBitmap && !clear) { auto bitmap = checkBitmap(L, -1); if (!bitmap || !bitmap->getData() || !bitmap->width() || !bitmap->height()) luaL_error(L, "Cannot display an unloaded Bitmap"); if (bitmap->width() > 2047 || bitmap->height() > 2047) luaL_error(L, "Bitmap dimensions exceed the LVGL image header limit"); - uint8_t* pixels = nullptr; uint32_t size = 0; if (bitmap->getFormat() == BMP_ARGB4444) { size = bitmap->width() * bitmap->height() * 3u; @@ -2021,8 +2034,19 @@ void LvglWidgetImage::parseParam(lua_State *L, const char *key) luaExtraMemoryUsage > LUA_MEM_EXTRA_MAX - size) luaL_error(L, "Bitmap image conversion exceeds Lua bitmap memory limit"); + } else if (bitmap->getFormat() != BMP_RGB565) { + luaL_error(L, "Unsupported Bitmap pixel format"); + } + + lua_pushvalue(L, -1); + int ref = ::getRef(L, LUA_REGISTRYINDEX); + uint8_t* pixels = nullptr; + if (size) { pixels = (uint8_t*)malloc(size); - if (!pixels) luaL_error(L, "Cannot allocate Bitmap image conversion"); + if (!pixels) { + clearRef(L, ref); + luaL_error(L, "Cannot allocate Bitmap image conversion"); + } // LVGL needs RGB565 + alpha, not the legacy packed ARGB4444 layout. const auto src = bitmap->getData(); for (uint32_t i = 0; i < bitmap->width() * bitmap->height(); ++i) { @@ -2035,15 +2059,10 @@ void LvglWidgetImage::parseParam(lua_State *L, const char *key) pixels[i * 3 + 1] = rgb >> 8; pixels[i * 3 + 2] = (c >> 12) * 17; } - } else if (bitmap->getFormat() != BMP_RGB565) { - luaL_error(L, "Unsupported Bitmap pixel format"); } - clearBitmap(L); - filename.clearRef(L); - filename.txt.clear(); - lua_pushvalue(L, -1); - bitmapRef = ::getRef(L, LUA_REGISTRYINDEX); + releaseSource(L); + bitmapRef = ref; bitmapPixels = pixels; bitmapPixelsSize = size; luaExtraMemoryUsage += size; @@ -2052,12 +2071,31 @@ void LvglWidgetImage::parseParam(lua_State *L, const char *key) bitmapDescriptor.header.w = bitmap->width(); bitmapDescriptor.header.h = bitmap->height(); bitmapDescriptor.data_size = pixels ? size : bitmap->getDataSize(); - bitmapDescriptor.data = pixels ? pixels : (const uint8_t*)bitmap->getData(); - } else if (!strcmp(key, "file")) { - clearBitmap(L); - filename.clearRef(L); - filename.parse(L); + bitmapDescriptor.data = + pixels ? pixels : (const uint8_t*)bitmap->getData(); + } else { + int ref = LUA_REFNIL; + const char* path = ""; + if (hasFile && lua_isfunction(L, -2)) { + lua_pushvalue(L, -2); + ref = ::getRef(L, LUA_REGISTRYINDEX); + } else if (hasFile) { + path = luaL_checkstring(L, -2); + } + releaseSource(L); + filename.function = ref; + filename.txt = path; filename.forceUpdate(); + } + sourceChanged = true; + lua_pop(L, 2); +} + +void LvglWidgetImage::parseParam(lua_State *L, const char *key) +{ + if (!key) luaL_error(L, "Invalid image property"); + if (!strcmp(key, "file") || !strcmp(key, "bitmap")) { + return; } else if (!strcmp(key, "fill")) { fillFrame = getLuaBool(L); } else { diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index 13708e66c28..77eaa35c85b 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -692,6 +692,8 @@ class LvglWidgetImage : public LvglWidgetObject void build(lua_State *L) override; void getParams(lua_State *L, int index) override; void parseParam(lua_State *L, const char *key) override; + void parseSource(lua_State *L, int index); + void releaseSource(lua_State *L); void refresh() override; void clearBitmap(lua_State* L); static void windowDeleted(lv_event_t* event); diff --git a/radio/src/tests/images/color/bitmap_2x2.jpg b/radio/src/tests/images/color/bitmap_2x2.jpg new file mode 100644 index 0000000000000000000000000000000000000000..313c5fd8a8076c204bdeea9ac72d84257c7cf4bc GIT binary patch literal 777 zcmex=LK$;OGwtxvP%d~(Q!3HV(|CYfbAS1sdzc?emK*3ngfWgAa)0YKg8W4cl zs$izT71^Gf{S2E}UN&&fc=N-l?*9P>K@J8c1}0`kB?cxzMrJ|A|3?_)fp)Sof&o|? zkYHqDW?^Mx=iubx1}fMpz`(@F%*@2X%*qO~hOrhX&%h$cDx_%W$R-?^$gWfnAuRebI{N?Mn?>~P20{IIVo)B*V zNr=zT{3QtV7ZVE$GYdP&UyMxUAdd^Ouqqm|2{{I`Cl(4T88vc8f2KE_o9%~}YXK;@p{B@hbnSlumn3)1c))-U1o+|LKhO|HpcyPIEbKrez|F}mKr;A$7nm#=|KDQZVP*s-CT2kfdxpPL P4lHKs(!OG2{Qo8Z*v8(u literal 0 HcmV?d00001 diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 5fa78a16e57..bb44c9382cb 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -89,7 +89,15 @@ class LuaBitmapImage : public ::testing::Test return 1; }, 1); lua_setglobal(L, "trackedImage"); - + ASSERT_TRUE(exec( + "function lateParams(key,value) " + "for i=1,1000 do " + "local n=8+i%64; local p={};" + "for j=1,n do p['padding'..j]=true end;" + "p['bad'..i]=1; p[key]=value;" + "for j=1,n do p['padding'..j]=nil end;" + "if next(p)==key then return p end end;" + "error('Cannot arrange source before invalid property') end")); } void TearDown() override @@ -151,7 +159,16 @@ class LuaBitmapImage : public ::testing::Test return ref; } - + int registryResources() + { + int count = 0; + lua_pushnil(L); + while (lua_next(L, LUA_REGISTRYINDEX)) { + if (lua_isfunction(L, -1) || lua_isuserdata(L, -1)) ++count; + lua_pop(L, 1); + } + return count; + } }; TEST_F(LuaBitmapImage, BorrowsRgb565AndKeepsUserdataAlive) @@ -203,6 +220,21 @@ TEST_F(LuaBitmapImage, UsesBitmapOpenResultWithoutCopying) EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 480 * 272 * 2); } +TEST_F(LuaBitmapImage, BorrowsDecodedJpegWithoutCopying) +{ + ASSERT_TRUE(exec("bitmap=Bitmap.open('images/color/bitmap_2x2.jpg');" + "local w,h=Bitmap.getSize(bitmap); assert(w==2 and h==2);" + "image=lvgl.image({bitmap=bitmap,w=2,h=2})")); + lua_getglobal(L, "bitmap"); + auto bitmap = checkBitmap(L, -1); + lua_pop(L, 1); + ASSERT_NE(bitmap, nullptr); + EXPECT_EQ(bitmap->getFormat(), BMP_RGB565); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + EXPECT_EQ(source()->header.cf, LV_IMG_CF_TRUE_COLOR); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + bitmap->getDataSize()); +} + TEST_F(LuaBitmapImage, ConvertsArgb4444AndAccountsForPixels) { auto bitmap = addBitmap("bitmap", BMP_ARGB4444); @@ -488,6 +520,57 @@ TEST_F(LuaBitmapImage, ParentCleanupDeletesWrapperExactlyOnceBeforeNativeClose) EXPECT_EQ(luaExtraMemoryUsage, initialUsage); } +TEST_F(LuaBitmapImage, RejectedUpdatesPreservePixelsAndReleaseCandidates) +{ + addBitmap("bitmap", BMP_ARGB4444); + addBitmap("second", BMP_ARGB4444); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=2,h=1})")); + auto descriptor = source(); + auto pixels = descriptor->data; + auto usage = luaExtraMemoryUsage; + auto refs = registryResources(); + ASSERT_TRUE(exec("image:set({x=1,y=2})")); + EXPECT_EQ(source(), descriptor); + EXPECT_EQ(source()->data, pixels); + ASSERT_TRUE(exec( + "for i=1,100 do " + "assert(not pcall(image.set,image,{file={}}));" + "assert(not pcall(image.set,image,{file=function() return '' end,bad=1}));" + "assert(not pcall(image.set,image,{bitmap=second,bad=1}));" + "assert(not pcall(image.set,image,lateParams('bitmap',second)));" + "assert(not pcall(image.set,image,{bitmap=second,x={}}));" + "assert(not pcall(image.set,image,{bitmap=false,borderPad={left={}}}));" + "end; collectgarbage()")); + EXPECT_EQ(source(), descriptor); + EXPECT_EQ(source()->data, pixels); + EXPECT_EQ(luaExtraMemoryUsage, usage); + EXPECT_EQ(registryResources(), refs); + const uint8_t expected[] = {0x00, 0xf8, 0x88, 0xe0, 0x07, 0xff}; + EXPECT_EQ(memcmp(source()->data, expected, sizeof(expected)), 0); +} + +TEST_F(LuaBitmapImage, RejectedUpdatePreservesFilenameCallback) +{ + ASSERT_TRUE(exec( + "calls=0; local old=function() calls=calls+1;" + "return 'images/color/edgetx.png' end;" + "weak=setmetatable({old},{__mode='v'});" + "image=lvgl.image({file=old,w=2,h=1}); old=nil;" + "for i=1,100 do " + "local rejected=function() return 'images/color/bitmap_320x240.png' end;" + "local params={file=rejected,x={}};" + "assert(not pcall(image.set,image,params)); end;" + "collectgarbage(); assert(weak[1]); calls=0")); + EXPECT_TRUE(manager.callRefs(L)); + ASSERT_TRUE(exec("assert(calls==1)")); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); + ASSERT_TRUE(exec("image:set({file='images/color/bitmap_320x240.png'});" + "collectgarbage(); assert(weak[1]==nil); calls=0")); + EXPECT_TRUE(manager.callRefs(L)); + ASSERT_TRUE(exec("assert(calls==0)")); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); +} + TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) { ASSERT_TRUE(exec("parent=lvgl.label({text='parent'});" @@ -498,6 +581,24 @@ TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) EXPECT_EQ(manager.getTempParent(), nullptr); } +TEST_F(LuaBitmapImage, UpdatesIgnoreSourceMetamethods) +{ + addBitmap("first", BMP_ARGB4444); + addBitmap("second", BMP_ARGB4444); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=first,w=2,h=1})")); + auto usage = luaExtraMemoryUsage; + ASSERT_TRUE(exec( + "calls=0; image:set(setmetatable({x=1},{__index=function(t,k)" + "calls=calls+1; image:set({bitmap=second}); return second end}));" + "assert(calls==0)")); + EXPECT_EQ(luaExtraMemoryUsage, usage); + ASSERT_TRUE(exec("image:set({bitmap=false})")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 8); + ASSERT_TRUE(exec("image:set({bitmap=first})")); + const uint8_t expected[] = {0x00, 0xf8, 0x88, 0xe0, 0x07, 0xff}; + EXPECT_EQ(memcmp(source()->data, expected, sizeof(expected)), 0); +} + } // namespace #endif From 864507a22e8c7c2a11113454183cff4ea7138472 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:10:30 -0700 Subject: [PATCH 04/15] fix(lua): release image resources after constructor errors Address review point 3. Protect image construction and explicitly clean failed objects, native windows, registry refs and conversion charges. Restore parent context on failure, including child construction. Include repeated constructor failure and resource-baseline regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 32 +++++++++--- radio/src/lua/lua_lvgl_widget.cpp | 17 +++++++ radio/src/lua/lua_lvgl_widget.h | 3 +- radio/src/tests/lua_lvgl.cpp | 77 +++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 9 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 69ce78fbb16..40010a95066 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -424,23 +424,39 @@ static int luaLvglObjEx(lua_State *L, std::function cre { if (luaScriptManager && (!fullscreenOnly || luaScriptManager->isFullscreen())) { LvglWidgetObjectBase* p = nullptr; - LvglWidgetObjectBase* prevParent = nullptr; + auto prevParent = luaScriptManager->getTempParent(); if (lua_gettop(L) == 2) { p = LvglWidgetObjectBase::checkLvgl(L, 1, true); if (p && p->hasDeletedWindow()) return luaL_error(L, "Parent window has been deleted"); - if (p) { - prevParent = luaScriptManager->getTempParent(); - luaScriptManager->setTempParent(p); - } } LvglWidgetParams params(L, -1); + if (p) luaScriptManager->setTempParent(p); auto obj = create(); - obj->create(L, -1); - - if (params.hasChildren) addChildren(L, obj); + bool created = false; + lua_pushlightuserdata(L, obj); + lua_pushlightuserdata(L, &created); + lua_pushboolean(L, params.hasChildren); + lua_pushcclosure(L, [](lua_State* L) { + auto obj = + (LvglWidgetObjectBase*)lua_touserdata(L, lua_upvalueindex(1)); + obj->create(L, 1); + *(bool*)lua_touserdata(L, lua_upvalueindex(2)) = true; + if (lua_toboolean(L, lua_upvalueindex(3))) addChildren(L, obj); + return 0; + }, 3); + lua_pushvalue(L, -2); + if (lua_pcall(L, 1, 0, 0) != LUA_OK) { + luaScriptManager->setTempParent(prevParent); + if (created) { + auto window = obj->getWindow(); + obj->release(L); + if (window) window->closeWindow(); + } + return lua_error(L); + } obj->push(L); diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 1520096ceef..f6b0af5d0da 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -1984,6 +1984,23 @@ void LvglWidgetArc::build(lua_State *L) //----------------------------------------------------------------------------- +void LvglWidgetImage::create(lua_State *L, int index) +{ + lua_pushlightuserdata(L, this); + lua_pushcclosure(L, [](lua_State* L) { + auto self = (LvglWidgetImage*)lua_touserdata(L, lua_upvalueindex(1)); + self->LvglWidgetObjectBase::create(L, 1); + return 0; + }, 1); + lua_pushvalue(L, index < 0 ? index - 1 : index); + if (lua_pcall(L, 1, 0, 0) != LUA_OK) { + clearRefs(L); + if (window) window->closeWindow(); + delete this; + lua_error(L); + } +} + void LvglWidgetImage::getParams(lua_State* L, int index) { if (hasDeletedWindow()) luaL_error(L, "Image window has been deleted"); diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index 77eaa35c85b..57ef28194a2 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -267,7 +267,7 @@ class LvglWidgetObjectBase virtual void setSize(coord_t w, coord_t h) {} virtual void setFloating(bool isFloating) {} - void create(lua_State *L, int index); + virtual void create(lua_State *L, int index); void update(lua_State *L); virtual Window *getWindow() const = 0; @@ -674,6 +674,7 @@ class LvglWidgetImage : public LvglWidgetObject bool callRefs(lua_State *L) override; void clearRefs(lua_State *L) override; void release(lua_State *L) override { clearRefs(L); delete this; } + void create(lua_State *L, int index) override; bool hasDeletedWindow() const override { return !window && luaRef != LUA_REFNIL; diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index bb44c9382cb..9c32985bfda 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -571,6 +571,83 @@ TEST_F(LuaBitmapImage, RejectedUpdatePreservesFilenameCallback) EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); } +TEST_F(LuaBitmapImage, FailedConstructorsReleaseRefsBudgetAndWrappers) +{ + for (auto format : {BMP_RGB565, BMP_ARGB4444}) { + addBitmap("bitmap", format); + auto refs = registryResources(); + ASSERT_TRUE(exec( + "weak=setmetatable({bitmap},{__mode='v'});" + "for i=1,100 do " + "assert(not pcall(trackedImage,{bitmap=bitmap,badkey=1}));" + "assert(not pcall(trackedImage,lateParams('bitmap',bitmap)));" + "assert(not pcall(trackedImage,{bitmap=bitmap,x={}}));" + "assert(not pcall(trackedImage,{bitmap=bitmap,borderPad={left={}}}));" + "assert(not pcall(trackedImage,{bitmap=bitmap,w=2,h=1," + "file=function() return '' end}));" + "assert(not pcall(trackedImage,{bitmap=bitmap,w=2,h=1," + "size=function() return {} end}));" + "end; bitmap=nil; collectgarbage(); collectgarbage();" + "assert(weak[1]==nil)")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(registryResources(), refs); + EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), 0); + } + EXPECT_EQ(destroyedImages, 1200); +} + +TEST_F(LuaBitmapImage, FailedFilenameConstructorReleasesCapturedBitmap) +{ + addBitmap("bitmap", BMP_ARGB4444); + auto refs = registryResources(); + ASSERT_TRUE(exec( + "weak=setmetatable({bitmap},{__mode='v'});" + "local function source(b) return function() return b end end;" + "for i=1,100 do " + "assert(not pcall(trackedImage,{file=source(bitmap),bad=1}));" + "assert(not pcall(trackedImage,{file=source(bitmap),w=2,h=1}));" + "end; bitmap=nil; collectgarbage(); collectgarbage();" + "assert(weak[1]==nil)")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(destroyedImages, 200); + EXPECT_EQ(registryResources(), refs); +} + +TEST_F(LuaBitmapImage, FailedLayoutConstructorsReleaseBitmapResources) +{ + addBitmap("bitmap", BMP_ARGB4444); + auto refs = registryResources(); + ASSERT_TRUE(exec("for i=1,100 do " + "assert(not pcall(lvgl.build," + "{{type='image',bitmap=bitmap,badkey=1}}));" + "end; bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(registryResources(), refs); +} + +TEST_F(LuaBitmapImage, FailedExplicitParentConstructorRestoresParent) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("box=lvgl.box({w=100,h=100});" + "assert(not pcall(lvgl.image,box,{bitmap=bitmap,bad=1}));" + "image=lvgl.image({bitmap=bitmap,w=2,h=1})")); + EXPECT_EQ(image()->getWindow()->getParent(), manager.root); +} + +TEST_F(LuaBitmapImage, FailedChildConstructionReleasesImageAndRestoresParent) +{ + addBitmap("bitmap", BMP_ARGB4444); + auto refs = registryResources(); + ASSERT_TRUE(exec("for i=1,100 do " + "assert(not pcall(lvgl.image,{bitmap=bitmap,w=2,h=1," + "children={{type='image',bitmap=bitmap,badkey=1}}}));" + "end; bitmap=nil; collectgarbage()")); + EXPECT_EQ(manager.getTempParent(), nullptr); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(registryResources(), refs); + EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), 0); +} + TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) { ASSERT_TRUE(exec("parent=lvgl.label({text='parent'});" From 717342fbde7bb5f933b44f7b22efdf370bcab5f6 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:15:50 -0700 Subject: [PATCH 05/15] docs(lua): use firmware versions for Bitmap image support Address review point 4. Remove the experimental capability constant and document main's Bitmap addition in 3.0.0 without guessing a 2.12 backport release. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 7 ++----- radio/src/tests/lua_lvgl.cpp | 3 +-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 40010a95066..4c7d8e904e9 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -639,9 +639,6 @@ static int luaLvglGetPerf(lua_State *L) extern "C" { // lvgl functions LROT_BEGIN(lvgllib, NULL, 0) - // Capabilities - LROT_NUMENTRY(IMAGE_BITMAP_SUPPORTED, 1) - LROT_FUNCENTRY(clear, luaLvglClear) LROT_FUNCENTRY(build, luaLvglBuild) LROT_FUNCENTRY(isAppMode, luaLvglIsAppMode) @@ -853,12 +850,12 @@ LROT_BEGIN(lvgllib, NULL, 0) @retval table LVGL object @notice `w` and `h` are required. - @notice Detect Bitmap support with `lvgl.IMAGE_BITMAP_SUPPORTED == 1`. + @notice `image:set({file=...})` changes the displayed source and replaces any previous filename callback. @notice The image keeps its Bitmap alive until replaced, cleared, or deleted. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. @notice Invalid source values or parameters and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. - @status current Introduced in 2.11.0, floating added in 2.11.6 + @status current Introduced in 2.11.0, floating added in 2.11.6, bitmap added in 3.0.0 */ LROT_FUNCENTRY(image, [](lua_State* L) { return luaLvglObjEx(L, []() { return new LvglWidgetImage(); }); }) /*luadoc diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 9c32985bfda..5668e024783 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -175,8 +175,7 @@ TEST_F(LuaBitmapImage, BorrowsRgb565AndKeepsUserdataAlive) { auto bitmap = addBitmap("bitmap"); ASSERT_TRUE( - exec("assert(lvgl.IMAGE_BITMAP_SUPPORTED == 1);" - "image = lvgl.image({bitmap=bitmap, w=2, h=1});" + exec("image = lvgl.image({bitmap=bitmap, w=2, h=1});" "weak = setmetatable({bitmap}, {__mode='v'});" "bitmap = nil; collectgarbage(); assert(weak[1])")); EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); From 13a1c6b1272098a426ed1852e2221323f51e0799 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:15:50 -0700 Subject: [PATCH 06/15] fix(lua): resolve file and bitmap=false independently of field order Address review point 5. Allow file plus bitmap=false to select the file source regardless of table traversal order, while rejecting file plus an actual Bitmap. Document and test both source-field orders. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 4 ++-- radio/src/lua/lua_lvgl_widget.cpp | 2 +- radio/src/tests/lua_lvgl.cpp | 29 +++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 4c7d8e904e9..96526568a37 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -843,14 +843,14 @@ LROT_BEGIN(lvgllib, NULL, 0) @param params (table): @commonparams lvgl_object - * `file` (string or function) full path to the image file on the SD card, or a callback returning that path; mutually exclusive with `bitmap`, defaults to an empty string + * `file` (string or function) full path to the image file on the SD card, or a callback returning that path; mutually exclusive with a Bitmap source, but may be combined with `bitmap=false`, defaults to an empty string * `bitmap` (Bitmap or false) existing Bitmap object assigned directly, not a callback; false clears the source and releases the image's Bitmap reference, defaults to nil * `fill` (boolean) if true the image is scaled to completely fill the frame (may be cropped); if false it is scaled to fit entirely within the frame (may have empty borders), defaults to false @retval table LVGL object @notice `w` and `h` are required. - @notice `image:set({file=...})` changes the displayed source and replaces any previous filename callback. + @notice `image:set({file=...})` changes the displayed source and replaces any previous filename callback. If combined with `bitmap=false`, the filename source is used. @notice The image keeps its Bitmap alive until replaced, cleared, or deleted. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. @notice Invalid source values or parameters and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index f6b0af5d0da..d91e18e0c90 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -2028,7 +2028,7 @@ void LvglWidgetImage::parseSource(lua_State* L, int index) bool hasFile = !lua_isnil(L, -2); bool hasBitmap = !lua_isnil(L, -1); bool clear = lua_isboolean(L, -1) && !lua_toboolean(L, -1); - if (hasFile && hasBitmap) + if (hasFile && hasBitmap && !clear) luaL_error(L, "Specify either file or bitmap, not both"); if (!hasFile && !hasBitmap) { lua_pop(L, 2); diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 5668e024783..d2162e2a556 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -647,6 +647,35 @@ TEST_F(LuaBitmapImage, FailedChildConstructionReleasesImageAndRestoresParent) EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), 0); } +TEST_F(LuaBitmapImage, FileAndFalseResolveIndependentlyOfIterationOrder) +{ + addBitmap("bitmap"); + ASSERT_TRUE(exec( + "local tested={};" + "for i=8,256 do " + "local p={w=2,h=1};" + "for j=1,i do p['padding'..j]=true end;" + "if i%2==0 then p.bitmap=false; p.file='images/color/edgetx.png' " + "else p.file='images/color/edgetx.png'; p.bitmap=false end;" + "for j=1,i do p['padding'..j]=nil end;" + "local first; for key in pairs(p) do " + "if key=='file' or key=='bitmap' then first=key; break end end;" + "image=lvgl.image(p);" + "image:set({bitmap=bitmap}); image:set(p); tested[first]=true;" + "if tested.file and tested.bitmap then break end end;" + "assert(tested.file and tested.bitmap)")); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); + ASSERT_TRUE(exec("image:set({bitmap=false,file=function()" + "return 'images/color/bitmap_320x240.png' end})")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("assert(not pcall(image.set,image," + "{bitmap=bitmap,file='images/color/edgetx.png'}));" + "assert(not pcall(image.set,image," + "{file=function() return '' end,bitmap=bitmap}))")); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); +} + TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) { ASSERT_TRUE(exec("parent=lvgl.label({text='parent'});" From 46c307b32f918bbab5d1db18a3fafc2a8ab8f817 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:40:37 -0700 Subject: [PATCH 07/15] fix(lua): forget cleared image objects after lvgl.clear(image) lvgl.clear(image) cleans the StaticImage window, which deletes its internal LVGL image object without updating StaticImage::image. A later source change or image teardown then deleted the stale pointer again. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/gui/colorlcd/libui/static.cpp | 7 +++++++ radio/src/gui/colorlcd/libui/static.h | 2 ++ radio/src/tests/lua_lvgl.cpp | 18 ++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/radio/src/gui/colorlcd/libui/static.cpp b/radio/src/gui/colorlcd/libui/static.cpp index 15adc5a915e..8ffb71a3d8d 100644 --- a/radio/src/gui/colorlcd/libui/static.cpp +++ b/radio/src/gui/colorlcd/libui/static.cpp @@ -259,6 +259,13 @@ void StaticImage::clearSource() image = nullptr; } +void StaticImage::clear() +{ + Window::clear(); + // lv_obj_clean() has deleted the image object. + image = nullptr; +} + bool StaticImage::hasImage() const { lv_img_t* img = (lv_img_t*)image; diff --git a/radio/src/gui/colorlcd/libui/static.h b/radio/src/gui/colorlcd/libui/static.h index 22f9ff6d37e..c365749a2f2 100644 --- a/radio/src/gui/colorlcd/libui/static.h +++ b/radio/src/gui/colorlcd/libui/static.h @@ -158,6 +158,8 @@ class StaticImage : public Window void clearSource(); bool hasImage() const; + void clear() override; + protected: bool fillFrame; bool dontEnlarge = false; diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index d2162e2a556..f54d86e0f6c 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -676,6 +676,24 @@ TEST_F(LuaBitmapImage, FileAndFalseResolveIndependentlyOfIterationOrder) EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); } +TEST_F(LuaBitmapImage, ClearedImageChildrenCanBeReplacedAndReleased) +{ + auto bitmap = addBitmap("bitmap"); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=2,h=1});" + "lvgl.clear(image)")); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); + ASSERT_TRUE(exec("image:set({bitmap=bitmap})")); + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + ASSERT_TRUE(exec("lvgl.clear(image); image:set({bitmap=false});" + "image:set({file='images/color/edgetx.png'});" + "lvgl.clear(image); image:set({bitmap=bitmap});" + "lvgl.clear(image)")); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); + manager.clearRefs(L); + ASSERT_TRUE(exec("image=nil; bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) { ASSERT_TRUE(exec("parent=lvgl.label({text='parent'});" From 4ffde2356be4b936ca9087b6508c8df4e10515b0 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:44:40 -0700 Subject: [PATCH 08/15] fix(lua): restore the LVGL parent when building children fails lvgl.build() and children of page/dialog constructors set a temporary parent while building nested objects but never restored it when a child raised. Later constructors were attached to that parent, which might already have been released and deleted. Build nested objects in a protected call and restore the previous parent before rethrowing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 37 +++++++++++++++++++++-------- radio/src/tests/lua_lvgl.cpp | 15 ++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 96526568a37..5a3f9486f21 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -390,14 +390,29 @@ static void buildLvgl(lua_State *L, int srcIndex, int refIndex) } } +// Calls fn with the top nargs values, restoring the temporary parent if it +// raises so later objects are not attached to a failed build's parent. +static void pcallRestoringParent(lua_State *L, int nargs, lua_CFunction fn, + LvglWidgetObjectBase* prevParent) +{ + lua_pushcfunction(L, fn); + lua_insert(L, -(nargs + 1)); + if (lua_pcall(L, nargs, 0, 0) != LUA_OK) { + luaScriptManager->setTempParent(prevParent); + lua_error(L); + } +} + static void addChildren(lua_State *L, LvglWidgetObjectBase* obj) { if (obj->getWindow()) { lua_getfield(L, -1, "children"); auto prevParent = luaScriptManager->getTempParent(); luaScriptManager->setTempParent(obj); - buildLvgl(L, -1, 0); - lua_pop(L, 1); + pcallRestoringParent(L, 1, [](lua_State* L) { + buildLvgl(L, -1, 0); + return 0; + }, prevParent); luaScriptManager->setTempParent(prevParent); } } @@ -493,23 +508,25 @@ static int luaLvglBuild(lua_State *L) { if (luaScriptManager) { LvglWidgetObjectBase* p = nullptr; - LvglWidgetObjectBase* prevParent = nullptr; + auto prevParent = luaScriptManager->getTempParent(); if (lua_gettop(L) == 2) { p = LvglWidgetObjectBase::checkLvgl(L, 1, true); if (p && p->hasDeletedWindow()) return luaL_error(L, "Parent window has been deleted"); - if (p) { - prevParent = luaScriptManager->getTempParent(); - luaScriptManager->setTempParent(p); - } } + if (p) luaScriptManager->setTempParent(p); + // Return array of lvgl object references lua_newtable(L); - buildLvgl(L, -2, -1); + lua_pushvalue(L, -2); + lua_pushvalue(L, -2); + pcallRestoringParent(L, 2, [](lua_State* L) { + buildLvgl(L, -2, -1); + return 0; + }, prevParent); - if (p) - luaScriptManager->setTempParent((prevParent)); + luaScriptManager->setTempParent(prevParent); } else { lua_pushnil(L); } diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index f54d86e0f6c..046fba4669a 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -722,6 +722,21 @@ TEST_F(LuaBitmapImage, UpdatesIgnoreSourceMetamethods) EXPECT_EQ(memcmp(source()->data, expected, sizeof(expected)), 0); } +TEST_F(LuaBitmapImage, FailedBuildChildrenRestoreParent) +{ + addBitmap("bitmap"); + ASSERT_TRUE(exec("box=lvgl.box({w=100,h=100});" + "assert(not pcall(lvgl.build,{{type='image',bitmap=bitmap," + "w=2,h=1,children={{type='label',bad=1}}}}));" + "assert(not pcall(lvgl.build,box,{{type='box'," + "children={{type='label',bad=1}}}}))")); + EXPECT_EQ(manager.getTempParent(), nullptr); + manager.clearRefs(L); + auto children = lv_obj_get_child_cnt(manager.root->getLvObj()); + ASSERT_TRUE(exec("label=lvgl.label({text='after'})")); + EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), children + 1); +} + } // namespace #endif From 1eb93e5483fac23f5bf3f48ba4080b9854a6f0ff Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:11:18 -0700 Subject: [PATCH 09/15] fix(lua): defer image wrapper deletion until callbacks return An image callback (visible, color, size, pos, opacity or file) that calls lvgl.clear() released and deleted the image while its callRefs() and pcallUpdate*() frames were still running, so they used a freed object. Free released image wrappers from MainWindow::emptyTrash(), the same top-level point that frees closed windows, and detach them from their window on release so the remaining callback code is a no-op. Also skip setSource() when a file callback deleted the image's window. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/gui/colorlcd/libui/mainwindow.cpp | 11 ++ radio/src/gui/colorlcd/libui/mainwindow.h | 5 + radio/src/lua/lua_lvgl_widget.cpp | 14 +- radio/src/lua/lua_lvgl_widget.h | 2 +- radio/src/tests/lua_lvgl.cpp | 158 +++++++++++++++++++- 5 files changed, 186 insertions(+), 4 deletions(-) diff --git a/radio/src/gui/colorlcd/libui/mainwindow.cpp b/radio/src/gui/colorlcd/libui/mainwindow.cpp index e111b28e16e..71e1d4d7ea4 100644 --- a/radio/src/gui/colorlcd/libui/mainwindow.cpp +++ b/radio/src/gui/colorlcd/libui/mainwindow.cpp @@ -46,12 +46,23 @@ MainWindow::MainWindow() : Window(nullptr, {0, 0, LCD_W, LCD_H}) etx_solid_bg(lvobj); } +std::vector> MainWindow::deferredDeletes; + +void MainWindow::deleteLater(std::function deleter) +{ + deferredDeletes.push_back(std::move(deleter)); +} + void MainWindow::emptyTrash() { for (auto window : trash) { delete window; } trash.clear(); + + auto deleters = std::move(deferredDeletes); + deferredDeletes.clear(); + for (auto& deleter : deleters) deleter(); } void MainWindow::run(bool trash) diff --git a/radio/src/gui/colorlcd/libui/mainwindow.h b/radio/src/gui/colorlcd/libui/mainwindow.h index 0b4e0cab8a5..1d32400f30d 100644 --- a/radio/src/gui/colorlcd/libui/mainwindow.h +++ b/radio/src/gui/colorlcd/libui/mainwindow.h @@ -55,11 +55,16 @@ class MainWindow: public Window void blockUntilClose(bool checkPwr, std::function closeCondition, bool isError = false); + // Runs deleter with the next top-level emptyTrash(), like closed windows, + // once outer stack frames can no longer reference the object. + static void deleteLater(std::function deleter); + protected: BitmapBuffer *backgroundBitmap = nullptr; bool widgetRefreshEnable = true; static MainWindow * _instance; + static std::vector> deferredDeletes; void emptyTrash(); }; diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index d91e18e0c90..3b8707e8018 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -27,6 +27,7 @@ #include "keyboard_base.h" #include "lua_event.h" #include "lua_widget.h" +#include "mainwindow.h" #include "menu.h" #include "numberedit.h" #include "page.h" @@ -2130,7 +2131,8 @@ bool LvglWidgetImage::callRefs(lua_State *L) if (filename.function != LUA_REFNIL) { if (pcallFunc(L, filename.function, 1)) { const char *s = luaL_checkstring(L, -1); - if (filename.changedText(s)) { + // The callback may have deleted the window. + if (filename.changedText(s) && window) { ((StaticImage*)window)->setSource(filename.chars()); } lua_settop(L, t); @@ -2154,6 +2156,16 @@ void LvglWidgetImage::clearRefs(lua_State *L) LvglWidgetObject::clearRefs(L); } +void LvglWidgetImage::release(lua_State* L) +{ + clearRefs(L); + // A callback of this image (or its parent) may have cleared the UI, so + // callers further up the stack can still be using this object. Detach it + // from its window (closed by the caller) and delete it from the main loop. + window = nullptr; + MainWindow::deleteLater([this]() { delete this; }); +} + void LvglWidgetImage::windowDeleted(lv_event_t* event) { auto self = (LvglWidgetImage*)lv_event_get_user_data(event); diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index 57ef28194a2..8bee07c0522 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -673,7 +673,7 @@ class LvglWidgetImage : public LvglWidgetObject bool callRefs(lua_State *L) override; void clearRefs(lua_State *L) override; - void release(lua_State *L) override { clearRefs(L); delete this; } + void release(lua_State *L) override; void create(lua_State *L, int index) override; bool hasDeletedWindow() const override { diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 046fba4669a..76f1851fc8e 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -28,6 +28,9 @@ #include "lua/lua_widget.h" #include "mainwindow.h" #include "view_main.h" +#include "standalone_lua.h" + +extern lua_State* lsStandalone; extern void lv_stb_init(); @@ -43,7 +46,13 @@ class ImageScriptManager : public LuaScriptManager return (tempParent && tempParent->getWindow()) ? tempParent->getWindow() : root; } - void clear() override { root->clear(); } + lua_State* L = nullptr; + // Mirrors LuaWidget::clear() and StandaloneLuaWindow::clear(). + void clear() override + { + if (L) clearRefs(L); + root->clear(); + } bool useLvglLayout() const override { return true; } bool isAppMode() const override { return false; } bool isWidget() override { return true; } @@ -78,6 +87,7 @@ class LuaBitmapImage : public ::testing::Test ASSERT_NE(L, nullptr); luaRegisterLibraries(L); manager.root = new Window(nullptr, {0, 0, 100, 100}); + manager.L = L; previousManager = luaScriptManager; luaScriptManager = &manager; lua_pushlightuserdata(L, &destroyedImages); @@ -416,6 +426,8 @@ TEST_F(LuaBitmapImage, ParentClearPreservesRegistrySlotUntilDeferredCleanup) EXPECT_EQ(destroyedImages, 0); EXPECT_EQ(imageRef(), ref); EXPECT_TRUE(manager.callRefs(L)); + EXPECT_EQ(destroyedImages, 0); + MainWindow::instance()->run(); EXPECT_EQ(destroyedImages, 1); ASSERT_TRUE(exec("label:set({text='still valid'});" "assert(not pcall(image.show,image))")); @@ -439,9 +451,11 @@ TEST_F(LuaBitmapImage, FilenameOnlyNativeDeletionKeepsDeferredHandle) "label=lvgl.label({text='unrelated'})")); EXPECT_TRUE(manager.callRefs(L)); manager.clearRefs(L); + MainWindow::instance()->run(); EXPECT_EQ(destroyedImages, 1); manager.clearRefs(L); ASSERT_TRUE(exec("image=nil; collectgarbage(); collectgarbage()")); + MainWindow::instance()->run(); EXPECT_EQ(destroyedImages, 1); } @@ -476,6 +490,7 @@ TEST_F(LuaBitmapImage, CollectedCreatorCoroutineDoesNotOwnDeletionState) ASSERT_TRUE(exec("collectgarbage()")); EXPECT_EQ(luaExtraMemoryUsage, initialUsage); manager.clearRefs(L); + MainWindow::instance()->run(); EXPECT_EQ(destroyedImages, 1); } @@ -511,10 +526,13 @@ TEST_F(LuaBitmapImage, ParentCleanupDeletesWrapperExactlyOnceBeforeNativeClose) ASSERT_TRUE(exec("image=trackedImage({bitmap=bitmap,w=2,h=1}); bitmap=nil")); auto window = image()->getWindow(); manager.clearRefs(L); - EXPECT_EQ(destroyedImages, 1); + EXPECT_EQ(destroyedImages, 0); window->closeWindow(); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); ASSERT_TRUE(exec("image=nil; collectgarbage(); collectgarbage()")); manager.clearRefs(L); + MainWindow::instance()->run(); EXPECT_EQ(destroyedImages, 1); EXPECT_EQ(luaExtraMemoryUsage, initialUsage); } @@ -737,6 +755,142 @@ TEST_F(LuaBitmapImage, FailedBuildChildrenRestoreParent) EXPECT_EQ(lv_obj_get_child_cnt(manager.root->getLvObj()), children + 1); } +TEST_F(LuaBitmapImage, ScriptClearFromImageCallbacksDefersDeletion) +{ + struct Case { const char* create; const char* callback; }; + const Case cases[] = { + {"{bitmap=bitmap,w=2,h=1}", "visible=function() CLEAR return true end"}, + {"{bitmap=bitmap,w=2,h=1}", "color=function() CLEAR return 0 end"}, + {"{bitmap=bitmap,w=2,h=1}", "opacity=function() CLEAR return 255 end"}, + {"{bitmap=bitmap,w=2,h=1}", "size=function() CLEAR return 2,1 end"}, + {"{bitmap=bitmap,w=2,h=1}", "pos=function() CLEAR return 0,0 end"}, + {"{file='images/color/edgetx.png',w=2,h=1}", + "file=function() CLEAR return 'images/color/bitmap_320x240.png' end"}, + }; + for (const auto& c : cases) { + SCOPED_TRACE(c.callback); + addBitmap("bitmap", BMP_ARGB4444); + auto refs = registryResources(); + std::string callback = c.callback; + callback.replace(callback.find("CLEAR"), 5, + "lvgl.clear(); lvgl.clear();" + "after=lvgl.label({text='after'});"); + std::string code = std::string("image=trackedImage(") + c.create + + "); image:set({" + callback + "})"; + ASSERT_TRUE(exec(code.c_str())); + auto destroyed = destroyedImages; + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_EQ(destroyedImages, destroyed); + ASSERT_TRUE(exec("assert(not pcall(image.show,image));" + "after:set({text='still valid'})")); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, destroyed + 1); + EXPECT_TRUE(manager.callRefs(L)); + manager.clearRefs(L); + MainWindow::instance()->run(); + ASSERT_TRUE(exec("image=nil; after=nil; bitmap=nil; collectgarbage()")); + EXPECT_EQ(destroyedImages, destroyed + 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_LE(registryResources(), refs); + } +} + +TEST_F(LuaBitmapImage, ChildCallbackClearsParentImage) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec( + "image=trackedImage({bitmap=bitmap,w=20,h=20});" + "child=lvgl.image(image,{bitmap=bitmap,w=2,h=1});" + "child:set({visible=function() lvgl.clear(); lvgl.clear();" + "after=lvgl.label({text='after'}); return true end})")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_EQ(destroyedImages, 0); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); + ASSERT_TRUE(exec("assert(not pcall(child.show,child));" + "after:set({text='still valid'});" + "image=nil; child=nil; bitmap=nil; collectgarbage()")); + manager.clearRefs(L); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, ImageCallbackErrorAfterScriptClear) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image=trackedImage({bitmap=bitmap,w=2,h=1});" + "image:set({visible=function() lvgl.clear();" + "error('after clear') end})")); + EXPECT_FALSE(manager.callRefs(L)); + EXPECT_EQ(destroyedImages, 0); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); + ASSERT_TRUE(exec("image=nil; bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); +} + +TEST_F(LuaBitmapImage, FileCallbackClosingItsDialog) +{ + ViewMain::instance(); + ASSERT_TRUE(exec( + "dialog=lvgl.dialog({title='Bitmap test'});" + "image=lvgl.image(dialog,{file='images/color/edgetx.png',w=2,h=1});" + "image:set({file=function() lvgl.close(dialog);" + "return 'images/color/bitmap_320x240.png' end})")); + EXPECT_TRUE(manager.callRefs(L)); + ASSERT_TRUE(exec("assert(not pcall(image.show,image))")); + manager.clearRefs(L); + MainWindow::instance()->run(); + ASSERT_TRUE(exec("image=nil; dialog=nil; collectgarbage()")); +} + +TEST_F(LuaBitmapImage, StandaloneScriptClearFromImageCallback) +{ + ViewMain::instance(); + auto S = luaL_newstate(); + ASSERT_NE(S, nullptr); + luaRegisterLibraries(S); + lua_pushlightuserdata(S, &destroyedImages); + lua_pushcclosure(S, [](lua_State* L) { + auto obj = new TrackedImage((int*)lua_touserdata(L, lua_upvalueindex(1))); + obj->create(L, 1); + obj->push(L); + return 1; + }, 1); + lua_setglobal(S, "trackedImage"); + ASSERT_EQ(luaL_dostring(S, + "frames=0;" + "function run(event) frames=frames+1;" + "if frames==1 then bitmap=Bitmap.open('images/color/bitmap_2x2.jpg');" + "image=trackedImage({bitmap=bitmap,w=2,h=2});" + "image:set({visible=function() lvgl.clear();" + "after=lvgl.label({text='after'}); return true end}) end;" + "return 0 end"), LUA_OK); + lua_getglobal(S, "run"); + int runRef = luaL_ref(S, LUA_REGISTRYINDEX); + auto savedState = luaState; + lsStandalone = S; + StandaloneLuaWindow::setup(true, LUA_REFNIL, runRef); + auto window = StandaloneLuaWindow::instance(); + ASSERT_NE(window, nullptr); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); + MainWindow::instance()->run(); + lua_getglobal(S, "frames"); + EXPECT_EQ(lua_tointeger(S, -1), 2); + lua_pop(S, 1); + EXPECT_EQ(luaL_dostring(S, "after:set({text='still valid'});" + "assert(not pcall(image.show,image))"), LUA_OK); + window->closeWindow(); + MainWindow::instance()->run(); + EXPECT_EQ(lsStandalone, nullptr); + EXPECT_EQ(destroyedImages, 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + luaScriptManager = &manager; + luaState = savedState; +} + } // namespace #endif From de8df18654dc5ec74a6c552c2d0327dd0674bb7f Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:12:08 -0700 Subject: [PATCH 10/15] fix(lua): keep the displayed image when clearing an image's children lvgl.clear(image) cleaned the StaticImage window, deleting its internal LVGL image object along with the image's Lua children. The image then stayed blank, because neither a Bitmap source nor an unchanged filename or callback result is applied again, while a Bitmap source stayed referenced and charged to the budget. The internal image object is not one of the image's children, so keep it when clearing; only content added on top is removed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/gui/colorlcd/libui/static.cpp | 12 +++-- radio/src/tests/lua_lvgl.cpp | 64 ++++++++++++++++++++----- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/radio/src/gui/colorlcd/libui/static.cpp b/radio/src/gui/colorlcd/libui/static.cpp index 8ffb71a3d8d..06d0d789396 100644 --- a/radio/src/gui/colorlcd/libui/static.cpp +++ b/radio/src/gui/colorlcd/libui/static.cpp @@ -261,9 +261,15 @@ void StaticImage::clearSource() void StaticImage::clear() { - Window::clear(); - // lv_obj_clean() has deleted the image object. - image = nullptr; + // Only remove content added on top; the displayed image is not a child. + deleteChildren(); + if (lvobj) { + for (int i = (int)lv_obj_get_child_cnt(lvobj) - 1; i >= 0; i -= 1) { + auto child = lv_obj_get_child(lvobj, i); + if (child != image) lv_obj_del(child); + } + } + invalidate(); } bool StaticImage::hasImage() const diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 76f1851fc8e..a74e4920734 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -694,22 +694,60 @@ TEST_F(LuaBitmapImage, FileAndFalseResolveIndependentlyOfIterationOrder) EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); } -TEST_F(LuaBitmapImage, ClearedImageChildrenCanBeReplacedAndReleased) +TEST_F(LuaBitmapImage, ClearingImageChildrenKeepsItsSource) { - auto bitmap = addBitmap("bitmap"); - ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=2,h=1});" + for (auto format : {BMP_RGB565, BMP_ARGB4444}) { + SCOPED_TRACE(format); + auto bitmap = addBitmap("bitmap", format); + ASSERT_TRUE(exec( + "image=trackedImage({bitmap=bitmap,w=20,h=20});" + "label=lvgl.label(image,{text='child'});" + "child=lvgl.image(image,{bitmap=bitmap,w=2,h=1})")); + auto descriptor = source(); + auto pixels = descriptor->data; + auto usage = luaExtraMemoryUsage; + ASSERT_TRUE(exec("lvgl.clear(image)")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 1u); + EXPECT_EQ(source(), descriptor); + EXPECT_EQ(source()->data, pixels); + if (format == BMP_RGB565) + EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); + // Only the child's conversion buffer is released. + EXPECT_EQ(luaExtraMemoryUsage, + usage - (format == BMP_ARGB4444 ? 6u : 0u)); + ASSERT_TRUE(exec("assert(not pcall(child.show,child));" + "assert(not pcall(label.set,label,{text='x'}));" + "image:set({bitmap=bitmap}); lvgl.clear(image);" + "image:set({bitmap=false})")); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); + manager.clearRefs(L); + MainWindow::instance()->run(); + ASSERT_TRUE(exec("image=nil; child=nil; label=nil; bitmap=nil;" + "collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + } + EXPECT_EQ(destroyedImages, 2); +} + +TEST_F(LuaBitmapImage, ClearingImageChildrenKeepsFilenameSources) +{ + ASSERT_TRUE(exec("image=lvgl.image({file='images/color/edgetx.png',w=2,h=1});" "lvgl.clear(image)")); - EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); - ASSERT_TRUE(exec("image:set({bitmap=bitmap})")); - EXPECT_EQ(source()->data, (const uint8_t*)bitmap->getData()); - ASSERT_TRUE(exec("lvgl.clear(image); image:set({bitmap=false});" - "image:set({file='images/color/edgetx.png'});" - "lvgl.clear(image); image:set({bitmap=bitmap});" + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); + ASSERT_TRUE(exec("image:set({file=function()" + "return 'images/color/bitmap_320x240.png' end})")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("lvgl.clear(image)")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("image:set({file='images/color/edgetx.png'});" "lvgl.clear(image)")); - EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); - manager.clearRefs(L); - ASSERT_TRUE(exec("image=nil; bitmap=nil; collectgarbage()")); - EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); } TEST_F(LuaBitmapImage, WindowlessObjectsRemainValidParents) From 014da21bd683f091e7c99dc6b54aec84ce56cb39 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:12:35 -0700 Subject: [PATCH 11/15] fix(lua): keep the current image until a new filename callback runs Switching an image from a filename string to a filename callback marked the source changed with an empty path, so refresh() removed the image and it stayed blank until the next callRefs() evaluated the callback. Leave the displayed file in place until the callback's first result is applied. Switching from a Bitmap still releases its pixels immediately. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/lua_lvgl_widget.cpp | 5 ++++- radio/src/tests/lua_lvgl.cpp | 32 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 3b8707e8018..8b92c961f12 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -2091,6 +2091,7 @@ void LvglWidgetImage::parseSource(lua_State* L, int index) bitmapDescriptor.data_size = pixels ? size : bitmap->getDataSize(); bitmapDescriptor.data = pixels ? pixels : (const uint8_t*)bitmap->getData(); + sourceChanged = true; } else { int ref = LUA_REFNIL; const char* path = ""; @@ -2104,8 +2105,10 @@ void LvglWidgetImage::parseSource(lua_State* L, int index) filename.function = ref; filename.txt = path; filename.forceUpdate(); + // A callback replaces the displayed file when it first runs; keep the + // current pixels until then. A Bitmap source was released above. + sourceChanged = ref == LUA_REFNIL; } - sourceChanged = true; lua_pop(L, 2); } diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index a74e4920734..3ef787c7f27 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -929,6 +929,38 @@ TEST_F(LuaBitmapImage, StandaloneScriptClearFromImageCallback) luaState = savedState; } +TEST_F(LuaBitmapImage, FilenameCallbackKeepsPixelsUntilItRuns) +{ + ASSERT_TRUE(exec("image=lvgl.image({file='images/color/edgetx.png',w=2,h=1});" + "image:set({file=function()" + "return 'images/color/bitmap_320x240.png' end})")); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("image:set({file=function()" + "return 'images/color/edgetx.png' end}); image:hide()")); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/bitmap_320x240.png"); + ASSERT_TRUE(exec("image:show()")); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); +} + +TEST_F(LuaBitmapImage, FilenameCallbackReleasesBitmapImmediately) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=2,h=1});" + "weak=setmetatable({bitmap},{__mode='v'}); bitmap=nil;" + "image:set({file=function()" + "return 'images/color/edgetx.png' end}); collectgarbage();" + "assert(weak[1]==nil)")); + EXPECT_EQ(lv_obj_get_child_cnt(image()->getWindow()->getLvObj()), 0u); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); +} + } // namespace #endif From 98e6aec8803ca3759969c2cd92d76aa92e26c6f1 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:12:40 -0700 Subject: [PATCH 12/15] docs(lua): note image Bitmap budget headroom and clear behavior The new conversion buffer is allocated before the old one is released, so a replace keeps the current image if it fails. Document that this briefly needs budget for both conversions, and that lvgl.clear(image) removes only the image's children. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/api_colorlcd_lvgl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/radio/src/lua/api_colorlcd_lvgl.cpp b/radio/src/lua/api_colorlcd_lvgl.cpp index 5a3f9486f21..d8b2788d12c 100644 --- a/radio/src/lua/api_colorlcd_lvgl.cpp +++ b/radio/src/lua/api_colorlcd_lvgl.cpp @@ -868,8 +868,8 @@ LROT_BEGIN(lvgllib, NULL, 0) @notice `w` and `h` are required. @notice `image:set({file=...})` changes the displayed source and replaces any previous filename callback. If combined with `bitmap=false`, the filename source is used. - @notice The image keeps its Bitmap alive until replaced, cleared, or deleted. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. - @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. + @notice The image keeps its Bitmap alive until replaced or deleted; `lvgl.clear(image)` removes only the image's children. Use `image:set({bitmap=false})` to release this reference immediately, including when hidden. A nil table field is absent and does not clear it. Drop script references and run `collectgarbage()` to reclaim unused Bitmap memory. + @notice RGB565 Bitmaps (including JPEGs) are borrowed without copying pixels. ARGB4444 Bitmaps require an RGB565-plus-alpha conversion, charged to the existing Lua bitmap memory budget. Replacing one ARGB4444 Bitmap with another briefly needs budget for both conversions; near the limit, set `bitmap=false` first. @notice Invalid source values or parameters and conversion allocation or budget failures raise a Lua error without replacing the previous source. Check `Bitmap.getSize()` after `Bitmap.open()`; a failed load returns zero width and height. @status current Introduced in 2.11.0, floating added in 2.11.6, bitmap added in 3.0.0 From 086f504bd951d304d92cb48d01bc87378785c16b Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:40:35 -0700 Subject: [PATCH 13/15] fix(lua): drop image resources acquired after a mid-update release GC steps while image:set() parses its parameters can run a __gc finalizer that clears the UI or closes the image's window. Parsing then continued, taking a new Bitmap ref, conversion buffer and budget charge, and callback refs, on an already released image. Nothing freed them, and the conversion charge outlived the Lua state. Parse image parameters in a protected call. Afterwards, release everything if the image was released meanwhile, or drop the new Bitmap if its window was deleted, then rethrow any parse error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/lua_lvgl_widget.cpp | 22 +++++++-- radio/src/lua/lua_lvgl_widget.h | 1 + radio/src/tests/lua_lvgl.cpp | 79 +++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 3 deletions(-) diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index 8b92c961f12..d39c0808908 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -2005,9 +2005,24 @@ void LvglWidgetImage::create(lua_State *L, int index) void LvglWidgetImage::getParams(lua_State* L, int index) { if (hasDeletedWindow()) luaL_error(L, "Image window has been deleted"); - LvglWidgetObjectBase::getParams(L, index); - // Parsed last, so a rejected property never replaces the source. - parseSource(L, index); + // GC steps while parsing can run finalizers that clear the UI or close + // this image's window. Parse in a protected call so anything acquired + // after that can be dropped, whether parsing succeeds or fails. + lua_pushlightuserdata(L, this); + lua_pushcclosure(L, [](lua_State* L) { + auto self = (LvglWidgetImage*)lua_touserdata(L, lua_upvalueindex(1)); + self->LvglWidgetObjectBase::getParams(L, 1); + // Parsed last, so a rejected property never replaces the source. + self->parseSource(L, 1); + return 0; + }, 1); + lua_pushvalue(L, index < 0 ? index - 1 : index); + int status = lua_pcall(L, 1, 0, 0); + if (released) + clearRefs(L); + else if (hasDeletedWindow()) + clearBitmap(L); + if (status != LUA_OK) lua_error(L); } void LvglWidgetImage::releaseSource(lua_State* L) @@ -2161,6 +2176,7 @@ void LvglWidgetImage::clearRefs(lua_State *L) void LvglWidgetImage::release(lua_State* L) { + released = true; clearRefs(L); // A callback of this image (or its parent) may have cleared the UI, so // callers further up the stack can still be using this object. Detach it diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index 8bee07c0522..f3111703d70 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -689,6 +689,7 @@ class LvglWidgetImage : public LvglWidgetObject uint32_t bitmapPixelsSize = 0; lua_State* bitmapState = nullptr; bool sourceChanged = false; + bool released = false; void build(lua_State *L) override; void getParams(lua_State *L, int index) override; diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 3ef787c7f27..a0ff674d315 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -30,6 +30,11 @@ #include "view_main.h" #include "standalone_lua.h" +extern "C" { +#include "thirdparty/Lua/src/lgc.h" +#include "thirdparty/Lua/src/lstate.h" +} + extern lua_State* lsStandalone; extern void lv_stb_init(); @@ -169,6 +174,30 @@ class LuaBitmapImage : public ::testing::Test return ref; } + // Calls image:set() with a queued __gc finalizer that runs + // `action` at the first GC check inside the call. + int setWithPendingFinalizer(const char* params, const char* action) + { + std::string code = std::string( + "finalized=false; collectgarbage('stop');" + "do local t=setmetatable({},{__gc=function() finalized=true;") + + action + " end}) end"; + if (!exec(code.c_str())) return LUA_ERRRUN; + lua_getglobal(L, "image"); + lua_getfield(L, -1, "set"); + lua_insert(L, -2); + lua_getglobal(L, params); + luaC_runtilstate(L, ~bitmask(GCSpause)); + luaC_runtilstate(L, bitmask(GCScallfin)); + EXPECT_NE(G(L)->tobefnz, nullptr); + G(L)->gcrunning = 1; + luaE_setdebt(G(L), 1); + int status = lua_pcall(L, 2, 0, 0); + if (status != LUA_OK) lua_pop(L, 1); + EXPECT_TRUE(exec("assert(finalized)")); + return status; + } + int registryResources() { int count = 0; @@ -961,6 +990,56 @@ TEST_F(LuaBitmapImage, FilenameCallbackReleasesBitmapImmediately) EXPECT_STREQ((const char*)source(), "A/images/color/edgetx.png"); } +TEST_F(LuaBitmapImage, FinalizerClearingDuringUpdateReleasesNewSources) +{ + struct Case { const char* params; bool ok; }; + const Case cases[] = { + {"params={bitmap=bitmap}", true}, + {"params={bitmap=bitmap,visible=function() return true end," + "color=function() return 0 end}", true}, + {"params={file=function() return 'images/color/edgetx.png' end}", true}, + {"params={visible=function() return true end,bitmap=bitmap,x={}}", false}, + }; + for (auto format : {BMP_RGB565, BMP_ARGB4444}) { + for (const auto& c : cases) { + SCOPED_TRACE(c.params); + addBitmap("bitmap", format); + auto refs = registryResources(); + ASSERT_TRUE(exec("image=trackedImage({bitmap=false,w=2,h=1})")); + ASSERT_TRUE(exec(c.params)); + auto destroyed = destroyedImages; + EXPECT_EQ(setWithPendingFinalizer("params", "lvgl.clear()") == LUA_OK, + c.ok); + EXPECT_EQ(registryResources(), refs); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE(exec("assert(not pcall(image.show,image))")); + EXPECT_EQ(destroyedImages, destroyed); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, destroyed + 1); + ASSERT_TRUE(exec("image=nil; params=nil; bitmap=nil;" + "collectgarbage(); collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + } + } +} + +TEST_F(LuaBitmapImage, FinalizerClosingWindowDuringUpdateReleasesNewBitmap) +{ + ViewMain::instance(); + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("dialog=lvgl.dialog({title='Bitmap test'});" + "image=lvgl.image(dialog,{bitmap=false,w=2,h=1});" + "params={bitmap=bitmap}")); + EXPECT_EQ(setWithPendingFinalizer("params", "lvgl.close(dialog)"), LUA_OK); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage + 4); + ASSERT_TRUE(exec("assert(not pcall(image.set,image,{bitmap=bitmap}));" + "params=nil; bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + manager.clearRefs(L); + MainWindow::instance()->run(); + ASSERT_TRUE(exec("image=nil; dialog=nil; collectgarbage()")); +} + } // namespace #endif From a652b531510aa120095cbe4ab898bb6b08333ab9 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:41:19 -0700 Subject: [PATCH 14/15] fix(lua): ignore filename callback results after a source change A filename callback that replaced the image source while running, for example with image:set({bitmap=b}) or {bitmap=false}, had its returned path applied afterwards. That overwrote the new Bitmap or cleared state, kept the Bitmap charged while a file was shown, and dropped the new source for good since the callback was gone. Count committed source changes and apply a callback result only when no change happened during the call. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/lua_lvgl_widget.cpp | 8 ++++-- radio/src/lua/lua_lvgl_widget.h | 1 + radio/src/tests/lua_lvgl.cpp | 42 +++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index d39c0808908..d3648cab181 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -2124,6 +2124,7 @@ void LvglWidgetImage::parseSource(lua_State* L, int index) // current pixels until then. A Bitmap source was released above. sourceChanged = ref == LUA_REFNIL; } + sourceGeneration += 1; lua_pop(L, 2); } @@ -2147,10 +2148,13 @@ bool LvglWidgetImage::callRefs(lua_State *L) if (isVisible()) { int t = lua_gettop(L); if (filename.function != LUA_REFNIL) { + auto generation = sourceGeneration; if (pcallFunc(L, filename.function, 1)) { const char *s = luaL_checkstring(L, -1); - // The callback may have deleted the window. - if (filename.changedText(s) && window) { + // The callback may have replaced the source or deleted the window; + // then its result is obsolete. + if (generation == sourceGeneration && window && + filename.changedText(s)) { ((StaticImage*)window)->setSource(filename.chars()); } lua_settop(L, t); diff --git a/radio/src/lua/lua_lvgl_widget.h b/radio/src/lua/lua_lvgl_widget.h index f3111703d70..fa51b399f36 100644 --- a/radio/src/lua/lua_lvgl_widget.h +++ b/radio/src/lua/lua_lvgl_widget.h @@ -690,6 +690,7 @@ class LvglWidgetImage : public LvglWidgetObject lua_State* bitmapState = nullptr; bool sourceChanged = false; bool released = false; + uint32_t sourceGeneration = 0; void build(lua_State *L) override; void getParams(lua_State *L, int index) override; diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index a0ff674d315..4aec495f807 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -1040,6 +1040,48 @@ TEST_F(LuaBitmapImage, FinalizerClosingWindowDuringUpdateReleasesNewBitmap) ASSERT_TRUE(exec("image=nil; dialog=nil; collectgarbage()")); } +TEST_F(LuaBitmapImage, ReplacedFilenameCallbackResultIsDiscarded) +{ + // Expected display after the first and second callRefs(): nullptr means + // no image, "bitmap" the Bitmap, otherwise a file path. The obsolete + // outer result (bitmap_480x272.png) must never be shown. + struct Case { const char* replace; const char* first; const char* second; }; + const char* bitmapSource = "bitmap"; + const Case cases[] = { + {"image:set({bitmap=bitmap})", bitmapSource, bitmapSource}, + {"image:set({bitmap=false})", nullptr, nullptr}, + {"image:set({file='images/color/edgetx.png'})", + "A/images/color/edgetx.png", "A/images/color/edgetx.png"}, + // A new callback keeps the current image until it first runs. + {"image:set({file=function() return 'images/color/edgetx.png' end})", + "A/images/color/bitmap_320x240.png", "A/images/color/edgetx.png"}, + }; + for (const auto& c : cases) { + SCOPED_TRACE(c.replace); + auto bitmap = addBitmap("bitmap", BMP_ARGB4444); + std::string code = std::string( + "image=lvgl.image({file='images/color/bitmap_320x240.png',w=2,h=1});" + "outer=0; image:set({file=function() outer=outer+1;") + + c.replace + " return 'images/color/bitmap_480x272.png' end})"; + ASSERT_TRUE(exec(code.c_str())); + for (auto expected : {c.first, c.second}) { + EXPECT_TRUE(manager.callRefs(L)); + auto lvobj = image()->getWindow()->getLvObj(); + EXPECT_EQ(lv_obj_get_child_cnt(lvobj), expected ? 1u : 0u); + if (expected == bitmapSource) { + EXPECT_EQ(lv_img_src_get_type(source()), LV_IMG_SRC_VARIABLE); + EXPECT_EQ(luaExtraMemoryUsage, + initialUsage + bitmap->getDataSize() + 6u); + } else if (expected) { + EXPECT_STREQ((const char*)source(), expected); + } + } + ASSERT_TRUE(exec("assert(outer==1); image:set({bitmap=false});" + "image=nil; bitmap=nil; collectgarbage()")); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + } +} + } // namespace #endif From ca94527342cf8f3497ed9e98587d7642b284de89 Mon Sep 17 00:00:00 2001 From: Thomas Kistler <542423+thomaskistler@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:41:52 -0700 Subject: [PATCH 15/15] fix(lua): release children of a pending clear on deleted images callRefs() returned early for an image whose window was deleted natively, so a pending lvgl.clear(image) never released its children. Their handles, callbacks and anything the callbacks captured stayed referenced until the whole script UI was torn down. Still run no callbacks for such an image, but release the children of a pending clear. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- radio/src/lua/lua_lvgl_widget.cpp | 10 ++++++++- radio/src/tests/lua_lvgl.cpp | 35 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/radio/src/lua/lua_lvgl_widget.cpp b/radio/src/lua/lua_lvgl_widget.cpp index d3648cab181..527feffd1a4 100644 --- a/radio/src/lua/lua_lvgl_widget.cpp +++ b/radio/src/lua/lua_lvgl_widget.cpp @@ -2142,7 +2142,15 @@ void LvglWidgetImage::parseParam(lua_State *L, const char *key) bool LvglWidgetImage::callRefs(lua_State *L) { - if (!window) return true; + if (!window) { + // Natively deleted: run no callbacks, but still release the children + // of a pending lvgl.clear(image). + if (clearRequest) { + clearRequest = false; + clearChildRefs(L); + } + return true; + } if (!LvglWidgetObject::callRefs(L)) return false; if (isVisible()) { diff --git a/radio/src/tests/lua_lvgl.cpp b/radio/src/tests/lua_lvgl.cpp index 4aec495f807..dce425ec5e2 100644 --- a/radio/src/tests/lua_lvgl.cpp +++ b/radio/src/tests/lua_lvgl.cpp @@ -1082,6 +1082,41 @@ TEST_F(LuaBitmapImage, ReplacedFilenameCallbackResultIsDiscarded) } } +TEST_F(LuaBitmapImage, DeletedImageReleasesChildrenOfPendingClear) +{ + addBitmap("bitmap", BMP_ARGB4444); + ASSERT_TRUE(exec("image=lvgl.image({bitmap=bitmap,w=20,h=20})")); + auto refs = registryResources(); + lua_getglobal(L, "image"); + manager.setTempParent(LvglWidgetObjectBase::checkLvgl(L, -1, true)); + lua_pop(L, 1); + auto created = exec( + "calls=0; local function capture(b) return function()" + "calls=calls+1; return b~=nil end end;" + "child=trackedImage({bitmap=bitmap,w=2,h=1,visible=capture(bitmap)});" + "label=lvgl.label({text='child',visible=capture(bitmap)});" + "weak=setmetatable({bitmap},{__mode='v'}); bitmap=nil; calls=0"); + manager.setTempParent(nullptr); + ASSERT_TRUE(created); + ASSERT_TRUE(exec("lvgl.clear(image)")); + image()->getWindow()->closeWindow(); + EXPECT_TRUE(manager.callRefs(L)); + EXPECT_TRUE(manager.callRefs(L)); + ASSERT_TRUE(exec("collectgarbage(); assert(weak[1]==nil); assert(calls==0);" + "assert(not pcall(child.show,child));" + "assert(not pcall(label.set,label,{text='x'}))")); + // Only the image handle remains; native deletion released its Bitmap. + EXPECT_EQ(registryResources(), refs - 1); + EXPECT_EQ(luaExtraMemoryUsage, initialUsage); + EXPECT_EQ(destroyedImages, 0); + MainWindow::instance()->run(); + EXPECT_EQ(destroyedImages, 1); + manager.clearRefs(L); + MainWindow::instance()->run(); + ASSERT_TRUE(exec("image=nil; child=nil; label=nil; collectgarbage()")); + EXPECT_EQ(destroyedImages, 1); +} + } // namespace #endif