From 52f0d32612646d1a2176c95063163a355ec77b00 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 16 Jun 2026 08:33:16 +0200 Subject: [PATCH] Move canvas async work into app runtime --- docs/modernization/debt.md | 15 ++++ docs/modernization/roadmap.md | 13 +-- docs/modernization/tasks.md | 21 +++-- src/app.cpp | 25 ++++-- src/app_runtime.cpp | 61 ++++++++++++++ src/app_runtime.h | 9 ++ src/canvas.cpp | 104 +++--------------------- src/legacy_canvas_draw_merge_services.h | 13 +++ src/node_canvas.cpp | 9 +- 9 files changed, 151 insertions(+), 119 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index c73de39b..97ec36dc 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,21 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 2026-06-16: `DEBT-0003` was narrowed again. Canvas async + import/export/save/open background work now runs through an + `AppRuntime`-owned queue/worker in `src/app_runtime.h/.cpp`, and + `src/canvas.cpp` no longer defines a retained static canvas async worker; + broader app task ownership and retained runtime singleton reach remain. +- 2026-06-16: `DEBT-0037` was narrowed again. `App::rec_loop()` in + `src/app.cpp` now routes its wait plus iteration plan/encode shell through a + local helper instead of carrying that orchestration inline; retained + recording loop control, readback call sites, and MP4 execution remain. +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` grid-mode draw + callback setup now routes through + `make_legacy_canvas_draw_merge_grid_modes_draw(...)` in + `src/legacy_canvas_draw_merge_services.h` instead of keeping that callback + loop inline in `NodeCanvas::draw()`; broader canvas draw orchestration and + retained GL resource ownership remain. - 2026-06-16: `DEBT-0003` was narrowed again. Prepared-file background work now runs through an `AppRuntime`-owned queue/worker in `src/app_runtime.h/.cpp`, and `src/app_events.cpp` no longer defines a diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index ac6e2dac..49cbc904 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -117,8 +117,8 @@ Current architecture mismatches that must be treated as real blockers: checkerboard background setup now route through retained draw-merge helpers, with the cache-to-screen checkerboard-plane callback setup also reduced and the merged-path per-plane merged-texture draw plus the smoothing-mask face - shader/draw pass plus heightmap and current-mode callback setup now routed - through the same retained helper family. + shader/draw pass plus heightmap, current-mode, and grid-mode callback setup + now routed through the same retained helper family. - `app_layout.cpp` and `app_dialogs.cpp` are still mixed shell/controller files rather than thin composition/binding surfaces. - `App`, `Canvas`, `Node`, retained workers, and platform entrypoints still use @@ -135,10 +135,11 @@ Current architecture mismatches that must be treated as real blockers: while the main Win32 entrypoint now groups window/GL/task/VR state behind a retained local state object instead of separate process-wide globals, the canvas async worker now sits behind a named retained local worker-state - helper instead of a bare static accessor, the prepared-file worker now lives - under `AppRuntime` instead of a retained static app-events worker, and - `App::rec_loop()` has a smaller local iteration/encode shell even though the - retained recording loop still owns the worker-side readback flow. + helper instead of a bare static accessor, the prepared-file worker and the + canvas async import/export/save/open worker now live under `AppRuntime` + instead of retained static app-events/canvas workers, and `App::rec_loop()` + now has a smaller local wait/iteration/encode shell even though the retained + recording loop still owns the worker-side readback flow. - Modern C++23 usage exists in extracted components, especially `std::span`, explicit result/status objects, and a few concepts, but the live app still does not consistently express ownership, thread affinity, or renderer diff --git a/docs/modernization/tasks.md b/docs/modernization/tasks.md index 6746793c..c269568e 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -147,8 +147,10 @@ Current slice: `make_legacy_canvas_draw_merge_heightmap_draw(...)`, but the node still owns current-mode traversal and broader post-draw orchestration. - `NodeCanvas` current-mode draw callback setup now also routes through - `make_legacy_canvas_draw_merge_current_modes_draw(...)`, but grid-mode - traversal and broader post-draw orchestration are still inline. + `make_legacy_canvas_draw_merge_current_modes_draw(...)`, and grid-mode + callback setup now also routes through + `make_legacy_canvas_draw_merge_grid_modes_draw(...)`, but broader post-draw + orchestration is still inline. - `NodeCanvas` smoothing-mask face shader setup plus per-face draw execution now also route through `execute_legacy_canvas_draw_merge_smask_faces(...)`, but the node still owns @@ -352,6 +354,9 @@ Current slice: object instead of separate file-scope globals - prepared-file background work now runs through an `AppRuntime`-owned worker queue instead of a retained static worker in `src/app_events.cpp` +- canvas async import/export/save/open background work now also runs through an + `AppRuntime`-owned worker queue instead of a retained static worker in + `src/canvas.cpp` - retained `App` composition, task call sites, and platform/runtime entrypoint coupling are still not fully reduced behind the runtime contract @@ -409,16 +414,18 @@ Current slice: - `src/app_events.cpp` prepared-file worker ownership and `src/canvas.cpp` async import/export/save/open worker ownership now also sit behind named retained local worker-state helpers instead of bare static worker accessors -- the prepared-file worker has now moved again into `AppRuntime`, removing the - retained static worker from `src/app_events.cpp`; the broader canvas async - worker still remains local because that slice is wider +- the prepared-file worker and the broader canvas async import/export/save/open + worker have now both moved into `AppRuntime`, removing the retained static + workers from `src/app_events.cpp` and `src/canvas.cpp` - preview background rendering, recording, and the retained `NodePanelGrid::bake_uvs()` worker now also use `std::jthread`, but their retained loop/control flow is still open - `App::rec_loop()` now routes its frame encode/update chunk through a local helper, its iteration-context setup now also routes through a local helper, - and `App::update()` no longer carries the dead update mutex residue, but - retained recording loop control and readback ownership are still open + and its wait/plan/encode iteration shell now also routes through a local + helper, while `App::update()` no longer carries the dead update mutex + residue; retained recording loop control and readback ownership are still + open Write scope: - `src/canvas.cpp` diff --git a/src/app.cpp b/src/app.cpp index 05c0fccb..fa0dc5fd 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -689,22 +689,31 @@ namespace if (plan.update_frame_label) app.update_rec_frames(); } + + template + bool process_recording_worker_iteration(App& app) + { + std::unique_lock lock(app.rec_mutex); + app.rec_cv.wait(lock); + + const auto iteration = make_recording_worker_iteration_context(app); + if (!iteration.plan.continue_running) + return false; + + if (iteration.plan.encode_frame && iteration.legacy_canvas && iteration.canvas_document && iteration.encoder) + encode_recording_frame(app, iteration.plan, iteration.legacy_canvas, iteration.canvas_document, iteration.encoder); + return true; + } } void App::rec_loop() { BT_SetTerminate(); rec_running = true; - while(rec_running) + while (rec_running) { - std::unique_lock lock(rec_mutex); - rec_cv.wait(lock/*, [this] { return !(rec_frames.empty() && rec_running); }*/); - const auto iteration = make_recording_worker_iteration_context(*this); - if (!iteration.plan.continue_running) + if (!process_recording_worker_iteration(*this)) break; - - if (iteration.plan.encode_frame && iteration.legacy_canvas && iteration.canvas_document && iteration.encoder) - encode_recording_frame(*this, iteration.plan, iteration.legacy_canvas, iteration.canvas_document, iteration.encoder); } } diff --git a/src/app_runtime.cpp b/src/app_runtime.cpp index 0e00f289..dec02294 100644 --- a/src/app_runtime.cpp +++ b/src/app_runtime.cpp @@ -8,11 +8,16 @@ AppRuntime::AppRuntime() { prepared_file_worker_main(stop_token); }) + , canvas_async_worker_([this](std::stop_token stop_token) + { + canvas_async_worker_main(stop_token); + }) { } AppRuntime::~AppRuntime() { + canvas_async_worker_stop(); prepared_file_worker_stop(); } @@ -47,6 +52,17 @@ void AppRuntime::prepared_file_task(std::function task) prepared_file_cv_.notify_one(); } +void AppRuntime::canvas_async_task(std::function task) +{ + { + std::lock_guard lock(canvas_async_task_mutex_); + if (!canvas_async_running_) + return; + canvas_async_tasklist_.push_back(std::move(task)); + } + canvas_async_cv_.notify_one(); +} + void AppRuntime::prepared_file_worker_main(std::stop_token stop_token) { for (;;) @@ -78,6 +94,37 @@ void AppRuntime::prepared_file_worker_main(std::stop_token stop_token) } } +void AppRuntime::canvas_async_worker_main(std::stop_token stop_token) +{ + for (;;) + { + std::function task; + { + std::unique_lock lock(canvas_async_task_mutex_); + canvas_async_cv_.wait(lock, [this, &stop_token] + { + return stop_token.stop_requested() || !canvas_async_running_ || !canvas_async_tasklist_.empty(); + }); + if ((stop_token.stop_requested() || !canvas_async_running_) && canvas_async_tasklist_.empty()) + break; + task = std::move(canvas_async_tasklist_.front()); + canvas_async_tasklist_.pop_front(); + } + + if (task) + { + try + { + task(); + } + catch (...) + { + LOG("canvas async worker task failed"); + } + } + } +} + void AppRuntime::prepared_file_worker_stop() { { @@ -92,6 +139,20 @@ void AppRuntime::prepared_file_worker_stop() } } +void AppRuntime::canvas_async_worker_stop() +{ + { + std::lock_guard lock(canvas_async_task_mutex_); + canvas_async_running_ = false; + } + canvas_async_cv_.notify_all(); + if (canvas_async_worker_.joinable()) + { + canvas_async_worker_.request_stop(); + canvas_async_worker_.join(); + } +} + void AppRuntime::render_thread_tick(App& app) { static uint32_t count = 0; diff --git a/src/app_runtime.h b/src/app_runtime.h index 8c1349d4..e6313bb3 100644 --- a/src/app_runtime.h +++ b/src/app_runtime.h @@ -45,6 +45,7 @@ public: void notify_render_worker() noexcept; void notify_ui_worker() noexcept; void prepared_file_task(std::function task); + void canvas_async_task(std::function task); void render_thread_tick(App& app); void render_thread_main(App& app, std::stop_token stop_token); @@ -205,6 +206,8 @@ public: private: void prepared_file_worker_main(std::stop_token stop_token); void prepared_file_worker_stop(); + void canvas_async_worker_main(std::stop_token stop_token); + void canvas_async_worker_stop(); std::deque> prepared_file_tasklist_; std::mutex prepared_file_task_mutex_; @@ -212,6 +215,12 @@ private: std::jthread prepared_file_worker_; bool prepared_file_running_ = true; + std::deque> canvas_async_tasklist_; + std::mutex canvas_async_task_mutex_; + std::condition_variable canvas_async_cv_; + std::jthread canvas_async_worker_; + bool canvas_async_running_ = true; + std::deque render_tasklist_; std::mutex render_task_mutex_; std::condition_variable render_cv_; diff --git a/src/canvas.cpp b/src/canvas.cpp index 5f3f583a..52df7180 100644 --- a/src/canvas.cpp +++ b/src/canvas.cpp @@ -35,90 +35,6 @@ namespace { -class LegacyCanvasAsyncWorker final { -public: - LegacyCanvasAsyncWorker() - : worker_([this](std::stop_token stop_token) { - run(stop_token); - }) - { - } - - ~LegacyCanvasAsyncWorker() - { - shutdown(); - } - - void post(std::function task) - { - { - std::lock_guard lock(mutex_); - if (stopping_) - return; - tasks_.push_back(std::move(task)); - } - cv_.notify_one(); - } - -private: - void shutdown() - { - { - std::lock_guard lock(mutex_); - stopping_ = true; - } - cv_.notify_all(); - } - - void run(std::stop_token stop_token) - { - for (;;) { - std::function task; - { - std::unique_lock lock(mutex_); - cv_.wait(lock, [&] { - return stopping_ || stop_token.stop_requested() || !tasks_.empty(); - }); - if ((stopping_ || stop_token.stop_requested()) && tasks_.empty()) - break; - task = std::move(tasks_.front()); - tasks_.pop_front(); - } - - if (task) { - try { - task(); - } catch (...) { - LOG("canvas async worker task failed"); - } - } - } - } - - std::mutex mutex_; - std::condition_variable cv_; - std::deque> tasks_; - bool stopping_ = false; - std::jthread worker_; -}; - -class RetainedCanvasAsyncWorkerState final { -public: - void post(std::function task) - { - worker_.post(std::move(task)); - } - -private: - LegacyCanvasAsyncWorker worker_; -}; - -RetainedCanvasAsyncWorkerState& canvas_async_worker_state() -{ - static RetainedCanvasAsyncWorkerState state; - return state; -} - GLint current_canvas_stroke_internal_format() { const auto renderer_features = ShaderManager::render_device_features(); @@ -2857,7 +2773,7 @@ void Canvas::import_equirectangular(std::string file_path, std::shared_ptrcheck_license()) { - canvas_async_worker_state().post([this, file_path = std::move(file_path), layer = std::move(layer)] { + App::I->runtime().canvas_async_task([this, file_path = std::move(file_path), layer = std::move(layer)] { BT_SetTerminate(); import_equirectangular_thread(file_path, layer); }); @@ -2949,7 +2865,7 @@ void Canvas::export_equirectangular(std::string file_path, std::function { if (App::I->check_license()) { - canvas_async_worker_state().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_equirectangular_thread(file_path); if (on_complete) @@ -3035,7 +2951,7 @@ void Canvas::export_depth(std::string file_name, std::function on_comple { if (App::I->check_license()) { - canvas_async_worker_state().post([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_depth_thread(file_name); if (on_complete) @@ -3142,7 +3058,7 @@ void Canvas::export_layers(std::string path, std::function on_complete) { if (App::I->check_license()) { - canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_layers_thread(path); if (on_complete) @@ -3168,7 +3084,7 @@ void Canvas::export_anim_frames(std::string path, std::function on_compl { if (App::I->check_license()) { - canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_anim_frames_thread(path); if (on_complete) @@ -3193,7 +3109,7 @@ void Canvas::export_anim_mp4(std::string path, std::function on_complete { if (App::I->check_license()) { - canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_anim_mp4_thread(path); if (on_complete) @@ -3231,7 +3147,7 @@ void Canvas::export_cube_faces(std::string file_name, std::function on_c { if (App::I->check_license()) { - canvas_async_worker_state().post([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_cube_faces_thread(file_name); if (on_complete) @@ -3284,7 +3200,7 @@ void Canvas::project_save(std::function on_complete) if (App::I->check_license()) { const auto file_path = App::I->doc_path; - canvas_async_worker_state().post([this, file_path, on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_path, on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); bool ret = project_save_thread(file_path, true); if (on_complete) @@ -3298,7 +3214,7 @@ void Canvas::project_save(std::string file_path, std::function on_co LOG("saving %s", file_path.c_str()); if (App::I->check_license()) { - canvas_async_worker_state().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); bool ret = project_save_thread(file_path, true); if (on_complete) @@ -3579,7 +3495,7 @@ bool Canvas::project_save_thread(std::string file_path, bool show_progress) void Canvas::project_open(std::string file_path, std::function on_complete) { - canvas_async_worker_state().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + App::I->runtime().canvas_async_task([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); bool result = project_open_thread(file_path); if (on_complete) diff --git a/src/legacy_canvas_draw_merge_services.h b/src/legacy_canvas_draw_merge_services.h index 5e114432..a0797278 100644 --- a/src/legacy_canvas_draw_merge_services.h +++ b/src/legacy_canvas_draw_merge_services.h @@ -263,6 +263,19 @@ template }; } +template +[[nodiscard]] inline auto make_legacy_canvas_draw_merge_grid_modes_draw( + ModesT* modes, + const glm::mat4& ortho_proj, + const glm::mat4& proj, + const glm::mat4& camera) +{ + return [modes, ortho_proj, proj, camera] { + for (auto& mode : *modes) + mode->on_Draw(ortho_proj, proj, camera); + }; +} + struct LegacyCanvasDrawMergeSmaskFacesExecution { std::function set_active_texture_unit; std::function enable_blend; diff --git a/src/node_canvas.cpp b/src/node_canvas.cpp index 6565a7ef..c370384d 100644 --- a/src/node_canvas.cpp +++ b/src/node_canvas.cpp @@ -765,10 +765,11 @@ void NodeCanvas::draw() }, }); }, - .draw_grid_modes = [&] { - for (auto& mode : Canvas::modes[(int)kCanvasMode::Grid]) - mode->on_Draw(ortho_proj, proj, camera); - }, + .draw_grid_modes = pp::panopainter::make_legacy_canvas_draw_merge_grid_modes_draw( + &Canvas::modes[(int)kCanvasMode::Grid], + ortho_proj, + proj, + camera), .draw_heightmap = pp::panopainter::make_legacy_canvas_draw_merge_heightmap_draw(App::I->grid, proj, camera), .draw_current_modes = pp::panopainter::make_legacy_canvas_draw_merge_current_modes_draw( m_canvas->m_mode,