diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 989738ec..c73de39b 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,22 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 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 + retained static prepared-file worker; broader app task ownership and canvas + async worker ownership remain. +- 2026-06-16: `DEBT-0037` was narrowed again. `App::rec_loop()` in + `src/app.cpp` now routes its worker-iteration pointer lookup plus + `plan_recording_worker_iteration(...)` setup through a local helper instead + of carrying that setup inline; retained recording loop control, readback call + sites, and MP4 execution remain. +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` current-mode draw + callback setup now routes through + `make_legacy_canvas_draw_merge_current_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-0037` was narrowed again. `App::rec_loop()` in `src/app.cpp` now routes its coherent frame encode/update chunk through a local helper instead of carrying dirty-stroke clearing, equirect PBO diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index bd3abb16..ac6e2dac 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 callback setup now routed through the same - retained helper family. + shader/draw pass plus heightmap and current-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 @@ -134,10 +134,11 @@ Current architecture mismatches that must be treated as real blockers: coordination flags now use `std::atomic` instead of unsynchronized globals, 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 - prepared-file and canvas async workers now sit behind named retained local - worker-state helpers instead of bare static accessors, and `App::rec_loop()` - has a smaller local encode/update shell even though the retained recording - loop still owns the worker-side readback flow. + 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. - 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 93db09ff..6746793c 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -146,6 +146,9 @@ Current slice: - `NodeCanvas` heightmap draw callback setup now also routes through `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. - `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 @@ -347,6 +350,8 @@ Current slice: - `main.cpp` Win32 window handles, GL task/mutex state, splash-dialog state, stylus timers, and VR worker state now sit behind one retained local state 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` - retained `App` composition, task call sites, and platform/runtime entrypoint coupling are still not fully reduced behind the runtime contract @@ -404,12 +409,16 @@ 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 - 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, and `App::update()` no longer carries the dead update mutex residue, - but retained recording loop control and readback ownership are still open + 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 Write scope: - `src/canvas.cpp` diff --git a/src/app.cpp b/src/app.cpp index 613c62ad..05c0fccb 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -643,6 +643,29 @@ void App::rec_export(std::string path) namespace { + template + struct RecordingWorkerIterationContext + { + Canvas* legacy_canvas = nullptr; + CanvasDocument* canvas_document = nullptr; + CanvasEncoder* encoder = nullptr; + pp::app::RecordingWorkerIterationPlan plan{}; + }; + + template + RecordingWorkerIterationContext make_recording_worker_iteration_context(App& app) + { + RecordingWorkerIterationContext context; + context.legacy_canvas = Canvas::I; + context.canvas_document = app.canvas ? app.canvas->m_canvas.get() : nullptr; + context.encoder = context.legacy_canvas ? context.legacy_canvas->m_encoder.get() : nullptr; + context.plan = pp::app::plan_recording_worker_iteration( + app.rec_running, + context.encoder != nullptr, + context.legacy_canvas != nullptr && context.canvas_document != nullptr); + return context; + } + template void encode_recording_frame( App& app, @@ -676,18 +699,12 @@ void App::rec_loop() { std::unique_lock lock(rec_mutex); rec_cv.wait(lock/*, [this] { return !(rec_frames.empty() && rec_running); }*/); - auto* legacy_canvas = Canvas::I; - auto* canvas_document = canvas ? canvas->m_canvas.get() : nullptr; - auto* encoder = legacy_canvas ? legacy_canvas->m_encoder.get() : nullptr; - const auto plan = pp::app::plan_recording_worker_iteration( - rec_running, - encoder != nullptr, - legacy_canvas != nullptr && canvas_document != nullptr); - if (!plan.continue_running) + const auto iteration = make_recording_worker_iteration_context(*this); + if (!iteration.plan.continue_running) break; - if (plan.encode_frame && legacy_canvas && canvas_document && encoder) - encode_recording_frame(*this, plan, legacy_canvas, canvas_document, encoder); + 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_events.cpp b/src/app_events.cpp index cc378e3d..093e22cc 100644 --- a/src/app_events.cpp +++ b/src/app_events.cpp @@ -6,12 +6,7 @@ #include "app_core/document_sharing.h" #include "platform_api/platform_services.h" -#include -#include #include -#include -#include -#include #ifdef __LINUX__ #include @@ -22,88 +17,6 @@ namespace { -class LegacyPreparedFileWorker final { -public: - LegacyPreparedFileWorker() - : worker_([this](std::stop_token stop_token) { - run(stop_token); - }) - { - } - - ~LegacyPreparedFileWorker() - { - 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("prepared file worker task failed"); - } - } - } - } - - std::mutex mutex_; - std::condition_variable cv_; - std::deque> tasks_; - bool stopping_ = false; - std::jthread worker_; -}; - -struct RetainedPreparedFileWorkerState final { - void post(std::function task) - { - worker.post(std::move(task)); - } - - LegacyPreparedFileWorker worker; -}; - -RetainedPreparedFileWorkerState& retained_prepared_file_worker_state() -{ - static RetainedPreparedFileWorkerState state; - return state; -} - [[nodiscard]] GLint rgba8_internal_format() noexcept { return static_cast(pp::renderer::gl::rgba8_internal_format()); @@ -293,7 +206,7 @@ void App::pick_file_save(const std::string& type, const std::string& default_nam LOG("App::pick_file_save %s", target.path.c_str()); if (target.write_on_background_thread) { auto* app = this; - retained_prepared_file_worker_state().post([ + runtime_.prepared_file_task([ app, writer = std::move(writer), callback = std::move(callback), diff --git a/src/app_runtime.cpp b/src/app_runtime.cpp index 37017039..0e00f289 100644 --- a/src/app_runtime.cpp +++ b/src/app_runtime.cpp @@ -3,6 +3,19 @@ #include "app.h" +AppRuntime::AppRuntime() + : prepared_file_worker_([this](std::stop_token stop_token) + { + prepared_file_worker_main(stop_token); + }) +{ +} + +AppRuntime::~AppRuntime() +{ + prepared_file_worker_stop(); +} + bool AppRuntime::is_render_thread() const noexcept { return std::this_thread::get_id() == render_thread_id_; @@ -23,6 +36,62 @@ void AppRuntime::notify_ui_worker() noexcept ui_cv_.notify_all(); } +void AppRuntime::prepared_file_task(std::function task) +{ + { + std::lock_guard lock(prepared_file_task_mutex_); + if (!prepared_file_running_) + return; + prepared_file_tasklist_.push_back(std::move(task)); + } + prepared_file_cv_.notify_one(); +} + +void AppRuntime::prepared_file_worker_main(std::stop_token stop_token) +{ + for (;;) + { + std::function task; + { + std::unique_lock lock(prepared_file_task_mutex_); + prepared_file_cv_.wait(lock, [this, &stop_token] + { + return stop_token.stop_requested() || !prepared_file_running_ || !prepared_file_tasklist_.empty(); + }); + if ((stop_token.stop_requested() || !prepared_file_running_) && prepared_file_tasklist_.empty()) + break; + task = std::move(prepared_file_tasklist_.front()); + prepared_file_tasklist_.pop_front(); + } + + if (task) + { + try + { + task(); + } + catch (...) + { + LOG("prepared file worker task failed"); + } + } + } +} + +void AppRuntime::prepared_file_worker_stop() +{ + { + std::lock_guard lock(prepared_file_task_mutex_); + prepared_file_running_ = false; + } + prepared_file_cv_.notify_all(); + if (prepared_file_worker_.joinable()) + { + prepared_file_worker_.request_stop(); + prepared_file_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 ea4ee0c4..8c1349d4 100644 --- a/src/app_runtime.h +++ b/src/app_runtime.h @@ -34,6 +34,9 @@ struct AppTask : public std::packaged_task class AppRuntime { public: + AppRuntime(); + ~AppRuntime(); + [[nodiscard]] bool is_render_thread() const noexcept; [[nodiscard]] bool is_ui_thread() const noexcept; [[nodiscard]] bool request_redraw() const noexcept { return request_redraw_; } @@ -41,6 +44,7 @@ public: void notify_render_worker() noexcept; void notify_ui_worker() noexcept; + void prepared_file_task(std::function task); void render_thread_tick(App& app); void render_thread_main(App& app, std::stop_token stop_token); @@ -199,6 +203,15 @@ public: } private: + void prepared_file_worker_main(std::stop_token stop_token); + void prepared_file_worker_stop(); + + std::deque> prepared_file_tasklist_; + std::mutex prepared_file_task_mutex_; + std::condition_variable prepared_file_cv_; + std::jthread prepared_file_worker_; + bool prepared_file_running_ = true; + std::deque render_tasklist_; std::mutex render_task_mutex_; std::condition_variable render_cv_; diff --git a/src/legacy_canvas_draw_merge_services.h b/src/legacy_canvas_draw_merge_services.h index b161cbf5..5e114432 100644 --- a/src/legacy_canvas_draw_merge_services.h +++ b/src/legacy_canvas_draw_merge_services.h @@ -250,6 +250,19 @@ template }; } +template +[[nodiscard]] inline auto make_legacy_canvas_draw_merge_current_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 741c6e27..6565a7ef 100644 --- a/src/node_canvas.cpp +++ b/src/node_canvas.cpp @@ -770,10 +770,11 @@ void NodeCanvas::draw() mode->on_Draw(ortho_proj, proj, camera); }, .draw_heightmap = pp::panopainter::make_legacy_canvas_draw_merge_heightmap_draw(App::I->grid, proj, camera), - .draw_current_modes = [&] { - for (auto& mode : *m_canvas->m_mode) - mode->on_Draw(ortho_proj, proj, camera); - }, + .draw_current_modes = pp::panopainter::make_legacy_canvas_draw_merge_current_modes_draw( + m_canvas->m_mode, + ortho_proj, + proj, + camera), }); if (m_density != 1.f)