From ed9709ade8d31d169852d989f04a2f048ea1293f Mon Sep 17 00:00:00 2001 From: omigamedev Date: Sat, 6 Jun 2026 11:52:49 +0200 Subject: [PATCH] Move project save target planning to app core --- docs/modernization/build-inventory.md | 11 ++++- docs/modernization/debt.md | 7 +++ docs/modernization/roadmap.md | 11 ++++- src/app_core/document_canvas.h | 51 +++++++++++++++++++ src/canvas.cpp | 14 ++++-- tests/CMakeLists.txt | 18 +++++++ tests/app_core/document_canvas_tests.cpp | 50 +++++++++++++++++++ tools/pano_cli/main.cpp | 62 ++++++++++++++++++++++++ 8 files changed, 217 insertions(+), 7 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 10cc860..77c77e4 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -297,6 +297,12 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.p 1024x1024 draw/readback plan counts. It is covered by `pano_cli_plan_canvas_document_snapshot_smoke` plus the payload-bearing snapshot smoke. +- `pano_cli plan-canvas-project-save-target` exposes the app-core planner for + retained project-save target paths, including the requested PPI path, + temporary `.tmp.ppi` path, and timelapse `.pptl` sidecar path. The live + `Canvas::project_save_thread` consumes the same planner before retained + serialization, and the command is covered by forward-slash, Windows + backslash, and invalid-path smokes. - Live equirectangular, layer, animation-frame, and cube-face export adapters now prepare and log the same payload-bearing canvas document snapshot plus shared paint-renderer export-readiness report. @@ -1174,8 +1180,9 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.p overwrite prompts, document field updates, title updates, and keyboard/dialog cleanup. Existing Save, Save As, Save Version, and save-before-workflow prepare and log a payload-bearing canvas document snapshot report, run the - app-core pure PPI save-writer route for payload-complete snapshots, and log - generated byte counts before delegating to retained `Canvas::project_save`. + app-core pure PPI save-writer route for payload-complete snapshots, log + generated byte counts, and derive project-save target/tmp/timelapse paths + through `pp_app_core` before delegating to retained `Canvas::project_save`. Retained legacy UI/canvas execution and actual live save serialization remain tracked by `DEBT-0040`, `DEBT-0041`, and `DEBT-0042`; the pure snapshot-to-PPI export handoff is diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 1092d6e..2bd1238 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -534,6 +534,13 @@ agent or engineer to remove them without reconstructing context from chat. `Canvas::project_save` continues. The retained writer still owns actual save serialization, app metadata mutation, progress/threading, and compatibility quirks. +- 2026-06-06: DEBT-0040/DEBT-0042 were narrowed again. `pp_app_core` now owns + the retained project-save target path planner for the requested PPI path, + temporary `.tmp.ppi` path, and timelapse `.pptl` sidecar path; live + `Canvas::project_save_thread` consumes it and + `pano_cli plan-canvas-project-save-target` exposes it for automation. Actual + PPI serialization, temporary-file swap execution, progress/threading, + timelapse sidecar serialization, and app metadata mutation remain retained. - 2026-06-05: DEBT-0010/DEBT-0013 were narrowed again. `pp_app_core` now exports payload-complete or metadata-only canvas document snapshots through the pure `pp_document` PPI writer and rejects snapshots that still require diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index dab8e2a..de51671 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -705,7 +705,11 @@ save-writer route/byte count before retained save continues. The app-core snapshot boundary also has a tested pure PPI export helper, and `pano_cli plan-canvas-document-snapshot` runs that helper for payload-complete snapshots and reports generated byte/dirty-face summaries plus the same -save-writer route JSON. The same automation now feeds payload-complete snapshots through the shared +save-writer route JSON. `pp_app_core` also owns the retained project-save target +path planner for target, temporary PPI, and timelapse sidecar paths; live +`Canvas::project_save_thread` consumes that planner and +`pano_cli plan-canvas-project-save-target` exposes it for automation. The same +automation now feeds payload-complete snapshots through the shared `pp_paint_renderer::prepare_document_frame_export_readiness` report, which records renderer-neutral six-face texture upload commands and encodes the active document frame's six composited faces to PNG bytes. This gives CLI @@ -2304,6 +2308,11 @@ Results: the same save-writer route, and payload-complete snapshots now run the pure `pp_document` PPI exporter and decoded-project summary before emitting `ppiExport` JSON. +- `pano_cli plan-canvas-project-save-target` now exposes the app-core planner + for retained project-save target paths, including the target PPI path, + temporary `.tmp.ppi` path, and timelapse `.pptl` sidecar. The live + `Canvas::project_save_thread` consumes the same planner before retained + serialization, reducing inline path compatibility logic in the legacy writer. - The same payload-complete snapshot automation now uploads the active document frame through `pp_paint_renderer::upload_document_frame_faces` and the `RecordingRenderDevice`, emitting `rendererUpload` JSON with texture, diff --git a/src/app_core/document_canvas.h b/src/app_core/document_canvas.h index 5b0b2a6..f6a67d9 100644 --- a/src/app_core/document_canvas.h +++ b/src/app_core/document_canvas.h @@ -96,6 +96,13 @@ struct DocumentCanvasPpiExportResult { std::vector bytes; }; +struct DocumentCanvasProjectSaveTargetPlan { + std::string target_path; + std::string file_name; + std::string temporary_path; + std::string timelapse_path; +}; + class DocumentCanvasClearServices { public: virtual ~DocumentCanvasClearServices() = default; @@ -299,6 +306,50 @@ export_document_canvas_save_snapshot_to_ppi(const DocumentCanvasSnapshotResult& }); } +[[nodiscard]] inline pp::foundation::Result +plan_document_canvas_project_save_target( + std::string_view data_directory, + std::string_view target_path) +{ + if (data_directory.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("project save data directory must not be empty")); + } + if (target_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("project save target path must not be empty")); + } + + const auto basename_start = target_path.find_last_of("/\\"); + const auto file_name_start = basename_start == std::string_view::npos ? 0U : basename_start + 1U; + auto file_name = target_path.substr(file_name_start); + if (file_name.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("project save target file name must not be empty")); + } + + constexpr std::string_view ppi_extension = ".ppi"; + if (file_name.size() > ppi_extension.size() + && file_name.substr(file_name.size() - ppi_extension.size()) == ppi_extension) { + file_name.remove_suffix(ppi_extension.size()); + } + + DocumentCanvasProjectSaveTargetPlan plan; + plan.target_path = std::string(target_path); + plan.file_name = std::string(file_name); + plan.temporary_path.reserve(data_directory.size() + plan.file_name.size() + 10U); + plan.temporary_path += data_directory; + plan.temporary_path += "/"; + plan.temporary_path += plan.file_name; + plan.temporary_path += ".tmp.ppi"; + plan.timelapse_path.reserve(data_directory.size() + plan.file_name.size() + 6U); + plan.timelapse_path += data_directory; + plan.timelapse_path += "/"; + plan.timelapse_path += plan.file_name; + plan.timelapse_path += ".pptl"; + return pp::foundation::Result::success(std::move(plan)); +} + [[nodiscard]] inline pp::foundation::Result plan_document_canvas_clear( bool has_canvas, float r = 0.0F, diff --git a/src/canvas.cpp b/src/canvas.cpp index 47bb5d5..038eac1 100644 --- a/src/canvas.cpp +++ b/src/canvas.cpp @@ -4,6 +4,7 @@ #include "app.h" #include "legacy_gl_renderbuffer_dispatch.h" #include "legacy_ui_gl_dispatch.h" +#include "app_core/document_canvas.h" #include "texture.h" #include "node_progress_bar.h" #include "paint_renderer/compositor.h" @@ -2371,10 +2372,15 @@ bool Canvas::project_save_thread(std::string file_path, bool show_progress) // sprintf(name, "%s/latlong.ppi", data_path.c_str()); FILE* fp; - auto start = file_path.rfind('/') + 1; - std::string file_name = file_path.substr(start, file_path.length() - start - strlen(".ppi")); - std::string tmp_path = App::I->data_path + '/' + file_name + ".tmp.ppi"; - std::string lapse_path = App::I->data_path + '/' + file_name + ".pptl"; + const auto save_target = pp::app::plan_document_canvas_project_save_target(App::I->data_path, file_path); + if (!save_target) { + LOG("cannot plan project save target for %s: %s", file_path.c_str(), save_target.status().message); + return false; + } + const auto& save_paths = save_target.value(); + const std::string& file_name = save_paths.file_name; + const std::string& tmp_path = save_paths.temporary_path; + const std::string& lapse_path = save_paths.timelapse_path; LOG("file name %s", file_name.c_str()); LOG("tmp path %s", tmp_path.c_str()); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1055e73..8c9821a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1526,6 +1526,24 @@ if(TARGET pano_cli) LABELS "app;document;integration;desktop-fast;fuzz" WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_canvas_project_save_target_smoke + COMMAND pano_cli plan-canvas-project-save-target --data-dir D:/Paint/data --path D:/Paint/projects/demo.ppi) + set_tests_properties(pano_cli_plan_canvas_project_save_target_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-canvas-project-save-target\".*\"dataDirectory\":\"D:/Paint/data\".*\"targetPath\":\"D:/Paint/projects/demo.ppi\".*\"fileName\":\"demo\".*\"temporaryPath\":\"D:/Paint/data/demo.tmp.ppi\".*\"timelapsePath\":\"D:/Paint/data/demo.pptl\"") + + add_test(NAME pano_cli_plan_canvas_project_save_target_backslash_smoke + COMMAND pano_cli plan-canvas-project-save-target --data-dir D:/Paint/data --path "D:\\Paint\\projects\\demo.ppi") + set_tests_properties(pano_cli_plan_canvas_project_save_target_backslash_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-canvas-project-save-target\".*\"fileName\":\"demo\".*\"temporaryPath\":\"D:/Paint/data/demo.tmp.ppi\".*\"timelapsePath\":\"D:/Paint/data/demo.pptl\"") + + add_test(NAME pano_cli_plan_canvas_project_save_target_rejects_empty_path + COMMAND pano_cli plan-canvas-project-save-target --path "") + set_tests_properties(pano_cli_plan_canvas_project_save_target_rejects_empty_path PROPERTIES + LABELS "app;document;integration;desktop-fast;fuzz" + WILL_FAIL TRUE) + add_test(NAME pano_cli_plan_canvas_document_snapshot_smoke COMMAND pano_cli plan-canvas-document-snapshot --width 128 --height 64 --layers 3 --frames 2 --current-layer 2 --current-frame 1 --hidden-layer 0 --alpha-locked-layer 2 --opacity 0.5 --blend-mode 4 --pending-face-payloads-per-layer 6) set_tests_properties(pano_cli_plan_canvas_document_snapshot_smoke PROPERTIES diff --git a/tests/app_core/document_canvas_tests.cpp b/tests/app_core/document_canvas_tests.cpp index 243cfb3..48a7789 100644 --- a/tests/app_core/document_canvas_tests.cpp +++ b/tests/app_core/document_canvas_tests.cpp @@ -240,6 +240,53 @@ void save_writer_route_falls_back_for_pending_payloads(pp::tests::Harness& harne plan.fallback_reason == "canvas document snapshot still requires renderer payload readback"); } +void project_save_target_plan_preserves_legacy_paths(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_canvas_project_save_target( + "D:/Paint/data", + "D:/Paint/projects/demo.ppi"); + + PP_EXPECT(harness, plan); + if (!plan) { + return; + } + + PP_EXPECT(harness, plan.value().target_path == "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, plan.value().file_name == "demo"); + PP_EXPECT(harness, plan.value().temporary_path == "D:/Paint/data/demo.tmp.ppi"); + PP_EXPECT(harness, plan.value().timelapse_path == "D:/Paint/data/demo.pptl"); +} + +void project_save_target_plan_accepts_windows_backslashes(pp::tests::Harness& harness) +{ + const auto plan = pp::app::plan_document_canvas_project_save_target( + "D:/Paint/data", + "D:\\Paint\\projects\\demo.ppi"); + + PP_EXPECT(harness, plan); + if (!plan) { + return; + } + + PP_EXPECT(harness, plan.value().file_name == "demo"); + PP_EXPECT(harness, plan.value().temporary_path == "D:/Paint/data/demo.tmp.ppi"); + PP_EXPECT(harness, plan.value().timelapse_path == "D:/Paint/data/demo.pptl"); +} + +void project_save_target_plan_rejects_empty_inputs(pp::tests::Harness& harness) +{ + const auto no_data = pp::app::plan_document_canvas_project_save_target("", "D:/Paint/demo.ppi"); + const auto no_target = pp::app::plan_document_canvas_project_save_target("D:/Paint/data", ""); + const auto no_name = pp::app::plan_document_canvas_project_save_target("D:/Paint/data", "D:/Paint/"); + + PP_EXPECT(harness, !no_data); + PP_EXPECT(harness, !no_target); + PP_EXPECT(harness, !no_name); + PP_EXPECT(harness, no_data.status().code == pp::foundation::StatusCode::invalid_argument); + PP_EXPECT(harness, no_target.status().code == pp::foundation::StatusCode::invalid_argument); + PP_EXPECT(harness, no_name.status().code == pp::foundation::StatusCode::invalid_argument); +} + void snapshot_plan_rejects_invalid_canvas_state(pp::tests::Harness& harness) { const std::uint32_t frames[] { 100U }; @@ -421,6 +468,9 @@ int main() harness.run("snapshot plan attaches captured face payloads", snapshot_plan_attaches_captured_face_payloads); harness.run("save writer route uses ppi writer for complete payloads", save_writer_route_uses_ppi_writer_for_complete_payloads); harness.run("save writer route falls back for pending payloads", save_writer_route_falls_back_for_pending_payloads); + harness.run("project save target plan preserves legacy paths", project_save_target_plan_preserves_legacy_paths); + harness.run("project save target plan accepts windows backslashes", project_save_target_plan_accepts_windows_backslashes); + harness.run("project save target plan rejects empty inputs", project_save_target_plan_rejects_empty_inputs); harness.run("snapshot plan rejects invalid canvas state", snapshot_plan_rejects_invalid_canvas_state); harness.run("clear plan records legacy canvas effects", clear_plan_records_legacy_canvas_effects); harness.run("clear plan noops without canvas", clear_plan_noops_without_canvas); diff --git a/tools/pano_cli/main.cpp b/tools/pano_cli/main.cpp index 0475cff..dc049b6 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -410,6 +410,11 @@ struct PlanCanvasClearArgs { float a = 0.0F; }; +struct PlanCanvasProjectSaveTargetArgs { + std::string data_directory = "D:/Paint/data"; + std::string target_path = "D:/Paint/projects/demo.ppi"; +}; + struct PlanCanvasDocumentSnapshotArgs { bool has_canvas = true; std::uint32_t width = 64; @@ -2541,6 +2546,7 @@ void print_help() << " plan-canvas-view-density [--density N] [--bad-float]\n" << " plan-canvas-view-cursor-mode [--mode N]\n" << " plan-canvas-cursor [--mode draw|erase|line|camera|grid|copy|cut|fill|mask-free|mask-line|bucket] [--visibility never|small-brush|not-painting|always] [--brush-size N] [--no-brush] [--drawing] [--alt] [--resizing] [--picking] [--bad-size]\n" + << " plan-canvas-project-save-target [--data-dir DIR] [--path FILE]\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" << " plan-history-operation --kind undo|redo|clear [--undo-count N] [--redo-count N] [--memory-bytes N]\n" << " plan-main-toolbar --command open|save|undo|redo|clear-history|clear-canvas|message-box|settings [--undo-count N] [--redo-count N] [--memory-bytes N] [--no-canvas]\n" @@ -6053,6 +6059,58 @@ int plan_canvas_clear(int argc, char** argv) return 0; } +pp::foundation::Status parse_plan_canvas_project_save_target_args( + int argc, + char** argv, + PlanCanvasProjectSaveTargetArgs& args) +{ + for (int i = 2; i < argc; ++i) { + const std::string_view key(argv[i]); + if (key == "--data-dir" || key == "--path") { + if (i + 1 >= argc) { + return pp::foundation::Status::invalid_argument("missing value for option"); + } + if (key == "--data-dir") { + args.data_directory = argv[++i]; + } else { + args.target_path = argv[++i]; + } + } else { + return pp::foundation::Status::invalid_argument("unknown option"); + } + } + + return pp::foundation::Status::success(); +} + +int plan_canvas_project_save_target(int argc, char** argv) +{ + PlanCanvasProjectSaveTargetArgs args; + const auto status = parse_plan_canvas_project_save_target_args(argc, argv, args); + if (!status.ok()) { + print_error("plan-canvas-project-save-target", status.message); + return 2; + } + + const auto plan = pp::app::plan_document_canvas_project_save_target( + args.data_directory, + args.target_path); + if (!plan) { + print_error("plan-canvas-project-save-target", plan.status().message); + return 2; + } + + const auto& value = plan.value(); + std::cout << "{\"ok\":true,\"command\":\"plan-canvas-project-save-target\"" + << ",\"dataDirectory\":\"" << json_escape(args.data_directory) + << "\",\"targetPath\":\"" << json_escape(value.target_path) + << "\",\"fileName\":\"" << json_escape(value.file_name) + << "\",\"temporaryPath\":\"" << json_escape(value.temporary_path) + << "\",\"timelapsePath\":\"" << json_escape(value.timelapse_path) + << "\"}\n"; + return 0; +} + pp::foundation::Status parse_plan_canvas_document_snapshot_args( int argc, char** argv, @@ -12265,6 +12323,10 @@ int main(int argc, char** argv) return plan_canvas_clear(argc, argv); } + if (command == "plan-canvas-project-save-target") { + return plan_canvas_project_save_target(argc, argv); + } + if (command == "plan-canvas-document-snapshot") { return plan_canvas_document_snapshot(argc, argv); }