densify: T&T-tuned fusion, decoupled confidence recalibration, bounded fusion memory - #1309
Merged
Merged
Conversation
…sing - Added a webbing gate to the mesh cleaning process to enhance decimation accuracy. - Updated CleanParams in various locations to ensure proper handling of cavity-capping faces. - Introduced a new documentation file detailing tuned parameters for Tanks & Temples. - Refactored mesh refinement functions to streamline the cleaning and simplification processes.
…upled from fusion - fusion: a rescued point (kept only through the in-map prior) is dropped when any view contradicts it (sees behind it or disputes its normal); a supported point is dropped when normal-contradicting views outnumber its supporting views; prior weight 3 -> 4 - confidence recalibration: own depth tolerance (CONFIRM_DEPTH 0.5%) and a triangulation-angle independence weight replace fusion's depth/reprojection thresholds (CPU + CUDA) - fusion confidence floor on the recalibrated scale (FUSE_MIN_CONF 0.07) - fusion reprojection threshold default 1.0 -> 0.6 px, validated at R0 and R1 on six T&T scenes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…al T&T numbers - DMapCache::UseImage no longer caches a depth-map whose file fails to load (logged, left empty); both fusion loops skip such a reference, and EstimateNormalMaps skips it instead of aborting (which left later neighbors without normals). A neighbor flagged for the current reference cannot be evicted mid-fusion (LRU eviction stops at the reference), so no per-probe empty-map check is needed in the fusion walk - recalibration: CPU sweep uses the shared parameter snapshot and gate helpers of the CUDA kernel; both skip a neighbor whose depth weight is already negligible; AngleW with one sqrt - comments and parameter help compacted and brought to the current behavior - docs: fusion/confidence design docs describe the current state (floors, no line numbers, one-line rejected alternatives); T&T doc records the shipped defaults (6-scene cloud 0.7148, Truck 0.7625/0.6901, Meetingroom 0.5516/0.5068 cloud/mesh) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The fusion depth-map cache is re-budgeted after every fused map from the memory free at that moment, minus the safety margin and what the next map may add to the point-cloud, capped at four working sets and never below one. The PatchMatch CUDA/Metal pools are released once estimation ends, so fusion no longer starts with ~9 GB private. T&T R0 nv24: cloud F1 within 0.0003, fusion 6m10s->4m40s (Truck), 7m06s->5m51s (Meetingroom), fusion peak private 18-20 GB -> 15 GB. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical concurrency and distinct-view counting issues, along with cache and documentation corrections, remain unresolved.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This pull request retunes densification fusion and confidence recalibration, improves unreadable depth-map handling, bounds fusion memory, and updates mesh refinement.
Changes:
- Adds contradiction-aware fusion and decoupled CPU/CUDA confidence gates.
- Reworks cache budgeting and releases GPU pools before fusion.
- Separates mesh webbing cleanup from per-vertex refinement.
- Updates defaults, tests, documentation, and benchmark reporting.
Review notes:
- Critical (1 vote):
DMapCache.cpphas a concurrent load/failure cleanup race. - Critical (2 votes):
SceneDensify.cppmay count one contradicting view twice. - Moderate (1 vote): Cache budgeting may underestimate later point-cloud growth.
- Moderate (1 vote): Meetingroom memory figures in the documentation need reconciliation.
- Nit (1 vote): CLI help incompletely describes
fDepthDiffThreshold.
| File | Summary |
|---|---|
libs/MVS/SceneRefineCUDA.cpp |
Uses shared mesh preparation and simplification. |
libs/MVS/SceneRefineCommon.h |
Adds shared webbing-gated refinement helpers. |
libs/MVS/SceneRefine.cpp |
Uses shared mesh preparation and simplification. |
libs/MVS/SceneDensify.cpp |
Updates fusion, confidence, cache budgeting, and memory handling. |
libs/MVS/MeshHalfMesh.cpp |
Enforces mesh-cleaning invariants. |
libs/MVS/Mesh.h |
Updates cleanup defaults and invariants. |
libs/MVS/DMapCache.h |
Documents failed-load behavior. |
libs/MVS/DMapCache.cpp |
Handles unreadable depth-map loads. |
libs/MVS/DepthMap.h |
Updates fusion option documentation. |
libs/MVS/DepthMap.cpp |
Updates fusion defaults and descriptions. |
libs/MVS/ConfidenceRefine.h |
Adds shared recalibration parameters and angle weighting. |
libs/MVS/ConfidenceCUDA.h |
Updates CUDA confidence descriptors. |
libs/MVS/ConfidenceCUDA.cu |
Implements angle-weighted confidence gating. |
docs/TanksAndTemples.md |
Adds benchmark methodology and results. |
docs/design/MeshRefinement.md |
Documents mesh-stage separation. |
docs/design/DepthMapFusion.md |
Documents fusion guards and memory behavior. |
docs/design/DepthMapConfidence.md |
Documents confidence recalibration. |
apps/Viewer/Scene.h |
Updates viewer fusion defaults. |
apps/Viewer/Scene.cpp |
Enables mesh webbing cleanup. |
apps/TextureMesh/TextureMesh.cpp |
Enables mesh webbing cleanup. |
apps/Tests/TestsMVS.cpp |
Adds staged mesh-cleaning coverage. |
apps/DensifyPointCloud/DensifyPointCloud.cpp |
Updates CLI defaults and help text. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+99
to
+103
| if (!arrDepthData[idxImage].Load(fileName, loadFlags)) { | ||
| // corrupt or truncated file: leave the map empty and uncached, callers test IsEmpty() | ||
| arrDepthData[idxImage].Release(); | ||
| VERBOSE("error: depth-map '%s' could not be loaded", fileName.c_str()); | ||
| return false; |
Owner
Author
There was a problem hiding this comment.
This can't happen in the current callers, so no locking was added. The scenario needs two concurrent UseImage calls for the same image:
- Fusion loads the reference serially, and its OpenMP preload iterates that reference's neighbours, which are distinct views.
- The standalone confidence phase serializes same-image access with each
DepthData's own lock.
Two same-image loads would already race with each other without the Release() (both would write one DepthData and account its memory twice), so the precondition belongs to the caller. a3eccd8 documents it on UseImage: thread-safe for distinct images, and same-image calls must be serialized by the caller.
Comment on lines
+2694
to
+2696
| if (OPTDENSE::nFuseViolationMax < 0 || (rescued ? | ||
| fusedViolViews.size() + fusedNormViews.size() <= (unsigned)OPTDENSE::nFuseViolationMax : | ||
| fusedNormViews.size() <= fusedViews.size())) { |
Owner
Author
There was a problem hiding this comment.
Valid, fixed in a3eccd8. Free-space and normal contradictions now go into one deduplicated view set (fusedContraViews), so a view that contradicts both ways through different probes counts once. The normal-only set stays for the rule on supported points. Output is identical at the default nFuseViolationMax = 0, where the old sum was zero exactly when the union is empty.
…intrinsics - remove the recalibration compute-time accumulators, ConfAdjustRequest::computeNS, the dmap-read and peak-cache figures and the log lines that only reported them; DMapCache::GetHitStats unused - confidence kernels and ConfAdjustRequest take fx, fy, cx, cy instead of k00, k11, k02, k12 - AngleW states its precondition (X in front of both cameras) instead of a dead zero guard Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…recommended settings - MakeConfRefineParams is defined outside the CUDA block: the CPU confidence sweep uses it too - fusion's contradiction guard collects free-space and normal contradictions in one deduplicated view set, so a view contradicting both ways counts once (identical output at the default 0) - DMapCache::UseImage documents that same-image calls must be serialized by the caller - docs/TanksAndTemples.md opens with the recommended cloud and mesh settings; six-scene cloud and mesh tables at the shipped defaults (mean F1 0.7145 cloud, 0.6607 mesh) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| MDEFVAR_OPTDENSE_float(fDescriptorMinMagnitudeThreshold, "Descriptor Min Magnitude Threshold", "minimum patch texture variance accepted when matching two patches (0 - disabled)", "0.02") // 0.02: pixels with patch texture variance below 0.0004 (0.02^2) will be removed from depthmap; 0.12: patch texture variance below 0.02 (0.12^2) is considered texture-less | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "maximum relative difference between measured and depth projected pixel", "1.0") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "maximum variance allowed for the depths during fusion", "0.01") | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "fusion: maximum distance, in pixels, between a joining pixel and the fused point's projection", "0.6") |
| MDEFVAR_OPTDENSE_float(fDescriptorMinMagnitudeThreshold, "Descriptor Min Magnitude Threshold", "minimum patch texture variance accepted when matching two patches (0 - disabled)", "0.02") // 0.02: pixels with patch texture variance below 0.0004 (0.02^2) will be removed from depthmap; 0.12: patch texture variance below 0.02 (0.12^2) is considered texture-less | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "maximum relative difference between measured and depth projected pixel", "1.0") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "maximum variance allowed for the depths during fusion", "0.01") | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "fusion: maximum distance, in pixels, between a joining pixel and the fused point's projection", "0.6") |
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "maximum relative difference between measured and depth projected pixel", "1.0") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "maximum variance allowed for the depths during fusion", "0.01") | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "fusion: maximum distance, in pixels, between a joining pixel and the fused point's projection", "0.6") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "fusion: maximum relative depth difference between a joining pixel and the fused point (also the depth-map speckle and gap filters' agreement tolerance)", "0.01") |
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "maximum relative difference between measured and depth projected pixel", "1.0") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "maximum variance allowed for the depths during fusion", "0.01") | ||
| MDEFVAR_OPTDENSE_float(fDepthReprojectionErrorThreshold, "Depth Reprojection Error Threshold", "fusion: maximum distance, in pixels, between a joining pixel and the fused point's projection", "0.6") | ||
| MDEFVAR_OPTDENSE_float(fDepthDiffThreshold, "Depth Diff Threshold", "fusion: maximum relative depth difference between a joining pixel and the fused point (also the depth-map speckle and gap filters' agreement tolerance)", "0.01") |
| MDEFVAR_OPTDENSE_float(fNCCThresholdKeep, "NCC Threshold Keep", "Maximum 1-NCC score accepted for a match", "0.9", "0.5") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels (0 disables); default 3 favors completeness and suits the usual pipeline where mesh reconstruction follows and cleans the few extra outliers, use 2 when the dense point-cloud is the final output (fewer outliers, slightly lower completeness)", "3.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: max free-space-violating neighbor views allowed on a point rescued only by Fuse Prior Weight's virtual support (same free-space-violation test as the confidence recalibration); non-rescued points are never affected (-1 disables the guard, byte-identical to pre-guard fusion; 0 - strict/default, drop rescued points contradicted by any free-space ray)", "0") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels, granted only to points no view contradicts (0 disables)", "4.0") |
| MDEFVAR_OPTDENSE_float(fNCCThresholdKeep, "NCC Threshold Keep", "Maximum 1-NCC score accepted for a match", "0.9", "0.5") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels (0 disables); default 3 favors completeness and suits the usual pipeline where mesh reconstruction follows and cleans the few extra outliers, use 2 when the dense point-cloud is the final output (fewer outliers, slightly lower completeness)", "3.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: max free-space-violating neighbor views allowed on a point rescued only by Fuse Prior Weight's virtual support (same free-space-violation test as the confidence recalibration); non-rescued points are never affected (-1 disables the guard, byte-identical to pre-guard fusion; 0 - strict/default, drop rescued points contradicted by any free-space ray)", "0") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels, granted only to points no view contradicts (0 disables)", "4.0") |
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels (0 disables); default 3 favors completeness and suits the usual pipeline where mesh reconstruction follows and cleans the few extra outliers, use 2 when the dense point-cloud is the final output (fewer outliers, slightly lower completeness)", "3.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: max free-space-violating neighbor views allowed on a point rescued only by Fuse Prior Weight's virtual support (same free-space-violation test as the confidence recalibration); non-rescued points are never affected (-1 disables the guard, byte-identical to pre-guard fusion; 0 - strict/default, drop rescued points contradicted by any free-space ray)", "0") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels, granted only to points no view contradicts (0 disables)", "4.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: contradiction guard; max distinct views contradicting a point rescued only by Fuse Prior Weight's virtual support (seeing behind it, or agreeing in depth but disputing its normal), while a point kept on real support is dropped when the views disputing its normal outnumber its supporting views (-1 disables the guard, 0 - default)", "0") |
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels (0 disables); default 3 favors completeness and suits the usual pipeline where mesh reconstruction follows and cleans the few extra outliers, use 2 when the dense point-cloud is the final output (fewer outliers, slightly lower completeness)", "3.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: max free-space-violating neighbor views allowed on a point rescued only by Fuse Prior Weight's virtual support (same free-space-violation test as the confidence recalibration); non-rescued points are never affected (-1 disables the guard, byte-identical to pre-guard fusion; 0 - strict/default, drop rescued points contradicted by any free-space ray)", "0") | ||
| MDEFVAR_OPTDENSE_float(fFusePriorWeight, "Fuse Prior Weight", "fusion: weight of the intra-map geometric prior as virtual view/pixel support, to keep inliers on a coherent surface seen by too few views/pixels, granted only to points no view contradicts (0 disables)", "4.0") | ||
| MDEFVAR_OPTDENSE_int32(nFuseViolationMax, "Fuse Violation Max", "fusion: contradiction guard; max distinct views contradicting a point rescued only by Fuse Prior Weight's virtual support (seeing behind it, or agreeing in depth but disputing its normal), while a point kept on real support is dropped when the views disputing its normal outnumber its supporting views (-1 disables the guard, 0 - default)", "0") |
…ve; AngleW asserts - the fusion cache budget reports at normal verbosity, once per fusion, when it falls to the working set (it keeps going: the reserve is generous, so a tight budget costs re-reads) - FusionMemoryReserve counts the whole cList growth chain a large map triggers, peak = the last two buffers, instead of the first step only - AngleW asserts its positive denominator; ConfidenceRefine.h takes ASSERT from Config.h - T&T doc: Courthouse's R0 failure is recorded as a measurement of the previous memory handling Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…fidenceRefine.h Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
NormalFromGrad was only used by the kernel, so it moves into ConfidenceCUDA.cu; the normals are CUDA::Point3 and the neighbor rotation and normal reads Eigen maps over the descriptor arrays. ConfidenceCUDA.cu includes CUDA/Maths.h like the other CUDA sources, which also provides ASSERT. fountain-P11 R1: recalibrated confidence statistics within the kernel's run-to-run spread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Common/DepthGeometry.h holds, on Eigen types, what the depth-map code had in up to three copies: DepthSimilarity/IsDepthSimilar (moved from Util.inl, now also callable on the device), the least-squares depth plane fit, FitDepthGradient (3x3 depth-similar neighbors), the plane's normal and InterpolatePlaneDepth (PatchMatch plane propagation). Users: - PatchMatch CUDA: the 4-neighbor normal and neighbor plane propagation (its own copies removed); output bit-identical on 4M random cases, kernels 24-32 SASS instructions shorter; - confidence kernels: the prior's fit and normal, the camera as a LinearCameraModel; the device map views expose (row,col) and rows/cols like a DepthMap, so no accessor adapters are needed; - CPU: EstimateNormalMap, ComputeIntraMapPrior and DepthEstimator::InterpolatePixel; DepthGradientEstimator and ConfRefine::DepthPlaneFit/IsDepthSimilarF are removed. The CPU normal and propagation now run in float like the GPU (1 ULP; 21 of 4M propagations flip the fallback at the FLT_EPSILON guard or the dMax bound). HOST_DEVICE (Config.h) replaces CR_HD and REFINE_HD. fountain-P11 R1: confidence and point counts within the run-to-run spread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The ternary absolute difference introduced with DepthGeometry.h compiles to a data-dependent branch on MSVC, which mispredicts on noisy depths: the 3x3 depth-plane fit ran 2x slower than before (normal map 3072x2048: 201 ms vs 100 ms). Eigen::numext::abs is branchless on the host and the device, like the ABS it replaced, with identical results. Measured against ba66971 (P-cores, same flags): - CPU normal map (fit + normal) 6-9% faster, plane propagation 3-5% faster; - CUDA: device results bit-identical, PatchMatch kernels 24-32 SASS instructions shorter; - fountain-P11 R1 densify, 3 alternating runs each: estimation, fusion and total times within the run-to-run spread (total 11.26 s vs 11.25 s). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.
Tanks-and-Temples tuning of the densify fusion and confidence recalibration, a fix for unreadable depth-maps in fusion, and bounded fusion memory. Every number below comes from the official T&T evaluator, with every cloud and mesh re-aligned to the ground truth on every run.
Fusion
fFusePriorWeightgoes from 3 to 4.FUSE_MIN_CONF0.07).Confidence recalibration
Unreadable depth-maps
DMapCache::UseImageno longer caches a map whose file fails to load; the failure is logged. Both fusion loops skip such a reference.EstimateNormalMapsskips the map instead of aborting, which used to leave later neighbours without normals.Fusion memory
Mesh
CleanParamsadjustments in mesh cleaning.Validation
At
--resolution-level 0 --number-views 24, full densify thenReconstructMeshat defaults:The contradiction guard adds +0.015 mean cloud F1 over six scenes, with every scene improving; Ignatius and Barn were held out of the rule's design. The 6-scene mean cloud F1 at R0 is 0.7148. Full tables are in
docs/design/DepthMapFusion.md§5,docs/design/DepthMapConfidence.mdanddocs/TanksAndTemples.md.Supersedes #1308.
🤖 Generated with Claude Code