From 7c7670335511fe754450acd076c8f4c9e8bd4f8c Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 3 Jun 2026 13:09:12 +0200 Subject: [PATCH] Add document resize service boundary --- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 8 ++- src/app_core/document_resize.h | 25 +++++++ src/app_dialogs.cpp | 35 ++++++++-- tests/app_core/document_resize_tests.cpp | 87 ++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 8 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index d6c4406..f5d3620 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -37,7 +37,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0016 | Open | Modernization | Clipboard get/set requests now consume pure `pp_app_core` planning through `pano_cli plan-clipboard-read` and `pano_cli plan-clipboard-write`, and Windows live execution uses injected `WindowsPlatformServices`, but Apple/Android clipboard execution still reaches retained fallback adapter branches from `App::clipboard_get_text` and `App::clipboard_set_text` | Keep picker/color text clipboard behavior stable while platform shells are extracted incrementally | `pp_app_core_document_platform_io_tests`; `pano_cli plan-clipboard-write --text #ff00aa`; `ctest --preset desktop-fast --build-config Debug` | Clipboard execution is owned by injected `pp_platform_*` services for every supported platform | | 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 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-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-0022 | Open | Modernization | Animation panel frame command planning now consumes pure `pp_app_core` through `NodePanelAnimation` and `pano_cli plan-animation-operation`, and `pp_legacy_ui_core` temporarily links `pp_app_core`, but live execution still mutates legacy `Canvas`/`Layer` frame state and animation playback state 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 next --total-duration 5 --current-frame 4`; `ctest --preset desktop-fast --build-config Debug` | Animation frame/timeline execution is owned by the document/app command boundary with legacy `Canvas`/`Layer`/UI nodes acting only as adapters or removed entirely | | DEBT-0023 | Open | Modernization | Brush/color/preset UI planning now consumes pure `pp_app_core` through `App::init_sidebar`, restored/docked floating-panel callbacks, and `pano_cli plan-brush-operation`, but live execution still mutates legacy `Brush`, calls legacy brush texture loading, and refreshes legacy quick/stroke/color widgets directly | Preserve existing brush UI behavior while brush commands move toward a brush/app 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`; `ctest --preset desktop-fast --build-config Debug` | Brush color/texture/preset execution is owned by a brush/app command boundary with legacy `Brush`/UI nodes acting only as adapters or removed entirely | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 19cdeb2..e74978c 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -482,8 +482,9 @@ cursor bridges continue. app-core clipboard text decisions used by live clipboard get/set requests before retained platform clipboard bridges continue. `pano_cli plan-document-resize` exposes the app-core resize dialog state and -selected-resolution commit plan used by the live document resize dialog before -legacy `Canvas` resize execution and `ActionManager` history clearing continue. +selected-resolution commit plan used by the live document resize dialog, and +resize execution now dispatches through `DocumentResizeServices` before the +legacy `Canvas` resize adapter 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. @@ -1164,7 +1165,8 @@ Results: app-core recording lifecycle/export decisions as JSON. - `pp_app_core_document_resize_tests` passed, covering resize dialog state, unknown current-resolution labeling, selected-resolution mapping, square - canvas sizing, history-clearing intent, and invalid selection rejection. + canvas sizing, history-clearing intent, invalid selection rejection, service + dispatch order, optional history clearing, and invalid-dimension rejection. - `pano_cli_plan_document_resize_smoke` and `pano_cli_plan_document_resize_rejects_invalid_selection` passed and expose live document-resize planning as JSON automation. diff --git a/src/app_core/document_resize.h b/src/app_core/document_resize.h index d0d10c4..29c4479 100644 --- a/src/app_core/document_resize.h +++ b/src/app_core/document_resize.h @@ -21,6 +21,15 @@ struct DocumentResizePlan { bool clears_history = false; }; +class DocumentResizeServices { +public: + virtual ~DocumentResizeServices() = default; + + virtual void resize_document(int width, int height) = 0; + virtual void update_title() = 0; + virtual void clear_history() = 0; +}; + [[nodiscard]] inline DocumentResizeDialogState make_document_resize_dialog_state( int current_resolution) { @@ -54,4 +63,20 @@ struct DocumentResizePlan { }); } +[[nodiscard]] inline pp::foundation::Status execute_document_resize_plan( + const DocumentResizePlan& plan, + DocumentResizeServices& services) +{ + if (plan.width <= 0 || plan.height <= 0) { + return pp::foundation::Status::out_of_range("resize dimensions must be positive"); + } + + services.resize_document(plan.width, plan.height); + services.update_title(); + if (plan.clears_history) { + services.clear_history(); + } + return pp::foundation::Status::success(); +} + } diff --git a/src/app_dialogs.cpp b/src/app_dialogs.cpp index bfee383..5708aef 100644 --- a/src/app_dialogs.cpp +++ b/src/app_dialogs.cpp @@ -550,6 +550,33 @@ void App::dialog_export_depth() void App::dialog_resize() { + class LegacyDocumentResizeServices final : public pp::app::DocumentResizeServices { + public: + explicit LegacyDocumentResizeServices(App& app) noexcept + : app_(app) + { + } + + void resize_document(int width, int height) override + { + if (app_.canvas) + app_.canvas->m_canvas->resize(width, height); + } + + void update_title() override + { + app_.title_update(); + } + + void clear_history() override + { + ActionManager::clear(); + } + + private: + App& app_; + }; + auto dialog = std::make_shared(); dialog->set_manager(&layout); dialog->init(); @@ -567,10 +594,10 @@ void App::dialog_resize() dialog->destroy(); return; } - if (canvas) - canvas->m_canvas->resize(plan.value().width, plan.value().height); - App::I->title_update(); - ActionManager::clear(); + LegacyDocumentResizeServices services(*this); + const auto status = pp::app::execute_document_resize_plan(plan.value(), services); + if (!status.ok()) + LOG("Document resize failed: %s", status.message); dialog->destroy(); }; } diff --git a/tests/app_core/document_resize_tests.cpp b/tests/app_core/document_resize_tests.cpp index 2bae75d..c2a7771 100644 --- a/tests/app_core/document_resize_tests.cpp +++ b/tests/app_core/document_resize_tests.cpp @@ -1,8 +1,40 @@ #include "app_core/document_resize.h" #include "test_harness.h" +#include + namespace { +class FakeDocumentResizeServices final : public pp::app::DocumentResizeServices { +public: + void resize_document(int width, int height) override + { + resize_calls += 1; + last_width = width; + last_height = height; + call_order += "resize;"; + } + + void update_title() override + { + title_updates += 1; + call_order += "title;"; + } + + void clear_history() override + { + history_clears += 1; + call_order += "history;"; + } + + int resize_calls = 0; + int title_updates = 0; + int history_clears = 0; + int last_width = 0; + int last_height = 0; + std::string call_order; +}; + void dialog_state_labels_current_resolution(pp::tests::Harness& harness) { const auto state = pp::app::make_document_resize_dialog_state(2048); @@ -37,6 +69,58 @@ void resize_plan_rejects_invalid_selection(pp::tests::Harness& harness) PP_EXPECT(harness, !pp::app::plan_document_resize(6)); } +void resize_executor_dispatches_canvas_title_and_history(pp::tests::Harness& harness) +{ + FakeDocumentResizeServices services; + const auto plan = pp::app::plan_document_resize(4); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, pp::app::execute_document_resize_plan(plan.value(), services).ok()); + } + + PP_EXPECT(harness, services.resize_calls == 1); + PP_EXPECT(harness, services.last_width == 4096); + PP_EXPECT(harness, services.last_height == 4096); + PP_EXPECT(harness, services.title_updates == 1); + PP_EXPECT(harness, services.history_clears == 1); + PP_EXPECT(harness, services.call_order == "resize;title;history;"); +} + +void resize_executor_preserves_optional_history_clear(pp::tests::Harness& harness) +{ + FakeDocumentResizeServices services; + const pp::app::DocumentResizePlan plan { + 1024, + 1024, + 1024, + false, + }; + + PP_EXPECT(harness, pp::app::execute_document_resize_plan(plan, services).ok()); + PP_EXPECT(harness, services.resize_calls == 1); + PP_EXPECT(harness, services.title_updates == 1); + PP_EXPECT(harness, services.history_clears == 0); + PP_EXPECT(harness, services.call_order == "resize;title;"); +} + +void resize_executor_rejects_invalid_dimensions(pp::tests::Harness& harness) +{ + FakeDocumentResizeServices services; + const pp::app::DocumentResizePlan plan { + 0, + 0, + 1024, + true, + }; + + const auto status = pp::app::execute_document_resize_plan(plan, services); + PP_EXPECT(harness, !status.ok()); + PP_EXPECT(harness, status.code == pp::foundation::StatusCode::out_of_range); + PP_EXPECT(harness, services.resize_calls == 0); + PP_EXPECT(harness, services.title_updates == 0); + PP_EXPECT(harness, services.history_clears == 0); +} + } int main() @@ -46,5 +130,8 @@ int main() harness.run("dialog state survives unknown resolution", dialog_state_survives_unknown_resolution); harness.run("resize plan maps selection to square canvas", resize_plan_maps_selection_to_square_canvas); harness.run("resize plan rejects invalid selection", resize_plan_rejects_invalid_selection); + harness.run("resize executor dispatches canvas title and history", resize_executor_dispatches_canvas_title_and_history); + harness.run("resize executor preserves optional history clear", resize_executor_preserves_optional_history_clear); + harness.run("resize executor rejects invalid dimensions", resize_executor_rejects_invalid_dimensions); return harness.finish(); }