From fdc1defaba9efa74739b1e5014a3aaf3741ca3b0 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 3 Jun 2026 10:20:37 +0200 Subject: [PATCH] Extract layer operation planning --- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 20 +- src/app_core/document_layer.h | 283 ++++++++++++++++++++++++ src/app_layout.cpp | 140 +++++++++--- tests/CMakeLists.txt | 24 ++ tests/app_core/document_layer_tests.cpp | 133 +++++++++++ tools/pano_cli/main.cpp | 188 ++++++++++++++++ 7 files changed, 752 insertions(+), 38 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 06e3433..dc78d9b 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -38,7 +38,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0017 | Open | Modernization | Startup storage path preparation, `App::clipboard_get_text`, `App::clipboard_set_text`, `App::show_cursor`, `App::hide_cursor`, `App::showKeyboard`, `App::hideKeyboard`, `App::display_file`, `App::share_file`, native app/window close, UI-thread lifecycle hooks, render-context acquire/release/present hooks, render-target binding hooks, render platform hint hooks, render debug callback hooks, render-capture frame hooks, recording cleanup, live asset/layout reload policy, diagnostic stacktrace/crash hooks, per-frame platform hooks, `App::pick_image`, `App::pick_file`, the non-writer `App::pick_file_save`, `App::pick_dir`, and prepared-file save/download handoff now call the SDK-free `pp::platform::PlatformServices` interface, and Windows injects `WindowsPlatformServices` from `src/platform_windows/windows_platform_services.*`; non-Windows live implementations still use `src/platform_legacy/legacy_platform_services.*`, a named fallback adapter that forwards to retained Apple/Android/Linux/Web bridge functions and retained no-op branches | Preserve behavior while moving platform execution behind a testable service boundary before platform shell implementations are injected | `pp_platform_api_tests`; `pp_app_core_document_platform_io_tests`; `ctest --preset desktop-fast --build-config Debug`; `powershell -ExecutionPolicy Bypass -File scripts\automation\package-smoke.ps1 -Preset windows-msvc-default -Configuration Debug` | Replace `src/platform_legacy/legacy_platform_services.*` with injected `pp_platform_*` service implementations owned by each non-Windows platform shell | | DEBT-0019 | Open | Modernization | MSVC warning C4100 is muted globally through `pp_project_warnings` with `/wd4100` | Legacy callbacks, virtual hooks, serializer methods, and platform/API compatibility functions carry many intentionally unused parameters during the component split; muting this keeps stricter warning builds focused on higher-signal migration issues | `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter`; `ctest --preset desktop-fast --build-config Debug` | Remove `/wd4100`, mark intentionally unused parameters with names/comments or `[[maybe_unused]]`, and make the Windows app and headless tests pass without C4100 warnings | | DEBT-0020 | Open | Modernization | Document resize dialog state and selected-resolution planning now consume pure `pp_app_core` through `NodeDialogResize`, `App::dialog_resize`, and `pano_cli plan-document-resize`, but live resize execution still calls legacy `Canvas::resize` and clears legacy `ActionManager` history directly | Preserve existing layer/frame GPU resize behavior while the document model and canvas execution boundary are extracted incrementally | `pp_app_core_document_resize_tests`; `pano_cli plan-document-resize --current-resolution 2048 --selected-resolution-index 4`; `ctest --preset desktop-fast --build-config Debug` | Document resize execution is owned by a document/app boundary with legacy `Canvas` acting only as an adapter or removed entirely | -| DEBT-0021 | Open | Modernization | Layer rename planning now consumes pure `pp_app_core` through `App::dialog_layer_rename` and `pano_cli plan-layer-rename`, but live rename execution still mutates legacy `Canvas` layer state, `NodeLayer`, and `ActionManager` undo entries directly | Preserve existing UI/canvas behavior while document layer commands and undo history are extracted incrementally | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-rename --old-name Base --new-name Paint`; `ctest --preset desktop-fast --build-config Debug` | Layer rename execution is owned by the document/app command boundary with legacy `Canvas`/UI nodes acting only as adapters or removed entirely | +| DEBT-0021 | Open | Modernization | Layer rename and layer panel operation planning now consume pure `pp_app_core` through `App::dialog_layer_rename`, `App::init_sidebar` layer callbacks, `pano_cli plan-layer-rename`, and `pano_cli plan-layer-operation`, but live execution still mutates legacy `Canvas` layer state, `NodeLayer`/`NodePanelLayer`, and `ActionManager` undo entries directly | Preserve existing UI/canvas behavior while document layer commands and undo history are extracted incrementally | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-rename --old-name Base --new-name Paint`; `pano_cli plan-layer-operation --kind add --layer-count 2 --index 1 --name Paint`; `ctest --preset desktop-fast --build-config Debug` | Layer command execution is owned by the document/app command boundary with legacy `Canvas`/UI nodes acting only as adapters or removed entirely | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 4637440..d3b45a2 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -250,7 +250,8 @@ Implementation tasks: - Set C++23 through target features, not raw compiler flags. - Add warning profiles: - - MSVC: `/W4 /permissive- /Zc:__cplusplus /Zc:preprocessor`. + - MSVC: `/W4 /permissive- /Zc:__cplusplus /Zc:preprocessor`, with + `C4100` muted temporarily under `DEBT-0019`. - Optional MSVC analysis preset: `/analyze`. - Clang/GCC: `-Wall -Wextra -Wpedantic -Wconversion -Wshadow -Wnull-dereference`. @@ -289,7 +290,8 @@ Gate: - Desktop library targets compile with strict diagnostics. - New warnings caused by refactor are fixed or locally justified. -- No global blanket warning suppression for project code. +- Any global warning suppression must have an open debt entry, validation + command, and removal condition. ## Phase 3: Test Harness And Agent-Ready Automation @@ -484,6 +486,10 @@ legacy `Canvas` resize execution and `ActionManager` history clearing continue. `pano_cli plan-layer-rename` exposes the app-core layer rename decision used by the live layer rename dialog before legacy `Canvas` layer mutation and `ActionManager` undo wiring continue. +`pano_cli plan-layer-operation` exposes app-core planning for layer add, +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. `pp_platform_api` now owns a headless `PlatformServices` interface for startup storage path preparation, clipboard text, cursor visibility, virtual-keyboard visibility, UI-thread lifecycle hooks, render-context @@ -1091,11 +1097,19 @@ Results: `pano_cli_plan_document_resize_rejects_invalid_selection` passed and expose live document-resize planning as JSON automation. - `pp_app_core_document_layer_tests` passed, covering changed layer rename, - unchanged no-op rename, empty-name rejection, and overlong-name rejection. + unchanged no-op rename, empty-name rejection, overlong-name rejection, layer + add/duplicate/select/reorder/remove planning, metadata planning, bad-index + rejection, bad-opacity rejection, bad-blend-mode rejection, and transient + highlight behavior. - `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 layer-rename planning as JSON automation. +- `pano_cli_plan_layer_operation_add_smoke`, + `pano_cli_plan_layer_operation_reorder_no_op_smoke`, + `pano_cli_plan_layer_operation_highlight_smoke`, and + `pano_cli_plan_layer_operation_rejects_bad_opacity` passed and expose live + layer-panel operation planning as JSON automation. - `pp_app_core_document_sharing_tests` passed, covering saved-path gating before platform share execution. - `pano_cli_plan_share_file_unsaved_smoke` and diff --git a/src/app_core/document_layer.h b/src/app_core/document_layer.h index 04b1155..deab20d 100644 --- a/src/app_core/document_layer.h +++ b/src/app_core/document_layer.h @@ -2,7 +2,9 @@ #include "foundation/result.h" +#include #include +#include #include #include #include @@ -10,18 +12,79 @@ namespace pp::app { inline constexpr std::size_t document_layer_name_max_length = 128; +inline constexpr int document_layer_legacy_blend_mode_count = 5; enum class DocumentLayerRenameAction { no_op_same_name, rename_and_record_undo, }; +enum class DocumentLayerOperation { + add, + duplicate, + select, + reorder, + remove, + set_opacity, + set_visibility, + set_alpha_lock, + set_blend_mode, + set_highlight, +}; + struct DocumentLayerRenamePlan { std::string old_name; std::string new_name; DocumentLayerRenameAction action = DocumentLayerRenameAction::no_op_same_name; }; +struct DocumentLayerOperationPlan { + DocumentLayerOperation operation = DocumentLayerOperation::select; + int index = 0; + int from_index = 0; + int to_index = 0; + int insert_index = 0; + int source_index = 0; + std::string name; + float opacity = 1.0F; + bool flag = false; + int blend_mode = 0; + bool mutates_document = false; + bool marks_unsaved = false; + bool reloads_animation_layers = false; + bool updates_title = false; +}; + +[[nodiscard]] inline pp::foundation::Status validate_layer_index( + int layer_count, + int index) noexcept +{ + if (layer_count <= 0) { + return pp::foundation::Status::invalid_argument("document must contain at least one layer"); + } + + if (index < 0 || index >= layer_count) { + return pp::foundation::Status::out_of_range("layer index is outside the document"); + } + + return pp::foundation::Status::success(); +} + +[[nodiscard]] inline pp::foundation::Status validate_layer_insert_index( + int layer_count, + int index) noexcept +{ + if (layer_count < 0) { + return pp::foundation::Status::invalid_argument("layer count must not be negative"); + } + + if (index < 0 || index > layer_count) { + return pp::foundation::Status::out_of_range("layer insert index is outside the document"); + } + + return pp::foundation::Status::success(); +} + [[nodiscard]] inline pp::foundation::Result plan_document_layer_rename( std::string_view old_name, std::string_view requested_name) @@ -45,4 +108,224 @@ struct DocumentLayerRenamePlan { return pp::foundation::Result::success(std::move(plan)); } +[[nodiscard]] inline pp::foundation::Result plan_document_layer_add( + int layer_count, + int insert_index, + std::string_view name) +{ + const auto index_status = validate_layer_insert_index(layer_count, insert_index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + const auto rename = plan_document_layer_rename({}, name); + if (!rename) { + return pp::foundation::Result::failure(rename.status()); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::add; + plan.insert_index = insert_index; + plan.name = std::string(name); + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.reloads_animation_layers = true; + plan.updates_title = true; + return pp::foundation::Result::success(std::move(plan)); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_duplicate( + int layer_count, + int source_index) +{ + const auto index_status = validate_layer_index(layer_count, source_index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::duplicate; + plan.source_index = source_index; + plan.insert_index = source_index + 1; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.reloads_animation_layers = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_select( + int layer_count, + int index) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::select; + plan.index = index; + plan.reloads_animation_layers = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_reorder( + int layer_count, + int from_index, + int to_index) +{ + auto index_status = validate_layer_index(layer_count, from_index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + index_status = validate_layer_index(layer_count, to_index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::reorder; + plan.from_index = from_index; + plan.to_index = to_index; + plan.mutates_document = from_index != to_index; + plan.marks_unsaved = plan.mutates_document; + plan.reloads_animation_layers = plan.mutates_document; + plan.updates_title = plan.mutates_document; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_remove( + int layer_count, + int index) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + if (layer_count <= 1) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("document must keep at least one layer")); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::remove; + plan.index = index; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.reloads_animation_layers = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_opacity( + int layer_count, + int index, + float opacity) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + if (!std::isfinite(opacity) || opacity < 0.0F || opacity > 1.0F) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("layer opacity must be finite and within 0..1")); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::set_opacity; + plan.index = index; + plan.opacity = opacity; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_visibility( + int layer_count, + int index, + bool visible) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::set_visibility; + plan.index = index; + plan.flag = visible; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.reloads_animation_layers = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_alpha_lock( + int layer_count, + int index, + bool locked) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::set_alpha_lock; + plan.index = index; + plan.flag = locked; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_blend_mode( + int layer_count, + int index, + int blend_mode) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + if (blend_mode < 0 || blend_mode >= document_layer_legacy_blend_mode_count) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("layer blend mode is outside the supported range")); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::set_blend_mode; + plan.index = index; + plan.blend_mode = blend_mode; + plan.mutates_document = true; + plan.marks_unsaved = true; + plan.updates_title = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_layer_highlight( + int layer_count, + int index, + bool highlight) +{ + const auto index_status = validate_layer_index(layer_count, index); + if (!index_status.ok()) { + return pp::foundation::Result::failure(index_status); + } + + DocumentLayerOperationPlan plan; + plan.operation = DocumentLayerOperation::set_highlight; + plan.index = index; + plan.flag = highlight; + return pp::foundation::Result::success(plan); +} + } diff --git a/src/app_layout.cpp b/src/app_layout.cpp index 7292de1..f8a495d 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -7,6 +7,7 @@ #include "node_dialog_picker.h" #include "node_panel_floating.h" #include "app_core/app_preferences.h" +#include "app_core/document_layer.h" #include "app_core/app_status.h" #include "settings.h" #include "serializer.h" @@ -182,17 +183,30 @@ void App::init_sidebar() }; layers->on_layer_add = [this](Node*, std::shared_ptr layer, int index) { - Canvas::I->layer_add(layers->m_layers.back()->m_label_text.c_str(), layer, index); - Canvas::I->m_unsaved = true; + const auto plan = pp::app::plan_document_layer_add( + static_cast(Canvas::I->m_layers.size()), + index, + layers->m_layers.back()->m_label_text); + if (!plan) + return; + Canvas::I->layer_add(plan.value().name, layer, plan.value().insert_index); + Canvas::I->m_unsaved = plan.value().marks_unsaved; Canvas::I->anim_update(); - animation->load_layers(); - title_update(); + if (plan.value().reloads_animation_layers) + animation->load_layers(); + if (plan.value().updates_title) + title_update(); }; layers->on_layer_duplicate = [this](Node*, int source_index) { - Canvas::I->layer_add(layers->m_layers.back()->m_label_text.c_str(), nullptr, source_index + 1); - auto& dst = Canvas::I->m_layers[source_index + 1]; - auto& src = Canvas::I->m_layers[source_index]; + const auto plan = pp::app::plan_document_layer_duplicate( + static_cast(Canvas::I->m_layers.size()), + source_index); + if (!plan) + return; + Canvas::I->layer_add(layers->m_layers.back()->m_label_text.c_str(), nullptr, plan.value().insert_index); + auto& dst = Canvas::I->m_layers[plan.value().insert_index]; + auto& src = Canvas::I->m_layers[plan.value().source_index]; for (int i = 1; i < src->frames_count(); i++) dst->add_frame(); Canvas::I->anim_update(); @@ -217,57 +231,115 @@ void App::init_sidebar() dst->m_opacity = src->m_opacity; dst->m_blend_mode = src->m_blend_mode; dst->m_alpha_locked = src->m_alpha_locked; - Canvas::I->m_unsaved = true; - animation->load_layers(); - title_update(); + Canvas::I->m_unsaved = plan.value().marks_unsaved; + if (plan.value().reloads_animation_layers) + animation->load_layers(); + if (plan.value().updates_title) + title_update(); }; layers->on_layer_change = [this](Node*, int old_idx, int new_idx) { - canvas->m_canvas->m_current_layer_idx = new_idx; - animation->load_layers(); + const auto plan = pp::app::plan_document_layer_select( + static_cast(canvas->m_canvas->m_layers.size()), + new_idx); + if (!plan) + return; + canvas->m_canvas->m_current_layer_idx = plan.value().index; + if (plan.value().reloads_animation_layers) + animation->load_layers(); }; layers->on_layer_order = [this](Node*, int old_idx, int new_idx) { - canvas->m_canvas->layer_order(old_idx, new_idx); - canvas->m_canvas->m_unsaved = true; - animation->load_layers(); - title_update(); + const auto plan = pp::app::plan_document_layer_reorder( + static_cast(canvas->m_canvas->m_layers.size()), + old_idx, + new_idx); + if (!plan || !plan.value().mutates_document) + return; + canvas->m_canvas->layer_order(plan.value().from_index, plan.value().to_index); + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().reloads_animation_layers) + animation->load_layers(); + if (plan.value().updates_title) + title_update(); }; layers->on_layer_delete = [this](Node*, int idx) { - canvas->m_canvas->layer_remove(idx); - canvas->m_canvas->m_unsaved = true; - animation->load_layers(); - title_update(); + const auto plan = pp::app::plan_document_layer_remove( + static_cast(canvas->m_canvas->m_layers.size()), + idx); + if (!plan) + return; + canvas->m_canvas->layer_remove(plan.value().index); + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().reloads_animation_layers) + animation->load_layers(); + if (plan.value().updates_title) + title_update(); }; layers->on_layer_opacity_changed = [this](Node*, int idx, float value) { - canvas->m_canvas->m_layers[idx]->m_opacity = value; - canvas->m_canvas->m_unsaved = true; - title_update(); + const auto plan = pp::app::plan_document_layer_opacity( + static_cast(canvas->m_canvas->m_layers.size()), + idx, + value); + if (!plan) + return; + canvas->m_canvas->m_layers[plan.value().index]->m_opacity = plan.value().opacity; + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().updates_title) + title_update(); }; layers->on_layer_visibility_changed = [this](Node*, int idx, bool visible) { - canvas->m_canvas->m_layers[idx]->m_visible = visible; - canvas->m_canvas->m_unsaved = true; - animation->load_layers(); - title_update(); + const auto plan = pp::app::plan_document_layer_visibility( + static_cast(canvas->m_canvas->m_layers.size()), + idx, + visible); + if (!plan) + return; + canvas->m_canvas->m_layers[plan.value().index]->m_visible = plan.value().flag; + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().reloads_animation_layers) + animation->load_layers(); + if (plan.value().updates_title) + title_update(); }; layers->on_layer_alpha_lock_changed = [this](Node*, int idx, bool locked) { - canvas->m_canvas->m_layers[idx]->m_alpha_locked = locked; - canvas->m_canvas->m_unsaved = true; - title_update(); + const auto plan = pp::app::plan_document_layer_alpha_lock( + static_cast(canvas->m_canvas->m_layers.size()), + idx, + locked); + if (!plan) + return; + canvas->m_canvas->m_layers[plan.value().index]->m_alpha_locked = plan.value().flag; + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().updates_title) + title_update(); }; layers->on_layer_blend_mode_changed = [this](Node*, int idx, int mode) { - canvas->m_canvas->m_layers[idx]->m_blend_mode = mode; - canvas->m_canvas->m_unsaved = true; - title_update(); + const auto plan = pp::app::plan_document_layer_blend_mode( + static_cast(canvas->m_canvas->m_layers.size()), + idx, + mode); + if (!plan) + return; + canvas->m_canvas->m_layers[plan.value().index]->m_blend_mode = plan.value().blend_mode; + canvas->m_canvas->m_unsaved = plan.value().marks_unsaved; + if (plan.value().updates_title) + title_update(); }; layers->on_layer_highlight_changed = [this](Node*, int idx, bool highlight) { - canvas->m_canvas->m_layers[idx]->m_hightlight = highlight; + const auto plan = pp::app::plan_document_layer_highlight( + static_cast(canvas->m_canvas->m_layers.size()), + idx, + highlight); + if (!plan) + return; + canvas->m_canvas->m_layers[plan.value().index]->m_hightlight = plan.value().flag; }; if (auto* button = layout[main_id]->find("btn-stroke")) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c499485..e42e6b2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -718,6 +718,30 @@ if(TARGET pano_cli) LABELS "app;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_layer_operation_add_smoke + COMMAND pano_cli plan-layer-operation --kind add --layer-count 2 --index 1 --name Paint) + set_tests_properties(pano_cli_plan_layer_operation_add_smoke PROPERTIES + LABELS "app;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-layer-operation\".*\"operation\":\"add\".*\"insertIndex\":1.*\"name\":\"Paint\".*\"marksUnsaved\":true.*\"reloadsAnimationLayers\":true") + + add_test(NAME pano_cli_plan_layer_operation_reorder_no_op_smoke + COMMAND pano_cli plan-layer-operation --kind reorder --layer-count 3 --from-index 1 --to-index 1) + set_tests_properties(pano_cli_plan_layer_operation_reorder_no_op_smoke PROPERTIES + LABELS "app;integration;desktop-fast;fuzz" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-layer-operation\".*\"operation\":\"reorder\".*\"fromIndex\":1.*\"toIndex\":1.*\"mutatesDocument\":false.*\"marksUnsaved\":false") + + add_test(NAME pano_cli_plan_layer_operation_highlight_smoke + COMMAND pano_cli plan-layer-operation --kind highlight --layer-count 2 --index 1 --enabled) + set_tests_properties(pano_cli_plan_layer_operation_highlight_smoke PROPERTIES + LABELS "app;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-layer-operation\".*\"operation\":\"set-highlight\".*\"index\":1.*\"flag\":true.*\"mutatesDocument\":false.*\"updatesTitle\":false") + + add_test(NAME pano_cli_plan_layer_operation_rejects_bad_opacity + COMMAND pano_cli plan-layer-operation --kind opacity --layer-count 2 --index 1 --opacity 1.5) + set_tests_properties(pano_cli_plan_layer_operation_rejects_bad_opacity PROPERTIES + LABELS "app;integration;desktop-fast;fuzz" + WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_share_file_unsaved_smoke COMMAND pano_cli plan-share-file) set_tests_properties(pano_cli_plan_share_file_unsaved_smoke PROPERTIES diff --git a/tests/app_core/document_layer_tests.cpp b/tests/app_core/document_layer_tests.cpp index d496ef9..d0ce969 100644 --- a/tests/app_core/document_layer_tests.cpp +++ b/tests/app_core/document_layer_tests.cpp @@ -1,6 +1,7 @@ #include "app_core/document_layer.h" #include "test_harness.h" +#include #include namespace { @@ -50,6 +51,133 @@ void layer_rename_rejects_overlong_name(pp::tests::Harness& harness) } } +void layer_add_validates_insert_index_and_name(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_layer_add(2, 1, "Paint"); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().operation == pp::app::DocumentLayerOperation::add); + PP_EXPECT(harness, plan.value().insert_index == 1); + PP_EXPECT(harness, plan.value().name == "Paint"); + PP_EXPECT(harness, plan.value().marks_unsaved); + PP_EXPECT(harness, plan.value().reloads_animation_layers); + } + + PP_EXPECT(harness, !pp::app::plan_document_layer_add(2, -1, "Paint")); + PP_EXPECT(harness, !pp::app::plan_document_layer_add(2, 3, "Paint")); + PP_EXPECT(harness, !pp::app::plan_document_layer_add(2, 1, "")); +} + +void layer_duplicate_select_and_reorder_validate_indices(pp::tests::Harness& harness) +{ + const auto duplicate = pp::app::plan_document_layer_duplicate(3, 1); + PP_EXPECT(harness, duplicate); + if (duplicate) { + PP_EXPECT(harness, duplicate.value().source_index == 1); + PP_EXPECT(harness, duplicate.value().insert_index == 2); + PP_EXPECT(harness, duplicate.value().marks_unsaved); + } + + const auto select = pp::app::plan_document_layer_select(3, 2); + PP_EXPECT(harness, select); + if (select) { + PP_EXPECT(harness, select.value().index == 2); + PP_EXPECT(harness, !select.value().marks_unsaved); + PP_EXPECT(harness, select.value().reloads_animation_layers); + } + + const auto reorder = pp::app::plan_document_layer_reorder(3, 2, 0); + PP_EXPECT(harness, reorder); + if (reorder) { + PP_EXPECT(harness, reorder.value().from_index == 2); + PP_EXPECT(harness, reorder.value().to_index == 0); + PP_EXPECT(harness, reorder.value().marks_unsaved); + } + + const auto no_op_reorder = pp::app::plan_document_layer_reorder(3, 1, 1); + PP_EXPECT(harness, no_op_reorder); + if (no_op_reorder) { + PP_EXPECT(harness, !no_op_reorder.value().mutates_document); + PP_EXPECT(harness, !no_op_reorder.value().marks_unsaved); + } + + PP_EXPECT(harness, !pp::app::plan_document_layer_duplicate(3, 3)); + PP_EXPECT(harness, !pp::app::plan_document_layer_select(3, -1)); + PP_EXPECT(harness, !pp::app::plan_document_layer_reorder(3, 0, 3)); +} + +void layer_remove_keeps_at_least_one_layer(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_layer_remove(2, 0); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().operation == pp::app::DocumentLayerOperation::remove); + PP_EXPECT(harness, plan.value().index == 0); + PP_EXPECT(harness, plan.value().marks_unsaved); + PP_EXPECT(harness, plan.value().reloads_animation_layers); + } + + PP_EXPECT(harness, !pp::app::plan_document_layer_remove(1, 0)); + PP_EXPECT(harness, !pp::app::plan_document_layer_remove(2, 2)); +} + +void layer_metadata_plans_validate_values(pp::tests::Harness& harness) +{ + const auto opacity = pp::app::plan_document_layer_opacity(2, 1, 0.25F); + PP_EXPECT(harness, opacity); + if (opacity) { + PP_EXPECT(harness, opacity.value().operation == pp::app::DocumentLayerOperation::set_opacity); + PP_EXPECT(harness, opacity.value().opacity == 0.25F); + PP_EXPECT(harness, opacity.value().marks_unsaved); + PP_EXPECT(harness, !opacity.value().reloads_animation_layers); + } + + const auto visibility = pp::app::plan_document_layer_visibility(2, 1, false); + PP_EXPECT(harness, visibility); + if (visibility) { + PP_EXPECT(harness, visibility.value().operation == pp::app::DocumentLayerOperation::set_visibility); + PP_EXPECT(harness, !visibility.value().flag); + PP_EXPECT(harness, visibility.value().reloads_animation_layers); + } + + const auto alpha_lock = pp::app::plan_document_layer_alpha_lock(2, 1, true); + PP_EXPECT(harness, alpha_lock); + if (alpha_lock) { + PP_EXPECT(harness, alpha_lock.value().operation == pp::app::DocumentLayerOperation::set_alpha_lock); + PP_EXPECT(harness, alpha_lock.value().flag); + } + + const auto blend = pp::app::plan_document_layer_blend_mode(2, 1, 4); + PP_EXPECT(harness, blend); + if (blend) { + PP_EXPECT(harness, blend.value().operation == pp::app::DocumentLayerOperation::set_blend_mode); + PP_EXPECT(harness, blend.value().blend_mode == 4); + } + + PP_EXPECT(harness, !pp::app::plan_document_layer_opacity(2, 1, -0.1F)); + PP_EXPECT(harness, !pp::app::plan_document_layer_opacity(2, 1, 1.1F)); + PP_EXPECT(harness, !pp::app::plan_document_layer_opacity(2, 1, std::nanf(""))); + PP_EXPECT(harness, !pp::app::plan_document_layer_blend_mode(2, 1, -1)); + PP_EXPECT(harness, !pp::app::plan_document_layer_blend_mode(2, 1, 5)); + PP_EXPECT(harness, !pp::app::plan_document_layer_visibility(2, 2, true)); + PP_EXPECT(harness, !pp::app::plan_document_layer_alpha_lock(2, 2, true)); +} + +void layer_highlight_is_transient(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_layer_highlight(2, 1, true); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().operation == pp::app::DocumentLayerOperation::set_highlight); + PP_EXPECT(harness, plan.value().flag); + PP_EXPECT(harness, !plan.value().mutates_document); + PP_EXPECT(harness, !plan.value().marks_unsaved); + PP_EXPECT(harness, !plan.value().updates_title); + } + + PP_EXPECT(harness, !pp::app::plan_document_layer_highlight(2, 2, true)); +} + } int main() @@ -59,5 +187,10 @@ int main() harness.run("layer rename ignores unchanged name", layer_rename_ignores_unchanged_name); harness.run("layer rename rejects empty name", layer_rename_rejects_empty_name); harness.run("layer rename rejects overlong name", layer_rename_rejects_overlong_name); + harness.run("layer add validates insert index and name", layer_add_validates_insert_index_and_name); + harness.run("layer duplicate select and reorder validate indices", layer_duplicate_select_and_reorder_validate_indices); + harness.run("layer remove keeps at least one layer", layer_remove_keeps_at_least_one_layer); + harness.run("layer metadata plans validate values", layer_metadata_plans_validate_values); + harness.run("layer highlight is transient", layer_highlight_is_transient); return harness.finish(); } diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index cd02f28..111a268 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -225,6 +225,19 @@ struct PlanLayerRenameArgs { std::string new_name; }; +struct PlanLayerOperationArgs { + std::string kind = "select"; + int layer_count = 1; + int index = 0; + int from_index = 0; + int to_index = 0; + int source_index = 0; + std::string name = "Layer"; + float opacity = 1.0F; + bool flag = false; + int blend_mode = 0; +}; + struct SimulateAppSessionArgs { bool has_canvas = true; bool new_document = false; @@ -470,6 +483,34 @@ const char* document_layer_rename_action_name(pp::app::DocumentLayerRenameAction return "no-op-same-name"; } +const char* document_layer_operation_name(pp::app::DocumentLayerOperation operation) noexcept +{ + switch (operation) { + case pp::app::DocumentLayerOperation::add: + return "add"; + case pp::app::DocumentLayerOperation::duplicate: + return "duplicate"; + case pp::app::DocumentLayerOperation::select: + return "select"; + case pp::app::DocumentLayerOperation::reorder: + return "reorder"; + case pp::app::DocumentLayerOperation::remove: + return "remove"; + case pp::app::DocumentLayerOperation::set_opacity: + return "set-opacity"; + case pp::app::DocumentLayerOperation::set_visibility: + return "set-visibility"; + case pp::app::DocumentLayerOperation::set_alpha_lock: + return "set-alpha-lock"; + case pp::app::DocumentLayerOperation::set_blend_mode: + return "set-blend-mode"; + case pp::app::DocumentLayerOperation::set_highlight: + return "set-highlight"; + } + + return "select"; +} + const char* document_file_write_decision_name(pp::app::DocumentFileWriteDecision decision) noexcept { switch (decision) { @@ -708,6 +749,7 @@ void print_help() << " 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-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" << " plan-share-file [--path FILE]\n" << " plan-picked-path [--path FILE]\n" << " plan-display-file [--path FILE]\n" @@ -2351,6 +2393,148 @@ int plan_layer_rename(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_layer_operation_args( + int argc, + char** argv, + PlanLayerOperationArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--kind" || key == "--name") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + if (key == "--kind") { + args.kind = argv[++i]; + } else { + args.name = argv[++i]; + } + } else if (key == "--layer-count" || key == "--index" || key == "--from-index" + || key == "--to-index" || key == "--source-index" || key == "--blend-mode") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + const auto value = pp::foundation::parse_u32(argv[++i]); + if (!value) { + return value.status(); + } + if (key == "--layer-count") { + args.layer_count = static_cast(value.value()); + } else if (key == "--index") { + args.index = static_cast(value.value()); + } else if (key == "--from-index") { + args.from_index = static_cast(value.value()); + } else if (key == "--to-index") { + args.to_index = static_cast(value.value()); + } else if (key == "--source-index") { + args.source_index = static_cast(value.value()); + } else { + args.blend_mode = static_cast(value.value()); + } + } else if (key == "--opacity") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + const auto value = parse_float_arg(argv[++i]); + if (!value) { + return value.status(); + } + args.opacity = value.value(); + } else if (key == "--enabled") { + args.flag = true; + } else if (key == "--disabled") { + args.flag = false; + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +pp::foundation::Result make_layer_operation_plan( + const PlanLayerOperationArgs& args) +{ + if (args.kind == "add") { + return pp::app::plan_document_layer_add(args.layer_count, args.index, args.name); + } + if (args.kind == "duplicate") { + return pp::app::plan_document_layer_duplicate(args.layer_count, args.source_index); + } + if (args.kind == "select") { + return pp::app::plan_document_layer_select(args.layer_count, args.index); + } + if (args.kind == "reorder") { + return pp::app::plan_document_layer_reorder(args.layer_count, args.from_index, args.to_index); + } + if (args.kind == "remove") { + return pp::app::plan_document_layer_remove(args.layer_count, args.index); + } + if (args.kind == "opacity") { + return pp::app::plan_document_layer_opacity(args.layer_count, args.index, args.opacity); + } + if (args.kind == "visibility") { + return pp::app::plan_document_layer_visibility(args.layer_count, args.index, args.flag); + } + if (args.kind == "alpha-lock") { + return pp::app::plan_document_layer_alpha_lock(args.layer_count, args.index, args.flag); + } + if (args.kind == "blend-mode") { + return pp::app::plan_document_layer_blend_mode(args.layer_count, args.index, args.blend_mode); + } + if (args.kind == "highlight") { + return pp::app::plan_document_layer_highlight(args.layer_count, args.index, args.flag); + } + + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("unknown layer operation kind")); +} + +int plan_layer_operation(int argc, char** argv) +{ + PlanLayerOperationArgs args; + const auto status = parse_plan_layer_operation_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-layer-operation", status.message); + return 2; + } + + const auto plan = make_layer_operation_plan(args); + if (!plan) { + print_error("plan-layer-operation", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-layer-operation\"" + << ",\"state\":{\"kind\":\"" << json_escape(args.kind) + << "\",\"layerCount\":" << args.layer_count + << ",\"index\":" << args.index + << ",\"fromIndex\":" << args.from_index + << ",\"toIndex\":" << args.to_index + << ",\"sourceIndex\":" << args.source_index + << ",\"name\":\"" << json_escape(args.name) + << "\",\"opacity\":" << args.opacity + << ",\"flag\":" << json_bool(args.flag) + << ",\"blendMode\":" << args.blend_mode + << "},\"plan\":{\"operation\":\"" << document_layer_operation_name(value.operation) + << "\",\"index\":" << value.index + << ",\"fromIndex\":" << value.from_index + << ",\"toIndex\":" << value.to_index + << ",\"insertIndex\":" << value.insert_index + << ",\"sourceIndex\":" << value.source_index + << ",\"name\":\"" << json_escape(value.name) + << "\",\"opacity\":" << value.opacity + << ",\"flag\":" << json_bool(value.flag) + << ",\"blendMode\":" << value.blend_mode + << ",\"mutatesDocument\":" << json_bool(value.mutates_document) + << ",\"marksUnsaved\":" << json_bool(value.marks_unsaved) + << ",\"reloadsAnimationLayers\":" << json_bool(value.reloads_animation_layers) + << ",\"updatesTitle\":" << json_bool(value.updates_title) + << "}}\n"; + return 0; +} + pp::foundation::Status parse_plan_share_file_args( int argc, char** argv, @@ -4759,6 +4943,10 @@ int main(int argc, char** argv) return plan_layer_rename(argc, argv); } + if (command == "plan-layer-operation") { + return plan_layer_operation(argc, argv); + } + if (command == "plan-share-file") { return plan_share_file(argc, argv); }