From b7b39a16df7642ce40ac728656f88aeede976285 Mon Sep 17 00:00:00 2001 From: "torlando-agent[bot]" <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:02:03 +0000 Subject: [PATCH] fix: keep navigation responsive during style activation --- .../Hardware/TDeck/MapStyleCatalog.cpp | 7 ++- lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.h | 7 ++- lib/tdeck_ui/UI/LXMF/MapScreen.cpp | 43 ++++++++++++++----- lib/tdeck_ui/UI/LXMF/MapScreen.h | 8 ++++ .../build_scripts/test_map_screen_contract.py | 13 ++++-- tests/native/test_map_style_catalog.cpp | 16 +++++++ tests/native/test_map_style_catalog.py | 2 +- 7 files changed, 80 insertions(+), 16 deletions(-) diff --git a/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.cpp b/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.cpp index b1677525..3d586b74 100644 --- a/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.cpp +++ b/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.cpp @@ -185,7 +185,9 @@ MapStyleCatalogResult MapStyleCatalog::discover() { } MapStyleCatalogResult MapStyleCatalog::activate(std::uint32_t expected_catalog_generation, - const char* style_id) { + const char* style_id, + BeginCommitCallback begin_commit, + void* commit_context) { if (expected_catalog_generation != catalog_generation_) return MapStyleCatalogResult::STALE_CATALOG; std::size_t style_index = 0U; if (!allowedIndex(style_id, style_index)) return MapStyleCatalogResult::UNKNOWN_STYLE; @@ -252,6 +254,9 @@ MapStyleCatalogResult MapStyleCatalog::activate(std::uint32_t expected_catalog_g } offset += written; } + if (begin_commit != NULL && !begin_commit(commit_context)) { + storage_.abortWrite(); return MapStyleCatalogResult::CANCELLED; + } if (storage_.commitWrite() != TileStoreResult::OK) { storage_.abortWrite(); return MapStyleCatalogResult::WRITE_FAILED; } diff --git a/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.h b/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.h index 44e832f3..fa3e2d6d 100644 --- a/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.h +++ b/lib/tdeck_ui/Hardware/TDeck/MapStyleCatalog.h @@ -23,6 +23,7 @@ enum class MapStyleCatalogResult : std::uint8_t { STALE_CATALOG, UNKNOWN_STYLE, GENERATION_EXHAUSTED, + CANCELLED, WRITE_FAILED, READBACK_MISMATCH }; @@ -39,6 +40,8 @@ struct MapStyleSummary { /** Fixed-allowlist PMAS discovery and redundant-slot transactional activation. */ class MapStyleCatalog { public: + typedef bool (*BeginCommitCallback)(void* context); + static const std::size_t MAX_STYLES = 4U; static const char ACTIVE_SLOT_0_PATH[]; static const char ACTIVE_SLOT_1_PATH[]; @@ -48,7 +51,9 @@ public: MapStyleCatalogResult discover(); MapStyleCatalogResult activate(std::uint32_t expected_catalog_generation, - const char* style_id); + const char* style_id, + BeginCommitCallback begin_commit = NULL, + void* commit_context = NULL); std::size_t count() const { return count_; } const MapStyleSummary* style(std::size_t index) const { diff --git a/lib/tdeck_ui/UI/LXMF/MapScreen.cpp b/lib/tdeck_ui/UI/LXMF/MapScreen.cpp index 9037a393..3826d078 100644 --- a/lib/tdeck_ui/UI/LXMF/MapScreen.cpp +++ b/lib/tdeck_ui/UI/LXMF/MapScreen.cpp @@ -386,6 +386,24 @@ void MapScreen::publishStyleCatalog(std::uint32_t activation_token, unlockState(); } +bool MapScreen::beginStyleActivationCommit(void* raw_context) { + StyleActivationCommitContext* context = + static_cast(raw_context); + if (context == NULL || context->screen == NULL) return false; + MapScreen* screen = context->screen; + if (!screen->lockState(portMAX_DELAY)) return false; + const bool admitted = + screen->screen_visible_.load(std::memory_order_acquire) && + !screen->style_lifecycle_exhausted_ && + context->lifecycle_epoch == screen->style_lifecycle_epoch_ && + screen->style_selector_.activationOwnedBy(context->token); + // This is the lifecycle linearization point. Release before commitWrite() + // so LVGL navigation never waits on SD I/O. A hide that wins this lock + // rejects publication; a commit admitted first represents visible intent. + screen->unlockState(); + return admitted; +} + void MapScreen::workerLoop() { std::uint32_t handled_pack_refresh_epoch = pack_refresh_epoch_.load(std::memory_order_acquire); @@ -448,28 +466,33 @@ void MapScreen::workerLoop() { unlockState(); } if (have_style_request) { - Hardware::TDeck::MapStyleCatalogResult activation = - Hardware::TDeck::MapStyleCatalogResult::ACTIVE_IO_INDETERMINATE; bool activation_admitted = false; - // Linearize durable activation with hide(). If hide wins this lock, - // the stale request is cancelled without touching the active slots. - // If activation wins, hide cannot mark the screen hidden until the - // bounded SD commit has completed. if (lockState(portMAX_DELAY)) { activation_admitted = screen_visible_.load(std::memory_order_acquire) && !style_lifecycle_exhausted_ && style_request_lifecycle_epoch == style_lifecycle_epoch_ && style_selector_.activationOwnedBy(style_request.token); - if (activation_admitted) { - activation = style_catalog_.activate( - style_request.catalog_generation, style_request.style_id); - } else { + if (!activation_admitted) { (void)style_selector_.cancelPending(style_request.token); } unlockState(); } if (!activation_admitted) continue; + StyleActivationCommitContext commit_context = { + this, style_request.token, style_request_lifecycle_epoch}; + const Hardware::TDeck::MapStyleCatalogResult activation = + style_catalog_.activate(style_request.catalog_generation, + style_request.style_id, + &MapScreen::beginStyleActivationCommit, + &commit_context); + if (activation == Hardware::TDeck::MapStyleCatalogResult::CANCELLED) { + if (lockState(portMAX_DELAY)) { + (void)style_selector_.cancelPending(style_request.token); + unlockState(); + } + continue; + } bool success = activation == Hardware::TDeck::MapStyleCatalogResult::OK; if (success) { const Hardware::TDeck::MapTilePackResult initialized = pack_.initialize(); diff --git a/lib/tdeck_ui/UI/LXMF/MapScreen.h b/lib/tdeck_ui/UI/LXMF/MapScreen.h index de8aecd3..dd447a5f 100644 --- a/lib/tdeck_ui/UI/LXMF/MapScreen.h +++ b/lib/tdeck_ui/UI/LXMF/MapScreen.h @@ -56,6 +56,14 @@ public: bool applyOneCompletion(); private: + struct StyleActivationCommitContext { + MapScreen* screen; + std::uint32_t token; + std::uint32_t lifecycle_epoch; + }; + + static bool beginStyleActivationCommit(void* context); + lv_obj_t* screen_; lv_obj_t* toolbar_; lv_obj_t* viewport_; diff --git a/tests/build_scripts/test_map_screen_contract.py b/tests/build_scripts/test_map_screen_contract.py index 83d3f174..84df2a0b 100644 --- a/tests/build_scripts/test_map_screen_contract.py +++ b/tests/build_scripts/test_map_screen_contract.py @@ -193,11 +193,18 @@ def test_style_switch_is_bounded_and_worker_owned(): activation = worker.index("style_catalog_.activate") admission_lock = worker.rindex("lockState(portMAX_DELAY)", 0, activation) admission_guard = worker.rindex("activation_admitted =", 0, activation) - admission_unlock = worker.index("unlockState();", activation) - assert admission_lock < admission_guard < activation < admission_unlock + admission_unlock = worker.rindex("unlockState();", 0, activation) + assert admission_lock < admission_guard < admission_unlock < activation assert "style_request_lifecycle_epoch == style_lifecycle_epoch_" in worker[admission_guard:activation] assert "style_selector_.activationOwnedBy(style_request.token)" in worker[admission_guard:activation] - assert "if (!activation_admitted) continue;" in worker[activation:] + assert "if (!activation_admitted) continue;" in worker[:activation] + assert "&MapScreen::beginStyleActivationCommit" in worker + commit_guard = function_body(source, "bool MapScreen::beginStyleActivationCommit(void* raw_context)") + assert "lockState(portMAX_DELAY)" in commit_guard + assert "screen_visible_.load" in commit_guard + assert "lifecycle_epoch == screen->style_lifecycle_epoch_" in commit_guard + assert "activationOwnedBy(context->token)" in commit_guard + assert commit_guard.index("screen->unlockState()") < commit_guard.index("return admitted") committed_reload = worker.index("pack_.initialize()", activation) cache_invalidation = worker.index("decoded_tile_cache_.clear()", committed_reload) selector_completion = worker.index("style_selector_.complete", committed_reload) diff --git a/tests/native/test_map_style_catalog.cpp b/tests/native/test_map_style_catalog.cpp index 56612038..cdcf7f11 100644 --- a/tests/native/test_map_style_catalog.cpp +++ b/tests/native/test_map_style_catalog.cpp @@ -26,6 +26,7 @@ void fail(const char* expression, int line) { } #define CHECK(expression) do { if (!(expression)) fail(#expression, __LINE__); } while (false) void beginTest() { ++tests_run; } +bool rejectCommit(void*) { return false; } std::uint32_t crc32(const std::uint8_t* input, std::size_t length) { std::uint32_t crc = UINT32_C(0xffffffff); @@ -290,6 +291,20 @@ void testStaleAndUnknownActivationAreRejectedWithoutWrite() { CHECK(catalog.activate(generation, "rogue") == MapStyleCatalogResult::UNKNOWN_STYLE); CHECK(storage.find(MapStyleCatalog::ACTIVE_SLOT_1_PATH) == NULL); } +void testCancellationAbortsBeforePublishingActiveSlot() { + beginTest(); FakeStorage storage; addStyle(storage, "osm-bright"); + addStyle(storage, "dark-matter"); addActive(storage, 0U, "osm-bright", 4U); + MapStyleCatalog catalog(storage); CHECK(catalog.discover() == MapStyleCatalogResult::OK); + CHECK(catalog.activate(catalog.generation(), "dark-matter", &rejectCommit, + NULL) == MapStyleCatalogResult::CANCELLED); + CHECK(storage.abort_calls == 1U); + File* target = storage.find(MapStyleCatalog::ACTIVE_SLOT_1_PATH); + CHECK(target != NULL && target->size == 0U); + File* active = storage.find(MapStyleCatalog::ACTIVE_SLOT_0_PATH); + ActiveMapSetView view = {}; + CHECK(active != NULL && ActiveMapSetCodec::decode(active->bytes, active->size, view)); + CHECK(std::strcmp(view.map_set_id, "osm-bright") == 0); +} void testActivationTargetsMissingThenOlderSlotAndVerifiesBytes() { beginTest(); FakeStorage storage; addStyle(storage, "osm-bright"); addStyle(storage, "dark-matter"); addActive(storage, 0U, "osm-bright", 4U); MapStyleCatalog catalog(storage); @@ -381,6 +396,7 @@ int main() { testDiscoveryRejectsMismatchedAndMalformedStyleRecords(); testMissingCurrentRecordIsSynthesizedWithoutWrite(); testStaleAndUnknownActivationAreRejectedWithoutWrite(); + testCancellationAbortsBeforePublishingActiveSlot(); testActivationTargetsMissingThenOlderSlotAndVerifiesBytes(); testActivationSemanticallyValidatesBeforeAnySlotWrite(); testActivationRejectsIndeterminateConflictAndExhaustion(); diff --git a/tests/native/test_map_style_catalog.py b/tests/native/test_map_style_catalog.py index 0a2b5bde..9ab03c1e 100644 --- a/tests/native/test_map_style_catalog.py +++ b/tests/native/test_map_style_catalog.py @@ -35,4 +35,4 @@ def test_map_style_catalog(tmp_path: Path, sanitize: bool) -> None: env["UBSAN_OPTIONS"] = "halt_on_error=1:print_stacktrace=1" ran = subprocess.run([str(binary)], capture_output=True, text=True, timeout=60, env=env) assert ran.returncode == 0, ran.stdout + ran.stderr - assert ran.stdout == "map style catalog: 11 tests passed\n" + assert ran.stdout == "map style catalog: 12 tests passed\n"