From 6ab64ccc821bcc7257ac25f4fb3e349cd2aeb072 Mon Sep 17 00:00:00 2001 From: omigamedev Date: Thu, 4 Jun 2026 14:49:22 +0200 Subject: [PATCH] Centralize legacy brush package import --- CMakeLists.txt | 1 + cmake/PanoPainterSources.cmake | 2 + docs/modernization/build-inventory.md | 13 ++- docs/modernization/debt.md | 3 +- docs/modernization/roadmap.md | 15 ++- src/app_core/brush_package_import.h | 56 +++++++++++ src/legacy_brush_package_import_services.cpp | 48 ++++++++++ src/legacy_brush_package_import_services.h | 17 ++++ src/legacy_document_open_services.cpp | 16 +++- tests/CMakeLists.txt | 43 +++++++++ tests/app_core/brush_package_import_tests.cpp | 73 ++++++++++++++ ...li_plan_brush_package_import_failure.cmake | 39 ++++++++ tools/pano_cli/main.cpp | 95 +++++++++++++++++++ 13 files changed, 415 insertions(+), 6 deletions(-) create mode 100644 src/app_core/brush_package_import.h create mode 100644 src/legacy_brush_package_import_services.cpp create mode 100644 src/legacy_brush_package_import_services.h create mode 100644 tests/app_core/brush_package_import_tests.cpp create mode 100644 tests/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 43399ed..fb48841 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -227,6 +227,7 @@ add_library(pp_app_core STATIC src/app_core/app_preferences.h src/app_core/app_status.h src/app_core/app_startup.h + src/app_core/brush_package_import.h src/app_core/brush_package_export.h src/app_core/brush_ui.h src/app_core/canvas_hotkey.h diff --git a/cmake/PanoPainterSources.cmake b/cmake/PanoPainterSources.cmake index f09c4d0..e81c4e2 100644 --- a/cmake/PanoPainterSources.cmake +++ b/cmake/PanoPainterSources.cmake @@ -86,6 +86,8 @@ set(PP_PANOPAINTER_APP_SOURCES src/legacy_app_preference_services.h src/legacy_app_startup_services.cpp src/legacy_app_startup_services.h + src/legacy_brush_package_import_services.cpp + src/legacy_brush_package_import_services.h src/legacy_brush_package_export_services.cpp src/legacy_brush_package_export_services.h src/legacy_cloud_services.cpp diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 9367d10..605d02d 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -617,8 +617,17 @@ Known local toolchain state: - `src/legacy_document_open_services.*` is the current app-shell bridge between `pp_app_core` document-open plans and live ABR/PPBR import prompts, unsaved-project discard prompts, project opening, layer UI refresh, title - updates, and action-history clearing; remaining legacy execution ownership is - tracked by `DEBT-0039`. + updates, and action-history clearing. Accepted brush import prompts now + delegate import execution to `src/legacy_brush_package_import_services.*`; + remaining legacy document-open ownership is tracked by `DEBT-0039`. +- `src/legacy_brush_package_import_services.*` is the current app-shell bridge + between `pp_app_core` ABR/PPBR brush package import requests and live + `NodePanelBrushPreset::import_abr`/`import_ppbr` execution. It preserves the + detached legacy import worker threads and preset-panel storage ownership while + brush asset/paint/UI ownership is tracked by `DEBT-0048`. +- `pp_app_core_brush_package_import_tests` covers ABR and PPBR import kind + naming, path validation, service dispatch, and empty-path rejection without + requiring a window, brush preset panel, or filesystem read. - `src/legacy_document_session_services.*` is the current app-shell bridge between `pp_app_core` document-session decisions and live close prompts, save dialogs, save-version routing, existing-project saves, and diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 155e7cc..6c1345b 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -56,7 +56,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0036 | Open | Modernization | `pp_renderer_api`, `pp_paint_renderer`, `pano_cli plan-paint-feedback`, and `pano_cli plan-stroke-composite` can choose backend-neutral complex paint feedback strategies for fixed-function blending, framebuffer-fetch-capable renderers, or ping-pong render targets. OpenGL extension detection now stores `pp::renderer::RenderDeviceFeatures` through `ShaderManager`, using `pp_renderer_gl::render_device_features` as the backend conversion point. `pp_paint_renderer::plan_canvas_blend_gate` owns the compatibility mapping from persisted layer/brush blend indices to the extracted stroke-composite planner, and live `Canvas::draw_merge` plus `NodeCanvas` panorama rendering both call it with the stored renderer-neutral feature set for their existing shader-blend gates and destination-copy versus framebuffer-fetch decisions. `pp_paint_renderer::plan_canvas_stroke_feedback` also owns the current destination-feedback decision, and live `Canvas::stroke_draw`, thumbnail layer blending, and `NodeStrokePreview` brush-preview rendering use it for framebuffer-fetch versus destination-copy decisions. Actual live stroke rasterization, dual-brush compositing, pattern feedback math, thumbnail layer compositing, and brush-preview compositing still use legacy OpenGL canvas/UI execution | Preserve current painting behavior while the renderer boundary matures for OpenGL parity and later Vulkan/Metal experiments | `pp_renderer_api_tests`; `pp_renderer_gl_capabilities_tests`; `pp_paint_renderer_compositor_tests`; `pano_cli plan-paint-feedback --framebuffer-fetch --explicit-transitions --render-only`; `pano_cli plan-paint-feedback --texture-copy`; `pano_cli plan-stroke-composite --stroke-blend 10 --framebuffer-fetch --explicit-transitions --render-only`; `pano_cli plan-stroke-composite --layer-blend 4 --dual-blend --texture-copy`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Live stroke/layer compositing chooses its feedback path through `pp_paint_renderer` and renderer services, with OpenGL golden parity and Vulkan/Metal lab tests covering framebuffer-fetch and ping-pong behavior | | DEBT-0037 | Open | Modernization | Recording lifecycle/export planning and execution dispatch now consume pure `pp_app_core` through `App::rec_start`, `App::rec_stop`, `App::rec_clear`, `App::rec_export`, `pano_cli plan-recording-session`, and the `RecordingServices` boundary; live execution is centralized in `src/legacy_recording_services.*`, but the bridge still owns legacy recording thread startup/shutdown, platform recorded-file cleanup, progress UI, PBO readback through `App::rec_loop`, and `MP4Encoder::write_mp4` execution | Preserve current timelapse/MP4 behavior while recording moves toward app/document/renderer/video services | `pp_app_core_document_recording_tests`; `pano_cli plan-recording-session --running --frame-count 12`; `pano_cli plan-recording-session --platform-clears-files`; `ctest --preset desktop-fast --build-config Debug` | Recording thread lifecycle, frame readback, platform cleanup, progress reporting, and MP4 writing are owned by injected app/renderer/video services with `App` methods acting only as adapters | | DEBT-0038 | Open | Modernization | Cloud upload/browse/bulk planning and execution dispatch now consume pure `pp_app_core` through `App::cloud_upload`, `App::cloud_upload_all`, `App::cloud_browse`, `pano_cli plan-cloud-upload`, `pano_cli plan-cloud-upload-all`, `pano_cli plan-cloud-browse`, and the `CloudServices` boundary; live execution is centralized in `src/legacy_cloud_services.*`, but the bridge still uses legacy save-before-upload, `upload`/`download` network helpers, progress/message UI, OpenGL context guarding, `NodeDialogCloud`, `Canvas` project open, layer refresh, and `ActionManager` reset | Preserve current cloud behavior while cloud/network/document import flows move toward app/document/platform services | `pp_app_core_document_cloud_tests`; `pano_cli plan-cloud-upload --new-document --unsaved`; `pano_cli plan-cloud-browse --selected-file demo.ppi`; `pano_cli plan-cloud-upload-all --file-count 3`; `ctest --preset desktop-fast --build-config Debug` | Cloud upload/download, save-before-upload, progress reporting, cloud browse dialog, downloaded project opening, layer refresh, OpenGL context ownership, and action-history reset are owned by injected app/document/network/platform/renderer services with `App` methods acting only as adapters | -| DEBT-0039 | Open | Modernization | Document-open planning and execution dispatch now consume pure `pp_app_core` through `App::open_document`, `pano_cli plan-open-route`, `DocumentOpenServices`, and `src/legacy_document_open_services.*`, but the bridge still opens ABR/PPBR import prompts, launches legacy brush preset import threads, applies unsaved-project discard prompts, calls legacy project-open execution, refreshes layer UI, updates the app title, and clears legacy history directly | Preserve current file-open/import behavior while document loading and brush import move toward app/document/asset/UI services | `pp_app_core_document_route_tests`; `pp_app_core_document_session_tests`; `pano_cli plan-open-route --path D:/Paint/Scenes/demo.ppi --unsaved`; `pano_cli plan-open-route --path D:/Paint/Brushes/clouds.ABR --unsaved`; `ctest --preset desktop-fast --build-config Debug` | Brush import prompting/execution, project-open execution, unsaved-project discard prompting, layer refresh, title updates, and history clearing are owned by injected app/document/asset/UI services with `App::open_document` acting only as an adapter | +| DEBT-0039 | Open | Modernization | Document-open planning and execution dispatch now consume pure `pp_app_core` through `App::open_document`, `pano_cli plan-open-route`, `DocumentOpenServices`, and `src/legacy_document_open_services.*`, but the bridge still opens ABR/PPBR import prompts before delegating import execution to `src/legacy_brush_package_import_services.*`, applies unsaved-project discard prompts, calls legacy project-open execution, refreshes layer UI, updates the app title, and clears legacy history directly | Preserve current file-open/import behavior while document loading and brush import move toward app/document/asset/UI services | `pp_app_core_document_route_tests`; `pp_app_core_document_session_tests`; `pano_cli plan-open-route --path D:/Paint/Scenes/demo.ppi --unsaved`; `pano_cli plan-open-route --path D:/Paint/Brushes/clouds.ABR --unsaved`; `ctest --preset desktop-fast --build-config Debug` | Brush import prompting, project-open execution, unsaved-project discard prompting, layer refresh, title updates, and history clearing are owned by injected app/document/asset/UI services with `App::open_document` acting only as an adapter | | DEBT-0040 | Open | Modernization | Close request, document save, and save-before-workflow planning/execution dispatch now consume pure `pp_app_core` through `App::request_close`, `App::save_document`, `App::continue_document_workflow_after_optional_save`, `pano_cli simulate-app-session`, `DocumentSaveServices`, `CloseRequestServices`, `DocumentWorkflowServices`, and `src/legacy_document_session_services.*`, but the bridge still opens legacy message boxes/save dialogs, calls `Canvas::I->project_save`, mutates the unsaved flag on close confirmation, invokes native app close, and routes save-version through the retained legacy dialog | Preserve current close/save/dirty-workflow behavior while document session execution moves toward app/document/UI/platform services | `pp_app_core_document_session_tests`; `pano_cli simulate-app-session --unsaved --save-intent save-dirty-version`; `pano_cli simulate-app-session --no-canvas`; `pano_cli plan-document-file --work-dir D:/Paint --name demo --target-exists`; `pano_cli plan-document-version --directory D:/Paint --doc-name demo.01 --existing-path D:/Paint/demo.02.ppi`; `ctest --preset desktop-fast --build-config Debug` | Close prompt execution, native close requests, dirty-workflow save prompts, existing-project saves, save dialogs, save-version execution, and unsaved-flag mutation are owned by injected app/document/UI/platform services with `App` methods acting only as adapters | | DEBT-0041 | Open | Modernization | Accepted new-document planning/execution dispatch now consumes pure `pp_app_core` through `App::dialog_newdoc`, `pano_cli plan-new-document`, `NewDocumentServices`, and `src/legacy_document_session_services.*`, but the bridge still mutates legacy app document fields, clears legacy layer UI, resizes legacy `Canvas`, clears legacy history, creates the default layer through legacy UI, mutates unsaved/new-document flags, updates the title, and handles keyboard/dialog cleanup directly | Preserve current New Document dialog behavior while document creation moves toward app/document/UI services | `pp_app_core_document_session_tests`; `pano_cli plan-new-document --work-dir D:/Paint --name demo --resolution-index 3`; `pano_cli plan-new-document --work-dir D:/Paint --name demo --resolution-index 3 --target-exists`; `pano_cli simulate-app-session --save-intent save`; `ctest --preset desktop-fast --build-config Debug` | New document creation, overwrite confirmation, canvas/document allocation, default layer creation, history clearing, title updates, dirty/new-document state, and keyboard/dialog cleanup are owned by injected app/document/UI services with `App::dialog_newdoc` acting only as a UI adapter | | DEBT-0042 | Open | Modernization | Accepted Save As and Save Version planning/execution dispatch now consumes pure `pp_app_core` through `App::dialog_save`, `App::dialog_save_ver`, `pano_cli plan-document-file`, `pano_cli plan-document-version`, `DocumentFileSaveServices`, `DocumentVersionSaveServices`, and `src/legacy_document_session_services.*`, but the bridge still opens legacy overwrite prompts, calls legacy `Canvas::project_save`, mutates app document name/path/directory fields, marks version saves dirty before saving, updates the title, and handles keyboard/dialog cleanup directly | Preserve current Save As and Save Version behavior while document persistence moves toward app/document/storage/UI services | `pp_app_core_document_session_tests`; `pano_cli plan-document-file --work-dir D:/Paint --name demo --target-exists`; `pano_cli plan-document-version --directory D:/Paint --doc-name demo.01 --existing-path D:/Paint/demo.02.ppi`; `pano_cli simulate-app-session --save-intent save-as`; `pano_cli simulate-app-session --save-intent save-version`; `ctest --preset desktop-fast --build-config Debug` | Save As overwrite prompting, project-save execution, app document metadata updates, title updates, version-save dirty-state handling, and keyboard/dialog cleanup are owned by injected app/document/storage/UI services with `App::dialog_save` and `App::dialog_save_ver` acting only as UI adapters | @@ -65,6 +65,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0045 | Open | Modernization | Options-menu preference execution now consumes pure `pp_app_core` through UI scale, viewport scale, RTL direction, VR mode, VR-controller, auto-timelapse, and canvas cursor-mode callbacks plus `AppPreferenceServices` and `src/legacy_app_preference_services.*`, but the bridge still calls legacy `App::set_ui_scale`, `App::set_ui_rtl`, `App::vr_start`, `App::vr_stop`, `NodeCanvas::set_density`, `NodeCanvas::set_cursor_visibility`, `App::rec_start`, `App::rec_stop`, and `Settings::save` directly | Preserve current options-menu behavior while preferences move toward app/UI/platform/storage services | `pp_app_core_app_preferences_tests`; `pano_cli plan-app-preferences --ui-scale 1.5 --display-density 2 --current-scale 1.6 --scale-option 1 --scale-option 1.5 --rtl`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Preference persistence, UI/layout direction, viewport density, cursor mode, VR mode start/stop/failure handling, VR-controller state, and auto-timelapse recording side effects are owned by injected app/UI/platform/storage services with options-menu callbacks acting only as UI adapters | | DEBT-0046 | Open | Modernization | Startup preference/runtime execution now consumes pure `pp_app_core` through `App::init`, `pano_cli plan-app-startup`, `AppStartupServices`, and `src/legacy_app_startup_services.*`, but the bridge still calls legacy `Settings::set`, `Settings::save`, `App::rec_start`, app VR-controller state mutation, and message-box license warning execution directly | Preserve current startup behavior while app startup moves toward app/preferences/storage/recording/UI services | `pp_app_core_app_startup_tests`; `pano_cli plan-app-startup --run-counter 7 --vr-controllers-disabled --license-invalid`; `pano_cli plan-app-startup --run-counter -1`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Startup preference persistence, auto-timelapse startup, stored VR-controller state, license validation/warning, and startup UI/runtime side effects are owned by injected app/preferences/storage/recording/UI services with `App::init` acting only as orchestration | | DEBT-0047 | Open | Modernization | PPBR brush package export request validation and execution dispatch now consume pure `pp_app_core` through `App::dialog_ppbr_export`, `pano_cli plan-brush-package-export`, `BrushPackageExportServices`, and `src/legacy_brush_package_export_services.*`, but the bridge still reads `NodeDialogExportPPBR`, carries the legacy `Image` header object outside the pure request, converts to `NodePanelBrushPreset::PPBRInfo`, calls `NodePanelBrushPreset::export_ppbr`, owns desktop worker-thread dispatch, dialog destruction, mobile/Web completion, and success-message behavior directly | Preserve current PPBR export behavior while brush assets, PPBR serialization, picker completion, and UI lifetime move toward asset/storage/UI/platform services | `pp_app_core_brush_package_export_tests`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --author Artist --dest-path D:/Paint/BrushPreviews --export-data --header-image`; `pano_cli plan-brush-package-export`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr --dest-path D:/Paint/BrushPreviews --no-export-data`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | PPBR metadata collection, header-image ownership, serialization, picker-selected path execution, desktop threading, dialog lifetime, and success UI are owned by injected brush asset/storage/UI/platform services with `App::dialog_ppbr_export` acting only as a UI adapter | +| DEBT-0048 | Open | Modernization | ABR/PPBR brush package import execution now consumes pure `pp_app_core` through document-open confirmation callbacks, `pano_cli plan-brush-package-import`, `BrushPackageImportServices`, and `src/legacy_brush_package_import_services.*`, but the bridge still launches detached legacy `NodePanelBrushPreset::import_abr`/`import_ppbr` worker threads and depends on the legacy preset panel as the importer/storage owner | Preserve current brush import behavior while brush package parsing, preset storage, progress/error reporting, and UI refresh move toward asset/paint/UI services | `pp_app_core_brush_package_import_tests`; `pano_cli plan-brush-package-import --kind ppbr --path D:/Paint/Brushes/clouds.ppbr`; `pano_cli plan-brush-package-import --kind abr --path D:/Paint/Brushes/clouds.abr`; `pano_cli plan-brush-package-import --kind ppbr`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | ABR/PPBR parsing, preset creation/storage, import threading/progress, duplicate asset policy, and UI refresh are owned by injected brush asset/paint/UI services with document-open callbacks only confirming user intent | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index fe5ead8..0969980 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -746,7 +746,11 @@ behind the renderer boundary. `src/legacy_document_open_services.*`, preserving ABR/PPBR import prompts, unsaved-project discard prompts, project open, layer refresh, title updates, and history clearing while those live effects remain tracked under -`DEBT-0039`. +`DEBT-0039`. Accepted ABR/PPBR import prompts now delegate import execution to +the app-core brush package import executor and +`src/legacy_brush_package_import_services.*`, preserving detached legacy preset +panel import threads while retained brush asset execution remains tracked under +`DEBT-0048`. `App::request_close`, `App::save_document`, and `App::continue_document_workflow_after_optional_save` now route through app-core document-session executors and `src/legacy_document_session_services.*`, @@ -1383,6 +1387,15 @@ Results: `pano_cli_plan_brush_package_export_smoke`, `pano_cli_plan_brush_package_export_rejects_empty_path`, and `pano_cli_plan_brush_package_export_dest_without_data_smoke`. +- `PanoPainter`, `pp_app_core_brush_package_import_tests`, and `pano_cli` built + after ABR/PPBR brush package import execution moved behind app-core brush + import services. +- Focused brush import CTest coverage passed for + `pp_app_core_brush_package_import_tests`, + `pano_cli_plan_brush_package_import_ppbr_smoke`, + `pano_cli_plan_brush_package_import_abr_smoke`, + `pano_cli_plan_brush_package_import_rejects_empty_path`, and + `pano_cli_plan_brush_package_import_rejects_unknown_kind`. - `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_package_import.h b/src/app_core/brush_package_import.h new file mode 100644 index 0000000..ef398be --- /dev/null +++ b/src/app_core/brush_package_import.h @@ -0,0 +1,56 @@ +#pragma once + +#include "foundation/result.h" + +#include + +namespace pp::app { + +enum class BrushPackageImportKind { + abr, + ppbr, +}; + +class BrushPackageImportServices { +public: + virtual ~BrushPackageImportServices() = default; + + virtual void import_brush_package(BrushPackageImportKind kind, std::string_view path) = 0; +}; + +[[nodiscard]] inline const char* brush_package_import_kind_name(BrushPackageImportKind kind) noexcept +{ + switch (kind) { + case BrushPackageImportKind::abr: + return "abr"; + case BrushPackageImportKind::ppbr: + return "ppbr"; + } + + return "abr"; +} + +[[nodiscard]] inline pp::foundation::Status validate_brush_package_import_path(std::string_view path) noexcept +{ + if (path.empty()) { + return pp::foundation::Status::invalid_argument("brush package import path must not be empty"); + } + + return pp::foundation::Status::success(); +} + +[[nodiscard]] inline pp::foundation::Status execute_brush_package_import( + BrushPackageImportKind kind, + std::string_view path, + BrushPackageImportServices& services) +{ + const auto status = validate_brush_package_import_path(path); + if (!status.ok()) { + return status; + } + + services.import_brush_package(kind, path); + return pp::foundation::Status::success(); +} + +} // namespace pp::app diff --git a/src/legacy_brush_package_import_services.cpp b/src/legacy_brush_package_import_services.cpp new file mode 100644 index 0000000..d35d882 --- /dev/null +++ b/src/legacy_brush_package_import_services.cpp @@ -0,0 +1,48 @@ +#include "pch.h" + +#include "legacy_brush_package_import_services.h" + +#include "app.h" +#include "node_panel_brush.h" + +#include +#include + +namespace pp::panopainter { +namespace { + +class LegacyBrushPackageImportServices final : public pp::app::BrushPackageImportServices { +public: + explicit LegacyBrushPackageImportServices(App& app) noexcept + : app_(app) + { + } + + void import_brush_package(pp::app::BrushPackageImportKind kind, std::string_view path) override + { + auto presets = app_.presets; + const auto path_string = std::string(path); + if (kind == pp::app::BrushPackageImportKind::abr) { + std::thread(&NodePanelBrushPreset::import_abr, presets, path_string).detach(); + return; + } + + std::thread(&NodePanelBrushPreset::import_ppbr, presets, path_string).detach(); + } + +private: + App& app_; +}; + +} // namespace + +pp::foundation::Status execute_legacy_brush_package_import( + App& app, + pp::app::BrushPackageImportKind kind, + std::string_view path) +{ + LegacyBrushPackageImportServices services(app); + return pp::app::execute_brush_package_import(kind, path, services); +} + +} // namespace pp::panopainter diff --git a/src/legacy_brush_package_import_services.h b/src/legacy_brush_package_import_services.h new file mode 100644 index 0000000..5096558 --- /dev/null +++ b/src/legacy_brush_package_import_services.h @@ -0,0 +1,17 @@ +#pragma once + +#include "app_core/brush_package_import.h" +#include "foundation/result.h" + +#include + +class App; + +namespace pp::panopainter { + +[[nodiscard]] pp::foundation::Status execute_legacy_brush_package_import( + App& app, + pp::app::BrushPackageImportKind kind, + std::string_view path); + +} // namespace pp::panopainter diff --git a/src/legacy_document_open_services.cpp b/src/legacy_document_open_services.cpp index ad2da4a..bc60b2e 100644 --- a/src/legacy_document_open_services.cpp +++ b/src/legacy_document_open_services.cpp @@ -3,7 +3,9 @@ #include "legacy_document_open_services.h" #include "app.h" +#include "legacy_brush_package_import_services.h" #include "legacy_history_services.h" +#include "log.h" #include "node_panel_brush.h" #include "node_panel_layer.h" @@ -50,7 +52,12 @@ public: auto* app = &app_; auto mb = app_.message_box("Import ABR", "Would you like to import the brushes?", true); mb->on_submit = [app, path = route.path](Node* target) { - std::thread(&NodePanelBrushPreset::import_abr, app->presets, path).detach(); + const auto status = pp::panopainter::execute_legacy_brush_package_import( + *app, + pp::app::BrushPackageImportKind::abr, + path); + if (!status.ok()) + LOG("ABR import failed: %s", status.message); target->destroy(); }; } @@ -60,7 +67,12 @@ public: auto* app = &app_; auto mb = app_.message_box("Import PPBR", "Would you like to import the brushes?", true); mb->on_submit = [app, path = route.path](Node* target) { - std::thread(&NodePanelBrushPreset::import_ppbr, app->presets, path).detach(); + const auto status = pp::panopainter::execute_legacy_brush_package_import( + *app, + pp::app::BrushPackageImportKind::ppbr, + path); + if (!status.ok()) + LOG("PPBR import failed: %s", status.message); target->destroy(); }; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9127d83..0484988 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -298,6 +298,16 @@ add_test(NAME pp_app_core_brush_package_export_tests COMMAND pp_app_core_brush_p set_tests_properties(pp_app_core_brush_package_export_tests PROPERTIES LABELS "app;paint;assets;desktop-fast;fuzz") +add_executable(pp_app_core_brush_package_import_tests + app_core/brush_package_import_tests.cpp) +target_link_libraries(pp_app_core_brush_package_import_tests PRIVATE + pp_app_core + pp_test_harness) + +add_test(NAME pp_app_core_brush_package_import_tests COMMAND pp_app_core_brush_package_import_tests) +set_tests_properties(pp_app_core_brush_package_import_tests PROPERTIES + LABELS "app;paint;assets;desktop-fast;fuzz") + add_executable(pp_app_core_canvas_tool_ui_tests app_core/canvas_tool_ui_tests.cpp) target_link_libraries(pp_app_core_canvas_tool_ui_tests PRIVATE @@ -887,6 +897,39 @@ if(TARGET pano_cli) WILL_FAIL TRUE PASS_REGULAR_EXPRESSION "\"command\":\"plan-app-startup\".*\"message\":\"run counter must not be negative\"") + add_test(NAME pano_cli_plan_brush_package_import_ppbr_smoke + COMMAND pano_cli plan-brush-package-import + --kind ppbr + --path D:/Paint/Brushes/clouds.ppbr) + set_tests_properties(pano_cli_plan_brush_package_import_ppbr_smoke PROPERTIES + LABELS "app;paint;assets;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-import\".*\"kind\":\"ppbr\".*\"path\":\"D:/Paint/Brushes/clouds.ppbr\".*\"dispatches\":1") + + add_test(NAME pano_cli_plan_brush_package_import_abr_smoke + COMMAND pano_cli plan-brush-package-import + --kind abr + --path D:/Paint/Brushes/clouds.abr) + set_tests_properties(pano_cli_plan_brush_package_import_abr_smoke PROPERTIES + LABELS "app;paint;assets;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-brush-package-import\".*\"kind\":\"abr\".*\"path\":\"D:/Paint/Brushes/clouds.abr\".*\"dispatches\":1") + + add_test(NAME pano_cli_plan_brush_package_import_rejects_empty_path + COMMAND "${CMAKE_COMMAND}" + -DPANO_CLI=$ + "-DEXPECTED_OUTPUT=brush package import path must not be empty" + -P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake") + set_tests_properties(pano_cli_plan_brush_package_import_rejects_empty_path PROPERTIES + LABELS "app;paint;assets;integration;desktop-fast;fuzz") + + add_test(NAME pano_cli_plan_brush_package_import_rejects_unknown_kind + COMMAND "${CMAKE_COMMAND}" + -DPANO_CLI=$ + -DEXPECT_UNKNOWN_KIND=ON + "-DEXPECTED_OUTPUT=unknown brush package import kind" + -P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake") + set_tests_properties(pano_cli_plan_brush_package_import_rejects_unknown_kind PROPERTIES + LABELS "app;paint;assets;integration;desktop-fast;fuzz") + add_test(NAME pano_cli_plan_brush_package_export_smoke COMMAND pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr diff --git a/tests/app_core/brush_package_import_tests.cpp b/tests/app_core/brush_package_import_tests.cpp new file mode 100644 index 0000000..5d71da2 --- /dev/null +++ b/tests/app_core/brush_package_import_tests.cpp @@ -0,0 +1,73 @@ +#include "app_core/brush_package_import.h" +#include "test_harness.h" + +#include +#include + +namespace { + +class FakeBrushPackageImportServices final : public pp::app::BrushPackageImportServices { +public: + void import_brush_package(pp::app::BrushPackageImportKind kind, std::string_view path) override + { + imports += 1; + last_kind = kind; + last_path = std::string(path); + } + + int imports = 0; + pp::app::BrushPackageImportKind last_kind = pp::app::BrushPackageImportKind::abr; + std::string last_path; +}; + +void names_import_kinds(pp::tests::Harness& harness) +{ + PP_EXPECT(harness, std::string_view(pp::app::brush_package_import_kind_name(pp::app::BrushPackageImportKind::abr)) == "abr"); + PP_EXPECT(harness, std::string_view(pp::app::brush_package_import_kind_name(pp::app::BrushPackageImportKind::ppbr)) == "ppbr"); +} + +void executor_dispatches_abr_and_ppbr(pp::tests::Harness& harness) +{ + FakeBrushPackageImportServices services; + + PP_EXPECT( + harness, + pp::app::execute_brush_package_import( + pp::app::BrushPackageImportKind::abr, + "D:/Paint/Brushes/clouds.abr", + services).ok()); + PP_EXPECT(harness, services.imports == 1); + PP_EXPECT(harness, services.last_kind == pp::app::BrushPackageImportKind::abr); + PP_EXPECT(harness, services.last_path == "D:/Paint/Brushes/clouds.abr"); + + PP_EXPECT( + harness, + pp::app::execute_brush_package_import( + pp::app::BrushPackageImportKind::ppbr, + "D:/Paint/Brushes/clouds.ppbr", + services).ok()); + PP_EXPECT(harness, services.imports == 2); + PP_EXPECT(harness, services.last_kind == pp::app::BrushPackageImportKind::ppbr); + PP_EXPECT(harness, services.last_path == "D:/Paint/Brushes/clouds.ppbr"); +} + +void executor_rejects_empty_paths_before_dispatch(pp::tests::Harness& harness) +{ + FakeBrushPackageImportServices services; + + PP_EXPECT( + harness, + !pp::app::execute_brush_package_import(pp::app::BrushPackageImportKind::abr, "", services).ok()); + PP_EXPECT(harness, services.imports == 0); +} + +} // namespace + +int main() +{ + pp::tests::Harness harness; + harness.run("names import kinds", names_import_kinds); + harness.run("executor dispatches abr and ppbr", executor_dispatches_abr_and_ppbr); + harness.run("executor rejects empty paths before dispatch", executor_rejects_empty_paths_before_dispatch); + return harness.finish(); +} diff --git a/tests/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake b/tests/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake new file mode 100644 index 0000000..14826a2 --- /dev/null +++ b/tests/cmake/expect_pano_cli_plan_brush_package_import_failure.cmake @@ -0,0 +1,39 @@ +if(NOT DEFINED PANO_CLI) + message(FATAL_ERROR "PANO_CLI must be set") +endif() + +if(NOT DEFINED EXPECTED_OUTPUT) + message(FATAL_ERROR "EXPECTED_OUTPUT must be set") +endif() + +if(NOT DEFINED EXPECT_UNKNOWN_KIND) + set(EXPECT_UNKNOWN_KIND OFF) +endif() + +if(EXPECT_UNKNOWN_KIND) + execute_process( + COMMAND "${PANO_CLI}" plan-brush-package-import + --kind zip + --path D:/Paint/Brushes/clouds.zip + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error) +else() + execute_process( + COMMAND "${PANO_CLI}" plan-brush-package-import + --kind ppbr + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error) +endif() + +if(result EQUAL 0) + message(FATAL_ERROR "Expected pano_cli plan-brush-package-import to fail, but it exited 0") +endif() + +set(combined_output "${output}${error}") +string(FIND "${combined_output}" "${EXPECTED_OUTPUT}" expected_index) +if(expected_index LESS 0) + message(FATAL_ERROR + "Expected output to contain '${EXPECTED_OUTPUT}', got: ${combined_output}") +endif() diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index e8e7002..03d8b04 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -2,6 +2,7 @@ #include "app_core/app_preferences.h" #include "app_core/app_status.h" #include "app_core/app_startup.h" +#include "app_core/brush_package_import.h" #include "app_core/brush_package_export.h" #include "app_core/brush_ui.h" #include "app_core/canvas_hotkey.h" @@ -247,6 +248,11 @@ struct PlanBrushPackageExportArgs { bool has_header_image = false; }; +struct PlanBrushPackageImportArgs { + std::string kind = "ppbr"; + std::string path; +}; + struct PlanAppStatusArgs { std::string document_name = "no-name"; bool unsaved = false; @@ -1874,6 +1880,7 @@ void print_help() << " 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-startup [--run-counter N] [--auto-timelapse-disabled] [--vr-controllers-disabled] [--license-invalid]\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] [--framebuffer-fetch] [--float32] [--float32-linear] [--float16]\n" + << " plan-brush-package-import --kind abr|ppbr --path FILE\n" << " plan-brush-package-export --path FILE [--author NAME] [--email EMAIL] [--url URL] [--description TEXT] [--dest-path DIR] [--export-data|--no-export-data] [--header-image]\n" << " plan-tools-menu --command panels|options|clear-grids|reset-camera|shortcuts|sonarpen [--sonarpen-available]\n" << " plan-tools-panel --panel presets|color|color-advanced|layers|brush|grids|animation [--already-visible]\n" @@ -3502,6 +3509,90 @@ int plan_app_startup(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_brush_package_import_args( + int argc, + char** argv, + PlanBrushPackageImportArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--kind" || key == "--path") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + if (key == "--kind") { + args.kind = argv[++i]; + } else { + args.path = argv[++i]; + } + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +pp::foundation::Result parse_brush_package_import_kind( + std::string_view kind) +{ + if (kind == "abr") { + return pp::foundation::Result::success( + pp::app::BrushPackageImportKind::abr); + } + if (kind == "ppbr") { + return pp::foundation::Result::success( + pp::app::BrushPackageImportKind::ppbr); + } + + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("unknown brush package import kind")); +} + +class CliBrushPackageImportServices final : public pp::app::BrushPackageImportServices { +public: + void import_brush_package(pp::app::BrushPackageImportKind kind, std::string_view path) override + { + imports += 1; + last_kind = kind; + last_path = std::string(path); + } + + int imports = 0; + pp::app::BrushPackageImportKind last_kind = pp::app::BrushPackageImportKind::ppbr; + std::string last_path; +}; + +int plan_brush_package_import(int argc, char** argv) +{ + PlanBrushPackageImportArgs args; + const auto status = parse_plan_brush_package_import_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-brush-package-import", status.message); + return 2; + } + + const auto kind = parse_brush_package_import_kind(args.kind); + if (!kind) { + print_error("plan-brush-package-import", kind.status().message); + return 2; + } + + CliBrushPackageImportServices services; + const auto import_status = pp::app::execute_brush_package_import(kind.value(), args.path, services); + if (!import_status.ok()) { + print_error("plan-brush-package-import", import_status.message); + return 2; + } + + std::cout << "{\"ok\":true,\"command\":\"plan-brush-package-import\"" + << ",\"request\":{\"kind\":\"" << pp::app::brush_package_import_kind_name(services.last_kind) + << "\",\"path\":\"" << json_escape(services.last_path) + << "\"},\"dispatches\":" << services.imports + << "}\n"; + return 0; +} + pp::foundation::Status parse_plan_brush_package_export_args( int argc, char** argv, @@ -8674,6 +8765,10 @@ int main(int argc, char** argv) return plan_app_startup(argc, argv); } + if (command == "plan-brush-package-import") { + return plan_brush_package_import(argc, argv); + } + if (command == "plan-brush-package-export") { return plan_brush_package_export(argc, argv); }