diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index b282705..943e563 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -586,9 +586,9 @@ Known local toolchain state: document-open action planning and executor dispatch/rejection, save-request, close-request executor dispatch/no-op preservation, document-save executor dispatch/no-op preservation, save-before-workflow executor dispatch, - new-document target/resolution/overwrite planning, document file target, - combined save-file overwrite planning, and save-version target decisions - without requiring a window, canvas, or message box. + new-document target/resolution/overwrite planning and executor dispatch, + document file target, combined save-file overwrite planning, and save-version + target decisions without requiring a window, canvas, or message box. - `src/legacy_document_open_services.*` is the current app-shell bridge between `pp_app_core` document-open plans and live ABR/PPBR import prompts, unsaved-project discard prompts, project opening, layer UI refresh, title @@ -597,8 +597,10 @@ Known local toolchain state: - `src/legacy_document_session_services.*` is the current app-shell bridge between `pp_app_core` document-session decisions and live close prompts, save dialogs, save-version routing, existing-project saves, and - save-before-workflow prompts; retained legacy UI/canvas execution remains - tracked by `DEBT-0040`. + save-before-workflow prompts. It also bridges accepted new-document plans to + legacy canvas resize/layer setup, overwrite prompts, title updates, and + keyboard/dialog cleanup. Retained legacy UI/canvas execution remains tracked + by `DEBT-0040` and `DEBT-0041`. - `src/legacy_history_services.*` is the current app-shell bridge between `pp_app_core` history plans and legacy `ActionManager`; toolbar and `NodeCanvas` hotkeys share it while document-history extraction remains diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index c65f8a2..97adf5c 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -58,6 +58,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0038 | Open | Modernization | Cloud upload/browse/bulk planning and execution dispatch now consume pure `pp_app_core` through `App::cloud_upload`, `App::cloud_upload_all`, `App::cloud_browse`, `pano_cli plan-cloud-upload`, `pano_cli plan-cloud-upload-all`, `pano_cli plan-cloud-browse`, and the `CloudServices` boundary; live execution is centralized in `src/legacy_cloud_services.*`, but the bridge still uses legacy save-before-upload, `upload`/`download` network helpers, progress/message UI, OpenGL context guarding, `NodeDialogCloud`, `Canvas` project open, layer refresh, and `ActionManager` reset | Preserve current cloud behavior while cloud/network/document import flows move toward app/document/platform services | `pp_app_core_document_cloud_tests`; `pano_cli plan-cloud-upload --new-document --unsaved`; `pano_cli plan-cloud-browse --selected-file demo.ppi`; `pano_cli plan-cloud-upload-all --file-count 3`; `ctest --preset desktop-fast --build-config Debug` | Cloud upload/download, save-before-upload, progress reporting, cloud browse dialog, downloaded project opening, layer refresh, OpenGL context ownership, and action-history reset are owned by injected app/document/network/platform/renderer services with `App` methods acting only as adapters | | DEBT-0039 | Open | Modernization | Document-open planning and execution dispatch now consume pure `pp_app_core` through `App::open_document`, `pano_cli plan-open-route`, `DocumentOpenServices`, and `src/legacy_document_open_services.*`, but the bridge still opens ABR/PPBR import prompts, launches legacy brush preset import threads, applies unsaved-project discard prompts, calls legacy project-open execution, refreshes layer UI, updates the app title, and clears legacy history directly | Preserve current file-open/import behavior while document loading and brush import move toward app/document/asset/UI services | `pp_app_core_document_route_tests`; `pp_app_core_document_session_tests`; `pano_cli plan-open-route --path D:/Paint/Scenes/demo.ppi --unsaved`; `pano_cli plan-open-route --path D:/Paint/Brushes/clouds.ABR --unsaved`; `ctest --preset desktop-fast --build-config Debug` | Brush import prompting/execution, project-open execution, unsaved-project discard prompting, layer refresh, title updates, and history clearing are owned by injected app/document/asset/UI services with `App::open_document` acting only as an adapter | | DEBT-0040 | Open | Modernization | Close request, document save, and save-before-workflow planning/execution dispatch now consume pure `pp_app_core` through `App::request_close`, `App::save_document`, `App::continue_document_workflow_after_optional_save`, `pano_cli simulate-app-session`, `DocumentSaveServices`, `CloseRequestServices`, `DocumentWorkflowServices`, and `src/legacy_document_session_services.*`, but the bridge still opens legacy message boxes/save dialogs, calls `Canvas::I->project_save`, mutates the unsaved flag on close confirmation, invokes native app close, and routes save-version through the retained legacy dialog | Preserve current close/save/dirty-workflow behavior while document session execution moves toward app/document/UI/platform services | `pp_app_core_document_session_tests`; `pano_cli simulate-app-session --unsaved --save-intent save-dirty-version`; `pano_cli simulate-app-session --no-canvas`; `pano_cli plan-document-file --work-dir D:/Paint --name demo --target-exists`; `pano_cli plan-document-version --directory D:/Paint --doc-name demo.01 --existing-path D:/Paint/demo.02.ppi`; `ctest --preset desktop-fast --build-config Debug` | Close prompt execution, native close requests, dirty-workflow save prompts, existing-project saves, save dialogs, save-version execution, and unsaved-flag mutation are owned by injected app/document/UI/platform services with `App` methods acting only as adapters | +| DEBT-0041 | Open | Modernization | Accepted new-document planning/execution dispatch now consumes pure `pp_app_core` through `App::dialog_newdoc`, `pano_cli plan-new-document`, `NewDocumentServices`, and `src/legacy_document_session_services.*`, but the bridge still mutates legacy app document fields, clears legacy layer UI, resizes legacy `Canvas`, clears legacy history, creates the default layer through legacy UI, mutates unsaved/new-document flags, updates the title, and handles keyboard/dialog cleanup directly | Preserve current New Document dialog behavior while document creation moves toward app/document/UI services | `pp_app_core_document_session_tests`; `pano_cli plan-new-document --work-dir D:/Paint --name demo --resolution-index 3`; `pano_cli plan-new-document --work-dir D:/Paint --name demo --resolution-index 3 --target-exists`; `pano_cli simulate-app-session --save-intent save`; `ctest --preset desktop-fast --build-config Debug` | New document creation, overwrite confirmation, canvas/document allocation, default layer creation, history clearing, title updates, dirty/new-document state, and keyboard/dialog cleanup are owned by injected app/document/UI services with `App::dialog_newdoc` acting only as a UI adapter | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 1e39153..db800c2 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -739,6 +739,11 @@ app-core document-session executors and `src/legacy_document_session_services.*` preserving close prompts, save dialogs, save-version routing, existing-project save execution, and dirty-workflow save-before-continue prompts while retained legacy UI/canvas behavior remains tracked under `DEBT-0040`. +`App::dialog_newdoc` now routes accepted new-document plans through the +app-core new-document executor and `src/legacy_document_session_services.*`, +preserving target overwrite prompts, legacy canvas resize/layer setup, history +clearing, title updates, dirty/new-document flag mutation, and keyboard/dialog +cleanup while retained execution remains tracked under `DEBT-0041`. Implementation tasks: @@ -1287,6 +1292,13 @@ Results: `pp_app_core_document_session_tests`, `pano_cli_simulate_app_session_*`, and `pano_cli_plan_document_file/version_*` smoke tests after the live bridge split. +- `PanoPainter`, `pp_app_core_document_session_tests`, and `pano_cli` built + after accepted new-document execution moved behind the new-document services + bridge. A clean rebuild was required once because MSVC reported the known + Debug PDB `LNK1103` corruption, after which the build passed. +- Focused new-document/session CTest coverage passed for + `pp_app_core_document_session_tests`, `pano_cli_plan_new_document_*`, and + `pano_cli_simulate_app_session_*` smoke tests after the live bridge split. - `pp_app_core_document_recording_tests` passed, covering recording start/stop, clear, platform recorded-file cleanup, frame-count reset, export progress totals, and oversized progress-total clamping. diff --git a/src/app_core/document_session.h b/src/app_core/document_session.h index e8edd08..c7e3e94 100644 --- a/src/app_core/document_session.h +++ b/src/app_core/document_session.h @@ -112,6 +112,14 @@ struct NewDocumentPlan { DocumentFileWriteDecision write_decision = DocumentFileWriteDecision::save_now; }; +class NewDocumentServices { +public: + virtual ~NewDocumentServices() = default; + + virtual void create_new_document(const NewDocumentPlan& plan) = 0; + virtual void prompt_overwrite_new_document(const NewDocumentPlan& plan) = 0; +}; + [[nodiscard]] constexpr ProjectOpenDecision plan_project_open(bool has_unsaved_changes) noexcept { return has_unsaved_changes @@ -369,6 +377,22 @@ template return pp::foundation::Result::success(std::move(plan)); } +[[nodiscard]] inline pp::foundation::Status execute_new_document_plan( + const NewDocumentPlan& plan, + NewDocumentServices& services) +{ + switch (plan.write_decision) { + case DocumentFileWriteDecision::save_now: + services.create_new_document(plan); + return pp::foundation::Status::success(); + case DocumentFileWriteDecision::prompt_overwrite: + services.prompt_overwrite_new_document(plan); + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown new document write decision"); +} + [[nodiscard]] inline bool has_legacy_two_character_version_suffix(std::string_view document_name) noexcept { const auto dot = document_name.rfind('.'); diff --git a/src/app_dialogs.cpp b/src/app_dialogs.cpp index d7a9e64..cfa384b 100644 --- a/src/app_dialogs.cpp +++ b/src/app_dialogs.cpp @@ -7,7 +7,6 @@ #include "legacy_document_canvas_services.h" #include "legacy_document_layer_services.h" #include "legacy_document_session_services.h" -#include "legacy_history_services.h" #include "settings.h" #include "node_dialog_open.h" #include "node_dialog_browse.h" @@ -183,46 +182,9 @@ void App::dialog_newdoc() return; } - auto action = [this, dialog, plan = plan.value()] { - doc_name = plan.target.name; - doc_path = plan.target.path; - doc_filename = plan.target.name + ".ppi"; - doc_dir = plan.target.directory; - - layers->clear(); - canvas->m_canvas->m_layers.clear(); - canvas->m_canvas->resize(plan.resolution, plan.resolution); - canvas->reset_camera(); - pp::panopainter::clear_legacy_history(); - - layers->add_layer("Default", false, true); - - canvas->m_canvas->m_unsaved = true; - canvas->m_canvas->m_newdoc = false; - title_update(); - - dialog->destroy(); - App::I->hideKeyboard(); - }; - - if (plan.value().write_decision == pp::app::DocumentFileWriteDecision::prompt_overwrite) - { - // ask confirm is file already exist - auto msgbox = new NodeMessageBox(); - msgbox->set_manager(&layout); - msgbox->init(); - msgbox->m_title->set_text("Warning"); - msgbox->m_message->set_text("A document with this name already exists, continue?"); - msgbox->btn_ok->on_click = [this, msgbox, action](Node*) { - action(); - msgbox->destroy(); - }; - layout[main_id]->add_child(msgbox); - } - else - { - action(); - } + const auto status = pp::panopainter::execute_legacy_new_document_plan(*this, plan.value(), dialog); + if (!status.ok()) + LOG("New document action failed: %s", status.message); }; dialog->btn_cancel->on_click = [this, dialog](Node*) diff --git a/src/legacy_document_session_services.cpp b/src/legacy_document_session_services.cpp index 039a756..6aed955 100644 --- a/src/legacy_document_session_services.cpp +++ b/src/legacy_document_session_services.cpp @@ -3,10 +3,74 @@ #include "legacy_document_session_services.h" #include "app.h" +#include "legacy_history_services.h" +#include "node_dialog_open.h" + +#include namespace pp::panopainter { namespace { +void create_legacy_new_document( + App& app, + const pp::app::NewDocumentPlan& plan, + const std::shared_ptr& dialog) +{ + app.doc_name = plan.target.name; + app.doc_path = plan.target.path; + app.doc_filename = plan.target.name + ".ppi"; + app.doc_dir = plan.target.directory; + + app.layers->clear(); + app.canvas->m_canvas->m_layers.clear(); + app.canvas->m_canvas->resize(plan.resolution, plan.resolution); + app.canvas->reset_camera(); + pp::panopainter::clear_legacy_history(); + + app.layers->add_layer("Default", false, true); + + app.canvas->m_canvas->m_unsaved = true; + app.canvas->m_canvas->m_newdoc = false; + app.title_update(); + + dialog->destroy(); + App::I->hideKeyboard(); +} + +class LegacyNewDocumentServices final : public pp::app::NewDocumentServices { +public: + LegacyNewDocumentServices(App& app, std::shared_ptr dialog) noexcept + : app_(app) + , dialog_(std::move(dialog)) + { + } + + void create_new_document(const pp::app::NewDocumentPlan& plan) override + { + create_legacy_new_document(app_, plan, dialog_); + } + + void prompt_overwrite_new_document(const pp::app::NewDocumentPlan& plan) override + { + auto msgbox = new NodeMessageBox(); + msgbox->set_manager(&app_.layout); + msgbox->init(); + msgbox->m_title->set_text("Warning"); + msgbox->m_message->set_text("A document with this name already exists, continue?"); + auto* app = &app_; + auto dialog = dialog_; + msgbox->btn_ok->on_click = [app, msgbox, dialog, plan](Node*) { + create_legacy_new_document(*app, plan, dialog); + msgbox->destroy(); + }; + app_.layout[app_.main_id]->add_child(msgbox); + } + +private: + App& app_; + std::shared_ptr dialog_; +}; + class LegacyCloseRequestServices final : public pp::app::CloseRequestServices { public: LegacyCloseRequestServices(App& app, bool& dialog_already_opened) noexcept @@ -140,4 +204,13 @@ pp::foundation::Status execute_legacy_document_workflow_decision( return pp::app::execute_document_workflow_decision(decision, services); } +pp::foundation::Status execute_legacy_new_document_plan( + App& app, + const pp::app::NewDocumentPlan& plan, + std::shared_ptr dialog) +{ + LegacyNewDocumentServices services(app, std::move(dialog)); + return pp::app::execute_new_document_plan(plan, services); +} + } // namespace pp::panopainter diff --git a/src/legacy_document_session_services.h b/src/legacy_document_session_services.h index 05706ab..0ad0f8e 100644 --- a/src/legacy_document_session_services.h +++ b/src/legacy_document_session_services.h @@ -4,8 +4,10 @@ #include "foundation/result.h" #include +#include class App; +class NodeDialogNewDoc; namespace pp::panopainter { @@ -23,4 +25,9 @@ namespace pp::panopainter { pp::app::DocumentWorkflowDecision decision, std::function action); +[[nodiscard]] pp::foundation::Status execute_legacy_new_document_plan( + App& app, + const pp::app::NewDocumentPlan& plan, + std::shared_ptr dialog); + } // namespace pp::panopainter diff --git a/tests/app_core/document_session_tests.cpp b/tests/app_core/document_session_tests.cpp index 4d98012..b69ae11 100644 --- a/tests/app_core/document_session_tests.cpp +++ b/tests/app_core/document_session_tests.cpp @@ -107,6 +107,31 @@ public: std::string call_order; }; +class FakeNewDocumentServices final : public pp::app::NewDocumentServices { +public: + void create_new_document(const pp::app::NewDocumentPlan& plan) override + { + creates += 1; + last_name = plan.target.name; + last_resolution = plan.resolution; + call_order += "create;"; + } + + void prompt_overwrite_new_document(const pp::app::NewDocumentPlan& plan) override + { + overwrite_prompts += 1; + last_name = plan.target.name; + last_resolution = plan.resolution; + call_order += "overwrite;"; + } + + int creates = 0; + int overwrite_prompts = 0; + int last_resolution = 0; + std::string last_name; + std::string call_order; +}; + [[nodiscard]] pp::app::DocumentOpenRoute project_route() { return { @@ -547,6 +572,32 @@ void new_document_plan_rejects_invalid_inputs(pp::tests::Harness& harness) PP_EXPECT(harness, invalid_resolution.status().code == pp::foundation::StatusCode::out_of_range); } +void new_document_executor_dispatches_save_and_overwrite_paths(pp::tests::Harness& harness) +{ + FakeNewDocumentServices services; + auto create = pp::app::plan_new_document( + "D:/Paint", + "demo", + 1, + [](const std::string&) { return false; }); + auto overwrite = pp::app::plan_new_document( + "D:/Paint", + "demo", + 2, + [](const std::string& path) { return path == "D:/Paint/demo.ppi"; }); + + PP_EXPECT(harness, create); + PP_EXPECT(harness, overwrite); + PP_EXPECT(harness, pp::app::execute_new_document_plan(create.value(), services).ok()); + PP_EXPECT(harness, pp::app::execute_new_document_plan(overwrite.value(), services).ok()); + + PP_EXPECT(harness, services.creates == 1); + PP_EXPECT(harness, services.overwrite_prompts == 1); + PP_EXPECT(harness, services.last_name == "demo"); + PP_EXPECT(harness, services.last_resolution == 1536); + PP_EXPECT(harness, services.call_order == "create;overwrite;"); +} + void document_version_target_starts_at_first_version(pp::tests::Harness& harness) { const auto target = pp::app::find_next_document_version_target( @@ -642,6 +693,7 @@ int main() new_document_plan_builds_target_resolution_and_write_decision); harness.run("new document plan prompts for existing target", new_document_plan_prompts_for_existing_target); harness.run("new document plan rejects invalid inputs", new_document_plan_rejects_invalid_inputs); + harness.run("new document executor dispatches save and overwrite paths", new_document_executor_dispatches_save_and_overwrite_paths); harness.run("document version target starts at first version", document_version_target_starts_at_first_version); harness.run("document version target increments existing suffix", document_version_target_increments_existing_suffix); harness.run("document version target skips existing paths", document_version_target_skips_existing_paths);