Seg Mesh Extracting and Seg Extracting from GarfVDB#61
Conversation
Signed-off-by: Zoe <zlalena@nvidia.com>
| from scipy.spatial import cKDTree | ||
|
|
||
| # Import directly to avoid fvdb_reality_capture.tools.__init__ pulling optional deps (e.g. DLNR). | ||
| from fvdb_reality_capture.tools._filter_splats import ( |
There was a problem hiding this comment.
🚩 issue: We shouldn't import through the internal _ modules. These examples are meant to demonstrate best practices to users and if DLNR is causing an issue in fvdb_reality_capture, we should fix that and not try to work around it in the examples.
| help="Clustering scale (absolute or fraction of max; see --scale-is-fraction-of-max)", | ||
| ) | ||
| parser.add_argument( | ||
| "--scale-is-fraction-of-max", |
There was a problem hiding this comment.
🎵 note (non-blocking): This option is a bit confusing, I think this should just be expressed as an option that turns off this behaviour. My understanding is that by default, the value of this option is True and the scale value is a fraction of the max_grouping_scale (in 'world' units), but this option when set to false (--scale-is-fraction-of-max=False?), turns the interpretation of scale into a world-unit measurement.
I think it makes more sense to just have this option be something like --scale-is-world-unit (by default it would be off) to turn on interpreting the scale value as the world unit and not a fraction.
swahtz
left a comment
swahtz
left a comment
There was a problem hiding this comment.
Overall looks pretty good, just a few organizational changes that are important and documentation/user experience changes to address.
| "--min-splat-scale", | ||
| type=float, | ||
| default=0.1, | ||
| help="Drop Gaussians with scale below this value (0 disables)", |
There was a problem hiding this comment.
💡 suggestion: I think it's worth mentioning whether the minimum scale here is in world units or a similar fraction of the max scale as above.
| "--min-cluster-gaussians", | ||
| type=int, | ||
| default=200, | ||
| help="Drop clusters smaller than this (0 disables)", |
There was a problem hiding this comment.
💡 suggestion: 'smaller' is a bit confusing because it can be read as a 3D size/scale. I think it probably makes more sense to say "Drop clusters with a number of member Gaussians less than this value"
| clustering_gs_model = gs_model | ||
| subsample_mask: torch.Tensor | None = None | ||
| if max_gaussians_for_clustering > 0 and gs_model.num_gaussians > max_gaussians_for_clustering: | ||
| logger.warning( |
There was a problem hiding this comment.
🤓 nitpick (non-blocking): I think this should just be an 'info' message like the one below, I don't believe this is warning about anything
|
|
||
| Loads a Gaussian splat reconstruction and segmentation checkpoint, clusters | ||
| per-Gaussian affinity features at a chosen scale, filters clusters, then writes | ||
| ``n`` segment ``.ply`` files (one per selected cluster). |
There was a problem hiding this comment.
🤓 nitpick (non-blocking): I think it's worth mentioning these ply files are Gaussian splat scenes as well, so they're not going to be confused with ply meshes.
| ``n`` segment ``.ply`` files (one per selected cluster). | |
| ``n`` segment ``.ply`` Gaussian splat scenes (one per selected cluster). |
| logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") | ||
|
|
||
| parser = argparse.ArgumentParser( | ||
| description="Segment full scene mesh based on a PLY Gaussian Splat segment", |
There was a problem hiding this comment.
🚩 issue: I'm confused about what this script does based on this description and the documentation at the top of the file. The description should be improved so users understand what this script will do.
There was a problem hiding this comment.
💡 suggestion: If this mesh extraction is going to be directly related to the segment extraction in GARfVDB, then I think we should just move this file to the instance_segementation/garfvdb directory. Otherwise, we should include a README, environment files, pyproject.toml, etc. in this directory for a user to be able to run this project.
| import logging | ||
| from pathlib import Path | ||
|
|
||
| import igl |
There was a problem hiding this comment.
☑️ todo: We should include igl and point_cloud_utils in GARfVDB's environment file so these modules are available in the example.
| return vertex_colors is not None and vertex_colors.size > 0 and vertex_colors.shape[0] > 0 | ||
|
|
||
|
|
||
| def _normalize_vertex_colors(colors: np.ndarray) -> np.ndarray: |
There was a problem hiding this comment.
| def _normalize_vertex_colors(colors: np.ndarray) -> np.ndarray: | |
| def _as_float_vertex_colors(colors: np.ndarray) -> np.ndarray: |
This isn't really normalization in the statistical/vector sense, I'd suggest we change the name to something like _as_float_vertex_colors or _as_unit_float_vertex_colors so it's clear we're not changing the meaning in any way, we're just scaling to change formats.
| str(output_path), | ||
| extracted_vertices, | ||
| extracted_faces_reindexed, | ||
| _normalize_vertex_colors(extracted_colors), |
There was a problem hiding this comment.
| _normalize_vertex_colors(extracted_colors), | |
| extracted_colors, |
☑️ todo: We already called this function on the colors at line 154, I think we can just avoid calling this again.
2 participants
Cleaned up versions of the code used for the GTC SJ lab