Repository navigation
Drop stale DevelopableMerge heap entries instead of re-queueing them (0.4.1) - #9
Merged
Merged
Conversation
The merge that changes a root's moments pushes every pair around it again at its true cost, so a re-queued stale entry is a duplicate. Duplicates grew by one per merge of either root, quadratic in the merges a dominant chart absorbs: a 147k-face block mesh spent 2,346 s in segmentation, now 68 s. Output is bit-identical; ctest 615/615, python 103/103. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The algorithm change is sound and release metadata is aligned; only a minor missing changelog link definition remains.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Optimizes chart segmentation by discarding redundant stale heap entries and prepares the 0.4.1 release.
Changes:
- Prevents quadratic heap growth in
DevelopableMerge. - Bumps package and documentation versions to 0.4.1.
- Adds performance results to the changelog.
| File | Description |
|---|---|
src/AtlasCharting.cpp |
Drops stale merge-heap entries. |
CMakeLists.txt |
Bumps project version. |
vcpkg.json |
Bumps manifest version. |
README.md |
Updates wheel installation URL. |
docs/PYTHON.md |
Updates wheel URL and documented version. |
CHANGELOG.md |
Documents the optimization and release. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). | ||
| This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [0.4.1] |
cdcseacave
added a commit
that referenced
this pull request
Oct 11, 2026
…reshold tests - DevelopableMerge counts heap pops (AtlasSegmentStats, now also for the initial merge). The mesh.ply stats test asserts pops <= pushes in every merge; the pre-0.4.1 re-queue fails it (49,955 pops for 15,034 pushes in the initial merge). - CloseHoles: the outline classifier flood-fills only the components that hold a fill candidate, instead of labelling the whole mesh, and no thread pool starts when there is nothing to fill. 800k-face closed sphere plus one isolated triangle: 32 ms -> 2.3 ms (1.4 ms without the triangle); new perf-harness guard. - Tests on both sides of the 3/4 outline share: a 16-sided washer filled at inner radius 0.65 and kept open at 0.66, a tetrahedron missing its base filled at height 1 and kept open at 0.2, at unit scale and scaled 1000x far from the origin. The hole regressions now check ValidateHalfMesh(). The shallow-shell case is documented with the washer. - CHANGELOG: link definitions for 0.4.1, 0.4.2 and 0.5.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Chart segmentation went quadratic on a mesh with one dominant chart.
DevelopableMergere-queued every stale heap entry at its new cost, but the merge that made it stale (doMerge, thentryPush(r, n)over every neighbour) had already pushed the same pair fresh. So every merge of either root added one more copy of each pair around it, and the work grew with the square of the merges a large chart absorbs.Stale entries are now dropped. Every stale entry has a fresh duplicate at its true cost, or would be rejected anyway (budget, blocked pair, or
wouldEnclose, which is monotone), so the change leaves the result bit-identical.Also bumps to 0.4.1 (CMakeLists, vcpkg.json, CHANGELOG, install URLs in README and docs/PYTHON.md). Tag
v0.4.1after merge to build the wheels.Measurements (L4 dev box, 2026-09-30)
Production block meshes from a 1,494-image DroneDeploy plan (radiance block pipeline), standalone
halfmesh.unwrapat 3840:Pushes, merges and chart counts are unchanged in every merge call, and the output PLY is byte-identical to the 0.4.0 build's on both meshes.
Tests
ctest: 615/615, including the bit-exact golden corpuspython/tests: 103/103🤖 Generated with Claude Code