diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 8ff305e..b7b29df 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -49,7 +49,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0029 | Open | Modernization | Image import route planning now consumes pure `pp_app_core` through the File menu and `pano_cli plan-image-import`, but live execution still calls legacy `Canvas::import_equirectangular` or legacy import transform mode setup directly after image loading | Preserve current File > Import behavior while image import moves toward document/app/asset command services | `pp_app_core_document_import_tests`; `pano_cli plan-image-import --width 4096 --height 2048`; `pano_cli plan-image-import --width 1024 --height 1024`; `ctest --preset desktop-fast --build-config Debug` | Image loading, equirectangular import, transform-placement import, and failure reporting are owned by document/app/asset services with File-menu callbacks acting only as adapters | | DEBT-0030 | Open | Modernization | File export menu action planning now consumes pure `pp_app_core` through the File menu and `pano_cli plan-export-menu`, but live execution still opens legacy export dialogs and then reaches legacy canvas/render/video export code | Preserve current export menu behavior while export command execution moves toward document/app/renderer/video services | `pp_app_core_document_export_tests`; `pano_cli plan-export-menu --kind png`; `pano_cli plan-export-menu --kind animation-mp4 --demo`; `pano_cli plan-export-menu --kind layers --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Export menu routing, license gating, target creation, image/layer/cube/depth/animation/timelapse execution, and error reporting are owned by document/app services with File-menu callbacks acting only as adapters | | DEBT-0031 | Open | Modernization | Top-level File menu command planning now consumes pure `pp_app_core` through `App::init_menu_file` and `pano_cli plan-file-menu`, but live execution still invokes legacy dialogs, platform pickers, cloud code, share code, and canvas import/export paths directly | Preserve File menu behavior while app workflows move toward app/document/platform command services | `pp_app_core_file_menu_tests`; `pano_cli plan-file-menu --command save-as`; `pano_cli plan-file-menu --command import`; `pano_cli plan-file-menu --command cloud-upload`; `ctest --preset desktop-fast --build-config Debug` | File menu routing, picker dispatch, save/share/cloud/resize/export execution, and image/project import execution are owned by app/document/platform services with `App::init_menu_file` acting only as a UI adapter | -| DEBT-0032 | Open | Modernization | Layer menu command planning now consumes pure `pp_app_core` through `App::init_menu_layer` and `pano_cli plan-layer-menu`, but live execution still calls legacy `Canvas::clear`, `App::dialog_layer_rename`, `NodePanelLayer::merge`, and reads `Canvas::I` animation/layer state directly | Preserve existing Layer menu behavior while layer commands move toward document/app services | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-menu --command merge --current-index 2 --lower-name Paint`; `pano_cli plan-layer-menu --command rename --no-current-layer`; `ctest --preset desktop-fast --build-config Debug` | Layer clear, rename, merge-down execution, animation gating, and selected-layer state are owned by document/app services with Layer-menu callbacks acting only as UI adapters | +| DEBT-0032 | Open | Modernization | Layer menu command planning and execution dispatch now consume pure `pp_app_core` through `App::init_menu_layer`, `pano_cli plan-layer-menu`, and the `DocumentLayerMenuServices` boundary, but the live adapter still calls legacy `Canvas::clear`, `App::dialog_layer_rename`, `NodePanelLayer::merge`, and reads `Canvas::I` animation/layer state directly | Preserve existing Layer menu behavior while layer commands move toward document/app services | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-menu --command merge --current-index 2 --lower-name Paint`; `pano_cli plan-layer-menu --command rename --no-current-layer`; `ctest --preset desktop-fast --build-config Debug` | Layer clear, rename, merge-down execution, animation gating, and selected-layer state are owned by injected document/app services with Layer-menu callbacks acting only as UI adapters and no legacy Layer menu adapter | | DEBT-0033 | Open | Modernization | Tools menu planning and direct command execution dispatch now consume pure `pp_app_core` through `App::init_menu_tools`, `pano_cli plan-tools-menu`, `pano_cli plan-tools-panel`, and the `ToolsMenuServices` boundary, but live adapters still construct legacy `NodePanelFloating` panels, mutate legacy panel nodes, clear `CanvasModeGrid`, reset `NodeCanvas` camera state, open legacy shortcuts UI, and call the iOS SonarPen bridge directly | Preserve current Tools menu behavior while UI shell actions move toward app/UI/platform services | `pp_app_core_tools_menu_tests`; `pano_cli plan-tools-menu --command shortcuts`; `pano_cli plan-tools-panel --panel layers`; `pano_cli plan-tools-panel --panel animation --already-visible`; `ctest --preset desktop-fast --build-config Debug` | Tools panel creation, submenu routing, grid clear, camera reset, shortcuts dialog, and SonarPen dispatch are owned by injected app/UI/platform services with `App::init_menu_tools` acting only as a UI adapter and no legacy Tools adapter | | DEBT-0034 | Open | Modernization | About menu command planning and execution dispatch now consume pure `pp_app_core` through `App::init_menu_about`, `pano_cli plan-about-menu`, and the `AboutMenuServices` boundary, but the live adapter still opens legacy About/manual/what's-new dialogs, invokes the injected crash hook, and runs the legacy Canvas stroke performance test directly | Preserve About menu behavior while dialogs and diagnostics move toward app/UI/platform services | `pp_app_core_about_menu_tests`; `pano_cli plan-about-menu --command news --version-major 2 --version-minor 5 --version-fix 7`; `pano_cli plan-about-menu --command performance --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | About/manual/what's-new dialog dispatch, crash-test dispatch, and performance-test execution are owned by injected app/UI/platform services with `App::init_menu_about` acting only as a UI adapter and no legacy About adapter | | DEBT-0035 | Open | Modernization | Main toolbar/status command planning and execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `pano_cli plan-main-toolbar`, and the `MainToolbarServices` boundary, but the live adapter still opens legacy open/save/settings/message-box dialogs, mutates legacy `ActionManager` history, and clears the legacy `Canvas` directly | Preserve reachable toolbar/status behavior while app shell commands move toward app/document/UI services | `pp_app_core_main_toolbar_tests`; `pano_cli plan-main-toolbar --command undo --undo-count 2`; `pano_cli plan-main-toolbar --command clear-canvas --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Open/save/settings/message-box routing, undo/redo/clear-history execution, and canvas-clear execution are owned by injected app/document/UI services with `App::init_toolbar_main` acting only as a UI adapter and no legacy toolbar adapter | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index eee9b4d..888816a 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -492,8 +492,9 @@ duplicate, select, reorder, remove, opacity, visibility, alpha-lock, blend-mode, and highlight actions used by the live layer panel before legacy `Canvas` and UI layer execution continue. `pano_cli plan-layer-menu` exposes app-core planning for Layer menu clear, -rename, and merge-down labels/actions before legacy canvas/layer execution -continues. +rename, and merge-down labels/actions, and direct Layer menu commands now +dispatch through `DocumentLayerMenuServices` before the legacy canvas/layer UI +adapter continues execution. `pano_cli plan-animation-operation` exposes app-core planning for animation frame add, duplicate, remove, duration adjustment, timeline moves, timeline goto/next/previous, and onion-size updates used by the live animation panel @@ -1170,8 +1171,8 @@ Results: add/duplicate/select/reorder/remove planning, metadata planning, bad-index rejection, bad-opacity rejection, bad-blend-mode rejection, transient highlight behavior, Layer menu labels/actions, merge-down routing, animated - merge blocking, missing selection handling, and bad Layer menu state - rejection. + merge blocking, missing selection handling, bad Layer menu state rejection, + Layer menu executor dispatch, and no-op menu execution preservation. - `pano_cli_plan_layer_rename_smoke`, `pano_cli_plan_layer_rename_no_op_smoke`, and `pano_cli_plan_layer_rename_rejects_empty_name` passed and expose live diff --git a/src/app_core/document_layer.h b/src/app_core/document_layer.h index 11b991b..31f1172 100644 --- a/src/app_core/document_layer.h +++ b/src/app_core/document_layer.h @@ -78,6 +78,16 @@ struct DocumentLayerMenuPlan { int to_index = 0; }; +class DocumentLayerMenuServices { +public: + virtual ~DocumentLayerMenuServices() = default; + + virtual void clear_current_layer() = 0; + virtual void show_rename_dialog() = 0; + virtual void merge_with_lower_layer(int from_index, int to_index) = 0; + virtual void show_merge_animated_not_supported() = 0; +}; + [[nodiscard]] inline pp::foundation::Status validate_layer_index( int layer_count, int index) noexcept @@ -410,4 +420,29 @@ struct DocumentLayerMenuPlan { return pp::foundation::Result::success(std::move(plan)); } +[[nodiscard]] inline pp::foundation::Status execute_document_layer_menu_plan( + const DocumentLayerMenuPlan& plan, + DocumentLayerMenuServices& services) +{ + switch (plan.action) { + case DocumentLayerMenuAction::clear_current_layer: + services.clear_current_layer(); + return pp::foundation::Status::success(); + case DocumentLayerMenuAction::show_rename_dialog: + services.show_rename_dialog(); + return pp::foundation::Status::success(); + case DocumentLayerMenuAction::merge_with_lower_layer: + services.merge_with_lower_layer(plan.from_index, plan.to_index); + return pp::foundation::Status::success(); + case DocumentLayerMenuAction::show_merge_animated_not_supported: + services.show_merge_animated_not_supported(); + return pp::foundation::Status::success(); + case DocumentLayerMenuAction::no_op_select_layer: + case DocumentLayerMenuAction::no_op_select_upper_layer: + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown document layer menu action"); +} + } diff --git a/src/app_layout.cpp b/src/app_layout.cpp index d5a0c30..624b2af 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -419,6 +419,39 @@ private: App& app_; }; +class LegacyDocumentLayerMenuServices final : public pp::app::DocumentLayerMenuServices { +public: + explicit LegacyDocumentLayerMenuServices(App& app) noexcept + : app_(app) + { + } + + void clear_current_layer() override + { + if (app_.canvas && app_.canvas->m_canvas) + app_.canvas->m_canvas->clear(); + } + + void show_rename_dialog() override + { + app_.dialog_layer_rename(); + } + + void merge_with_lower_layer(int from_index, int to_index) override + { + if (app_.layers) + app_.layers->merge(from_index, to_index, true); + } + + void show_merge_animated_not_supported() override + { + app_.message_box("Not supported", "Merging animated layers is not supported yet."); + } + +private: + App& app_; +}; + void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan) { LegacyMainToolbarServices services(app); @@ -443,6 +476,14 @@ void execute_tools_menu_plan(App& app, const pp::app::ToolsMenuPlan& plan) LOG("Tools menu action failed: %s", status.message); } +void execute_document_layer_menu_plan(App& app, const pp::app::DocumentLayerMenuPlan& plan) +{ + LegacyDocumentLayerMenuServices services(app); + const auto status = pp::app::execute_document_layer_menu_plan(plan, services); + if (!status.ok()) + LOG("Layer menu action failed: %s", status.message); +} + } // namespace void App::title_update() @@ -1832,8 +1873,7 @@ void App::init_menu_layer() popup->find("layer-clear")->on_click = [this, popup](Node*) { const auto plan = make_layer_menu_plan(pp::app::DocumentLayerMenuCommand::clear, *this); - if (plan.action == pp::app::DocumentLayerMenuAction::clear_current_layer) - canvas->m_canvas->clear(); + execute_document_layer_menu_plan(*this, plan); popup->mouse_release(); popup->destroy(); }; @@ -1846,8 +1886,7 @@ void App::init_menu_layer() popup->find("layer-rename")->on_click = [this, popup](Node*) { const auto plan = make_layer_menu_plan(pp::app::DocumentLayerMenuCommand::rename, *this); - if (plan.action == pp::app::DocumentLayerMenuAction::show_rename_dialog) - dialog_layer_rename(); + execute_document_layer_menu_plan(*this, plan); popup->mouse_release(); popup->destroy(); }; @@ -1860,14 +1899,7 @@ void App::init_menu_layer() popup->find("layer-merge")->on_click = [this, popup](Node*) { const auto plan = make_layer_menu_plan(pp::app::DocumentLayerMenuCommand::merge_down, *this); - if (plan.action == pp::app::DocumentLayerMenuAction::show_merge_animated_not_supported) - { - message_box("Not supported", "Merging animated layers is not supported yet."); - } - else if (plan.action == pp::app::DocumentLayerMenuAction::merge_with_lower_layer) - { - layers->merge(plan.from_index, plan.to_index, true); - } + execute_document_layer_menu_plan(*this, plan); popup->mouse_release(); popup->destroy(); }; diff --git a/tests/app_core/document_layer_tests.cpp b/tests/app_core/document_layer_tests.cpp index 372b86f..6c568c7 100644 --- a/tests/app_core/document_layer_tests.cpp +++ b/tests/app_core/document_layer_tests.cpp @@ -6,6 +6,31 @@ namespace { +class FakeDocumentLayerMenuServices final : public pp::app::DocumentLayerMenuServices { +public: + void clear_current_layer() override { clear_calls += 1; } + void show_rename_dialog() override { rename_dialogs += 1; } + void merge_with_lower_layer(int from_index, int to_index) override + { + merge_calls += 1; + last_from_index = from_index; + last_to_index = to_index; + } + void show_merge_animated_not_supported() override { merge_blocked_messages += 1; } + + [[nodiscard]] int total_calls() const noexcept + { + return clear_calls + rename_dialogs + merge_calls + merge_blocked_messages; + } + + int clear_calls = 0; + int rename_dialogs = 0; + int merge_calls = 0; + int merge_blocked_messages = 0; + int last_from_index = -1; + int last_to_index = -1; +}; + void layer_rename_records_changed_name(pp::tests::Harness& harness) { const auto plan = pp::app::plan_document_layer_rename("Base", "Paint"); @@ -287,6 +312,98 @@ void layer_menu_handles_missing_selection_and_bad_state(pp::tests::Harness& harn "Paint")); } +void layer_menu_executor_dispatches_menu_actions(pp::tests::Harness& harness) +{ + FakeDocumentLayerMenuServices services; + + const auto clear = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::clear, + true, + 1, + 1, + "Paint", + "Base"); + PP_EXPECT(harness, clear); + if (clear) { + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(clear.value(), services).ok()); + PP_EXPECT(harness, services.clear_calls == 1); + } + + const auto rename = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::rename, + true, + 1, + 1, + "Paint", + "Base"); + PP_EXPECT(harness, rename); + if (rename) { + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(rename.value(), services).ok()); + PP_EXPECT(harness, services.rename_dialogs == 1); + } + + const auto merge = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::merge_down, + true, + 2, + 1, + "Ink", + "Paint"); + PP_EXPECT(harness, merge); + if (merge) { + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(merge.value(), services).ok()); + PP_EXPECT(harness, services.merge_calls == 1); + PP_EXPECT(harness, services.last_from_index == 2); + PP_EXPECT(harness, services.last_to_index == 1); + } + + const auto animated = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::merge_down, + true, + 2, + 3, + "Ink", + "Paint"); + PP_EXPECT(harness, animated); + if (animated) { + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(animated.value(), services).ok()); + PP_EXPECT(harness, services.merge_blocked_messages == 1); + } +} + +void layer_menu_executor_preserves_no_op_actions(pp::tests::Harness& harness) +{ + FakeDocumentLayerMenuServices services; + + const auto missing = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::clear, + false, + 0, + 1, + "", + ""); + PP_EXPECT(harness, missing); + if (missing) { + PP_EXPECT(harness, missing.value().action == pp::app::DocumentLayerMenuAction::no_op_select_layer); + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(missing.value(), services).ok()); + } + + const auto base_layer = pp::app::plan_document_layer_menu( + pp::app::DocumentLayerMenuCommand::merge_down, + true, + 0, + 1, + "Base", + ""); + PP_EXPECT(harness, base_layer); + if (base_layer) { + PP_EXPECT(harness, base_layer.value().action == pp::app::DocumentLayerMenuAction::no_op_select_upper_layer); + PP_EXPECT(harness, pp::app::execute_document_layer_menu_plan(base_layer.value(), services).ok()); + } + + PP_EXPECT(harness, services.total_calls() == 0); +} + } int main() @@ -304,5 +421,7 @@ int main() harness.run("layer menu labels selected layer commands", layer_menu_labels_selected_layer_commands); harness.run("layer menu plans merge down or blocks it", layer_menu_plans_merge_down_or_blocks_it); harness.run("layer menu handles missing selection and bad state", layer_menu_handles_missing_selection_and_bad_state); + harness.run("layer menu executor dispatches menu actions", layer_menu_executor_dispatches_menu_actions); + harness.run("layer menu executor preserves no op actions", layer_menu_executor_preserves_no_op_actions); return harness.finish(); }