From 640ebc4be47c346ff74b877a8836e258222a5f7c Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 16 Jun 2026 08:16:59 +0200 Subject: [PATCH] Trim recording loop and retain async worker state --- docs/modernization/debt.md | 11 ++++++++ docs/modernization/roadmap.md | 6 ++++- docs/modernization/tasks.md | 6 +++++ src/app.cpp | 47 +++++++++++++++++++++-------------- src/app_events.cpp | 17 ++++++++++--- src/canvas.cpp | 37 +++++++++++++++++---------- 6 files changed, 87 insertions(+), 37 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 627c3db9..989738ec 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,17 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 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 + creation, the worker-side `yield`, `ImageRef` creation, encode, and frame + label refresh inline; retained recording loop control, readback call sites, + and MP4 execution remain. +- 2026-06-16: `DEBT-0003` was narrowed again. The prepared-file worker in + `src/app_events.cpp` and the canvas async worker in `src/canvas.cpp` now sit + behind named retained local worker-state helpers instead of bare static + worker accessors; retained app/canvas singleton reach and broader runtime + ownership remain. - 2026-06-16: `DEBT-0003` was narrowed again. `src/main.cpp` now keeps Win32 window handles, GL/task mutex state, stylus timers, splash-dialog state, and VR worker state behind one retained local state object instead of separate diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index f0f0213e..bd3abb16 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -133,7 +133,11 @@ Current architecture mismatches that must be treated as real blockers: ownership instead of raw global lifetime, and the Windows main-loop/VR 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. + 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. - 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 663073ea..93db09ff 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -401,9 +401,15 @@ Current slice: workers with explicit UI-thread handoff - canvas async import/export/save/open and timelapse export now also use owned worker queues instead of detached threads +- `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 - 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 Write scope: - `src/canvas.cpp` diff --git a/src/app.cpp b/src/app.cpp index 1e970407..613c62ad 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -473,11 +473,6 @@ void App::draw(float dt) void App::update(float dt) { - static std::mutex mutex; - - // avoid multiple threads to update the scene - //std::lock_guard lock(mutex); - const auto update_plan = pp::app::plan_app_frame_update(redraw, animate); if (!update_plan.update_frame) return; @@ -646,6 +641,33 @@ void App::rec_export(std::string path) LOG("Recording export action failed: %s", status.message); } +namespace +{ + template + void encode_recording_frame( + App& app, + pp::app::RecordingWorkerIterationPlan const& plan, + Canvas* legacy_canvas, + CanvasDocument* canvas_document, + CanvasEncoder* encoder) + { + if (plan.clear_dirty_stroke) + canvas_document->m_dirty_stroke = false; + + PBO equirect = legacy_canvas->m_layers_merge.gen_equirect_pbo(encoder->frame_size()); + std::this_thread::yield(); + + ImageRef img; + img.create(equirect.width, equirect.height, equirect.map()); + encoder->encode(img); + equirect.unmap(); + LOG("rec frame encoded"); + + if (plan.update_frame_label) + app.update_rec_frames(); + } +} + void App::rec_loop() { BT_SetTerminate(); @@ -665,20 +687,7 @@ void App::rec_loop() break; if (plan.encode_frame && legacy_canvas && canvas_document && encoder) - { - if (plan.clear_dirty_stroke) - canvas_document->m_dirty_stroke = false; - PBO equirect = legacy_canvas->m_layers_merge.gen_equirect_pbo( - encoder->frame_size()); - std::this_thread::yield(); - ImageRef img; - img.create(equirect.width, equirect.height, equirect.map()); - encoder->encode(img); - equirect.unmap(); - LOG("rec frame encoded"); - if (plan.update_frame_label) - update_rec_frames(); - } + encode_recording_frame(*this, plan, legacy_canvas, canvas_document, encoder); } } diff --git a/src/app_events.cpp b/src/app_events.cpp index c2f643e8..cc378e3d 100644 --- a/src/app_events.cpp +++ b/src/app_events.cpp @@ -89,10 +89,19 @@ private: std::jthread worker_; }; -LegacyPreparedFileWorker& prepared_file_worker() +struct RetainedPreparedFileWorkerState final { + void post(std::function task) + { + worker.post(std::move(task)); + } + + LegacyPreparedFileWorker worker; +}; + +RetainedPreparedFileWorkerState& retained_prepared_file_worker_state() { - static LegacyPreparedFileWorker worker; - return worker; + static RetainedPreparedFileWorkerState state; + return state; } [[nodiscard]] GLint rgba8_internal_format() noexcept @@ -284,7 +293,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; - prepared_file_worker().post([ + retained_prepared_file_worker_state().post([ app, writer = std::move(writer), callback = std::move(callback), diff --git a/src/canvas.cpp b/src/canvas.cpp index aa51f2ea..5f3f583a 100644 --- a/src/canvas.cpp +++ b/src/canvas.cpp @@ -102,10 +102,21 @@ private: std::jthread worker_; }; -LegacyCanvasAsyncWorker& canvas_async_worker() +class RetainedCanvasAsyncWorkerState final { +public: + void post(std::function task) + { + worker_.post(std::move(task)); + } + +private: + LegacyCanvasAsyncWorker worker_; +}; + +RetainedCanvasAsyncWorkerState& canvas_async_worker_state() { - static LegacyCanvasAsyncWorker worker; - return worker; + static RetainedCanvasAsyncWorkerState state; + return state; } GLint current_canvas_stroke_internal_format() @@ -2846,7 +2857,7 @@ void Canvas::import_equirectangular(std::string file_path, std::shared_ptrcheck_license()) { - canvas_async_worker().post([this, file_path = std::move(file_path), layer = std::move(layer)] { + canvas_async_worker_state().post([this, file_path = std::move(file_path), layer = std::move(layer)] { BT_SetTerminate(); import_equirectangular_thread(file_path, layer); }); @@ -2938,7 +2949,7 @@ void Canvas::export_equirectangular(std::string file_path, std::function { if (App::I->check_license()) { - canvas_async_worker().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_equirectangular_thread(file_path); if (on_complete) @@ -3024,7 +3035,7 @@ void Canvas::export_depth(std::string file_name, std::function on_comple { if (App::I->check_license()) { - canvas_async_worker().post([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_depth_thread(file_name); if (on_complete) @@ -3131,7 +3142,7 @@ void Canvas::export_layers(std::string path, std::function on_complete) { if (App::I->check_license()) { - canvas_async_worker().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_layers_thread(path); if (on_complete) @@ -3157,7 +3168,7 @@ void Canvas::export_anim_frames(std::string path, std::function on_compl { if (App::I->check_license()) { - canvas_async_worker().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_anim_frames_thread(path); if (on_complete) @@ -3182,7 +3193,7 @@ void Canvas::export_anim_mp4(std::string path, std::function on_complete { if (App::I->check_license()) { - canvas_async_worker().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, path = std::move(path), on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); export_anim_mp4_thread(path); if (on_complete) @@ -3220,7 +3231,7 @@ void Canvas::export_cube_faces(std::string file_name, std::function on_c { if (App::I->check_license()) { - canvas_async_worker().post([this, file_name = std::move(file_name), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([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) @@ -3273,7 +3284,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().post([this, file_path, on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([this, file_path, on_complete = std::move(on_complete)]() mutable { BT_SetTerminate(); bool ret = project_save_thread(file_path, true); if (on_complete) @@ -3287,7 +3298,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().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([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) @@ -3568,7 +3579,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().post([this, file_path = std::move(file_path), on_complete = std::move(on_complete)]() mutable { + canvas_async_worker_state().post([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)