Repository navigation
Feat: HdV2 render Gaussian splats as points - #4786
adsk-gagnonf wants to merge 8 commits into
Conversation
Add HdVP2GaussianSplat to handle the particleField rprim (USD 26.05+). Splats are drawn as small 5x5 quads for now. Most of the gaussianSplat.cpp logic is copied from or based on points.cpp.
| PXR_NAMESPACE_OPEN_SCOPE | ||
|
|
||
| namespace { |
There was a problem hiding this comment.
Is there a reason for placing this code in the pixar namespace instead of using just:
namespace MAYAUSD_NS_DEF
There was a problem hiding this comment.
I followed the principles used in points.cpp as a source of inspiration. for vt* types should I do "using PXR_NS" while inside MAYAUSD_NS_DEF or place PXR_NS:: in front of every types
There was a problem hiding this comment.
The problem is we have many old files in the repo which were inherited from Pixar (thus their namespace). New files should use the MayaUsd namespace and use PXR_NS:: in front of Usd types.
There was a problem hiding this comment.
I used PXR_NS:: in the header and using namespace PXR_NS; in the .cpp, is that OK?
seando-adsk
left a comment
seando-adsk
left a comment
There was a problem hiding this comment.
Please try to remove the pixar namespace and put this stuff into MayaUsd namespace. This will require using PXR_NS:: on all the USD classes/functions.
| result = value.UncheckedGet<VtFloatArray>(); | ||
| return true; | ||
| } | ||
| if (value.IsHolding<VtHalfArray>()) { |
There was a problem hiding this comment.
replaced implementation with the Vt...Cast approach
|
|
||
| /*! \brief Fill the color buffer from the first spherical harmonics coefficient of each splat. | ||
|
|
||
| Splats are drawn opaque, so alpha is always 1. |
There was a problem hiding this comment.
Why did we deal with opacities above if splats are draw opaque always?
There was a problem hiding this comment.
The idea is that this implementation will be built upon aiming towards full salts display, implementation of alpha blended points is planned, we want to have most of the splat logic exist in the class and ready to be expended when depth sorting is figured out
There was a problem hiding this comment.
OK, but from other code i saw its actually used yeah? to filter out indices?
Otherwise, if it is not currently used, i would remove from the PR. maybe with a comment explaining high level how to load opacities. At Autodesk nothing is more permanent than something temporary...
There was a problem hiding this comment.
Yes, we do filter out small alpha points, but in practice it's mostly redondant as low opacity splats are already mostly culled in the offline training phase.
I'll remove it for now. I am convinced it will be added back very soon, but it is low impact so It's fine
| "color.", | ||
| id.GetText(), | ||
| shCoeffs.size(), | ||
| numSplats); |
There was a problem hiding this comment.
"using default color", formatter split on two lines, default color is at line 203: const GfVec3f defaultColor = param->GetDrawScene().GetDefaultColor(HdPrimTypeTokens->points);
There was a problem hiding this comment.
maybe bring it closer then, there are early returns between here and usage a defualt color, which would render the message misleading
| unsigned int offset = 0; | ||
| for (size_t i = 0; i < numSplats; ++i) { | ||
| const GfVec3f color | ||
| = hasSH ? GfVec3f(0.5f) + kSH_C0 * shCoeffs[i * shStride] : defaultColor; |
There was a problem hiding this comment.
Can you comment on the 0.5 here / math
There was a problem hiding this comment.
it's the 3dgs spherical harmonics sRGB mapping to RGB for band 0 (constant color) of the SH . I'll had a comment in the code
| const GfVec3f color | ||
| = hasSH ? GfVec3f(0.5f) + kSH_C0 * shCoeffs[i * shStride] : defaultColor; | ||
| bufferData[offset++] = GfClamp(color[0], 0.0f, 1.0f); | ||
| bufferData[offset++] = GfClamp(color[1], 0.0f, 1.0f); |
There was a problem hiding this comment.
What does it mean for colors to be out of range here and clamped?
There was a problem hiding this comment.
the SHs can learn values that causes the resulting color to be negative we clamp to 0 in that case, it's a regular operation in 3dgs spherical harmonics handling, I'll add a comment in the code
| | HdChangeTracker::DirtyDisplayStyle | HdChangeTracker::DirtyRepr | ||
| | HdChangeTracker::DirtyMaterialId | HdChangeTracker::DirtyTransform | ||
| | HdChangeTracker::DirtyVisibility | HdChangeTracker::DirtyRenderTag | ||
| | DirtySelectionHighlight; |
There was a problem hiding this comment.
Instancing of splat works? And what happens with display styles and selection highlight?
There was a problem hiding this comment.
Instancing of splats is not handled yet. Only the prototype is drawn, without the per-instance transforms, so I removed the instancer dirty bit for now. Supporting it can be a separate story, after the full splat rendering.
Display styles and selection highlight behave like MayaUSD points. Splats are drawn in the shaded modes only, they are not displayed in wireframe or bounding box mode, and a selected splat is drawn in the highlight color. A bounding box display style could be added in a future story.
| TRACE_FUNCTION(); | ||
|
|
||
| const unsigned int numVisibleSplats = _splatSharedData._numVisibleSplats; | ||
| if (numVisibleSplats == 0 || !drawItemData._indexBuffer) { |
There was a problem hiding this comment.
What is the !indexBuffer case?
There was a problem hiding this comment.
Over-cautious, since the index buffer is always created with the render item. I removed the check.
| unsigned int numIndices = 0; | ||
| for (size_t i = 0; i < opacities.size(); ++i) { | ||
| if (opacities[i] >= kMinVisibleOpacity) { | ||
| indexData[numIndices++] = static_cast<int>(i); |
There was a problem hiding this comment.
I see so inivisible splats are just removed completely, i think its worth commenting on this around where i asked above
There was a problem hiding this comment.
added comment
| if (_selectionStatus != kUnselected || inTemplateMode) { | ||
| const MColor color = _GetHighlightColor(HdPrimTypeTokens->points); | ||
| shader = _delegate->Get3dFatPointShader(color); | ||
| isTransparent = (color.a < 0.999f); |
There was a problem hiding this comment.
(minor) maybe define an epsilon or use one that exist already
There was a problem hiding this comment.
fixed by defining a constant
| // Determine if the render item should be enabled or not. | ||
| if (itemDirtyBits | ||
| & (HdChangeTracker::DirtyVisibility | HdChangeTracker::DirtyRenderTag | ||
| | HdChangeTracker::DirtyPoints | HdChangeTracker::DirtyPrimvar |
There was a problem hiding this comment.
Why are dirty points/selection/extent relevent?
Oh i guess you dont know if point counts changed with just that bit...
There was a problem hiding this comment.
But yeah there's also DirtyTopology
There was a problem hiding this comment.
The splat count comes from the positions primvar, so DirtyPrimvar covers it. A particle field has no topology, so I removed DirtyTopology.
3 participants
Add HdVP2GaussianSplat to handle the particleField rprim (USD 26.05+). Splats are drawn as small 5x5 quads for now. Most of the gaussianSplat.cpp logic is copied from or based on points.cpp.