diff --git a/CMakeLists.txt b/CMakeLists.txt index 474a7b3..b99fc70 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -228,6 +228,7 @@ add_library(pp_app_core STATIC src/app_core/brush_ui.h src/app_core/canvas_tool_ui.h src/app_core/document_animation.h + src/app_core/document_canvas.h src/app_core/document_cloud.h src/app_core/document_export.cpp src/app_core/document_layer.h diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 2f41b4b..e2106bc 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -45,6 +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 | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 8c5150e..b8e2c02 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -507,6 +507,10 @@ toolbar active-state refresh used by `App::update` before legacy `Canvas` mode state remains the source of truth. `NodeCanvas` stylus eraser and `E` key 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. `pano_cli plan-grid-operation` exposes app-core planning for grid heightmap pick/load/reload/clear, lightmap render capability/limit checks, and heightmap commit used by the live grid panel before legacy image loading, OpenGL texture @@ -1180,6 +1184,13 @@ Results: `pano_cli_plan_canvas_tool_state_copy_smoke`, and `pano_cli_plan_canvas_tool_state_rejects_unknown` passed and expose draw 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. +- `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 + canvas clear planning as JSON automation. - `pp_app_core_history_ui_tests` passed, covering undo/redo availability, no-op history commands, clear-history stack/memory state, memory-only clear, and negative metric rejection. diff --git a/src/app_core/document_canvas.h b/src/app_core/document_canvas.h new file mode 100644 index 0000000..b218e0e --- /dev/null +++ b/src/app_core/document_canvas.h @@ -0,0 +1,58 @@ +#pragma once + +#include "foundation/result.h" + +#include + +namespace pp::app { + +struct DocumentCanvasClearPlan { + float r = 0.0F; + float g = 0.0F; + float b = 0.0F; + float a = 0.0F; + bool clears_canvas = false; + bool records_undo = false; + bool marks_unsaved = false; + bool no_op = true; +}; + +[[nodiscard]] inline pp::foundation::Status validate_clear_color_channel(float value) noexcept +{ + if (!std::isfinite(value)) { + return pp::foundation::Status::invalid_argument("clear color channel must be finite"); + } + if (value < 0.0F || value > 1.0F) { + return pp::foundation::Status::out_of_range("clear color channel must be within 0..1"); + } + return pp::foundation::Status::success(); +} + +[[nodiscard]] inline pp::foundation::Result plan_document_canvas_clear( + bool has_canvas, + float r = 0.0F, + float g = 0.0F, + float b = 0.0F, + float a = 0.0F) noexcept +{ + const float channels[] { r, g, b, a }; + for (const float channel : channels) { + const auto status = validate_clear_color_channel(channel); + if (!status.ok()) { + return pp::foundation::Result::failure(status); + } + } + + DocumentCanvasClearPlan plan; + plan.r = r; + plan.g = g; + plan.b = b; + plan.a = a; + plan.clears_canvas = has_canvas; + plan.records_undo = has_canvas; + plan.marks_unsaved = has_canvas; + plan.no_op = !has_canvas; + return pp::foundation::Result::success(plan); +} + +} // namespace pp::app diff --git a/src/app_layout.cpp b/src/app_layout.cpp index 5bcb789..a444544 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -10,6 +10,7 @@ #include "app_core/brush_ui.h" #include "app_core/canvas_tool_ui.h" #include "app_core/document_layer.h" +#include "app_core/document_canvas.h" #include "app_core/app_status.h" #include "app_core/history_ui.h" #include "settings.h" @@ -149,8 +150,13 @@ void App::init_toolbar_main() { button->on_click = [this](Node*) { //exit(0); - if (canvas) - canvas->m_canvas->clear({ 0, 0, 0, 0 }); + const auto plan = pp::app::plan_document_canvas_clear(static_cast(canvas)); + if (plan && plan.value().clears_canvas) + canvas->m_canvas->clear({ + plan.value().r, + plan.value().g, + plan.value().b, + plan.value().a }); }; } if (auto* button = layout[main_id]->find("btn-popup")) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cd0c582..b327acb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -328,6 +328,16 @@ add_test(NAME pp_app_core_document_route_tests COMMAND pp_app_core_document_rout set_tests_properties(pp_app_core_document_route_tests PROPERTIES LABELS "app;desktop-fast;fuzz") +add_executable(pp_app_core_document_canvas_tests + app_core/document_canvas_tests.cpp) +target_link_libraries(pp_app_core_document_canvas_tests PRIVATE + pp_app_core + pp_test_harness) + +add_test(NAME pp_app_core_document_canvas_tests COMMAND pp_app_core_document_canvas_tests) +set_tests_properties(pp_app_core_document_canvas_tests PROPERTIES + LABELS "app;document;desktop-fast;fuzz") + add_executable(pp_app_core_document_export_tests app_core/document_export_tests.cpp) target_link_libraries(pp_app_core_document_export_tests PRIVATE @@ -760,6 +770,24 @@ if(TARGET pano_cli) LABELS "app;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_canvas_clear_smoke + COMMAND pano_cli plan-canvas-clear --r 0 --g 0.1 --b 0.2 --a 0.3) + set_tests_properties(pano_cli_plan_canvas_clear_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-canvas-clear\".*\"r\":0.*\"g\":0.1.*\"b\":0.2.*\"a\":0.3.*\"clearsCanvas\":true.*\"recordsUndo\":true.*\"marksUnsaved\":true") + + add_test(NAME pano_cli_plan_canvas_clear_no_canvas_smoke + COMMAND pano_cli plan-canvas-clear --no-canvas) + set_tests_properties(pano_cli_plan_canvas_clear_no_canvas_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast;fuzz" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-canvas-clear\".*\"hasCanvas\":false.*\"clearsCanvas\":false.*\"recordsUndo\":false.*\"marksUnsaved\":false.*\"noOp\":true") + + add_test(NAME pano_cli_plan_canvas_clear_rejects_bad_color + COMMAND pano_cli plan-canvas-clear --r 1.5) + set_tests_properties(pano_cli_plan_canvas_clear_rejects_bad_color PROPERTIES + LABELS "app;document;integration;desktop-fast;fuzz" + WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_layer_rename_smoke COMMAND pano_cli plan-layer-rename --old-name Base --new-name Paint) set_tests_properties(pano_cli_plan_layer_rename_smoke PROPERTIES diff --git a/tests/app_core/document_canvas_tests.cpp b/tests/app_core/document_canvas_tests.cpp new file mode 100644 index 0000000..fd72f37 --- /dev/null +++ b/tests/app_core/document_canvas_tests.cpp @@ -0,0 +1,57 @@ +#include "app_core/document_canvas.h" +#include "test_harness.h" + +#include + +namespace { + +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); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().clears_canvas); + PP_EXPECT(harness, plan.value().records_undo); + PP_EXPECT(harness, plan.value().marks_unsaved); + PP_EXPECT(harness, !plan.value().no_op); + PP_EXPECT(harness, plan.value().r == 0.0F); + PP_EXPECT(harness, plan.value().g == 0.1F); + PP_EXPECT(harness, plan.value().b == 0.2F); + PP_EXPECT(harness, plan.value().a == 0.3F); + } +} + +void clear_plan_noops_without_canvas(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_canvas_clear(false); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, !plan.value().clears_canvas); + PP_EXPECT(harness, !plan.value().records_undo); + PP_EXPECT(harness, !plan.value().marks_unsaved); + PP_EXPECT(harness, plan.value().no_op); + } +} + +void clear_plan_rejects_bad_color_channels(pp::tests::Harness& harness) +{ + PP_EXPECT(harness, !pp::app::plan_document_canvas_clear(true, -0.01F, 0.0F, 0.0F, 0.0F)); + PP_EXPECT(harness, !pp::app::plan_document_canvas_clear(true, 0.0F, 1.01F, 0.0F, 0.0F)); + PP_EXPECT(harness, !pp::app::plan_document_canvas_clear( + true, + 0.0F, + 0.0F, + std::numeric_limits::infinity(), + 0.0F)); +} + +} // namespace + +int main() +{ + pp::tests::Harness harness; + 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); + return harness.finish(); +} diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index ff516bb..c41ebca 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -3,6 +3,7 @@ #include "app_core/brush_ui.h" #include "app_core/canvas_tool_ui.h" #include "app_core/document_animation.h" +#include "app_core/document_canvas.h" #include "app_core/document_export.h" #include "app_core/document_cloud.h" #include "app_core/document_layer.h" @@ -226,6 +227,14 @@ struct PlanDocumentResizeArgs { int selected_resolution_index = 0; }; +struct PlanCanvasClearArgs { + bool has_canvas = true; + float r = 0.0F; + float g = 0.0F; + float b = 0.0F; + float a = 0.0F; +}; + struct PlanLayerRenameArgs { std::string old_name = "Layer 1"; std::string new_name; @@ -1005,6 +1014,7 @@ void print_help() << " plan-recording-session [--running] [--frame-count N] [--platform-deletes-recorded-files]\n" << " plan-app-preferences [--ui-scale N] [--display-density N] [--current-scale N] [--scale-option N] [--viewport-scale N] [--rtl] [--timelapse-disabled] [--recording-running] [--vr-controllers-disabled] [--cursor-mode N]\n" << " plan-app-status [--doc-name NAME] [--unsaved] [--resolution N] [--resolution-index N] [--zoom N] [--history-bytes N] [--recording-running] [--encoder-available] [--encoded-frames N]\n" + << " plan-canvas-clear [--no-canvas] [--r N] [--g N] [--b N] [--a N]\n" << " plan-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" @@ -2609,6 +2619,79 @@ int plan_document_resize(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_canvas_clear_args( + int argc, + char** argv, + PlanCanvasClearArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--r" || key == "--g" || key == "--b" || key == "--a") { + 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(); + } + if (key == "--r") { + args.r = value.value(); + } else if (key == "--g") { + args.g = value.value(); + } else if (key == "--b") { + args.b = value.value(); + } else { + args.a = value.value(); + } + } else if (key == "--no-canvas") { + args.has_canvas = false; + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +int plan_canvas_clear(int argc, char** argv) +{ + PlanCanvasClearArgs args; + const auto status = parse_plan_canvas_clear_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-canvas-clear", status.message); + return 2; + } + + const auto plan = pp::app::plan_document_canvas_clear( + args.has_canvas, + args.r, + args.g, + args.b, + args.a); + if (!plan) { + print_error("plan-canvas-clear", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-canvas-clear\"" + << ",\"state\":{\"hasCanvas\":" << json_bool(args.has_canvas) + << ",\"r\":" << args.r + << ",\"g\":" << args.g + << ",\"b\":" << args.b + << ",\"a\":" << args.a + << "},\"plan\":{\"r\":" << value.r + << ",\"g\":" << value.g + << ",\"b\":" << value.b + << ",\"a\":" << value.a + << ",\"clearsCanvas\":" << json_bool(value.clears_canvas) + << ",\"recordsUndo\":" << json_bool(value.records_undo) + << ",\"marksUnsaved\":" << json_bool(value.marks_unsaved) + << ",\"noOp\":" << json_bool(value.no_op) + << "}}\n"; + return 0; +} + pp::foundation::Status parse_plan_layer_rename_args( int argc, char** argv, @@ -5999,6 +6082,10 @@ int main(int argc, char** argv) return plan_document_resize(argc, argv); } + if (command == "plan-canvas-clear") { + return plan_canvas_clear(argc, argv); + } + if (command == "plan-layer-rename") { return plan_layer_rename(argc, argv); }