diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 54d13c1..747715d 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -656,13 +656,16 @@ Known local toolchain state: asset/storage/UI/platform ownership is tracked by `DEBT-0047`. - `src/assets/brush_package.*` owns the first headless PPBR package helpers: header validation, legacy-compatible version acceptance, export path - normalization, and preview-data-directory planning. Live - `NodePanelBrushPreset::export_ppbr`/`import_ppbr` consumes these helpers, but - legacy Serializer/Image payload parsing, preview rendering, preset storage, - and strict-version cleanup remain tracked by `DEBT-0047` and `DEBT-0049`. + normalization, preview-data-directory planning, and imported brush + tip/pattern image target paths. Live + `NodePanelBrushPreset::export_ppbr`/`import_ppbr` and ABR import image writes + consume these helpers, but legacy Serializer/Image payload parsing, preview + rendering, preset storage, duplicate policy, and strict-version cleanup remain + tracked by `DEBT-0047`, `DEBT-0048`, and `DEBT-0049`. - `pp_assets_brush_package_tests` covers PPBR header parsing, truncated/bad magic rejection, legacy version tolerance, export package/data path planning, - legacy extension containment, and paths the legacy regex could not match. + legacy extension containment, paths the legacy regex could not match, brush + tip/pattern image target planning, and invalid imported image target inputs. - `pp_app_core_brush_package_export_tests` covers PPBR export request path validation, metadata preservation, legacy-flexible destination/export-data combinations, service dispatch, and malformed request rejection without diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 3cea81b..4351dae 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -65,7 +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.*`; PPBR header/path planning now consumes `pp_assets::brush_package`, 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_assets_brush_package_tests`; `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 clouds`; `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 | +| 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.*`; imported brush tip/pattern target paths now consume `pp_assets::brush_package`, 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_assets_brush_package_tests`; `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 | | DEBT-0049 | Open | Modernization | `pp_assets::validate_ppbr_header` intentionally preserves the legacy PPBR version check from `NodePanelBrushPreset::import_ppbr`, which accepts files when either major is `0` or minor is `1` instead of requiring exactly version `0.1` | Avoid rejecting existing brush packages before compatibility fixtures prove the stricter rule is safe | `pp_assets_brush_package_tests`; `pano_cli plan-brush-package-export --path D:/Paint/clouds.ppbr`; `ctest --preset desktop-fast --build-config Debug`; `cmake --build --preset windows-msvc-default --config Debug --target PanoPainter` | Add PPBR compatibility fixtures for accepted/rejected historical package versions, then require canonical `0.1` or an explicit supported-version matrix and update live import accordingly | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index f8da1d2..e99f3f9 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -795,6 +795,12 @@ The live PPBR import/export path consumes those helpers, while legacy Serializer/Image payload reading, stroke preview generation, preset storage, and the historical permissive version check remain tracked under `DEBT-0047` and `DEBT-0049`. +ABR and PPBR import image target planning for brush tips and patterns also now +uses `pp_assets::brush_package`, so the legacy preset panel no longer owns the +`data/brushes`, `data/brushes/thumbs`, `data/patterns`, and +`data/patterns/thumbs` path construction rules. Actual ABR/PPBR parsing, +duplicate policy, preset creation, save/reload, and progress/UI refresh remain +legacy-owned under `DEBT-0048`. Implementation tasks: @@ -1410,6 +1416,12 @@ Results: - Focused PPBR asset CTest coverage passed for `pp_assets_brush_package_tests` and the brush package export CLI tests, including path-without-directory rejection and legacy no-export-data data-directory planning. +- `PanoPainter`, `pp_assets_brush_package_tests`, + `pp_app_core_brush_package_import_tests`, and `pano_cli` built after ABR and + PPBR imported brush tip/pattern target paths moved into `pp_assets`. +- Focused brush import storage CTest coverage passed for + `pp_assets_brush_package_tests` and the brush package import/export CLI + smoke/failure 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/assets/brush_package.cpp b/src/assets/brush_package.cpp index 6e6c619..81ca23d 100644 --- a/src/assets/brush_package.cpp +++ b/src/assets/brush_package.cpp @@ -126,4 +126,34 @@ pp::foundation::Result plan_ppbr_export_paths( return pp::foundation::Result::success(std::move(paths)); } +pp::foundation::Result plan_brush_package_image_target_paths( + std::string_view data_path, + BrushPackageImageKind kind, + std::string_view image_name, + std::string_view image_extension) +{ + if (data_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush package data path must not be empty")); + } + if (image_name.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush package image name must not be empty")); + } + if (!is_word_extension(image_extension)) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("brush package image extension contains unsupported characters")); + } + + const auto directory = kind == BrushPackageImageKind::brush_tip ? "brushes" : "patterns"; + const std::string base_path = std::string(data_path) + "/" + directory + "/" + std::string(image_name) + + "." + std::string(image_extension); + + return pp::foundation::Result::success(BrushPackageImageTargetPaths { + .image_path = base_path, + .thumbnail_path = std::string(data_path) + "/" + directory + "/thumbs/" + std::string(image_name) + + "." + std::string(image_extension), + }); +} + } // namespace pp::assets diff --git a/src/assets/brush_package.h b/src/assets/brush_package.h index e94bf76..711e192 100644 --- a/src/assets/brush_package.h +++ b/src/assets/brush_package.h @@ -19,11 +19,21 @@ enum class PpbrDataDirectoryPolicy { override_directory, }; +enum class BrushPackageImageKind { + brush_tip, + pattern, +}; + struct PpbrHeader { std::uint16_t major = 0; std::uint16_t minor = 0; }; +struct BrushPackageImageTargetPaths { + std::string image_path; + std::string thumbnail_path; +}; + struct PpbrExportPaths { std::string package_path; std::string directory; @@ -50,4 +60,10 @@ struct PpbrExportPaths { bool export_data, PpbrDataDirectoryPolicy data_directory_policy); +[[nodiscard]] pp::foundation::Result plan_brush_package_image_target_paths( + std::string_view data_path, + BrushPackageImageKind kind, + std::string_view image_name, + std::string_view image_extension); + } // namespace pp::assets diff --git a/src/node_panel_brush.cpp b/src/node_panel_brush.cpp index 735057a..edb0808 100644 --- a/src/node_panel_brush.cpp +++ b/src/node_panel_brush.cpp @@ -876,8 +876,17 @@ bool NodePanelBrushPreset::import_ppbr(const std::string& path) { Image img; sr >> img; - std::string path = App::I->data_path + "/brushes/" + img.file_name + "." + img.file_ext; - std::string path_thumb = App::I->data_path + "/brushes/thumbs/" + img.file_name + "." + img.file_ext; + const auto target_paths = pp::assets::plan_brush_package_image_target_paths( + App::I->data_path, + pp::assets::BrushPackageImageKind::brush_tip, + img.file_name, + img.file_ext); + if (!target_paths) { + LOG("import_ppbr invalid brush image target: %s", target_paths.status().message); + return false; + } + const auto& path = target_paths.value().image_path; + const auto& path_thumb = target_paths.value().thumbnail_path; if (!Asset::exist(path)) { img.save_png(path); @@ -898,8 +907,17 @@ bool NodePanelBrushPreset::import_ppbr(const std::string& path) { Image img; sr >> img; - std::string path = App::I->data_path + "/patterns/" + img.file_name + "." + img.file_ext; - std::string path_thumb = App::I->data_path + "/patterns/thumbs/" + img.file_name + "." + img.file_ext; + const auto target_paths = pp::assets::plan_brush_package_image_target_paths( + App::I->data_path, + pp::assets::BrushPackageImageKind::pattern, + img.file_name, + img.file_ext); + if (!target_paths) { + LOG("import_ppbr invalid pattern image target: %s", target_paths.status().message); + return false; + } + const auto& path = target_paths.value().image_path; + const auto& path_thumb = target_paths.value().thumbnail_path; if (!Asset::exist(path)) { img.save_png(path); @@ -970,8 +988,17 @@ bool NodePanelBrushPreset::import_abr(const std::string& path) auto ii = abr.m_samples.begin(); std::advance(ii, i); const auto& samp = *ii; - std::string path_high = App::I->data_path + "/brushes/" + samp.first + ".png"; - std::string path_thumb = App::I->data_path + "/brushes/thumbs/" + samp.first + ".png"; + const auto target_paths = pp::assets::plan_brush_package_image_target_paths( + App::I->data_path, + pp::assets::BrushPackageImageKind::brush_tip, + samp.first, + "png"); + if (!target_paths) { + LOG("import_abr invalid brush image target: %s", target_paths.status().message); + return; + } + const auto& path_high = target_paths.value().image_path; + const auto& path_thumb = target_paths.value().thumbnail_path; auto padded = samp.second->resize_squared(glm::u8vec4(255)); //auto high = padded.resize_power2(); //high.save(path_high); @@ -987,8 +1014,17 @@ bool NodePanelBrushPreset::import_abr(const std::string& path) auto ii = abr.m_patterns.begin(); std::advance(ii, i); const auto& patt = *ii; - std::string path_high = App::I->data_path + "/patterns/" + patt.first + ".png"; - std::string path_thumb = App::I->data_path + "/patterns/thumbs/" + patt.first + ".png"; + const auto target_paths = pp::assets::plan_brush_package_image_target_paths( + App::I->data_path, + pp::assets::BrushPackageImageKind::pattern, + patt.first, + "png"); + if (!target_paths) { + LOG("import_abr invalid pattern image target: %s", target_paths.status().message); + return; + } + const auto& path_high = target_paths.value().image_path; + const auto& path_thumb = target_paths.value().thumbnail_path; patt.second->save_png(path_high); auto thumb = patt.second->resize(64, 64); thumb.save_png(path_thumb); diff --git a/tests/assets/brush_package_tests.cpp b/tests/assets/brush_package_tests.cpp index 2366c05..4bd1b28 100644 --- a/tests/assets/brush_package_tests.cpp +++ b/tests/assets/brush_package_tests.cpp @@ -141,6 +141,56 @@ void rejects_export_paths_that_legacy_regex_could_not_match(pp::tests::Harness& pp::assets::PpbrDataDirectoryPolicy::next_to_package)); } +void plans_imported_brush_image_targets(pp::tests::Harness& harness) +{ + const auto brush = pp::assets::plan_brush_package_image_target_paths( + "D:/Paint/data", + pp::assets::BrushPackageImageKind::brush_tip, + "cloud", + "png"); + PP_EXPECT(harness, brush); + if (brush) { + PP_EXPECT(harness, brush.value().image_path == "D:/Paint/data/brushes/cloud.png"); + PP_EXPECT(harness, brush.value().thumbnail_path == "D:/Paint/data/brushes/thumbs/cloud.png"); + } + + const auto pattern = pp::assets::plan_brush_package_image_target_paths( + "D:/Paint/data", + pp::assets::BrushPackageImageKind::pattern, + "paper", + "jpg"); + PP_EXPECT(harness, pattern); + if (pattern) { + PP_EXPECT(harness, pattern.value().image_path == "D:/Paint/data/patterns/paper.jpg"); + PP_EXPECT(harness, pattern.value().thumbnail_path == "D:/Paint/data/patterns/thumbs/paper.jpg"); + } +} + +void rejects_invalid_imported_brush_image_targets(pp::tests::Harness& harness) +{ + PP_EXPECT( + harness, + !pp::assets::plan_brush_package_image_target_paths( + "", + pp::assets::BrushPackageImageKind::brush_tip, + "cloud", + "png")); + PP_EXPECT( + harness, + !pp::assets::plan_brush_package_image_target_paths( + "D:/Paint/data", + pp::assets::BrushPackageImageKind::brush_tip, + "", + "png")); + PP_EXPECT( + harness, + !pp::assets::plan_brush_package_image_target_paths( + "D:/Paint/data", + pp::assets::BrushPackageImageKind::pattern, + "paper", + "png!")); +} + } // namespace int main() @@ -151,5 +201,7 @@ int main() harness.run("plans export package and data paths", plans_export_package_and_data_paths); harness.run("preserves legacy extension containment rule", preserves_legacy_extension_containment_rule); harness.run("rejects export paths that legacy regex could not match", rejects_export_paths_that_legacy_regex_could_not_match); + harness.run("plans imported brush image targets", plans_imported_brush_image_targets); + harness.run("rejects invalid imported brush image targets", rejects_invalid_imported_brush_image_targets); return harness.finish(); }