From 921fc8f00b828ff644db10fa5ea67dcb1ce10c4d Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 3 Jun 2026 20:00:15 +0200 Subject: [PATCH] Dispatch layer operations through app core --- docs/modernization/capability-map.md | 4 +- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 13 +- src/app_core/document_layer.h | 94 +++++++++ src/app_layout.cpp | 234 ++++++++++++++------- tests/app_core/document_layer_tests.cpp | 264 ++++++++++++++++++++++++ 6 files changed, 528 insertions(+), 83 deletions(-) diff --git a/docs/modernization/capability-map.md b/docs/modernization/capability-map.md index d454f03..7ce5d54 100644 --- a/docs/modernization/capability-map.md +++ b/docs/modernization/capability-map.md @@ -45,8 +45,8 @@ and validation command. | Capability | Current Area | Target Owner | Required Tests | | --- | --- | --- | --- | -| Layer add/remove/move/merge | `Canvas`, `Layer`, actions | `pp_document` | Undo/redo invariant tests | -| Blend/opacity/visibility/alpha lock | `Layer`, UI panels, shaders | `pp_document`, `pp_paint_renderer` | CPU model and render golden | +| Layer add/remove/move/merge | `Canvas`, `Layer`, actions | `pp_document`, `pp_app_core` | Operation planning, service-dispatch, no-op preservation, undo/redo invariant tests | +| Blend/opacity/visibility/alpha lock | `Layer`, UI panels, shaders | `pp_document`, `pp_app_core`, `pp_paint_renderer` | Metadata planning, service-dispatch, CPU model and render golden | | Selection mask | `Canvas` mask layer | `pp_document`, `pp_paint_renderer` | Mask apply/clear edge cases | | Animation frames | `LayerFrame`, animation panel | `pp_document`, `pp_panopainter_ui` | Duration, duplicate, remove, seek | | MP4/timelapse export | `MP4Encoder`, recording thread, export dialogs | `pp_assets`, `pp_paint_renderer`, `pp_app_core`, app | Recording lifecycle/progress decision tests, smoke export, cancellation, suggested-name tests | diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 4a54ce9..67d55f3 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 | Unreferenced-parameter warnings are muted globally through `pp_project_warnings` with MSVC `/wd4100` and Clang/GCC `-Wno-unused-parameter` | 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`; `cmake --build --preset linux-clang --target pp_foundation` | Remove `/wd4100` and `-Wno-unused-parameter`, mark intentionally unused parameters with names/comments or `[[maybe_unused]]`, and make the Windows app plus headless Clang/GCC tests pass without unreferenced-parameter warnings | | DEBT-0020 | Open | Modernization | Document resize dialog state, selected-resolution planning, and execution dispatch now consume pure `pp_app_core` through `NodeDialogResize`, `App::dialog_resize`, `pano_cli plan-document-resize`, and the `DocumentResizeServices` boundary, but the live adapter still calls legacy `Canvas::resize`, updates the legacy app title, and clears legacy `ActionManager` history | 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 injected document/app services with no legacy resize adapter, title shim, or direct `ActionManager` history clearing | -| 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 | +| DEBT-0021 | Open | Modernization | Layer rename planning and layer panel operation planning/execution dispatch now consume pure `pp_app_core` through `App::dialog_layer_rename`, `App::init_sidebar` layer callbacks, `pano_cli plan-layer-rename`, `pano_cli plan-layer-operation`, and the `DocumentLayerOperationServices` boundary, but the live operation adapter still mutates legacy `Canvas` layer state and `NodeLayer`/`NodePanelLayer`, while rename still reaches `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 | | DEBT-0022 | Open | Modernization | Animation panel frame command planning, panel action planning, panel-control/timeline execution dispatch, selected-frame click dispatch, playback tick stepping, and play-mode toggles now consume pure `pp_app_core` through `NodePanelAnimation`, `pano_cli plan-animation-operation`, `pano_cli plan-animation-panel-action`, and `DocumentAnimationServices`, and `pp_legacy_ui_core` temporarily links `pp_app_core`, but the live adapter still mutates or reads legacy `Canvas`/`Layer` frame state and canvas mode directly | Preserve existing animation panel behavior while timeline/frame commands move toward the document/app command boundary | `pp_app_core_document_animation_tests`; `pano_cli plan-animation-operation --kind add --frame-count 2 --current-frame 0`; `pano_cli plan-animation-operation --kind select --frame-count 3 --selected-frame 1 --layer-index 2 --layer-id 42`; `pano_cli plan-animation-operation --kind playback --total-duration 5 --current-frame 4 --offset 1`; `pano_cli plan-animation-operation --kind toggle-playback --playing`; `pano_cli plan-animation-panel-action --action next --total-duration 5 --current-frame 4`; `ctest --preset desktop-fast --build-config Debug` | Animation frame/timeline/playback execution is owned by injected document/app timeline services with no legacy `Canvas`/`Layer`/canvas-mode adapter and UI nodes acting only as adapters or removed entirely | | DEBT-0023 | Open | Modernization | Brush/color/preset/stroke-settings UI planning, texture-list add/remove/reorder planning, stroke-panel slider/toggle/blend/reset planning, and execution dispatch now consume pure `pp_app_core` through `App::init_sidebar`, `NodePanelBrush`, `NodePanelStroke`, restored/docked floating-panel callbacks, `pano_cli plan-brush-operation`, `pano_cli plan-brush-texture-list`, `pano_cli plan-brush-stroke-control`, `BrushUiServices`, `BrushTextureListServices`, and `BrushStrokeControlServices`, but the live adapter still mutates legacy `Brush`/`Canvas::I`, loads/saves legacy brush texture images, and refreshes legacy quick/stroke/color widgets | Preserve existing brush UI behavior while brush commands move toward a brush/app/asset command boundary and asset-managed texture selection | `pp_app_core_brush_ui_tests`; `pano_cli plan-brush-operation --kind color --r 0.25 --g 0.5 --b 0.75 --a 1`; `pano_cli plan-brush-operation --kind pattern --path data/patterns/noise.png --thumb data/patterns/thumbs/noise.png`; `pano_cli plan-brush-texture-list --kind add --dir brushes --data-path data --source C:/Temp/soft.png`; `pano_cli plan-brush-stroke-control --kind float --setting tip-size --value 42.5`; `pano_cli plan-brush-stroke-control --kind blend --setting pattern --blend-mode 3`; `ctest --preset desktop-fast --build-config Debug` | Brush color/texture/preset/stroke-settings, texture-list, and stroke-control execution are owned by injected brush/app/asset/UI services with no legacy brush/canvas adapter | | DEBT-0024 | Open | Modernization | Grid/heightmap/lightmap UI planning now consumes pure `pp_app_core` through `NodePanelGrid` and `pano_cli plan-grid-operation`, but live execution still performs legacy image loading, OpenGL texture updates, nanort lightmap baking, progress UI, and `Canvas::draw_objects` commit directly | Preserve grid/lightmap behavior while moving renderable grid commands toward app/renderer/document boundaries | `pp_app_core_grid_ui_tests`; `pano_cli plan-grid-operation --kind render --float32 --texture-resolution 1024 --samples 32`; `ctest --preset desktop-fast --build-config Debug` | Grid heightmap/lightmap execution is owned by app/renderer/document services with `NodePanelGrid` acting only as UI adapter | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index b77eb68..9b28bdd 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -492,8 +492,9 @@ 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. +and highlight actions used by the live layer panel. Direct layer-panel +operations now dispatch through `DocumentLayerOperationServices` before the +legacy `Canvas` and UI layer adapter continues execution. `pano_cli plan-layer-menu` exposes app-core planning for Layer menu clear, rename, and merge-down labels/actions, and direct Layer menu commands now dispatch through `DocumentLayerMenuServices` before the legacy canvas/layer UI @@ -1240,9 +1241,11 @@ Results: 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, transient - highlight behavior, Layer menu labels/actions, merge-down routing, animated - merge blocking, missing selection handling, bad Layer menu state rejection, - Layer menu executor dispatch, and no-op menu execution preservation. + highlight behavior, layer operation executor dispatch, layer operation + side-effect dispatch, no-op operation preservation, malformed operation + rejection, Layer menu labels/actions, merge-down routing, animated 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 31f1172..4c0dd64 100644 --- a/src/app_core/document_layer.h +++ b/src/app_core/document_layer.h @@ -88,6 +88,25 @@ public: virtual void show_merge_animated_not_supported() = 0; }; +class DocumentLayerOperationServices { +public: + virtual ~DocumentLayerOperationServices() = default; + + virtual void add_layer(std::string_view name, int insert_index) = 0; + virtual void duplicate_layer(int source_index, int insert_index) = 0; + virtual void select_layer(int index) = 0; + virtual void reorder_layer(int from_index, int to_index) = 0; + virtual void remove_layer(int index) = 0; + virtual void set_layer_opacity(int index, float opacity) = 0; + virtual void set_layer_visibility(int index, bool visible) = 0; + virtual void set_layer_alpha_lock(int index, bool locked) = 0; + virtual void set_layer_blend_mode(int index, int blend_mode) = 0; + virtual void set_layer_highlight(int index, bool highlighted) = 0; + virtual void mark_unsaved() = 0; + virtual void reload_animation_layers() = 0; + virtual void update_title() = 0; +}; + [[nodiscard]] inline pp::foundation::Status validate_layer_index( int layer_count, int index) noexcept @@ -420,6 +439,81 @@ public: return pp::foundation::Result::success(std::move(plan)); } +inline void execute_document_layer_operation_side_effects( + const DocumentLayerOperationPlan& plan, + DocumentLayerOperationServices& services) +{ + if (plan.marks_unsaved) + services.mark_unsaved(); + if (plan.reloads_animation_layers) + services.reload_animation_layers(); + if (plan.updates_title) + services.update_title(); +} + +[[nodiscard]] inline pp::foundation::Status execute_document_layer_operation_plan( + const DocumentLayerOperationPlan& plan, + DocumentLayerOperationServices& services) +{ + switch (plan.operation) { + case DocumentLayerOperation::add: + if (!plan.mutates_document || plan.name.empty()) { + return pp::foundation::Status::invalid_argument("layer add plan must mutate with a name"); + } + services.add_layer(plan.name, plan.insert_index); + break; + case DocumentLayerOperation::duplicate: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer duplicate plan must mutate the document"); + } + services.duplicate_layer(plan.source_index, plan.insert_index); + break; + case DocumentLayerOperation::select: + services.select_layer(plan.index); + break; + case DocumentLayerOperation::reorder: + if (plan.mutates_document) + services.reorder_layer(plan.from_index, plan.to_index); + break; + case DocumentLayerOperation::remove: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer remove plan must mutate the document"); + } + services.remove_layer(plan.index); + break; + case DocumentLayerOperation::set_opacity: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer opacity plan must mutate the document"); + } + services.set_layer_opacity(plan.index, plan.opacity); + break; + case DocumentLayerOperation::set_visibility: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer visibility plan must mutate the document"); + } + services.set_layer_visibility(plan.index, plan.flag); + break; + case DocumentLayerOperation::set_alpha_lock: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer alpha-lock plan must mutate the document"); + } + services.set_layer_alpha_lock(plan.index, plan.flag); + break; + case DocumentLayerOperation::set_blend_mode: + if (!plan.mutates_document) { + return pp::foundation::Status::invalid_argument("layer blend-mode plan must mutate the document"); + } + services.set_layer_blend_mode(plan.index, plan.blend_mode); + break; + case DocumentLayerOperation::set_highlight: + services.set_layer_highlight(plan.index, plan.flag); + break; + } + + execute_document_layer_operation_side_effects(plan, services); + return pp::foundation::Status::success(); +} + [[nodiscard]] inline pp::foundation::Status execute_document_layer_menu_plan( const DocumentLayerMenuPlan& plan, DocumentLayerMenuServices& services) diff --git a/src/app_layout.cpp b/src/app_layout.cpp index 8aa748d..c6acc4a 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -605,6 +605,142 @@ private: App& app_; }; +class LegacyDocumentLayerOperationServices final : public pp::app::DocumentLayerOperationServices { +public: + LegacyDocumentLayerOperationServices( + App& app, + const std::shared_ptr& pending_layer = nullptr) noexcept + : app_(app) + , pending_layer_(pending_layer) + { + } + + void add_layer(std::string_view name, int insert_index) override + { + auto* canvas = legacy_canvas(); + if (!canvas) + return; + + canvas->layer_add(std::string(name), pending_layer_, insert_index); + canvas->anim_update(); + } + + void duplicate_layer(int source_index, int insert_index) override + { + auto* canvas = legacy_canvas(); + if (!canvas) + return; + + std::string duplicated_name = "Layer"; + if (app_.layers && !app_.layers->m_layers.empty()) + duplicated_name = app_.layers->m_layers.back()->m_label_text; + + canvas->layer_add(duplicated_name, nullptr, insert_index); + auto& dst = canvas->m_layers[insert_index]; + auto& src = canvas->m_layers[source_index]; + for (int i = 1; i < src->frames_count(); i++) + dst->add_frame(); + canvas->anim_update(); + for (int frame = 0; frame < src->frames_count(); frame++) + { + for (int i = 0; i < 6; i++) + { + if (!src->face(i)) + continue; + bool loaded = src->frame(frame).gpu_load(); + dst->frame(frame).gpu_load(); + dst->rtt(i, frame).copy(src->rtt(i)); + dst->face(i, frame) = src->face(i); + dst->box(i, frame) = src->box(i); + if (!loaded) + { + dst->frame(frame).gpu_unload(); + src->frame(frame).gpu_unload(); + } + } + } + dst->m_opacity = src->m_opacity; + dst->m_blend_mode = src->m_blend_mode; + dst->m_alpha_locked = src->m_alpha_locked; + } + + void select_layer(int index) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_current_layer_idx = index; + } + + void reorder_layer(int from_index, int to_index) override + { + if (auto* canvas = legacy_canvas()) + canvas->layer_order(from_index, to_index); + } + + void remove_layer(int index) override + { + if (auto* canvas = legacy_canvas()) + canvas->layer_remove(index); + } + + void set_layer_opacity(int index, float opacity) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_layers[index]->m_opacity = opacity; + } + + void set_layer_visibility(int index, bool visible) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_layers[index]->m_visible = visible; + } + + void set_layer_alpha_lock(int index, bool locked) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_layers[index]->m_alpha_locked = locked; + } + + void set_layer_blend_mode(int index, int blend_mode) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_layers[index]->m_blend_mode = blend_mode; + } + + void set_layer_highlight(int index, bool highlighted) override + { + if (auto* canvas = legacy_canvas()) + canvas->m_layers[index]->m_hightlight = highlighted; + } + + void mark_unsaved() override + { + if (auto* canvas = legacy_canvas()) + canvas->m_unsaved = true; + } + + void reload_animation_layers() override + { + if (app_.animation) + app_.animation->load_layers(); + } + + void update_title() override + { + app_.title_update(); + } + +private: + [[nodiscard]] Canvas* legacy_canvas() const noexcept + { + if (app_.canvas && app_.canvas->m_canvas) + return app_.canvas->m_canvas.get(); + return Canvas::I; + } + + App& app_; + std::shared_ptr pending_layer_; +}; + void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan) { LegacyMainToolbarServices services(app); @@ -637,6 +773,17 @@ void execute_document_layer_menu_plan(App& app, const pp::app::DocumentLayerMenu LOG("Layer menu action failed: %s", status.message); } +void execute_document_layer_operation_plan( + App& app, + const pp::app::DocumentLayerOperationPlan& plan, + const std::shared_ptr& pending_layer = nullptr) +{ + LegacyDocumentLayerOperationServices services(app, pending_layer); + const auto status = pp::app::execute_document_layer_operation_plan(plan, services); + if (!status.ok()) + LOG("Layer operation failed: %s", status.message); +} + } // namespace void App::title_update() @@ -833,13 +980,7 @@ void App::init_sidebar() 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(); - if (plan.value().reloads_animation_layers) - animation->load_layers(); - if (plan.value().updates_title) - title_update(); + execute_document_layer_operation_plan(*this, plan.value(), layer); }; layers->on_layer_duplicate = [this](Node*, int source_index) { @@ -848,49 +989,16 @@ void App::init_sidebar() 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(); - for (int frame = 0; frame < src->frames_count(); frame++) - { - for (int i = 0; i < 6; i++) - { - if (!src->face(i)) - continue; - bool loaded = src->frame(frame).gpu_load(); - dst->frame(frame).gpu_load(); - dst->rtt(i, frame).copy(src->rtt(i)); - dst->face(i, frame) = src->face(i); - dst->box(i, frame) = src->box(i); - if (!loaded) - { - dst->frame(frame).gpu_unload(); - src->frame(frame).gpu_unload(); - } - } - } - 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 = plan.value().marks_unsaved; - if (plan.value().reloads_animation_layers) - animation->load_layers(); - if (plan.value().updates_title) - title_update(); + execute_document_layer_operation_plan(*this, plan.value()); }; - layers->on_layer_change = [this](Node*, int old_idx, int new_idx) { + layers->on_layer_change = [this](Node*, int, int new_idx) { 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_order = [this](Node*, int old_idx, int new_idx) { @@ -898,14 +1006,9 @@ void App::init_sidebar() static_cast(canvas->m_canvas->m_layers.size()), old_idx, new_idx); - if (!plan || !plan.value().mutates_document) + if (!plan) 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_delete = [this](Node*, int idx) { @@ -914,12 +1017,7 @@ void App::init_sidebar() 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_opacity_changed = [this](Node*, int idx, float value) { @@ -929,10 +1027,7 @@ void App::init_sidebar() 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_visibility_changed = [this](Node*, int idx, bool visible) { @@ -942,12 +1037,7 @@ void App::init_sidebar() 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_alpha_lock_changed = [this](Node*, int idx, bool locked) { @@ -957,10 +1047,7 @@ void App::init_sidebar() 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_blend_mode_changed = [this](Node*, int idx, int mode) { @@ -970,10 +1057,7 @@ void App::init_sidebar() 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(); + execute_document_layer_operation_plan(*this, plan.value()); }; layers->on_layer_highlight_changed = [this](Node*, int idx, bool highlight) { @@ -983,7 +1067,7 @@ void App::init_sidebar() highlight); if (!plan) return; - canvas->m_canvas->m_layers[plan.value().index]->m_hightlight = plan.value().flag; + execute_document_layer_operation_plan(*this, plan.value()); }; if (auto* button = layout[main_id]->find("btn-stroke")) { diff --git a/tests/app_core/document_layer_tests.cpp b/tests/app_core/document_layer_tests.cpp index 6c568c7..6677f83 100644 --- a/tests/app_core/document_layer_tests.cpp +++ b/tests/app_core/document_layer_tests.cpp @@ -31,6 +31,110 @@ public: int last_to_index = -1; }; +class FakeDocumentLayerOperationServices final : public pp::app::DocumentLayerOperationServices { +public: + void add_layer(std::string_view name, int insert_index) override + { + add_calls += 1; + last_name = std::string(name); + last_insert_index = insert_index; + } + + void duplicate_layer(int source_index, int insert_index) override + { + duplicate_calls += 1; + last_source_index = source_index; + last_insert_index = insert_index; + } + + void select_layer(int index) override + { + select_calls += 1; + last_index = index; + } + + void reorder_layer(int from_index, int to_index) override + { + reorder_calls += 1; + last_from_index = from_index; + last_to_index = to_index; + } + + void remove_layer(int index) override + { + remove_calls += 1; + last_index = index; + } + + void set_layer_opacity(int index, float opacity) override + { + opacity_calls += 1; + last_index = index; + last_opacity = opacity; + } + + void set_layer_visibility(int index, bool visible) override + { + visibility_calls += 1; + last_index = index; + last_flag = visible; + } + + void set_layer_alpha_lock(int index, bool locked) override + { + alpha_lock_calls += 1; + last_index = index; + last_flag = locked; + } + + void set_layer_blend_mode(int index, int blend_mode) override + { + blend_mode_calls += 1; + last_index = index; + last_blend_mode = blend_mode; + } + + void set_layer_highlight(int index, bool highlighted) override + { + highlight_calls += 1; + last_index = index; + last_flag = highlighted; + } + + void mark_unsaved() override { unsaved_marks += 1; } + void reload_animation_layers() override { animation_reloads += 1; } + void update_title() override { title_updates += 1; } + + [[nodiscard]] int mutation_calls() const noexcept + { + return add_calls + duplicate_calls + reorder_calls + remove_calls + opacity_calls + + visibility_calls + alpha_lock_calls + blend_mode_calls; + } + + int add_calls = 0; + int duplicate_calls = 0; + int select_calls = 0; + int reorder_calls = 0; + int remove_calls = 0; + int opacity_calls = 0; + int visibility_calls = 0; + int alpha_lock_calls = 0; + int blend_mode_calls = 0; + int highlight_calls = 0; + int unsaved_marks = 0; + int animation_reloads = 0; + int title_updates = 0; + int last_index = -1; + int last_from_index = -1; + int last_to_index = -1; + int last_insert_index = -1; + int last_source_index = -1; + int last_blend_mode = -1; + float last_opacity = -1.0F; + bool last_flag = false; + std::string last_name; +}; + void layer_rename_records_changed_name(pp::tests::Harness& harness) { const auto plan = pp::app::plan_document_layer_rename("Base", "Paint"); @@ -203,6 +307,162 @@ void layer_highlight_is_transient(pp::tests::Harness& harness) PP_EXPECT(harness, !pp::app::plan_document_layer_highlight(2, 2, true)); } +void layer_operation_executor_dispatches_document_mutations(pp::tests::Harness& harness) +{ + FakeDocumentLayerOperationServices services; + + const auto add = pp::app::plan_document_layer_add(2, 1, "Paint"); + PP_EXPECT(harness, add); + if (add) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(add.value(), services).ok()); + PP_EXPECT(harness, services.add_calls == 1); + PP_EXPECT(harness, services.last_name == "Paint"); + PP_EXPECT(harness, services.last_insert_index == 1); + PP_EXPECT(harness, services.unsaved_marks == 1); + PP_EXPECT(harness, services.animation_reloads == 1); + PP_EXPECT(harness, services.title_updates == 1); + } + + const auto duplicate = pp::app::plan_document_layer_duplicate(3, 1); + PP_EXPECT(harness, duplicate); + if (duplicate) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(duplicate.value(), services).ok()); + PP_EXPECT(harness, services.duplicate_calls == 1); + PP_EXPECT(harness, services.last_source_index == 1); + PP_EXPECT(harness, services.last_insert_index == 2); + PP_EXPECT(harness, services.unsaved_marks == 2); + PP_EXPECT(harness, services.animation_reloads == 2); + PP_EXPECT(harness, services.title_updates == 2); + } + + const auto reorder = pp::app::plan_document_layer_reorder(3, 2, 0); + PP_EXPECT(harness, reorder); + if (reorder) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(reorder.value(), services).ok()); + PP_EXPECT(harness, services.reorder_calls == 1); + PP_EXPECT(harness, services.last_from_index == 2); + PP_EXPECT(harness, services.last_to_index == 0); + PP_EXPECT(harness, services.unsaved_marks == 3); + PP_EXPECT(harness, services.animation_reloads == 3); + PP_EXPECT(harness, services.title_updates == 3); + } + + const auto remove = pp::app::plan_document_layer_remove(3, 1); + PP_EXPECT(harness, remove); + if (remove) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(remove.value(), services).ok()); + PP_EXPECT(harness, services.remove_calls == 1); + PP_EXPECT(harness, services.last_index == 1); + PP_EXPECT(harness, services.unsaved_marks == 4); + PP_EXPECT(harness, services.animation_reloads == 4); + PP_EXPECT(harness, services.title_updates == 4); + } +} + +void layer_operation_executor_dispatches_selection_and_metadata(pp::tests::Harness& harness) +{ + FakeDocumentLayerOperationServices services; + + const auto select = pp::app::plan_document_layer_select(3, 2); + PP_EXPECT(harness, select); + if (select) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(select.value(), services).ok()); + PP_EXPECT(harness, services.select_calls == 1); + PP_EXPECT(harness, services.last_index == 2); + PP_EXPECT(harness, services.unsaved_marks == 0); + PP_EXPECT(harness, services.animation_reloads == 1); + PP_EXPECT(harness, services.title_updates == 0); + } + + const auto opacity = pp::app::plan_document_layer_opacity(3, 1, 0.25F); + PP_EXPECT(harness, opacity); + if (opacity) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(opacity.value(), services).ok()); + PP_EXPECT(harness, services.opacity_calls == 1); + PP_EXPECT(harness, services.last_index == 1); + PP_EXPECT(harness, services.last_opacity == 0.25F); + PP_EXPECT(harness, services.unsaved_marks == 1); + PP_EXPECT(harness, services.animation_reloads == 1); + PP_EXPECT(harness, services.title_updates == 1); + } + + const auto visibility = pp::app::plan_document_layer_visibility(3, 1, false); + PP_EXPECT(harness, visibility); + if (visibility) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(visibility.value(), services).ok()); + PP_EXPECT(harness, services.visibility_calls == 1); + PP_EXPECT(harness, !services.last_flag); + PP_EXPECT(harness, services.unsaved_marks == 2); + PP_EXPECT(harness, services.animation_reloads == 2); + PP_EXPECT(harness, services.title_updates == 2); + } + + const auto alpha_lock = pp::app::plan_document_layer_alpha_lock(3, 1, true); + PP_EXPECT(harness, alpha_lock); + if (alpha_lock) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(alpha_lock.value(), services).ok()); + PP_EXPECT(harness, services.alpha_lock_calls == 1); + PP_EXPECT(harness, services.last_flag); + PP_EXPECT(harness, services.unsaved_marks == 3); + PP_EXPECT(harness, services.animation_reloads == 2); + PP_EXPECT(harness, services.title_updates == 3); + } + + const auto blend = pp::app::plan_document_layer_blend_mode(3, 1, 4); + PP_EXPECT(harness, blend); + if (blend) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(blend.value(), services).ok()); + PP_EXPECT(harness, services.blend_mode_calls == 1); + PP_EXPECT(harness, services.last_blend_mode == 4); + PP_EXPECT(harness, services.unsaved_marks == 4); + PP_EXPECT(harness, services.title_updates == 4); + } +} + +void layer_operation_executor_preserves_no_op_and_transient_actions(pp::tests::Harness& harness) +{ + FakeDocumentLayerOperationServices services; + + 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, pp::app::execute_document_layer_operation_plan(no_op_reorder.value(), services).ok()); + PP_EXPECT(harness, services.reorder_calls == 0); + PP_EXPECT(harness, services.mutation_calls() == 0); + PP_EXPECT(harness, services.unsaved_marks == 0); + PP_EXPECT(harness, services.animation_reloads == 0); + PP_EXPECT(harness, services.title_updates == 0); + } + + const auto highlight = pp::app::plan_document_layer_highlight(3, 1, true); + PP_EXPECT(harness, highlight); + if (highlight) { + PP_EXPECT(harness, pp::app::execute_document_layer_operation_plan(highlight.value(), services).ok()); + PP_EXPECT(harness, services.highlight_calls == 1); + PP_EXPECT(harness, services.last_index == 1); + PP_EXPECT(harness, services.last_flag); + PP_EXPECT(harness, services.unsaved_marks == 0); + PP_EXPECT(harness, services.animation_reloads == 0); + PP_EXPECT(harness, services.title_updates == 0); + } +} + +void layer_operation_executor_rejects_malformed_mutation_plans(pp::tests::Harness& harness) +{ + FakeDocumentLayerOperationServices services; + + pp::app::DocumentLayerOperationPlan malformed; + malformed.operation = pp::app::DocumentLayerOperation::add; + malformed.insert_index = 1; + malformed.mutates_document = true; + + const auto status = pp::app::execute_document_layer_operation_plan(malformed, services); + PP_EXPECT(harness, !status.ok()); + PP_EXPECT(harness, status.code == pp::foundation::StatusCode::invalid_argument); + PP_EXPECT(harness, services.add_calls == 0); + PP_EXPECT(harness, services.unsaved_marks == 0); +} + void layer_menu_labels_selected_layer_commands(pp::tests::Harness& harness) { const auto clear = pp::app::plan_document_layer_menu( @@ -418,6 +678,10 @@ int main() 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); + harness.run("layer operation executor dispatches document mutations", layer_operation_executor_dispatches_document_mutations); + harness.run("layer operation executor dispatches selection and metadata", layer_operation_executor_dispatches_selection_and_metadata); + harness.run("layer operation executor preserves no op and transient actions", layer_operation_executor_preserves_no_op_and_transient_actions); + harness.run("layer operation executor rejects malformed mutation plans", layer_operation_executor_rejects_malformed_mutation_plans); 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);