diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 3e376fd..afe35bf 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, 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, 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 and execution dispatch now consume pure `pp_app_core` through `App::init_sidebar`, restored/docked floating-panel callbacks, `pano_cli plan-brush-operation`, and the `BrushUiServices` boundary, but the live adapter still mutates legacy `Brush`, calls legacy brush texture loading, and refreshes legacy quick/stroke/color widgets | 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/stroke-settings execution is owned by injected brush/app/asset/UI services with no legacy brush adapter | +| DEBT-0023 | Open | Modernization | Brush/color/preset/stroke-settings UI planning, texture-list add/remove/reorder planning, and execution dispatch now consume pure `pp_app_core` through `App::init_sidebar`, `NodePanelBrush`, restored/docked floating-panel callbacks, `pano_cli plan-brush-operation`, `pano_cli plan-brush-texture-list`, `BrushUiServices`, and `BrushTextureListServices`, but the live adapter still mutates legacy `Brush`, 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`; `ctest --preset desktop-fast --build-config Debug` | Brush color/texture/preset/stroke-settings and texture-list execution are owned by injected brush/app/asset/UI services with no legacy brush 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 | | 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, but the live adapter 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 execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `NodeCanvas`, `pano_cli plan-history-operation`, and the `HistoryUiServices` boundary, but the live adapter 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 8aeeb2d..ab82b02 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -511,6 +511,10 @@ changes, tip/pattern/dual texture changes, preset brush replacement, and stroke settings refreshes used by the live brush, quick, color, and floating panel callbacks. Brush UI execution now dispatches through `BrushUiServices` before the legacy `Brush`/panel adapter mutates brush state or loads brush resources. +`pano_cli plan-brush-texture-list` exposes app-core planning for brush/pattern +texture add, remove, and reorder actions, and `NodePanelBrush` now dispatches +those actions through `BrushTextureListServices` before the legacy image +load/save and UI-list adapter continues. `pano_cli plan-canvas-tool` exposes app-core planning for draw/erase/line, camera, grid, copy, cut, fill, mask, flood-fill, pick, and touch-lock toolbar commands. Canvas tool execution now dispatches through `CanvasToolServices` @@ -1226,15 +1230,21 @@ Results: live animation-panel planning as JSON automation. - `pp_app_core_brush_ui_tests` passed, covering brush color channel validation, invalid color rejection, texture-path validation, preset-brush availability, - preserve-current-color intent, stroke-settings refresh intent, service - dispatch ordering, texture/preset execution payloads, and invalid execution - payload rejection. + preserve-current-color intent, stroke-settings refresh intent, texture-list + add target path planning, user-texture removal intent, clamped reorder intent, + service dispatch ordering, texture/preset/list execution payloads, execution + failure preservation, and invalid execution payload rejection. - `pano_cli_plan_brush_operation_color_smoke`, `pano_cli_plan_brush_operation_texture_smoke`, `pano_cli_plan_brush_operation_preset_smoke`, `pano_cli_plan_brush_operation_rejects_bad_color`, and `pano_cli_plan_brush_operation_rejects_empty_texture` passed and expose live brush/color/preset UI planning as JSON automation. +- `pano_cli_plan_brush_texture_list_add_smoke`, + `pano_cli_plan_brush_texture_list_remove_user_smoke`, + `pano_cli_plan_brush_texture_list_move_edge_smoke`, and + `pano_cli_plan_brush_texture_list_rejects_bad_source` passed and expose live + brush/pattern texture-list planning as JSON automation. - `pp_app_core_grid_ui_tests` passed, covering heightmap pick/load/reload/clear planning, lightmap capability and limit checks, missing-heightmap no-op behavior, and commit canvas gating. diff --git a/src/app_core/brush_ui.h b/src/app_core/brush_ui.h index 5cc2c5c..135365b 100644 --- a/src/app_core/brush_ui.h +++ b/src/app_core/brush_ui.h @@ -2,6 +2,7 @@ #include "foundation/result.h" +#include #include #include #include @@ -21,6 +22,12 @@ enum class BrushUiOperation { stroke_settings_changed, }; +enum class BrushTextureListOperation { + add_texture, + remove_texture, + move_texture, +}; + struct BrushUiPlan { BrushUiOperation operation = BrushUiOperation::stroke_settings_changed; BrushUiTextureSlot texture_slot = BrushUiTextureSlot::tip; @@ -37,6 +44,24 @@ struct BrushUiPlan { bool update_brush_ui = false; }; +struct BrushTextureListPlan { + BrushTextureListOperation operation = BrushTextureListOperation::add_texture; + int item_count = 0; + int current_index = -1; + int target_index = -1; + int move_offset = 0; + std::string source_path; + std::string high_path; + std::string thumbnail_path; + std::string brush_name; + bool user_texture = false; + bool deletes_texture_files = false; + bool saves_list = false; + bool notifies_selection = false; + bool converts_brush_alpha = false; + bool no_op = false; +}; + class BrushUiServices { public: virtual ~BrushUiServices() = default; @@ -47,6 +72,41 @@ public: virtual void refresh_brush_ui(bool update_color_ui, bool update_brush_ui) = 0; }; +class BrushTextureListServices { +public: + virtual ~BrushTextureListServices() = default; + + virtual pp::foundation::Status add_texture_from_source( + std::string_view source_path, + std::string_view high_path, + std::string_view thumbnail_path, + std::string_view brush_name, + bool converts_brush_alpha) = 0; + virtual void remove_texture(int index, bool delete_texture_files) = 0; + virtual void move_texture(int from_index, int to_index) = 0; + virtual void select_texture(int index) = 0; + virtual void save_texture_list() = 0; +}; + +[[nodiscard]] inline pp::foundation::Result brush_texture_source_stem( + std::string_view source_path) noexcept +{ + const auto slash = source_path.find_last_of("/\\"); + const auto name_begin = slash == std::string_view::npos ? 0U : slash + 1U; + if (name_begin >= source_path.size()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture source path must contain a file name")); + } + + const auto dot = source_path.find_last_of('.'); + if (dot == std::string_view::npos || dot <= name_begin || dot + 1U >= source_path.size()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture source path must include a file extension")); + } + + return pp::foundation::Result::success(source_path.substr(name_begin, dot - name_begin)); +} + [[nodiscard]] inline pp::foundation::Status validate_brush_ui_color_channel(float value) noexcept { if (!std::isfinite(value) || value < 0.0F || value > 1.0F) { @@ -129,6 +189,93 @@ public: return plan; } +[[nodiscard]] inline pp::foundation::Result plan_brush_texture_list_add( + std::string_view directory_name, + std::string_view data_path, + std::string_view source_path) +{ + if (directory_name.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture directory must not be empty")); + } + if (data_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture data path must not be empty")); + } + + const auto stem = brush_texture_source_stem(source_path); + if (!stem) { + return pp::foundation::Result::failure(stem.status()); + } + + BrushTextureListPlan plan; + plan.operation = BrushTextureListOperation::add_texture; + plan.source_path = std::string(source_path); + plan.brush_name = std::string(stem.value()); + plan.high_path = std::string(data_path) + "/" + std::string(directory_name) + "/" + plan.brush_name + ".png"; + plan.thumbnail_path = std::string(data_path) + "/" + std::string(directory_name) + "/thumbs/" + + plan.brush_name + ".png"; + plan.user_texture = true; + plan.saves_list = true; + plan.converts_brush_alpha = directory_name == "brushes"; + return pp::foundation::Result::success(std::move(plan)); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_texture_list_remove( + int item_count, + int current_index, + bool current_is_user_texture) +{ + if (item_count <= 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture 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 texture index is outside the list")); + } + + BrushTextureListPlan plan; + plan.operation = BrushTextureListOperation::remove_texture; + 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.user_texture = current_is_user_texture; + plan.deletes_texture_files = current_is_user_texture; + plan.saves_list = true; + plan.notifies_selection = plan.target_index >= 0; + return pp::foundation::Result::success(plan); +} + +[[nodiscard]] inline pp::foundation::Result plan_brush_texture_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 texture 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 texture index is outside the list")); + } + if (offset == 0) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush texture move offset must not be zero")); + } + + BrushTextureListPlan plan; + plan.operation = BrushTextureListOperation::move_texture; + 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::Status execute_brush_ui_plan( const BrushUiPlan& plan, BrushUiServices& services) @@ -168,4 +315,59 @@ public: return pp::foundation::Status::invalid_argument("unknown brush UI operation"); } +[[nodiscard]] inline pp::foundation::Status execute_brush_texture_list_plan( + const BrushTextureListPlan& plan, + BrushTextureListServices& services) +{ + switch (plan.operation) { + case BrushTextureListOperation::add_texture: + { + if (plan.source_path.empty() || plan.high_path.empty() || plan.thumbnail_path.empty() + || plan.brush_name.empty()) { + return pp::foundation::Status::invalid_argument("brush texture add plan has incomplete paths"); + } + + const auto add_status = services.add_texture_from_source( + plan.source_path, + plan.high_path, + plan.thumbnail_path, + plan.brush_name, + plan.converts_brush_alpha); + if (!add_status.ok()) { + return add_status; + } + if (plan.saves_list) { + services.save_texture_list(); + } + return pp::foundation::Status::success(); + } + + case BrushTextureListOperation::remove_texture: + if (plan.item_count <= 0 || plan.current_index < 0 || plan.current_index >= plan.item_count) { + return pp::foundation::Status::out_of_range("brush texture remove plan has invalid selection"); + } + services.remove_texture(plan.current_index, plan.deletes_texture_files); + if (plan.notifies_selection && plan.target_index >= 0) { + services.select_texture(plan.target_index); + } + if (plan.saves_list) { + services.save_texture_list(); + } + return pp::foundation::Status::success(); + + case BrushTextureListOperation::move_texture: + if (plan.item_count <= 0 || plan.current_index < 0 || plan.current_index >= plan.item_count + || plan.target_index < 0 || plan.target_index >= plan.item_count) { + return pp::foundation::Status::out_of_range("brush texture move plan has invalid indices"); + } + services.move_texture(plan.current_index, plan.target_index); + if (plan.saves_list) { + services.save_texture_list(); + } + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown brush texture list operation"); +} + } // namespace pp::app diff --git a/src/node_panel_brush.cpp b/src/node_panel_brush.cpp index c61088e..33e122a 100644 --- a/src/node_panel_brush.cpp +++ b/src/node_panel_brush.cpp @@ -1,6 +1,7 @@ #include "pch.h" #include "log.h" #include "node_panel_brush.h" +#include "app_core/brush_ui.h" #include "asset.h" #include "texture.h" @@ -75,6 +76,116 @@ Node* NodePanelBrush::clone_instantiate() const return new NodePanelBrush(); } +void NodePanelBrush::execute_texture_list_plan(const pp::app::BrushTextureListPlan& plan) +{ + class LegacyBrushTextureListServices final : public pp::app::BrushTextureListServices { + public: + explicit LegacyBrushTextureListServices(NodePanelBrush& panel) noexcept + : panel_(panel) + { + } + + pp::foundation::Status add_texture_from_source( + std::string_view source_path, + std::string_view high_path, + std::string_view thumbnail_path, + std::string_view brush_name, + bool converts_brush_alpha) override + { + Image img; + if (!img.load_file(std::string(source_path))) { + return pp::foundation::Status::invalid_argument("brush texture source could not be loaded"); + } + + if (converts_brush_alpha) { + img.gayscale_alpha(); + } + + auto thumbnail_image = img.resize(64, 64).resize_squared(glm::u8vec4(255)); + thumbnail_image.save_png(std::string(thumbnail_path)); + img.save_png(std::string(high_path)); + + NodeButtonBrush* brush = new NodeButtonBrush; + panel_.m_container->add_child(brush); + brush->init(); + brush->create(); + brush->loaded(); + const auto thumbnail_path_string = std::string(thumbnail_path); + brush->set_icon(thumbnail_path_string.c_str()); + brush->thumb_path = std::string(thumbnail_path); + brush->high_path = std::string(high_path); + brush->brush_name = std::string(brush_name); + brush->m_user_brush = true; + brush->on_click = std::bind(&NodePanelBrush::handle_click, &panel_, std::placeholders::_1); + return pp::foundation::Status::success(); + } + + void remove_texture(int index, bool delete_texture_files) override + { + auto* brush = brush_at(index); + if (!brush) { + return; + } + + if (delete_texture_files) { + Asset::delete_file(brush->thumb_path); + Asset::delete_file(brush->high_path); + } + + if (panel_.m_current == brush) { + panel_.m_current = nullptr; + } + panel_.m_container->remove_child(brush); + } + + void move_texture(int from_index, int to_index) override + { + if (auto* brush = brush_at(from_index)) { + panel_.m_container->move_child(brush, to_index); + } + } + + void select_texture(int index) override + { + if (panel_.m_current) { + panel_.m_current->m_selected = false; + } + + panel_.m_current = brush_at(index); + if (!panel_.m_current) { + return; + } + + panel_.m_current->m_selected = true; + if (panel_.on_brush_changed) { + panel_.on_brush_changed(&panel_, index); + } + } + + void save_texture_list() override + { + panel_.save(); + } + + private: + NodeButtonBrush* brush_at(int index) const + { + if (index < 0 || index >= static_cast(panel_.m_container->m_children.size())) { + return nullptr; + } + return static_cast(panel_.m_container->m_children[index].get()); + } + + NodePanelBrush& panel_; + }; + + LegacyBrushTextureListServices services(*this); + const auto status = pp::app::execute_brush_texture_list_plan(plan, services); + if (!status.ok()) { + LOG("Brush texture list action failed: %s", status.message); + } +} + void NodePanelBrush::init() { init_template_file("data/dialogs/panel-brushes.xml", "tpl-panel-brushes"); @@ -82,41 +193,9 @@ void NodePanelBrush::init() m_btn_add = find("btn-add"); m_btn_add->on_click = [this](Node*) { App::I->pick_file({ "JPG", "PNG" }, [this](std::string path) { - std::string name, base, ext; - std::regex r(R"((.*)[\\/]([^\\/]+)\.(\w+)$)"); - std::smatch m; - if (!std::regex_search(path, m, r)) - return; - base = m[1].str(); - name = m[2].str(); - ext = m[3].str(); - Image img; - if (!m_dir_name.empty() && img.load_file(path)) - { - std::string path_high = App::I->data_path + "/" + m_dir_name + "/" + name + ".png"; - std::string path_thumb = App::I->data_path + "/" + m_dir_name + "/thumbs/" + name + ".png"; - - //img = img.resize_squared(glm::u8vec4(255)); - if (m_dir_name == "brushes") - img.gayscale_alpha(); - - auto thumb = img.resize(64, 64).resize_squared(glm::u8vec4(255)); - thumb.save_png(path_thumb); - //auto po2 = img.resize_power2(); - img.save_png(path_high); - - NodeButtonBrush* brush = new NodeButtonBrush; - m_container->add_child(brush); - brush->init(); - brush->create(); - brush->loaded(); - brush->set_icon(path_thumb.c_str()); - brush->thumb_path = path_thumb; - brush->high_path = path_high; - brush->brush_name = name; - brush->m_user_brush = true; - brush->on_click = std::bind(&NodePanelBrush::handle_click, this, std::placeholders::_1); - save(); + const auto plan = pp::app::plan_brush_texture_list_add(m_dir_name, App::I->data_path, path); + if (plan) { + execute_texture_list_plan(plan.value()); } }); }; @@ -126,26 +205,13 @@ void NodePanelBrush::init() if (m_current) { int idx = m_container->get_child_index(m_current); - if (m_current->m_user_brush) - { - // only delete user brushes - Asset::delete_file(m_current->thumb_path); - Asset::delete_file(m_current->high_path); + const auto plan = pp::app::plan_brush_texture_list_remove( + static_cast(m_container->m_children.size()), + idx, + m_current->m_user_brush); + if (plan) { + execute_texture_list_plan(plan.value()); } - m_container->remove_child(m_current); - if (m_container->m_children.size() > 0) - { - idx = std::max(0, std::min(idx, (int)m_container->m_children.size() - 1)); - m_current = (NodeButtonBrush*)m_container->m_children[idx].get(); - m_current->m_selected = true; - if (on_brush_changed) - on_brush_changed(this, idx); - } - else - { - m_current = nullptr; - } - save(); } }; @@ -154,9 +220,13 @@ void NodePanelBrush::init() if (m_current) { int idx = m_container->get_child_index(m_current); - idx = std::max(0, std::min(idx - 1, (int)m_container->m_children.size() - 1)); - m_container->move_child(m_current, idx); - save(); + const auto plan = pp::app::plan_brush_texture_list_move( + static_cast(m_container->m_children.size()), + idx, + -1); + if (plan) { + execute_texture_list_plan(plan.value()); + } } }; @@ -165,9 +235,13 @@ void NodePanelBrush::init() if (m_current) { int idx = m_container->get_child_index(m_current); - idx = std::max(0, std::min(idx + 1, (int)m_container->m_children.size() - 1)); - m_container->move_child(m_current, idx); - save(); + const auto plan = pp::app::plan_brush_texture_list_move( + static_cast(m_container->m_children.size()), + idx, + 1); + if (plan) { + execute_texture_list_plan(plan.value()); + } } }; diff --git a/src/node_panel_brush.h b/src/node_panel_brush.h index e6707b4..9d04590 100644 --- a/src/node_panel_brush.h +++ b/src/node_panel_brush.h @@ -9,6 +9,10 @@ #include "serializer.h" #include "node_button.h" +namespace pp::app { +struct BrushTextureListPlan; +} + class NodeButtonBrush : public NodeButtonCustom, public Serializer::Type { public: @@ -38,6 +42,7 @@ class NodePanelBrush : public Node NodeButtonCustom* m_btn_down; NodeButtonCustom* m_btn_remove; bool m_interacted = false; + void execute_texture_list_plan(const pp::app::BrushTextureListPlan& plan); public: NodeScroll* m_container; std::string m_dir_name; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 645d07b..feb4255 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1132,6 +1132,30 @@ if(TARGET pano_cli) LABELS "app;paint;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_brush_texture_list_add_smoke + COMMAND pano_cli plan-brush-texture-list --kind add --dir brushes --data-path data --source C:/Temp/soft.png) + set_tests_properties(pano_cli_plan_brush_texture_list_add_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-texture-list\".*\"operation\":\"add-texture\".*\"source\":\"C:/Temp/soft.png\".*\"path\":\"data/brushes/soft.png\".*\"thumb\":\"data/brushes/thumbs/soft.png\".*\"brushName\":\"soft\".*\"userTexture\":true.*\"convertsBrushAlpha\":true") + + add_test(NAME pano_cli_plan_brush_texture_list_remove_user_smoke + COMMAND pano_cli plan-brush-texture-list --kind remove --item-count 3 --current-index 2 --user-texture) + set_tests_properties(pano_cli_plan_brush_texture_list_remove_user_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-texture-list\".*\"operation\":\"remove-texture\".*\"itemCount\":3.*\"currentIndex\":2.*\"targetIndex\":1.*\"deletesTextureFiles\":true.*\"notifiesSelection\":true") + + add_test(NAME pano_cli_plan_brush_texture_list_move_edge_smoke + COMMAND pano_cli plan-brush-texture-list --kind move --item-count 3 --current-index 0 --offset -1) + set_tests_properties(pano_cli_plan_brush_texture_list_move_edge_smoke PROPERTIES + LABELS "app;paint;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-texture-list\".*\"operation\":\"move-texture\".*\"currentIndex\":0.*\"targetIndex\":0.*\"moveOffset\":-1.*\"noOp\":true") + + add_test(NAME pano_cli_plan_brush_texture_list_rejects_bad_source + COMMAND pano_cli plan-brush-texture-list --kind add --source no-extension) + set_tests_properties(pano_cli_plan_brush_texture_list_rejects_bad_source PROPERTIES + LABELS "app;paint;integration;desktop-fast;fuzz" + WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_canvas_tool_draw_smoke COMMAND pano_cli plan-canvas-tool --kind draw) set_tests_properties(pano_cli_plan_canvas_tool_draw_smoke PROPERTIES diff --git a/tests/app_core/brush_ui_tests.cpp b/tests/app_core/brush_ui_tests.cpp index 6fb9a2d..5e4def5 100644 --- a/tests/app_core/brush_ui_tests.cpp +++ b/tests/app_core/brush_ui_tests.cpp @@ -65,6 +65,76 @@ public: std::string call_order; }; +class FakeBrushTextureListServices final : public pp::app::BrushTextureListServices { +public: + pp::foundation::Status add_texture_from_source( + std::string_view source_path, + std::string_view high_path, + std::string_view thumbnail_path, + std::string_view brush_name, + bool converts_brush_alpha) override + { + if (fail_add) { + call_order += "add-failed;"; + return pp::foundation::Status::invalid_argument("fake add failure"); + } + + adds += 1; + last_source_path = std::string(source_path); + last_high_path = std::string(high_path); + last_thumbnail_path = std::string(thumbnail_path); + last_brush_name = std::string(brush_name); + last_converts_brush_alpha = converts_brush_alpha; + call_order += "add;"; + return pp::foundation::Status::success(); + } + + void remove_texture(int index, bool delete_texture_files) override + { + removes += 1; + last_index = index; + last_deletes_texture_files = delete_texture_files; + call_order += "remove;"; + } + + void move_texture(int from_index, int to_index) override + { + moves += 1; + last_index = from_index; + last_target_index = to_index; + call_order += "move;"; + } + + void select_texture(int index) override + { + selections += 1; + last_target_index = index; + call_order += "select;"; + } + + void save_texture_list() override + { + saves += 1; + call_order += "save;"; + } + + int adds = 0; + int removes = 0; + int moves = 0; + int selections = 0; + int saves = 0; + int last_index = -1; + int last_target_index = -1; + std::string last_source_path; + std::string last_high_path; + std::string last_thumbnail_path; + std::string last_brush_name; + bool last_converts_brush_alpha = false; + bool last_deletes_texture_files = false; + bool fail_add = false; + std::string call_order; +}; + void color_plan_validates_all_channels(pp::tests::Harness& harness) { const auto plan = pp::app::plan_brush_ui_color(0.25F, 0.5F, 0.75F, 1.0F); @@ -132,6 +202,82 @@ void stroke_settings_plan_updates_brush_preview(pp::tests::Harness& harness) PP_EXPECT(harness, !plan.loads_brush_resources); } +void texture_list_add_plans_target_paths_and_rejects_bad_input(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_brush_texture_list_add( + "brushes", + "D:/Paint/data", + "C:/Users/artist/My Brush.JPG"); + PP_EXPECT(harness, plan); + if (plan) { + PP_EXPECT(harness, plan.value().operation == pp::app::BrushTextureListOperation::add_texture); + PP_EXPECT(harness, plan.value().source_path == "C:/Users/artist/My Brush.JPG"); + PP_EXPECT(harness, plan.value().brush_name == "My Brush"); + PP_EXPECT(harness, plan.value().high_path == "D:/Paint/data/brushes/My Brush.png"); + PP_EXPECT(harness, plan.value().thumbnail_path == "D:/Paint/data/brushes/thumbs/My Brush.png"); + PP_EXPECT(harness, plan.value().user_texture); + PP_EXPECT(harness, plan.value().saves_list); + PP_EXPECT(harness, plan.value().converts_brush_alpha); + } + + const auto pattern = pp::app::plan_brush_texture_list_add( + "patterns", + "D:/Paint/data", + R"(C:\Textures\noise.png)"); + PP_EXPECT(harness, pattern); + if (pattern) { + PP_EXPECT(harness, !pattern.value().converts_brush_alpha); + PP_EXPECT(harness, pattern.value().brush_name == "noise"); + } + + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_add("", "D:/Paint/data", "a.png")); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_add("brushes", "", "a.png")); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_add("brushes", "D:/Paint/data", "no-extension")); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_add("brushes", "D:/Paint/data", "C:/dir/")); +} + +void texture_list_remove_and_move_plans_handle_edges(pp::tests::Harness& harness) +{ + const auto remove_middle = pp::app::plan_brush_texture_list_remove(3, 1, true); + PP_EXPECT(harness, remove_middle); + if (remove_middle) { + PP_EXPECT(harness, remove_middle.value().operation == pp::app::BrushTextureListOperation::remove_texture); + PP_EXPECT(harness, remove_middle.value().current_index == 1); + PP_EXPECT(harness, remove_middle.value().target_index == 1); + PP_EXPECT(harness, remove_middle.value().deletes_texture_files); + PP_EXPECT(harness, remove_middle.value().notifies_selection); + PP_EXPECT(harness, remove_middle.value().saves_list); + } + + const auto remove_last = pp::app::plan_brush_texture_list_remove(1, 0, false); + PP_EXPECT(harness, remove_last); + if (remove_last) { + PP_EXPECT(harness, remove_last.value().target_index == -1); + PP_EXPECT(harness, !remove_last.value().deletes_texture_files); + PP_EXPECT(harness, !remove_last.value().notifies_selection); + } + + const auto move_up_edge = pp::app::plan_brush_texture_list_move(3, 0, -1); + PP_EXPECT(harness, move_up_edge); + if (move_up_edge) { + PP_EXPECT(harness, move_up_edge.value().operation == pp::app::BrushTextureListOperation::move_texture); + PP_EXPECT(harness, move_up_edge.value().target_index == 0); + PP_EXPECT(harness, move_up_edge.value().no_op); + } + + const auto move_down = pp::app::plan_brush_texture_list_move(3, 1, 1); + PP_EXPECT(harness, move_down); + if (move_down) { + PP_EXPECT(harness, move_down.value().target_index == 2); + PP_EXPECT(harness, !move_down.value().no_op); + } + + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_remove(0, 0, true)); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_remove(2, 2, true)); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_move(2, 0, 0)); + PP_EXPECT(harness, !pp::app::plan_brush_texture_list_move(2, -1, 1)); +} + void executor_dispatches_color_and_refresh(pp::tests::Harness& harness) { FakeBrushUiServices services; @@ -197,6 +343,51 @@ void executor_dispatches_stroke_refresh_only(pp::tests::Harness& harness) PP_EXPECT(harness, services.call_order == "refresh;"); } +void texture_list_executor_dispatches_and_preserves_failure(pp::tests::Harness& harness) +{ + FakeBrushTextureListServices services; + + const auto add = pp::app::plan_brush_texture_list_add("brushes", "D:/Paint/data", "C:/Temp/soft.png"); + PP_EXPECT(harness, add); + if (add) { + PP_EXPECT(harness, pp::app::execute_brush_texture_list_plan(add.value(), services).ok()); + } + + const auto remove = pp::app::plan_brush_texture_list_remove(3, 2, true); + PP_EXPECT(harness, remove); + if (remove) { + PP_EXPECT(harness, pp::app::execute_brush_texture_list_plan(remove.value(), services).ok()); + } + + const auto move = pp::app::plan_brush_texture_list_move(3, 1, -1); + PP_EXPECT(harness, move); + if (move) { + PP_EXPECT(harness, pp::app::execute_brush_texture_list_plan(move.value(), services).ok()); + } + + PP_EXPECT(harness, services.adds == 1); + PP_EXPECT(harness, services.last_source_path == "C:/Temp/soft.png"); + PP_EXPECT(harness, services.last_high_path == "D:/Paint/data/brushes/soft.png"); + PP_EXPECT(harness, services.last_thumbnail_path == "D:/Paint/data/brushes/thumbs/soft.png"); + PP_EXPECT(harness, services.last_brush_name == "soft"); + PP_EXPECT(harness, services.last_converts_brush_alpha); + PP_EXPECT(harness, services.removes == 1); + PP_EXPECT(harness, services.moves == 1); + PP_EXPECT(harness, services.selections == 1); + PP_EXPECT(harness, services.saves == 3); + PP_EXPECT(harness, services.last_deletes_texture_files); + PP_EXPECT(harness, services.call_order == "add;save;remove;select;save;move;save;"); + + FakeBrushTextureListServices failing_services; + failing_services.fail_add = true; + PP_EXPECT(harness, add); + if (add) { + PP_EXPECT(harness, !pp::app::execute_brush_texture_list_plan(add.value(), failing_services).ok()); + } + PP_EXPECT(harness, failing_services.saves == 0); + PP_EXPECT(harness, failing_services.call_order == "add-failed;"); +} + void executor_rejects_invalid_plan_payloads(pp::tests::Harness& harness) { FakeBrushUiServices services; @@ -218,6 +409,20 @@ void executor_rejects_invalid_plan_payloads(pp::tests::Harness& harness) PP_EXPECT(harness, services.color_sets == 0); PP_EXPECT(harness, services.texture_sets == 0); PP_EXPECT(harness, services.refreshes == 0); + + FakeBrushTextureListServices list_services; + pp::app::BrushTextureListPlan add; + add.operation = pp::app::BrushTextureListOperation::add_texture; + add.source_path = "source.png"; + PP_EXPECT(harness, !pp::app::execute_brush_texture_list_plan(add, list_services).ok()); + + pp::app::BrushTextureListPlan move; + move.operation = pp::app::BrushTextureListOperation::move_texture; + move.item_count = 1; + move.current_index = 0; + move.target_index = 1; + PP_EXPECT(harness, !pp::app::execute_brush_texture_list_plan(move, list_services).ok()); + PP_EXPECT(harness, list_services.call_order.empty()); } } // namespace @@ -229,9 +434,12 @@ int main() harness.run("texture plan validates path and slot", texture_plan_validates_path_and_slot); harness.run("preset plan preserves color and requires brush", preset_plan_preserves_color_and_requires_brush); harness.run("stroke settings plan updates brush preview", stroke_settings_plan_updates_brush_preview); + 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("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); + harness.run("texture list executor dispatches and preserves failure", texture_list_executor_dispatches_and_preserves_failure); harness.run("executor rejects invalid plan payloads", executor_rejects_invalid_plan_payloads); return harness.finish(); } diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index fbd84f3..608bb3d 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -316,6 +316,17 @@ struct PlanBrushOperationArgs { bool has_brush = true; }; +struct PlanBrushTextureListArgs { + std::string kind = "add"; + std::string directory_name = "brushes"; + std::string data_path = "data"; + std::string source_path = "source.png"; + int item_count = 1; + int current_index = 0; + int offset = 1; + bool current_is_user_texture = false; +}; + struct PlanGridOperationArgs { std::string kind = "pick"; std::string path; @@ -1087,6 +1098,20 @@ const char* brush_ui_operation_name(pp::app::BrushUiOperation operation) noexcep return "stroke-settings-changed"; } +const char* brush_texture_list_operation_name(pp::app::BrushTextureListOperation operation) noexcept +{ + switch (operation) { + case pp::app::BrushTextureListOperation::add_texture: + return "add-texture"; + case pp::app::BrushTextureListOperation::remove_texture: + return "remove-texture"; + case pp::app::BrushTextureListOperation::move_texture: + return "move-texture"; + } + + return "add-texture"; +} + const char* canvas_tool_operation_name(pp::app::CanvasToolOperation operation) noexcept { switch (operation) { @@ -1613,6 +1638,7 @@ void print_help() << " plan-animation-operation --kind add|duplicate|remove|duration|move|select|goto|next|prev|playback|toggle-playback|onion [--frame-count N] [--total-duration N] [--current-frame N] [--selected-frame N] [--layer-index N] [--layer-id N] [--current-duration N] [--delta N] [--offset N] [--onion-size N] [--playing]\n" << " 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-canvas-tool --kind draw|erase|line|camera|grid|copy|cut|fill|mask-free|mask-line|bucket|pick|touch-lock [--current-mode-draw]\n" << " plan-canvas-tool-state [--mode draw|erase|line|camera|grid|copy|cut|fill|mask-free|mask-line|bucket] [--picking] [--touch-lock]\n" << " plan-grid-operation --kind pick|load|reload|clear|render|commit [--path FILE] [--no-heightmap] [--no-canvas] [--float32] [--float16] [--texture-resolution N] [--samples N]\n" @@ -4283,6 +4309,116 @@ int plan_brush_operation(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_brush_texture_list_args( + int argc, + char** argv, + PlanBrushTextureListArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--kind" || key == "--dir" || key == "--data-path" || key == "--source") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + if (key == "--kind") { + args.kind = argv[++i]; + } else if (key == "--dir") { + args.directory_name = argv[++i]; + } else if (key == "--data-path") { + args.data_path = argv[++i]; + } else { + args.source_path = 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 == "--user-texture") { + args.current_is_user_texture = true; + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +pp::foundation::Result make_brush_texture_list_plan( + const PlanBrushTextureListArgs& args) +{ + if (args.kind == "add") { + return pp::app::plan_brush_texture_list_add(args.directory_name, args.data_path, args.source_path); + } + if (args.kind == "remove") { + return pp::app::plan_brush_texture_list_remove( + args.item_count, + args.current_index, + args.current_is_user_texture); + } + 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_texture_list_move(args.item_count, args.current_index, offset); + } + + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("unknown brush texture list operation kind")); +} + +int plan_brush_texture_list(int argc, char** argv) +{ + PlanBrushTextureListArgs args; + const auto status = parse_plan_brush_texture_list_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-brush-texture-list", status.message); + return 2; + } + + const auto plan = make_brush_texture_list_plan(args); + if (!plan) { + print_error("plan-brush-texture-list", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-brush-texture-list\"" + << ",\"state\":{\"kind\":\"" << json_escape(args.kind) + << "\",\"dir\":\"" << json_escape(args.directory_name) + << "\",\"dataPath\":\"" << json_escape(args.data_path) + << "\",\"source\":\"" << json_escape(args.source_path) + << "\",\"itemCount\":" << args.item_count + << ",\"currentIndex\":" << args.current_index + << ",\"offset\":" << args.offset + << ",\"currentIsUserTexture\":" << json_bool(args.current_is_user_texture) + << "},\"plan\":{\"operation\":\"" << brush_texture_list_operation_name(value.operation) + << "\",\"itemCount\":" << value.item_count + << ",\"currentIndex\":" << value.current_index + << ",\"targetIndex\":" << value.target_index + << ",\"moveOffset\":" << value.move_offset + << ",\"source\":\"" << json_escape(value.source_path) + << "\",\"path\":\"" << json_escape(value.high_path) + << "\",\"thumb\":\"" << json_escape(value.thumbnail_path) + << "\",\"brushName\":\"" << json_escape(value.brush_name) + << "\",\"userTexture\":" << json_bool(value.user_texture) + << ",\"deletesTextureFiles\":" << json_bool(value.deletes_texture_files) + << ",\"savesList\":" << json_bool(value.saves_list) + << ",\"notifiesSelection\":" << json_bool(value.notifies_selection) + << ",\"convertsBrushAlpha\":" << json_bool(value.converts_brush_alpha) + << ",\"noOp\":" << json_bool(value.no_op) + << "}}\n"; + return 0; +} + pp::foundation::Status parse_plan_canvas_tool_args( int argc, char** argv, @@ -7405,6 +7541,10 @@ int main(int argc, char** argv) return plan_brush_operation(argc, argv); } + if (command == "plan-brush-texture-list") { + return plan_brush_texture_list(argc, argv); + } + if (command == "plan-canvas-tool") { return plan_canvas_tool(argc, argv); }