diff --git a/docs/audits/UI_STORAGE_HOT_PATH_BYPASS_AUDIT.md b/docs/audits/UI_STORAGE_HOT_PATH_BYPASS_AUDIT.md index f1122bee..3ed7aadb 100644 --- a/docs/audits/UI_STORAGE_HOT_PATH_BYPASS_AUDIT.md +++ b/docs/audits/UI_STORAGE_HOT_PATH_BYPASS_AUDIT.md @@ -21,15 +21,15 @@ LVGL FS for SD-backed files, or wait on a display-shared SPI resource. | --- | --- | --- | --- | | `platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp` UI tile source | LVGL FS path was present in ESP map source | High confusion risk: ESP UI source appeared able to read through LVGL FS | Burned in this cut. ESP UI source is path-only; SD reads remain in the worker `SdMapTileFileSystem`. | | `platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp` worker tile source | SD through `SdRuntimeFile` | Accepted only in worker domain | Keep. This is the map storage adapter behind the tile async runtime. | -| `platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp` tile decode | LVGL image decoder, CPU/memory work | High when executed from UI event drain | Burned from the UI hot path in this cut. The ESP map worker now publishes already-decoded native image descriptors after releasing the SD bus. | +| `platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp` tile decode | LVGL image decoder, CPU/memory work | Medium. LVGL has no OS lock on ESP, so decoding must stay UI-owned until a non-LVGL decoder adapter exists | Remaining legacy. Worker-side LVGL decode was rejected because `LV_USE_OS` is `LV_OS_NONE`. | | `modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp::locale_preview_font` | External font load via `lv_binfont_create` / LVGL FS | High. Locale/settings UI preview could synchronously load SD-backed font packs | Burned in this cut for ESP. Preview uses only already-loaded fonts on ESP. | -| `modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp::activate_locale_internal` | External UI font load | High on ESP if locale UI font is SD-backed | Burned from the ESP UI hot path in this cut. ESP defers unloaded external UI fonts and falls back to the builtin latin UI font chain. | +| `modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp::activate_locale_internal` | External UI/content font load | High on ESP if page rendering loads SD-backed fonts | Partially burned. Startup registry activation may load active external fonts once; ordinary UI activation/preview/content hot paths still defer. | | `modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp` external pack scan | LVGL FS dir/file reads | Medium. Usually registry/startup path, but still synchronous | Remaining legacy. Needs pack catalog worker or startup preflight outside visible UI frame budget. | | `modules/ui_shared/src/ui/screens/gps/gps_page_runtime.cpp` route loaders | `ui::fs::read_text_file` | Medium to high when user opens/loads a route | Remaining legacy. Move GPX/CSV parse into route/track runtime worker and return overlay points by event. | | `platform/esp/arduino_common/src/ui/screens/team/team_ui_store.cpp` | SD exists/open/write/remove/rename | High. A UI screen store owns durable persistence | Remaining legacy. Move team snapshot/key/event/chat/position persistence behind `TEAM_ACTION_RUNTIME_SPEC` storage worker. | | `platform/esp/arduino_common/src/ui/runtime/pack_repository.cpp` | Flash/LVGL FS open/dir/read/write | Medium. Pack management can block UI actions | Remaining legacy. Keep as explicit pack runtime adapter only after UI callers submit commands; remove direct page calls. | | `modules/ui_shared/src/ui/widgets/busy_overlay.cpp` | `lv_refr_now` display flush | Medium if called while storage owns display-shared SPI | Burned in this cut. Busy overlay now invalidates and lets the normal UI tick render. | -| startup shells / boot log | `lv_refr_now` / manual `lv_timer_handler` display flush | Medium. Startup could bypass normal UI tick and contend with early SD/display setup | Burned in this cut. Startup paths now invalidate and let the normal UI tick render. | +| startup shells / boot log | `lv_refr_now` / manual `lv_timer_handler` display flush | Medium. Startup runs before normal UI tick exists | Refined. Boot log avoids `lv_refr_now`; startup boot presentation invalidates and services one LVGL tick so the splash can appear during setup. | | `platform/esp/arduino_common/src/LV_Helper_v9.cpp` | LVGL FS adapter to SD/flash | Adapter risk, not business risk | Keep as platform adapter. UI hot paths must not use it for SD-backed files. | | `platform/esp/boards/src/display/DisplayInterface.cpp` | display SPI lock/flush | Frame-critical adapter | Keep. Other workers must yield to this domain through bounded wait, hold, and cooldown policy. | @@ -55,17 +55,17 @@ Otherwise it falls back to the base font and returns immediately. This prevents settings/list rendering from repeatedly entering `lv_binfont_create()` and LVGL FS just because Chinese text appeared in a preview. -### Locale activation cannot synchronously load external UI fonts on ESP +### Locale activation is split between startup and UI hot paths -`activate_locale_internal()` no longer calls external `lv_binfont_create()` from -ESP activation/startup paths. If the selected locale points at an unloaded -external UI font pack, the locale remains active but the UI font chain falls back -to `builtin-latin-ui` and logs the external font as deferred. +`activate_locale_internal()` may synchronously load active external UI/content +fonts only while the registry is rebuilding during startup. Ordinary UI +activation, preview, and content missing-glyph paths still defer unloaded +external fonts. -This prioritizes UI liveness over synchronous Chinese UI font availability. The -proper recovery path is an i18n font-load runtime command that loads the -external font outside the UI owner context and publishes a font-chain update -event. +This restores Chinese UI after boot without allowing page rendering to repeatedly +enter `lv_binfont_create()`. The proper recovery path is still an i18n font-load +runtime command that loads external fonts outside interactive UI paths and +publishes a font-chain update event. ### Busy overlay no longer forces immediate display refresh @@ -76,30 +76,24 @@ rendering. This keeps slow-operation feedback from directly competing with SD or other display-shared SPI work in the same call stack. -### Startup paths no longer force display refresh +### Startup paths do not force immediate refresh -Boot log updates and startup shell boot presentation no longer call -`lv_refr_now()` or manually drive `lv_timer_handler()`. They invalidate the -affected layer/object and return. +Boot log updates no longer call `lv_refr_now()`. Startup shell boot presentation +invalidates the top layer and services `lv_timer_handler()` once because the +normal UI tick is not running yet during setup. -This removes startup display refresh re-entry from the same period where SD, -radio, font catalog, and display initialization can still be contending for -shared resources. +This restores boot visibility without reintroducing direct forced refresh. -### Map tile decode moved out of UI event drain +### Invalid map focus no longer scans default London tiles -The ESP map active path previously read tile bytes in the worker but decoded the -PNG/JPG payload in `apply_map_tile_event()` on the UI owner context. +The map page previously treated `snapshot.header.valid` as enough to make the +map focus valid. That made no-fix/no-viewport startup fall back to the London +default coordinate and then probe SD for tiles such as +`/maps/base/osm/12/2046/1362.png`. -The ESP event sink now decodes the compressed payload before enqueueing the -event. The UI owner receives an already-decoded native image descriptor and only -adopts it into the decoded cache / LVGL image object. - -This removes the active UI call to `lv_image_decoder_open()`. The remaining -technical debt is that the worker-side decoder still uses LVGL's decoder API as -a platform decoder adapter. That is no longer a UI hot-path bypass, but a later -decoder-adapter cut should replace it with a decoder that is explicitly safe to -run outside the UI owner context. +The focus is now valid only when there is a real viewport center or a valid self +position. Without either, the map viewport cleans up tile records and does not +enqueue tile IO. ## Remaining Burn-Down Order @@ -112,7 +106,7 @@ run outside the UI owner context. worker owned by the team action runtime. 4. Convert pack repository actions into commands/events. Keep LVGL FS and flash APIs only in platform adapters. -5. Replace worker-side LVGL image decoding with a platform decoder adapter that +5. Replace UI-side LVGL image decoding with a platform decoder adapter that has no dependency on LVGL global decoder state. ## Verification Gate For Each Follow-Up Cut diff --git a/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h b/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h index be884ec8..a4fae502 100644 --- a/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h +++ b/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h @@ -44,15 +44,6 @@ struct LoadTileCommand using MapTileEventKind = MapTileAsyncEventKind; -struct MapTileDecodedImage -{ - void* native_handle = nullptr; - void (*release)(void*) = nullptr; - uint16_t width = 0; - uint16_t height = 0; - std::size_t data_size = 0; -}; - class MapTileEvent { public: @@ -62,7 +53,6 @@ class MapTileEvent MapTileRef tile{}; MapTileFormat format = MapTileFormat::Unknown; MapTilePayload payload{}; - MapTileDecodedImage decoded{}; std::size_t payload_size = 0; int32_t error = 0; }; diff --git a/modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp b/modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp index 6f0114bd..feab5928 100644 --- a/modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp +++ b/modules/ui_shared/src/ui/i18n/resource_pack_registry.cpp @@ -159,6 +159,7 @@ std::vector s_content_supplement_packs; bool s_registry_ready = false; unsigned s_missing_content_font_diagnostics = 0; +bool s_allow_sync_external_font_activation = false; class ScopedFontLoadOverlay { @@ -1216,6 +1217,16 @@ bool can_load_font_from_content_hot_path(const FontPackRecord& pack) #endif } +bool can_load_font_from_activation_path(const FontPackRecord& pack) +{ +#if defined(ESP_PLATFORM) || defined(ARDUINO_ARCH_ESP32) + return pack.builtin || s_allow_sync_external_font_activation; +#else + (void)pack; + return true; +#endif +} + std::vector current_content_pack_sequence() { std::vector packs; @@ -2342,8 +2353,21 @@ bool activate_locale_internal(LocalePackRecord* locale, FontPackRecord* preserve if (s_active_ui_font_pack != nullptr && !is_font_runtime_loaded(*s_active_ui_font_pack)) { -#if defined(ESP_PLATFORM) || defined(ARDUINO_ARCH_ESP32) - if (!s_active_ui_font_pack->builtin) + if (can_load_font_from_activation_path(*s_active_ui_font_pack)) + { + if (!ensure_font_pack_loaded(s_active_ui_font_pack)) + { + if (locale != nullptr && locale->id != kDefaultLocaleId) + { + LocalePackRecord* fallback = resolve_active_locale(kDefaultLocaleId); + if (fallback && fallback != locale) + { + return activate_locale_internal(fallback, preserved_content_pack); + } + } + } + } + else { std::printf("%s font load deferred id=%s role=active_ui reason=ui_activation active_locale=%s source=%s\n", kLogTag, @@ -2357,23 +2381,25 @@ bool activate_locale_internal(LocalePackRecord* locale, FontPackRecord* preserve s_active_ui_font_pack = fallback_ui; } } - else if (!ensure_font_pack_loaded(s_active_ui_font_pack)) + } + + if (s_active_content_font_pack != nullptr && + !is_font_runtime_loaded(*s_active_content_font_pack)) + { + if (can_load_font_from_activation_path(*s_active_content_font_pack)) { - s_active_ui_font_pack = find_pack_by_id(s_font_packs, kBuiltinLatinFontPackId); + (void)ensure_font_pack_loaded(s_active_content_font_pack); } -#else - if (!ensure_font_pack_loaded(s_active_ui_font_pack)) + else { - if (locale != nullptr && locale->id != kDefaultLocaleId) - { - LocalePackRecord* fallback = resolve_active_locale(kDefaultLocaleId); - if (fallback && fallback != locale) - { - return activate_locale_internal(fallback, preserved_content_pack); - } - } + std::printf("%s font load deferred id=%s role=active_content reason=ui_activation active_locale=%s source=%s\n", + kLogTag, + s_active_content_font_pack->id.c_str(), + s_active_locale ? s_active_locale->id.c_str() : "", + s_active_content_font_pack->source_path.empty() + ? "" + : s_active_content_font_pack->source_path.c_str()); } -#endif } if (can_preserve_content_pack(preserved_content_pack, s_active_ui_font_pack, s_active_content_font_pack)) @@ -2424,7 +2450,9 @@ void rebuild_registry() s_registry_ready = true; const std::string preferred_locale = migrate_legacy_locale_if_needed(); + s_allow_sync_external_font_activation = true; (void)activate_locale(resolve_active_locale(preferred_locale)); + s_allow_sync_external_font_activation = false; std::printf("%s registry rebuild end active_locale=%s locale_count=%lu ime_count=%lu enabled_ime=%lu active_ime=%s ui_chain=%s content_chain=%s\n", kLogTag, s_active_locale ? s_active_locale->id.c_str() : "", diff --git a/modules/ui_shared/src/ui/screens/gps/gps_page_runtime.cpp b/modules/ui_shared/src/ui/screens/gps/gps_page_runtime.cpp index e0196d9e..4d982353 100644 --- a/modules/ui_shared/src/ui/screens/gps/gps_page_runtime.cpp +++ b/modules/ui_shared/src/ui/screens/gps/gps_page_runtime.cpp @@ -304,7 +304,7 @@ bool commit_pending_map_pan_from_screen() std::isfinite(snapshot.viewport.center_lat) && std::isfinite(snapshot.viewport.center_lon) && (snapshot.viewport.center_lat != 0.0 || snapshot.viewport.center_lon != 0.0); - model.focus_point.valid = has_viewport_center || snapshot.self.valid || snapshot.header.valid; + model.focus_point.valid = has_viewport_center || snapshot.self.valid; model.focus_point.lat = has_viewport_center ? snapshot.viewport.center_lat : (snapshot.self.valid ? snapshot.self.lat diff --git a/modules/ui_shared/src/ui/startup_shell.cpp b/modules/ui_shared/src/ui/startup_shell.cpp index 85b6de36..8abd7f01 100644 --- a/modules/ui_shared/src/ui/startup_shell.cpp +++ b/modules/ui_shared/src/ui/startup_shell.cpp @@ -58,6 +58,7 @@ void present_boot_overlay_now() { lv_obj_invalidate(top); } + lv_timer_handler(); } } // namespace diff --git a/modules/ui_shared/src/ui/startup_ui_shell.cpp b/modules/ui_shared/src/ui/startup_ui_shell.cpp index 869bf730..c5fdf44d 100644 --- a/modules/ui_shared/src/ui/startup_ui_shell.cpp +++ b/modules/ui_shared/src/ui/startup_ui_shell.cpp @@ -26,6 +26,7 @@ void present_boot_overlay_now() { lv_obj_invalidate(top); } + lv_timer_handler(); } bool resolve_display_time(struct tm* out_tm) 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 e4018c31..7a6404fb 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 @@ -414,30 +414,6 @@ uint8_t* allocate_tile_payload(std::size_t size) MALLOC_CAP_8BIT)); } -void release_decoded_image_handle(void* handle) -{ - auto* img_dsc = static_cast(handle); - if (img_dsc == nullptr) - { - return; - } - lv_image_cache_drop(img_dsc); - if (img_dsc->data != nullptr) - { - lv_free((void*)img_dsc->data); - } - lv_free(img_dsc); -} - -void release_tile_decoded_image(ui::map_tiles::MapTileDecodedImage& decoded) -{ - if (decoded.native_handle != nullptr && decoded.release != nullptr) - { - decoded.release(decoded.native_handle); - } - decoded = {}; -} - void release_tile_payload(ui::map_tiles::MapTileAsyncEvent& event) { if (event.payload.data != nullptr) @@ -445,7 +421,6 @@ void release_tile_payload(ui::map_tiles::MapTileAsyncEvent& event) heap_caps_free(const_cast(event.payload.data)); } event.payload = {}; - release_tile_decoded_image(event.decoded); event.payload_size = 0; } @@ -812,33 +787,26 @@ class MapTileEventQueue final : public ui::map_tiles::IMapTileEventSink bool publish(const ui::map_tiles::MapTileAsyncEvent& event) override { ui::map_tiles::MapTileAsyncEvent owned = event; - if (owned.kind == ui::map_tiles::MapTileAsyncEventKind::Ready) + if (owned.kind == ui::map_tiles::MapTileAsyncEventKind::Ready && + event.payload.data != nullptr && + event.payload.size > 0) { - if (event.payload.data == nullptr || event.payload.size == 0) + uint8_t* payload = allocate_tile_payload(event.payload.size); + if (payload == nullptr) { - log_map_tile_event_failure("payload_missing", owned, -22); + log_map_tile_event_failure("payload_alloc", owned, -12); owned.kind = ui::map_tiles::MapTileAsyncEventKind::Failed; - owned.error = -22; - owned.payload = {}; - owned.payload_size = 0; - } - else if (lv_image_dsc_t* decoded = decode_payload_to_image_desc(event.tile, event.payload)) - { - owned.decoded.native_handle = decoded; - owned.decoded.release = release_decoded_image_handle; - owned.decoded.width = static_cast(decoded->header.w); - owned.decoded.height = static_cast(decoded->header.h); - owned.decoded.data_size = decoded->data_size; + owned.error = -12; owned.payload = {}; owned.payload_size = 0; } else { - log_map_tile_event_failure("decode_worker", owned, -12); - owned.kind = ui::map_tiles::MapTileAsyncEventKind::Failed; - owned.error = -12; - owned.payload = {}; - owned.payload_size = 0; + std::memcpy(payload, event.payload.data, event.payload.size); + owned.payload.data = payload; + owned.payload.size = event.payload.size; + owned.payload.format = event.payload.format; + owned.payload.ref = event.payload.ref; } } @@ -2374,23 +2342,21 @@ static void mark_missing_base_tile(TileContext& ctx, MapTile& tile) } } -static DecodedTileCache* adopt_decoded_image_to_cache( - TileContext& ctx, - const ui::map_tiles::MapTileRef& ref, - ui::map_tiles::MapTileDecodedImage& decoded) +static DecodedTileCache* decode_payload_to_cache(TileContext& ctx, + const ui::map_tiles::MapTileRef& ref, + const ui::map_tiles::MapTilePayload& payload) { + if (payload.data == nullptr || payload.size == 0) + { + return nullptr; + } + DecodedTileCache* cached = find_cached_tile_ref(ref); if (cached != nullptr && cached->img_dsc != NULL) { return cached; } - auto* img_dsc = static_cast(decoded.native_handle); - if (img_dsc == nullptr) - { - return nullptr; - } - refresh_live_tile_decode_cache_usage(ctx); DecodedTileCache* cache_slot = get_lru_cache_slot(tile_decode_cache_limit(ctx)); if (cache_slot == NULL && evict_invisible_cached_tile_object(ctx)) @@ -2403,6 +2369,12 @@ static DecodedTileCache* adopt_decoded_image_to_cache( return nullptr; } + lv_image_dsc_t* img_dsc = decode_payload_to_image_desc(ref, payload); + if (img_dsc == nullptr) + { + return nullptr; + } + cache_slot->img_dsc = img_dsc; cache_slot->x = static_cast(ref.x); cache_slot->y = static_cast(ref.y); @@ -2411,8 +2383,6 @@ static DecodedTileCache* adopt_decoded_image_to_cache( cache_slot->map_source = map_source_for_layer(ref.layer); cache_slot->last_used_ms = sys::millis_now(); cache_slot->lvgl_ref_count = 0; - - decoded = {}; return cache_slot; } @@ -2662,11 +2632,11 @@ static bool apply_map_tile_event(TileContext& ctx, ui::map_tiles::MapTileAsyncEv return true; } - DecodedTileCache* cache = adopt_decoded_image_to_cache(ctx, event.tile, event.decoded); + DecodedTileCache* cache = decode_payload_to_cache(ctx, event.tile, event.payload); if (cache == nullptr) { retry_not_before = now_ms + kMapTileLayerCacheBackoffMs; - log_map_tile_event_failure("decoded_image", event, event.error); + log_map_tile_event_failure("decode", event, event.error); release_tile_payload(event); return false; }