From ef50f4a361465ce132eb060a72801b1a238a42af Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 3 Jun 2026 11:41:28 +0200 Subject: [PATCH] Extract image import route planning --- CMakeLists.txt | 1 + docs/modernization/debt.md | 1 + docs/modernization/roadmap.md | 12 ++++ src/app_core/document_import.h | 43 +++++++++++++ src/app_layout.cpp | 7 ++- tests/CMakeLists.txt | 28 +++++++++ tests/app_core/document_import_tests.cpp | 56 +++++++++++++++++ tools/pano_cli/main.cpp | 79 ++++++++++++++++++++++++ 8 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 src/app_core/document_import.h create mode 100644 tests/app_core/document_import_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index b99fc70..4e535ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,6 +231,7 @@ add_library(pp_app_core STATIC src/app_core/document_canvas.h src/app_core/document_cloud.h src/app_core/document_export.cpp + src/app_core/document_import.h src/app_core/document_layer.h src/app_core/document_platform_io.h src/app_core/document_recording.h diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index e2106bc..5cfa954 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -46,6 +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 now consumes pure `pp_app_core` through `App::init_toolbar_main` and `pano_cli plan-canvas-clear`, but live execution still calls legacy `Canvas::clear`, which records `ActionLayerClear`, clears the current layer/frame, and marks legacy `Canvas::I` unsaved directly | 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 document/app services with toolbar callbacks acting only as adapters | +| 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 | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index b8e2c02..c1ccfa0 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -511,6 +511,11 @@ legacy canvas mode execution continues. clear-current-layer command, including clear color validation, no-canvas handling, undo recording intent, and dirty-state intent before legacy `Canvas::clear` execution 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. `pano_cli plan-grid-operation` exposes app-core planning for grid heightmap pick/load/reload/clear, lightmap render capability/limit checks, and heightmap commit used by the live grid panel before legacy image loading, OpenGL texture @@ -1191,6 +1196,13 @@ Results: `pano_cli_plan_canvas_clear_no_canvas_smoke`, and `pano_cli_plan_canvas_clear_rejects_bad_color` passed and expose toolbar 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. +- `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 + > Import route planning as JSON automation. - `pp_app_core_history_ui_tests` passed, covering undo/redo availability, no-op history commands, clear-history stack/memory state, memory-only clear, and negative metric rejection. diff --git a/src/app_core/document_import.h b/src/app_core/document_import.h new file mode 100644 index 0000000..7c48816 --- /dev/null +++ b/src/app_core/document_import.h @@ -0,0 +1,43 @@ +#pragma once + +#include "foundation/result.h" + +namespace pp::app { + +enum class DocumentImageImportAction { + import_equirectangular, + place_transform, +}; + +struct DocumentImageImportPlan { + int width = 0; + int height = 0; + DocumentImageImportAction action = DocumentImageImportAction::place_transform; + bool imports_equirectangular = false; + bool enters_transform_mode = false; +}; + +[[nodiscard]] inline pp::foundation::Result plan_document_image_import( + 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")); + } + + const auto wide_equirect = static_cast(width) == static_cast(height) * 2LL; + const auto vertical_cube_strip = width == height / 6; + + DocumentImageImportPlan plan; + plan.width = width; + plan.height = height; + plan.imports_equirectangular = wide_equirect || vertical_cube_strip; + plan.enters_transform_mode = !plan.imports_equirectangular; + plan.action = plan.imports_equirectangular + ? DocumentImageImportAction::import_equirectangular + : DocumentImageImportAction::place_transform; + return pp::foundation::Result::success(plan); +} + +} // namespace pp::app diff --git a/src/app_layout.cpp b/src/app_layout.cpp index a444544..07992d7 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -11,6 +11,7 @@ #include "app_core/canvas_tool_ui.h" #include "app_core/document_layer.h" #include "app_core/document_canvas.h" +#include "app_core/document_import.h" #include "app_core/app_status.h" #include "app_core/history_ui.h" #include "settings.h" @@ -746,7 +747,11 @@ void App::init_menu_file() pick_image([this](std::string path){ Image img; img.load_file(path); - if (img.width == img.height / 6 || img.width == img.height * 2) + const auto import_plan = pp::app::plan_document_image_import(img.width, img.height); + if (!import_plan) + return; + + if (import_plan.value().imports_equirectangular) { Canvas::I->import_equirectangular(path); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b327acb..2b2ef6a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -338,6 +338,16 @@ add_test(NAME pp_app_core_document_canvas_tests COMMAND pp_app_core_document_can set_tests_properties(pp_app_core_document_canvas_tests PROPERTIES LABELS "app;document;desktop-fast;fuzz") +add_executable(pp_app_core_document_import_tests + app_core/document_import_tests.cpp) +target_link_libraries(pp_app_core_document_import_tests PRIVATE + pp_app_core + pp_test_harness) + +add_test(NAME pp_app_core_document_import_tests COMMAND pp_app_core_document_import_tests) +set_tests_properties(pp_app_core_document_import_tests PROPERTIES + LABELS "app;document;desktop-fast;fuzz") + add_executable(pp_app_core_document_export_tests app_core/document_export_tests.cpp) target_link_libraries(pp_app_core_document_export_tests PRIVATE @@ -788,6 +798,24 @@ if(TARGET pano_cli) LABELS "app;document;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_image_import_wide_equirect_smoke + COMMAND pano_cli plan-image-import --width 4096 --height 2048) + set_tests_properties(pano_cli_plan_image_import_wide_equirect_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-image-import\".*\"width\":4096.*\"height\":2048.*\"action\":\"import-equirectangular\".*\"importsEquirectangular\":true.*\"entersTransformMode\":false") + + add_test(NAME pano_cli_plan_image_import_transform_smoke + COMMAND pano_cli plan-image-import --width 1024 --height 1024) + set_tests_properties(pano_cli_plan_image_import_transform_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-image-import\".*\"width\":1024.*\"height\":1024.*\"action\":\"place-transform\".*\"importsEquirectangular\":false.*\"entersTransformMode\":true") + + add_test(NAME pano_cli_plan_image_import_rejects_invalid_dimensions + COMMAND pano_cli plan-image-import --width 0 --height 1024) + set_tests_properties(pano_cli_plan_image_import_rejects_invalid_dimensions PROPERTIES + LABELS "app;document;integration;desktop-fast;fuzz" + WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_layer_rename_smoke COMMAND pano_cli plan-layer-rename --old-name Base --new-name Paint) set_tests_properties(pano_cli_plan_layer_rename_smoke PROPERTIES diff --git a/tests/app_core/document_import_tests.cpp b/tests/app_core/document_import_tests.cpp new file mode 100644 index 0000000..4d354f9 --- /dev/null +++ b/tests/app_core/document_import_tests.cpp @@ -0,0 +1,56 @@ +#include "app_core/document_import.h" +#include "test_harness.h" + +namespace { + +void import_plan_detects_wide_equirectangular_images(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_image_import(4096, 2048); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().action == pp::app::DocumentImageImportAction::import_equirectangular); + PP_EXPECT(harness, plan.value().imports_equirectangular); + PP_EXPECT(harness, !plan.value().enters_transform_mode); + } +} + +void import_plan_detects_legacy_vertical_cube_strips(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_image_import(512, 3072); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().action == pp::app::DocumentImageImportAction::import_equirectangular); + PP_EXPECT(harness, plan.value().imports_equirectangular); + PP_EXPECT(harness, !plan.value().enters_transform_mode); + } +} + +void import_plan_places_regular_images_as_transform_sources(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_image_import(1024, 1024); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().action == pp::app::DocumentImageImportAction::place_transform); + PP_EXPECT(harness, !plan.value().imports_equirectangular); + PP_EXPECT(harness, plan.value().enters_transform_mode); + } +} + +void import_plan_rejects_invalid_dimensions(pp::tests::Harness& harness) +{ + PP_EXPECT(harness, !pp::app::plan_document_image_import(0, 1024)); + PP_EXPECT(harness, !pp::app::plan_document_image_import(1024, 0)); + PP_EXPECT(harness, !pp::app::plan_document_image_import(-1, 1024)); +} + +} + +int main() +{ + pp::tests::Harness harness; + harness.run("import plan detects wide equirectangular images", import_plan_detects_wide_equirectangular_images); + 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); + return harness.finish(); +} diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index c41ebca..3ab6edf 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -5,6 +5,7 @@ #include "app_core/document_animation.h" #include "app_core/document_canvas.h" #include "app_core/document_export.h" +#include "app_core/document_import.h" #include "app_core/document_cloud.h" #include "app_core/document_layer.h" #include "app_core/document_platform_io.h" @@ -235,6 +236,11 @@ struct PlanCanvasClearArgs { float a = 0.0F; }; +struct PlanImageImportArgs { + int width = 0; + int height = 0; +}; + struct PlanLayerRenameArgs { std::string old_name = "Layer 1"; std::string new_name; @@ -488,6 +494,18 @@ const char* document_open_plan_action_name(pp::app::DocumentOpenPlanAction actio return "open-project-now"; } +const char* document_image_import_action_name(pp::app::DocumentImageImportAction action) noexcept +{ + switch (action) { + case pp::app::DocumentImageImportAction::import_equirectangular: + return "import-equirectangular"; + case pp::app::DocumentImageImportAction::place_transform: + return "place-transform"; + } + + return "place-transform"; +} + const char* close_request_decision_name(pp::app::CloseRequestDecision decision) noexcept { switch (decision) { @@ -1015,6 +1033,7 @@ void print_help() << " plan-app-preferences [--ui-scale N] [--display-density N] [--current-scale N] [--scale-option N] [--viewport-scale N] [--rtl] [--timelapse-disabled] [--recording-running] [--vr-controllers-disabled] [--cursor-mode N]\n" << " plan-app-status [--doc-name NAME] [--unsaved] [--resolution N] [--resolution-index N] [--zoom N] [--history-bytes N] [--recording-running] [--encoder-available] [--encoded-frames N]\n" << " plan-canvas-clear [--no-canvas] [--r N] [--g N] [--b N] [--a N]\n" + << " plan-image-import --width N --height N\n" << " plan-document-resize [--current-resolution N] [--selected-resolution-index N]\n" << " plan-layer-rename --old-name NAME --new-name NAME\n" << " plan-layer-operation --kind add|duplicate|select|reorder|remove|opacity|visibility|alpha-lock|blend-mode|highlight [--layer-count N] [--index N] [--from-index N] [--to-index N] [--source-index N] [--name NAME] [--opacity N] [--blend-mode N] [--enabled]\n" @@ -2692,6 +2711,62 @@ int plan_canvas_clear(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_image_import_args( + int argc, + char** argv, + PlanImageImportArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--width" || key == "--height") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + const auto value = parse_i32_arg(argv[++i]); + if (!value) { + return value.status(); + } + if (key == "--width") { + args.width = value.value(); + } else { + args.height = value.value(); + } + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +int plan_image_import(int argc, char** argv) +{ + PlanImageImportArgs args; + const auto status = parse_plan_image_import_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-image-import", status.message); + return 2; + } + + const auto plan = pp::app::plan_document_image_import(args.width, args.height); + if (!plan) { + print_error("plan-image-import", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-image-import\"" + << ",\"state\":{\"width\":" << args.width + << ",\"height\":" << args.height + << "},\"plan\":{\"width\":" << value.width + << ",\"height\":" << value.height + << ",\"action\":\"" << document_image_import_action_name(value.action) + << "\",\"importsEquirectangular\":" << json_bool(value.imports_equirectangular) + << ",\"entersTransformMode\":" << json_bool(value.enters_transform_mode) + << "}}\n"; + return 0; +} + pp::foundation::Status parse_plan_layer_rename_args( int argc, char** argv, @@ -6086,6 +6161,10 @@ int main(int argc, char** argv) return plan_canvas_clear(argc, argv); } + if (command == "plan-image-import") { + return plan_image_import(argc, argv); + } + if (command == "plan-layer-rename") { return plan_layer_rename(argc, argv); }