From 1a64118b2c2114440849b0f4f02e8baebc81f7ae Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 17 Jun 2026 18:43:43 +0200 Subject: [PATCH] Thin app dialog and renderer adapter ownership --- cmake/PanoPainterSources.cmake | 2 + docs/modernization/debt.md | 20 +++ docs/modernization/tasks.md | 6 +- src/app_dialogs_workflow.cpp | 38 +----- src/canvas_layer.cpp | 31 ++--- ...cy_app_shell_performance_test_services.cpp | 43 ++++++ ...gacy_app_shell_performance_test_services.h | 15 +++ src/legacy_app_shell_services.cpp | 33 +---- src/legacy_brush_package_export_services.cpp | 126 +++++------------- src/legacy_document_session_services.cpp | 39 +++++- src/legacy_document_session_services.h | 6 + 11 files changed, 181 insertions(+), 178 deletions(-) create mode 100644 src/legacy_app_shell_performance_test_services.cpp create mode 100644 src/legacy_app_shell_performance_test_services.h diff --git a/cmake/PanoPainterSources.cmake b/cmake/PanoPainterSources.cmake index 6f9decf2..9e890247 100644 --- a/cmake/PanoPainterSources.cmake +++ b/cmake/PanoPainterSources.cmake @@ -94,6 +94,8 @@ set(PP_LEGACY_APP_SOURCES src/legacy_canvas_mode_transform.cpp src/legacy_app_shell_services.cpp src/legacy_app_shell_services.h + src/legacy_app_shell_performance_test_services.cpp + src/legacy_app_shell_performance_test_services.h src/legacy_canvas_tool_services.cpp src/legacy_canvas_tool_services.h src/legacy_canvas_view_services.cpp diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 74c135fd..f8e7ce72 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -18,6 +18,26 @@ agent or engineer to remove them without reconstructing context from chat. ## Reductions +- 2026-06-17: `DEBT-0036` was narrowed again. `src/canvas_layer.cpp` now + routes retained `Layer` / `LayerFrame` render queueing through + `src/renderer_gl/render_runtime_dispatch.h` instead of direct + `App::I->render_task*` calls; retained `Canvas::I` draw-state use and the + broader canvas renderer execution remain. +- 2026-06-17: `DEBT-0034` was narrowed again. + `LegacyAboutMenuServices::run_performance_test(...)` in + `src/legacy_app_shell_services.cpp` now delegates the retained stroke/timing + workflow to `src/legacy_app_shell_performance_test_services.*`, so the About + menu adapter no longer owns the inline render-task/canvas execution body. +- 2026-06-17: `DEBT-0040`/`DEBT-0042` were narrowed again. `App::dialog_save()` + and `App::dialog_save_ver()` in `src/app_dialogs_workflow.cpp` now delegate + Save As / Save Version acceptance through + `src/legacy_document_session_services.*`, so the app dialog shell no longer + owns the retained plan/target lookup entrypoints for those save paths. +- 2026-06-17: `DEBT-0047` was narrowed again. The desktop async PPBR export + path in `src/legacy_brush_package_export_services.cpp` now uses + `AppRuntime::canvas_async_task` plus named completion helpers instead of a + file-static worker singleton, while retained preset-panel execution and + dialog-owned request extraction still remain. - 2026-06-17: `DEBT-0036` was narrowed again. Retained `Shape`, `src/shader.cpp`, and `src/font.cpp` now queue render-thread create/update/ destroy work through `src/renderer_gl/render_runtime_dispatch.h` instead of diff --git a/docs/modernization/tasks.md b/docs/modernization/tasks.md index 170b512f..aad5b2b9 100644 --- a/docs/modernization/tasks.md +++ b/docs/modernization/tasks.md @@ -53,9 +53,9 @@ Key facts: - `Canvas::I` still appears hundreds of times in retained canvas modes, panels, and workflow bridges. - Raw `Node*` and callback captures remain a dominant UI lifetime risk. -- `CanvasLayer` and retained stroke-preview/runtime draw paths still depend on - legacy render/runtime helpers, but `RTT`, `Texture2D`, `Shape`, `Shader`, - and `TextMesh` no longer call `App::I` directly for queueing. +- Retained stroke-preview/runtime draw paths still depend on legacy + render/runtime helpers, but `RTT`, `Texture2D`, `Shape`, `Shader`, + `TextMesh`, and `CanvasLayer` no longer call `App::I` directly for queueing. - `AppRuntime` now owns synchronized running flags plus explicit post/reject, same-thread execution, and queue-drain behavior, but broader singleton reach and app-shell ownership remain. diff --git a/src/app_dialogs_workflow.cpp b/src/app_dialogs_workflow.cpp index 5f65df9d..5f87ffe5 100644 --- a/src/app_dialogs_workflow.cpp +++ b/src/app_dialogs_workflow.cpp @@ -43,21 +43,7 @@ void wire_document_save_dialog_buttons( { dialog->btn_ok->on_click = [&app, dialog](Node*) { - std::string name = dialog->input->m_text; - const auto plan = pp::app::plan_document_file_save( - app.work_path, - name, - [](const std::string& path) { - return Asset::exist(path); - }); - if (!plan) - { - app.message_box("Warning", "You need to specify a name to file."); - return; - } - - const auto status = - pp::panopainter::execute_legacy_document_file_save_plan(app, plan.value(), dialog); + const auto status = pp::panopainter::execute_legacy_document_file_save_dialog(app, dialog); if (!status.ok()) LOG("Document file save action failed: %s", status.message); }; @@ -67,24 +53,6 @@ void wire_document_save_dialog_buttons( }; } -void save_document_version(App& app) -{ - const auto target = pp::app::find_next_document_version_target( - app.doc_dir, - app.doc_name, - [](const std::string& path) { - return Asset::exist(path); - }); - if (!target) { - app.message_box("Saving Error", target.status().message); - return; - } - - const auto status = pp::panopainter::execute_legacy_document_version_save(app, target.value()); - if (!status.ok()) - LOG("Document version save action failed: %s", status.message); -} - } // namespace void App::continue_document_workflow_after_optional_save(std::function action) @@ -247,7 +215,9 @@ void App::dialog_save_ver() return; } - save_document_version(*this); + const auto status = pp::panopainter::execute_legacy_document_version_save_dialog(*this); + if (!status.ok()) + LOG("Document version save action failed: %s", status.message); } void App::save_document(pp::app::DocumentSaveIntent intent) diff --git a/src/canvas_layer.cpp b/src/canvas_layer.cpp index 4dab2fd8..66703b89 100644 --- a/src/canvas_layer.cpp +++ b/src/canvas_layer.cpp @@ -5,6 +5,7 @@ #include "legacy_ui_gl_dispatch.h" #include "log.h" #include "renderer_gl/opengl_capabilities.h" +#include "renderer_gl/render_runtime_dispatch.h" #include "rtt.h" #include "util.h" @@ -105,7 +106,7 @@ TextureCube Layer::gen_cube() ret.create(w); for (int i = 0; i < 6; i++) { - App::I->render_task([&] + pp::renderer::gl::render_runtime_dispatch().render_task([&] { ret.bind(); rtt(i).bindFramebuffer(); @@ -120,7 +121,7 @@ Texture2D Layer::gen_equirect(glm::ivec2 size /*= { 0, 0 }*/) { Texture2D ret; - App::I->render_task([&] + pp::renderer::gl::render_runtime_dispatch().render_task([&] { gl_state gl; gl.save(); @@ -177,7 +178,7 @@ PBO Layer::gen_equirect_pbo(glm::ivec2 size /*= { 0, 0 }*/) latlong.create(size.x, size.y); std::this_thread::yield(); - App::I->render_task([&] + pp::renderer::gl::render_runtime_dispatch().render_task([&] { apply_layer_capability(pp::renderer::gl::blend_state(), false); @@ -317,7 +318,7 @@ void Layer::duplicate_frame(int frame) void Layer::frames_gpu_update() { - App::I->render_task_async([=] + pp::renderer::gl::render_runtime_dispatch().render_task_async([=] { for (int j = 0; j < m_frames.size(); j++) { @@ -471,22 +472,6 @@ LayerFrame& LayerFrame::operator=(LayerFrame&& other) bool LayerFrame::create(int width, int height, int duration /*= 1*/) { bool success = true; - //App::I->render_task([&] - //{ - // for (int i = 0; i < 6; i++) - // { - // if (!m_rtt[i].create(width, height)) - // { - // success = false; - // return; - // } - // m_rtt[i].bindFramebuffer(); - // m_rtt[i].clear(); - // m_rtt[i].unbindFramebuffer(); - // m_dirty_box[i] = glm::vec4(width, height, 0, 0); // reset bounding box - // m_dirty_face[i] = false; - // } - //}); for (int i = 0; i < 6; i++) { m_dirty_box[i] = glm::vec4(width, height, 0, 0); // reset bounding box @@ -517,7 +502,7 @@ bool LayerFrame::resize(int width, int height) void LayerFrame::clear(const glm::vec4& c) { - App::I->render_task([&] + pp::renderer::gl::render_runtime_dispatch().render_task([&] { // push clear color state const auto cc = query_layer_clear_color(); @@ -566,7 +551,7 @@ LayerFrame LayerFrame::clone() noexcept void LayerFrame::restore(const Snapshot& snap) { - App::I->render_task([this, &snap] + pp::renderer::gl::render_runtime_dispatch().render_task([this, &snap] { clear({ 0, 0, 0, 0 }); for (int i = 0; i < 6; i++) @@ -605,7 +590,7 @@ LayerFrame::Snapshot LayerFrame::snapshot(std::array* dirty_box /* Snapshot snap; snap.width = w; snap.height = h; - App::I->render_task([this, &snap, dirty_face, dirty_box] + pp::renderer::gl::render_runtime_dispatch().render_task([this, &snap, dirty_face, dirty_box] { for (int i = 0; i < 6; i++) { diff --git a/src/legacy_app_shell_performance_test_services.cpp b/src/legacy_app_shell_performance_test_services.cpp new file mode 100644 index 00000000..69de273f --- /dev/null +++ b/src/legacy_app_shell_performance_test_services.cpp @@ -0,0 +1,43 @@ +#include "pch.h" + +#include "legacy_app_shell_performance_test_services.h" + +#include "app.h" +#include "canvas.h" + +namespace pp::panopainter { + +std::string run_legacy_about_menu_performance_test( + App& app, + Canvas& canvas, + int performance_iterations) +{ + std::string message; + app.render_task([&] + { + const auto start = std::chrono::high_resolution_clock::now(); + canvas.stroke_start({ 0, 0, 0 }, 0.9f); + for (int i = 0; i < performance_iterations; ++i) + { + canvas.stroke_update({ 100, 100, 0 }, 0.9f); + canvas.stroke_update({ 200, 200, 0 }, 0.9f); + canvas.stroke_update({ 200, 100, 0 }, 0.9f); + canvas.stroke_update({ 100, 200, 0 }, 0.9f); + canvas.stroke_update({ 300, 300, 0 }, 0.9f); + canvas.stroke_update({ 200, 500, 0 }, 0.9f); + canvas.stroke_update({ 500, 500, 0 }, 0.9f); + canvas.stroke_update({ 400, 400, 0 }, 0.9f); + canvas.stroke_update({ 0, 200, 0 }, 0.9f); + canvas.stroke_update({ 200, 0, 0 }, 0.9f); + canvas.stroke_draw(); + } + canvas.stroke_end(); + const auto diff = std::chrono::high_resolution_clock::now() - start; + const auto ms = std::chrono::duration_cast(diff).count(); + LOG("%lld ms", static_cast(ms)); + message = "Time " + std::to_string(ms) + " ms"; + }); + return message; +} + +} // namespace pp::panopainter diff --git a/src/legacy_app_shell_performance_test_services.h b/src/legacy_app_shell_performance_test_services.h new file mode 100644 index 00000000..0c54b1ef --- /dev/null +++ b/src/legacy_app_shell_performance_test_services.h @@ -0,0 +1,15 @@ +#pragma once + +#include + +class App; +class Canvas; + +namespace pp::panopainter { + +[[nodiscard]] std::string run_legacy_about_menu_performance_test( + App& app, + Canvas& canvas, + int performance_iterations); + +} // namespace pp::panopainter diff --git a/src/legacy_app_shell_services.cpp b/src/legacy_app_shell_services.cpp index 30242cdf..e48c669f 100644 --- a/src/legacy_app_shell_services.cpp +++ b/src/legacy_app_shell_services.cpp @@ -5,6 +5,7 @@ #include "app.h" #include "app_core/document_import.h" #include "legacy_app_dialog_services.h" +#include "legacy_app_shell_performance_test_services.h" #include "legacy_canvas_view_services.h" #include "legacy_document_image_import_services.h" #include "legacy_document_canvas_services.h" @@ -217,36 +218,14 @@ public: void run_performance_test(const pp::app::AboutMenuPlan& plan) override { - if (!Canvas::I) + if (!app_.canvas || !app_.canvas->m_canvas) return; LOG("perf"); - std::string message; - const int performance_iterations = plan.performance_iterations; - app_.render_task([&] - { - auto start = std::chrono::high_resolution_clock::now(); - Canvas::I->stroke_start({ 0, 0, 0 }, 0.9f); - for (int i = 0; i < performance_iterations; i++) - { - Canvas::I->stroke_update({ 100, 100, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 100, 0 }, 0.9f); - Canvas::I->stroke_update({ 100, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 300, 300, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 500, 0 }, 0.9f); - Canvas::I->stroke_update({ 500, 500, 0 }, 0.9f); - Canvas::I->stroke_update({ 400, 400, 0 }, 0.9f); - Canvas::I->stroke_update({ 0, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 0, 0 }, 0.9f); - Canvas::I->stroke_draw(); - } - Canvas::I->stroke_end(); - auto diff = std::chrono::high_resolution_clock::now() - start; - auto ms = std::chrono::duration_cast(diff).count(); - LOG("%lld ms", ms); - message = "Time " + std::to_string(ms) + " ms"; - }); + const auto message = run_legacy_about_menu_performance_test( + app_, + *app_.canvas->m_canvas, + plan.performance_iterations); app_.message_box("Performance test", message); } diff --git a/src/legacy_brush_package_export_services.cpp b/src/legacy_brush_package_export_services.cpp index 4fc05023..41b65da3 100644 --- a/src/legacy_brush_package_export_services.cpp +++ b/src/legacy_brush_package_export_services.cpp @@ -7,88 +7,44 @@ #include "node_dialog_export_ppbr.h" #include "node_panel_brush.h" -#include -#include #include #include -#include -#include -#include namespace pp::panopainter { namespace { -class LegacyBrushPackageWorker final { -public: - LegacyBrushPackageWorker() - : worker_([this](std::stop_token stop_token) { - run(stop_token); - }) - { - } - - ~LegacyBrushPackageWorker() - { - 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("brush package export worker task failed"); - } - } - } - } - - std::mutex mutex_; - std::condition_variable cv_; - std::deque> tasks_; - bool stopping_ = false; - std::jthread worker_; -}; - -LegacyBrushPackageWorker& brush_package_worker() +void queue_legacy_brush_package_export_success_prompt( + App& app, + std::shared_ptr dialog, + std::string path) { - static LegacyBrushPackageWorker worker; - return worker; + const auto plan = pp::app::plan_brush_package_export_success_dialog(path); + app.ui_task([dialog = std::move(dialog), plan] { + if (dialog) { + pp::panopainter::close_legacy_dialog_node(*dialog); + } + App::I->message_box(plan.title, plan.message, plan.show_cancel); + }); +} + +void queue_legacy_brush_package_export_job( + App& app, + std::shared_ptr dialog, + std::shared_ptr presets, + std::string path_string, + NodePanelBrushPreset::PPBRInfo info) +{ + app.runtime().canvas_async_task([app = &app, + presets = std::move(presets), + dialog = std::move(dialog), + path_string = std::move(path_string), + info = std::move(info)]() mutable { + BT_SetTerminate(); + if (presets) { + presets->export_ppbr(path_string, info); + } + queue_legacy_brush_package_export_success_prompt(*app, std::move(dialog), std::move(path_string)); + }); } NodePanelBrushPreset::PPBRInfo to_legacy_ppbr_info( @@ -124,21 +80,13 @@ public: const auto info = to_legacy_ppbr_info(request, dialog_); auto presets = app_.presets; if (mode_ == LegacyBrushPackageExportMode::desktop_async_close_and_message) { - auto* app = &app_; auto dialog = std::static_pointer_cast(dialog_.shared_from_this()); - brush_package_worker().post([app, presets, dialog, path_string, info] { - BT_SetTerminate(); - if (presets) { - presets->export_ppbr(path_string, info); - } - const auto plan = pp::app::plan_brush_package_export_success_dialog(path_string); - app->ui_task([dialog, plan] { - if (dialog) { - pp::panopainter::close_legacy_dialog_node(*dialog); - } - App::I->message_box(plan.title, plan.message, plan.show_cancel); - }); - }); + queue_legacy_brush_package_export_job( + app_, + std::move(dialog), + std::move(presets), + path_string, + info); return; } diff --git a/src/legacy_document_session_services.cpp b/src/legacy_document_session_services.cpp index 09a269a7..f57047e2 100644 --- a/src/legacy_document_session_services.cpp +++ b/src/legacy_document_session_services.cpp @@ -231,7 +231,7 @@ private: std::shared_ptr dialog_; }; -void save_legacy_document_version(App& app, const pp::app::DocumentVersionTarget& target); +pp::foundation::Status save_legacy_document_version(App& app, const pp::app::DocumentVersionTarget& target); class LegacyDocumentVersionSaveServices final : public pp::app::DocumentVersionSaveServices { public: @@ -249,7 +249,7 @@ private: App& app_; }; -void save_legacy_document_version(App& app, const pp::app::DocumentVersionTarget& target) +pp::foundation::Status save_legacy_document_version(App& app, const pp::app::DocumentVersionTarget& target) { const auto history_status = pp::panopainter::execute_legacy_history_plan( pp::app::plan_document_version_save_history(target)); @@ -261,6 +261,7 @@ void save_legacy_document_version(App& app, const pp::app::DocumentVersionTarget app.canvas->m_canvas->m_unsaved = true; app.title_update(); project_save_after_snapshot(app, app.doc_path); + return pp::foundation::Status::success(); } class LegacyCloseRequestServices final : public pp::app::CloseRequestServices { @@ -450,6 +451,24 @@ pp::foundation::Status execute_legacy_document_file_save_plan( return pp::app::execute_document_file_save_plan(plan, services); } +pp::foundation::Status execute_legacy_document_file_save_dialog( + App& app, + std::shared_ptr dialog) +{ + const auto plan = pp::app::plan_document_file_save( + app.work_path, + dialog->input->m_text, + [](const std::string& path) { + return Asset::exist(path); + }); + if (!plan) { + app.message_box("Warning", "You need to specify a name to file."); + return pp::foundation::Status::success(); + } + + return execute_legacy_document_file_save_plan(app, plan.value(), std::move(dialog)); +} + pp::foundation::Status execute_legacy_document_version_save( App& app, const pp::app::DocumentVersionTarget& target) @@ -458,6 +477,22 @@ pp::foundation::Status execute_legacy_document_version_save( return pp::app::execute_document_version_save(target, services); } +pp::foundation::Status execute_legacy_document_version_save_dialog(App& app) +{ + const auto target = pp::app::find_next_document_version_target( + app.doc_dir, + app.doc_name, + [](const std::string& path) { + return Asset::exist(path); + }); + if (!target) { + app.message_box("Saving Error", target.status().message); + return pp::foundation::Status::success(); + } + + return execute_legacy_document_version_save(app, target.value()); +} + void execute_legacy_document_save_before_cloud_upload(App& app) { Canvas::I->project_save_thread(app.doc_path, true); diff --git a/src/legacy_document_session_services.h b/src/legacy_document_session_services.h index 03b2ec64..0af4993a 100644 --- a/src/legacy_document_session_services.h +++ b/src/legacy_document_session_services.h @@ -36,10 +36,16 @@ namespace pp::panopainter { const pp::app::DocumentFileSavePlan& plan, std::shared_ptr dialog); +[[nodiscard]] pp::foundation::Status execute_legacy_document_file_save_dialog( + App& app, + std::shared_ptr dialog); + [[nodiscard]] pp::foundation::Status execute_legacy_document_version_save( App& app, const pp::app::DocumentVersionTarget& target); +[[nodiscard]] pp::foundation::Status execute_legacy_document_version_save_dialog(App& app); + void execute_legacy_document_save_before_cloud_upload(App& app); } // namespace pp::panopainter