Add canvas clear service boundary

2026-06-03 13:13:43 +02:00
parent 7c76703355
commit a6306c2759
7 changed files with 145 additions and 11 deletions
--- a/docs/modernization/debt.md
+++ b/docs/modernization/debt.md
@@ -45,7 +45,7 @@ agent or engineer to remove them without reconstructing context from chat.
 | DEBT-0025 | Open | Modernization | Quick brush/color slot and mini-state planning now consumes pure `pp_app_core` through `NodePanelQuick` and `pano_cli plan-quick-operation`, but live execution still mutates legacy quick UI widgets, `Brush` previews, color picker popup state, and preset popup state directly | Preserve quick-panel behavior while quick brush/color commands move toward a brush/app command boundary with safer automation coverage | `pp_app_core_quick_ui_tests`; `pano_cli plan-quick-operation --kind brush --current-index 0 --slot-index 2`; `pano_cli plan-quick-operation --kind restore --brush-index 2 --color-index 1 --fire-event`; `ctest --preset desktop-fast --build-config Debug` | Quick-panel selection, popup, restore, reset, brush preview, and color execution are owned by app/brush/UI services with `NodePanelQuick` acting only as UI adapter |
 | DEBT-0026 | Open | Modernization | Toolbar and canvas history command planning now consumes pure `pp_app_core` through `App::init_toolbar_main`, `NodeCanvas`, and `pano_cli plan-history-operation`, but live execution still mutates legacy `ActionManager` stacks and `Canvas::I` unsaved state directly | Preserve undo/redo/clear behavior while moving action history toward document/app command services | `pp_app_core_history_ui_tests`; `pano_cli plan-history-operation --kind undo --undo-count 2`; `pano_cli plan-history-operation --kind clear --undo-count 2 --redo-count 1 --memory-bytes 4096`; `ctest --preset desktop-fast --build-config Debug` | Undo/redo/clear execution is owned by document/app history services with toolbar and canvas input acting only as adapters |
 | DEBT-0027 | Open | Modernization | Canvas draw-tool toolbar command, canvas input mode switching, and active-state planning now consume pure `pp_app_core` through `App::init_toolbar_draw`, `App::update`, `NodeCanvas`, `pano_cli plan-canvas-tool`, and `pano_cli plan-canvas-tool-state`, but live execution/state storage still mutates or reads legacy `Canvas` mode state, pen picking state, touch-lock state, and transform copy/cut action objects directly | Preserve current toolbar, stylus eraser, and keyboard draw/erase behavior while canvas input/tools move toward an app/document command boundary | `pp_app_core_canvas_tool_ui_tests`; `pano_cli plan-canvas-tool --kind copy`; `pano_cli plan-canvas-tool-state --mode draw --picking --touch-lock`; `ctest --preset desktop-fast --build-config Debug` | Canvas tool selection, toolbar state refresh, picking, touch lock, stylus eraser/key mode switching, and transform action execution are owned by app/document/canvas services with toolbar/canvas callbacks acting only as adapters |
-| DEBT-0028 | Open | Modernization | Canvas clear command planning now consumes pure `pp_app_core` through `App::init_toolbar_main` and `pano_cli plan-canvas-clear`, but live execution still calls legacy `Canvas::clear`, which records `ActionLayerClear`, clears the current layer/frame, and marks legacy `Canvas::I` unsaved directly | Preserve clear-current-layer behavior while canvas/document commands move toward document/app command services | `pp_app_core_document_canvas_tests`; `pano_cli plan-canvas-clear --r 0 --g 0.1 --b 0.2 --a 0.3`; `pano_cli plan-canvas-clear --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Canvas clear execution, undo recording, dirty-state updates, and clear color handling are owned by document/app services with toolbar callbacks acting only as adapters |
+| DEBT-0028 | Open | Modernization | Canvas clear command planning and execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `pano_cli plan-canvas-clear`, and the `DocumentCanvasClearServices` boundary, but the live adapter still calls legacy `Canvas::clear`, which records `ActionLayerClear`, clears the current layer/frame, and marks legacy `Canvas::I` unsaved | Preserve clear-current-layer behavior while canvas/document commands move toward document/app command services | `pp_app_core_document_canvas_tests`; `pano_cli plan-canvas-clear --r 0 --g 0.1 --b 0.2 --a 0.3`; `pano_cli plan-canvas-clear --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Canvas clear execution, undo recording, dirty-state updates, and clear color handling are owned by injected document/app services with no legacy canvas-clear adapter |
 | DEBT-0029 | Open | Modernization | Image import route planning now consumes pure `pp_app_core` through the File menu and `pano_cli plan-image-import`, but live execution still calls legacy `Canvas::import_equirectangular` or legacy import transform mode setup directly after image loading | Preserve current File > Import behavior while image import moves toward document/app/asset command services | `pp_app_core_document_import_tests`; `pano_cli plan-image-import --width 4096 --height 2048`; `pano_cli plan-image-import --width 1024 --height 1024`; `ctest --preset desktop-fast --build-config Debug` | Image loading, equirectangular import, transform-placement import, and failure reporting are owned by document/app/asset services with File-menu callbacks acting only as adapters |
 | DEBT-0030 | Open | Modernization | File export menu action planning and execution dispatch now consume pure `pp_app_core` through the File menu, `pano_cli plan-export-menu`, and the `DocumentExportMenuServices` boundary, but the live adapter still opens legacy export dialogs and then reaches legacy canvas/render/video export code | Preserve current export menu behavior while export command execution moves toward document/app/renderer/video services | `pp_app_core_document_export_tests`; `pano_cli plan-export-menu --kind png`; `pano_cli plan-export-menu --kind animation-mp4 --demo`; `pano_cli plan-export-menu --kind layers --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Export menu routing, license gating, target creation, image/layer/cube/depth/animation/timelapse execution, and error reporting are owned by injected document/app/renderer/video services with File-menu callbacks acting only as UI adapters and no legacy export adapter |
 | DEBT-0031 | Open | Modernization | Top-level File menu command planning and execution dispatch now consume pure `pp_app_core` through `App::init_menu_file`, `pano_cli plan-file-menu`, and the `FileMenuServices` boundary, but the live adapter still invokes legacy dialogs, platform pickers, cloud code, share code, and canvas import/export paths directly | Preserve File menu behavior while app workflows move toward app/document/platform command services | `pp_app_core_file_menu_tests`; `pano_cli plan-file-menu --command save-as`; `pano_cli plan-file-menu --command import`; `pano_cli plan-file-menu --command cloud-upload`; `ctest --preset desktop-fast --build-config Debug` | File menu routing, picker dispatch, save/share/cloud/resize/export execution, and image/project import execution are owned by injected app/document/platform services with `App::init_menu_file` acting only as a UI adapter and no legacy File menu adapter |
--- a/docs/modernization/roadmap.md
+++ b/docs/modernization/roadmap.md
@@ -514,8 +514,9 @@ draw/erase mode switching also consume the same app-core command planner before
 legacy canvas mode execution continues.
 `pano_cli plan-canvas-clear` exposes app-core planning for the main toolbar
 clear-current-layer command, including clear color validation, no-canvas
-handling, undo recording intent, and dirty-state intent before legacy
-`Canvas::clear` execution continues.
+handling, undo recording intent, and dirty-state intent; live toolbar execution
+now dispatches through `DocumentCanvasClearServices` before the legacy
+`Canvas::clear` adapter continues.
 `pano_cli plan-image-import` exposes app-core planning for File > Import image
 route decisions, including wide equirectangular images, legacy vertical cube
 strips, regular transform-placement images, and invalid image dimensions before
@@ -1235,7 +1236,8 @@ Results:
  toolbar active-state refresh as JSON automation.
 - `pp_app_core_document_canvas_tests` passed, covering clear-current-layer
  undo/dirty intent, no-canvas no-op behavior, and invalid clear color
-  rejection.
+  rejection, service dispatch color forwarding, no-op execution preservation,
+  and invalid execution color rejection.
 - `pano_cli_plan_canvas_clear_smoke`,
  `pano_cli_plan_canvas_clear_no_canvas_smoke`, and
  `pano_cli_plan_canvas_clear_rejects_bad_color` passed and expose toolbar
--- a/src/app_core/document_canvas.h
+++ b/src/app_core/document_canvas.h
@@ -17,6 +17,13 @@ struct DocumentCanvasClearPlan {
    bool no_op = true;
 };

+class DocumentCanvasClearServices {
+public:
+    virtual ~DocumentCanvasClearServices() = default;
+
+    virtual void clear_current_canvas(float r, float g, float b, float a) = 0;
+};
+
 [[nodiscard]] inline pp::foundation::Status validate_clear_color_channel(float value) noexcept
 {
    if (!std::isfinite(value)) {
@@ -55,4 +62,24 @@ struct DocumentCanvasClearPlan {
    return pp::foundation::Result<DocumentCanvasClearPlan>::success(plan);
 }

+[[nodiscard]] inline pp::foundation::Status execute_document_canvas_clear_plan(
+    const DocumentCanvasClearPlan& plan,
+    DocumentCanvasClearServices& services)
+{
+    const float channels[] { plan.r, plan.g, plan.b, plan.a };
+    for (const float channel : channels) {
+        const auto status = validate_clear_color_channel(channel);
+        if (!status.ok()) {
+            return status;
+        }
+    }
+
+    if (plan.no_op || !plan.clears_canvas) {
+        return pp::foundation::Status::success();
+    }
+
+    services.clear_current_canvas(plan.r, plan.g, plan.b, plan.a);
+    return pp::foundation::Status::success();
+}
+
 } // namespace pp::app
--- a/src/app_core/main_toolbar.h
+++ b/src/app_core/main_toolbar.h
@@ -41,6 +41,7 @@ struct MainToolbarPlan {
    bool records_undo = false;
    bool marks_unsaved = false;
    bool no_op = false;
+    DocumentCanvasClearPlan canvas_clear;
 };

 class MainToolbarServices {
@@ -52,7 +53,7 @@ public:
    virtual void invoke_undo() = 0;
    virtual void invoke_redo() = 0;
    virtual void clear_history() = 0;
-    virtual void clear_canvas() = 0;
+    virtual void clear_canvas(const DocumentCanvasClearPlan& plan) = 0;
    virtual void show_message_box() = 0;
    virtual void show_settings_dialog() = 0;
 };
@@ -139,6 +140,7 @@ public:
        plan.records_undo = clear.value().records_undo;
        plan.marks_unsaved = clear.value().marks_unsaved;
        plan.no_op = clear.value().no_op;
+        plan.canvas_clear = clear.value();
        return pp::foundation::Result<MainToolbarPlan>::success(plan);
    }

@@ -178,7 +180,7 @@ public:
        services.clear_history();
        return pp::foundation::Status::success();
    case MainToolbarAction::clear_canvas:
-        services.clear_canvas();
+        services.clear_canvas(plan.canvas_clear);
        return pp::foundation::Status::success();
    case MainToolbarAction::show_message_box:
        services.show_message_box();
--- a/src/app_layout.cpp
+++ b/src/app_layout.cpp
@@ -305,12 +305,31 @@ public:
        ActionManager::clear();
    }

-    void clear_canvas() override
+    void clear_canvas(const pp::app::DocumentCanvasClearPlan& plan) override
    {
-        if (!app_.canvas || !app_.canvas->m_canvas)
-            return;
+        class LegacyDocumentCanvasClearServices final : public pp::app::DocumentCanvasClearServices {
+        public:
+            explicit LegacyDocumentCanvasClearServices(App& app) noexcept
+                : app_(app)
+            {
+            }

-        app_.canvas->m_canvas->clear({ 0.0F, 0.0F, 0.0F, 0.0F });
+            void clear_current_canvas(float r, float g, float b, float a) override
+            {
+                if (!app_.canvas || !app_.canvas->m_canvas)
+                    return;
+
+                app_.canvas->m_canvas->clear({ r, g, b, a });
+            }
+
+        private:
+            App& app_;
+        };
+
+        LegacyDocumentCanvasClearServices services(app_);
+        const auto status = pp::app::execute_document_canvas_clear_plan(plan, services);
+        if (!status.ok())
+            LOG("Canvas clear failed: %s", status.message);
    }

    void show_message_box() override
--- a/tests/app_core/document_canvas_tests.cpp
+++ b/tests/app_core/document_canvas_tests.cpp
@@ -2,9 +2,30 @@
 #include "test_harness.h"

 #include <limits>
+#include <string>

 namespace {

+class FakeDocumentCanvasClearServices final : public pp::app::DocumentCanvasClearServices {
+public:
+    void clear_current_canvas(float r, float g, float b, float a) override
+    {
+        clear_calls += 1;
+        last_r = r;
+        last_g = g;
+        last_b = b;
+        last_a = a;
+        call_order += "clear;";
+    }
+
+    int clear_calls = 0;
+    float last_r = 0.0F;
+    float last_g = 0.0F;
+    float last_b = 0.0F;
+    float last_a = 0.0F;
+    std::string call_order;
+};
+
 void clear_plan_records_legacy_canvas_effects(pp::tests::Harness& harness)
 {
    const auto plan = pp::app::plan_document_canvas_clear(true, 0.0F, 0.1F, 0.2F, 0.3F);
@@ -45,6 +66,54 @@ void clear_plan_rejects_bad_color_channels(pp::tests::Harness& harness)
        0.0F));
 }

+void clear_executor_dispatches_color_to_service(pp::tests::Harness& harness)
+{
+    FakeDocumentCanvasClearServices services;
+    const auto plan = pp::app::plan_document_canvas_clear(true, 0.25F, 0.5F, 0.75F, 1.0F);
+    PP_EXPECT(harness, plan);
+    if (plan) {
+        PP_EXPECT(harness, pp::app::execute_document_canvas_clear_plan(plan.value(), services).ok());
+    }
+
+    PP_EXPECT(harness, services.clear_calls == 1);
+    PP_EXPECT(harness, services.last_r == 0.25F);
+    PP_EXPECT(harness, services.last_g == 0.5F);
+    PP_EXPECT(harness, services.last_b == 0.75F);
+    PP_EXPECT(harness, services.last_a == 1.0F);
+    PP_EXPECT(harness, services.call_order == "clear;");
+}
+
+void clear_executor_preserves_noop_without_canvas(pp::tests::Harness& harness)
+{
+    FakeDocumentCanvasClearServices services;
+    const auto plan = pp::app::plan_document_canvas_clear(false);
+    PP_EXPECT(harness, plan);
+    if (plan) {
+        PP_EXPECT(harness, pp::app::execute_document_canvas_clear_plan(plan.value(), services).ok());
+    }
+
+    PP_EXPECT(harness, services.clear_calls == 0);
+}
+
+void clear_executor_rejects_invalid_color(pp::tests::Harness& harness)
+{
+    FakeDocumentCanvasClearServices services;
+    pp::app::DocumentCanvasClearPlan plan;
+    plan.clears_canvas = true;
+    plan.records_undo = true;
+    plan.marks_unsaved = true;
+    plan.no_op = false;
+    plan.r = 0.0F;
+    plan.g = 0.0F;
+    plan.b = 1.5F;
+    plan.a = 0.0F;
+
+    const auto status = pp::app::execute_document_canvas_clear_plan(plan, services);
+    PP_EXPECT(harness, !status.ok());
+    PP_EXPECT(harness, status.code == pp::foundation::StatusCode::out_of_range);
+    PP_EXPECT(harness, services.clear_calls == 0);
+}
+
 } // namespace

 int main()
@@ -53,5 +122,8 @@ int main()
    harness.run("clear plan records legacy canvas effects", clear_plan_records_legacy_canvas_effects);
    harness.run("clear plan noops without canvas", clear_plan_noops_without_canvas);
    harness.run("clear plan rejects bad color channels", clear_plan_rejects_bad_color_channels);
+    harness.run("clear executor dispatches color to service", clear_executor_dispatches_color_to_service);
+    harness.run("clear executor preserves noop without canvas", clear_executor_preserves_noop_without_canvas);
+    harness.run("clear executor rejects invalid color", clear_executor_rejects_invalid_color);
    return harness.finish();
 }
--- a/tests/app_core/main_toolbar_tests.cpp
+++ b/tests/app_core/main_toolbar_tests.cpp
@@ -10,7 +10,11 @@ public:
    void invoke_undo() override { undo_calls += 1; }
    void invoke_redo() override { redo_calls += 1; }
    void clear_history() override { clear_history_calls += 1; }
-    void clear_canvas() override { clear_canvas_calls += 1; }
+    void clear_canvas(const pp::app::DocumentCanvasClearPlan& plan) override
+    {
+        clear_canvas_calls += 1;
+        last_clear = plan;
+    }
    void show_message_box() override { message_boxes += 1; }
    void show_settings_dialog() override { settings_dialogs += 1; }

@@ -34,6 +38,7 @@ public:
    int clear_canvas_calls = 0;
    int message_boxes = 0;
    int settings_dialogs = 0;
+    pp::app::DocumentCanvasClearPlan last_clear;
 };

 void direct_dialog_commands_are_available(pp::tests::Harness& harness)
@@ -107,6 +112,9 @@ void canvas_clear_requires_live_canvas(pp::tests::Harness& harness)
        PP_EXPECT(harness, clear.value().requires_canvas);
        PP_EXPECT(harness, clear.value().records_undo);
        PP_EXPECT(harness, clear.value().marks_unsaved);
+        PP_EXPECT(harness, clear.value().canvas_clear.clears_canvas);
+        PP_EXPECT(harness, clear.value().canvas_clear.records_undo);
+        PP_EXPECT(harness, clear.value().canvas_clear.marks_unsaved);
        PP_EXPECT(harness, !clear.value().no_op);
    }

@@ -123,6 +131,7 @@ void canvas_clear_requires_live_canvas(pp::tests::Harness& harness)
        PP_EXPECT(harness, missing_canvas.value().requires_canvas);
        PP_EXPECT(harness, !missing_canvas.value().records_undo);
        PP_EXPECT(harness, !missing_canvas.value().marks_unsaved);
+        PP_EXPECT(harness, !missing_canvas.value().canvas_clear.clears_canvas);
        PP_EXPECT(harness, missing_canvas.value().no_op);
    }
 }
@@ -165,6 +174,9 @@ void executor_dispatches_to_service_boundary(pp::tests::Harness& harness)
        const auto status = pp::app::execute_main_toolbar_plan(clear_canvas.value(), services);
        PP_EXPECT(harness, status.ok());
        PP_EXPECT(harness, services.clear_canvas_calls == 1);
+        PP_EXPECT(harness, services.last_clear.clears_canvas);
+        PP_EXPECT(harness, services.last_clear.records_undo);
+        PP_EXPECT(harness, services.last_clear.marks_unsaved);
    }

    auto redo_empty = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::redo);