diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index 77c77e4..f70d1fa 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -301,8 +301,11 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.p 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. + serialization. The command also reports the app-core write-mode plan for + direct first saves versus existing-target temporary writes, including the + retained direct-write fallback when the temporary file cannot be opened, and + it is covered by forward-slash, Windows backslash, existing-target, 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. @@ -1182,7 +1185,8 @@ powershell -ExecutionPolicy Bypass -File scripts\automation\apple-remote-build.p prepare and log a payload-bearing canvas document snapshot report, run the 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`. + plus direct-vs-temporary write-mode decisions 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 2bd1238..c8e42a8 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -541,6 +541,14 @@ agent or engineer to remove them without reconstructing context from chat. `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-06: DEBT-0040/DEBT-0042 were narrowed again. The same app-core + project-save planner now owns the direct-write versus temporary-write mode + decision for new and existing targets, including the retained compatibility + fallback to direct target writes when the temporary file cannot be opened. + `Canvas::project_save_thread` consumes that policy before retained + serialization, and `pano_cli plan-canvas-project-save-target` reports it in + JSON. Actual PPI bytes, 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 de51671..a8ca4f0 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -709,7 +709,11 @@ 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 +app-core boundary now also plans the retained save write mode, distinguishing +direct first saves from existing-target temporary writes that swap into place +and preserving the legacy direct-write fallback when the temporary file cannot +be opened. 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 @@ -2313,6 +2317,9 @@ Results: 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. + It also reports the app-core write-mode plan for direct first saves versus + existing-target temporary writes, including the retained fallback to direct + target writes when the temporary file cannot be opened. - 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 f6a67d9..c8e4d12 100644 --- a/src/app_core/document_canvas.h +++ b/src/app_core/document_canvas.h @@ -103,6 +103,21 @@ struct DocumentCanvasProjectSaveTargetPlan { std::string timelapse_path; }; +enum class DocumentCanvasProjectSaveWriteAction { + write_direct_to_target, + write_temporary_then_swap, +}; + +struct DocumentCanvasProjectSaveWritePlan { + DocumentCanvasProjectSaveWriteAction action = DocumentCanvasProjectSaveWriteAction::write_direct_to_target; + std::string write_path; + std::string target_path; + std::string temporary_path; + bool target_exists = false; + bool uses_temporary = false; + bool falls_back_to_direct_on_temporary_open_failure = false; +}; + class DocumentCanvasClearServices { public: virtual ~DocumentCanvasClearServices() = default; @@ -350,6 +365,38 @@ plan_document_canvas_project_save_target( return pp::foundation::Result::success(std::move(plan)); } +[[nodiscard]] inline pp::foundation::Result +plan_document_canvas_project_save_write( + const DocumentCanvasProjectSaveTargetPlan& target, + bool target_exists) +{ + if (target.target_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("project save write target path must not be empty")); + } + + DocumentCanvasProjectSaveWritePlan plan; + plan.target_exists = target_exists; + plan.target_path = target.target_path; + plan.temporary_path = target.temporary_path; + + if (!target_exists) { + plan.write_path = target.target_path; + return pp::foundation::Result::success(std::move(plan)); + } + + if (target.temporary_path.empty()) { + return pp::foundation::Result::failure( + pp::foundation::Status::invalid_argument("project save temporary path must not be empty")); + } + + plan.action = DocumentCanvasProjectSaveWriteAction::write_temporary_then_swap; + plan.write_path = target.temporary_path; + plan.uses_temporary = true; + plan.falls_back_to_direct_on_temporary_open_failure = true; + 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 038eac1..e32ff68 100644 --- a/src/canvas.cpp +++ b/src/canvas.cpp @@ -2370,7 +2370,7 @@ bool Canvas::project_save_thread(std::string file_path, bool show_progress) // static char name[128]; // sprintf(name, "%s/latlong.ppi", data_path.c_str()); - FILE* fp; + FILE* fp = nullptr; const auto save_target = pp::app::plan_document_canvas_project_save_target(App::I->data_path, file_path); if (!save_target) { @@ -2385,15 +2385,25 @@ bool Canvas::project_save_thread(std::string file_path, bool show_progress) LOG("file name %s", file_name.c_str()); LOG("tmp path %s", tmp_path.c_str()); - bool use_tmp = false; - // check if file already exists - if ((fp = fopen(file_path.c_str(), "rb"))) - { + bool target_exists = false; + if ((fp = fopen(file_path.c_str(), "rb"))) { fclose(fp); + fp = nullptr; + target_exists = true; + } + + const auto write_plan = pp::app::plan_document_canvas_project_save_write(save_paths, target_exists); + if (!write_plan) { + LOG("cannot plan project save write for %s: %s", file_path.c_str(), write_plan.status().message); + return false; + } + + bool use_tmp = write_plan.value().uses_temporary; + if (write_plan.value().uses_temporary) + { LOG("use tmp file"); - // use tmp file for writing - use_tmp = true; - if (!(fp = fopen(tmp_path.c_str(), "wb"))) + fp = fopen(write_plan.value().write_path.c_str(), "wb"); + if (!fp) { LOG("cannot write tmp project to %s", tmp_path.c_str()); use_tmp = false; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8c9821a..b18371c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1530,7 +1530,7 @@ if(TARGET pano_cli) 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\"") + 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\".*\"writePlan\":\\{\"action\":\"write-direct-to-target\",\"targetExists\":false,\"usesTemporary\":false,\"writePath\":\"D:/Paint/projects/demo.ppi\",\"fallbackDirectOnTemporaryOpenFailure\":false\\}") 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") @@ -1538,6 +1538,12 @@ if(TARGET pano_cli) 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_existing_smoke + COMMAND pano_cli plan-canvas-project-save-target --data-dir D:/Paint/data --path D:/Paint/projects/demo.ppi --target-exists) + set_tests_properties(pano_cli_plan_canvas_project_save_target_existing_smoke PROPERTIES + LABELS "app;document;integration;desktop-fast" + PASS_REGULAR_EXPRESSION "\"command\":\"plan-canvas-project-save-target\".*\"targetPath\":\"D:/Paint/projects/demo.ppi\".*\"writePlan\":\\{\"action\":\"write-temporary-then-swap\",\"targetExists\":true,\"usesTemporary\":true,\"writePath\":\"D:/Paint/data/demo.tmp.ppi\",\"fallbackDirectOnTemporaryOpenFailure\":true\\}") + 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 diff --git a/tests/app_core/document_canvas_tests.cpp b/tests/app_core/document_canvas_tests.cpp index 48a7789..ce42e85 100644 --- a/tests/app_core/document_canvas_tests.cpp +++ b/tests/app_core/document_canvas_tests.cpp @@ -287,6 +287,72 @@ void project_save_target_plan_rejects_empty_inputs(pp::tests::Harness& harness) PP_EXPECT(harness, no_name.status().code == pp::foundation::StatusCode::invalid_argument); } +void project_save_write_plan_writes_direct_for_new_targets(pp::tests::Harness& harness) +{ + const auto target = pp::app::plan_document_canvas_project_save_target( + "D:/Paint/data", + "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, target); + if (!target) { + return; + } + + const auto plan = pp::app::plan_document_canvas_project_save_write(target.value(), false); + + PP_EXPECT(harness, plan); + if (!plan) { + return; + } + + PP_EXPECT(harness, plan.value().action == pp::app::DocumentCanvasProjectSaveWriteAction::write_direct_to_target); + PP_EXPECT(harness, plan.value().write_path == "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, plan.value().target_path == "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, !plan.value().target_exists); + PP_EXPECT(harness, !plan.value().uses_temporary); + PP_EXPECT(harness, !plan.value().falls_back_to_direct_on_temporary_open_failure); +} + +void project_save_write_plan_prefers_temporary_for_existing_targets(pp::tests::Harness& harness) +{ + const auto target = pp::app::plan_document_canvas_project_save_target( + "D:/Paint/data", + "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, target); + if (!target) { + return; + } + + const auto plan = pp::app::plan_document_canvas_project_save_write(target.value(), true); + + PP_EXPECT(harness, plan); + if (!plan) { + return; + } + + PP_EXPECT(harness, plan.value().action == pp::app::DocumentCanvasProjectSaveWriteAction::write_temporary_then_swap); + PP_EXPECT(harness, plan.value().write_path == "D:/Paint/data/demo.tmp.ppi"); + PP_EXPECT(harness, plan.value().target_path == "D:/Paint/projects/demo.ppi"); + PP_EXPECT(harness, plan.value().temporary_path == "D:/Paint/data/demo.tmp.ppi"); + PP_EXPECT(harness, plan.value().target_exists); + PP_EXPECT(harness, plan.value().uses_temporary); + PP_EXPECT(harness, plan.value().falls_back_to_direct_on_temporary_open_failure); +} + +void project_save_write_plan_rejects_missing_paths(pp::tests::Harness& harness) +{ + pp::app::DocumentCanvasProjectSaveTargetPlan empty_target; + const auto no_target = pp::app::plan_document_canvas_project_save_write(empty_target, false); + + pp::app::DocumentCanvasProjectSaveTargetPlan no_temporary; + no_temporary.target_path = "D:/Paint/projects/demo.ppi"; + const auto missing_temporary = pp::app::plan_document_canvas_project_save_write(no_temporary, true); + + PP_EXPECT(harness, !no_target); + PP_EXPECT(harness, !missing_temporary); + PP_EXPECT(harness, no_target.status().code == pp::foundation::StatusCode::invalid_argument); + PP_EXPECT(harness, missing_temporary.status().code == pp::foundation::StatusCode::invalid_argument); +} + void snapshot_plan_rejects_invalid_canvas_state(pp::tests::Harness& harness) { const std::uint32_t frames[] { 100U }; @@ -471,6 +537,9 @@ int main() 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("project save write plan writes direct for new targets", project_save_write_plan_writes_direct_for_new_targets); + harness.run("project save write plan prefers temporary for existing targets", project_save_write_plan_prefers_temporary_for_existing_targets); + harness.run("project save write plan rejects missing paths", project_save_write_plan_rejects_missing_paths); 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 dc049b6..2a40524 100644 --- a/tools/pano_cli/main.cpp +++ b/tools/pano_cli/main.cpp @@ -413,6 +413,7 @@ struct PlanCanvasClearArgs { struct PlanCanvasProjectSaveTargetArgs { std::string data_directory = "D:/Paint/data"; std::string target_path = "D:/Paint/projects/demo.ppi"; + bool target_exists = false; }; struct PlanCanvasDocumentSnapshotArgs { @@ -966,6 +967,19 @@ const char* document_canvas_save_writer_action_name(pp::app::DocumentCanvasSaveW return "use-legacy-project-save"; } +const char* document_canvas_project_save_write_action_name( + pp::app::DocumentCanvasProjectSaveWriteAction action) noexcept +{ + switch (action) { + case pp::app::DocumentCanvasProjectSaveWriteAction::write_direct_to_target: + return "write-direct-to-target"; + case pp::app::DocumentCanvasProjectSaveWriteAction::write_temporary_then_swap: + return "write-temporary-then-swap"; + } + + return "write-direct-to-target"; +} + const char* document_workflow_decision_name(pp::app::DocumentWorkflowDecision decision) noexcept { switch (decision) { @@ -2546,7 +2560,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-canvas-project-save-target [--data-dir DIR] [--path FILE] [--target-exists]\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" @@ -6075,6 +6089,8 @@ pp::foundation::Status parse_plan_canvas_project_save_target_args( } else { args.target_path = argv[++i]; } + } else if (key == "--target-exists") { + args.target_exists = true; } else { return pp::foundation::Status::invalid_argument("unknown option"); } @@ -6100,14 +6116,31 @@ int plan_canvas_project_save_target(int argc, char** argv) return 2; } + const auto write_plan = pp::app::plan_document_canvas_project_save_write( + plan.value(), + args.target_exists); + if (!write_plan) { + print_error("plan-canvas-project-save-target", write_plan.status().message); + return 2; + } + const auto& value = plan.value(); + const auto& write_value = write_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"; + << "\",\"writePlan\":{\"action\":\"" + << document_canvas_project_save_write_action_name(write_value.action) + << "\",\"targetExists\":" << json_bool(write_value.target_exists) + << ",\"usesTemporary\":" << json_bool(write_value.uses_temporary) + << ",\"writePath\":\"" << json_escape(write_value.write_path) + << "\",\"fallbackDirectOnTemporaryOpenFailure\":" + << json_bool(write_value.falls_back_to_direct_on_temporary_open_failure) + << "}" + << "}\n"; return 0; }