From ad76aeb751fe764b8cd1f64e0e339fdc60a9ea8b Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 16 Jun 2026 08:57:15 +0200 Subject: [PATCH] Trim main task queue, recording label, and canvas draw callbacks --- docs/modernization/debt.md | 16 ++++++ docs/modernization/roadmap.md | 19 +++++--- docs/modernization/tasks.md | 9 ++++ src/app.cpp | 10 +--- src/legacy_canvas_draw_merge_services.h | 20 +++++++- src/legacy_recording_services.cpp | 27 ++++++++-- src/main.cpp | 65 ++++++++++++++++--------- src/node_canvas.cpp | 14 +++--- 8 files changed, 130 insertions(+), 50 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 60d8ca7c..5c40b39f 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. `main.cpp` main-thread queued + task state now lives behind a narrow retained helper instead of + `RetainedState.main_tasklist` / `main_task_mutex` directly; broader Win32 + bootstrap/runtime ownership remains. +- 2026-06-16: `DEBT-0037` was narrowed again. `App::update_rec_frames()` in + `src/app.cpp` now delegates recording label refresh through + `update_legacy_recording_frame_label(...)` in + `src/legacy_recording_services.cpp` instead of building the label directly; + retained recording label lookup, encoder-state reads, and MP4 execution + remain. +- 2026-06-16: `DEBT-0036` was narrowed again. `NodeCanvas` non-`draw_merged` + per-frame layer draw callback setup now routes through + `make_legacy_canvas_draw_merge_layer_frame_draw(...)` in + `src/legacy_canvas_draw_merge_services.h` instead of keeping that callback + body inline in `NodeCanvas::draw()`; broader canvas draw orchestration and + retained GL resource ownership remain. - 2026-06-16: `DEBT-0003` was narrowed again. The Win32 async GL/context lock state now lives in `src/platform_windows/windows_platform_services.cpp` instead of `src/main.cpp` retained state, and `main.cpp` only seeds the diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 6293bcda..38dc26fd 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -117,9 +117,10 @@ 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 checkerboard background-plane callback plus per-plane - merged-texture draw callback plus the smoothing-mask face shader/draw pass - plus heightmap, current-mode, and grid-mode callback setup now routed - through the same retained helper family. + merged-texture draw callback plus non-`draw_merged` per-frame layer draw + callback plus the smoothing-mask face 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 @@ -137,13 +138,17 @@ Current architecture mismatches that must be treated as real blockers: retained local state object instead of separate process-wide globals, the Win32 async GL/context lock state now lives under `src/platform_windows/windows_platform_services.cpp` instead of `main.cpp` - retained state, the canvas async worker now sits behind a named retained + retained state, the main-thread queued task state now sits behind a narrow + retained helper instead of `RetainedState.main_tasklist` / + `main_task_mutex`, the canvas async worker now sits behind a named retained local worker-state 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 delegates worker-iteration orchestration - into the retained recording bridge even though that retained recording path - still owns the worker-side readback flow. + workers, `App::rec_loop()` now delegates worker-iteration orchestration into + the retained recording bridge, and `App::update_rec_frames()` now delegates + recording label refresh through that same retained recording path even though + the bridge still owns worker-side readback flow and encoder-state label + reads. - 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 4ad8cb8f..a320124f 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -159,6 +159,10 @@ Current slice: also routes through `make_legacy_canvas_draw_merge_layer_texture_draw(...)`, but the node still owns broader live layer traversal and renderer-state sequencing. +- `NodeCanvas` non-`draw_merged` per-frame layer draw callback setup now also + routes through `make_legacy_canvas_draw_merge_layer_frame_draw(...)`, but + the node still owns broader live layer traversal and renderer-state + sequencing. - `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 @@ -364,6 +368,8 @@ Current slice: `src/platform_windows/windows_platform_services.cpp` instead of `main.cpp` retained state, and `main.cpp` only seeds that platform-owned context handle pair during initialization and context recreation +- `main.cpp` main-thread queued task state now sits behind a narrow retained + helper instead of `RetainedState.main_tasklist` / `main_task_mutex` directly - 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 @@ -436,6 +442,9 @@ Current slice: retained recording bridge in `src/legacy_recording_services.cpp`, while `App::update()` no longer carries the dead update mutex residue; retained recording loop control, readback ownership, and MP4 execution are still open +- `App::update_rec_frames()` now delegates recording label refresh through + `src/legacy_recording_services.cpp`, but retained recording label lookup, + encoder-state reads, and MP4 execution still stay on the legacy bridge Write scope: - `src/canvas.cpp` diff --git a/src/app.cpp b/src/app.cpp index 59f89b73..d7c96ec9 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -214,6 +214,7 @@ void App::initLog() namespace pp::panopainter { bool process_legacy_recording_worker_iteration(App& app); + void update_legacy_recording_frame_label(App& app); } bool App::check_license() @@ -566,14 +567,7 @@ void App::update_memory_usage(size_t bytes) void App::update_rec_frames() { - if (auto txt = layout[main_id]->find("txt-rec")) - { - const auto label = pp::app::make_recording_frame_label( - rec_running, - Canvas::I->m_encoder != nullptr, - Canvas::I->m_encoder ? Canvas::I->m_encoder->frames_count() : 0); - txt->set_text(label.text.c_str()); - } + pp::panopainter::update_legacy_recording_frame_label(*this); } int App::res_from_index(int i) diff --git a/src/legacy_canvas_draw_merge_services.h b/src/legacy_canvas_draw_merge_services.h index b3b5aea3..954ceabd 100644 --- a/src/legacy_canvas_draw_merge_services.h +++ b/src/legacy_canvas_draw_merge_services.h @@ -529,7 +529,25 @@ template rtt(plane_index).unbindTexture(); }, - }); + }); + }; +} + +template +[[nodiscard]] inline auto make_legacy_canvas_draw_merge_layer_frame_draw( + LayerT* layer, + FacePlaneT* face_plane, + SetActiveTextureUnit set_active_texture_unit, + int plane_index, + float layer_opacity) +{ + return [layer, face_plane, set_active_texture_unit, plane_index, layer_opacity](int frame, float onion_alpha) { + ShaderManager::u_float(kShaderUniform::Alpha, layer_opacity * onion_alpha); + set_active_texture_unit(0); + layer->rtt(plane_index, frame).bindTexture(); + face_plane->draw_fill(); + set_active_texture_unit(0); + layer->rtt(plane_index, frame).unbindTexture(); }; } diff --git a/src/legacy_recording_services.cpp b/src/legacy_recording_services.cpp index eb21b6ef..ae7ca39b 100644 --- a/src/legacy_recording_services.cpp +++ b/src/legacy_recording_services.cpp @@ -4,11 +4,14 @@ #include "app.h" #include "canvas.h" +#include "app_core/app_status.h" #include "legacy_app_dialog_services.h" #include "legacy_ui_overlay_services.h" #include "node_progress_bar.h" namespace pp::panopainter { +void update_legacy_recording_frame_label(App& app); + namespace { pp::app::RecordingWorkerIterationPlan make_recording_worker_iteration_plan(App& app) @@ -42,9 +45,25 @@ void encode_recording_frame( LOG("rec frame encoded"); if (plan.update_frame_label) - app.update_rec_frames(); + update_legacy_recording_frame_label(app); } +} // namespace + +void update_legacy_recording_frame_label(App& app) +{ + if (auto txt = app.layout[app.main_id]->find("txt-rec")) + { + const auto label = pp::app::make_recording_frame_label( + app.rec_running, + Canvas::I->m_encoder != nullptr, + Canvas::I->m_encoder ? Canvas::I->m_encoder->frames_count() : 0); + txt->set_text(label.text.c_str()); + } +} + +namespace { + class LegacyRecordingServices final : public pp::app::RecordingServices { public: explicit LegacyRecordingServices(App& app) noexcept @@ -54,7 +73,7 @@ public: void start_thread() override { - app_.update_rec_frames(); + update_legacy_recording_frame_label(app_); app_.rec_thread = std::jthread(&App::rec_loop, &app_); } @@ -65,7 +84,7 @@ public: app_.rec_cv.notify_all(); if (app_.rec_thread.joinable()) app_.rec_thread.join(); - app_.update_rec_frames(); + update_legacy_recording_frame_label(app_); } void delete_recorded_files() override @@ -80,7 +99,7 @@ public: void update_frame_label() override { - app_.update_rec_frames(); + update_legacy_recording_frame_label(app_); } void begin_export(int progress_total) override diff --git a/src/main.cpp b/src/main.cpp index ffc5eeb5..17ebc0ba 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -54,8 +54,6 @@ struct RetainedState std::map vkey_map; wchar_t window_title[512]{}; std::jthread hmd_renderer; - std::deque> main_tasklist; - std::mutex main_task_mutex; float timer_stylus = 0; float timer_ink_touch = 0; float timer_ink_pen = 0; @@ -72,6 +70,46 @@ RetainedState& retained_state() return state; } +namespace { + +struct RetainedMainTaskQueue final +{ + std::deque> tasklist; + std::mutex task_mutex; +}; + +RetainedMainTaskQueue& retained_main_task_queue() +{ + static RetainedMainTaskQueue queue; + return queue; +} + +template +void enqueue_main_task(Callable&& task) +{ + auto& queue = retained_main_task_queue(); + std::lock_guard lock(queue.task_mutex); + queue.tasklist.emplace_back(std::forward(task)); +} + +void drain_main_tasks() +{ + std::deque> working_list; + { + auto& queue = retained_main_task_queue(); + std::lock_guard lock(queue.task_mutex); + working_list = std::move(queue.tasklist); + } + + while (!working_list.empty()) + { + working_list.front()(); + working_list.pop_front(); + } +} + +} + std::atomic vr_frames{0}; std::atomic running{-1}; std::atomic_bool vr_running{false}; @@ -163,8 +201,7 @@ std::string GetLastErrorAsString() void destroy_window() { auto& state = retained_state(); - std::lock_guard lock(state.main_task_mutex); - state.main_tasklist.emplace_back([hWnd = state.hWnd] { + enqueue_main_task([hWnd = state.hWnd] { PostMessage(hWnd, WM_USER_CLOSE, 0, 0); }); } @@ -225,8 +262,7 @@ void win32_update_fps(int frames) swprintf_s(title_fps, L"%s - %d fps", state.window_title, frames); { - std::lock_guard lock(state.main_task_mutex); - state.main_tasklist.emplace_back([hWnd = state.hWnd] { + enqueue_main_task([hWnd = state.hWnd] { SetWindowText(hWnd, title_fps); }); } @@ -991,22 +1027,7 @@ int main(int argc, char** argv) } // list of tasks for the main thread - { - std::deque> working_list; - { - std::lock_guard lock(state.main_task_mutex); - working_list = std::move(state.main_tasklist); - } - - if (!working_list.empty()) - { - while (!working_list.empty()) - { - working_list.front()(); - working_list.pop_front(); - } - } - } + drain_main_tasks(); } // Clean up WacomTablet::I.terminate(); diff --git a/src/node_canvas.cpp b/src/node_canvas.cpp index e5636a68..7debf28b 100644 --- a/src/node_canvas.cpp +++ b/src/node_canvas.cpp @@ -469,14 +469,12 @@ void NodeCanvas::draw() m_canvas->m_plane_transform[plane_index] * glm::translate(glm::vec3(0, 0, -1)); - const auto draw_layer_frame = [&](int frame, float onion_alpha) { - ShaderManager::u_float(kShaderUniform::Alpha, m_canvas->m_layers[layer_index]->m_opacity * onion_alpha); - set_active_texture_unit(0); - m_canvas->m_layers[layer_index]->rtt(plane_index, frame).bindTexture(); - m_face_plane.draw_fill(); - set_active_texture_unit(0); - m_canvas->m_layers[layer_index]->rtt(plane_index, frame).unbindTexture(); - }; + const auto draw_layer_frame = pp::panopainter::make_legacy_canvas_draw_merge_layer_frame_draw( + m_canvas->m_layers[layer_index].get(), + &m_face_plane, + set_active_texture_unit, + plane_index, + m_canvas->m_layers[layer_index]->m_opacity); pp::panopainter::execute_legacy_canvas_draw_merge_layer_plane( m_canvas->m_current_stroke && m_canvas->m_current_mode == kCanvasMode::Erase && m_canvas->m_show_tmp && m_canvas->m_current_layer_idx == layer_index,