From 617a7cf0b9bfa7df648dd86e1061bfb8a771a722 Mon Sep 17 00:00:00 2001 From: liu weikai Date: Mon, 21 Sep 2026 18:17:06 +0800 Subject: [PATCH] Avoid redundant PNG map tile pixel copies --- CHANGELOG.md | Bin 119120 -> 119380 bytes .../map_tiles/lvgl_tile_image.h | 101 ++++++++++++ .../include/ui/widgets/map/map_tiles.h | 2 +- .../src/ui/widgets/map/map_tiles.cpp | 47 ++---- tests/map_gesture/CMakeLists.txt | 1 + tests/map_tile_image/CMakeLists.txt | 24 +++ tests/map_tile_image/test_image.cpp | 151 ++++++++++++++++++ 7 files changed, 292 insertions(+), 34 deletions(-) create mode 100644 platform/esp/arduino_common/include/platform/esp/arduino_common/map_tiles/lvgl_tile_image.h create mode 100644 tests/map_tile_image/CMakeLists.txt create mode 100644 tests/map_tile_image/test_image.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index b63dab5631b069d4362f32efc3dca89ab38968e9..0e7aa947f2483e47179e4d904b710536435ce195 100644 GIT binary patch delta 172 zcmcbxi2ceE_6@U`L_-XED8FGL)k)aq!rZB`#J{T)Ic>*)rWRDF3(-%}Qim(PS z_%XOo-zdW<9IU`lz>vvM0oJ3ykj#(|=BEPnDln7)c?w|jQW#2sa;ZRGE|AOxiWf0d qf>jg&?MVZ|OrX31ke$bnGC8+YoEu5q +#include + +namespace platform::esp::map_tiles +{ + +// A cache image owns either a completed decoder session or copied pixels. +// Keep ownership out of the cache policy and out of device/storage selection. +class LvglTileImage final : public lv_image_dsc_t +{ + public: + // allocate_owner must return storage compatible with lv_free. + static lv_image_dsc_t* capture(lv_image_decoder_dsc_t& decoder, + void* (*allocate_owner)(size_t) = lv_malloc) + { + const lv_draw_buf_t* pixels = decoder.decoded; + if (!pixels || !pixels->data || !pixels->data_size || !pixels->header.w || !pixels->header.h) + { + return nullptr; + } + void* storage = allocate_owner(sizeof(LvglTileImage)); + if (!storage) return nullptr; + auto* image = new (storage) LvglTileImage(); + image->header = pixels->header; + // Storage ownership belongs to this object, not to consumers of its view. + image->header.flags &= ~LV_IMAGE_FLAGS_ALLOCATED; + image->data_size = pixels->data_size; + + if (canRetainSession(decoder)) + { + image->data = pixels->data; + image->session_ = decoder; + // LODEPNG has consumed all input during open; close only destroys + // decoded. Keep a stable source descriptor, but not compressed data. + image->source_ = *static_cast(decoder.src); + image->source_.data = nullptr; + image->source_.data_size = 0; + image->session_.src = &image->source_; + decoder = {}; // Move the session; the caller's close is now a no-op. + } + else + { + auto* copy = static_cast(lv_malloc(pixels->data_size)); + if (!copy) + { + image->~LvglTileImage(); + lv_free(image); + return nullptr; + } + std::memcpy(copy, pixels->data, pixels->data_size); + image->data = copy; + } + return image; + } + + // Only descriptors returned by capture may be passed here. + static void destroy(lv_image_dsc_t* descriptor) + { + if (!descriptor) return; + lv_image_cache_drop(descriptor); + auto* image = static_cast(descriptor); + image->~LvglTileImage(); + lv_free(image); + } + + private: + LvglTileImage() : lv_image_dsc_t{} {} + ~LvglTileImage() + { + if (session_.decoder) + lv_image_decoder_close(&session_); + else + lv_free(const_cast(data)); + } + LvglTileImage(const LvglTileImage&) = delete; + LvglTileImage& operator=(const LvglTileImage&) = delete; + + static bool canRetainSession(const lv_image_decoder_dsc_t& decoder) + { + // Audited against the pinned LVGL release. Unknown decoders/versions + // retain the copy path: they may borrow input or own additional state. +#if LVGL_VERSION_MAJOR == 9 && LVGL_VERSION_MINOR == 4 && LVGL_VERSION_PATCH == 0 + return decoder.decoder && decoder.decoder->name && + std::strcmp(decoder.decoder->name, "LODEPNG") == 0 && + decoder.src_type == LV_IMAGE_SRC_VARIABLE && decoder.src && + decoder.args.no_cache && !decoder.cache_entry && !decoder.user_data; +#else + return false; +#endif + } + + lv_image_decoder_dsc_t session_{}; + lv_image_dsc_t source_{}; +}; + +} // namespace platform::esp::map_tiles diff --git a/platform/esp/arduino_common/include/ui/widgets/map/map_tiles.h b/platform/esp/arduino_common/include/ui/widgets/map/map_tiles.h index 3d3c0e17..11ff1085 100644 --- a/platform/esp/arduino_common/include/ui/widgets/map/map_tiles.h +++ b/platform/esp/arduino_common/include/ui/widgets/map/map_tiles.h @@ -58,7 +58,7 @@ struct DecodedTileCache int32_t x, y, z; // Tile coordinates uint8_t map_source; // Base map source (OSM/Terrain/Satellite) ui::map_tiles::MapTileLayer layer; // Base or contour layer represented by this cache entry - lv_image_dsc_t* img_dsc; // Decoded image descriptor (RGB565 data in RAM) + lv_image_dsc_t* img_dsc; // Owned decoded image view (decoder-native pixel format) uint32_t last_used_ms; // For LRU eviction uint8_t lvgl_ref_count; // Number of live LVGL image objects referencing img_dsc }; diff --git a/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp b/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp index 610bf591..ff70917c 100644 --- a/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp +++ b/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp @@ -8,6 +8,7 @@ #include "freertos/semphr.h" #include "freertos/task.h" #include "lvgl.h" +#include "platform/esp/arduino_common/map_tiles/lvgl_tile_image.h" #include "platform/esp/arduino_common/map_tiles/map_tile_command_queue.h" #include "platform/esp/arduino_common/map_tiles/map_tile_event_queue.h" #include "platform/esp/arduino_common/storage/sd_card_runtime.h" @@ -43,7 +44,7 @@ void MapPoiPayloadDeleter::operator()(uint8_t* data) const noexcept { heap_caps_ #endif // Use LVGL's decoder API to decode PNG images -// We'll decode PNG to RGB565 and cache it in RAM to avoid re-decoding on every render +// Cache the decoder's native pixel format to avoid re-decoding on every render. // This approach uses LVGL's built-in decoder, avoiding direct lodepng dependency // Debug logging control @@ -495,7 +496,17 @@ lv_image_dsc_t* decode_payload_to_image_desc(const ui::map_tiles::MapTileRef& re return nullptr; } - lv_image_dsc_t* img_dsc = static_cast(lv_malloc(sizeof(lv_image_dsc_t))); + // Session metadata must not turn a PSRAM optimization into internal heap + // growth. ESP's lv_free ultimately calls heap_caps_free for this storage. + lv_image_dsc_t* img_dsc = platform::esp::map_tiles::LvglTileImage::capture( + decoder_dsc, [](size_t size) -> void* + { +#if HAS_PSRAM + return heap_caps_malloc(size, MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT); +#else + return lv_malloc(size); +#endif + }); if (img_dsc == NULL) { log_map_tile_decode_failure("alloc_desc", @@ -506,31 +517,6 @@ lv_image_dsc_t* decode_payload_to_image_desc(const ui::map_tiles::MapTileRef& re close_decoder(); return nullptr; } - std::memset(img_dsc, 0, sizeof(lv_image_dsc_t)); - - uint8_t* img_data = static_cast(lv_malloc(data_size)); - if (img_data == NULL) - { - log_map_tile_decode_failure("alloc_pixels", - ref, - payload_format, - data_size, - -12); - lv_free(img_dsc); - close_decoder(); - return nullptr; - } - - std::memcpy(img_data, decoded_buf->data, data_size); - img_dsc->header.magic = LV_IMAGE_HEADER_MAGIC; - img_dsc->header.w = decoded_buf->header.w; - img_dsc->header.h = decoded_buf->header.h; - img_dsc->header.cf = decoded_buf->header.cf; - img_dsc->header.flags = 0; - img_dsc->header.stride = decoded_buf->header.stride; - img_dsc->data_size = data_size; - img_dsc->data = img_data; - close_decoder(); return img_dsc; } @@ -927,12 +913,7 @@ class LvglDecodedTileCache final : public ui::map_tiles::IMapTileDecoderCache { if (slot.img_dsc != NULL) { - lv_image_cache_drop(slot.img_dsc); - if (slot.img_dsc->data != NULL) - { - lv_free((void*)slot.img_dsc->data); - } - lv_free(slot.img_dsc); + platform::esp::map_tiles::LvglTileImage::destroy(slot.img_dsc); slot.img_dsc = NULL; } } diff --git a/tests/map_gesture/CMakeLists.txt b/tests/map_gesture/CMakeLists.txt index 4bd57630..181453d3 100644 --- a/tests/map_gesture/CMakeLists.txt +++ b/tests/map_gesture/CMakeLists.txt @@ -39,3 +39,4 @@ enable_testing() add_test(NAME map_gesture_regression COMMAND map_gesture_regression) set_tests_properties(map_gesture_regression PROPERTIES TIMEOUT 30) add_subdirectory("${REPO}/tests/settings_navigation" settings_navigation) +add_subdirectory("${REPO}/tests/map_tile_image" map_tile_image) diff --git a/tests/map_tile_image/CMakeLists.txt b/tests/map_tile_image/CMakeLists.txt new file mode 100644 index 00000000..9526f2ea --- /dev/null +++ b/tests/map_tile_image/CMakeLists.txt @@ -0,0 +1,24 @@ +cmake_minimum_required(VERSION 3.20) +project(map_tile_image LANGUAGES C CXX) +set(CMAKE_CXX_STANDARD 17) +set(REPO "${CMAKE_CURRENT_LIST_DIR}/../..") +if(NOT TARGET lvgl) + set(LVGL_DIR "" CACHE PATH "Pinned LVGL 9.4.0 source directory") + set(LV_BUILD_CONF_PATH "${REPO}/tests/map_poi_view/lv_conf.h" CACHE PATH "" FORCE) + set(CONFIG_LV_BUILD_DEMOS OFF CACHE BOOL "" FORCE) + set(CONFIG_LV_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE) + set(CONFIG_LV_USE_THORVG_INTERNAL OFF CACHE BOOL "" FORCE) + add_subdirectory(${LVGL_DIR} lvgl) +endif() +target_compile_definitions(lvgl PUBLIC LV_USE_LODEPNG=1) +add_executable(map_tile_image test_image.cpp) +target_include_directories(map_tile_image PRIVATE "${REPO}/platform/esp/arduino_common/include") +target_link_libraries(map_tile_image PRIVATE lvgl) +if(MSVC) + target_compile_options(map_tile_image PRIVATE /UNDEBUG) +else() + target_compile_options(map_tile_image PRIVATE -UNDEBUG -Wall -Wextra) +endif() +enable_testing() +add_test(NAME map_tile_image COMMAND map_tile_image) +set_tests_properties(map_tile_image PROPERTIES TIMEOUT 60) diff --git a/tests/map_tile_image/test_image.cpp b/tests/map_tile_image/test_image.cpp new file mode 100644 index 00000000..e64e9103 --- /dev/null +++ b/tests/map_tile_image/test_image.cpp @@ -0,0 +1,151 @@ +#include "lvgl.h" +#include "src/draw/lv_draw_buf_private.h" +#include "src/draw/lv_image_decoder_private.h" +#define LODEPNG_NO_COMPILE_CPP +#include "src/libs/lodepng/lodepng.h" +#include "src/misc/cache/instance/lv_image_cache.h" +#include +#include +#include +#include +#include +#include + +// Inject failures into the production ownership adapter, not into the decoder. +static int allocation_calls; +static int fail_allocation; +static size_t largest_allocation; +static void* capture_malloc(size_t bytes) +{ + ++allocation_calls; + largest_allocation = std::max(largest_allocation, bytes); + if (allocation_calls == fail_allocation) return nullptr; + return lv_malloc(bytes); +} +#define lv_malloc capture_malloc +#include "platform/esp/arduino_common/map_tiles/lvgl_tile_image.h" +#undef lv_malloc +using platform::esp::map_tiles::LvglTileImage; + +static lv_draw_buf_free_cb_t original_free; +static const void* watched_buffer; +static int watched_frees; +static void tracked_free(void* buffer) +{ + if (buffer == watched_buffer) ++watched_frees; + original_free(buffer); +} + +static void exercise(unsigned width, unsigned height, bool alpha, bool fallback, int fail) +{ + std::vector rgba(width * height * 4); + for (size_t i = 0; i < rgba.size(); i += 4) + { + rgba[i] = static_cast(i / 4); + rgba[i + 1] = 93; + rgba[i + 2] = 211; + rgba[i + 3] = alpha ? static_cast(i / 16) : 255; + } + uint8_t* png = nullptr; + size_t png_size = 0; + assert(lodepng_encode32(&png, &png_size, rgba.data(), width, height) == 0); + lv_image_dsc_t source{}; + source.header.magic = LV_IMAGE_HEADER_MAGIC; + source.header.cf = LV_COLOR_FORMAT_RAW_ALPHA; + source.data = png; + source.data_size = static_cast(png_size); + lv_image_decoder_dsc_t decoder{}; + lv_image_decoder_args_t args{}; + args.no_cache = true; + args.stride_align = true; + assert(lv_image_decoder_open(&decoder, &source, &args) == LV_RESULT_OK); + assert(decoder.decoded && decoder.decoded->header.cf == LV_COLOR_FORMAT_ARGB8888); + const auto* pixels = decoder.decoded->data; + const uint32_t bytes = decoder.decoded->data_size; + const uint32_t stride = decoder.decoded->header.stride; + std::vector expected(pixels, pixels + bytes); + watched_buffer = decoder.decoded->unaligned_data; + watched_frees = 0; + + // Unknown decoder names must take the compatibility path, even with the + // same pixel format. Restore the real decoder before its original close. + auto* original_decoder = decoder.decoder; + lv_image_decoder_t unknown = *original_decoder; + unknown.name = "UNKNOWN"; + if (fallback) decoder.decoder = &unknown; + allocation_calls = 0; + fail_allocation = fail; + largest_allocation = 0; + lv_image_dsc_t* image = LvglTileImage::capture(decoder); + fail_allocation = 0; + if (fail) + { + assert(!image); + assert(decoder.decoded && decoder.decoded->data == pixels); + assert(watched_frees == 0); + decoder.decoder = original_decoder; + lv_image_decoder_close(&decoder); + assert(watched_frees == 1); + } + else + { + assert(image && image->data_size == bytes && image->header.stride == stride); + assert(image->header.cf == LV_COLOR_FORMAT_ARGB8888); + if (fallback) + { + assert(image->data != pixels && allocation_calls == 2); + decoder.decoder = original_decoder; + } + else + { + assert(image->data == pixels); // Proves no full-image copy. + assert(!decoder.decoder && !decoder.decoded); + assert(allocation_calls == 1 && largest_allocation < 1024); + } + lv_image_decoder_close(&decoder); + assert(watched_frees == (fallback ? 1 : 0)); + // The input payload and its descriptor may disappear before rendering. + lv_free(png); + png = nullptr; + source = {}; + assert(std::memcmp(image->data, expected.data(), bytes) == 0); + lv_obj_t* obj = lv_image_create(lv_screen_active()); + lv_image_set_src(obj, image); + lv_refr_now(nullptr); + assert(std::memcmp(image->data, expected.data(), bytes) == 0); + lv_obj_delete(obj); + LvglTileImage::destroy(image); + assert(watched_frees == 1); // Eviction destroys the original exactly once. + } + lv_free(png); + watched_buffer = nullptr; +} + +int main() +{ + lv_init(); + auto* display = lv_display_create(256, 256); + std::vector framebuffer(256 * 256 * 2); + lv_display_set_buffers(display, framebuffer.data(), nullptr, framebuffer.size(), LV_DISPLAY_RENDER_MODE_DIRECT); + lv_display_set_flush_cb(display, [](lv_display_t* disp, const lv_area_t*, uint8_t*) + { lv_display_flush_ready(disp); }); + auto* handlers = lv_draw_buf_get_image_handlers(); + original_free = handlers->buf_free_cb; + handlers->buf_free_cb = tracked_free; + + lv_image_decoder_dsc_t empty{}; + assert(!LvglTileImage::capture(empty)); + LvglTileImage::destroy(nullptr); + for (int cycle = 0; cycle < 24; ++cycle) + { + exercise(256, 256, cycle % 2 != 0, false, 0); + exercise(17, 13, true, false, 0); // Non-tile dimensions / stride. + } + exercise(256, 256, false, true, 0); + exercise(256, 256, true, false, 1); // Owner allocation fails. + exercise(256, 256, true, true, 2); // Fallback pixel allocation fails. + handlers->buf_free_cb = original_free; + lv_display_delete(display); + lv_deinit(); + std::puts("PNG zero-copy ownership, rendering, eviction, fallback and allocation failure passed"); +}