From 47c35fb8592fe4d635787c95d179cad700e0ed72 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Thu, 4 Jun 2026 15:15:01 +0200 Subject: [PATCH] Route brush preset list planning --- docs/modernization/build-inventory.md | 9 +- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 12 +++ src/app_core/brush_ui.h | 138 ++++++++++++++++++++++++ src/node_panel_brush.cpp | 147 ++++++++++++++++++-------- src/node_panel_brush.h | 2 + tests/CMakeLists.txt | 24 +++++ tests/app_core/brush_ui_tests.cpp | 80 ++++++++++++++ tools/pano_cli/main.cpp | 129 ++++++++++++++++++++++ 9 files changed, 497 insertions(+), 46 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 747715d..942ed2a 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -193,10 +193,13 @@ Known local toolchain state: `NodePanelAnimation` friend adapter remain tracked by `DEBT-0022`. - `src/legacy_brush_ui_services.*` is the current UI-shell bridge for brush color, texture, preset, stroke-refresh, brush texture-list, and stroke-control - execution. It keeps those live paths on the `pp_app_core` contracts while + execution. `NodePanelBrushPreset` now consumes `pp_app_core` preset-list + planning for add/select/move/remove/clear before directly mutating retained + legacy child nodes. These paths stay on the `pp_app_core` contracts while legacy `Brush`, `Canvas::I`, image load/save, `NodePanelBrush`, - `NodePanelStroke`, quick/color refreshes, and the temporary - `NodePanelBrush` friend adapter remain tracked by `DEBT-0023`. + `NodePanelStroke`, quick/color refreshes, direct preset child-node mutation, + and the temporary `NodePanelBrush` friend adapter remain tracked by + `DEBT-0023`. - `src/legacy_grid_ui_services.*` is the current UI-shell bridge for grid heightmap picker/load/reload/clear, lightmap render, and heightmap commit execution. It keeps those live paths on the `pp_app_core` contracts while diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 4351dae..a5c4a0d 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -40,7 +40,7 @@ agent or engineer to remove them without reconstructing context from chat. | 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, and live resize shares `src/legacy_document_canvas_services.*` with canvas clear commands, but the shared live bridge still calls legacy `Canvas::resize`, updates the legacy app title, and clears legacy `ActionManager` history through the history bridge | 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 planning/execution dispatch 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`, `DocumentLayerRenameServices`, and `DocumentLayerOperationServices`, and the live execution adapters are centralized in `src/legacy_document_layer_services.*`, but that shared bridge still mutates legacy `Canvas` layer state, `NodeLayer`/`NodePanelLayer`, and `ActionManager` undo entries | 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`; live execution is centralized in `src/legacy_document_animation_services.*`, but that bridge still mutates or reads legacy `Canvas`/`Layer` frame state, canvas mode, animation-panel timeline/playback fields, and uses a temporary `NodePanelAnimation` friend adapter | 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`, and live execution is centralized in `src/legacy_brush_ui_services.*`, but the bridge still mutates legacy `Brush`/`Canvas::I`, loads/saves legacy brush texture images, refreshes legacy quick/stroke/color widgets, and uses a temporary `NodePanelBrush` friend adapter to reach private list state | 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 or `NodePanelBrush` friend access | +| DEBT-0023 | Open | Modernization | Brush/color/preset/stroke-settings UI planning, texture-list add/remove/reorder planning, brush preset-list add/select/move/remove/clear planning, stroke-panel slider/toggle/blend/reset planning, and execution dispatch now consume pure `pp_app_core` through `App::init_sidebar`, `NodePanelBrush`, `NodePanelBrushPreset`, `NodePanelStroke`, restored/docked floating-panel callbacks, `pano_cli plan-brush-operation`, `pano_cli plan-brush-texture-list`, `pano_cli plan-brush-preset-list`, `pano_cli plan-brush-stroke-control`, `BrushUiServices`, `BrushTextureListServices`, and `BrushStrokeControlServices`, and live execution is centralized in `src/legacy_brush_ui_services.*` where possible, but preset-list execution still mutates legacy `NodePanelBrushPreset` child nodes directly while the bridge still mutates legacy `Brush`/`Canvas::I`, loads/saves legacy brush texture images, refreshes legacy quick/stroke/color widgets, and uses a temporary `NodePanelBrush` friend adapter to reach private list state | Preserve existing brush UI behavior while brush commands move toward a brush/app/asset command boundary and asset-managed texture/preset 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-preset-list --kind remove --item-count 1 --current-index 0`; `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, preset-list, and stroke-control execution are owned by injected brush/app/asset/UI services with no legacy brush/canvas adapter, direct `NodePanelBrushPreset` child mutation, or `NodePanelBrush` friend access | | DEBT-0024 | Open | Modernization | Grid/heightmap/lightmap UI planning and execution dispatch now consume pure `pp_app_core` through `NodePanelGrid`, `pano_cli plan-grid-operation`, and the `GridUiServices` boundary; live execution is centralized in `src/legacy_grid_ui_services.*`, but the bridge still performs legacy image loading, OpenGL texture updates, nanort lightmap baking/progress, and `Canvas::draw_objects` commit execution | 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 | | DEBT-0025 | Open | Modernization | Quick brush/color slot and mini-state planning and execution dispatch now consume pure `pp_app_core` through `NodePanelQuick`, `pano_cli plan-quick-operation`, and the `QuickUiServices` boundary; live execution is centralized in `src/legacy_quick_ui_services.*`, but the bridge still mutates legacy quick UI widgets, `Brush` previews, color picker popup state, and preset popup state | 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 injected app/brush/UI services with no legacy quick-panel adapter | | DEBT-0026 | Open | Modernization | Toolbar history command planning and canvas hotkey history dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `NodeCanvas`, `pano_cli plan-history-operation`, and the `HistoryUiServices` boundary, and both live callers share `src/legacy_history_services.*` for saturated legacy history metrics and execution, but the shared live bridge still mutates legacy `ActionManager` stacks 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 injected document/app history services with no legacy `ActionManager` adapter | diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index e99f3f9..9e684c9 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -742,6 +742,13 @@ layer-refresh, and action-history work remains tracked under `DEBT-0038`. `pano_cli parse-layout` exercises the XML layout path. Continue expanding document behavior toward legacy Canvas parity and then port OpenGL classes behind the renderer boundary. +Brush preset-list add/select/move/remove/clear decisions now consume +`pp_app_core` through `NodePanelBrushPreset` and +`pano_cli plan-brush-preset-list`, so preset UI callbacks share tested +headless index/selection planning before mutating legacy child nodes. The +remaining direct `NodePanelBrushPreset` child-node execution, legacy +`Brush` cloning, and preset save/reload behavior stay tracked under +`DEBT-0023`. `App::open_document` now routes through the app-core document-open executor and `src/legacy_document_open_services.*`, preserving ABR/PPBR import prompts, unsaved-project discard prompts, project open, layer refresh, title updates, @@ -1422,6 +1429,11 @@ Results: - Focused brush import storage CTest coverage passed for `pp_assets_brush_package_tests` and the brush package import/export CLI smoke/failure tests. +- `PanoPainter`, `pp_app_core_brush_ui_tests`, and `pano_cli` built after brush + preset-list add/select/move/remove/clear planning moved into `pp_app_core`. +- Focused brush preset-list CTest coverage passed for + `pp_app_core_brush_ui_tests` and `pano_cli_plan_brush_preset_list_*` smoke + tests. - `pp_app_core_document_recording_tests` passed, covering recording start/stop, clear, platform recorded-file cleanup, frame-count reset, export progress totals, and oversized progress-total clamping. diff --git a/src/app_core/brush_ui.h b/src/app_core/brush_ui.h index fd26f9a..9638de1 100644 --- a/src/app_core/brush_ui.h +++ b/src/app_core/brush_ui.h @@ -28,6 +28,14 @@ enum class BrushTextureListOperation { move_texture, }; +enum class BrushPresetListOperation { + add_current_brush, + remove_preset, + move_preset, + select_preset, + clear_presets, +}; + enum class BrushStrokeControlOperation { set_float, set_bool, @@ -139,6 +147,20 @@ struct BrushTextureListPlan { bool no_op = false; }; +struct BrushPresetListPlan { + BrushPresetListOperation operation = BrushPresetListOperation::select_preset; + int item_count = 0; + int current_index = -1; + int target_index = -1; + int move_offset = 0; + bool saves_list = false; + bool updates_empty_notification = false; + bool selects_target = false; + bool clears_selection = false; + bool notifies_brush_changed = false; + bool no_op = false; +}; + struct BrushStrokeControlPlan { BrushStrokeControlOperation operation = BrushStrokeControlOperation::set_float; BrushStrokeFloatSetting float_setting = BrushStrokeFloatSetting::tip_size; @@ -474,6 +496,122 @@ public: return pp::foundation::Result::success(plan); } +[[nodiscard]] inline pp::foundation::Result plan_brush_preset_list_add( + int item_count, + bool has_current_brush) +{ + if (item_count < 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("brush preset item count must not be negative")); + } + if (!has_current_brush) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("current brush must be available to add a preset")); + } + + BrushPresetListPlan plan; + plan.operation = BrushPresetListOperation::add_current_brush; + plan.item_count = item_count; + plan.target_index = item_count; + plan.saves_list = true; + plan.updates_empty_notification = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_preset_list_remove( + int item_count, + int current_index) +{ + if (item_count <= 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush preset list must contain an item to remove")); + } + if (current_index < 0 || current_index >= item_count) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("selected brush preset index is outside the list")); + } + + BrushPresetListPlan plan; + plan.operation = BrushPresetListOperation::remove_preset; + plan.item_count = item_count; + plan.current_index = current_index; + plan.target_index = item_count > 1 ? std::min(current_index, item_count - 2) : -1; + plan.saves_list = true; + plan.updates_empty_notification = true; + plan.selects_target = plan.target_index >= 0; + plan.clears_selection = plan.target_index < 0; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_preset_list_move( + int item_count, + int current_index, + int offset) +{ + if (item_count <= 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush preset list must contain an item to move")); + } + if (current_index < 0 || current_index >= item_count) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("selected brush preset index is outside the list")); + } + if (offset == 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush preset move offset must not be zero")); + } + + BrushPresetListPlan plan; + plan.operation = BrushPresetListOperation::move_preset; + plan.item_count = item_count; + plan.current_index = current_index; + plan.target_index = std::clamp(current_index + offset, 0, item_count - 1); + plan.move_offset = offset; + plan.saves_list = true; + plan.no_op = plan.target_index == current_index; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_preset_list_select( + int item_count, + int index) +{ + if (item_count <= 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush preset list must contain an item to select")); + } + if (index < 0 || index >= item_count) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("selected brush preset index is outside the list")); + } + + BrushPresetListPlan plan; + plan.operation = BrushPresetListOperation::select_preset; + plan.item_count = item_count; + plan.current_index = index; + plan.target_index = index; + plan.selects_target = true; + plan.notifies_brush_changed = true; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_preset_list_clear(int item_count) +{ + if (item_count < 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::out_of_range("brush preset item count must not be negative")); + } + + BrushPresetListPlan plan; + plan.operation = BrushPresetListOperation::clear_presets; + plan.item_count = item_count; + plan.saves_list = true; + plan.updates_empty_notification = true; + plan.clears_selection = true; + plan.no_op = item_count == 0; + return pp::foundation::Result::success(plan); +} + [[nodiscard]] inline pp::foundation::Status execute_brush_ui_plan( const BrushUiPlan& plan, BrushUiServices& services) diff --git a/src/node_panel_brush.cpp b/src/node_panel_brush.cpp index edb0808..cd65d3a 100644 --- a/src/node_panel_brush.cpp +++ b/src/node_panel_brush.cpp @@ -407,25 +407,97 @@ Node* NodePanelBrushPreset::clone_instantiate() const return new NodePanelBrushPreset(); } +void NodePanelBrushPreset::execute_preset_list_plan(const pp::app::BrushPresetListPlan& plan) +{ + switch (plan.operation) + { + case pp::app::BrushPresetListOperation::add_current_brush: + if (!Canvas::I || !Canvas::I->m_current_brush) + return; + for (auto p : s_panels) + p->add_brush(std::make_shared(*Canvas::I->m_current_brush)); + break; + + case pp::app::BrushPresetListOperation::move_preset: + for (auto p : s_panels) + { + if (plan.current_index >= 0 && plan.current_index < static_cast(p->m_container->m_children.size())) + p->m_container->move_child(p->m_container->m_children[plan.current_index].get(), plan.target_index); + } + break; + + case pp::app::BrushPresetListOperation::remove_preset: + for (auto p : s_panels) + { + if (plan.current_index < 0 || plan.current_index >= static_cast(p->m_container->m_children.size())) + continue; + bool new_current = !p->m_current || p->m_container->get_child_index(p->m_current) == plan.current_index; + p->m_container->m_children[plan.current_index]->destroy(); + if (plan.clears_selection) + { + p->m_current = nullptr; + } + else if (new_current && plan.selects_target) + { + p->m_current = static_cast(p->m_container->m_children[plan.target_index].get()); + p->m_current->m_selected = true; + } + p->m_notification->SetVisibility(p->m_container->m_children.size() == 0); + } + break; + + case pp::app::BrushPresetListOperation::select_preset: + for (auto p : s_panels) + { + if (p->m_current) + p->m_current->m_selected = false; + p->m_current = static_cast(p->m_container->get_child_at(plan.target_index)); + p->m_current->m_selected = true; + p->m_interacted = true; + } + if (plan.notifies_brush_changed && on_brush_changed) + on_brush_changed(this, m_current->m_brush); + break; + + case pp::app::BrushPresetListOperation::clear_presets: + for (auto p : s_panels) + { + p->m_container->remove_all_children(); + p->m_current = nullptr; + p->m_notification->SetVisibility(p->m_container->m_children.size() == 0); + } + break; + } + + if (plan.saves_list) + save(); +} + void NodePanelBrushPreset::init() { init_template_file("data/dialogs/panel-brushes.xml", "tpl-panel-brush-preset"); m_container = find("brushes"); m_btn_add = find("btn-add"); m_btn_add->on_click = [this] (Node*) { - for (auto p : s_panels) - p->add_brush(std::make_shared(*Canvas::I->m_current_brush)); - save(); + const auto plan = pp::app::plan_brush_preset_list_add( + static_cast(m_container->m_children.size()), + Canvas::I && Canvas::I->m_current_brush); + if (plan) { + execute_preset_list_plan(plan.value()); + } }; m_btn_up = find("btn-up"); m_btn_up->on_click = [this](Node*) { if (m_current) { int idx = m_container->get_child_index(m_current); - int to = std::max(idx - 1, 0); - for (auto p : s_panels) - p->m_container->move_child(p->m_container->m_children[idx].get(), to); - save(); + const auto plan = pp::app::plan_brush_preset_list_move( + static_cast(m_container->m_children.size()), + idx, + -1); + if (plan) { + execute_preset_list_plan(plan.value()); + } } }; m_btn_down = find("btn-down"); @@ -433,10 +505,13 @@ void NodePanelBrushPreset::init() if (m_current) { int idx = m_container->get_child_index(m_current); - int to = std::min(idx + 1, (int)m_container->m_children.size() - 1); - for (auto p : s_panels) - p->m_container->move_child(p->m_container->m_children[idx].get(), to); - save(); + const auto plan = pp::app::plan_brush_preset_list_move( + static_cast(m_container->m_children.size()), + idx, + 1); + if (plan) { + execute_preset_list_plan(plan.value()); + } } }; /* @@ -456,23 +531,12 @@ void NodePanelBrushPreset::init() if (!m_current) return; int index = m_container->get_child_index(m_current); - for (auto p : s_panels) - { - bool new_current = !p->m_current || p->m_container->get_child_index(p->m_current) == index; - p->m_container->m_children[index]->destroy(); - if (p->m_container->m_children.empty()) - { - p->m_current = nullptr; - } - else if (new_current) - { - int next = std::min((int)p->m_container->m_children.size() - 1, index); - p->m_current = (NodeBrushPresetItem*)p->m_container->m_children[next].get(); - p->m_current->m_selected = true; - } - p->m_notification->SetVisibility(p->m_container->m_children.size() == 0); + const auto plan = pp::app::plan_brush_preset_list_remove( + static_cast(m_container->m_children.size()), + index); + if (plan) { + execute_preset_list_plan(plan.value()); } - save(); }; m_btn_menu = find("btn-menu"); m_btn_menu->on_click = [this](Node* b) { @@ -508,8 +572,11 @@ void NodePanelBrushPreset::init() mb->btn_ok->m_text->set_text("Yes"); mb->btn_cancel->m_text->set_text("No"); mb->btn_ok->on_click = mb->on_submit = [this, mb](Node*) { - App::I->presets->clear_brushes(); - App::I->presets->save(); + const auto plan = pp::app::plan_brush_preset_list_clear( + static_cast(m_container->m_children.size())); + if (plan) { + execute_preset_list_plan(plan.value()); + } mb->destroy(); }; break; @@ -576,16 +643,12 @@ kEventResult NodePanelBrushPreset::handle_event(Event* e) void NodePanelBrushPreset::handle_click(Node* target) { int idx = m_container->get_child_index(target); - for (auto p : s_panels) - { - if (p->m_current) - p->m_current->m_selected = false; - p->m_current = (NodeBrushPresetItem*)p->m_container->get_child_at(idx); - p->m_current->m_selected = true; - p->m_interacted = true; + const auto plan = pp::app::plan_brush_preset_list_select( + static_cast(m_container->m_children.size()), + idx); + if (plan) { + execute_preset_list_plan(plan.value()); } - if (on_brush_changed) - on_brush_changed(this, m_current->m_brush); } bool NodePanelBrushPreset::save() @@ -1067,10 +1130,10 @@ bool NodePanelBrushPreset::import_brush(const std::string& path) void NodePanelBrushPreset::clear_brushes() { - for (auto p : s_panels) - { - p->m_container->remove_all_children(); - p->m_notification->SetVisibility(p->m_container->m_children.size() == 0); + const auto plan = pp::app::plan_brush_preset_list_clear( + static_cast(m_container->m_children.size())); + if (plan) { + execute_preset_list_plan(plan.value()); } } diff --git a/src/node_panel_brush.h b/src/node_panel_brush.h index aa3295f..f5d61c0 100644 --- a/src/node_panel_brush.h +++ b/src/node_panel_brush.h @@ -11,6 +11,7 @@ namespace pp::app { struct BrushTextureListPlan; +struct BrushPresetListPlan; } namespace pp::panopainter { class LegacyBrushTextureListServices; @@ -99,6 +100,7 @@ class NodePanelBrushPreset : public Node NodeButton* m_btn_import; NodeButton* m_btn_download; Node* m_notification; + void execute_preset_list_plan(const pp::app::BrushPresetListPlan& plan); public: struct PPBRInfo { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 645a347..0227e5c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1317,6 +1317,30 @@ if(TARGET pano_cli) LABELS "app;paint;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_brush_preset_list_add_smoke + COMMAND pano_cli plan-brush-preset-list --kind add --item-count 2) + set_tests_properties(pano_cli_plan_brush_preset_list_add_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-preset-list\".*\"operation\":\"add-current-brush\".*\"itemCount\":2.*\"targetIndex\":2.*\"savesList\":true.*\"updatesEmptyNotification\":true") + + add_test(NAME pano_cli_plan_brush_preset_list_remove_only_smoke + COMMAND pano_cli plan-brush-preset-list --kind remove --item-count 1 --current-index 0) + set_tests_properties(pano_cli_plan_brush_preset_list_remove_only_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-preset-list\".*\"operation\":\"remove-preset\".*\"itemCount\":1.*\"targetIndex\":-1.*\"selectsTarget\":false.*\"clearsSelection\":true") + + add_test(NAME pano_cli_plan_brush_preset_list_move_edge_smoke + COMMAND pano_cli plan-brush-preset-list --kind up --item-count 3 --current-index 0) + set_tests_properties(pano_cli_plan_brush_preset_list_move_edge_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-preset-list\".*\"operation\":\"move-preset\".*\"currentIndex\":0.*\"targetIndex\":0.*\"moveOffset\":-1.*\"noOp\":true") + + add_test(NAME pano_cli_plan_brush_preset_list_select_smoke + COMMAND pano_cli plan-brush-preset-list --kind select --item-count 3 --current-index 2) + set_tests_properties(pano_cli_plan_brush_preset_list_select_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-preset-list\".*\"operation\":\"select-preset\".*\"targetIndex\":2.*\"savesList\":false.*\"notifiesBrushChanged\":true") + add_test(NAME pano_cli_plan_brush_stroke_control_float_smoke COMMAND pano_cli plan-brush-stroke-control --kind float --setting tip-size --value 42.5) set_tests_properties(pano_cli_plan_brush_stroke_control_float_smoke PROPERTIES diff --git a/tests/app_core/brush_ui_tests.cpp b/tests/app_core/brush_ui_tests.cpp index a32f6e6..103fcda 100644 --- a/tests/app_core/brush_ui_tests.cpp +++ b/tests/app_core/brush_ui_tests.cpp @@ -404,6 +404,84 @@ void texture_list_remove_and_move_plans_handle_edges(pp::tests::Harness& harness PP_EXPECT(harness, !pp::app::plan_brush_texture_list_move(2, -1, 1)); } +void preset_list_plans_add_select_move_remove_and_clear(pp::tests::Harness& harness) +{ + const auto add = pp::app::plan_brush_preset_list_add(2, true); + PP_EXPECT(harness, add); + if (add) { + PP_EXPECT(harness, add.value().operation == pp::app::BrushPresetListOperation::add_current_brush); + PP_EXPECT(harness, add.value().target_index == 2); + PP_EXPECT(harness, add.value().saves_list); + PP_EXPECT(harness, add.value().updates_empty_notification); + } + + const auto select = pp::app::plan_brush_preset_list_select(3, 1); + PP_EXPECT(harness, select); + if (select) { + PP_EXPECT(harness, select.value().operation == pp::app::BrushPresetListOperation::select_preset); + PP_EXPECT(harness, select.value().target_index == 1); + PP_EXPECT(harness, select.value().selects_target); + PP_EXPECT(harness, select.value().notifies_brush_changed); + PP_EXPECT(harness, !select.value().saves_list); + } + + const auto move_up_edge = pp::app::plan_brush_preset_list_move(3, 0, -1); + PP_EXPECT(harness, move_up_edge); + if (move_up_edge) { + PP_EXPECT(harness, move_up_edge.value().target_index == 0); + PP_EXPECT(harness, move_up_edge.value().no_op); + PP_EXPECT(harness, move_up_edge.value().saves_list); + } + + const auto move_down = pp::app::plan_brush_preset_list_move(3, 1, 1); + PP_EXPECT(harness, move_down); + if (move_down) { + PP_EXPECT(harness, move_down.value().operation == pp::app::BrushPresetListOperation::move_preset); + PP_EXPECT(harness, move_down.value().target_index == 2); + PP_EXPECT(harness, !move_down.value().no_op); + } + + const auto remove_middle = pp::app::plan_brush_preset_list_remove(3, 1); + PP_EXPECT(harness, remove_middle); + if (remove_middle) { + PP_EXPECT(harness, remove_middle.value().operation == pp::app::BrushPresetListOperation::remove_preset); + PP_EXPECT(harness, remove_middle.value().target_index == 1); + PP_EXPECT(harness, remove_middle.value().selects_target); + PP_EXPECT(harness, !remove_middle.value().clears_selection); + PP_EXPECT(harness, remove_middle.value().saves_list); + } + + const auto remove_only = pp::app::plan_brush_preset_list_remove(1, 0); + PP_EXPECT(harness, remove_only); + if (remove_only) { + PP_EXPECT(harness, remove_only.value().target_index == -1); + PP_EXPECT(harness, !remove_only.value().selects_target); + PP_EXPECT(harness, remove_only.value().clears_selection); + } + + const auto clear_empty = pp::app::plan_brush_preset_list_clear(0); + PP_EXPECT(harness, clear_empty); + if (clear_empty) { + PP_EXPECT(harness, clear_empty.value().operation == pp::app::BrushPresetListOperation::clear_presets); + PP_EXPECT(harness, clear_empty.value().no_op); + PP_EXPECT(harness, clear_empty.value().saves_list); + PP_EXPECT(harness, clear_empty.value().clears_selection); + } +} + +void preset_list_plans_reject_breaking_points(pp::tests::Harness& harness) +{ + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_add(-1, true)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_add(0, false)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_select(0, 0)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_select(2, 2)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_remove(0, 0)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_remove(2, -1)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_move(2, 0, 0)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_move(2, 3, 1)); + PP_EXPECT(harness, !pp::app::plan_brush_preset_list_clear(-1)); +} + void executor_dispatches_color_and_refresh(pp::tests::Harness& harness) { FakeBrushUiServices services; @@ -617,6 +695,8 @@ int main() harness.run("stroke control plans validate values and reject breaking points", stroke_control_plans_validate_values_and_reject_breaking_points); harness.run("texture list add plans target paths and rejects bad input", texture_list_add_plans_target_paths_and_rejects_bad_input); harness.run("texture list remove and move plans handle edges", texture_list_remove_and_move_plans_handle_edges); + harness.run("preset list plans add select move remove and clear", preset_list_plans_add_select_move_remove_and_clear); + harness.run("preset list plans reject breaking points", preset_list_plans_reject_breaking_points); harness.run("executor dispatches color and refresh", executor_dispatches_color_and_refresh); harness.run("executor dispatches texture and preset", executor_dispatches_texture_and_preset); harness.run("executor dispatches stroke refresh only", executor_dispatches_stroke_refresh_only); diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index 47306b9..a17e4cc 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -368,6 +368,14 @@ struct PlanBrushTextureListArgs { bool current_is_user_texture = false; }; +struct PlanBrushPresetListArgs { + std::string kind = "select"; + int item_count = 1; + int current_index = 0; + int offset = 1; + bool has_current_brush = true; +}; + struct PlanBrushStrokeControlArgs { std::string kind = "float"; std::string setting = "tip-size"; @@ -1199,6 +1207,24 @@ const char* brush_texture_list_operation_name(pp::app::BrushTextureListOperation return "add-texture"; } +const char* brush_preset_list_operation_name(pp::app::BrushPresetListOperation operation) noexcept +{ + switch (operation) { + case pp::app::BrushPresetListOperation::add_current_brush: + return "add-current-brush"; + case pp::app::BrushPresetListOperation::remove_preset: + return "remove-preset"; + case pp::app::BrushPresetListOperation::move_preset: + return "move-preset"; + case pp::app::BrushPresetListOperation::select_preset: + return "select-preset"; + case pp::app::BrushPresetListOperation::clear_presets: + return "clear-presets"; + } + + return "select-preset"; +} + const char* brush_stroke_control_operation_name(pp::app::BrushStrokeControlOperation operation) noexcept { switch (operation) { @@ -1897,6 +1923,7 @@ void print_help() << " plan-animation-panel-action --action goto|next|prev|playback|toggle-playback [--total-duration N] [--current-frame N] [--target-frame N] [--playing]\n" << " plan-brush-operation --kind color|tip|pattern|dual|preset|settings [--path FILE] [--thumb FILE] [--r N] [--g N] [--b N] [--a N] [--no-brush]\n" << " plan-brush-texture-list --kind add|remove|move [--dir NAME] [--data-path DIR] [--source FILE] [--item-count N] [--current-index N] [--offset N] [--user-texture]\n" + << " plan-brush-preset-list --kind add|remove|move|up|down|select|clear [--item-count N] [--current-index N] [--offset N] [--no-current-brush]\n" << " plan-brush-stroke-control --kind float|bool|blend|tip-aspect-reset|default-reset [--setting NAME] [--value N] [--enabled|--disabled] [--blend-mode N]\n" << " plan-paint-feedback [--width N] [--height N] [--simple|--complex] [--framebuffer-fetch] [--texture-copy] [--blit] [--explicit-transitions] [--render-only] [--depth]\n" << " plan-stroke-composite [--width N] [--height N] [--layer-blend N] [--stroke-blend N] [--dual-blend] [--pattern-blend] [--framebuffer-fetch] [--texture-copy] [--blit] [--explicit-transitions] [--render-only] [--depth]\n" @@ -5032,6 +5059,104 @@ int plan_brush_texture_list(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_brush_preset_list_args( + int argc, + char** argv, + PlanBrushPresetListArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--kind") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + args.kind = argv[++i]; + } else if (key == "--item-count" || key == "--current-index" || key == "--offset") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + const auto value = parse_i32_arg(argv[++i]); + if (!value) { + return value.status(); + } + if (key == "--item-count") { + args.item_count = value.value(); + } else if (key == "--current-index") { + args.current_index = value.value(); + } else { + args.offset = value.value(); + } + } else if (key == "--no-current-brush") { + args.has_current_brush = false; + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +pp::foundation::Result make_brush_preset_list_plan( + const PlanBrushPresetListArgs& args) +{ + if (args.kind == "add") { + return pp::app::plan_brush_preset_list_add(args.item_count, args.has_current_brush); + } + if (args.kind == "remove") { + return pp::app::plan_brush_preset_list_remove(args.item_count, args.current_index); + } + if (args.kind == "move" || args.kind == "up" || args.kind == "down") { + const int offset = args.kind == "up" ? -1 : (args.kind == "down" ? 1 : args.offset); + return pp::app::plan_brush_preset_list_move(args.item_count, args.current_index, offset); + } + if (args.kind == "select") { + return pp::app::plan_brush_preset_list_select(args.item_count, args.current_index); + } + if (args.kind == "clear") { + return pp::app::plan_brush_preset_list_clear(args.item_count); + } + + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("unknown brush preset list operation kind")); +} + +int plan_brush_preset_list(int argc, char** argv) +{ + PlanBrushPresetListArgs args; + const auto status = parse_plan_brush_preset_list_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-brush-preset-list", status.message); + return 2; + } + + const auto plan = make_brush_preset_list_plan(args); + if (!plan) { + print_error("plan-brush-preset-list", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-brush-preset-list\"" + << ",\"state\":{\"kind\":\"" << json_escape(args.kind) + << "\",\"itemCount\":" << args.item_count + << ",\"currentIndex\":" << args.current_index + << ",\"offset\":" << args.offset + << ",\"hasCurrentBrush\":" << json_bool(args.has_current_brush) + << "},\"plan\":{\"operation\":\"" << brush_preset_list_operation_name(value.operation) + << "\",\"itemCount\":" << value.item_count + << ",\"currentIndex\":" << value.current_index + << ",\"targetIndex\":" << value.target_index + << ",\"moveOffset\":" << value.move_offset + << ",\"savesList\":" << json_bool(value.saves_list) + << ",\"updatesEmptyNotification\":" << json_bool(value.updates_empty_notification) + << ",\"selectsTarget\":" << json_bool(value.selects_target) + << ",\"clearsSelection\":" << json_bool(value.clears_selection) + << ",\"notifiesBrushChanged\":" << json_bool(value.notifies_brush_changed) + << ",\"noOp\":" << json_bool(value.no_op) + << "}}\n"; + return 0; +} + pp::foundation::Result parse_brush_stroke_float_setting( std::string_view setting) { @@ -8852,6 +8977,10 @@ int main(int argc, char** argv) return plan_brush_texture_list(argc, argv); } + if (command == "plan-brush-preset-list") { + return plan_brush_preset_list(argc, argv); + } + if (command == "plan-brush-stroke-control") { return plan_brush_stroke_control(argc, argv); }