From 44aebf61b2ab8a6b68351feb8bfb1e38212218db Mon Sep 17 00:00:00 2001 From: omigamedev Date: Mon, 1 Jun 2026 09:03:46 +0200 Subject: [PATCH] Add document frame move coverage --- docs/modernization/build-inventory.md | 4 +- docs/modernization/roadmap.md | 8 ++-- src/document/document.cpp | 63 +++++++++++++++++++++++---- src/document/document.h | 2 + tests/document/document_tests.cpp | 37 ++++++++++++++++ 5 files changed, 101 insertions(+), 13 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index ce9c976..b3fe6bb 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -87,8 +87,8 @@ Known local toolchain state: `pp_paint`, `pp_document`, `pp_renderer_api`, `pp_paint_renderer`, `pp_ui_core`, `pano_cli`, and their current headless test binaries, including foundation event/logging/task queue coverage, PNG metadata, PPI - header, settings document, paint brush/stroke coverage, UI color parsing, and - layout XML parse coverage. + header, settings document, document frame move/duration coverage, paint + brush/stroke coverage, UI color parsing, and layout XML parse coverage. - `pano_cli inspect-image` reports PNG IHDR metadata as JSON and is covered by `pano_cli_inspect_png_metadata_smoke` with a tiny IHDR fixture. - `panopainter_validate_shaders` validates the current combined GLSL shader diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 3ee93d0..372c6d7 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -315,8 +315,9 @@ key/value limit tests. `pp_paint` has started with pure brush parameter validation/stamp evaluation, CPU reference math for the five current shader blend modes, and deterministic stroke spacing/interpolation. `pp_document` has -started with a pure canvas/layer/frame model, layer metadata operations, and -layer/frame/undo-redo history invariant tests. `pp_renderer_api` has started with renderer-neutral +started with a pure canvas/layer/frame model, layer metadata operations, frame +move/duration queries, and layer/frame/undo-redo history invariant tests. +`pp_renderer_api` has started with renderer-neutral texture/readback descriptors and validation tests. `pp_paint_renderer` has started with deterministic CPU layer compositing over renderer extents using the paint blend reference. `pp_ui_core` has started with XML-layout-facing @@ -560,7 +561,8 @@ Results: - `pp_paint_brush_tests` passed. - `pp_paint_blend_tests` passed. - `pp_paint_stroke_tests` passed. -- `pp_document_tests` passed. +- `pp_document_tests` passed, including frame move, duration, and history + invariants. - `pp_renderer_api_tests` passed. - `pp_paint_renderer_compositor_tests` passed. - `pp_ui_core_color_tests` passed. diff --git a/src/document/document.cpp b/src/document/document.cpp index 87ff5fa..e009bad 100644 --- a/src/document/document.cpp +++ b/src/document/document.cpp @@ -55,6 +55,15 @@ namespace { return pp::foundation::Status::success(); } +[[nodiscard]] pp::foundation::Status validate_frame_index(std::size_t index, std::size_t frame_count) noexcept +{ + if (index >= frame_count) { + return pp::foundation::Status::out_of_range("frame index is outside the document"); + } + + return pp::foundation::Status::success(); +} + } pp::foundation::Result CanvasDocument::create(DocumentConfig config) @@ -96,6 +105,15 @@ std::size_t CanvasDocument::active_frame_index() const noexcept return active_frame_index_; } +std::uint64_t CanvasDocument::animation_duration_ms() const noexcept +{ + std::uint64_t duration = 0; + for (const auto& frame : frames_) { + duration += frame.duration_ms; + } + return duration; +} + std::span CanvasDocument::layers() const noexcept { return layers_; @@ -265,9 +283,10 @@ pp::foundation::Result CanvasDocument::add_frame(std::uint32_t dura pp::foundation::Result CanvasDocument::duplicate_frame(std::size_t index) { - if (index >= frames_.size()) { + const auto index_status = validate_frame_index(index, frames_.size()); + if (!index_status.ok()) { return pp::foundation::Result::failure( - pp::foundation::Status::out_of_range("frame index is outside the document")); + index_status); } if (frames_.size() >= max_frame_count) { @@ -283,8 +302,9 @@ pp::foundation::Result CanvasDocument::duplicate_frame(std::size_t pp::foundation::Status CanvasDocument::remove_frame(std::size_t index) { - if (index >= frames_.size()) { - return pp::foundation::Status::out_of_range("frame index is outside the document"); + const auto index_status = validate_frame_index(index, frames_.size()); + if (!index_status.ok()) { + return index_status; } if (frames_.size() == 1U) { @@ -301,10 +321,36 @@ pp::foundation::Status CanvasDocument::remove_frame(std::size_t index) return pp::foundation::Status::success(); } +pp::foundation::Status CanvasDocument::move_frame(std::size_t from, std::size_t to) +{ + if (from >= frames_.size() || to >= frames_.size()) { + return pp::foundation::Status::out_of_range("frame index is outside the document"); + } + + if (from == to) { + return pp::foundation::Status::success(); + } + + const auto frame = frames_[from]; + frames_.erase(frames_.begin() + static_cast(from)); + frames_.insert(frames_.begin() + static_cast(to), frame); + + if (active_frame_index_ == from) { + active_frame_index_ = to; + } else if (from < active_frame_index_ && active_frame_index_ <= to) { + --active_frame_index_; + } else if (to <= active_frame_index_ && active_frame_index_ < from) { + ++active_frame_index_; + } + + return pp::foundation::Status::success(); +} + pp::foundation::Status CanvasDocument::set_frame_duration(std::size_t index, std::uint32_t duration_ms) noexcept { - if (index >= frames_.size()) { - return pp::foundation::Status::out_of_range("frame index is outside the document"); + const auto index_status = validate_frame_index(index, frames_.size()); + if (!index_status.ok()) { + return index_status; } if (duration_ms < min_frame_duration_ms) { @@ -317,8 +363,9 @@ pp::foundation::Status CanvasDocument::set_frame_duration(std::size_t index, std pp::foundation::Status CanvasDocument::set_active_frame(std::size_t index) noexcept { - if (index >= frames_.size()) { - return pp::foundation::Status::out_of_range("frame index is outside the document"); + const auto index_status = validate_frame_index(index, frames_.size()); + if (!index_status.ok()) { + return index_status; } active_frame_index_ = index; diff --git a/src/document/document.h b/src/document/document.h index e9207b5..5960d8b 100644 --- a/src/document/document.h +++ b/src/document/document.h @@ -44,6 +44,7 @@ public: [[nodiscard]] std::uint32_t height() const noexcept; [[nodiscard]] std::size_t active_layer_index() const noexcept; [[nodiscard]] std::size_t active_frame_index() const noexcept; + [[nodiscard]] std::uint64_t animation_duration_ms() const noexcept; [[nodiscard]] std::span layers() const noexcept; [[nodiscard]] std::span frames() const noexcept; @@ -59,6 +60,7 @@ public: [[nodiscard]] pp::foundation::Result add_frame(std::uint32_t duration_ms); [[nodiscard]] pp::foundation::Result duplicate_frame(std::size_t index); [[nodiscard]] pp::foundation::Status remove_frame(std::size_t index); + [[nodiscard]] pp::foundation::Status move_frame(std::size_t from, std::size_t to); [[nodiscard]] pp::foundation::Status set_frame_duration(std::size_t index, std::uint32_t duration_ms) noexcept; [[nodiscard]] pp::foundation::Status set_active_frame(std::size_t index) noexcept; diff --git a/tests/document/document_tests.cpp b/tests/document/document_tests.cpp index bbde62f..fa66d5b 100644 --- a/tests/document/document_tests.cpp +++ b/tests/document/document_tests.cpp @@ -31,6 +31,7 @@ void creates_document_with_default_layers(pp::tests::Harness& h) PP_EXPECT(h, document.value().active_layer_index() == 0U); PP_EXPECT(h, document.value().frames().size() == 1U); PP_EXPECT(h, document.value().frames()[0].duration_ms == 100U); + PP_EXPECT(h, document.value().animation_duration_ms() == 100U); PP_EXPECT(h, document.value().active_frame_index() == 0U); } @@ -173,10 +174,45 @@ void manages_animation_frames_and_duration(pp::tests::Harness& h) PP_EXPECT(h, document.frames()[2].duration_ms == 250U); PP_EXPECT(h, document.set_frame_duration(2, 333).ok()); PP_EXPECT(h, document.frames()[2].duration_ms == 333U); + PP_EXPECT(h, document.animation_duration_ms() == 683U); PP_EXPECT(h, document.remove_frame(1).ok()); PP_EXPECT(h, document.frames().size() == 2U); PP_EXPECT(h, document.active_frame_index() == 1U); + PP_EXPECT(h, document.animation_duration_ms() == 433U); +} + +void moves_frames_and_preserves_active_frame_identity(pp::tests::Harness& h) +{ + auto document_result = CanvasDocument::create( + DocumentConfig { .width = 64, .height = 64, .layer_count = 1 }); + PP_EXPECT(h, document_result.ok()); + auto document = document_result.value(); + + PP_EXPECT(h, document.set_frame_duration(0, 100).ok()); + PP_EXPECT(h, document.add_frame(200).ok()); + PP_EXPECT(h, document.add_frame(300).ok()); + PP_EXPECT(h, document.add_frame(400).ok()); + + PP_EXPECT(h, document.set_active_frame(2).ok()); + PP_EXPECT(h, document.move_frame(2, 0).ok()); + PP_EXPECT(h, document.active_frame_index() == 0U); + PP_EXPECT(h, document.frames()[0].duration_ms == 300U); + PP_EXPECT(h, document.frames()[1].duration_ms == 100U); + PP_EXPECT(h, document.frames()[2].duration_ms == 200U); + PP_EXPECT(h, document.frames()[3].duration_ms == 400U); + + PP_EXPECT(h, document.move_frame(3, 1).ok()); + PP_EXPECT(h, document.active_frame_index() == 0U); + PP_EXPECT(h, document.frames()[1].duration_ms == 400U); + PP_EXPECT(h, document.animation_duration_ms() == 1000U); + + const auto missing_from = document.move_frame(9, 0); + const auto missing_to = document.move_frame(0, 9); + PP_EXPECT(h, !missing_from.ok()); + PP_EXPECT(h, missing_from.code == StatusCode::out_of_range); + PP_EXPECT(h, !missing_to.ok()); + PP_EXPECT(h, missing_to.code == StatusCode::out_of_range); } void rejects_invalid_animation_frame_operations(pp::tests::Harness& h) @@ -331,6 +367,7 @@ int main() harness.run("updates_layer_metadata", updates_layer_metadata); harness.run("rejects_invalid_layer_metadata", rejects_invalid_layer_metadata); harness.run("manages_animation_frames_and_duration", manages_animation_frames_and_duration); + harness.run("moves_frames_and_preserves_active_frame_identity", moves_frames_and_preserves_active_frame_identity); harness.run("rejects_invalid_animation_frame_operations", rejects_invalid_animation_frame_operations); harness.run("records_document_history_and_restores_snapshots", records_document_history_and_restores_snapshots); harness.run("applying_after_undo_discards_redo_branch", applying_after_undo_discards_redo_branch);