From aefbf4a111a033631531ff01b606284d0f061607 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 10:13:44 -0400 Subject: [PATCH 01/15] test(packing): add F2 PackCharts suite (TDD red) + header scaffold Add TestChartPacking.cpp covering edge cases, absolute/normalize scaling, non-overlap, extent, padding, and an end-to-end tear/extract/flatten/pack test with per-wedge recovery via the back-maps. ChartPacking.hpp provides PackOptions/PackResult and a stubbed PackCharts so the suite compiles and fails (red); implementation follows in Phase 3. Co-Authored-By: Claude Opus 4.8 --- conductor/tracks/F2/plan.md | 14 +- include/OpenABF/ChartPacking.hpp | 144 ++++++++++++++++ tests/CMakeLists.txt | 1 + tests/src/TestChartPacking.cpp | 288 +++++++++++++++++++++++++++++++ 4 files changed, 440 insertions(+), 7 deletions(-) create mode 100644 include/OpenABF/ChartPacking.hpp create mode 100644 tests/src/TestChartPacking.cpp diff --git a/conductor/tracks/F2/plan.md b/conductor/tracks/F2/plan.md index 011c8d5..9c15e78 100644 --- a/conductor/tracks/F2/plan.md +++ b/conductor/tracks/F2/plan.md @@ -11,13 +11,13 @@ Design resolved 2026-06-20 (see spec.md → Design Decisions). - [x] 1.6 Resolve edge cases: empty list no-op, zero-area placed, null/empty throws, Dim>=2 ## Phase 2: Tests (write first) -- [ ] 2.1 Synthetic 2D charts: bbox computation correctness (min/max per mesh) -- [ ] 2.2 Assert packed chart bounding boxes do not overlap (padding respected) -- [ ] 2.3 Assert absolute-mode preserves relative chart sizes (no per-chart distortion) -- [ ] 2.4 Assert normalize=true fits all UVs within [0,1]² via single global scale -- [ ] 2.5 Assert returned PackResult extent bounds all packed charts -- [ ] 2.6 Degenerate cases: empty list, single chart, zero-area chart, null/empty throw -- [ ] 2.7 End-to-end: tear → extract_connected_components → LSCM → PackCharts, and +- [x] 2.1 Synthetic 2D charts: bbox computation correctness (min/max per mesh) +- [x] 2.2 Assert packed chart bounding boxes do not overlap (padding respected) +- [x] 2.3 Assert absolute-mode preserves relative chart sizes (no per-chart distortion) +- [x] 2.4 Assert normalize=true fits all UVs within [0,1]² via single global scale +- [x] 2.5 Assert returned PackResult extent bounds all packed charts +- [x] 2.6 Degenerate cases: empty list, single chart, zero-area chart, null/empty throw +- [x] 2.7 End-to-end: tear → extract_connected_components → LSCM → PackCharts, and verify per-wedge recovery via (face_map[f], vertex_map[corner.vertex.idx]) ## Phase 3: Implementation diff --git a/include/OpenABF/ChartPacking.hpp b/include/OpenABF/ChartPacking.hpp new file mode 100644 index 0000000..641b924 --- /dev/null +++ b/include/OpenABF/ChartPacking.hpp @@ -0,0 +1,144 @@ +/* +OpenABF +https://gitlab.com/educelab/OpenABF + +Copyright 2025 EduceLab + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +#pragma once + +#include +#include +#include +#include + +#include "OpenABF/Vec.hpp" + +namespace OpenABF +{ + +namespace detail +{ +/** @brief Deduce the dimensionality of a Vec type */ +template +struct VecDimensions; + +template +struct VecDimensions> { + static constexpr std::size_t value = N; +}; +} // namespace detail + +/** + * @brief Options controlling PackCharts behavior + * + * @tparam T Floating-point scalar type + */ +template +struct PackOptions { + /** + * @brief Fit the packed atlas into the unit square `[0,1]^2` + * + * When `false` (default), charts keep their absolute (physical) scale and + * are only translated. When `true`, a single global uniform scale is + * applied after layout so the whole atlas fits in `[0,1]^2`. A single + * global factor preserves relative chart sizes and cross-chart texel + * density; only the absolute units change. + */ + bool normalize{false}; + + /** + * @brief Target shelf width; overrides the `sqrt(total area)` heuristic + * + * Charts wrap to a new shelf when a row would exceed this width. If unset, + * the width defaults to `sqrt(sum of per-chart bounding-box areas)`, which + * yields a roughly square atlas. + */ + std::optional target_width{}; + + /** @brief Gutter added around each chart, in chart/absolute units */ + T padding{T(0)}; +}; + +/** + * @brief The bounding box of the packed atlas + * + * @tparam T Floating-point scalar type + */ +template +struct PackResult { + /** @brief Lower corner of the packed atlas */ + Vec min; + /** @brief Upper corner of the packed atlas */ + Vec max; +}; + +/** + * @brief Pack a set of parameterized charts into a shared coordinate frame + * + * Lays out a list of already-parameterized charts (2D meshes whose vertex + * `pos` holds `{u, v, ...}`) into a single shared frame using shelf packing, + * so that no two charts' bounding boxes overlap. Operates purely on geometry: + * each chart's vertex positions are translated (and, when `normalize` is set, + * uniformly scaled) **in place**. Topology, vertex indices, and face indices + * are untouched, so any `ExtractedComponent` back-maps a caller holds remain + * valid after packing. + * + * @par Scaling + * By default charts keep their absolute scale and are only translated; the + * returned extent is meaningful in physical units. With `opts.normalize`, one + * global uniform scale maps the packed atlas into `[0,1]^2`. + * + * @par Building a per-wedge UV map + * This function does not own a UV-map type. To build a per-corner ("wedge") + * UV map from the packed charts, key each wedge by **vertex identity**, not by + * corner position: a face's corner order is not stable (the half-edge mesh may + * reverse a mis-wound face at insertion time, and that permutation is not + * recorded). Given an `ExtractedComponent` `ec` for a chart, the robust key is: + * @code + * for (const auto& face : chart->faces()) { + * for (const auto& edge : *face) { // walk the face's corners + * auto origFace = ec.face_map[face->idx]; + * auto origVert = ec.vertex_map[edge->vertex->idx]; + * auto uv = edge->vertex->pos; // packed UV + * // wedge (origFace, origVert) -> uv + * } + * } + * @endcode + * + * @tparam MeshType A HalfEdgeMesh specialization + * @param charts Charts to pack; each chart's vertex positions are modified + * @param opts Packing options + * @return The bounding box of the packed atlas + * + * @throws std::invalid_argument If a chart pointer is null or a chart has no + * vertices. + */ +template +auto PackCharts(std::vector& charts, + const PackOptions& opts = + PackOptions{}) -> PackResult +{ + using T = typename MeshType::type; + static_assert( + detail::VecDimensions().pos)>::value >= 2, + "PackCharts requires mesh vertices with at least 2 position dimensions"); + + // STUB (TDD red phase): real layout is implemented in Phase 3. + (void)charts; + (void)opts; + return PackResult{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; +} + +} // namespace OpenABF diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9287893..e381070 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,6 +16,7 @@ set(tests src/TestParameterization.cpp src/TestMeshIOUtils.cpp src/TestMeshIO.cpp + src/TestChartPacking.cpp ) foreach(src ${tests}) diff --git a/tests/src/TestChartPacking.cpp b/tests/src/TestChartPacking.cpp new file mode 100644 index 0000000..d9d3c43 --- /dev/null +++ b/tests/src/TestChartPacking.cpp @@ -0,0 +1,288 @@ +#include +#include +#include +#include +#include + +#include + +#include "OpenABF/ChartPacking.hpp" +#include "OpenABF/OpenABF.hpp" + +using Mesh = OpenABF::HalfEdgeMesh; +using OpenABF::PackCharts; +using OpenABF::PackOptions; +using OpenABF::PackResult; + +namespace +{ + +/** @brief Build a w x h rectangle chart (two triangles) at the origin */ +auto MakeRectChart(float w, float h) -> Mesh::Pointer +{ + auto m = Mesh::New(); + m->insert_vertices({{0.f, 0.f, 0.f}, {w, 0.f, 0.f}, {w, h, 0.f}, {0.f, h, 0.f}}); + m->insert_faces({{0, 1, 2}, {0, 2, 3}}); + return m; +} + +struct BBox { + float minx{std::numeric_limits::max()}; + float miny{std::numeric_limits::max()}; + float maxx{std::numeric_limits::lowest()}; + float maxy{std::numeric_limits::lowest()}; + [[nodiscard]] auto width() const -> float { return maxx - minx; } + [[nodiscard]] auto height() const -> float { return maxy - miny; } +}; + +template +auto ChartBBox(const MeshPtr& m) -> BBox +{ + BBox b; + for (const auto& v : m->vertices()) { + b.minx = std::min(b.minx, v->pos[0]); + b.miny = std::min(b.miny, v->pos[1]); + b.maxx = std::max(b.maxx, v->pos[0]); + b.maxy = std::max(b.maxy, v->pos[1]); + } + return b; +} + +/** @brief True if two boxes overlap by more than eps (touching is allowed) */ +auto Overlaps(const BBox& a, const BBox& b, float eps) -> bool +{ + return (a.minx + eps < b.maxx) && (b.minx + eps < a.maxx) && (a.miny + eps < b.maxy) && + (b.miny + eps < a.maxy); +} + +} // namespace + +// --- Edge cases ------------------------------------------------------------ + +TEST(ChartPacking, EmptyListReturnsEmptyExtent) +{ + std::vector charts; + auto extent = PackCharts(charts); + EXPECT_FLOAT_EQ(extent.min[0], 0.f); + EXPECT_FLOAT_EQ(extent.min[1], 0.f); + EXPECT_FLOAT_EQ(extent.max[0], 0.f); + EXPECT_FLOAT_EQ(extent.max[1], 0.f); +} + +TEST(ChartPacking, NullChartThrows) +{ + std::vector charts{MakeRectChart(1.f, 1.f), nullptr}; + EXPECT_THROW(PackCharts(charts), std::invalid_argument); +} + +TEST(ChartPacking, EmptyChartThrows) +{ + std::vector charts{MakeRectChart(1.f, 1.f), Mesh::New()}; + EXPECT_THROW(PackCharts(charts), std::invalid_argument); +} + +TEST(ChartPacking, ZeroAreaChartIsPlacedWithoutThrowing) +{ + // A flat (collinear) chart has zero area but is a valid mesh: distinct + // vertices, non-zero edges. PackCharts must place it without crashing. + auto flat = Mesh::New(); + flat->insert_vertices({{0.f, 0.f, 0.f}, {1.f, 0.f, 0.f}, {2.f, 0.f, 0.f}}); + flat->insert_faces({{0, 1, 2}}); + std::vector charts{MakeRectChart(1.f, 1.f), flat}; + EXPECT_NO_THROW(PackCharts(charts)); +} + +// --- Absolute (default) mode ---------------------------------------------- + +TEST(ChartPacking, SingleChartMapsMinToOrigin) +{ + std::vector charts{MakeRectChart(3.f, 2.f)}; + auto extent = PackCharts(charts); + auto b = ChartBBox(charts[0]); + EXPECT_FLOAT_EQ(b.minx, 0.f); + EXPECT_FLOAT_EQ(b.miny, 0.f); + EXPECT_FLOAT_EQ(b.width(), 3.f); + EXPECT_FLOAT_EQ(b.height(), 2.f); + EXPECT_FLOAT_EQ(extent.max[0], 3.f); + EXPECT_FLOAT_EQ(extent.max[1], 2.f); +} + +TEST(ChartPacking, AbsoluteModePreservesChartSizes) +{ + std::vector ws{1.f, 2.f, 0.5f, 3.f}; + std::vector hs{1.f, 1.5f, 2.f, 0.7f}; + std::vector charts; + for (std::size_t i = 0; i < ws.size(); ++i) { + charts.push_back(MakeRectChart(ws[i], hs[i])); + } + PackCharts(charts); + for (std::size_t i = 0; i < charts.size(); ++i) { + auto b = ChartBBox(charts[i]); + EXPECT_FLOAT_EQ(b.width(), ws[i]) << "chart " << i; + EXPECT_FLOAT_EQ(b.height(), hs[i]) << "chart " << i; + } +} + +TEST(ChartPacking, ChartsDoNotOverlap) +{ + std::vector charts; + for (int i = 0; i < 6; ++i) { + charts.push_back(MakeRectChart(1.f + 0.3f * static_cast(i), 1.f)); + } + PackCharts(charts); + std::vector boxes; + for (const auto& c : charts) { + boxes.push_back(ChartBBox(c)); + } + for (std::size_t i = 0; i < boxes.size(); ++i) { + for (std::size_t j = i + 1; j < boxes.size(); ++j) { + EXPECT_FALSE(Overlaps(boxes[i], boxes[j], 1e-4f)) << "charts " << i << " and " << j; + } + } +} + +TEST(ChartPacking, ExtentBoundsAllCharts) +{ + std::vector charts{MakeRectChart(1.f, 1.f), MakeRectChart(2.f, 0.5f), + MakeRectChart(0.5f, 3.f)}; + auto extent = PackCharts(charts); + for (const auto& c : charts) { + for (const auto& v : c->vertices()) { + EXPECT_GE(v->pos[0], extent.min[0] - 1e-4f); + EXPECT_GE(v->pos[1], extent.min[1] - 1e-4f); + EXPECT_LE(v->pos[0], extent.max[0] + 1e-4f); + EXPECT_LE(v->pos[1], extent.max[1] + 1e-4f); + } + } +} + +TEST(ChartPacking, PaddingSeparatesChartsInSingleRow) +{ + // Force a single row with a large target width; padding should appear as a + // gap between the two charts, so the atlas width is w0 + padding + w1. + std::vector charts{MakeRectChart(1.f, 1.f), MakeRectChart(1.f, 1.f)}; + PackOptions opts; + opts.target_width = 1000.f; + opts.padding = 0.5f; + auto extent = PackCharts(charts, opts); + EXPECT_NEAR(extent.max[0] - extent.min[0], 2.5f, 1e-4f); + + auto b0 = ChartBBox(charts[0]); + auto b1 = ChartBBox(charts[1]); + auto gap = std::max(b1.minx - b0.maxx, b0.minx - b1.maxx); + EXPECT_GE(gap, 0.5f - 1e-4f); +} + +// --- Normalize mode -------------------------------------------------------- + +TEST(ChartPacking, NormalizeFitsUnitSquare) +{ + std::vector charts{MakeRectChart(2.f, 1.f), MakeRectChart(1.f, 3.f), + MakeRectChart(0.5f, 0.5f)}; + PackOptions opts; + opts.normalize = true; + auto extent = PackCharts(charts, opts); + + float maxCoord = 0.f; + for (const auto& c : charts) { + for (const auto& v : c->vertices()) { + EXPECT_GE(v->pos[0], -1e-4f); + EXPECT_GE(v->pos[1], -1e-4f); + EXPECT_LE(v->pos[0], 1.f + 1e-4f); + EXPECT_LE(v->pos[1], 1.f + 1e-4f); + maxCoord = std::max({maxCoord, v->pos[0], v->pos[1]}); + } + } + // The atlas should fill at least one axis of the unit square. + EXPECT_NEAR(maxCoord, 1.f, 1e-3f); + EXPECT_LE(extent.max[0], 1.f + 1e-4f); + EXPECT_LE(extent.max[1], 1.f + 1e-4f); +} + +TEST(ChartPacking, NormalizePreservesRelativeChartSizes) +{ + // One chart twice the linear size of the other: ratio must survive a single + // global uniform scale. + std::vector charts{MakeRectChart(1.f, 1.f), MakeRectChart(2.f, 2.f)}; + PackOptions opts; + opts.normalize = true; + PackCharts(charts, opts); + auto b0 = ChartBBox(charts[0]); + auto b1 = ChartBBox(charts[1]); + EXPECT_NEAR(b1.width() / b0.width(), 2.f, 1e-3f); + EXPECT_NEAR(b1.height() / b0.height(), 2.f, 1e-3f); +} + +// --- End-to-end pipeline + per-wedge recovery ------------------------------ + +TEST(ChartPacking, EndToEndTearExtractFlattenPackAndWedgeRecovery) +{ + using ABF = OpenABF::ABFPlusPlus; + using LSCM = OpenABF::AngleBasedLSCM; + + // 3x3 grid (matches the MultiChartFlatten example). + auto mesh = ABF::Mesh::New(); + mesh->insert_vertices({ + {0.f, 0.f, 0.f}, + {1.f, 0.f, 0.f}, + {2.f, 0.f, 0.f}, + {0.f, 1.f, 0.f}, + {1.f, 1.f, 0.f}, + {2.f, 1.f, 0.f}, + {0.f, 2.f, 0.f}, + {1.f, 2.f, 0.f}, + {2.f, 2.f, 0.f}, + }); + mesh->insert_faces({ + {0, 3, 1}, + {1, 3, 4}, + {1, 4, 2}, + {2, 4, 5}, + {3, 6, 4}, + {4, 6, 7}, + {4, 7, 5}, + {5, 7, 8}, + }); + + // Tear into two charts and extract. + mesh->split_path({1, 4, 7}); + auto ccs = mesh->extract_connected_components(); + ASSERT_EQ(ccs.size(), 2u); + + // Flatten each chart and collect the meshes for packing. + std::vector chartMeshes; + for (auto& cc : ccs) { + std::size_t iters{0}; + float grad{OpenABF::INF}; + ABF::Compute(cc.mesh, iters, grad); + LSCM::Compute(cc.mesh); + chartMeshes.push_back(cc.mesh); + } + + auto extent = PackCharts(chartMeshes); + + // Charts must not overlap after packing. + std::vector boxes; + for (const auto& c : chartMeshes) { + boxes.push_back(ChartBBox(c)); + } + EXPECT_FALSE(Overlaps(boxes[0], boxes[1], 1e-4f)); + + // Per-wedge recovery via the back-maps, keyed on vertex identity. Every + // (original face, original vertex) wedge must be unique, proving the + // documented recipe yields a well-formed per-wedge map. + std::set> wedges; + std::size_t corners = 0; + for (auto& cc : ccs) { + for (const auto& face : cc.mesh->faces()) { + for (const auto& edge : *face) { + auto origFace = cc.face_map[face->idx]; + auto origVert = cc.vertex_map[edge->vertex->idx]; + wedges.emplace(origFace, origVert); + ++corners; + } + } + } + EXPECT_EQ(corners, 8u * 3u); // 8 faces, 3 corners each + EXPECT_EQ(wedges.size(), corners); +} From 9612c612c69b34084762b58b6657eea9c4acc276 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 10:17:45 -0400 Subject: [PATCH 02/15] feat(packing): implement PackCharts shelf packing (TDD green) Implement multi-chart UV packing as a geometry-only free function that translates (and optionally uniformly scales) each chart's vertex positions in place. Charts are laid out with shelf packing using a sqrt-area target width (overridable); absolute scale is preserved by default with opt-in normalize to [0,1]^2 via a single global scale. Returns the packed atlas extent. Documents the vertex-identity per-wedge recipe and complexity. Wire ChartPacking.hpp into OpenABF.hpp and regenerate the single header. All 12 PackCharts tests pass (full suite 7/7). Co-Authored-By: Claude Opus 4.8 --- conductor/tracks/F2/plan.md | 21 +-- include/OpenABF/ChartPacking.hpp | 112 ++++++++++++- include/OpenABF/OpenABF.hpp | 2 + single_include/OpenABF/OpenABF.hpp | 252 +++++++++++++++++++++++++++++ 4 files changed, 373 insertions(+), 14 deletions(-) diff --git a/conductor/tracks/F2/plan.md b/conductor/tracks/F2/plan.md index 9c15e78..f86fe03 100644 --- a/conductor/tracks/F2/plan.md +++ b/conductor/tracks/F2/plan.md @@ -21,15 +21,16 @@ Design resolved 2026-06-20 (see spec.md → Design Decisions). verify per-wedge recovery via (face_map[f], vertex_map[corner.vertex.idx]) ## Phase 3: Implementation -- [ ] 3.1 Create `include/OpenABF/ChartPacking.hpp` with PackOptions, PackResult -- [ ] 3.2 Implement per-chart bbox + sqrt-area target width + shelf placement -- [ ] 3.3 Implement absolute (translate-only) and normalize (global uniform scale) modes -- [ ] 3.4 Implement padding, degenerate-input handling, static_assert(Dim>=2) -- [ ] 3.5 Document the vertex-identity per-wedge recipe in the header + complexity notes -- [ ] 3.6 Add include to `include/OpenABF/OpenABF.hpp` -- [ ] 3.7 Update `single_include.json` and run amalgamation script +- [x] 3.1 Create `include/OpenABF/ChartPacking.hpp` with PackOptions, PackResult +- [x] 3.2 Implement per-chart bbox + sqrt-area target width + shelf placement +- [x] 3.3 Implement absolute (translate-only) and normalize (global uniform scale) modes +- [x] 3.4 Implement padding, degenerate-input handling, static_assert(Dim>=2) +- [x] 3.5 Document the vertex-identity per-wedge recipe in the header + complexity notes +- [x] 3.6 Add include to `include/OpenABF/OpenABF.hpp` +- [x] 3.7 Update single-header via amalgamation script (single_include.json unchanged — + it already tracks OpenABF.hpp transitively) ## Phase 4: Verify -- [ ] 4.1 Run `ctest` — all tests pass -- [ ] 4.2 Run clang-format on changed files -- [ ] 4.3 Confirm single-header build matches multi-header +- [x] 4.1 Run `ctest` — all 7 suites pass (incl. new OpenABF_TestChartPacking) +- [x] 4.2 Run clang-format on changed files +- [x] 4.3 Confirm single-header build compiles and runs diff --git a/include/OpenABF/ChartPacking.hpp b/include/OpenABF/ChartPacking.hpp index 641b924..e5c73db 100644 --- a/include/OpenABF/ChartPacking.hpp +++ b/include/OpenABF/ChartPacking.hpp @@ -18,8 +18,13 @@ limitations under the License. */ #pragma once +#include +#include #include +#include +#include #include +#include #include #include @@ -117,6 +122,11 @@ struct PackResult { * } * @endcode * + * @par Complexity + * `O(n log n)` in the number of charts `n` (dominated by the height sort) plus + * `O(V)` in the total vertex count `V` (two passes: one to measure bounding + * boxes, one to apply the transform). Memory overhead is `O(n)`. + * * @tparam MeshType A HalfEdgeMesh specialization * @param charts Charts to pack; each chart's vertex positions are modified * @param opts Packing options @@ -135,10 +145,104 @@ auto PackCharts(std::vector& charts, detail::VecDimensions().pos)>::value >= 2, "PackCharts requires mesh vertices with at least 2 position dimensions"); - // STUB (TDD red phase): real layout is implemented in Phase 3. - (void)charts; - (void)opts; - return PackResult{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; + PackResult result{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; + if (charts.empty()) { + return result; + } + const std::size_t n = charts.size(); + + // Validate inputs and compute each chart's 2D bounding box (origin + size). + std::vector minX(n); + std::vector minY(n); + std::vector width(n); + std::vector height(n); + for (std::size_t i = 0; i < n; ++i) { + const auto& chart = charts[i]; + if (not chart or chart->num_vertices() == 0) { + throw std::invalid_argument("PackCharts: chart is null or has no vertices"); + } + auto mnX = std::numeric_limits::max(); + auto mnY = std::numeric_limits::max(); + auto mxX = std::numeric_limits::lowest(); + auto mxY = std::numeric_limits::lowest(); + for (const auto& v : chart->vertices()) { + mnX = std::min(mnX, v->pos[0]); + mnY = std::min(mnY, v->pos[1]); + mxX = std::max(mxX, v->pos[0]); + mxY = std::max(mxY, v->pos[1]); + } + minX[i] = mnX; + minY[i] = mnY; + width[i] = mxX - mnX; + height[i] = mxY - mnY; + } + + // Place taller charts first so shelves pack tightly. + std::vector order(n); + std::iota(order.begin(), order.end(), std::size_t{0}); + std::sort(order.begin(), order.end(), + [&](std::size_t a, std::size_t b) { return height[a] > height[b]; }); + + // Target shelf width: caller override, else sqrt(sum of padded chart areas) + // for a roughly square atlas. + const T pad = opts.padding; + T targetWidth; + if (opts.target_width) { + targetWidth = *opts.target_width; + } else { + auto areaSum = T(0); + for (std::size_t i = 0; i < n; ++i) { + areaSum += (width[i] + pad) * (height[i] + pad); + } + targetWidth = std::sqrt(areaSum); + } + + // Shelf layout. Charts are placed left-to-right; a row wraps to a new shelf + // once it would exceed targetWidth (a chart wider than targetWidth still + // gets placed alone at the start of a shelf). Every chart's bbox-min is + // mapped to a cursor position >= 0, so the packed atlas's lower corner is + // the origin. + std::vector offsetX(n); + std::vector offsetY(n); + auto cursorX = T(0); + auto cursorY = T(0); + auto shelfHeight = T(0); + auto atlasMaxX = T(0); + auto atlasMaxY = T(0); + for (const auto i : order) { + if (cursorX > T(0) and cursorX + width[i] > targetWidth) { + cursorX = T(0); + cursorY += shelfHeight + pad; + shelfHeight = T(0); + } + offsetX[i] = cursorX - minX[i]; + offsetY[i] = cursorY - minY[i]; + atlasMaxX = std::max(atlasMaxX, cursorX + width[i]); + atlasMaxY = std::max(atlasMaxY, cursorY + height[i]); + cursorX += width[i] + pad; + shelfHeight = std::max(shelfHeight, height[i]); + } + + // Optional normalization: a single global uniform scale that fits the + // packed atlas into [0,1]^2, preserving relative chart sizes. + auto scale = T(1); + if (opts.normalize) { + const auto extentMax = std::max(atlasMaxX, atlasMaxY); + if (extentMax > T(0)) { + scale = T(1) / extentMax; + } + } + + // Apply translation (+ optional scale about the origin) in place. + for (std::size_t i = 0; i < n; ++i) { + for (const auto& v : charts[i]->vertices()) { + v->pos[0] = (v->pos[0] + offsetX[i]) * scale; + v->pos[1] = (v->pos[1] + offsetY[i]) * scale; + } + } + + result.max = Vec{atlasMaxX * scale, atlasMaxY * scale}; + return result; } } // namespace OpenABF diff --git a/include/OpenABF/OpenABF.hpp b/include/OpenABF/OpenABF.hpp index 89e16b1..23754bb 100644 --- a/include/OpenABF/OpenABF.hpp +++ b/include/OpenABF/OpenABF.hpp @@ -30,5 +30,7 @@ limitations under the License. #include "OpenABF/AngleBasedLSCM.hpp" #include "OpenABF/HierarchicalLSCM.hpp" +#include "OpenABF/ChartPacking.hpp" + #include "OpenABF/MeshIO.hpp" // clang-format on diff --git a/single_include/OpenABF/OpenABF.hpp b/single_include/OpenABF/OpenABF.hpp index b21d488..c8ca501 100644 --- a/single_include/OpenABF/OpenABF.hpp +++ b/single_include/OpenABF/OpenABF.hpp @@ -4893,6 +4893,258 @@ class HierarchicalLSCM } // namespace OpenABF +// #include "OpenABF/ChartPacking.hpp" +/* +OpenABF +https://gitlab.com/educelab/OpenABF + +Copyright 2025 EduceLab + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// #include "OpenABF/Vec.hpp" + + +namespace OpenABF +{ + +namespace detail +{ +/** @brief Deduce the dimensionality of a Vec type */ +template +struct VecDimensions; + +template +struct VecDimensions> { + static constexpr std::size_t value = N; +}; +} // namespace detail + +/** + * @brief Options controlling PackCharts behavior + * + * @tparam T Floating-point scalar type + */ +template +struct PackOptions { + /** + * @brief Fit the packed atlas into the unit square `[0,1]^2` + * + * When `false` (default), charts keep their absolute (physical) scale and + * are only translated. When `true`, a single global uniform scale is + * applied after layout so the whole atlas fits in `[0,1]^2`. A single + * global factor preserves relative chart sizes and cross-chart texel + * density; only the absolute units change. + */ + bool normalize{false}; + + /** + * @brief Target shelf width; overrides the `sqrt(total area)` heuristic + * + * Charts wrap to a new shelf when a row would exceed this width. If unset, + * the width defaults to `sqrt(sum of per-chart bounding-box areas)`, which + * yields a roughly square atlas. + */ + std::optional target_width{}; + + /** @brief Gutter added around each chart, in chart/absolute units */ + T padding{T(0)}; +}; + +/** + * @brief The bounding box of the packed atlas + * + * @tparam T Floating-point scalar type + */ +template +struct PackResult { + /** @brief Lower corner of the packed atlas */ + Vec min; + /** @brief Upper corner of the packed atlas */ + Vec max; +}; + +/** + * @brief Pack a set of parameterized charts into a shared coordinate frame + * + * Lays out a list of already-parameterized charts (2D meshes whose vertex + * `pos` holds `{u, v, ...}`) into a single shared frame using shelf packing, + * so that no two charts' bounding boxes overlap. Operates purely on geometry: + * each chart's vertex positions are translated (and, when `normalize` is set, + * uniformly scaled) **in place**. Topology, vertex indices, and face indices + * are untouched, so any `ExtractedComponent` back-maps a caller holds remain + * valid after packing. + * + * @par Scaling + * By default charts keep their absolute scale and are only translated; the + * returned extent is meaningful in physical units. With `opts.normalize`, one + * global uniform scale maps the packed atlas into `[0,1]^2`. + * + * @par Building a per-wedge UV map + * This function does not own a UV-map type. To build a per-corner ("wedge") + * UV map from the packed charts, key each wedge by **vertex identity**, not by + * corner position: a face's corner order is not stable (the half-edge mesh may + * reverse a mis-wound face at insertion time, and that permutation is not + * recorded). Given an `ExtractedComponent` `ec` for a chart, the robust key is: + * @code + * for (const auto& face : chart->faces()) { + * for (const auto& edge : *face) { // walk the face's corners + * auto origFace = ec.face_map[face->idx]; + * auto origVert = ec.vertex_map[edge->vertex->idx]; + * auto uv = edge->vertex->pos; // packed UV + * // wedge (origFace, origVert) -> uv + * } + * } + * @endcode + * + * @par Complexity + * `O(n log n)` in the number of charts `n` (dominated by the height sort) plus + * `O(V)` in the total vertex count `V` (two passes: one to measure bounding + * boxes, one to apply the transform). Memory overhead is `O(n)`. + * + * @tparam MeshType A HalfEdgeMesh specialization + * @param charts Charts to pack; each chart's vertex positions are modified + * @param opts Packing options + * @return The bounding box of the packed atlas + * + * @throws std::invalid_argument If a chart pointer is null or a chart has no + * vertices. + */ +template +auto PackCharts(std::vector& charts, + const PackOptions& opts = + PackOptions{}) -> PackResult +{ + using T = typename MeshType::type; + static_assert( + detail::VecDimensions().pos)>::value >= 2, + "PackCharts requires mesh vertices with at least 2 position dimensions"); + + PackResult result{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; + if (charts.empty()) { + return result; + } + const std::size_t n = charts.size(); + + // Validate inputs and compute each chart's 2D bounding box (origin + size). + std::vector minX(n); + std::vector minY(n); + std::vector width(n); + std::vector height(n); + for (std::size_t i = 0; i < n; ++i) { + const auto& chart = charts[i]; + if (not chart or chart->num_vertices() == 0) { + throw std::invalid_argument("PackCharts: chart is null or has no vertices"); + } + auto mnX = std::numeric_limits::max(); + auto mnY = std::numeric_limits::max(); + auto mxX = std::numeric_limits::lowest(); + auto mxY = std::numeric_limits::lowest(); + for (const auto& v : chart->vertices()) { + mnX = std::min(mnX, v->pos[0]); + mnY = std::min(mnY, v->pos[1]); + mxX = std::max(mxX, v->pos[0]); + mxY = std::max(mxY, v->pos[1]); + } + minX[i] = mnX; + minY[i] = mnY; + width[i] = mxX - mnX; + height[i] = mxY - mnY; + } + + // Place taller charts first so shelves pack tightly. + std::vector order(n); + std::iota(order.begin(), order.end(), std::size_t{0}); + std::sort(order.begin(), order.end(), + [&](std::size_t a, std::size_t b) { return height[a] > height[b]; }); + + // Target shelf width: caller override, else sqrt(sum of padded chart areas) + // for a roughly square atlas. + const T pad = opts.padding; + T targetWidth; + if (opts.target_width) { + targetWidth = *opts.target_width; + } else { + auto areaSum = T(0); + for (std::size_t i = 0; i < n; ++i) { + areaSum += (width[i] + pad) * (height[i] + pad); + } + targetWidth = std::sqrt(areaSum); + } + + // Shelf layout. Charts are placed left-to-right; a row wraps to a new shelf + // once it would exceed targetWidth (a chart wider than targetWidth still + // gets placed alone at the start of a shelf). Every chart's bbox-min is + // mapped to a cursor position >= 0, so the packed atlas's lower corner is + // the origin. + std::vector offsetX(n); + std::vector offsetY(n); + auto cursorX = T(0); + auto cursorY = T(0); + auto shelfHeight = T(0); + auto atlasMaxX = T(0); + auto atlasMaxY = T(0); + for (const auto i : order) { + if (cursorX > T(0) and cursorX + width[i] > targetWidth) { + cursorX = T(0); + cursorY += shelfHeight + pad; + shelfHeight = T(0); + } + offsetX[i] = cursorX - minX[i]; + offsetY[i] = cursorY - minY[i]; + atlasMaxX = std::max(atlasMaxX, cursorX + width[i]); + atlasMaxY = std::max(atlasMaxY, cursorY + height[i]); + cursorX += width[i] + pad; + shelfHeight = std::max(shelfHeight, height[i]); + } + + // Optional normalization: a single global uniform scale that fits the + // packed atlas into [0,1]^2, preserving relative chart sizes. + auto scale = T(1); + if (opts.normalize) { + const auto extentMax = std::max(atlasMaxX, atlasMaxY); + if (extentMax > T(0)) { + scale = T(1) / extentMax; + } + } + + // Apply translation (+ optional scale about the origin) in place. + for (std::size_t i = 0; i < n; ++i) { + for (const auto& v : charts[i]->vertices()) { + v->pos[0] = (v->pos[0] + offsetX[i]) * scale; + v->pos[1] = (v->pos[1] + offsetY[i]) * scale; + } + } + + result.max = Vec{atlasMaxX * scale, atlasMaxY * scale}; + return result; +} + +} // namespace OpenABF + + // #include "OpenABF/MeshIO.hpp" From 3ef7a75404beaa0b9d2365050093a4848efb5360 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 10:20:06 -0400 Subject: [PATCH 03/15] fix(build): install ChartPacking.hpp in multiheader install set The multiheader install enumerates public headers explicitly; ChartPacking.hpp was missing, so the installed OpenABF.hpp failed to find it (CI install-test, Multiheader=ON). Add it to the install list. Co-Authored-By: Claude Opus 4.8 --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index d4f572e..67c547e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -29,6 +29,7 @@ if(OPENABF_MULTIHEADER) include/OpenABF/HalfEdgeMesh.hpp include/OpenABF/HierarchicalLSCM.hpp include/OpenABF/AngleBasedLSCM.hpp + include/OpenABF/ChartPacking.hpp include/OpenABF/Math.hpp include/OpenABF/Vec.hpp include/OpenABF/MeshIO.hpp From af2e52ca69af36904934a0b7777a77652f366a5a Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 10:23:45 -0400 Subject: [PATCH 04/15] fix(test): include only OpenABF.hpp in TestChartPacking The direct include of OpenABF/ChartPacking.hpp broke the single-header build (Multiheader=OFF), where only the amalgamated OpenABF.hpp exists. ChartPacking is included transitively via OpenABF.hpp, matching the other test files; drop the direct include. Co-Authored-By: Claude Opus 4.8 --- tests/src/TestChartPacking.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/src/TestChartPacking.cpp b/tests/src/TestChartPacking.cpp index d9d3c43..68aabce 100644 --- a/tests/src/TestChartPacking.cpp +++ b/tests/src/TestChartPacking.cpp @@ -6,7 +6,6 @@ #include -#include "OpenABF/ChartPacking.hpp" #include "OpenABF/OpenABF.hpp" using Mesh = OpenABF::HalfEdgeMesh; From 85290a87688e7438952224492f2a5da2e3caf08d Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 10:28:39 -0400 Subject: [PATCH 05/15] docs(example): pack multi-chart flatten output into a single atlas Update MultiChartFlatten to demonstrate PackCharts: flatten each connected component, pack the charts into a shared [0,1]^2 frame, merge them into one mesh, and write a single packed-atlas .obj instead of one file per chart. Co-Authored-By: Claude Opus 4.8 --- examples/src/MultiChartFlatten.cpp | 61 ++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index dd5b53d..09bc3fa 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -5,17 +5,20 @@ * * Builds a mesh with multiple connected components (here, a 3x3 grid torn * along two seams), extracts each component as an independent mesh, runs - * ABF++ + LSCM on each, and writes each flattened chart to its own .obj - * file. + * ABF++ + LSCM on each, packs the flattened charts into a shared coordinate + * frame with OpenABF::PackCharts, and writes the packed atlas to a single + * .obj file. * * The original mesh is never modified — the extracted sub-meshes own their - * own vertices and per-edge state, and each is parameterized in isolation. + * own vertices and per-edge state, and each is parameterized in isolation + * before being placed into the common frame. * * @see OpenABF::HalfEdgeMesh::split_path * @see OpenABF::HalfEdgeMesh::extract_connected_components + * @see OpenABF::PackCharts */ #include -#include +#include #include "OpenABF/OpenABF.hpp" @@ -23,9 +26,10 @@ int main() { using ABF = OpenABF::ABFPlusPlus; using LSCM = OpenABF::AngleBasedLSCM; + using Mesh = ABF::Mesh; // Build a 3x3 grid (9 vertices, 8 triangles) - auto mesh = ABF::Mesh::New(); + auto mesh = Mesh::New(); mesh->insert_vertices({ {0.f, 0.f, 0.f}, {1.f, 0.f, 0.f}, @@ -62,7 +66,8 @@ int main() auto charts = mesh->extract_connected_components(); std::cout << "Extracted " << charts.size() << " chart(s)\n"; - // Flatten each chart in isolation and write it out as its own .obj. + // Flatten each chart in isolation, collecting the parameterized meshes. + std::vector chartMeshes; for (std::size_t i = 0; i < charts.size(); ++i) { auto& cc = charts[i]; @@ -70,12 +75,10 @@ int main() float grad{OpenABF::INF}; ABF::Compute(cc.mesh, iters, grad); LSCM::Compute(cc.mesh); + chartMeshes.push_back(cc.mesh); - const auto out = "openabf_example_multi_chart_" + std::to_string(i) + ".obj"; - OpenABF::WriteMesh(out, cc.mesh); std::cout << "Chart " << i << ": " << cc.mesh->num_vertices() << " vertices, " - << cc.mesh->num_faces() << " faces, " << iters << " ABF++ iters -> " << out - << "\n"; + << cc.mesh->num_faces() << " faces, " << iters << " ABF++ iters\n"; // cc.vertex_map[chart_idx] -> original vertex idx // cc.face_map[chart_idx] -> original face idx @@ -83,8 +86,44 @@ int main() // keyed by source-mesh face corners. } + // Pack the flattened charts into a shared frame. `normalize` fits the whole + // atlas into [0,1]^2 via a single global uniform scale, which preserves the + // charts' relative sizes. Packing only edits each chart's 2D vertex + // positions in place; the per-chart vertex_map/face_map remain valid. + OpenABF::PackOptions opts; + opts.normalize = true; + auto extent = OpenABF::PackCharts(chartMeshes, opts); + std::cout << "Packed atlas extent: [" << extent.min[0] << ", " << extent.min[1] << "] -> [" + << extent.max[0] << ", " << extent.max[1] << "]\n"; + + // Merge the packed charts into a single mesh and write it as one atlas. + // The charts are disjoint in the packed frame, so concatenating their + // vertices and re-emitting their faces (with an index offset per chart) + // reconstructs a multi-component mesh in UV space. + auto packed = Mesh::New(); + std::size_t offset{0}; + for (const auto& chart : chartMeshes) { + for (const auto& v : chart->vertices()) { + packed->insert_vertex(v->pos[0], v->pos[1], v->pos[2]); + } + for (const auto& face : chart->faces()) { + std::vector idxs; + for (const auto& edge : *face) { + idxs.push_back(edge->vertex->idx + offset); + } + packed->insert_face(idxs); + } + offset += chart->num_vertices(); + } + + const std::string out = "openabf_example_multi_chart_packed.obj"; + OpenABF::WriteMesh(out, packed); + std::cout << "Wrote packed atlas: " << packed->num_vertices() << " vertices, " + << packed->num_faces() << " faces -> " << out << "\n"; + // The source mesh's 3D vertex positions are unchanged by the per-chart - // flattening — only the extracted sub-meshes hold the 2D UV result. + // flattening and packing — only the extracted sub-meshes hold the 2D UV + // result. std::cout << "Source mesh 3D positions intact: " << mesh->num_vertices() << " vertices, " << mesh->num_faces() << " faces\n"; } From 697b8975b9db89050be878f919e5a62198b090d8 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 13:40:58 -0400 Subject: [PATCH 06/15] feat(merge): add MergeMeshes, inverse of extract_connected_components Add MergeMeshes -> MergedMesh{mesh, vertex_source, face_source}: concatenates meshes into one and returns provenance maps (merged idx -> {input mesh, source idx}) so the back-map chain survives a merge (compose with each component's vertex_map/face_map to reach the torn source mesh). Preserves vertex positions/traits; edge/face traits are default-constructed. Switch the MultiChartFlatten example to use MergeMeshes instead of an inline concatenate that discarded provenance. Wire the header into OpenABF.hpp and the multiheader install set; regenerate the single header. New test suite TestMeshMerge covers counts, provenance, throws, and an extract->merge round-trip that recovers original face identity. Co-Authored-By: Claude Opus 4.8 --- CMakeLists.txt | 1 + conductor/tracks/F2/plan.md | 15 ++- examples/src/MultiChartFlatten.cpp | 28 ++---- include/OpenABF/MeshMerge.hpp | 103 ++++++++++++++++++++ include/OpenABF/OpenABF.hpp | 1 + single_include/OpenABF/OpenABF.hpp | 105 ++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/src/TestMeshMerge.cpp | 151 +++++++++++++++++++++++++++++ 8 files changed, 383 insertions(+), 22 deletions(-) create mode 100644 include/OpenABF/MeshMerge.hpp create mode 100644 tests/src/TestMeshMerge.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 67c547e..f3bd7bc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,6 +30,7 @@ if(OPENABF_MULTIHEADER) include/OpenABF/HierarchicalLSCM.hpp include/OpenABF/AngleBasedLSCM.hpp include/OpenABF/ChartPacking.hpp + include/OpenABF/MeshMerge.hpp include/OpenABF/Math.hpp include/OpenABF/Vec.hpp include/OpenABF/MeshIO.hpp diff --git a/conductor/tracks/F2/plan.md b/conductor/tracks/F2/plan.md index f86fe03..e94e121 100644 --- a/conductor/tracks/F2/plan.md +++ b/conductor/tracks/F2/plan.md @@ -31,6 +31,19 @@ Design resolved 2026-06-20 (see spec.md → Design Decisions). it already tracks OpenABF.hpp transitively) ## Phase 4: Verify -- [x] 4.1 Run `ctest` — all 7 suites pass (incl. new OpenABF_TestChartPacking) +- [x] 4.1 Run `ctest` — all suites pass (incl. OpenABF_TestChartPacking, OpenABF_TestMeshMerge) - [x] 4.2 Run clang-format on changed files - [x] 4.3 Confirm single-header build compiles and runs + +## Phase 5: MergeMeshes helper (added during review) +Rationale: the inline atlas merge in the example severs the back-map chain. +MergeMeshes is the inverse of extract_connected_components — it returns +provenance maps so merged → chart → M' composition keeps working. +- [x] 5.1 Tests first: concatenation counts, vertex/face provenance, null/empty + throw, round-trip extract→merge recovers original (torn-mesh) identity +- [x] 5.2 Implement `MergeMeshes` → `MergedMesh{mesh, vertex_source, face_source}` + in `include/OpenABF/MeshMerge.hpp` (preserves vertex traits/positions; + edge/face traits default-constructed) +- [x] 5.3 Wire into OpenABF.hpp + multiheader install list; regenerate single header +- [x] 5.4 Switch MultiChartFlatten example to use MergeMeshes +- [x] 5.5 Verify: full ctest, single-header build, install-test all pass diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index 09bc3fa..03d049c 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -97,29 +97,15 @@ int main() << extent.max[0] << ", " << extent.max[1] << "]\n"; // Merge the packed charts into a single mesh and write it as one atlas. - // The charts are disjoint in the packed frame, so concatenating their - // vertices and re-emitting their faces (with an index offset per chart) - // reconstructs a multi-component mesh in UV space. - auto packed = Mesh::New(); - std::size_t offset{0}; - for (const auto& chart : chartMeshes) { - for (const auto& v : chart->vertices()) { - packed->insert_vertex(v->pos[0], v->pos[1], v->pos[2]); - } - for (const auto& face : chart->faces()) { - std::vector idxs; - for (const auto& edge : *face) { - idxs.push_back(edge->vertex->idx + offset); - } - packed->insert_face(idxs); - } - offset += chart->num_vertices(); - } + // MergeMeshes returns provenance maps (vertex_source/face_source) that, when + // composed with each component's vertex_map/face_map, trace any atlas + // element back to the torn source mesh. + auto merged = OpenABF::MergeMeshes(chartMeshes); const std::string out = "openabf_example_multi_chart_packed.obj"; - OpenABF::WriteMesh(out, packed); - std::cout << "Wrote packed atlas: " << packed->num_vertices() << " vertices, " - << packed->num_faces() << " faces -> " << out << "\n"; + OpenABF::WriteMesh(out, merged.mesh); + std::cout << "Wrote packed atlas: " << merged.mesh->num_vertices() << " vertices, " + << merged.mesh->num_faces() << " faces -> " << out << "\n"; // The source mesh's 3D vertex positions are unchanged by the per-chart // flattening and packing — only the extracted sub-meshes hold the 2D UV diff --git a/include/OpenABF/MeshMerge.hpp b/include/OpenABF/MeshMerge.hpp new file mode 100644 index 0000000..f1efd53 --- /dev/null +++ b/include/OpenABF/MeshMerge.hpp @@ -0,0 +1,103 @@ +/* +OpenABF +https://gitlab.com/educelab/OpenABF + +Copyright 2025 EduceLab + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +#pragma once + +#include +#include +#include +#include + +namespace OpenABF +{ + +/** + * @brief Result of merging several meshes into one + * + * The inverse bookkeeping of `HalfEdgeMesh::extract_connected_components`: + * where extract splits one mesh into N with maps back to the source, merge + * combines N meshes into one with maps back to each source mesh. Compose + * `face_source`/`vertex_source` with the `face_map`/`vertex_map` of the + * components you merged to trace a merged element all the way back to the + * pre-extraction mesh. + * + * @tparam MeshType A HalfEdgeMesh specialization + */ +template +struct MergedMesh { + /** @brief The combined mesh */ + typename MeshType::Pointer mesh; + /** @brief `vertex_source[merged_idx] = {input mesh index, source vertex index}` */ + std::vector> vertex_source; + /** @brief `face_source[merged_idx] = {input mesh index, source face index}` */ + std::vector> face_source; +}; + +/** + * @brief Merge several meshes into a single mesh + * + * Concatenates the vertices and faces of each input mesh into one new mesh, + * offsetting face vertex indices per input so the inputs remain disjoint + * components. Returns the combined mesh alongside `vertex_source`/`face_source` + * maps that record, for every merged vertex and face, which input mesh and + * which source index it came from — the inverse of + * `extract_connected_components`, so the back-map chain survives the merge. + * + * Vertex positions and vertex traits are preserved (via the vertex copy + * constructor). Edge and face traits are default-constructed: merge rebuilds + * connectivity, so per-edge/per-face solver state is not carried over. + * + * @tparam MeshType A HalfEdgeMesh specialization + * @param meshes Meshes to merge + * @return The combined mesh and its provenance maps + * + * @throws std::invalid_argument If an input pointer is null or has no vertices. + */ +template +auto MergeMeshes(const std::vector& meshes) -> MergedMesh +{ + MergedMesh result{MeshType::New(), {}, {}}; + auto& out = result.mesh; + + for (std::size_t ci = 0; ci < meshes.size(); ++ci) { + const auto& src = meshes[ci]; + if (not src or src->num_vertices() == 0) { + throw std::invalid_argument("MergeMeshes: input mesh is null or has no vertices"); + } + // Vertices keep their relative order, so merged index == offset + sub + // index; record provenance alongside each insertion. + const auto offset = out->num_vertices(); + for (const auto& v : src->vertices()) { + out->insert_vertex(*v); + result.vertex_source.emplace_back(ci, v->idx); + } + // Re-emit each face against the offset vertex indices, preserving the + // source face's corner order. + for (const auto& face : src->faces()) { + std::vector idxs; + for (const auto& edge : *face) { + idxs.push_back(edge->vertex->idx + offset); + } + out->insert_face(idxs); + result.face_source.emplace_back(ci, face->idx); + } + } + return result; +} + +} // namespace OpenABF diff --git a/include/OpenABF/OpenABF.hpp b/include/OpenABF/OpenABF.hpp index 23754bb..c8f4d54 100644 --- a/include/OpenABF/OpenABF.hpp +++ b/include/OpenABF/OpenABF.hpp @@ -31,6 +31,7 @@ limitations under the License. #include "OpenABF/HierarchicalLSCM.hpp" #include "OpenABF/ChartPacking.hpp" +#include "OpenABF/MeshMerge.hpp" #include "OpenABF/MeshIO.hpp" // clang-format on diff --git a/single_include/OpenABF/OpenABF.hpp b/single_include/OpenABF/OpenABF.hpp index c8ca501..260b363 100644 --- a/single_include/OpenABF/OpenABF.hpp +++ b/single_include/OpenABF/OpenABF.hpp @@ -5144,6 +5144,111 @@ auto PackCharts(std::vector& charts, } // namespace OpenABF +// #include "OpenABF/MeshMerge.hpp" +/* +OpenABF +https://gitlab.com/educelab/OpenABF + +Copyright 2025 EduceLab + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + + +#include +#include +#include +#include + +namespace OpenABF +{ + +/** + * @brief Result of merging several meshes into one + * + * The inverse bookkeeping of `HalfEdgeMesh::extract_connected_components`: + * where extract splits one mesh into N with maps back to the source, merge + * combines N meshes into one with maps back to each source mesh. Compose + * `face_source`/`vertex_source` with the `face_map`/`vertex_map` of the + * components you merged to trace a merged element all the way back to the + * pre-extraction mesh. + * + * @tparam MeshType A HalfEdgeMesh specialization + */ +template +struct MergedMesh { + /** @brief The combined mesh */ + typename MeshType::Pointer mesh; + /** @brief `vertex_source[merged_idx] = {input mesh index, source vertex index}` */ + std::vector> vertex_source; + /** @brief `face_source[merged_idx] = {input mesh index, source face index}` */ + std::vector> face_source; +}; + +/** + * @brief Merge several meshes into a single mesh + * + * Concatenates the vertices and faces of each input mesh into one new mesh, + * offsetting face vertex indices per input so the inputs remain disjoint + * components. Returns the combined mesh alongside `vertex_source`/`face_source` + * maps that record, for every merged vertex and face, which input mesh and + * which source index it came from — the inverse of + * `extract_connected_components`, so the back-map chain survives the merge. + * + * Vertex positions and vertex traits are preserved (via the vertex copy + * constructor). Edge and face traits are default-constructed: merge rebuilds + * connectivity, so per-edge/per-face solver state is not carried over. + * + * @tparam MeshType A HalfEdgeMesh specialization + * @param meshes Meshes to merge + * @return The combined mesh and its provenance maps + * + * @throws std::invalid_argument If an input pointer is null or has no vertices. + */ +template +auto MergeMeshes(const std::vector& meshes) -> MergedMesh +{ + MergedMesh result{MeshType::New(), {}, {}}; + auto& out = result.mesh; + + for (std::size_t ci = 0; ci < meshes.size(); ++ci) { + const auto& src = meshes[ci]; + if (not src or src->num_vertices() == 0) { + throw std::invalid_argument("MergeMeshes: input mesh is null or has no vertices"); + } + // Vertices keep their relative order, so merged index == offset + sub + // index; record provenance alongside each insertion. + const auto offset = out->num_vertices(); + for (const auto& v : src->vertices()) { + out->insert_vertex(*v); + result.vertex_source.emplace_back(ci, v->idx); + } + // Re-emit each face against the offset vertex indices, preserving the + // source face's corner order. + for (const auto& face : src->faces()) { + std::vector idxs; + for (const auto& edge : *face) { + idxs.push_back(edge->vertex->idx + offset); + } + out->insert_face(idxs); + result.face_source.emplace_back(ci, face->idx); + } + } + return result; +} + +} // namespace OpenABF + // #include "OpenABF/MeshIO.hpp" diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e381070..2cc81e8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -17,6 +17,7 @@ set(tests src/TestMeshIOUtils.cpp src/TestMeshIO.cpp src/TestChartPacking.cpp + src/TestMeshMerge.cpp ) foreach(src ${tests}) diff --git a/tests/src/TestMeshMerge.cpp b/tests/src/TestMeshMerge.cpp new file mode 100644 index 0000000..2a9e3cd --- /dev/null +++ b/tests/src/TestMeshMerge.cpp @@ -0,0 +1,151 @@ +#include +#include +#include + +#include + +#include "OpenABF/OpenABF.hpp" + +using Mesh = OpenABF::HalfEdgeMesh; +using OpenABF::MergedMesh; +using OpenABF::MergeMeshes; + +namespace +{ + +/** @brief Build a w x h rectangle chart (two triangles) translated to (ox,oy) */ +auto MakeRectChart(float w, float h, float ox, float oy) -> Mesh::Pointer +{ + auto m = Mesh::New(); + m->insert_vertices( + {{ox, oy, 0.f}, {ox + w, oy, 0.f}, {ox + w, oy + h, 0.f}, {ox, oy + h, 0.f}}); + m->insert_faces({{0, 1, 2}, {0, 2, 3}}); + return m; +} + +} // namespace + +TEST(MeshMerge, EmptyListGivesEmptyMesh) +{ + std::vector charts; + auto merged = MergeMeshes(charts); + ASSERT_NE(merged.mesh, nullptr); + EXPECT_EQ(merged.mesh->num_vertices(), 0u); + EXPECT_EQ(merged.mesh->num_faces(), 0u); + EXPECT_TRUE(merged.vertex_source.empty()); + EXPECT_TRUE(merged.face_source.empty()); +} + +TEST(MeshMerge, NullChartThrows) +{ + std::vector charts{MakeRectChart(1.f, 1.f, 0.f, 0.f), nullptr}; + EXPECT_THROW(MergeMeshes(charts), std::invalid_argument); +} + +TEST(MeshMerge, EmptyChartThrows) +{ + std::vector charts{MakeRectChart(1.f, 1.f, 0.f, 0.f), Mesh::New()}; + EXPECT_THROW(MergeMeshes(charts), std::invalid_argument); +} + +TEST(MeshMerge, ConcatenatesVerticesAndFaces) +{ + std::vector charts{MakeRectChart(1.f, 1.f, 0.f, 0.f), + MakeRectChart(1.f, 1.f, 5.f, 0.f)}; + auto merged = MergeMeshes(charts); + EXPECT_EQ(merged.mesh->num_vertices(), 8u); + EXPECT_EQ(merged.mesh->num_faces(), 4u); + EXPECT_EQ(merged.vertex_source.size(), 8u); + EXPECT_EQ(merged.face_source.size(), 4u); +} + +TEST(MeshMerge, VertexSourcePreservesPositionsAndProvenance) +{ + std::vector charts{MakeRectChart(1.f, 1.f, 0.f, 0.f), + MakeRectChart(2.f, 3.f, 5.f, 0.f)}; + auto merged = MergeMeshes(charts); + + ASSERT_EQ(merged.vertex_source.size(), merged.mesh->num_vertices()); + for (std::size_t i = 0; i < merged.mesh->num_vertices(); ++i) { + auto [chart, sub] = merged.vertex_source[i]; + ASSERT_LT(chart, charts.size()); + ASSERT_LT(sub, charts[chart]->num_vertices()); + const auto& mergedV = merged.mesh->vertex(i); + const auto& srcV = charts[chart]->vertex(sub); + EXPECT_FLOAT_EQ(mergedV->pos[0], srcV->pos[0]) << "vertex " << i; + EXPECT_FLOAT_EQ(mergedV->pos[1], srcV->pos[1]) << "vertex " << i; + } +} + +TEST(MeshMerge, FaceSourceProvenanceIsValid) +{ + std::vector charts{MakeRectChart(1.f, 1.f, 0.f, 0.f), + MakeRectChart(1.f, 1.f, 5.f, 0.f)}; + auto merged = MergeMeshes(charts); + + ASSERT_EQ(merged.face_source.size(), merged.mesh->num_faces()); + // Each (chart, sub face) provenance is unique and in range. + std::set> seen; + for (const auto& [chart, sub] : merged.face_source) { + ASSERT_LT(chart, charts.size()); + ASSERT_LT(sub, charts[chart]->num_faces()); + EXPECT_TRUE(seen.emplace(chart, sub).second) << "duplicate face provenance"; + } +} + +// Composing the merge maps with extract's back-maps must recover original +// (torn-mesh) identity: this is the full chain merged -> chart -> M'. +TEST(MeshMerge, RoundTripExtractMergeRecoversOriginalIdentity) +{ + using ABF = OpenABF::ABFPlusPlus; + using LSCM = OpenABF::AngleBasedLSCM; + + auto mesh = ABF::Mesh::New(); + mesh->insert_vertices({ + {0.f, 0.f, 0.f}, + {1.f, 0.f, 0.f}, + {2.f, 0.f, 0.f}, + {0.f, 1.f, 0.f}, + {1.f, 1.f, 0.f}, + {2.f, 1.f, 0.f}, + {0.f, 2.f, 0.f}, + {1.f, 2.f, 0.f}, + {2.f, 2.f, 0.f}, + }); + mesh->insert_faces({ + {0, 3, 1}, + {1, 3, 4}, + {1, 4, 2}, + {2, 4, 5}, + {3, 6, 4}, + {4, 6, 7}, + {4, 7, 5}, + {5, 7, 8}, + }); + mesh->split_path({1, 4, 7}); + auto ccs = mesh->extract_connected_components(); + ASSERT_EQ(ccs.size(), 2u); + + std::vector chartMeshes; + std::size_t totalVerts = 0; + std::size_t totalFaces = 0; + for (auto& cc : ccs) { + LSCM::Compute(cc.mesh); + chartMeshes.push_back(cc.mesh); + totalVerts += cc.mesh->num_vertices(); + totalFaces += cc.mesh->num_faces(); + } + + auto merged = MergeMeshes(chartMeshes); + EXPECT_EQ(merged.mesh->num_vertices(), totalVerts); + EXPECT_EQ(merged.mesh->num_faces(), totalFaces); + + // merged face -> (chart, sub face) -> M' face via extract face_map. + // Every original (torn-mesh) face must be hit exactly once. + std::set origFaces; + for (const auto& [chart, sub] : merged.face_source) { + auto origFace = ccs[chart].face_map[sub]; + EXPECT_TRUE(origFaces.insert(origFace).second) << "duplicate original face " << origFace; + } + EXPECT_EQ(origFaces.size(), 8u); // all original faces recovered +} From 02e50305b0ca8e8a521a481c91c73aee1ebb24ee Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 13:45:55 -0400 Subject: [PATCH 07/15] docs(example): add UVMap construction reference (comment only) Add a non-compiled reference comment to MultiChartFlatten showing how to populate an educelab::core UVMap from the merged atlas: walk atlas faces back to the torn source mesh via face_source/face_map, take UVs from the packed chart vertices, and resolve corner positions by vertex identity (winding is not guaranteed stable). Demonstrates the optional WithChart trait for per-coordinate chart tagging. Co-Authored-By: Claude Opus 4.8 --- examples/src/MultiChartFlatten.cpp | 56 ++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index 03d049c..b492a94 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -107,6 +107,62 @@ int main() std::cout << "Wrote packed atlas: " << merged.mesh->num_vertices() << " vertices, " << merged.mesh->num_faces() << " faces -> " << out << "\n"; + /* + * Reference: building a per-wedge UVMap from the merged result + * ----------------------------------------------------------------------- + * OpenABF does not own a UV-map type, but the merged atlas plus the + * components' back-maps carry everything needed to populate one. The + * snippet below (not compiled here) targets educelab::core's UVMap: + * + * educelab/core/types/UVMap.hpp + * + * The UVMap is keyed by (face, corner) against the *torn source mesh* + * `mesh` — which still holds the original 3D geometry, since only the + * extracted charts were flattened. UV coordinates come from the packed + * chart vertices. Corner positions are resolved by *vertex identity*, not + * by traversal order: a face's winding may be reversed at insertion time, + * so the chart/atlas corner order is not guaranteed to match the source + * face's corner order (see PackCharts / HalfEdgeMesh::insert_face). + * + * #include + * #include "educelab/core/types/UVMap.hpp" + * using educelab::UVMap; + * using educelab::traits::WithChart; // optional: tag each UV by chart + * + * UVMap uv; + * + * for (std::size_t mf = 0; mf < merged.mesh->num_faces(); ++mf) { + * // Atlas face -> source chart + chart-local face -> source (M') face. + * const auto [chart, subFace] = merged.face_source[mf]; + * const auto srcFace = charts[chart].face_map[subFace]; + * + * // Source face corner order, keyed by M' vertex index. + * std::vector srcCorners; + * for (const auto& e : *mesh->faces()[srcFace]) { + * srcCorners.push_back(e->vertex->idx); + * } + * + * // Each atlas-face corner carries its packed UV in pos. + * for (const auto& e : *merged.mesh->faces()[mf]) { + * const auto [vChart, vSub] = merged.vertex_source[e->vertex->idx]; + * const auto srcVert = charts[vChart].vertex_map[vSub]; // M' vertex + * + * // Place the UV at the matching corner of the source face. + * const auto corner = static_cast(std::distance( + * srcCorners.begin(), + * std::find(srcCorners.begin(), srcCorners.end(), srcVert))); + * + * UVMap::Coordinate c{e->vertex->pos[0], + * e->vertex->pos[1]}; + * c.chart = chart; // WithChart mixin + * uv.map(srcFace, corner, uv.insert(c)); + * } + * } + * + * // uv.get_coordinate(srcFace, corner) now yields the packed UV for + * // each wedge of `mesh`, ready for OBJ `vt` emission. + */ + // The source mesh's 3D vertex positions are unchanged by the per-chart // flattening and packing — only the extracted sub-meshes hold the 2D UV // result. From dead335acaee9423f0f422eb2bbe9f68328d4bd0 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 13:55:31 -0400 Subject: [PATCH 08/15] docs(example): correct UVMap WithChart usage in reference comment WithChart's chart index denotes independent packing domains (separate [0,1]^2 atlases / usemtl texture pages), not the connected components of a single shared atlas. This example packs all CCs into one atlas via a single PackCharts call, so the snippet now uses the default UVMap (one domain) and documents that WithChart applies only when running multiple independent packings. Co-Authored-By: Claude Opus 4.8 --- examples/src/MultiChartFlatten.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index b492a94..d2815e5 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -124,12 +124,16 @@ int main() * so the chart/atlas corner order is not guaranteed to match the source * face's corner order (see PackCharts / HalfEdgeMesh::insert_face). * + * This single PackCharts call produced ONE shared [0,1]^2 atlas, i.e. one + * packing domain, so the default UVMap (no per-coordinate chart index) is + * the right type here — every wedge lives in the same domain. See the note + * after the snippet for when traits::WithChart applies. + * * #include * #include "educelab/core/types/UVMap.hpp" * using educelab::UVMap; - * using educelab::traits::WithChart; // optional: tag each UV by chart * - * UVMap uv; + * UVMap uv; * * for (std::size_t mf = 0; mf < merged.mesh->num_faces(); ++mf) { * // Atlas face -> source chart + chart-local face -> source (M') face. @@ -152,15 +156,22 @@ int main() * srcCorners.begin(), * std::find(srcCorners.begin(), srcCorners.end(), srcVert))); * - * UVMap::Coordinate c{e->vertex->pos[0], - * e->vertex->pos[1]}; - * c.chart = chart; // WithChart mixin - * uv.map(srcFace, corner, uv.insert(c)); + * uv.map(srcFace, corner, uv.insert(e->vertex->pos[0], + * e->vertex->pos[1])); * } * } * * // uv.get_coordinate(srcFace, corner) now yields the packed UV for * // each wedge of `mesh`, ready for OBJ `vt` emission. + * + * traits::WithChart is for MULTIPLE independent packing domains, not the + * connected components of a single atlas. Its `chart` index maps to a + * separate [0,1]^2 domain / texture page: educelab's write_obj groups + * faces into `usemtl materialN` by chart and emits one texture path per + * chart. You would use it if you partitioned the components into groups + * and ran PackCharts once per group (one atlas each), setting + * `coordinate.chart` to that group's domain index — not to tag the CCs + * that share this single packed frame. */ // The source mesh's 3D vertex positions are unchanged by the per-chart From fafe4d90ce2ca28e6fecdc21bbd139da0d2f0fcf Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 14:13:28 -0400 Subject: [PATCH 09/15] docs(example): note UVMap validity on torn and untorn mesh Document that the per-wedge UVMap is valid for both the torn and the untorn (pre-split) mesh because corners are resolved by vertex identity against the target mesh's own faces, which absorbs any insert_face winding reversal at build/extract/merge. Note the caveat that as-built corner order may differ from the raw input face list (tracked separately). Co-Authored-By: Claude Opus 4.8 --- examples/src/MultiChartFlatten.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index d2815e5..b9e6b52 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -124,11 +124,6 @@ int main() * so the chart/atlas corner order is not guaranteed to match the source * face's corner order (see PackCharts / HalfEdgeMesh::insert_face). * - * This single PackCharts call produced ONE shared [0,1]^2 atlas, i.e. one - * packing domain, so the default UVMap (no per-coordinate chart index) is - * the right type here — every wedge lives in the same domain. See the note - * after the snippet for when traits::WithChart applies. - * * #include * #include "educelab/core/types/UVMap.hpp" * using educelab::UVMap; @@ -164,14 +159,17 @@ int main() * // uv.get_coordinate(srcFace, corner) now yields the packed UV for * // each wedge of `mesh`, ready for OBJ `vt` emission. * - * traits::WithChart is for MULTIPLE independent packing domains, not the - * connected components of a single atlas. Its `chart` index maps to a - * separate [0,1]^2 domain / texture page: educelab's write_obj groups - * faces into `usemtl materialN` by chart and emits one texture path per - * chart. You would use it if you partitioned the components into groups - * and ran PackCharts once per group (one atlas each), setting - * `coordinate.chart` to that group's domain index — not to tag the CCs - * that share this single packed frame. + * This table is valid for BOTH the torn mesh and the untorn (pre-split) + * mesh. split_path preserves face indices and per-face winding (it never + * re-inserts faces), and the (face, corner) keys are resolved by vertex + * identity against `mesh`'s own faces — so any winding reversal insert_face + * applies (when M is built, when components are cloned by extract, and when + * charts are merged) is absorbed rather than baked into the keys. The rule + * that makes this work: consume the UVMap against the same HalfEdgeMesh (or + * one sharing its winding) and resolve corners by identity, never by a raw + * traversal index. Caveat: insert_face's auto-rewinding means a face's + * as-built corner order may differ from the raw input face list, and the + * mesh does not record that permutation (see issue tracker / bug track B9). */ // The source mesh's 3D vertex positions are unchanged by the per-chart From 9972af4f91f4cb24824dac0be77393a4925d96a8 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 14:13:28 -0400 Subject: [PATCH 10/15] docs(conductor): file B9 track for insert_face rewinding discrepancy (#100) insert_face auto-reverses mis-wound faces (intended, makes almost-manifold input manifold) but silently changes a face's corner order vs the raw input and records no input->as-built permutation. Surfaced during F2; downstream per-corner round-trips can silently desync. Track B9 / issue #100 capture the problem and candidate fixes (reversal flag, permutation accessor, strict mode, or docs). Co-Authored-By: Claude Opus 4.8 --- conductor/tracks.md | 1 + conductor/tracks/B9/plan.md | 28 +++++++++++++++++++ conductor/tracks/B9/spec.md | 55 +++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 conductor/tracks/B9/plan.md create mode 100644 conductor/tracks/B9/spec.md diff --git a/conductor/tracks.md b/conductor/tracks.md index aa328cb..a25b1cc 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -20,6 +20,7 @@ F2 or F5–F11 — based on priority at the time of starting. | open | F11 | Implement ACVD | [#62](https://github.com/educelab/OpenABF/issues/62) | 2026-03-20 | 2026-03-20 | | open | F2 | Multi-chart UV packing | [#18](https://github.com/educelab/OpenABF/issues/18) | 2026-03-13 | 2026-06-20 | | open | B8 | LSCM area-preserving rescale of flattening output | [#98](https://github.com/educelab/OpenABF/issues/98) | 2026-06-20 | 2026-06-20 | +| open | B9 | insert_face auto-rewinding silently changes face corner order vs input (unrecorded) | [#100](https://github.com/educelab/OpenABF/issues/100) | 2026-06-20 | 2026-06-20 | ## Archived Tracks diff --git a/conductor/tracks/B9/plan.md b/conductor/tracks/B9/plan.md new file mode 100644 index 0000000..2d6aece --- /dev/null +++ b/conductor/tracks/B9/plan.md @@ -0,0 +1,28 @@ +# B9 Implementation Plan + +Potential-bug track: insert_face auto-rewinding silently changes face corner +order vs the raw input, unrecorded. See spec.md and issue #100. + +## Phase 1: Investigation & decision +- [ ] 1.1 Reproduce: build a mesh with a deliberately mis-wound face; confirm + traversal corner order differs from input and nothing records it +- [ ] 1.2 Survey internal call sites that re-insert faces and could rewind + (insert_faces, clone_face_ used by extract_connected_components, any + merge/split helpers) and note where the discrepancy can leak +- [ ] 1.3 Choose an approach (flag / permutation accessor / strict mode / docs) + and record the decision in spec.md + +## Phase 2: Tests (write first) +- [ ] 2.1 Test asserting the chosen observable behavior on a mis-wound input face +- [ ] 2.2 If a recovery API is added: test that it maps corners back to input order +- [ ] 2.3 If a strict mode is added: test that mis-wound input throws under it + +## Phase 3: Implementation +- [ ] 3.1 Implement the chosen approach +- [ ] 3.2 Document auto-rewinding + corner-order effect on insert_face/insert_faces +- [ ] 3.3 Regenerate single header (and update install list if a header is added) + +## Phase 4: Verify +- [ ] 4.1 Run `ctest` — all suites pass +- [ ] 4.2 Run clang-format on changed files +- [ ] 4.3 Confirm single-header build compiles and runs diff --git a/conductor/tracks/B9/spec.md b/conductor/tracks/B9/spec.md new file mode 100644 index 0000000..2cb8e80 --- /dev/null +++ b/conductor/tracks/B9/spec.md @@ -0,0 +1,55 @@ +# B9 — insert_face auto-rewinding silently changes face corner order vs input + +## GitHub Issue +https://github.com/educelab/OpenABF/issues/100 + +## Summary +`HalfEdgeMesh::insert_face` auto-reverses a mis-wound face to keep the mesh's +winding globally consistent (`include/OpenABF/HalfEdgeMesh.hpp:1706-1748`). The +reversal is desirable for making almost-manifold input manifold, but it +**silently changes a face's stored corner/vertex traversal order relative to the +raw input face list**, and the mesh records neither that a reversal occurred nor +the input→as-built corner permutation. + +## Why this matters +Downstream consumers build a mesh from a known face list and later read back +per-face-corner data — applying a per-wedge UV map, transferring per-corner +attributes, emitting OBJ `vt`/`f` keyed by input corner order. For any reversed +face, the half-edge traversal order no longer matches the caller's input order, +with no signal. The discrepancy is silent. + +Discovered during F2 (multi-chart UV packing, PR #99). The robust workaround is +to key per-wedge data by **vertex identity**, never by raw traversal index — +correct but non-obvious, and the trap is currently unguarded. + +## Impact +- Silent mismatch: input corner order ≠ as-built corner order for reversed faces. +- Affects any round-trip of per-corner data through a `HalfEdgeMesh`. +- No API to detect a reversed face or recover the original corner order. + +## Out of scope +- Removing the auto-rewinding behavior itself (it is intentional and useful). + +## Acceptance Criteria +- [ ] Decision recorded on the chosen approach (see below). +- [ ] If a detection/recovery API is added: it reports, for each face, whether it + was reversed at insertion and/or maps a corner index back to input order, + with unit tests on a mesh containing at least one mis-wound input face. +- [ ] `insert_face` / `insert_faces` documentation prominently describes the + auto-rewinding behavior and its effect on corner order. +- [ ] A test constructs a mesh with a deliberately mis-wound face and asserts the + documented/observable behavior (and the recovery API if added). +- [ ] Single-header regenerated; multiheader install list updated if a new header + is introduced (none expected). + +## Candidate approaches (decide in Phase 1) +1. Record per-face reversal (flag) and/or store the input→as-built corner + permutation, exposed via an accessor. +2. Provide a query mapping a corner index back to input order. +3. Opt-in "strict" insertion mode that throws on mis-wound input instead of + silently reversing (caller fixes winding explicitly). +4. Minimum viable: document the behavior loudly + ship the identity-keying + guidance (already drafted in the F2 MultiChartFlatten reference comment). + +## Dependencies +- None. Independent of F2 (PR #99), though motivated by it. From 9abcb4a939299eb50d42dcdf11e83b38036cb651 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 14:29:25 -0400 Subject: [PATCH 11/15] docs(conductor): broaden B9 to require mapping back to original input topology MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rescope B9 beyond insert_face rewinding to the full problem: there is no recoverable path from a torn/parameterized/packed result back to the caller's original input topology. Two unrecorded identity shifts compose — insert_face corner-order reversal and split_edge seam-vertex duplication (original -> duplicate). Require B9 to provide invertible mappings for both, composing with F2's vertex_map/face_map and vertex_source/face_source so a consumer can name the original input face, corner position, and vertex for any atlas corner. Update issue #100 to match. Co-Authored-By: Claude Opus 4.8 --- conductor/tracks.md | 2 +- conductor/tracks/B9/plan.md | 38 ++++++++----- conductor/tracks/B9/spec.md | 110 +++++++++++++++++++++++------------- 3 files changed, 96 insertions(+), 54 deletions(-) diff --git a/conductor/tracks.md b/conductor/tracks.md index a25b1cc..8fa30f5 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -20,7 +20,7 @@ F2 or F5–F11 — based on priority at the time of starting. | open | F11 | Implement ACVD | [#62](https://github.com/educelab/OpenABF/issues/62) | 2026-03-20 | 2026-03-20 | | open | F2 | Multi-chart UV packing | [#18](https://github.com/educelab/OpenABF/issues/18) | 2026-03-13 | 2026-06-20 | | open | B8 | LSCM area-preserving rescale of flattening output | [#98](https://github.com/educelab/OpenABF/issues/98) | 2026-06-20 | 2026-06-20 | -| open | B9 | insert_face auto-rewinding silently changes face corner order vs input (unrecorded) | [#100](https://github.com/educelab/OpenABF/issues/100) | 2026-06-20 | 2026-06-20 | +| open | B9 | No recoverable mapping from torn/parameterized mesh back to original input topology (insert_face rewind + split_path duplication) | [#100](https://github.com/educelab/OpenABF/issues/100) | 2026-06-20 | 2026-06-20 | ## Archived Tracks diff --git a/conductor/tracks/B9/plan.md b/conductor/tracks/B9/plan.md index 2d6aece..8da1add 100644 --- a/conductor/tracks/B9/plan.md +++ b/conductor/tracks/B9/plan.md @@ -1,26 +1,34 @@ # B9 Implementation Plan -Potential-bug track: insert_face auto-rewinding silently changes face corner -order vs the raw input, unrecorded. See spec.md and issue #100. +Provide a recoverable mapping from a torn/parameterized mesh back to the +original input topology. Two unrecorded identity shifts must be invertible: +insert_face corner-order reversal, and split_edge seam-vertex duplication. +See spec.md and issue #100. ## Phase 1: Investigation & decision -- [ ] 1.1 Reproduce: build a mesh with a deliberately mis-wound face; confirm - traversal corner order differs from input and nothing records it -- [ ] 1.2 Survey internal call sites that re-insert faces and could rewind - (insert_faces, clone_face_ used by extract_connected_components, any - merge/split helpers) and note where the discrepancy can leak -- [ ] 1.3 Choose an approach (flag / permutation accessor / strict mode / docs) - and record the decision in spec.md +- [ ] 1.1 Reproduce both shifts: (a) a mis-wound input face whose as-built corner + order differs from input; (b) a torn seam whose duplicate vertex has no + recorded link to its original — confirm neither is recoverable today +- [ ] 1.2 Survey re-insertion/duplication sites: insert_face/insert_faces (reversal), + split_edge/split_path (duplication), clone_face_ (extraction) — confirm + how each affects the original→result chain +- [ ] 1.3 Choose the approach (vertex origin tracking, returned remaps, per-face + corner permutation, or a combination) and record the decision in spec.md ## Phase 2: Tests (write first) -- [ ] 2.1 Test asserting the chosen observable behavior on a mis-wound input face -- [ ] 2.2 If a recovery API is added: test that it maps corners back to input order -- [ ] 2.3 If a strict mode is added: test that mis-wound input throws under it +- [ ] 2.1 Duplicate → original vertex recovery across split_path (incl. multi-seam) +- [ ] 2.2 Reversed-face corner-order recovery for a mis-wound input face +- [ ] 2.3 Full round-trip: packed/merged atlas corner → F2 maps → torn-mesh corner + → B9 maps → original input face, input corner position, input vertex +- [ ] 2.4 Identity/no-op case: an untorn, correctly-wound mesh round-trips to identity ## Phase 3: Implementation -- [ ] 3.1 Implement the chosen approach -- [ ] 3.2 Document auto-rewinding + corner-order effect on insert_face/insert_faces -- [ ] 3.3 Regenerate single header (and update install list if a header is added) +- [ ] 3.1 Implement duplicate → original vertex mapping in split_edge/split_path +- [ ] 3.2 Implement corner-order recovery for insert_face (reversed flag/permutation) +- [ ] 3.3 Ensure the mappings survive extract_connected_components (and compose with + F2's vertex_map/face_map and vertex_source/face_source) +- [ ] 3.4 Document behavior + recovery API on insert_face/insert_faces/split_* +- [ ] 3.5 Regenerate single header (and update install list if a header is added) ## Phase 4: Verify - [ ] 4.1 Run `ctest` — all suites pass diff --git a/conductor/tracks/B9/spec.md b/conductor/tracks/B9/spec.md index 2cb8e80..f9b99b1 100644 --- a/conductor/tracks/B9/spec.md +++ b/conductor/tracks/B9/spec.md @@ -1,55 +1,89 @@ -# B9 — insert_face auto-rewinding silently changes face corner order vs input +# B9 — No recoverable mapping from a torn/parameterized mesh back to the original input topology ## GitHub Issue https://github.com/educelab/OpenABF/issues/100 ## Summary -`HalfEdgeMesh::insert_face` auto-reverses a mis-wound face to keep the mesh's -winding globally consistent (`include/OpenABF/HalfEdgeMesh.hpp:1706-1748`). The -reversal is desirable for making almost-manifold input manifold, but it -**silently changes a face's stored corner/vertex traversal order relative to the -raw input face list**, and the mesh records neither that a reversal occurred nor -the input→as-built corner permutation. +A consumer who builds a `HalfEdgeMesh` from a known face list, tears it +(`split_path`), extracts components, parameterizes, and packs them has **no +recorded path back to their original input topology**. Two distinct, +*independently unrecorded* identity shifts sit between the input and the result: + +1. **insert_face winding reversal (corner order).** `insert_face` auto-reverses + a mis-wound face to keep the mesh globally manifold + (`include/OpenABF/HalfEdgeMesh.hpp:1706-1748`). This is intended and useful, + but it permutes a face's stored corner order relative to the raw input face + list, and the permutation is not recorded. +2. **split_edge vertex duplication (vertex identity).** Tearing duplicates seam + vertices via `insert_vertex(oldStart->pos)` + (`include/OpenABF/HalfEdgeMesh.hpp:1445,1465`), appending new indices. + `split_path`/`split_edge` return `void` and record no `duplicate → original` + vertex map; the only thing the duplicate carries is a copied position. + +These **compose**: rewinding happens at build time, then seam splitting changes +which vertex identity sits at a (still position-stable) corner. Working purely +in the post-split mesh's own namespace is fine and single-level (this is what +the F2 `PackCharts`/`MultiChartFlatten` per-wedge recipe does — key by position, +resolve identity within M′). But the moment a user needs to relate results back +to **their original input** — original vertex indices and original (input) face +corner order — both shifts must be inverted, and neither is currently +recoverable. ## Why this matters -Downstream consumers build a mesh from a known face list and later read back -per-face-corner data — applying a per-wedge UV map, transferring per-corner -attributes, emitting OBJ `vt`/`f` keyed by input corner order. For any reversed -face, the half-edge traversal order no longer matches the caller's input order, -with no signal. The discrepancy is silent. +Round-tripping per-corner / per-vertex data to the *caller's own topology* is a +normal need: applying an externally authored per-wedge UV map, transferring +per-vertex attributes captured before tearing, or emitting output indexed the +way the caller supplied geometry. Today that bridge is silently impossible for +any reversed face or any seam-duplicated vertex. Discovered during F2 (PR #99). -Discovered during F2 (multi-chart UV packing, PR #99). The robust workaround is -to key per-wedge data by **vertex identity**, never by raw traversal index — -correct but non-obvious, and the trap is currently unguarded. +## Goal +Provide a **recoverable mapping from the torn/extracted result back to the +original input mesh**, covering both shifts: +- duplicate seam vertex → original input vertex, and +- as-built face corner order → raw input face corner order. -## Impact -- Silent mismatch: input corner order ≠ as-built corner order for reversed faces. -- Affects any round-trip of per-corner data through a `HalfEdgeMesh`. -- No API to detect a reversed face or recover the original corner order. +Composed with the existing `ExtractedComponent::vertex_map`/`face_map` (and +`MergedMesh::vertex_source`/`face_source` from F2), a consumer should be able to +take any corner of a packed/merged atlas and name the original input mesh's +face, input corner position, and input vertex it came from. ## Out of scope -- Removing the auto-rewinding behavior itself (it is intentional and useful). +- Removing auto-rewinding or seam duplication (both are intentional). ## Acceptance Criteria -- [ ] Decision recorded on the chosen approach (see below). -- [ ] If a detection/recovery API is added: it reports, for each face, whether it - was reversed at insertion and/or maps a corner index back to input order, - with unit tests on a mesh containing at least one mis-wound input face. -- [ ] `insert_face` / `insert_faces` documentation prominently describes the - auto-rewinding behavior and its effect on corner order. -- [ ] A test constructs a mesh with a deliberately mis-wound face and asserts the - documented/observable behavior (and the recovery API if added). -- [ ] Single-header regenerated; multiheader install list updated if a new header - is introduced (none expected). +- [ ] `split_edge`/`split_path` expose a recoverable **duplicate → original** + vertex mapping (e.g. returned map, accumulator, or an origin index stored + on duplicated vertices that survives extraction). +- [ ] `insert_face`/`insert_faces` expose whether a face was reversed and/or a + way to recover the raw input corner order (reversed flag or + input→as-built corner permutation accessor). +- [ ] A worked path demonstrates full round-trip: packed/merged atlas corner → + (F2 maps) → torn-mesh corner → (B9 maps) → original input face, input + corner position, and input vertex index. +- [ ] Unit tests on a mesh with (a) at least one deliberately mis-wound input + face and (b) at least one torn seam, asserting both inverse mappings + recover the original identities. +- [ ] `insert_face`/`split_*` documentation describes the behavior and points to + the recovery API. +- [ ] Single-header regenerated; multiheader install list updated if a new + header is introduced. ## Candidate approaches (decide in Phase 1) -1. Record per-face reversal (flag) and/or store the input→as-built corner - permutation, exposed via an accessor. -2. Provide a query mapping a corner index back to input order. -3. Opt-in "strict" insertion mode that throws on mis-wound input instead of - silently reversing (caller fixes winding explicitly). -4. Minimum viable: document the behavior loudly + ship the identity-keying - guidance (already drafted in the F2 MultiChartFlatten reference comment). +1. **Vertex origin tracking.** Store an `origin` (original input vertex index) + that is set on construction and copied to duplicates by `split_edge`, so + every vertex — original or duplicate — names its input vertex. Survives + `clone_face_`/extraction via the vertex copy path. +2. **Returned remaps.** `split_edge`/`split_path` return/accumulate + `duplicate → original` pairs; a separate per-face reversal record handles + corner order. +3. **Per-face corner permutation.** Record, per face, the input→as-built corner + order (a reversed flag suffices for triangles; a rotation+reversal for + general polygons), with an accessor. +4. **Combination + docs.** Likely (1)+(3): identity via vertex origin, corner + order via per-face reversal record, plus prominent documentation and the + identity-keying guidance already drafted in the F2 reference comment. ## Dependencies -- None. Independent of F2 (PR #99), though motivated by it. +- Independent of F2 (PR #99), but motivated by it; the F2 maps + (`vertex_map`/`face_map`, `vertex_source`/`face_source`) are the downstream + half of the chain B9 completes back to the input. From 59aaf2092731346e0a8c6622fdae3a44f1c0154f Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 16:08:56 -0400 Subject: [PATCH 12/15] feat: surround packed charts with padding on all sides (F2) PackCharts applied `padding` only as a gutter between charts, so charts on the atlas perimeter still touched the boundary (left/bottom at the origin, rightmost/topmost at the extent). For a texture atlas that lets edge charts bleed across the boundary/seam under filtering, mipmapping, or wrap addressing. Inset the whole shelf layout: the cursor starts and wraps at `pad`, and `pad` is added to the far extents, so every chart has >= padding of empty space on all four sides, including against the atlas boundary. The atlas lower corner stays at the origin. normalize now fits the padded atlas into [0,1]^2. The library default stays padding = 0 (flush); the MultiChartFlatten example sets a visible padding to demonstrate the gutter. Co-Authored-By: Claude Opus 4.8 --- examples/src/MultiChartFlatten.cpp | 5 ++++ include/OpenABF/ChartPacking.hpp | 38 +++++++++++++++++++++--------- single_include/OpenABF/OpenABF.hpp | 38 +++++++++++++++++++++--------- tests/src/TestChartPacking.cpp | 30 ++++++++++++++++++++--- 4 files changed, 86 insertions(+), 25 deletions(-) diff --git a/examples/src/MultiChartFlatten.cpp b/examples/src/MultiChartFlatten.cpp index b9e6b52..7a8e805 100644 --- a/examples/src/MultiChartFlatten.cpp +++ b/examples/src/MultiChartFlatten.cpp @@ -92,6 +92,11 @@ int main() // positions in place; the per-chart vertex_map/face_map remain valid. OpenABF::PackOptions opts; opts.normalize = true; + // Add a gutter around every chart so neighbouring charts -- and the atlas + // boundary -- don't touch. Without this, charts pack flush and texture + // filtering can bleed one chart's texels into another. `padding` is in + // absolute chart units and is applied before the normalize scaling. + opts.padding = 0.1f; auto extent = OpenABF::PackCharts(chartMeshes, opts); std::cout << "Packed atlas extent: [" << extent.min[0] << ", " << extent.min[1] << "] -> [" << extent.max[0] << ", " << extent.max[1] << "]\n"; diff --git a/include/OpenABF/ChartPacking.hpp b/include/OpenABF/ChartPacking.hpp index e5c73db..da21eb0 100644 --- a/include/OpenABF/ChartPacking.hpp +++ b/include/OpenABF/ChartPacking.hpp @@ -72,7 +72,15 @@ struct PackOptions { */ std::optional target_width{}; - /** @brief Gutter added around each chart, in chart/absolute units */ + /** + * @brief Gutter added around every chart, in chart/absolute units + * + * Applied on all four sides of each chart, including against the atlas + * boundary, so perimeter charts are inset from the returned extent by + * `padding` as well -- not merely separated from their neighbors. + * Defaults to `0` (charts laid out flush). `padding` is in absolute chart + * units and is applied before any `normalize` scaling. + */ T padding{T(0)}; }; @@ -199,19 +207,20 @@ auto PackCharts(std::vector& charts, // Shelf layout. Charts are placed left-to-right; a row wraps to a new shelf // once it would exceed targetWidth (a chart wider than targetWidth still - // gets placed alone at the start of a shelf). Every chart's bbox-min is - // mapped to a cursor position >= 0, so the packed atlas's lower corner is - // the origin. + // gets placed alone at the start of a shelf). The cursor starts at `pad` + // and wraps back to `pad`, so every chart is inset by at least `pad` from + // the atlas's lower corner; the atlas's lower corner itself stays at the + // origin. std::vector offsetX(n); std::vector offsetY(n); - auto cursorX = T(0); - auto cursorY = T(0); + auto cursorX = pad; + auto cursorY = pad; auto shelfHeight = T(0); auto atlasMaxX = T(0); auto atlasMaxY = T(0); for (const auto i : order) { - if (cursorX > T(0) and cursorX + width[i] > targetWidth) { - cursorX = T(0); + if (cursorX > pad and cursorX + width[i] > targetWidth) { + cursorX = pad; cursorY += shelfHeight + pad; shelfHeight = T(0); } @@ -223,11 +232,18 @@ auto PackCharts(std::vector& charts, shelfHeight = std::max(shelfHeight, height[i]); } + // The atlas extent includes the perimeter gutter: charts are inset by + // `pad` from the lower corner (the cursor starts at `pad`), so add `pad` + // to the far edges too. Every chart then has >= `pad` of empty space on + // all four sides, including against the atlas boundary. + const T atlasW = atlasMaxX + pad; + const T atlasH = atlasMaxY + pad; + // Optional normalization: a single global uniform scale that fits the - // packed atlas into [0,1]^2, preserving relative chart sizes. + // padded atlas into [0,1]^2, preserving relative chart sizes. auto scale = T(1); if (opts.normalize) { - const auto extentMax = std::max(atlasMaxX, atlasMaxY); + const auto extentMax = std::max(atlasW, atlasH); if (extentMax > T(0)) { scale = T(1) / extentMax; } @@ -241,7 +257,7 @@ auto PackCharts(std::vector& charts, } } - result.max = Vec{atlasMaxX * scale, atlasMaxY * scale}; + result.max = Vec{atlasW * scale, atlasH * scale}; return result; } diff --git a/single_include/OpenABF/OpenABF.hpp b/single_include/OpenABF/OpenABF.hpp index 260b363..f3900ad 100644 --- a/single_include/OpenABF/OpenABF.hpp +++ b/single_include/OpenABF/OpenABF.hpp @@ -4969,7 +4969,15 @@ struct PackOptions { */ std::optional target_width{}; - /** @brief Gutter added around each chart, in chart/absolute units */ + /** + * @brief Gutter added around every chart, in chart/absolute units + * + * Applied on all four sides of each chart, including against the atlas + * boundary, so perimeter charts are inset from the returned extent by + * `padding` as well -- not merely separated from their neighbors. + * Defaults to `0` (charts laid out flush). `padding` is in absolute chart + * units and is applied before any `normalize` scaling. + */ T padding{T(0)}; }; @@ -5096,19 +5104,20 @@ auto PackCharts(std::vector& charts, // Shelf layout. Charts are placed left-to-right; a row wraps to a new shelf // once it would exceed targetWidth (a chart wider than targetWidth still - // gets placed alone at the start of a shelf). Every chart's bbox-min is - // mapped to a cursor position >= 0, so the packed atlas's lower corner is - // the origin. + // gets placed alone at the start of a shelf). The cursor starts at `pad` + // and wraps back to `pad`, so every chart is inset by at least `pad` from + // the atlas's lower corner; the atlas's lower corner itself stays at the + // origin. std::vector offsetX(n); std::vector offsetY(n); - auto cursorX = T(0); - auto cursorY = T(0); + auto cursorX = pad; + auto cursorY = pad; auto shelfHeight = T(0); auto atlasMaxX = T(0); auto atlasMaxY = T(0); for (const auto i : order) { - if (cursorX > T(0) and cursorX + width[i] > targetWidth) { - cursorX = T(0); + if (cursorX > pad and cursorX + width[i] > targetWidth) { + cursorX = pad; cursorY += shelfHeight + pad; shelfHeight = T(0); } @@ -5120,11 +5129,18 @@ auto PackCharts(std::vector& charts, shelfHeight = std::max(shelfHeight, height[i]); } + // The atlas extent includes the perimeter gutter: charts are inset by + // `pad` from the lower corner (the cursor starts at `pad`), so add `pad` + // to the far edges too. Every chart then has >= `pad` of empty space on + // all four sides, including against the atlas boundary. + const T atlasW = atlasMaxX + pad; + const T atlasH = atlasMaxY + pad; + // Optional normalization: a single global uniform scale that fits the - // packed atlas into [0,1]^2, preserving relative chart sizes. + // padded atlas into [0,1]^2, preserving relative chart sizes. auto scale = T(1); if (opts.normalize) { - const auto extentMax = std::max(atlasMaxX, atlasMaxY); + const auto extentMax = std::max(atlasW, atlasH); if (extentMax > T(0)) { scale = T(1) / extentMax; } @@ -5138,7 +5154,7 @@ auto PackCharts(std::vector& charts, } } - result.max = Vec{atlasMaxX * scale, atlasMaxY * scale}; + result.max = Vec{atlasW * scale, atlasH * scale}; return result; } diff --git a/tests/src/TestChartPacking.cpp b/tests/src/TestChartPacking.cpp index 68aabce..6a0374c 100644 --- a/tests/src/TestChartPacking.cpp +++ b/tests/src/TestChartPacking.cpp @@ -157,14 +157,15 @@ TEST(ChartPacking, ExtentBoundsAllCharts) TEST(ChartPacking, PaddingSeparatesChartsInSingleRow) { - // Force a single row with a large target width; padding should appear as a - // gap between the two charts, so the atlas width is w0 + padding + w1. + // Force a single row with a large target width. Padding surrounds every + // chart on all sides, so the atlas width is + // pad + w0 + pad + w1 + pad = 0.5 + 1 + 0.5 + 1 + 0.5 = 3.5. std::vector charts{MakeRectChart(1.f, 1.f), MakeRectChart(1.f, 1.f)}; PackOptions opts; opts.target_width = 1000.f; opts.padding = 0.5f; auto extent = PackCharts(charts, opts); - EXPECT_NEAR(extent.max[0] - extent.min[0], 2.5f, 1e-4f); + EXPECT_NEAR(extent.max[0] - extent.min[0], 3.5f, 1e-4f); auto b0 = ChartBBox(charts[0]); auto b1 = ChartBBox(charts[1]); @@ -172,6 +173,29 @@ TEST(ChartPacking, PaddingSeparatesChartsInSingleRow) EXPECT_GE(gap, 0.5f - 1e-4f); } +TEST(ChartPacking, PaddingSurroundsChartsAtPerimeter) +{ + // Padding is a gutter on all four sides of every chart, including against + // the atlas boundary -- not just between neighbours. Use several charts so + // the layout wraps to multiple shelves, exercising both the left/bottom + // margins and the right/top margins. + const float pad = 0.5f; + std::vector charts; + for (int i = 0; i < 5; ++i) { + charts.push_back(MakeRectChart(1.f, 1.f)); + } + PackOptions opts; + opts.padding = pad; + auto extent = PackCharts(charts, opts); + for (std::size_t i = 0; i < charts.size(); ++i) { + auto b = ChartBBox(charts[i]); + EXPECT_GE(b.minx, extent.min[0] + pad - 1e-4f) << "chart " << i; + EXPECT_GE(b.miny, extent.min[1] + pad - 1e-4f) << "chart " << i; + EXPECT_LE(b.maxx, extent.max[0] - pad + 1e-4f) << "chart " << i; + EXPECT_LE(b.maxy, extent.max[1] - pad + 1e-4f) << "chart " << i; + } +} + // --- Normalize mode -------------------------------------------------------- TEST(ChartPacking, NormalizeFitsUnitSquare) From 3a4dea5fa8fc5a5782892e2f4f3c4411975c2424 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Sat, 20 Jun 2026 16:09:03 -0400 Subject: [PATCH 13/15] docs(conductor): record F2 perimeter-padding refinement (Phase 6) Document that `padding` surrounds every chart on all four sides (incl. the atlas perimeter) in Design Decision 5 and the acceptance criteria, and add Phase 6 to the plan capturing the review question and resolution. Co-Authored-By: Claude Opus 4.8 --- conductor/tracks/F2/plan.md | 20 ++++++++++++++++++++ conductor/tracks/F2/spec.md | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/conductor/tracks/F2/plan.md b/conductor/tracks/F2/plan.md index e94e121..10ccdee 100644 --- a/conductor/tracks/F2/plan.md +++ b/conductor/tracks/F2/plan.md @@ -47,3 +47,23 @@ provenance maps so merged → chart → M' composition keeps working. - [x] 5.3 Wire into OpenABF.hpp + multiheader install list; regenerate single header - [x] 5.4 Switch MultiChartFlatten example to use MergeMeshes - [x] 5.5 Verify: full ctest, single-header build, install-test all pass + +## Phase 6: Perimeter padding (added during review) +Rationale: review question "shouldn't we add padding around the packed +charts?". The original layout applied `padding` only as a gutter *between* +charts — perimeter charts still touched the atlas boundary (left/bottom at the +origin, rightmost/topmost at the extent). For a texture atlas this lets edge +charts bleed across the boundary/seam under filtering, mipmapping, or wrap +addressing. Resolution (user-confirmed): inset the whole layout so `padding` +surrounds every chart on all four sides; keep the library default `padding = 0` +and instead set a visible padding in the example. +- [x] 6.1 Tests first: assert padding insets charts from the atlas perimeter + (new PaddingSurroundsChartsAtPerimeter); update single-row extent + expectation (pad + w0 + pad + w1 + pad) +- [x] 6.2 Implement perimeter inset: cursor starts/wraps at `pad`; add `pad` to + far extents; normalize fits the padded atlas into [0,1]² +- [x] 6.3 Update header docs (padding surrounds charts; atlas lower corner stays + at origin) + spec Decision 5 / acceptance criteria +- [x] 6.4 Set a visible `padding` in the MultiChartFlatten example +- [x] 6.5 Regenerate single header; verify full ctest, example run, single-header + build, clang-format all pass diff --git a/conductor/tracks/F2/spec.md b/conductor/tracks/F2/spec.md index 7508e1c..927ebf2 100644 --- a/conductor/tracks/F2/spec.md +++ b/conductor/tracks/F2/spec.md @@ -74,7 +74,8 @@ and emit per-corner `vt` entries, build atlases, etc., from there. struct PackOptions { bool normalize = false; // fit packed atlas into [0,1]^2 std::optional target_width{}; // overrides sqrt-area heuristic - T padding = T(0); // per-chart gutter, absolute units + T padding = T(0); // gutter on all sides of every chart + // (incl. atlas perimeter), abs units }; struct PackResult { Vec min, max; }; // packed atlas extent @@ -98,6 +99,9 @@ and emit per-corner `vt` entries, build atlases, etc., from there. - [ ] Shelf-packing with the ~square target-width heuristic, overridable. - [ ] No charts' bounding boxes overlap (padding respected); the packed set is contained in the returned extent (and in `[0,1]²` when normalized). +- [ ] `padding` surrounds every chart on all four sides, including against the + atlas boundary (perimeter charts are inset from the extent by `padding`, + not just separated from neighbors). - [ ] Edge cases handled per Design Decision 6. - [ ] Header documents the vertex-identity per-wedge recipe (Decision 2). - [ ] Tests: synthetic 2D charts for deterministic geometric assertions plus From 21a6ed105a1ea16ab846f5b28ffd4baabd307255 Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Mon, 22 Jun 2026 07:08:22 +0200 Subject: [PATCH 14/15] Fix BFS bugs --- conductor/tracks/B9/plan.md | 55 ++++++----- conductor/tracks/B9/spec.md | 147 ++++++++++++++++------------- include/OpenABF/HalfEdgeMesh.hpp | 18 ++-- single_include/OpenABF/OpenABF.hpp | 18 ++-- 4 files changed, 139 insertions(+), 99 deletions(-) diff --git a/conductor/tracks/B9/plan.md b/conductor/tracks/B9/plan.md index 8da1add..48a9a3f 100644 --- a/conductor/tracks/B9/plan.md +++ b/conductor/tracks/B9/plan.md @@ -1,34 +1,43 @@ # B9 Implementation Plan -Provide a recoverable mapping from a torn/parameterized mesh back to the -original input topology. Two unrecorded identity shifts must be invertible: -insert_face corner-order reversal, and split_edge seam-vertex duplication. -See spec.md and issue #100. +Make every torn/extracted vertex name its pre-HEM (input) vertex, so a consumer +who wraps their own mesh in a HalfEdgeMesh to flatten can build a per-wedge UV +map that lands back on their pre-HEM mesh — seam-duplicated corners included. +The `insert_face` winding reversal is absorbed by identity-keyed corner +resolution and needs no separate record (see spec.md and issue #100). ## Phase 1: Investigation & decision -- [ ] 1.1 Reproduce both shifts: (a) a mis-wound input face whose as-built corner - order differs from input; (b) a torn seam whose duplicate vertex has no - recorded link to its original — confirm neither is recoverable today -- [ ] 1.2 Survey re-insertion/duplication sites: insert_face/insert_faces (reversal), - split_edge/split_path (duplication), clone_face_ (extraction) — confirm - how each affects the original→result chain -- [ ] 1.3 Choose the approach (vertex origin tracking, returned remaps, per-face - corner permutation, or a combination) and record the decision in spec.md +- [ ] 1.1 Reproduce the gap: tear a seam, confirm the duplicate vertex has no + recorded link to its pre-HEM origin and that an atlas seam corner + cannot be resolved to the consumer's `(face, corner)` today +- [ ] 1.2 Confirm the already-recoverable hops to keep scope tight: face index + (insert_faces order + split_path never re-inserts), non-seam vertex + index (insert_vertices order + tearing only appends), and that + identity-keyed corner resolution absorbs the winding reversal +- [ ] 1.3 Survey duplication/copy sites: split_edge/split_path (duplication) and + clone_face_ (extraction vertex copy) — confirm an `origin` field rides + the copy path; choose vertex-origin tracking vs returned remaps and + record the decision in spec.md ## Phase 2: Tests (write first) -- [ ] 2.1 Duplicate → original vertex recovery across split_path (incl. multi-seam) -- [ ] 2.2 Reversed-face corner-order recovery for a mis-wound input face -- [ ] 2.3 Full round-trip: packed/merged atlas corner → F2 maps → torn-mesh corner - → B9 maps → original input face, input corner position, input vertex -- [ ] 2.4 Identity/no-op case: an untorn, correctly-wound mesh round-trips to identity +- [ ] 2.1 Duplicate → pre-HEM vertex recovery across split_path (incl. multi-seam) +- [ ] 2.2 Full round-trip: packed/merged atlas corner → F2 maps → torn-HEM corner + → B9 mapping → pre-HEM face, corner, vertex; assert a seam corner lands + on the correct pre-HEM (face, corner) by identity +- [ ] 2.3 Mis-wound input face: confirm the winding reversal is absorbed — the + round-trip lands the correct corner with no corner-order record +- [ ] 2.4 Identity/no-op case: an untorn, correctly-wound mesh round-trips to the + identity mapping ## Phase 3: Implementation -- [ ] 3.1 Implement duplicate → original vertex mapping in split_edge/split_path -- [ ] 3.2 Implement corner-order recovery for insert_face (reversed flag/permutation) -- [ ] 3.3 Ensure the mappings survive extract_connected_components (and compose with - F2's vertex_map/face_map and vertex_source/face_source) -- [ ] 3.4 Document behavior + recovery API on insert_face/insert_faces/split_* -- [ ] 3.5 Regenerate single header (and update install list if a header is added) +- [ ] 3.1 Implement duplicate → pre-HEM vertex mapping in split_edge/split_path + (origin set at construction, copied to duplicates) +- [ ] 3.2 Ensure the mapping survives extract_connected_components (clone_face_ + vertex copy) and composes with F2's vertex_map/vertex_source +- [ ] 3.3 Document behavior + recovery API on split_*; update the + MultiChartFlatten.cpp reference comment to key corners against the + pre-HEM mesh using the vertex origin +- [ ] 3.4 Regenerate single header (and update install list if a header is added) ## Phase 4: Verify - [ ] 4.1 Run `ctest` — all suites pass diff --git a/conductor/tracks/B9/spec.md b/conductor/tracks/B9/spec.md index f9b99b1..4d16eec 100644 --- a/conductor/tracks/B9/spec.md +++ b/conductor/tracks/B9/spec.md @@ -1,89 +1,108 @@ -# B9 — No recoverable mapping from a torn/parameterized mesh back to the original input topology +# B9 — No recoverable mapping from torn seam-duplicate vertices back to the consumer's pre-HEM mesh ## GitHub Issue https://github.com/educelab/OpenABF/issues/100 +## Use case +Downstream consumers do **not** use `HalfEdgeMesh` as their primary mesh type. +They keep their own mesh (the "raw" / pre-HEM mesh — their own vertex array and +face list) and *wrap* it in a `HalfEdgeMesh` only to flatten: build the HEM with +`insert_vertices`/`insert_faces`, tear it (`split_path`), extract components, +parameterize, and pack/merge. They then build a **per-wedge UV map** from the +packed result and expect to apply it back onto their **pre-HEM mesh**, keyed by +their own `(face, corner)` identities. + ## Summary -A consumer who builds a `HalfEdgeMesh` from a known face list, tears it -(`split_path`), extracts components, parameterizes, and packs them has **no -recorded path back to their original input topology**. Two distinct, -*independently unrecorded* identity shifts sit between the input and the result: +That round-trip is `atlas corner → (F2 maps) → torn-HEM corner → pre-HEM mesh`. +Three of the four hops already round-trip cleanly: -1. **insert_face winding reversal (corner order).** `insert_face` auto-reverses - a mis-wound face to keep the mesh globally manifold - (`include/OpenABF/HalfEdgeMesh.hpp:1706-1748`). This is intended and useful, - but it permutes a face's stored corner order relative to the raw input face - list, and the permutation is not recorded. -2. **split_edge vertex duplication (vertex identity).** Tearing duplicates seam - vertices via `insert_vertex(oldStart->pos)` - (`include/OpenABF/HalfEdgeMesh.hpp:1445,1465`), appending new indices. - `split_path`/`split_edge` return `void` and record no `duplicate → original` - vertex map; the only thing the duplicate carries is a copied position. +- **Face index.** `insert_faces` inserts faces in order + (`include/OpenABF/HalfEdgeMesh.hpp:1040-1049`) and `split_path` only tears + edges (it never re-inserts faces), so pre-HEM face `i` == HEM face `i`, and + F2's `face_map`/`face_source` carry it back. +- **Corner order.** `insert_face` may auto-reverse a mis-wound face to keep the + mesh manifold (`include/OpenABF/HalfEdgeMesh.hpp:1706-1748`), permuting a + face's stored corner order relative to the raw input. This is **not** a + blocker: the documented per-wedge recipe resolves corners by *vertex + identity* against the consumer's own face, never by raw traversal index + (`include/OpenABF/ChartPacking.hpp:108-123`), so the reversal is **absorbed**, + not inverted. (See "Out of scope".) +- **Non-seam vertices.** `insert_vertices` preserves order and tearing only + *appends* new vertices, so a non-duplicated HEM vertex keeps its pre-HEM + index; F2's `vertex_map`/`vertex_source` carry it back. -These **compose**: rewinding happens at build time, then seam splitting changes -which vertex identity sits at a (still position-stable) corner. Working purely -in the post-split mesh's own namespace is fine and single-level (this is what -the F2 `PackCharts`/`MultiChartFlatten` per-wedge recipe does — key by position, -resolve identity within M′). But the moment a user needs to relate results back -to **their original input** — original vertex indices and original (input) face -corner order — both shifts must be inverted, and neither is currently -recoverable. +The one hop that breaks is **seam-vertex identity**. Tearing duplicates seam +vertices via `insert_vertex(oldStart->pos)` +(`include/OpenABF/HalfEdgeMesh.hpp:1445,1465`), appending a new index whose only +link to its origin is a copied position. `split_path`/`split_edge` return `void` +and record no `duplicate → original` vertex map. So a torn-HEM corner that lands +on a seam duplicate **cannot be expressed in the consumer's pre-HEM vertex +namespace** — and identity-based corner resolution against the consumer's own +face (the very mechanism that absorbs the winding reversal) fails for exactly +those corners. ## Why this matters -Round-tripping per-corner / per-vertex data to the *caller's own topology* is a -normal need: applying an externally authored per-wedge UV map, transferring -per-vertex attributes captured before tearing, or emitting output indexed the -way the caller supplied geometry. Today that bridge is silently impossible for -any reversed face or any seam-duplicated vertex. Discovered during F2 (PR #99). +The per-wedge UV recipe in `MultiChartFlatten.cpp` works entirely *within* the +torn HEM's own namespace, which is self-consistent. But the consumer's goal is +to land UVs on their **pre-HEM** mesh. For every non-seam corner that already +works; for a seam corner it silently cannot, because the duplicate has no +recoverable pre-HEM vertex identity. The same gap blocks scattering per-vertex +attributes captured on the pre-HEM mesh and emitting output indexed by the +consumer's original vertices. Discovered during F2 (PR #99). ## Goal -Provide a **recoverable mapping from the torn/extracted result back to the -original input mesh**, covering both shifts: -- duplicate seam vertex → original input vertex, and -- as-built face corner order → raw input face corner order. - -Composed with the existing `ExtractedComponent::vertex_map`/`face_map` (and -`MergedMesh::vertex_source`/`face_source` from F2), a consumer should be able to -take any corner of a packed/merged atlas and name the original input mesh's -face, input corner position, and input vertex it came from. +Make every torn/extracted vertex — original or seam duplicate — name its +**pre-HEM (input) vertex**, so the documented identity-keyed per-wedge recipe +resolves corners against the consumer's own faces for *all* corners, seams +included. Composed with F2's `vertex_map`/`face_map` and +`vertex_source`/`face_source`, a consumer can take any corner of a packed/merged +atlas and name the pre-HEM face, corner, and vertex it came from. ## Out of scope - Removing auto-rewinding or seam duplication (both are intentional). +- Recording the `insert_face` corner-order permutation / a reversed flag. + Identity-keyed corner resolution (`ChartPacking.hpp:108-123`) absorbs the + reversal, so it does **not** need to be inverted for this use case. The corner + order is recovered implicitly once seam-duplicate vertices carry their pre-HEM + identity (this AC), by locating each vertex within the consumer's own face. ## Acceptance Criteria -- [ ] `split_edge`/`split_path` expose a recoverable **duplicate → original** - vertex mapping (e.g. returned map, accumulator, or an origin index stored - on duplicated vertices that survives extraction). -- [ ] `insert_face`/`insert_faces` expose whether a face was reversed and/or a - way to recover the raw input corner order (reversed flag or - input→as-built corner permutation accessor). -- [ ] A worked path demonstrates full round-trip: packed/merged atlas corner → - (F2 maps) → torn-mesh corner → (B9 maps) → original input face, input - corner position, and input vertex index. -- [ ] Unit tests on a mesh with (a) at least one deliberately mis-wound input - face and (b) at least one torn seam, asserting both inverse mappings - recover the original identities. -- [ ] `insert_face`/`split_*` documentation describes the behavior and points to - the recovery API. +- [ ] `split_edge`/`split_path` expose a recoverable **duplicate → pre-HEM** + vertex mapping (e.g. an `origin` index stored on every vertex, set at + construction and copied to duplicates, that survives extraction; or a + returned/accumulated remap). +- [ ] The mapping survives `extract_connected_components` (`clone_face_` copies + vertices, so an `origin` field rides along) and composes with F2's + `vertex_map`/`vertex_source`. +- [ ] A worked path demonstrates the full round-trip: packed/merged atlas corner + → (F2 maps) → torn-HEM corner → (B9 mapping) → pre-HEM face, corner, and + vertex index — resolving the corner by vertex identity against the + consumer's own face, with seam-duplicate corners resolving correctly. +- [ ] Unit tests on a mesh with at least one torn seam assert the duplicate → + pre-HEM vertex mapping recovers the original identity, and that a seam + corner of the packed atlas lands on the correct pre-HEM `(face, corner)`. + Include a mis-wound input face to confirm the winding reversal is absorbed + (the round-trip still lands the right corner without a corner-order record). +- [ ] An untorn, correctly-wound mesh round-trips to the identity mapping. +- [ ] `split_*` documentation describes the behavior and points to the recovery + API; the `MultiChartFlatten.cpp` reference comment is updated to key + corners against the pre-HEM mesh using the new vertex origin. - [ ] Single-header regenerated; multiheader install list updated if a new header is introduced. ## Candidate approaches (decide in Phase 1) -1. **Vertex origin tracking.** Store an `origin` (original input vertex index) - that is set on construction and copied to duplicates by `split_edge`, so - every vertex — original or duplicate — names its input vertex. Survives - `clone_face_`/extraction via the vertex copy path. +1. **Vertex origin tracking (primary).** Store an `origin` (pre-HEM vertex index) + set on construction and copied to duplicates by `split_edge`, so every vertex + — original or duplicate — names its pre-HEM vertex. Survives + `clone_face_`/extraction via the vertex copy path. With this, corners are + located by identity in the consumer's own face and the winding reversal needs + no separate record. 2. **Returned remaps.** `split_edge`/`split_path` return/accumulate - `duplicate → original` pairs; a separate per-face reversal record handles - corner order. -3. **Per-face corner permutation.** Record, per face, the input→as-built corner - order (a reversed flag suffices for triangles; a rotation+reversal for - general polygons), with an accessor. -4. **Combination + docs.** Likely (1)+(3): identity via vertex origin, corner - order via per-face reversal record, plus prominent documentation and the - identity-keying guidance already drafted in the F2 reference comment. + `duplicate → original` pairs. Lighter-weight but does not survive extraction + without the caller threading it through, and does not give a uniform + "every vertex names its input vertex" accessor. ## Dependencies - Independent of F2 (PR #99), but motivated by it; the F2 maps (`vertex_map`/`face_map`, `vertex_source`/`face_source`) are the downstream - half of the chain B9 completes back to the input. + half of the chain B9 completes back to the pre-HEM mesh. diff --git a/include/OpenABF/HalfEdgeMesh.hpp b/include/OpenABF/HalfEdgeMesh.hpp index 5e58baa..10ae5e9 100644 --- a/include/OpenABF/HalfEdgeMesh.hpp +++ b/include/OpenABF/HalfEdgeMesh.hpp @@ -1218,19 +1218,22 @@ class HalfEdgeMesh continue; } - // Start a new connected component + // Start a new connected component. Mark faces visited as they are + // enqueued (not when dequeued) so each face is enqueued and + // expanded exactly once; otherwise a face is re-enqueued once per + // incident interior edge and the traversal blows up on large meshes. + visited[f->idx] = true; queue.push(f); while (not queue.empty()) { // Get the top of the queue auto p = queue.front(); queue.pop(); - // Mark as visited - visited[p->idx] = true; // Add the neighbor faces to the queue for (const auto& e : *p) { if (not e->pair->is_boundary()) { auto n = e->pair->face; if (not visited[n->idx]) { + visited[n->idx] = true; queue.push(n); } } @@ -1258,15 +1261,17 @@ class HalfEdgeMesh continue; } - // Start a new connected component + // Start a new connected component. Mark faces visited as they are + // enqueued (not when dequeued) so each face is enqueued and + // expanded exactly once; otherwise a face is re-enqueued once per + // incident interior edge and the traversal blows up on large meshes. current.clear(); + visited[f->idx] = true; queue.push(f); while (not queue.empty()) { // Get the top of the queue auto p = queue.front(); queue.pop(); - // Mark as visited - visited[p->idx] = true; // Add to this connected component current.emplace_back(p); // Add the neighbor faces to the queue @@ -1274,6 +1279,7 @@ class HalfEdgeMesh if (not e->pair->is_boundary()) { auto n = e->pair->face; if (not visited[n->idx]) { + visited[n->idx] = true; queue.push(n); } } diff --git a/single_include/OpenABF/OpenABF.hpp b/single_include/OpenABF/OpenABF.hpp index f3900ad..2895694 100644 --- a/single_include/OpenABF/OpenABF.hpp +++ b/single_include/OpenABF/OpenABF.hpp @@ -1662,19 +1662,22 @@ class HalfEdgeMesh continue; } - // Start a new connected component + // Start a new connected component. Mark faces visited as they are + // enqueued (not when dequeued) so each face is enqueued and + // expanded exactly once; otherwise a face is re-enqueued once per + // incident interior edge and the traversal blows up on large meshes. + visited[f->idx] = true; queue.push(f); while (not queue.empty()) { // Get the top of the queue auto p = queue.front(); queue.pop(); - // Mark as visited - visited[p->idx] = true; // Add the neighbor faces to the queue for (const auto& e : *p) { if (not e->pair->is_boundary()) { auto n = e->pair->face; if (not visited[n->idx]) { + visited[n->idx] = true; queue.push(n); } } @@ -1702,15 +1705,17 @@ class HalfEdgeMesh continue; } - // Start a new connected component + // Start a new connected component. Mark faces visited as they are + // enqueued (not when dequeued) so each face is enqueued and + // expanded exactly once; otherwise a face is re-enqueued once per + // incident interior edge and the traversal blows up on large meshes. current.clear(); + visited[f->idx] = true; queue.push(f); while (not queue.empty()) { // Get the top of the queue auto p = queue.front(); queue.pop(); - // Mark as visited - visited[p->idx] = true; // Add to this connected component current.emplace_back(p); // Add the neighbor faces to the queue @@ -1718,6 +1723,7 @@ class HalfEdgeMesh if (not e->pair->is_boundary()) { auto n = e->pair->face; if (not visited[n->idx]) { + visited[n->idx] = true; queue.push(n); } } From 10133282414c1922547fabe6418df1acf99795ee Mon Sep 17 00:00:00 2001 From: Seth Parker Date: Mon, 22 Jun 2026 07:34:51 +0200 Subject: [PATCH 15/15] refactor(packing): address PR #99 review feedback - Add static Vec::Dimensions; drop the detail::VecDimensions trait - Template PackResult on the mesh's vertex Vec type instead of Vec - Use std::minmax_element with structured bindings for chart bounds - MergeMeshes: batch faces through insert_faces so update_boundary runs - PackCharts: add minimize_bounding_box option (default on) that rotates each chart to its minimum-area orientation and stands its long axis vertical to match the tallest-first shelf strategy Co-Authored-By: Claude Opus 4.8 --- include/OpenABF/ChartPacking.hpp | 215 ++++++++++++++++++++++----- include/OpenABF/MeshMerge.hpp | 8 +- include/OpenABF/Vec.hpp | 3 + single_include/OpenABF/OpenABF.hpp | 227 +++++++++++++++++++++++------ tests/src/TestChartPacking.cpp | 70 ++++++++- 5 files changed, 440 insertions(+), 83 deletions(-) diff --git a/include/OpenABF/ChartPacking.hpp b/include/OpenABF/ChartPacking.hpp index da21eb0..64de550 100644 --- a/include/OpenABF/ChartPacking.hpp +++ b/include/OpenABF/ChartPacking.hpp @@ -19,30 +19,147 @@ limitations under the License. #pragma once #include +#include #include #include #include #include #include #include +#include #include #include -#include "OpenABF/Vec.hpp" - namespace OpenABF { namespace detail { -/** @brief Deduce the dimensionality of a Vec type */ -template -struct VecDimensions; +/** + * @brief Rotate a chart within its UV plane so its axis-aligned bounding box + * has minimum area, with the larger extent vertical + * + * The minimum-area enclosing rectangle of a planar point set always has one + * edge collinear with an edge of the set's convex hull, so it suffices to test + * the orientation induced by each hull edge. The chart is then stood on its + * long axis (larger extent vertical) so it aligns with PackCharts's + * tallest-first shelf strategy. Vertex positions are rotated in place about the + * origin; only the first two components are touched. Rotation preserves + * topology and vertex identity, so any back-maps remain valid. + * + * @tparam MeshType A HalfEdgeMesh specialization + */ +template +void MinimizeChartBoundingBox(const typename MeshType::Pointer& chart) +{ + using T = typename MeshType::type; + using Point = std::array; -template -struct VecDimensions> { - static constexpr std::size_t value = N; -}; + // Gather the 2D point set. + std::vector pts; + pts.reserve(chart->num_vertices()); + for (const auto& v : chart->vertices()) { + pts.push_back({v->pos[0], v->pos[1]}); + } + + // Convex hull via Andrew's monotone chain. Fewer than three unique points + // means a point or a segment, for which no rotation reduces the area. + std::sort(pts.begin(), pts.end()); + pts.erase(std::unique(pts.begin(), pts.end()), pts.end()); + const std::size_t m = pts.size(); + if (m < 3) { + return; + } + auto crossZ = [](const Point& o, const Point& a, const Point& b) -> T { + return (a[0] - o[0]) * (b[1] - o[1]) - (a[1] - o[1]) * (b[0] - o[0]); + }; + std::vector hull(2 * m); + std::size_t k = 0; + for (std::size_t i = 0; i < m; ++i) { + while (k >= 2 and crossZ(hull[k - 2], hull[k - 1], pts[i]) <= T(0)) { + --k; + } + hull[k++] = pts[i]; + } + for (std::size_t i = m - 1, t = k + 1; i > 0; --i) { + while (k >= t and crossZ(hull[k - 2], hull[k - 1], pts[i - 1]) <= T(0)) { + --k; + } + hull[k++] = pts[i - 1]; + } + hull.resize(k - 1); // drop the duplicated start point + const std::size_t h = hull.size(); + if (h < 3) { + return; + } + + // Bounding-box dimensions {width, height} of the hull after rotating every + // point by R(-theta), where (c, s) = (cos theta, sin theta). + auto boxFor = [&](T c, T s) -> Point { + auto mnX = std::numeric_limits::max(); + auto mnY = std::numeric_limits::max(); + auto mxX = std::numeric_limits::lowest(); + auto mxY = std::numeric_limits::lowest(); + for (const auto& p : hull) { + const auto rx = c * p[0] + s * p[1]; + const auto ry = -s * p[0] + c * p[1]; + mnX = std::min(mnX, rx); + mnY = std::min(mnY, ry); + mxX = std::max(mxX, rx); + mxY = std::max(mxY, ry); + } + return {mxX - mnX, mxY - mnY}; + }; + + // Seed with the current (unrotated) box so we only rotate on a strict + // area improvement. + auto bestCos = T(1); + auto bestSin = T(0); + auto bestBox = boxFor(T(1), T(0)); + auto bestArea = bestBox[0] * bestBox[1]; + for (std::size_t i = 0; i < h; ++i) { + const auto& p0 = hull[i]; + const auto& p1 = hull[(i + 1) % h]; + const auto ex = p1[0] - p0[0]; + const auto ey = p1[1] - p0[1]; + const auto len = std::sqrt(ex * ex + ey * ey); + if (len <= T(0)) { + continue; + } + // Align this hull edge with the x-axis (theta = atan2(ey, ex)). + const auto c = ex / len; + const auto s = ey / len; + const auto box = boxFor(c, s); + const auto area = box[0] * box[1]; + if (area < bestArea) { + bestArea = area; + bestCos = c; + bestSin = s; + bestBox = box; + } + } + + // Stand the chart on its long axis: the packer sorts tallest-first and + // fills horizontal shelves, so the larger extent should be vertical. If the + // min-area box is wider than tall, compose an extra 90-degree rotation + // (R90 * R(-theta), where R90 maps (x, y) -> (-y, x)). + if (bestBox[0] > bestBox[1]) { + const auto c = bestCos; + const auto s = bestSin; + bestCos = s; + bestSin = -c; + } + + // Apply the chosen rotation to every vertex in place (skip the identity). + if (bestCos != T(1) or bestSin != T(0)) { + for (const auto& v : chart->vertices()) { + const auto x = v->pos[0]; + const auto y = v->pos[1]; + v->pos[0] = bestCos * x + bestSin * y; + v->pos[1] = -bestSin * x + bestCos * y; + } + } +} } // namespace detail /** @@ -52,6 +169,19 @@ struct VecDimensions> { */ template struct PackOptions { + /** + * @brief Rotate each chart in-plane to minimize its bounding-box area + * + * When `true` (default), each chart is rotated within its UV plane before + * layout so its axis-aligned bounding box has minimum area, then stood on + * its long axis (larger extent vertical) to match the tallest-first shelf + * strategy. Shelf packing works on axis-aligned boxes, so tightening and + * consistently orienting each box lets charts nest more densely. The + * rotation is applied in place and preserves topology and vertex identity, + * so any back-maps a caller holds remain valid. + */ + bool minimize_bounding_box{true}; + /** * @brief Fit the packed atlas into the unit square `[0,1]^2` * @@ -87,14 +217,17 @@ struct PackOptions { /** * @brief The bounding box of the packed atlas * - * @tparam T Floating-point scalar type + * `min`/`max` use the same vector type as the input meshes' vertex positions; + * only the first two (`u`, `v`) components are meaningful. + * + * @tparam VecType The vertex position vector type of the packed meshes */ -template +template struct PackResult { /** @brief Lower corner of the packed atlas */ - Vec min; + VecType min; /** @brief Upper corner of the packed atlas */ - Vec max; + VecType max; }; /** @@ -103,10 +236,10 @@ struct PackResult { * Lays out a list of already-parameterized charts (2D meshes whose vertex * `pos` holds `{u, v, ...}`) into a single shared frame using shelf packing, * so that no two charts' bounding boxes overlap. Operates purely on geometry: - * each chart's vertex positions are translated (and, when `normalize` is set, - * uniformly scaled) **in place**. Topology, vertex indices, and face indices - * are untouched, so any `ExtractedComponent` back-maps a caller holds remain - * valid after packing. + * each chart's vertex positions are rotated (when `minimize_bounding_box` is + * set), translated, and (when `normalize` is set) uniformly scaled **in + * place**. Topology, vertex indices, and face indices are untouched, so any + * `ExtractedComponent` back-maps a caller holds remain valid after packing. * * @par Scaling * By default charts keep their absolute scale and are only translated; the @@ -133,7 +266,9 @@ struct PackResult { * @par Complexity * `O(n log n)` in the number of charts `n` (dominated by the height sort) plus * `O(V)` in the total vertex count `V` (two passes: one to measure bounding - * boxes, one to apply the transform). Memory overhead is `O(n)`. + * boxes, one to apply the transform). With `minimize_bounding_box`, each chart + * additionally costs an `O(v log v)` convex hull plus an `O(h v)` orientation + * search over its `h` hull edges. Memory overhead is `O(n)`. * * @tparam MeshType A HalfEdgeMesh specialization * @param charts Charts to pack; each chart's vertex positions are modified @@ -144,16 +279,17 @@ struct PackResult { * vertices. */ template -auto PackCharts(std::vector& charts, - const PackOptions& opts = - PackOptions{}) -> PackResult +auto PackCharts( + std::vector& charts, + const PackOptions& opts = PackOptions{}) + -> PackResult().pos)>> { using T = typename MeshType::type; - static_assert( - detail::VecDimensions().pos)>::value >= 2, - "PackCharts requires mesh vertices with at least 2 position dimensions"); + using VecType = std::decay_t().pos)>; + static_assert(VecType::Dimensions >= 2, + "PackCharts requires mesh vertices with at least 2 position dimensions"); - PackResult result{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; + PackResult result{}; if (charts.empty()) { return result; } @@ -169,20 +305,19 @@ auto PackCharts(std::vector& charts, if (not chart or chart->num_vertices() == 0) { throw std::invalid_argument("PackCharts: chart is null or has no vertices"); } - auto mnX = std::numeric_limits::max(); - auto mnY = std::numeric_limits::max(); - auto mxX = std::numeric_limits::lowest(); - auto mxY = std::numeric_limits::lowest(); - for (const auto& v : chart->vertices()) { - mnX = std::min(mnX, v->pos[0]); - mnY = std::min(mnY, v->pos[1]); - mxX = std::max(mxX, v->pos[0]); - mxY = std::max(mxY, v->pos[1]); + // Tighten the chart's bounding box by rotating it in-plane first. + if (opts.minimize_bounding_box) { + detail::MinimizeChartBoundingBox(chart); } - minX[i] = mnX; - minY[i] = mnY; - width[i] = mxX - mnX; - height[i] = mxY - mnY; + const auto& verts = chart->vertices(); + const auto cmpX = [](const auto& a, const auto& b) { return a->pos[0] < b->pos[0]; }; + const auto cmpY = [](const auto& a, const auto& b) { return a->pos[1] < b->pos[1]; }; + const auto [xlo, xhi] = std::minmax_element(verts.begin(), verts.end(), cmpX); + const auto [ylo, yhi] = std::minmax_element(verts.begin(), verts.end(), cmpY); + minX[i] = (*xlo)->pos[0]; + minY[i] = (*ylo)->pos[1]; + width[i] = (*xhi)->pos[0] - (*xlo)->pos[0]; + height[i] = (*yhi)->pos[1] - (*ylo)->pos[1]; } // Place taller charts first so shelves pack tightly. @@ -257,7 +392,9 @@ auto PackCharts(std::vector& charts, } } - result.max = Vec{atlasW * scale, atlasH * scale}; + // result.min is value-initialized to the origin; only u/v of max are set. + result.max[0] = atlasW * scale; + result.max[1] = atlasH * scale; return result; } diff --git a/include/OpenABF/MeshMerge.hpp b/include/OpenABF/MeshMerge.hpp index f1efd53..7de0b30 100644 --- a/include/OpenABF/MeshMerge.hpp +++ b/include/OpenABF/MeshMerge.hpp @@ -74,6 +74,11 @@ auto MergeMeshes(const std::vector& meshes) -> Merge MergedMesh result{MeshType::New(), {}, {}}; auto& out = result.mesh; + // Gather every face's (offset) vertex indices so they can be inserted in a + // single insert_faces() call, which rebuilds the mesh boundary once at the + // end via update_boundary(). Inserting faces one at a time with + // insert_face() would leave the boundary stale. + std::vector> faces; for (std::size_t ci = 0; ci < meshes.size(); ++ci) { const auto& src = meshes[ci]; if (not src or src->num_vertices() == 0) { @@ -93,10 +98,11 @@ auto MergeMeshes(const std::vector& meshes) -> Merge for (const auto& edge : *face) { idxs.push_back(edge->vertex->idx + offset); } - out->insert_face(idxs); + faces.push_back(std::move(idxs)); result.face_source.emplace_back(ci, face->idx); } } + out->insert_faces(faces); return result; } diff --git a/include/OpenABF/Vec.hpp b/include/OpenABF/Vec.hpp index c40ac8d..e4db265 100644 --- a/include/OpenABF/Vec.hpp +++ b/include/OpenABF/Vec.hpp @@ -23,6 +23,9 @@ class Vec using Container = std::array; public: + /** @brief Number of dimensions (elements) in the vector */ + static constexpr std::size_t Dimensions = Dims; + /** Element type */ using value_type = T; /** Vector size type */ diff --git a/single_include/OpenABF/OpenABF.hpp b/single_include/OpenABF/OpenABF.hpp index 2895694..0a96fac 100644 --- a/single_include/OpenABF/OpenABF.hpp +++ b/single_include/OpenABF/OpenABF.hpp @@ -175,6 +175,9 @@ class Vec using Container = std::array; public: + /** @brief Number of dimensions (elements) in the vector */ + static constexpr std::size_t Dimensions = Dims; + /** Element type */ using value_type = T; /** Vector size type */ @@ -4921,31 +4924,147 @@ limitations under the License. #include +#include #include #include #include #include #include #include +#include #include #include -// #include "OpenABF/Vec.hpp" - - namespace OpenABF { namespace detail { -/** @brief Deduce the dimensionality of a Vec type */ -template -struct VecDimensions; +/** + * @brief Rotate a chart within its UV plane so its axis-aligned bounding box + * has minimum area, with the larger extent vertical + * + * The minimum-area enclosing rectangle of a planar point set always has one + * edge collinear with an edge of the set's convex hull, so it suffices to test + * the orientation induced by each hull edge. The chart is then stood on its + * long axis (larger extent vertical) so it aligns with PackCharts's + * tallest-first shelf strategy. Vertex positions are rotated in place about the + * origin; only the first two components are touched. Rotation preserves + * topology and vertex identity, so any back-maps remain valid. + * + * @tparam MeshType A HalfEdgeMesh specialization + */ +template +void MinimizeChartBoundingBox(const typename MeshType::Pointer& chart) +{ + using T = typename MeshType::type; + using Point = std::array; -template -struct VecDimensions> { - static constexpr std::size_t value = N; -}; + // Gather the 2D point set. + std::vector pts; + pts.reserve(chart->num_vertices()); + for (const auto& v : chart->vertices()) { + pts.push_back({v->pos[0], v->pos[1]}); + } + + // Convex hull via Andrew's monotone chain. Fewer than three unique points + // means a point or a segment, for which no rotation reduces the area. + std::sort(pts.begin(), pts.end()); + pts.erase(std::unique(pts.begin(), pts.end()), pts.end()); + const std::size_t m = pts.size(); + if (m < 3) { + return; + } + auto crossZ = [](const Point& o, const Point& a, const Point& b) -> T { + return (a[0] - o[0]) * (b[1] - o[1]) - (a[1] - o[1]) * (b[0] - o[0]); + }; + std::vector hull(2 * m); + std::size_t k = 0; + for (std::size_t i = 0; i < m; ++i) { + while (k >= 2 and crossZ(hull[k - 2], hull[k - 1], pts[i]) <= T(0)) { + --k; + } + hull[k++] = pts[i]; + } + for (std::size_t i = m - 1, t = k + 1; i > 0; --i) { + while (k >= t and crossZ(hull[k - 2], hull[k - 1], pts[i - 1]) <= T(0)) { + --k; + } + hull[k++] = pts[i - 1]; + } + hull.resize(k - 1); // drop the duplicated start point + const std::size_t h = hull.size(); + if (h < 3) { + return; + } + + // Bounding-box dimensions {width, height} of the hull after rotating every + // point by R(-theta), where (c, s) = (cos theta, sin theta). + auto boxFor = [&](T c, T s) -> Point { + auto mnX = std::numeric_limits::max(); + auto mnY = std::numeric_limits::max(); + auto mxX = std::numeric_limits::lowest(); + auto mxY = std::numeric_limits::lowest(); + for (const auto& p : hull) { + const auto rx = c * p[0] + s * p[1]; + const auto ry = -s * p[0] + c * p[1]; + mnX = std::min(mnX, rx); + mnY = std::min(mnY, ry); + mxX = std::max(mxX, rx); + mxY = std::max(mxY, ry); + } + return {mxX - mnX, mxY - mnY}; + }; + + // Seed with the current (unrotated) box so we only rotate on a strict + // area improvement. + auto bestCos = T(1); + auto bestSin = T(0); + auto bestBox = boxFor(T(1), T(0)); + auto bestArea = bestBox[0] * bestBox[1]; + for (std::size_t i = 0; i < h; ++i) { + const auto& p0 = hull[i]; + const auto& p1 = hull[(i + 1) % h]; + const auto ex = p1[0] - p0[0]; + const auto ey = p1[1] - p0[1]; + const auto len = std::sqrt(ex * ex + ey * ey); + if (len <= T(0)) { + continue; + } + // Align this hull edge with the x-axis (theta = atan2(ey, ex)). + const auto c = ex / len; + const auto s = ey / len; + const auto box = boxFor(c, s); + const auto area = box[0] * box[1]; + if (area < bestArea) { + bestArea = area; + bestCos = c; + bestSin = s; + bestBox = box; + } + } + + // Stand the chart on its long axis: the packer sorts tallest-first and + // fills horizontal shelves, so the larger extent should be vertical. If the + // min-area box is wider than tall, compose an extra 90-degree rotation + // (R90 * R(-theta), where R90 maps (x, y) -> (-y, x)). + if (bestBox[0] > bestBox[1]) { + const auto c = bestCos; + const auto s = bestSin; + bestCos = s; + bestSin = -c; + } + + // Apply the chosen rotation to every vertex in place (skip the identity). + if (bestCos != T(1) or bestSin != T(0)) { + for (const auto& v : chart->vertices()) { + const auto x = v->pos[0]; + const auto y = v->pos[1]; + v->pos[0] = bestCos * x + bestSin * y; + v->pos[1] = -bestSin * x + bestCos * y; + } + } +} } // namespace detail /** @@ -4955,6 +5074,19 @@ struct VecDimensions> { */ template struct PackOptions { + /** + * @brief Rotate each chart in-plane to minimize its bounding-box area + * + * When `true` (default), each chart is rotated within its UV plane before + * layout so its axis-aligned bounding box has minimum area, then stood on + * its long axis (larger extent vertical) to match the tallest-first shelf + * strategy. Shelf packing works on axis-aligned boxes, so tightening and + * consistently orienting each box lets charts nest more densely. The + * rotation is applied in place and preserves topology and vertex identity, + * so any back-maps a caller holds remain valid. + */ + bool minimize_bounding_box{true}; + /** * @brief Fit the packed atlas into the unit square `[0,1]^2` * @@ -4990,14 +5122,17 @@ struct PackOptions { /** * @brief The bounding box of the packed atlas * - * @tparam T Floating-point scalar type + * `min`/`max` use the same vector type as the input meshes' vertex positions; + * only the first two (`u`, `v`) components are meaningful. + * + * @tparam VecType The vertex position vector type of the packed meshes */ -template +template struct PackResult { /** @brief Lower corner of the packed atlas */ - Vec min; + VecType min; /** @brief Upper corner of the packed atlas */ - Vec max; + VecType max; }; /** @@ -5006,10 +5141,10 @@ struct PackResult { * Lays out a list of already-parameterized charts (2D meshes whose vertex * `pos` holds `{u, v, ...}`) into a single shared frame using shelf packing, * so that no two charts' bounding boxes overlap. Operates purely on geometry: - * each chart's vertex positions are translated (and, when `normalize` is set, - * uniformly scaled) **in place**. Topology, vertex indices, and face indices - * are untouched, so any `ExtractedComponent` back-maps a caller holds remain - * valid after packing. + * each chart's vertex positions are rotated (when `minimize_bounding_box` is + * set), translated, and (when `normalize` is set) uniformly scaled **in + * place**. Topology, vertex indices, and face indices are untouched, so any + * `ExtractedComponent` back-maps a caller holds remain valid after packing. * * @par Scaling * By default charts keep their absolute scale and are only translated; the @@ -5036,7 +5171,9 @@ struct PackResult { * @par Complexity * `O(n log n)` in the number of charts `n` (dominated by the height sort) plus * `O(V)` in the total vertex count `V` (two passes: one to measure bounding - * boxes, one to apply the transform). Memory overhead is `O(n)`. + * boxes, one to apply the transform). With `minimize_bounding_box`, each chart + * additionally costs an `O(v log v)` convex hull plus an `O(h v)` orientation + * search over its `h` hull edges. Memory overhead is `O(n)`. * * @tparam MeshType A HalfEdgeMesh specialization * @param charts Charts to pack; each chart's vertex positions are modified @@ -5047,16 +5184,17 @@ struct PackResult { * vertices. */ template -auto PackCharts(std::vector& charts, - const PackOptions& opts = - PackOptions{}) -> PackResult +auto PackCharts( + std::vector& charts, + const PackOptions& opts = PackOptions{}) + -> PackResult().pos)>> { using T = typename MeshType::type; - static_assert( - detail::VecDimensions().pos)>::value >= 2, - "PackCharts requires mesh vertices with at least 2 position dimensions"); + using VecType = std::decay_t().pos)>; + static_assert(VecType::Dimensions >= 2, + "PackCharts requires mesh vertices with at least 2 position dimensions"); - PackResult result{Vec{T(0), T(0)}, Vec{T(0), T(0)}}; + PackResult result{}; if (charts.empty()) { return result; } @@ -5072,20 +5210,19 @@ auto PackCharts(std::vector& charts, if (not chart or chart->num_vertices() == 0) { throw std::invalid_argument("PackCharts: chart is null or has no vertices"); } - auto mnX = std::numeric_limits::max(); - auto mnY = std::numeric_limits::max(); - auto mxX = std::numeric_limits::lowest(); - auto mxY = std::numeric_limits::lowest(); - for (const auto& v : chart->vertices()) { - mnX = std::min(mnX, v->pos[0]); - mnY = std::min(mnY, v->pos[1]); - mxX = std::max(mxX, v->pos[0]); - mxY = std::max(mxY, v->pos[1]); + // Tighten the chart's bounding box by rotating it in-plane first. + if (opts.minimize_bounding_box) { + detail::MinimizeChartBoundingBox(chart); } - minX[i] = mnX; - minY[i] = mnY; - width[i] = mxX - mnX; - height[i] = mxY - mnY; + const auto& verts = chart->vertices(); + const auto cmpX = [](const auto& a, const auto& b) { return a->pos[0] < b->pos[0]; }; + const auto cmpY = [](const auto& a, const auto& b) { return a->pos[1] < b->pos[1]; }; + const auto [xlo, xhi] = std::minmax_element(verts.begin(), verts.end(), cmpX); + const auto [ylo, yhi] = std::minmax_element(verts.begin(), verts.end(), cmpY); + minX[i] = (*xlo)->pos[0]; + minY[i] = (*ylo)->pos[1]; + width[i] = (*xhi)->pos[0] - (*xlo)->pos[0]; + height[i] = (*yhi)->pos[1] - (*ylo)->pos[1]; } // Place taller charts first so shelves pack tightly. @@ -5160,7 +5297,9 @@ auto PackCharts(std::vector& charts, } } - result.max = Vec{atlasW * scale, atlasH * scale}; + // result.min is value-initialized to the origin; only u/v of max are set. + result.max[0] = atlasW * scale; + result.max[1] = atlasH * scale; return result; } @@ -5243,6 +5382,11 @@ auto MergeMeshes(const std::vector& meshes) -> Merge MergedMesh result{MeshType::New(), {}, {}}; auto& out = result.mesh; + // Gather every face's (offset) vertex indices so they can be inserted in a + // single insert_faces() call, which rebuilds the mesh boundary once at the + // end via update_boundary(). Inserting faces one at a time with + // insert_face() would leave the boundary stale. + std::vector> faces; for (std::size_t ci = 0; ci < meshes.size(); ++ci) { const auto& src = meshes[ci]; if (not src or src->num_vertices() == 0) { @@ -5262,10 +5406,11 @@ auto MergeMeshes(const std::vector& meshes) -> Merge for (const auto& edge : *face) { idxs.push_back(edge->vertex->idx + offset); } - out->insert_face(idxs); + faces.push_back(std::move(idxs)); result.face_source.emplace_back(ci, face->idx); } } + out->insert_faces(faces); return result; } diff --git a/tests/src/TestChartPacking.cpp b/tests/src/TestChartPacking.cpp index 6a0374c..8ffc1af 100644 --- a/tests/src/TestChartPacking.cpp +++ b/tests/src/TestChartPacking.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include #include @@ -25,6 +27,21 @@ auto MakeRectChart(float w, float h) -> Mesh::Pointer return m; } +/** @brief Build a w x h rectangle chart rotated by `theta` radians */ +auto MakeRotatedRectChart(float w, float h, float theta) -> Mesh::Pointer +{ + const float c = std::cos(theta); + const float s = std::sin(theta); + auto rot = [&](float x, float y) -> std::array { + return {c * x - s * y, s * x + c * y, 0.f}; + }; + std::vector> verts{rot(0.f, 0.f), rot(w, 0.f), rot(w, h), rot(0.f, h)}; + auto m = Mesh::New(); + m->insert_vertices(verts); + m->insert_faces({{0, 1, 2}, {0, 2, 3}}); + return m; +} + struct BBox { float minx{std::numeric_limits::max()}; float miny{std::numeric_limits::max()}; @@ -95,8 +112,11 @@ TEST(ChartPacking, ZeroAreaChartIsPlacedWithoutThrowing) TEST(ChartPacking, SingleChartMapsMinToOrigin) { + // Disable rotation so this test exercises translation/layout in isolation. std::vector charts{MakeRectChart(3.f, 2.f)}; - auto extent = PackCharts(charts); + PackOptions opts; + opts.minimize_bounding_box = false; + auto extent = PackCharts(charts, opts); auto b = ChartBBox(charts[0]); EXPECT_FLOAT_EQ(b.minx, 0.f); EXPECT_FLOAT_EQ(b.miny, 0.f); @@ -108,13 +128,17 @@ TEST(ChartPacking, SingleChartMapsMinToOrigin) TEST(ChartPacking, AbsoluteModePreservesChartSizes) { + // Disable rotation so the bounding boxes stay in their input orientation; + // this test is about absolute mode not rescaling charts. std::vector ws{1.f, 2.f, 0.5f, 3.f}; std::vector hs{1.f, 1.5f, 2.f, 0.7f}; std::vector charts; for (std::size_t i = 0; i < ws.size(); ++i) { charts.push_back(MakeRectChart(ws[i], hs[i])); } - PackCharts(charts); + PackOptions opts; + opts.minimize_bounding_box = false; + PackCharts(charts, opts); for (std::size_t i = 0; i < charts.size(); ++i) { auto b = ChartBBox(charts[i]); EXPECT_FLOAT_EQ(b.width(), ws[i]) << "chart " << i; @@ -196,6 +220,48 @@ TEST(ChartPacking, PaddingSurroundsChartsAtPerimeter) } } +// --- Bounding-box minimization (rotation) ---------------------------------- + +TEST(ChartPacking, MinimizeBoundingBoxTightensRotatedChart) +{ + // A 4x1 rectangle rotated off-axis has a loose axis-aligned bounding box. + // With minimize_bounding_box (the default), PackCharts rotates it back so + // the packed box collapses to the rectangle's true 4x1 area, standing the + // long axis vertical. + std::vector charts{MakeRotatedRectChart(4.f, 1.f, 0.6f)}; + PackCharts(charts); + auto b = ChartBBox(charts[0]); + EXPECT_NEAR(b.width() * b.height(), 4.f, 1e-2f); + // The long axis (~4) must be vertical; the short axis (~1) horizontal. + EXPECT_NEAR(b.height(), 4.f, 1e-2f); + EXPECT_NEAR(b.width(), 1.f, 1e-2f); +} + +TEST(ChartPacking, MinimizeBoundingBoxStandsWideChartUpright) +{ + // An axis-aligned chart that is already minimum-area but wider than tall is + // rotated 90 degrees so its long axis is vertical, matching the + // tallest-first shelf strategy. + std::vector charts{MakeRectChart(4.f, 1.f)}; + PackCharts(charts); + auto b = ChartBBox(charts[0]); + EXPECT_NEAR(b.width() * b.height(), 4.f, 1e-3f); + EXPECT_NEAR(b.height(), 4.f, 1e-3f); + EXPECT_NEAR(b.width(), 1.f, 1e-3f); +} + +TEST(ChartPacking, MinimizeBoundingBoxCanBeDisabled) +{ + // With minimization off, the off-axis rectangle keeps its loose bounding + // box, whose area is well above the rectangle's true 4x1 area. + std::vector charts{MakeRotatedRectChart(4.f, 1.f, 0.6f)}; + PackOptions opts; + opts.minimize_bounding_box = false; + PackCharts(charts, opts); + auto b = ChartBBox(charts[0]); + EXPECT_GT(b.width() * b.height(), 5.f); +} + // --- Normalize mode -------------------------------------------------------- TEST(ChartPacking, NormalizeFitsUnitSquare)