Files
ConformalLabpp/doc/reviewer/usability-audit-2026-05-31.md
Tarik Moussa fb9a15ab4a doc(reviewer): add usability audit 2026-05-31
External documentation and usability review of v0.10.0.
9 findings covering documentation correctness, API usability,
and new-user experience.

Critical:
  U1 — All examples show trivial identity map (natural theta),
       not real conformal flattening — new users see no-op output
  U2 — CGAL public API has zero runnable examples

Medium:
  U3 — contracts.md incorrect: check_gauss_bonnet row missing
       HyperIdeal restriction (deleted overload after Finding-B)
  U4 — README still shows v0.9.0 (current: v0.10.0, 277 tests)
  U5 — Discrete_conformal_map.h header comment describes Phase-8a
       state; all 5 entry functions already implemented
  U6 — New gauge-vertex overload (Finding-D) not reflected in
       README or examples — old verbose loop still shown

Minor:
  U7 — CONFORMALLAB_LOW_MEMORY_BUILD absent from getting-started.md
  U8 — CONFORMALLAB_LOW_MEMORY_BUILD absent from README table
  U9 — Layout2D.uv index semantics undocumented (v.idx() contract)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-31 01:28:20 +02:00

16 KiB
Raw Blame History

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

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 100121 ("Minimal usage")
  • code/examples/example_euclidean.cpp lines 7283 (Step 4)
  • code/examples/example_layout.cpp lines 4049 (set_natural_theta)
  • code/examples/example_hyper_ideal.cpp lines 8998 (Step 3)

Problem

Every example uses the "natural theta" trick:

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:

// 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:

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:

// 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

| `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

| `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:

| `enforce_gauss_bonnet()` | `theta_v[v]` set · **Euclidean or Spherical maps only** |
  Shifts all Θᵥ by δ = (lhsrhs)/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

**Status:** v0.9.0 — Phases 19a 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

**Status:** v0.10.0 — Phases 19b complete. Newton solvers for five DCE
models (Euclidean / Spherical / HyperIdeal / CP-Euclidean / Inversive-Distance),
priority-BFS layout in ℝ²/S²/Poincaré disk, GaussBonnet, 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 1416

Problem

/// 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:

/*!
\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 107110
  • code/examples/example_euclidean.cpp lines 6268
  • code/examples/example_layout.cpp lines 5259 (pin_first helper)

Problem

Finding-D (external-audit-2026-05-30) added a clean gauge-vertex overload:

assign_euclidean_vertex_dof_indices(mesh, maps, gauge_vertex);

But all examples still use the old verbose loop:

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:

// 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:

### 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 5382 ("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.hppLayout2D struct definition

Problem

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:

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:

// 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

Summary table

ID File Type Severity Status
U1 README + 4 examples Usability 🔴 Critical 🟡 Open
U2 examples/ (missing) Usability 🔴 Critical 🟡 Open
U3 contracts.md:16 Doc error 🟡 Medium 🟡 Open
U4 README.md:17 Stale 🟡 Medium 🟡 Open
U5 Discrete_conformal_map.h:14 Stale 🟡 Medium 🟡 Open
U6 README + 2 examples Stale 🟡 Medium 🟡 Open
U7 getting-started.md Missing 🟠 Minor 🟡 Open
U8 README.md Missing 🟠 Minor 🟡 Open
U9 layout.hpp + example Missing 🟠 Minor 🟡 Open

Priority order for fixing:

  1. U1 + U2 (new examples — most impactful for new users)
  2. U3 (correctness — contracts.md wrong after Finding-B)
  3. U4 + U5 + U6 (stale content — quick fixes)
  4. U7 + U8 + U9 (missing docs — minor)