From 5161b0a3f8ffe1fe396294b939734d5228b14119 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Mon, 20 Apr 2026 15:41:06 +1000 Subject: [PATCH] ifcviewer: remove meshopt_simplify path, keep only simplifySloppy Edge-collapse decimation (meshopt_simplify) returns BIM meshes unchanged due to per-triangle vertex duplication and non-manifold topology. The sloppy voxel-clustering decimator is faster, needs no shadow index welding, and produces good results at the sub-30px LOD1 threshold. Remove the non-sloppy branch, shadow buffer, IFC_LOD_SLOPPY and IFC_LOD_LOCK_BORDER env vars. Co-Authored-By: Claude Opus 4.6 --- src/ifcviewer/LodBuilder.cpp | 65 +++++++----------------------------- src/ifcviewer/LodBuilder.h | 4 +-- src/ifcviewer/README.md | 38 ++++++--------------- 3 files changed, 25 insertions(+), 82 deletions(-) diff --git a/src/ifcviewer/LodBuilder.cpp b/src/ifcviewer/LodBuilder.cpp index 35b97df44a..dbda389971 100644 --- a/src/ifcviewer/LodBuilder.cpp +++ b/src/ifcviewer/LodBuilder.cpp @@ -37,28 +37,17 @@ void buildLods(SidecarData& sd, const size_t total_vertex_count = sd.vertices.size() / vtx_stride_bytes; // Env var knobs so we can tune without rebuilding. - // IFC_LOD_LOCK_BORDER=1 re-enable LockBorder (off by default: BIM - // geometry is often non-manifold so locking - // borders prevents any collapse). // IFC_LOD_ERROR= override target_error (default 0.05 → 0.2). // IFC_LOD_RATIO= override target_ratio. // IFC_LOD_MIN_SAVINGS=<0..1> minimum fraction of tris saved to accept // (default 0.25). // IFC_LOD_DEBUG=1 print per-mesh diagnostics for the first // few meshes of each call. - // IFC_LOD_SLOPPY=0 disable sloppy (clustering) decimator. - // Default ON: BIM brep output is usually - // non-manifold, so edge-collapse simplify - // returns the input unchanged. - const char* env_lock = std::getenv("IFC_LOD_LOCK_BORDER"); const char* env_err = std::getenv("IFC_LOD_ERROR"); const char* env_ratio = std::getenv("IFC_LOD_RATIO"); const char* env_savings = std::getenv("IFC_LOD_MIN_SAVINGS"); const char* env_debug = std::getenv("IFC_LOD_DEBUG"); - const char* env_sloppy = std::getenv("IFC_LOD_SLOPPY"); - const bool lock_border = env_lock && env_lock[0] == '1'; - const bool use_sloppy = !(env_sloppy && env_sloppy[0] == '0'); if (env_err) target_error = static_cast(std::atof(env_err)); if (env_ratio) target_ratio = static_cast(std::atof(env_ratio)); float min_savings = 0.25f; @@ -71,10 +60,8 @@ void buildLods(SidecarData& sd, // Scratch buffers reused across meshes so we only allocate once. std::vector simplified; - std::vector shadow; std::vector dequant_pos; // 3 floats/vertex, dequantized simplified.reserve(1024); - shadow.reserve(1024); dequant_pos.reserve(1024 * 3); int dbg_printed = 0; @@ -128,46 +115,18 @@ void buildLods(SidecarData& sd, const size_t target_index_count = std::max( 3, static_cast(mesh.index_count * target_ratio) / 3 * 3); - // The instanced VBO stores each triangle's vertices separately, so the - // mesh's index buffer is topologically disconnected — every edge is - // boundary, every vertex is unique, and meshopt_simplify can't collapse - // anything. Build a shadow index buffer that welds by position, so - // shared-position vertices share an ID; then simplify on that. Output - // indices are still valid mesh-local IDs (canonical representatives), - // usable directly as LOD1 indices against the same VBO. - shadow.resize(mesh.index_count); - meshopt_generateShadowIndexBuffer( - shadow.data(), - indices, mesh.index_count, - positions, mesh.vertex_count, - sizeof(float) * 3, // compare only xyz - local_pos_stride); - + // Cluster-based (sloppy) decimator. Ignores topology entirely; + // ideal for BIM brep output which is usually non-manifold / has + // T-junctions / per-triangle vertex duplication. Quantises + // positions into voxel cells — no welding needed. simplified.resize(mesh.index_count); float result_error = 0.0f; - size_t new_index_count = 0; - - if (use_sloppy) { - // Cluster-based decimator. Ignores topology entirely; great for - // BIM brep output which is usually non-manifold / has T-junctions. - // Operates directly on the original indices — welding isn't - // needed since it quantises positions into voxel cells. - new_index_count = meshopt_simplifySloppy( - simplified.data(), - indices, mesh.index_count, - positions, mesh.vertex_count, local_pos_stride, - target_index_count, target_error, - &result_error); - } else { - const unsigned int options = - lock_border ? static_cast(meshopt_SimplifyLockBorder) : 0u; - new_index_count = meshopt_simplify( - simplified.data(), - shadow.data(), mesh.index_count, - positions, mesh.vertex_count, local_pos_stride, - target_index_count, target_error, - options, &result_error); - } + size_t new_index_count = meshopt_simplifySloppy( + simplified.data(), + indices, mesh.index_count, + positions, mesh.vertex_count, local_pos_stride, + target_index_count, target_error, + &result_error); if (debug && dbg_printed < 8) { std::fprintf(stderr, @@ -202,9 +161,9 @@ void buildLods(SidecarData& sd, if (debug) { std::fprintf(stderr, " [lod] summary: accepted=%d rejected_noreduce=%d rejected_savings=%d " - "(lock_border=%d target_error=%.3f target_ratio=%.3f min_savings=%.3f)\n", + "(target_error=%.3f target_ratio=%.3f min_savings=%.3f)\n", dbg_accepted, dbg_rejected_noreduce, dbg_rejected_savings, - lock_border ? 1 : 0, target_error, target_ratio, min_savings); + target_error, target_ratio, min_savings); } } diff --git a/src/ifcviewer/LodBuilder.h b/src/ifcviewer/LodBuilder.h index 0147ba82f9..df1638b58f 100644 --- a/src/ifcviewer/LodBuilder.h +++ b/src/ifcviewer/LodBuilder.h @@ -23,8 +23,8 @@ #include "SidecarCache.h" // Build a LOD1 index slice for every mesh in `sd` whose triangle count is -// above `min_triangles`, using meshoptimizer's edge-collapse decimator. The -// LOD1 indices are appended to `sd.indices`; each MeshInfo's +// above `min_triangles`, using meshoptimizer's sloppy (voxel-clustering) +// decimator. The LOD1 indices are appended to `sd.indices`; each MeshInfo's // lod1_ebo_byte_offset + lod1_index_count are populated to point at the // appended range. Meshes that don't qualify (too small) or where the // decimator couldn't meet the target within the error budget have diff --git a/src/ifcviewer/README.md b/src/ifcviewer/README.md index 25e52e8ce3..eb1e0d88df 100644 --- a/src/ifcviewer/README.md +++ b/src/ifcviewer/README.md @@ -540,44 +540,28 @@ shader) is unchanged. ##### Decimator choice: `meshopt_simplifySloppy` -The first attempt used `meshopt_simplify`, which is an edge-collapse -decimator. It returned every input mesh unchanged (`err = 0.0`) for two -reasons, both inherent to BIM brep output: - -1. **Per-triangle vertex duplication.** The instanced VBO stores each - triangle's vertices separately so that hard-edge normals can differ - across triangles. Topologically there are no shared vertices, so no - edges exist for `meshopt_simplify` to collapse. A - `meshopt_generateShadowIndexBuffer` welding pass (hash xyz only, - ignore the interleaved normal/colour) fixes this half cheaply — the - VBO isn't touched, only a per-call shadow index buffer is built. -2. **Non-manifold topology even after welding.** BIM brep output has - T-junctions, coplanar slivers, separate solids meeting at a plane, - and multi-material cuts. `meshopt_simplify` needs valid 2-manifold - edge pairs to score collapses; it refuses the non-manifold ones, the - priority queue never fires, and it returns the input untouched. - `meshopt_simplifySloppy` is a **voxel-clustering decimator** — it quantises positions into cells and merges everything in a cell to a -single point. Topology is irrelevant, so it works directly on the -original indices (welding isn't even needed). The trade-off is that it -rounds off sharp corners and can produce slightly degenerate triangles, -so it doesn't look great at mid-screen size. For a LOD1 that only -activates below 30 px projected radius that's invisible in practice. If -you ever want LOD1 to remain active at larger sizes, the only robust -fix is to pre-process BIM meshes into manifold form (fuse coplanar -faces, split at T-junctions) — a significant project unto itself. +single point. This is the only meshoptimizer decimator that works on +BIM brep output, which has per-triangle vertex duplication (hard-edge +normals) and non-manifold topology (T-junctions, coplanar slivers, +separate solids meeting at a plane). The edge-collapse decimator +(`meshopt_simplify`) needs 2-manifold edge pairs to score collapses; +on BIM geometry it returns the input unchanged. + +`simplifySloppy` rounds off sharp corners and can produce slightly +degenerate triangles, so it doesn't look great at mid-screen size. +For a LOD1 that only activates below 30 px projected radius that's +invisible in practice. ##### Tuning knobs (env vars) | Var | Default | Effect | |-----|---------|--------| | `IFC_LOD1_PX` | `30` | Projected sphere radius (px) below which LOD1 kicks in. `0` disables LOD1 entirely. | -| `IFC_LOD_SLOPPY` | `1` | `0` falls back to edge-collapse (`meshopt_simplify`) on shadow-welded indices. Typically produces zero LOD1 output for BIM — useful only for A/B comparison. | | `IFC_LOD_ERROR` | `0.2` | Target relative error passed to meshopt. | | `IFC_LOD_RATIO` | `0.25` | Target triangle-count ratio (LOD1 aims for 25 % of LOD0 tris). | | `IFC_LOD_MIN_SAVINGS` | `0.25` | Reject the LOD1 result if it doesn't shave at least this fraction of triangles. | -| `IFC_LOD_LOCK_BORDER` | `0` | `1` re-enables `meshopt_SimplifyLockBorder` (only meaningful with `IFC_LOD_SLOPPY=0`). | | `IFC_LOD_DEBUG` | `0` | `1` prints per-mesh `tris / target / got / err` for the first 8 candidate meshes plus an accept/reject summary per model. | ##### Measured results