Merge pull request 'review: usability audit v0.10.0 — all 11 findings resolved' (#34) from review/usability-audit-2026-05-31 into main
Some checks failed
C++ Tests / test-fast (push) Failing after 2m11s
C++ Tests / quality-gates (push) Has been skipped
API Docs / doc-build (push) Has been skipped
Markdown link check / check (push) Has been skipped
Mirror to Codeberg / mirror (push) Successful in 32s
C++ Tests / test-cgal (push) Has been skipped
Some checks failed
C++ Tests / test-fast (push) Failing after 2m11s
C++ Tests / quality-gates (push) Has been skipped
API Docs / doc-build (push) Has been skipped
Markdown link check / check (push) Has been skipped
Mirror to Codeberg / mirror (push) Successful in 32s
C++ Tests / test-cgal (push) Has been skipped
This commit is contained in:
@@ -13,11 +13,11 @@ of all preceding units.
|
||||
| `load_mesh()` | Valid file path, supported format (OFF/OBJ/PLY) | Manifold, oriented, triangulated `ConformalMesh` |
|
||||
| `setup_*_maps()` | Triangulated mesh | Initialised property maps; `lambda0` zeroed |
|
||||
| `compute_*_lambda0_from_mesh()` | `setup_*_maps()` called | `lambda0[e]` set from 3-D edge lengths |
|
||||
| `check_gauss_bonnet()` | `theta_v[v]` set for all vertices | Throws `std::runtime_error` if Σ(2π−Θᵥ) ≠ 2π·χ(M) |
|
||||
| `enforce_gauss_bonnet()` | `theta_v[v]` set | Σ(2π−Θᵥ) = 2π·χ(M) guaranteed; `theta_v` modified |
|
||||
| `check_gauss_bonnet()` | `theta_v[v]` set · **EuclideanMaps or SphericalMaps only** | Throws `std::runtime_error` if Σ(2π−Θᵥ) ≠ 2π·χ(M). Deleted overload for `HyperIdealMaps` — see note below. |
|
||||
| `enforce_gauss_bonnet()` | `theta_v[v]` set · **EuclideanMaps or SphericalMaps only** | Σ(2π−Θᵥ) = 2π·χ(M) guaranteed; `theta_v` modified. Deleted overload for `HyperIdealMaps` — see note below. |
|
||||
| `newton_euclidean()` | GB satisfied · DOFs assigned · `lambda0` initialised | `NewtonResult.x` — converged scale factors; `.converged`, `.iterations`, `.grad_inf_norm` |
|
||||
| `newton_spherical()` | GB satisfied · DOFs assigned · gauge vertex pinned | Same as above |
|
||||
| `newton_hyper_ideal()` | `assign_all_dof_indices()` called · `lambda0` initialised | Same as above |
|
||||
| `newton_hyper_ideal()` | `assign_all_dof_indices()` called · **no `lambda0` needed** · **no GB pre-check** | Same as above. HyperIdeal computes lengths internally from b_v, a_e (via ζ₁₃/ζ₁₄/ζ₁₅); `lambda0` is irrelevant. Energy is strictly convex → Newton converges for any targets without a pre-check. |
|
||||
| `compute_cut_graph()` | Closed, orientable, triangulated mesh | `CutGraph.cut_edge_flags` — 2g seam edges; `.genus` |
|
||||
| `euclidean_layout()` | `newton_euclidean()` converged | `Layout2D.uv[v]` · `.halfedge_uv[h]` · `HolonomyData.translations` |
|
||||
| `spherical_layout()` | `newton_spherical()` converged | `Layout3D.xyz[v]` |
|
||||
@@ -53,17 +53,28 @@ The solver never writes to pinned DOFs. All indexing is 0-based and contiguous.
|
||||
|
||||
---
|
||||
|
||||
## Gauss–Bonnet — the most common source of failure
|
||||
## Gauss–Bonnet — the most common source of failure for Euclidean and Spherical
|
||||
|
||||
Prescribing angles that violate Gauss–Bonnet means no conformal factor can realise the
|
||||
target metric — the Newton solver will iterate indefinitely without converging.
|
||||
|
||||
```cpp
|
||||
// Option A: verify before solving (throws on violation)
|
||||
check_gauss_bonnet(mesh, maps);
|
||||
// Option A: verify before solving — throws std::runtime_error on violation.
|
||||
// Works with EuclideanMaps and SphericalMaps only.
|
||||
check_gauss_bonnet(mesh, euclidean_maps); // or spherical_maps
|
||||
check_gauss_bonnet(mesh, spherical_maps);
|
||||
|
||||
// Option B: auto-correct (redistributes defect uniformly across all vertices)
|
||||
enforce_gauss_bonnet(mesh, maps);
|
||||
// Option B: auto-correct — redistributes the defect uniformly across all vertices.
|
||||
enforce_gauss_bonnet(mesh, euclidean_maps);
|
||||
enforce_gauss_bonnet(mesh, spherical_maps);
|
||||
|
||||
// ⚠ HyperIdealMaps: both functions have deleted overloads — calling them is a
|
||||
// compile error. Reason: the correct hyperbolic Gauss–Bonnet identity includes
|
||||
// a surface area term: Σ(2π−Θᵥ) − Area(M) = 2π·χ(M), not Σ(2π−Θᵥ) = 2π·χ(M).
|
||||
// No pre-check is needed for HyperIdeal: the energy is strictly convex
|
||||
// (Springborn 2020 Theorem 1.3), so newton_hyper_ideal converges for any targets.
|
||||
```
|
||||
|
||||
The target violation is `|Σ(2π−Θᵥ) − 2π·χ(M)| > tol`. Default tolerance: `1e-10`.
|
||||
Applicable to closed meshes only — for open meshes, pin the boundary directly
|
||||
and skip the Gauss–Bonnet check (the identity does not hold for meshes with boundary).
|
||||
|
||||
@@ -75,6 +75,21 @@ cmake --build build -j$(nproc)
|
||||
> **Note:** `-DWITH_CGAL=ON` implies `-DWITH_VIEWER=ON`. Do not use this in headless
|
||||
> environments — it will fail with `Failed to find wayland-scanner`.
|
||||
|
||||
### Mode 4 — Low-memory build (RAM-constrained CI / Raspberry Pi ≤ 4 GB)
|
||||
|
||||
For machines where the CGAL build OOMs (peak ~700 MB per compilation unit at `-O3`):
|
||||
|
||||
```bash
|
||||
cmake -S code -B build -DWITH_CGAL_TESTS=ON \
|
||||
-DCONFORMALLAB_LOW_MEMORY_BUILD=ON
|
||||
cmake --build build --target conformallab_cgal_tests -j1
|
||||
```
|
||||
|
||||
`LOW_MEMORY_BUILD` applies four measures: `-O0` (no debug info), PCH off,
|
||||
unity batch size 1, `--no-keep-memory` linker flag. Drops cc1plus peak from
|
||||
~700 MB to ~150-200 MB per TU. Tests run ~15× slower at `-O0` but all pass.
|
||||
**Always use `-j1`** — parallel compilation would defeat the memory savings.
|
||||
|
||||
---
|
||||
|
||||
## Running a single test
|
||||
@@ -114,16 +129,53 @@ After a full build (`-DWITH_CGAL=ON`):
|
||||
./bin/conformallab_core --help
|
||||
```
|
||||
|
||||
### CLI parameter reference
|
||||
|
||||
| Flag | Default | Description |
|
||||
|------|---------|-------------|
|
||||
| `-i / --input` | *required* | Input mesh file (OFF / OBJ / PLY) |
|
||||
| `-o / --output` | *(none)* | Save layout as OFF file |
|
||||
| `-j / --json` | *(none)* | Serialise solver result + UV to JSON |
|
||||
| `-x / --xml` | *(none)* | Serialise solver result + UV to XML |
|
||||
| `-g / --geometry` | `euclidean` | Target geometry: `euclidean` · `spherical` · `hyper_ideal` |
|
||||
| `-s / --show` | `false` | Open the input mesh in the interactive viewer |
|
||||
| `-v / --verbose` | `false` | Print mesh topology, DOF counts, convergence details |
|
||||
|
||||
> **Tip:** `./bin/conformallab_core --help` always shows the canonical up-to-date
|
||||
> list generated by CLI11. The table above matches `conformallab_cli.cpp`
|
||||
> as of v0.10.0.
|
||||
|
||||
## Example programs
|
||||
|
||||
```bash
|
||||
# PRIMARY USE CASE: conformally flatten a mesh to the plane
|
||||
./build/examples/example_flatten code/data/obj/cathead.obj flat.off
|
||||
# → non-trivial u_v (e.g. range ≈ 2.96), real conformal deformation
|
||||
|
||||
# CGAL public API: one-call interface (natural theta by default)
|
||||
./build/examples/example_cgal_api [input.off]
|
||||
|
||||
# Full pipeline with JSON/XML serialisation and round-trip test
|
||||
./build/examples/example_layout [input.off] [layout.off] [result.json]
|
||||
|
||||
# Solver test (natural theta — u_v ≈ 0, used for pipeline validation)
|
||||
./build/examples/example_euclidean [input.off] [output.off]
|
||||
|
||||
# Hyper-ideal (hyperbolic) functional
|
||||
./build/examples/example_hyper_ideal [input.off] [output.off]
|
||||
./build/examples/example_viewer [input.off] # interactive, requires WITH_VIEWER
|
||||
|
||||
# Interactive viewer (requires WITH_VIEWER)
|
||||
./build/examples/example_viewer [input.off]
|
||||
```
|
||||
|
||||
`example_layout.cpp` is the best starting point — it shows the complete pipeline in ~120 lines.
|
||||
**Start here:** `example_flatten.cpp` shows the primary use case — real conformal
|
||||
flattening with `Θ_v = 2π`. `example_layout.cpp` adds JSON/XML serialisation.
|
||||
|
||||
> **Note on "natural theta":** `example_euclidean` and `example_layout` use the
|
||||
> "natural theta" testing trick (`Θ_v = actual angle sum at x=0`), which makes
|
||||
> `x* = 0` trivially the equilibrium. The output `u_v ≈ 0` is expected and
|
||||
> correct for a pipeline test, but means **no conformal deformation was applied**.
|
||||
> For real UV parameterisation, use `example_flatten.cpp`.
|
||||
|
||||
**Expected output of `example_euclidean` on the built-in quad-strip mesh:**
|
||||
```
|
||||
|
||||
556
doc/reviewer/usability-audit-2026-05-31.md
Normal file
556
doc/reviewer/usability-audit-2026-05-31.md
Normal file
@@ -0,0 +1,556 @@
|
||||
# Usability & Documentation Audit — ConformalLabpp v0.10.0
|
||||
|
||||
**Date:** 2026-05-31
|
||||
**Auditor:** External reviewer (Claude Sonnet 4.6)
|
||||
**Branch:** `review/usability-audit-2026-05-31`
|
||||
**Base:** `main` (post external-audit-2026-05-30 merge)
|
||||
**Focus:** Documentation quality, API usability, new-user experience
|
||||
|
||||
This document is self-contained. A new session can pick up any finding
|
||||
and act on it without prior context. Each finding includes exact file paths,
|
||||
the concrete problem, and a precise fix with acceptance criteria.
|
||||
|
||||
Status legend: 🔴 Usability bug · 🟡 Doc error/stale · 🟠 Missing content
|
||||
|
||||
---
|
||||
|
||||
## How to read this document in a new session
|
||||
|
||||
```bash
|
||||
git checkout review/usability-audit-2026-05-31
|
||||
|
||||
# No build needed for most fixes — documentation and example changes only.
|
||||
# For changes that touch compilable code (examples), verify with:
|
||||
cmake -S code -B build-cgal -DWITH_CGAL_TESTS=ON -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
|
||||
cmake --build build-cgal --target conformallab_cgal_tests -j1
|
||||
ctest --test-dir build-cgal -R '^cgal\.' --output-on-failure
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U1 — 🔴 CRITICAL: All examples show the trivial identity map, not real conformal flattening
|
||||
|
||||
### Location
|
||||
- `README.md` lines 100–121 ("Minimal usage")
|
||||
- `code/examples/example_euclidean.cpp` lines 72–83 (Step 4)
|
||||
- `code/examples/example_layout.cpp` lines 40–49 (`set_natural_theta`)
|
||||
- `code/examples/example_hyper_ideal.cpp` lines 89–98 (Step 3)
|
||||
|
||||
### Problem
|
||||
|
||||
Every example uses the "natural theta" trick:
|
||||
```cpp
|
||||
auto G0 = euclidean_gradient(mesh, x0, maps);
|
||||
for (auto v : mesh.vertices())
|
||||
if (maps.v_idx[v] >= 0)
|
||||
maps.theta_v[v] -= G0[maps.v_idx[v]];
|
||||
```
|
||||
|
||||
This sets Θ_v to the *actual angle sums at x=0*, making x*=0 the equilibrium.
|
||||
The result: `u_v ≈ 0` everywhere — **no deformation**. The map is identity.
|
||||
|
||||
Natural theta is a valid test pattern (it proves Newton converges in 0 or 1
|
||||
steps and guards against solver regressions). But as the *only* pattern shown to
|
||||
a new user, it gives the false impression that the library produces trivial
|
||||
output. The real use case — **conformally flatten a surface to the plane** —
|
||||
is never demonstrated.
|
||||
|
||||
### What is missing
|
||||
|
||||
A "real-world" example that shows the actual use case:
|
||||
|
||||
```cpp
|
||||
// Conformally flatten a mesh to the plane.
|
||||
// Set Θ_v = 2π (regular interior vertex, zero cone angle defect) for all
|
||||
// free vertices; enforce_gauss_bonnet makes the assignment topologically
|
||||
// consistent. The result is a flat conformal map with minimal angle distortion.
|
||||
for (auto v : mesh.vertices())
|
||||
maps.theta_v[v] = conformallab::TWO_PI;
|
||||
conformallab::enforce_gauss_bonnet(mesh, maps);
|
||||
auto res = conformallab::newton_euclidean(mesh, x0, maps);
|
||||
auto layout = conformallab::euclidean_layout(mesh, res.x, maps);
|
||||
// layout.uv[v.idx()] now holds 2D coordinates of the flattening
|
||||
```
|
||||
|
||||
### Fix
|
||||
|
||||
1. **Add a new example** `code/examples/example_flatten.cpp`:
|
||||
- Takes a real mesh from `code/data/` (e.g. `cathead.obj` or `torus_8x8.off`)
|
||||
- Sets Θ_v = 2π, calls `enforce_gauss_bonnet`, solves Newton, computes layout
|
||||
- Saves a UV-mapped OFF file
|
||||
- Shows non-trivial u_v values at output
|
||||
|
||||
2. **Update README "Minimal usage"** to show the flatten use case instead of
|
||||
(or in addition to) natural theta. Add a comment explaining what natural
|
||||
theta is and when to use it.
|
||||
|
||||
3. **Add a comment** to all existing examples explaining that natural theta
|
||||
is a *testing convenience*, not a typical use case.
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `example_flatten.cpp` compiles and runs with a real mesh input
|
||||
- [ ] Output shows non-trivial `u_v ≠ 0` and a visible UV parameterisation
|
||||
- [ ] README "Minimal usage" demonstrates real flattening (not identity)
|
||||
- [ ] Existing examples have a comment: "Note: natural theta → x* = 0 (testing
|
||||
pattern). For real flattening, set theta_v = TWO_PI and call enforce_gauss_bonnet."
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U2 — 🔴 CRITICAL: No example for the CGAL public API
|
||||
|
||||
### Location
|
||||
- `code/include/CGAL/Discrete_conformal_map.h` (the "user-facing entry")
|
||||
- `code/examples/` (no example uses CGAL::discrete_conformal_map_euclidean)
|
||||
|
||||
### Problem
|
||||
|
||||
`Discrete_conformal_map.h` is documented as "User-facing entry point for the
|
||||
Discrete_conformal_map package." The Doxygen doc shows a minimal usage snippet:
|
||||
|
||||
```cpp
|
||||
auto result = CGAL::discrete_conformal_map_euclidean(mesh);
|
||||
```
|
||||
|
||||
But this snippet only exists in a Doxygen comment — no compilable example
|
||||
demonstrates it. All four `code/examples/*.cpp` use the *internal* API
|
||||
(`setup_euclidean_maps + newton_euclidean`). A CGAL user who reaches this
|
||||
library via the CGAL ecosystem has no runnable starting point.
|
||||
|
||||
The CGAL API and the internal API also produce *different result types*:
|
||||
- Internal: `NewtonResult.x` — DOF-index-indexed `std::vector<double>`
|
||||
- CGAL: `Conformal_map_result.u_per_vertex` — vertex-index-indexed `std::vector<FT>`
|
||||
|
||||
This discrepancy is not explained anywhere for users.
|
||||
|
||||
### Fix
|
||||
|
||||
Add `code/examples/example_cgal_api.cpp`:
|
||||
```cpp
|
||||
// Demonstrates the CGAL public API (Discrete_conformal_map.h).
|
||||
// Contrast with example_euclidean.cpp which uses the internal API directly.
|
||||
|
||||
#include <CGAL/Simple_cartesian.h>
|
||||
#include <CGAL/Surface_mesh.h>
|
||||
#include <CGAL/Discrete_conformal_map.h>
|
||||
// ...
|
||||
auto result = CGAL::discrete_conformal_map_euclidean(mesh);
|
||||
// result.u_per_vertex[v.idx()] — per-vertex scale factor
|
||||
// result.converged, result.iterations, result.gradient_norm
|
||||
```
|
||||
|
||||
Also add to README: a short paragraph explaining when to use CGAL API
|
||||
vs internal API (CGAL API: simpler, less control; internal API: full pipeline,
|
||||
holonomy, period matrix).
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `example_cgal_api.cpp` compiles with `-DWITH_CGAL=ON`
|
||||
- [ ] Example uses `CGAL::discrete_conformal_map_euclidean` (not internal API)
|
||||
- [ ] README explains the two API levels and when to use each
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U3 — 🟡 DOC ERROR: `contracts.md` incorrect after Finding-B
|
||||
|
||||
### Location
|
||||
`doc/api/contracts.md` line 16
|
||||
|
||||
### Problem
|
||||
|
||||
```markdown
|
||||
| `check_gauss_bonnet()` | `theta_v[v]` set for all vertices |
|
||||
Throws `std::runtime_error` if Σ(2π−Θᵥ) ≠ 2π·χ(M) |
|
||||
```
|
||||
|
||||
After the external-audit-2026-05-30 Finding-B fix, calling
|
||||
`check_gauss_bonnet(mesh, hyper_ideal_maps)` is a **compile error**
|
||||
(the overload is `= delete`). The contract table implies it works for
|
||||
all map types — which is now false.
|
||||
|
||||
Also missing from the table: the HyperIdeal functional needs no
|
||||
Gauss-Bonnet pre-check because its energy is strictly convex
|
||||
(Springborn 2020 Theorem 1.3) — Newton converges for any target angles.
|
||||
|
||||
### Fix
|
||||
|
||||
```markdown
|
||||
| `check_gauss_bonnet()` | `theta_v[v]` set · **Euclidean or Spherical maps only** |
|
||||
Throws if Σ(2π−Θᵥ) ≠ 2π·χ(M). **Not applicable to HyperIdealMaps** —
|
||||
deleted overload; HyperIdeal needs no pre-check (strictly convex energy). |
|
||||
```
|
||||
|
||||
Also add a row for `enforce_gauss_bonnet`:
|
||||
```markdown
|
||||
| `enforce_gauss_bonnet()` | `theta_v[v]` set · **Euclidean or Spherical maps only** |
|
||||
Shifts all Θᵥ by δ = (lhs−rhs)/V so that GB holds exactly.
|
||||
**Not applicable to HyperIdealMaps** — deleted overload. |
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `check_gauss_bonnet` row notes "Euclidean/Spherical only, not HyperIdeal"
|
||||
- [ ] `enforce_gauss_bonnet` row added with same note
|
||||
- [ ] A brief explanation why HyperIdeal doesn't need GB: "strictly convex energy"
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U4 — 🟡 DOC ERROR: Stale version in README (v0.9.0)
|
||||
|
||||
### Location
|
||||
`README.md` line 17
|
||||
|
||||
### Problem
|
||||
|
||||
```markdown
|
||||
**Status:** v0.9.0 — Phases 1–9a complete, Phase 8b-Lite CGAL API surface.
|
||||
```
|
||||
|
||||
The current release is **v0.10.0** (tagged on `main`; CLAUDE.md confirms this).
|
||||
CLAUDE.md was updated to v0.10.0 but the README was not.
|
||||
|
||||
### Fix
|
||||
|
||||
```markdown
|
||||
**Status:** v0.10.0 — Phases 1–9b complete. Newton solvers for five DCE
|
||||
models (Euclidean / Spherical / HyperIdeal / CP-Euclidean / Inversive-Distance),
|
||||
priority-BFS layout in ℝ²/S²/Poincaré disk, Gauss–Bonnet, tree-cotree cut
|
||||
graph, Möbius holonomy, period matrix (genus 1), fundamental domain,
|
||||
halfedge_uv texture atlas, JSON/XML serialisation, CLI app.
|
||||
277 tests passing, 0 skipped — see [`doc/api/tests.md`](doc/api/tests.md).
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] README status line shows v0.10.0 and the correct feature list
|
||||
- [ ] Test count updated to 277
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U5 — 🟡 DOC ERROR: CGAL header comment describes superseded state
|
||||
|
||||
### Location
|
||||
`code/include/CGAL/Discrete_conformal_map.h` lines 14–16
|
||||
|
||||
### Problem
|
||||
|
||||
```cpp
|
||||
/// This header provides a single function — `discrete_conformal_map_euclidean`
|
||||
/// — that computes a Euclidean discrete-conformal flattening … Spherical and
|
||||
/// hyperbolic variants are scheduled for Phase 8b.2 once the Euclidean
|
||||
/// pattern is validated
|
||||
```
|
||||
|
||||
But the file already contains `discrete_conformal_map_spherical()` and
|
||||
`discrete_conformal_map_hyper_ideal()` (and the other models). The comment
|
||||
describes Phase 8a state; the file is at Phase 8b-Lite.
|
||||
|
||||
### Fix
|
||||
|
||||
Update the `\file` Doxygen block at the top of `Discrete_conformal_map.h`:
|
||||
|
||||
```cpp
|
||||
/*!
|
||||
\file CGAL/Discrete_conformal_map.h
|
||||
\ingroup PkgConformalMapRef
|
||||
|
||||
User-facing entry points for the Discrete_conformal_map package.
|
||||
|
||||
This header provides five functions covering all three DCE geometries:
|
||||
- `CGAL::discrete_conformal_map_euclidean` (flat / ℝ²)
|
||||
- `CGAL::discrete_conformal_map_spherical` (S², genus 0)
|
||||
- `CGAL::discrete_conformal_map_hyper_ideal` (H², genus ≥ 1)
|
||||
- `CGAL::discrete_circle_packing_euclidean` → Discrete_circle_packing.h
|
||||
- `CGAL::discrete_inversive_distance_map` → Discrete_inversive_distance.h
|
||||
...
|
||||
*/
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `\file` doc block lists all five entry functions
|
||||
- [ ] No forward-reference to "planned Phase 8b.2" — it is already implemented
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U6 — 🟡 STALE EXAMPLES: New gauge-vertex overload not reflected
|
||||
|
||||
### Location
|
||||
- `README.md` lines 107–110
|
||||
- `code/examples/example_euclidean.cpp` lines 62–68
|
||||
- `code/examples/example_layout.cpp` lines 52–59 (`pin_first` helper)
|
||||
|
||||
### Problem
|
||||
|
||||
Finding-D (external-audit-2026-05-30) added a clean gauge-vertex overload:
|
||||
```cpp
|
||||
assign_euclidean_vertex_dof_indices(mesh, maps, gauge_vertex);
|
||||
```
|
||||
|
||||
But all examples still use the old verbose loop:
|
||||
```cpp
|
||||
auto vit = mesh.vertices().begin();
|
||||
maps.v_idx[*vit++] = -1; // pinned
|
||||
int idx = 0;
|
||||
for (; vit != mesh.vertices().end(); ++vit)
|
||||
maps.v_idx[*vit] = idx++;
|
||||
```
|
||||
|
||||
A new user copying from the examples will write the old error-prone pattern
|
||||
instead of the new clean overload — defeating the purpose of the fix.
|
||||
|
||||
### Fix
|
||||
|
||||
Replace the old loop in all three locations with the new overload:
|
||||
```cpp
|
||||
// Pin the first vertex, assign sequential DOF indices to the rest.
|
||||
int n = assign_euclidean_vertex_dof_indices(mesh, maps,
|
||||
*mesh.vertices().begin());
|
||||
```
|
||||
|
||||
Same for `assign_vertex_dof_indices` (spherical) and
|
||||
`assign_inversive_distance_vertex_dof_indices` wherever they appear in examples.
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] README "Minimal usage" uses the overload
|
||||
- [ ] `example_euclidean.cpp` uses the overload
|
||||
- [ ] `example_layout.cpp` `pin_first` helper replaced with the overload
|
||||
- [ ] Old manual loop removed from all user-facing code
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U7 — 🟠 MISSING: `CONFORMALLAB_LOW_MEMORY_BUILD` absent from `getting-started.md`
|
||||
|
||||
### Location
|
||||
`doc/getting-started.md` — "Build modes" section
|
||||
|
||||
### Problem
|
||||
|
||||
`getting-started.md` documents five compile-time modes but does not list
|
||||
`CONFORMALLAB_LOW_MEMORY_BUILD=ON`. A Raspberry Pi or low-RAM user who
|
||||
reads `getting-started.md` (the natural first stop) cannot find the flag.
|
||||
It only appears in CLAUDE.md and the README table.
|
||||
|
||||
### Fix
|
||||
|
||||
Add to `getting-started.md` after the `CONFORMALLAB_FAST_TEST_BUILD` entry:
|
||||
|
||||
```markdown
|
||||
### Low-memory build (RAM-constrained CI / Raspberry Pi)
|
||||
|
||||
For machines with ≤ 4 GB RAM, the default CGAL build (PCH + -O3) OOMs
|
||||
the compiler. `LOW_MEMORY_BUILD` uses `-O0`, disables PCH, and sets
|
||||
unity batch size to 1, keeping `cc1plus` peak at ~150-200 MB per TU:
|
||||
|
||||
```bash
|
||||
cmake -S code -B build -DWITH_CGAL_TESTS=ON \
|
||||
-DCONFORMALLAB_LOW_MEMORY_BUILD=ON
|
||||
cmake --build build --target conformallab_cgal_tests -j1
|
||||
```
|
||||
|
||||
Tests run ~15× slower at -O0 but all pass. Useful for CI runners with
|
||||
limited memory.
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `getting-started.md` documents `CONFORMALLAB_LOW_MEMORY_BUILD`
|
||||
- [ ] Example command shows `-j1` (required for memory safety)
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U8 — 🟠 MISSING: `LOW_MEMORY_BUILD` absent from README compile-time table
|
||||
|
||||
### Location
|
||||
`README.md` lines 53–82 ("Compile-time workflow modes")
|
||||
|
||||
### Problem
|
||||
|
||||
The README code block shows five compile-time patterns but omits
|
||||
`CONFORMALLAB_LOW_MEMORY_BUILD`. This is the flag that re-enabled CI
|
||||
and is directly relevant to anyone building on resource-constrained hardware.
|
||||
|
||||
### Fix
|
||||
|
||||
Add to the README compile-time examples:
|
||||
```bash
|
||||
# Low-memory build: -O0, no PCH, unity batch 1 (~150 MB peak per cc1plus).
|
||||
# Use on Raspberry Pi / CI runners with ≤ 4 GB RAM. Tests run ~15× slower.
|
||||
cmake -S code -B build -DWITH_CGAL_TESTS=ON -DCONFORMALLAB_LOW_MEMORY_BUILD=ON
|
||||
cmake --build build --target conformallab_cgal_tests -j1
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] README shows `LOW_MEMORY_BUILD` usage with a one-line explanation
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U9 — 🟠 MISSING: `layout.uv` indexing semantics not documented
|
||||
|
||||
### Location
|
||||
`code/examples/example_layout.cpp` line 127
|
||||
`code/include/layout.hpp` — `Layout2D` struct definition
|
||||
|
||||
### Problem
|
||||
|
||||
```cpp
|
||||
auto& p = layout.uv[v.idx()]; // example_layout.cpp:127
|
||||
```
|
||||
|
||||
`Layout2D.uv` is a `std::vector` indexed by the raw integer `.idx()` of
|
||||
`Vertex_index`. This is undocumented: nowhere is it stated that the vector
|
||||
length equals `num_vertices(mesh)` and that the index is contiguous. In CGAL,
|
||||
`vertex.idx()` is contiguous on a fresh `Surface_mesh` but may have gaps after
|
||||
vertex removals.
|
||||
|
||||
The `halfedge_uv` field (also in `Layout2D`) has the same issue for halfedge
|
||||
indices.
|
||||
|
||||
### Fix
|
||||
|
||||
In `layout.hpp`, add explicit documentation to the `Layout2D` struct:
|
||||
|
||||
```cpp
|
||||
struct Layout2D {
|
||||
/// UV coordinate of vertex v in the layout plane.
|
||||
/// Indexed by `v.idx()` — length equals `mesh.number_of_vertices()`.
|
||||
/// \pre No vertices have been removed from `mesh` since construction
|
||||
/// (i.e., `mesh.is_valid(false)` and no compaction was performed).
|
||||
std::vector<Eigen::Vector2d> uv;
|
||||
|
||||
/// UV of source(h) as seen from face(h), indexed by `h.idx()`.
|
||||
/// At seam halfedges the two opposite halfedges carry different UV values,
|
||||
/// enabling proper per-halfedge UV for GPU texture atlasing.
|
||||
std::vector<Eigen::Vector2d> halfedge_uv;
|
||||
...
|
||||
};
|
||||
```
|
||||
|
||||
Also update `example_layout.cpp` to add a comment:
|
||||
```cpp
|
||||
// layout.uv is indexed by v.idx() — valid as long as no vertices were
|
||||
// removed from the mesh after loading.
|
||||
auto& p = layout.uv[v.idx()];
|
||||
```
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `Layout2D.uv` and `Layout2D.halfedge_uv` have explicit Doxygen docs
|
||||
stating the index semantics and the no-compaction precondition
|
||||
- [ ] `example_layout.cpp` has an inline comment at the access site
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U10 — 🟠 MISSING: CLI parameter reference not in README or getting-started
|
||||
|
||||
### Location
|
||||
- `README.md` (has CLI examples but no parameter table)
|
||||
- `doc/getting-started.md` (same)
|
||||
- `code/src/apps/v0/conformallab_cli.cpp` lines 333–340 (source of truth)
|
||||
|
||||
### Problem
|
||||
|
||||
The CLI app (`conformallab_core`) is the primary non-programmatic entry point
|
||||
for the library. The README shows a few usage examples but no parameter
|
||||
reference. A user running `--help` gets CLI11's auto-generated output, but
|
||||
that output is not reproduced anywhere in the documentation.
|
||||
|
||||
Current parameters (from `conformallab_cli.cpp`):
|
||||
|
||||
| Flag | Default | Description |
|
||||
|------|---------|-------------|
|
||||
| `-i / --input` | *required* | Input mesh (OFF / OBJ / PLY) |
|
||||
| `-o / --output` | *(none)* | Output layout OFF file |
|
||||
| `-j / --json` | *(none)* | Serialise result to JSON |
|
||||
| `-x / --xml` | *(none)* | Serialise result to XML |
|
||||
| `-g / --geometry` | `euclidean` | Target geometry: `euclidean` \| `spherical` \| `hyper_ideal` |
|
||||
| `-s / --show` | `false` | Open input mesh in interactive viewer |
|
||||
| `-v / --verbose` | `false` | Print topology / DOF stats |
|
||||
|
||||
### Fix
|
||||
|
||||
Add a concise parameter table to `doc/getting-started.md` in the
|
||||
"First run — CLI app" section. The README already shows the most common
|
||||
invocations; the table can live in getting-started to avoid README bloat.
|
||||
|
||||
### Acceptance criteria
|
||||
- [ ] `doc/getting-started.md` has a parameter table matching the source
|
||||
- [ ] Each flag has a one-line description and notes the default
|
||||
- [ ] Cross-reference to `--help` for the canonical up-to-date list
|
||||
|
||||
---
|
||||
|
||||
## FINDING-U11 — 🟠 ROADMAP: CLI missing three useful parameters + two models
|
||||
|
||||
### Location
|
||||
`code/src/apps/v0/conformallab_cli.cpp`
|
||||
|
||||
### Problem (not a bug — a gap for a future session to fill)
|
||||
|
||||
**Missing solver-tuning parameters** (low effort, high value):
|
||||
|
||||
The Newton tolerance and iteration limit are hardcoded at their defaults
|
||||
(`tol=1e-8`, `max_iter=200`). Any user who wants to solve a stiff mesh
|
||||
more carefully or stop early has to recompile.
|
||||
|
||||
```bash
|
||||
# Not yet possible:
|
||||
./conformallab_core -i hard_mesh.off -g euclidean --tol 1e-12 --max-iter 1000
|
||||
./conformallab_core -i quick_check.off --tol 1e-4 --max-iter 20
|
||||
```
|
||||
|
||||
**Missing geometry modes** (medium effort):
|
||||
|
||||
The two Phase-9a functionals are not reachable from the CLI at all:
|
||||
|
||||
```bash
|
||||
# Not yet possible — needs run_cp_euclidean() + run_inversive_distance() helpers:
|
||||
./conformallab_core -i mesh.off -g cp_euclidean
|
||||
./conformallab_core -i mesh.off -g inversive_distance
|
||||
```
|
||||
|
||||
CP-Euclidean (`Discrete_circle_packing.h`) and Inversive-Distance
|
||||
(`Discrete_inversive_distance.h`) are fully implemented in the library and
|
||||
exposed via the CGAL public API, but a CLI user cannot reach them.
|
||||
|
||||
### Suggested implementation (for a future session)
|
||||
|
||||
1. **`--tol <double>` and `--max-iter <int>`:** one `app.add_option` each,
|
||||
thread through all three `run_*` functions. ~15 lines.
|
||||
|
||||
2. **`-g cp_euclidean` and `-g inversive_distance`:** add two new
|
||||
`run_cp_euclidean()` / `run_inversive_distance()` helpers following
|
||||
the existing `run_euclidean()` pattern. ~60 lines each. Add to the
|
||||
`-g` `CLI::IsMember` list. Register the two new geometry strings in
|
||||
the dispatch block.
|
||||
|
||||
3. **Update README + getting-started** to list all five geometry modes.
|
||||
|
||||
### Effort estimate
|
||||
- `--tol` + `--max-iter`: ~30 min
|
||||
- Phase-9a CLI exposure: ~2 hours (mostly copy-paste + adaptation)
|
||||
- Tests: add two CLI smoke tests (geometry converges on tetrahedron)
|
||||
|
||||
### Acceptance criteria (when implemented)
|
||||
- [ ] `--tol` and `--max-iter` are accepted and forwarded to all three/five solvers
|
||||
- [ ] `./conformallab_core -i mesh.off -g cp_euclidean -o out.off` works
|
||||
- [ ] `./conformallab_core -i mesh.off -g inversive_distance -o out.off` works
|
||||
- [ ] `--help` output lists all five geometry modes
|
||||
|
||||
---
|
||||
|
||||
## Summary table
|
||||
|
||||
| ID | File | Type | Severity | Status |
|
||||
|----|------|------|----------|--------|
|
||||
| U1 | README + 4 examples | Usability | 🔴 Critical | ✅ Fixed 2026-05-31 |
|
||||
| U2 | examples/ (missing) | Usability | 🔴 Critical | ✅ Fixed 2026-05-31 |
|
||||
| U3 | `contracts.md:16` | Doc error | 🟡 Medium | ✅ Fixed 2026-05-31 |
|
||||
| U4 | `README.md:17` | Stale | 🟡 Medium | ✅ Fixed 2026-05-31 |
|
||||
| U5 | `Discrete_conformal_map.h:14` | Stale | 🟡 Medium | ✅ Fixed 2026-05-31 |
|
||||
| U6 | README + 2 examples | Stale | 🟡 Medium | ✅ Fixed 2026-05-31 |
|
||||
| U7 | `getting-started.md` | Missing | 🟠 Minor | ✅ Fixed 2026-05-31 |
|
||||
| U8 | `README.md` | Missing | 🟠 Minor | ✅ Fixed 2026-05-31 |
|
||||
| U9 | `layout.hpp` + example | Missing | 🟠 Minor | ✅ Fixed 2026-05-31 |
|
||||
| U10 | `getting-started.md` | Missing | 🟠 Minor | ✅ Fixed 2026-05-31 |
|
||||
| U11 | `conformallab_cli.cpp` | Roadmap | 🟠 Minor | ✅ Added to phases.md (9h) |
|
||||
|
||||
**Priority order for fixing:**
|
||||
1. ~~U1 + U2~~ ✅ done
|
||||
2. U3 (correctness — contracts.md wrong after Finding-B)
|
||||
3. U4 + U5 + U6 (stale content — quick fixes)
|
||||
4. U7 + U8 + U9 + U10 (missing docs — minor)
|
||||
5. U11 (roadmap — CLI extensions, not urgent)
|
||||
@@ -287,6 +287,37 @@ mesh type.
|
||||
no new library surface). Effort: small (~2–3 days).
|
||||
```
|
||||
|
||||
9h — CLI usability extensions (infrastructure — no Java equivalent)
|
||||
──────────────────────────────────────────────────────────────────
|
||||
|
||||
Identified by usability-audit-2026-05-31 (Finding-U11). Two independent
|
||||
sub-tasks; either can land independently.
|
||||
|
||||
```
|
||||
9h.1 Newton solver tuning parameters (~30 min)
|
||||
Currently hardcoded in all three run_*() helpers:
|
||||
tol = 1e-8, max_iter = 200
|
||||
Add to conformallab_cli.cpp:
|
||||
app.add_option("--tol", tol, "Newton gradient tolerance [1e-8]");
|
||||
app.add_option("--max-iter", max_iter, "Newton iteration limit [200]");
|
||||
Thread both through run_euclidean / run_spherical / run_hyper_ideal.
|
||||
Update getting-started.md CLI parameter table.
|
||||
Status: 🔲 planned. Effort: ~30 min. No dependencies.
|
||||
|
||||
9h.2 Phase-9a models in CLI (~2–4 hours)
|
||||
The CP-Euclidean and Inversive-Distance functionals are fully
|
||||
implemented in the library and exposed via the CGAL public API
|
||||
(Discrete_circle_packing.h, Discrete_inversive_distance.h), but
|
||||
a CLI user cannot reach them. Add:
|
||||
-g cp_euclidean → run_cp_euclidean()
|
||||
-g inversive_distance → run_inversive_distance()
|
||||
Following the existing run_euclidean() pattern (~60 lines each).
|
||||
Add both geometry strings to the CLI::IsMember validator.
|
||||
Update README + getting-started.md parameter table.
|
||||
Status: 🔲 planned. Effort: ~2–4 h. Requires: 9a complete ✅.
|
||||
Java reference: none (both functionals are research / literature ports).
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ◼ New research directions — Phases 10d–10g (2026 library scan)
|
||||
|
||||
Reference in New Issue
Block a user