diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index c576cc7..5b58cc1 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -46,7 +46,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0026 | Open | Modernization | Toolbar and canvas history command planning now consumes pure `pp_app_core` through `App::init_toolbar_main`, `NodeCanvas`, and `pano_cli plan-history-operation`, but live execution still mutates legacy `ActionManager` stacks and `Canvas::I` unsaved state directly | Preserve undo/redo/clear behavior while moving action history toward document/app command services | `pp_app_core_history_ui_tests`; `pano_cli plan-history-operation --kind undo --undo-count 2`; `pano_cli plan-history-operation --kind clear --undo-count 2 --redo-count 1 --memory-bytes 4096`; `ctest --preset desktop-fast --build-config Debug` | Undo/redo/clear execution is owned by document/app history services with toolbar and canvas input acting only as adapters | | DEBT-0027 | Open | Modernization | Canvas draw-tool toolbar command, canvas input mode switching, and active-state planning now consume pure `pp_app_core` through `App::init_toolbar_draw`, `App::update`, `NodeCanvas`, `pano_cli plan-canvas-tool`, and `pano_cli plan-canvas-tool-state`, but live execution/state storage still mutates or reads legacy `Canvas` mode state, pen picking state, touch-lock state, and transform copy/cut action objects directly | Preserve current toolbar, stylus eraser, and keyboard draw/erase behavior while canvas input/tools move toward an app/document command boundary | `pp_app_core_canvas_tool_ui_tests`; `pano_cli plan-canvas-tool --kind copy`; `pano_cli plan-canvas-tool-state --mode draw --picking --touch-lock`; `ctest --preset desktop-fast --build-config Debug` | Canvas tool selection, toolbar state refresh, picking, touch lock, stylus eraser/key mode switching, and transform action execution are owned by app/document/canvas services with toolbar/canvas callbacks acting only as adapters | | DEBT-0028 | Open | Modernization | Canvas clear command planning and execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `pano_cli plan-canvas-clear`, and the `DocumentCanvasClearServices` boundary, but the live adapter still calls legacy `Canvas::clear`, which records `ActionLayerClear`, clears the current layer/frame, and marks legacy `Canvas::I` unsaved | Preserve clear-current-layer behavior while canvas/document commands move toward document/app command services | `pp_app_core_document_canvas_tests`; `pano_cli plan-canvas-clear --r 0 --g 0.1 --b 0.2 --a 0.3`; `pano_cli plan-canvas-clear --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Canvas clear execution, undo recording, dirty-state updates, and clear color handling are owned by injected document/app services with no legacy canvas-clear adapter | -| 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-0029 | Open | Modernization | Image import route planning and execution dispatch now consume pure `pp_app_core` through the File menu, `pano_cli plan-image-import`, and the `DocumentImageImportServices` boundary, but the live adapter still loads images with legacy `Image`, calls legacy `Canvas::import_equirectangular`, or configures legacy import transform mode directly | 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 injected document/app/asset services with File-menu callbacks acting only as adapters and no legacy image-import adapter | | DEBT-0030 | Open | Modernization | File export menu action planning and execution dispatch now consume pure `pp_app_core` through the File menu, `pano_cli plan-export-menu`, and the `DocumentExportMenuServices` boundary, but the live adapter 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 injected document/app/renderer/video services with File-menu callbacks acting only as UI adapters and no legacy export adapter | | DEBT-0031 | Open | Modernization | Top-level File menu command planning and execution dispatch now consume pure `pp_app_core` through `App::init_menu_file`, `pano_cli plan-file-menu`, and the `FileMenuServices` boundary, but the live adapter 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 injected app/document/platform services with `App::init_menu_file` acting only as a UI adapter and no legacy File menu adapter | | 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 | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 3d94463..6d01cc5 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -519,9 +519,10 @@ now dispatches through `DocumentCanvasClearServices` before the legacy `Canvas::clear` adapter continues. `pano_cli plan-image-import` exposes app-core planning for File > Import image route decisions, including wide equirectangular images, legacy vertical cube -strips, regular transform-placement images, and invalid image dimensions before -legacy `Canvas::import_equirectangular` or import transform-mode execution -continues. +strips, regular transform-placement images, and invalid image dimensions; live +File > Import execution now dispatches through `DocumentImageImportServices` +before legacy image loading, `Canvas::import_equirectangular`, or import +transform-mode setup continues. `pano_cli plan-file-menu` exposes app-core planning for the top-level File menu commands, including new/open/import, save/save-as/save-version, share, resize, cloud upload/browse, JPEG export, and export-submenu routing. Direct File menu @@ -1244,7 +1245,8 @@ Results: canvas clear planning as JSON automation. - `pp_app_core_document_import_tests` passed, covering wide equirectangular, legacy vertical cube strip, regular transform-placement, and invalid-dimension - import route decisions. + import route decisions, equirectangular service dispatch, transform import + dispatch, empty-path rejection, and invalid execution dimension rejection. - `pano_cli_plan_image_import_wide_equirect_smoke`, `pano_cli_plan_image_import_transform_smoke`, and `pano_cli_plan_image_import_rejects_invalid_dimensions` passed and expose File diff --git a/src/app_core/document_import.h b/src/app_core/document_import.h index 7c48816..fddda0e 100644 --- a/src/app_core/document_import.h +++ b/src/app_core/document_import.h @@ -2,6 +2,8 @@ #include "foundation/result.h" +#include + namespace pp::app { enum class DocumentImageImportAction { @@ -17,13 +19,31 @@ struct DocumentImageImportPlan { bool enters_transform_mode = false; }; -[[nodiscard]] inline pp::foundation::Result plan_document_image_import( +class DocumentImageImportServices { +public: + virtual ~DocumentImageImportServices() = default; + + virtual void import_equirectangular(std::string_view path) = 0; + virtual void enter_transform_import(std::string_view path) = 0; +}; + +[[nodiscard]] inline pp::foundation::Status validate_document_image_import_dimensions( int width, int height) noexcept { if (width <= 0 || height <= 0) { - return pp::foundation::Result::failure( - pp::foundation::Status::invalid_argument("image dimensions must be positive")); + return pp::foundation::Status::invalid_argument("image dimensions must be positive"); + } + return pp::foundation::Status::success(); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_image_import( + int width, + int height) noexcept +{ + const auto dimensions = validate_document_image_import_dimensions(width, height); + if (!dimensions.ok()) { + return pp::foundation::Result::failure(dimensions); } const auto wide_equirect = static_cast(width) == static_cast(height) * 2LL; @@ -40,4 +60,29 @@ struct DocumentImageImportPlan { return pp::foundation::Result::success(plan); } +[[nodiscard]] inline pp::foundation::Status execute_document_image_import_plan( + const DocumentImageImportPlan& plan, + std::string_view path, + DocumentImageImportServices& services) +{ + const auto dimensions = validate_document_image_import_dimensions(plan.width, plan.height); + if (!dimensions.ok()) { + return dimensions; + } + if (path.empty()) { + return pp::foundation::Status::invalid_argument("image import path must not be empty"); + } + + switch (plan.action) { + case DocumentImageImportAction::import_equirectangular: + services.import_equirectangular(path); + return pp::foundation::Status::success(); + case DocumentImageImportAction::place_transform: + services.enter_transform_import(path); + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown image import action"); +} + } // namespace pp::app diff --git a/src/app_layout.cpp b/src/app_layout.cpp index d3c1e8c..c6e95a8 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -143,17 +143,44 @@ public: if (!import_plan) return; - if (import_plan.value().imports_equirectangular) - { - Canvas::I->import_equirectangular(path); - } - else - { - auto m = static_cast(app_ptr->canvas->m_canvas->modes[(int)kCanvasMode::Import][0]); - m->m_action = CanvasModeTransform::ActionType::Import; - m->m_source_image = std::move(img); - Canvas::set_mode(kCanvasMode::Import); - } + class LegacyDocumentImageImportServices final : public pp::app::DocumentImageImportServices { + public: + LegacyDocumentImageImportServices(App& app, Image& image) noexcept + : app_(app) + , image_(image) + { + } + + void import_equirectangular(std::string_view import_path) override + { + if (Canvas::I) + Canvas::I->import_equirectangular(std::string(import_path)); + } + + void enter_transform_import(std::string_view) override + { + if (!app_.canvas || !app_.canvas->m_canvas) + return; + + auto* mode = static_cast( + app_.canvas->m_canvas->modes[(int)kCanvasMode::Import][0]); + mode->m_action = CanvasModeTransform::ActionType::Import; + mode->m_source_image = std::move(image_); + Canvas::set_mode(kCanvasMode::Import); + } + + private: + App& app_; + Image& image_; + }; + + LegacyDocumentImageImportServices services(*app_ptr, img); + const auto status = pp::app::execute_document_image_import_plan( + import_plan.value(), + path, + services); + if (!status.ok()) + LOG("Image import failed: %s", status.message); }); } diff --git a/tests/app_core/document_import_tests.cpp b/tests/app_core/document_import_tests.cpp index 4d354f9..724b850 100644 --- a/tests/app_core/document_import_tests.cpp +++ b/tests/app_core/document_import_tests.cpp @@ -1,8 +1,33 @@ #include "app_core/document_import.h" #include "test_harness.h" +#include +#include + namespace { +class FakeDocumentImageImportServices final : public pp::app::DocumentImageImportServices { +public: + void import_equirectangular(std::string_view path) override + { + equirectangular_imports += 1; + last_path = std::string(path); + call_order += "equirect;"; + } + + void enter_transform_import(std::string_view path) override + { + transform_imports += 1; + last_path = std::string(path); + call_order += "transform;"; + } + + int equirectangular_imports = 0; + int transform_imports = 0; + std::string last_path; + std::string call_order; +}; + void import_plan_detects_wide_equirectangular_images(pp::tests::Harness& harness) { const auto plan = pp::app::plan_document_image_import(4096, 2048); @@ -43,6 +68,72 @@ void import_plan_rejects_invalid_dimensions(pp::tests::Harness& harness) PP_EXPECT(harness, !pp::app::plan_document_image_import(-1, 1024)); } +void import_executor_dispatches_equirectangular_import(pp::tests::Harness& harness) +{ + FakeDocumentImageImportServices services; + const auto plan = pp::app::plan_document_image_import(4096, 2048); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, pp::app::execute_document_image_import_plan( + plan.value(), + "D:/Paint/equirect.png", + services).ok()); + } + + PP_EXPECT(harness, services.equirectangular_imports == 1); + PP_EXPECT(harness, services.transform_imports == 0); + PP_EXPECT(harness, services.last_path == "D:/Paint/equirect.png"); + PP_EXPECT(harness, services.call_order == "equirect;"); +} + +void import_executor_dispatches_transform_import(pp::tests::Harness& harness) +{ + FakeDocumentImageImportServices services; + const auto plan = pp::app::plan_document_image_import(1024, 1024); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, pp::app::execute_document_image_import_plan( + plan.value(), + "D:/Paint/flat.png", + services).ok()); + } + + PP_EXPECT(harness, services.equirectangular_imports == 0); + PP_EXPECT(harness, services.transform_imports == 1); + PP_EXPECT(harness, services.last_path == "D:/Paint/flat.png"); + PP_EXPECT(harness, services.call_order == "transform;"); +} + +void import_executor_rejects_empty_path(pp::tests::Harness& harness) +{ + FakeDocumentImageImportServices services; + const auto plan = pp::app::plan_document_image_import(1024, 1024); + PP_EXPECT(harness, plan); + if (plan) { + const auto status = pp::app::execute_document_image_import_plan(plan.value(), "", services); + PP_EXPECT(harness, !status.ok()); + PP_EXPECT(harness, status.code == pp::foundation::StatusCode::invalid_argument); + } + PP_EXPECT(harness, services.equirectangular_imports == 0); + PP_EXPECT(harness, services.transform_imports == 0); +} + +void import_executor_rejects_invalid_dimensions(pp::tests::Harness& harness) +{ + FakeDocumentImageImportServices services; + pp::app::DocumentImageImportPlan plan; + plan.width = 0; + plan.height = 1024; + plan.action = pp::app::DocumentImageImportAction::import_equirectangular; + plan.imports_equirectangular = true; + + const auto status = pp::app::execute_document_image_import_plan(plan, "D:/Paint/bad.png", services); + PP_EXPECT(harness, !status.ok()); + PP_EXPECT(harness, status.code == pp::foundation::StatusCode::invalid_argument); + PP_EXPECT(harness, services.equirectangular_imports == 0); + PP_EXPECT(harness, services.transform_imports == 0); +} + } int main() @@ -52,5 +143,9 @@ int main() harness.run("import plan detects legacy vertical cube strips", import_plan_detects_legacy_vertical_cube_strips); harness.run("import plan places regular images as transform sources", import_plan_places_regular_images_as_transform_sources); harness.run("import plan rejects invalid dimensions", import_plan_rejects_invalid_dimensions); + harness.run("import executor dispatches equirectangular import", import_executor_dispatches_equirectangular_import); + harness.run("import executor dispatches transform import", import_executor_dispatches_transform_import); + harness.run("import executor rejects empty path", import_executor_rejects_empty_path); + harness.run("import executor rejects invalid dimensions", import_executor_rejects_invalid_dimensions); return harness.finish(); }