Conversation
- Add libfontconfig1-dev to both CI workflows (required by plotters) - Update lint workflow Rust toolchain from 1.70.0 to stable - Update actions/checkout to v4 and rust-cache to v2 - Run cargo fmt to fix all formatting issues Closes #41 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses CI failures by installing the missing fontconfig system dependency in GitHub Actions and aligning the lint workflow toolchain, while also applying cargo fmt formatting changes across the Rust codebase.
Changes:
- Install
libfontconfig1-devin both2dtestandlintworkflows to satisfy theplotters -> font-kit -> yeslogic-fontconfig-sysnative dependency. - Update CI actions usage (checkout v4, rust-cache v2) and switch lint toolchain to
stable. - Apply
cargo fmtreformatting across source, tests, and benches.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/2dtest.yml |
Adds libfontconfig1-dev, updates checkout to v4, moves cache to rust-cache@v2 to unblock CI tests. |
.github/workflows/lint.yml |
Adds libfontconfig1-dev, updates lint toolchain to stable, uses rust-cache@v2 to unblock clippy/fmt CI. |
benches/mesh_benchmarks.rs |
Rustfmt-only formatting updates in benchmark setup code. |
src/advancing_front.rs |
Rustfmt-only formatting updates in implementation and tests. |
src/delaunay_refinement.rs |
Rustfmt-only formatting updates in function signature/logic and tests. |
src/export/gltf.rs |
Rustfmt-only formatting updates in glTF export + tests. |
src/export/gltf_quantized.rs |
Rustfmt-only formatting updates in quantized glTF export + tests. |
src/export/mod.rs |
Rustfmt-only formatting update to pub use formatting. |
src/export/obj.rs |
Rustfmt-only formatting updates in OBJ export + tests. |
src/export/stl.rs |
Rustfmt-only formatting updates in STL export tests. |
src/export/vtk.rs |
Rustfmt-only formatting updates in VTK export + tests. |
src/lib.rs |
Rustfmt-only formatting and module declaration reordering. |
src/marching_cubes.rs |
Rustfmt-only formatting updates in algorithm code and tests. |
src/model/tetrahedron.rs |
Rustfmt-only formatting update in circumsphere determinant calculation line wrapping. |
src/octree.rs |
Rustfmt-only formatting updates in implementation and tests. |
src/pipeline.rs |
Rustfmt-only formatting updates in tests. |
src/voxel_mesh.rs |
Rustfmt-only formatting updates in implementation and tests. |
src/wasm.rs |
Rustfmt-only formatting updates in wasm bindings and JS callback invocation formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for triangle in triangles { | ||
| let a_pos = vertices.iter().position(|(idx, _, _)| *idx == triangle.a.index).unwrap() + 1; | ||
| let b_pos = vertices.iter().position(|(idx, _, _)| *idx == triangle.b.index).unwrap() + 1; | ||
| let c_pos = vertices.iter().position(|(idx, _, _)| *idx == triangle.c.index).unwrap() + 1; | ||
| let a_pos = vertices | ||
| .iter() | ||
| .position(|(idx, _, _)| *idx == triangle.a.index) | ||
| .unwrap() | ||
| + 1; |
There was a problem hiding this comment.
Optional perf improvement: resolving vertex indices via vertices.iter().position(...) inside the triangle loop is O(V) per lookup (and repeated for a/b/c), making export scale as O(F·V). Consider building an index -> position map once after sorting vertices and using that for constant-time lookups.
| for face in faces { | ||
| let a_pos = vertices.iter().position(|(idx, _)| *idx == face.a.index).unwrap() + 1; | ||
| let b_pos = vertices.iter().position(|(idx, _)| *idx == face.b.index).unwrap() + 1; | ||
| let c_pos = vertices.iter().position(|(idx, _)| *idx == face.c.index).unwrap() + 1; | ||
| let a_pos = vertices | ||
| .iter() | ||
| .position(|(idx, _)| *idx == face.a.index) | ||
| .unwrap() | ||
| + 1; |
There was a problem hiding this comment.
Optional perf improvement: faces_to_obj does repeated vertices.iter().position(...) scans per face to compute indices. Building a precomputed index -> position map once can reduce this from O(F·V) to roughly O(F).
| for tet in tetrahedra { | ||
| let a = vertices.iter().position(|(idx, _)| *idx == tet.a.index).unwrap(); | ||
| let b = vertices.iter().position(|(idx, _)| *idx == tet.b.index).unwrap(); | ||
| let c = vertices.iter().position(|(idx, _)| *idx == tet.c.index).unwrap(); | ||
| let d = vertices.iter().position(|(idx, _)| *idx == tet.d.index).unwrap(); | ||
| let a = vertices | ||
| .iter() | ||
| .position(|(idx, _)| *idx == tet.a.index) | ||
| .unwrap(); | ||
| let b = vertices |
There was a problem hiding this comment.
Optional perf improvement: each tetrahedron maps point indices to VTK point ids by scanning vertices with .position(...) four times. Consider building a single index -> position lookup map once and reusing it to avoid O(C·V) behavior on large meshes.
| let mut indices = Vec::with_capacity(faces.len() * 3); | ||
| for face in faces { | ||
| let a = vertices.iter().position(|(idx, _)| *idx == face.a.index).unwrap() as u32; | ||
| let b = vertices.iter().position(|(idx, _)| *idx == face.b.index).unwrap() as u32; | ||
| let c = vertices.iter().position(|(idx, _)| *idx == face.c.index).unwrap() as u32; | ||
| let a = vertices | ||
| .iter() | ||
| .position(|(idx, _)| *idx == face.a.index) | ||
| .unwrap() as u32; |
There was a problem hiding this comment.
Optional perf improvement: index generation performs three .position(...) scans per face. Consider building an index -> position map after sorting vertices to keep indexing linear for large meshes.
| let mut indices = Vec::with_capacity(faces.len() * 3); | ||
| for face in faces { | ||
| for pt in [face.a, face.b, face.c] { | ||
| let pos = vertex_list.iter().position(|(idx, _)| *idx == pt.index).unwrap(); | ||
| let pos = vertex_list | ||
| .iter() | ||
| .position(|(idx, _)| *idx == pt.index) | ||
| .unwrap(); |
There was a problem hiding this comment.
Optional perf improvement: collect_unique_vertices uses vertex_list.iter().position(...) for every face vertex, which is O(F·V). Since vertex_list is sorted by index, using a precomputed index -> position map (or binary search) would make index generation scale better.
Summary
libfontconfig1-devsystem dependency to both CI workflows (required byplotters→font-kit→yeslogic-fontconfig-sys)1.70.0tostableactions/checkoutto v4 andrust-cacheto v2cargo fmtto fix all formatting issues across 16 source filesTest plan
2dtestworkflow passes (cargo check + cargo test)lintworkflow passes (cargo clippy + cargo fmt --check)Closes #41
🤖 Generated with Claude Code