viewer: point-cloud confidence filter, plus resize/K, gizmo and refine-default fixes - #1310
Merged
Merged
Conversation
… adjustment Viewer: a Point Confidence panel (open by default while the active layer's cloud carries per-point confidence, e.g. a .dmap) hides the points outside a [min, max] confidence window. The confidence is quantized to 8 bits and kept in the alpha of the point color, now stored as RGBA bytes instead of three floats (16 instead of 24 bytes per point on the GPU); the window is a per-layer shader uniform, so moving a slider uploads nothing and point indices (picking, selection) never change. Box/lasso selection applies the same test. Densify: replace the "skipping the postprocess confidence-adjust phase" message with the number of depth-maps whose confidence was adjusted on the GPU/CPU, and a warning for those where the adjustment failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… survive Mesh::Clean's first stage (halfmesh RemoveLongEdgeFacesCapped) ran with the halfmesh default cone 0.35. A long face at height h above a surface that crosses it is capped once h < cone x reach x its longest edge, so at 0.35 (1.4 x the edge) the gate punched holes along every wall base, column and stair side, in surface the graph-cut filled plausibly. Cone 0.2 (reach 4, candidate factor 2 unchanged) keeps those fills while the doorway debris and the border skirt of giant faces still go. Measured on the same raw surface per scene, 16 factor/reach/cone arms: Herz-Jesu-P8 boundary edges 17495 -> 12121 (-31%), F1 0.5261 -> 0.5255; Ignatius 0.7459 -> 0.7456; Truck 0.6731 -> 0.6646 (its truck-bed lid is the one thing the wider cone buys). The design doc's gate table is rewritten with the full sweep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SetupGizmoBuffers passed indices.size() * sizeof(uint32_t) to the typed VBO::SetData(const T*, count), which multiplies by sizeof(T) again: the upload asked the driver for 4x the index bytes and it read 3x past the end of the vector. It crashed at startup only when the overread crossed into an unmapped page (about 1 run in 8 under load, fault in the driver at a page-aligned address). Pass the vector instead, and drop the untyped byte-sized SetData/SetSubData overloads: unused, and the same-shaped count/byte pair is what made the mistake compile. A void* argument now fails to build instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resizes paired with a camera mapped pixels differently from how that camera's K was scaled, off by up to half a pixel (more on neighbor views) on odd-sized axes: - Image::ResizeImage/GetImage, ImageCache, the refine images and the CUDA/Metal neighbor depth priors resized to an explicit size (each axis by its own rounded ratio) while GetCamera() scales K uniformly by the ratio of the largest sides; - the neighbor views scaled to the reference resolution resized by the factor while their K came from the rounded size; - DepthMapsData::ScaleDepthData and the SGM levels resized by the factor while K was scaled per axis from the rounded size (and ScaleDepthData rebuilt K from the full image, not from a neighbor's already scaled camera). TImage::computeResize now does what its comment states: the largest side is rounded and the other follows from the resulting uniform factor, the grid GetCamera() assumes (same sizes as before wherever the scale comes from GetSizeScale). Image::ResizeUniform resizes a raster onto that grid with one factor on both axes; the pyramids resize to explicit sizes to match their per-axis K. ImportIgnoreMask resizes into a local buffer instead of the mask stored in the scene, which makes ImportKeepMask redundant. ImageResizeCameraTest checks every path: blob centroids vs projections 0.18-0.69 px off before, <= 0.009 px after. Truck (COLMAP OPENCV poses, --number-views 24): -r0 F1 0.7907 -> 0.7911, -r1 0.7763 -> 0.7744, both within the 0.003 repeat spread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RefineMesh was the only app whose --gpu-device defaulted to empty (CPU), so a CUDA build refined on the CPU unless the flag was given. It now defaults to -1; --gpu-device cpu (or -2) selects the CPU path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f272092 lowered the gate's reach from 4 to 2: it leaves fewer crease holes (Herz-Jesu-P8 boundary edges 12121 -> 7334) and the meshes look better to the eye, at a mean F1 of 0.6394 vs 0.6452 (Truck -0.016, its truck-bed lid survives). The design doc now describes reach 2 as shipped and why, and the code notes that reach 5 is the most accurate (0.6467). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RefineMesh replaced the stage name instead of appending to it: scene_dense.mvs -m scene_dense_mesh.ply wrote scene_dense_refine.*. The default output is now the input mesh's name plus "_refined" (the input scene's when the mesh comes with it), in the input scene's folder: scene_dense_mesh_refined.*, like the other stages that append to the previous one. The usage docs, the design doc and the pipeline scripts use the new name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Confidence-filter state can hide unweighted clouds, while normals and existing selection highlights bypass the filter.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds Viewer confidence filtering alongside image/camera resize consistency, mesh cleanup, refinement defaults, reporting, and buffer safety fixes.
Changes:
- Adds per-layer point-confidence filtering for rendering, picking, and selection.
- Aligns raster resizing with camera intrinsics and adds regression coverage.
- Updates mesh cleanup parameters, refinement defaults/naming, GPU reporting, and gizmo uploads.
| File | Description |
|---|---|
scripts/python/MvsScalablePipeline.py |
Uses refined-output suffixes. |
scripts/python/MvgMvsPipeline.py |
Updates refinement pipeline filenames. |
libs/MVS/SemiGlobalMatcher.h |
Uses explicit pyramid sizes. |
libs/MVS/SemiGlobalMatcher.cpp |
Aligns grayscale resize with intrinsics. |
libs/MVS/SceneRefineCommon.cpp |
Uses uniform resizing and shared mask import. |
libs/MVS/SceneDensify.h |
Adds confidence-adjustment counters. |
libs/MVS/SceneDensify.cpp |
Fixes pyramid cameras and reports adjustments. |
libs/MVS/PatchMatchMetal.mm |
Uniformly resizes depth priors. |
libs/MVS/PatchMatchCUDA.cpp |
Uniformly resizes depth priors. |
libs/MVS/MeshHalfMesh.cpp |
Narrows capped-face cleanup parameters. |
libs/MVS/ImageCache.cpp |
Aligns cached raster resizing. |
libs/MVS/Image.h |
Declares uniform resize helper. |
libs/MVS/Image.cpp |
Implements camera-consistent resizing. |
libs/MVS/DepthMap.h |
Uses uniform scaling and consolidates mask API. |
libs/MVS/DepthMap.cpp |
Makes mask import non-mutating. |
libs/Common/Types.inl |
Revises uniform resize-size calculation. |
docs/wiki/Usage.md |
Updates refined output examples. |
docs/design/SemiGlobalMatching.md |
Documents explicit pyramid sizing. |
docs/design/MeshRefinement.md |
Documents GPU and output defaults. |
docs/design/DelaunayMeshReconstruction.md |
Documents cleanup tuning and validation. |
apps/Viewer/Window.cpp |
Integrates confidence filtering with UI and selection. |
apps/Viewer/UI.h |
Declares confidence panel state/API. |
apps/Viewer/UI.cpp |
Implements confidence controls. |
apps/Viewer/shaders/pointcloud.vert |
Filters rendered points by confidence. |
apps/Viewer/shaders/picker_points.vert |
Filters pickable points by confidence. |
apps/Viewer/SelectionController.h |
Extends point classification API. |
apps/Viewer/SelectionController.cpp |
Excludes filtered points from selection. |
apps/Viewer/Scene.h |
Adds per-layer confidence state. |
apps/Viewer/Scene.cpp |
Refreshes confidence ranges. |
apps/Viewer/Renderer.h |
Adds confidence window render state. |
apps/Viewer/Renderer.cpp |
Uploads RGBA8 confidence and fixes gizmo upload. |
apps/Viewer/Common.h |
Implements confidence quantization/filtering. |
apps/Viewer/BufferObjects.h |
Removes byte-oriented VBO overloads. |
apps/Viewer/BufferObjects.cpp |
Removes unsafe overload implementations. |
apps/Viewer/AGENTS.md |
Documents confidence panel architecture. |
apps/Tests/TestsMVS.h |
Declares resize-camera regression test. |
apps/Tests/TestsMVS.cpp |
Tests raster/projection consistency. |
apps/Tests/Tests.cpp |
Registers the new test. |
apps/RefineMesh/RefineMesh.cpp |
Changes GPU and output-name defaults. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+137
to
+145
| if (!pointcloud.pointWeights.empty()) { | ||
| ASSERT(pointcloud.pointWeights.size() == pointcloud.points.size()); | ||
| minConf = FLT_MAX; maxConf = -FLT_MAX; | ||
| for (const MVS::PointCloud::WeightArr& weights: pointcloud.pointWeights) { | ||
| const float conf(Confidence(weights)); | ||
| minConf = MINF(minConf, conf); | ||
| maxConf = MAXF(maxConf, conf); | ||
| } | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in c7abad0: PointConfidenceFilter::Reset now resets the filter to the all-visible window when the cloud has no pointWeights, and IsShown asserts that a narrowed window only exists over a cloud with confidence.
| if (levelMin != filter.levelMin || levelMax != filter.levelMax) { | ||
| filter.levelMin = levelMin; | ||
| filter.levelMax = levelMax; | ||
| window.GetRenderer().SetPointConfidence(layer->id, filter); |
Owner
Author
There was a problem hiding this comment.
Fixed in c7abad0: the normal lines carry the point's quantized confidence in the w of their vertices and RenderPointCloudNormals draws per layer with its confidenceWindow; geometryselection.vert applies the same window, set from the active layer's range in RenderSelection/RenderSelectedGeometry (and the all-visible window for faces).
Address the review of the point-confidence filter: - a cloud without confidence (e.g. after a reconstruction workflow with constant weights releases pointWeights) resets the filter to show every point; a narrowed window used to survive, and box/lasso selection then indexed the empty weight array; - the normal lines carry the point's quantized confidence in the w of their vertices and are drawn per layer with its window, so the normals of hidden points are hidden too; - the selection highlight shader applies the same window to points, so a selected point hidden by the filter is not redrawn by the highlight. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A point selected before the confidence window was narrowed stays selected but hidden; deleting the selection now keeps it, so only what is visible is removed. Co-Authored-By: Claude Opus 5.5 <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.
Summary
A Viewer feature and four independent fixes that accumulated on this branch.
[min, max]confidence window whenever the active layer's cloud carries per-point confidence (e.g. a.dmap). The confidence is quantized to 8 bits in the alpha of the point color, now stored as RGBA bytes (16 instead of 24 bytes per point on the GPU). The window is a per-layer shader uniform, so moving a slider uploads nothing and point indices stay valid for picking and selection; box/lasso selection applies the same test.TImage::computeResizenow rounds the largest side and derives the other from the resulting uniform factor, which is the gridGetCamera()assumes;Image::ResizeUniformresizes onto that grid, and the pyramids resize to explicit sizes matching their per-axis K.ImportIgnoreMaskresizes into a local buffer, which makesImportKeepMaskredundant.SetupGizmoBufferspassed a byte count to the typedVBO::SetData(const T*, count), so the driver read 3x past the end of the index vector and crashed at startup in about 1 run in 8 under load. The vector is passed instead, and the untyped byte-sizedSetData/SetSubDataoverloads are removed so the same mistake no longer compiles.Mesh::Clean's first stage now callsRemoveLongEdgeFacesCappedwith reach 2 and cone 0.2 (was the halfmesh default reach 4, cone 0.35), so plausible graph-cut fills along wall bases, columns and stair sides survive. Reach 2 is chosen for visibly fewer holes over the most accurate arm (reach 5); the design doc and a code comment record both.--gpu-devicedefaults to-1(best GPU). It was the only app defaulting to empty (CPU), so a CUDA build refined on the CPU unless the flag was given.--gpu-device cpu(or-2) selects the CPU path.RefineMesh scene_dense.mvs -m scene_dense_mesh.plynow writesscene_dense_mesh_refined.*instead ofscene_dense_refine.*, appending the stage like the other apps; the usage docs and the pipeline scripts follow.MvsScalablePipeline.pyno longer recognizes existing*_refine.mvsoutputs as done.Validation
ImageResizeCameraTest: blob centroids vs projections 0.18-0.69 px off before, <= 0.009 px after--number-views 24):-r0F1 0.7907 -> 0.7911,-r10.7763 -> 0.7744, both within the 0.003 repeat spread--helpreports--gpu-device arg (=-1)Open items
RefineMesh --planar-vertex-ratio(CPU only) logs a CUDA "not implemented" error and the fallback message before refining on the CPU; the result is unchanged.🤖 Generated with Claude Code