HYDRA-1286 : Support displacement on GeomSubset wireframe highlights #242

debloip-adsk · 2025-01-28T19:58:35Z

No description provided.

…otingSceneIndex

debloip-adsk · 2025-01-28T20:27:00Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

@@ -268,45 +298,234 @@ class _RerootingSceneIndexPathArrayDataSource : public HdPathArrayDataSource
    HdPathArrayDataSourceHandle const _inputDataSource;
 };

+// Copied over from USD's rerootingSceneIndex.cpp
+class _RerootingSceneIndexContainerDataSource : public HdContainerDataSource


This class was previously re-implemented as RepathingContainerDataSource so that it could be externally exposed, but now we need no longer need it to be externally accessible due to the new utility method RepathInstancingDataSources, so we just go back to using the raw version copied over from USD.

I am not sure I understand why you are doing a copy of the usd class and not using it directly from usd ?

Those classes are not exposed, they are located in an anonymous namespace in a .cpp file (see USD's rerootingSceneIndex.cpp, so we can't use them directly. May be worth to make a PR to USD to expose these, I've logged HYDRA-1422 so we can discuss this.

debloip-adsk · 2025-01-28T20:27:24Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

+//We want to set the displayStyle of the selected prim to refinedWireOnSurf only if the displayStyle of the prim is refined (meaning shaded)
+HdContainerDataSourceHandle SetWireframeRepr(const HdContainerDataSourceHandle& dataSource, const GfVec4f& color)
+{
+    //Always edit its override wireframe color
+    auto edited = HdContainerDataSourceEditor(dataSource);
+    edited.Set(primvarsOverrideWireframeColorLocator,
+                        Fvp::PrimvarDataSource::New(
+                            HdRetainedTypedSampledDataSource<VtVec4fArray>::New(VtVec4fArray{color}),
+                            HdPrimvarSchemaTokens->constant,
+                            HdPrimvarSchemaTokens->color));
+
+    //Is the prim in refined displayStyle (meaning shaded) ?
+    if (HdLegacyDisplayStyleSchema styleSchema =
+            HdLegacyDisplayStyleSchema::GetFromParent(dataSource)) {
+
+        if (HdTokenArrayDataSourceHandle ds =
+                styleSchema.GetReprSelector()) {
+            VtArray<TfToken> ar = ds->GetTypedValue(0.0f);
+            TfToken refinedToken = ar[0];
+            if(HdReprTokens->refined == refinedToken){
+                //Is in refined display style, apply the wire on top of shaded reprselector
+                return HdOverlayContainerDataSource::New({ edited.Finish(), refinedWireDisplayStyleDataSource});
+            }
+        }else{
+            //No reprSelector found, assume it's in the Collection that we have set HdReprTokens->refined
+            return HdOverlayContainerDataSource::New({ edited.Finish(), refinedWireDisplayStyleDataSource});
+        }
+    }
+
+    //For the other case, we are only updating the wireframe color assuming we are already drawing lines
+    return edited.Finish();
+}
+
+Fvp::PrimSelection ConvertHydraToFvpSelection(const SdfPath& primPath, const HdSelectionSchema& selectionSchema) {
+    Fvp::PrimSelection primSelection;
+    primSelection.primPath = primPath;
+
+    auto nestedInstanceIndicesSchema = 
+#if HD_API_VERSION < 66
+    const_cast<HdSelectionSchema&>(selectionSchema).GetNestedInstanceIndices();
+#else
+    selectionSchema.GetNestedInstanceIndices();
+#endif
+    for (size_t iNestedInstanceIndices = 0; iNestedInstanceIndices < nestedInstanceIndicesSchema.GetNumElements(); iNestedInstanceIndices++) {
+        HdInstanceIndicesSchema instanceIndicesSchema = nestedInstanceIndicesSchema.GetElement(iNestedInstanceIndices);
+        auto instanceIndices = instanceIndicesSchema.GetInstanceIndices()->GetTypedValue(0);
+        primSelection.nestedInstanceIndices.push_back(
+            {
+                instanceIndicesSchema.GetInstancer()->GetTypedValue(0),
+                instanceIndicesSchema.GetPrototypeIndex()->GetTypedValue(0),
+                std::vector<int>(instanceIndices.begin(), instanceIndices.end())
+            }
+        );
+    }
+
+    return primSelection;
+}


These did not actually change, just reordered.

debloip-adsk · 2025-01-28T20:27:52Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

+    return primSelection;
+}
+
+PXR_NS::HdContainerDataSourceHandle RepathInstancingDataSources(


Replaces RepathingContainerDataSource for usage by the wireframe highlight scene indices

debloip-adsk · 2025-01-28T22:47:32Z

test/lib/mayaUsd/render/mayaToHydra/testGeomSubsetsWireframeHighlight.py

Existing tests did not change, just refactored things to allow the displacement test to use a different scene.

lanierd-adsk

Thanks for adding this. A few comments overall.

lanierd-adsk · 2025-01-29T09:16:15Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

@@ -268,45 +298,234 @@ class _RerootingSceneIndexPathArrayDataSource : public HdPathArrayDataSource
    HdPathArrayDataSourceHandle const _inputDataSource;
 };

+// Copied over from USD's rerootingSceneIndex.cpp
+class _RerootingSceneIndexContainerDataSource : public HdContainerDataSource


I am not sure I understand why you are doing a copy of the usd class and not using it directly from usd ?

lanierd-adsk · 2025-01-29T09:21:09Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

-namespace FVP_NS_DEF {
-
-TfTokenVector RepathingContainerDataSource::GetNames()
+PXR_NS::HdContainerDataSourceHandle ComputeSmoothNormals(const PXR_NS::HdContainerDataSourceHandle& meshPrimDataSource)


I think it would be good to set this in one of the utility class we have so it gets shared by all projects from maya-hydra in case we need it somewhere else. I would also rename the function to addSmoothNormals (I know it's me who named it ComputeSmoothNormals :-) )

Agree, moved it to fvpUtils and updated the name

lanierd-adsk · 2025-01-29T09:30:55Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

+    if (normalsValueDataSource) {
+        auto normals = normalsValueDataSource->GetTypedValue(0);
+        for (size_t iNormal = 0; iNormal < normals.size(); iNormal++) {
+            normals[iNormal] = GfVec3f(xformMatrix.GetInverse().GetTranspose().TransformDir(normals[iNormal]));


Could you please compute the transpose of the inverse in a materix out of the loop and applyTransformDir from it ?
I am not sure this gets optimized correctly by the different compilers we use when being inside the loop.

Good point, moved it out of the loop, thanks

lanierd-adsk · 2025-01-29T09:34:01Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

 }

-//We want to set the displayStyle of the selected prim to refinedWireOnSurf only if the displayStyle of the prim is refined (meaning shaded)
-HdContainerDataSourceHandle SetWireframeRepr(const HdContainerDataSourceHandle& dataSource, const GfVec4f& color)
+PXR_NS::HdContainerDataSourceHandle ForceDisplacement(const PXR_NS::HdContainerDataSourceHandle& meshPrimDataSource, float displacement)


I would add this to a utility class as well and call it ApplyDisplacement instead of ForceDisplacement

For both this one and ForceScale, I went to with "Force" to really emphasize that this is not something that should be naturally occurring, and is more of a workaround more than anything. This is also why I don't think we should move these over to common utilities, as we should be using this as little as possible. I've left both of these where they are for now, let me know if you have other thoughts on this

Ok, I've got your point, let's keep it like that.

lanierd-adsk · 2025-01-29T09:39:48Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

-                std::vector<int>(instanceIndices.begin(), instanceIndices.end())
-            }
-        );
+PXR_NS::HdContainerDataSourceHandle AddDependency(


It could also be part of the utility functions

Agree, moved it to fvpUtils

lanierd-adsk · 2025-01-29T09:42:59Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

@@ -756,4 +1029,81 @@ BaseWhSi::CollectInstancingPaths(const PXR_NS::SdfPath& primPath, InstancingPath
    }
 }

+VtValue 
+BaseWhSi::GetMaterialDisplacementValue(const PXR_NS::HdContainerDataSourceHandle& primDataSource) const


As well, I think it should be part of a utility function

Moved it (and GetMaterialPath) to fvpUtils

debloip-adsk · 2025-01-29T17:16:41Z

I also moved ConvertHydraToFvpSelection to fvpUtils

lanierd-adsk

Thanks for the reworking. Only 1 remaining question and we're done.

lanierd-adsk · 2025-01-30T15:20:39Z

lib/flowViewport/sceneIndex/wireframeHighlights/fvpBaseWhSi.cpp

+{
+    HdContainerDataSourceHandle editedMeshPrimDataSource = meshPrimDataSource;
+
+    VtValue displacementValue = GetMaterialDisplacementValue(geomSubsetPrimDataSource, *GetInputSceneIndex());


I recall there were 2 possible cases for the scale when the parent mesh of the geomsubset has a material and the geomsubset has a material as well :

the scale can be on the parent mesh 's material

the scale can be on the geomsubset material
Are these 2 cases handled in your code ? It seems to me, I may be wrong, that if the scale is on the parent mesh 's material, we'll miss it.
And I am not sure you can have scale on both materials, I think in this case we apply only one if I am correct ?

debloip-adsk added 16 commits January 27, 2025 13:10

HYDRA-1286 : Initial displacement (no updates)

e952874

HYDRA-1286 : Update GeomSubset highlight on displacement update

5f503d4

HYDRA-1286 : Add utility method to create data source dependencies

9c3f4e8

HYDRA-1286 : Extract method to make geomSubset highlights

7b62cc9

HYDRA-1286 : Support displacement in point instancing

5e3302d

HYDRA-1286 : Cleanup

80c825d

HYDRA-1286 : Handle scaling

bb01e29

HYDRA-1286 : Cleanup

9fe659f

HYDRA-1286 : Re-order methods

4e65cdc

HYDRA-1286 : Move TrimMeshForGeomSubset to FVP namespace

c7a5663

HYDRA-1286 : Update variable names

9be90b2

HYDRA-1286 : Update tokens

32e8a29

HYDRA-1286 : Add GeomSubset displacement test scene

d2bf391

HYDRA-1286 : Add GeomSubset displacement test

2149576

HYDRA-1286 : Go back to using only copy-pasted data sources from rero…

a9d252c

…otingSceneIndex

HYDRA-1286 : Handle conditional compilation

1045c15

debloip-adsk commented Jan 28, 2025

View reviewed changes

debloip-adsk marked this pull request as ready for review January 28, 2025 20:29

debloip-adsk self-assigned this Jan 28, 2025

HYDRA-1286 : Additional conditional compilation

8fe7bd0

debloip-adsk assigned debloip-adsk and unassigned debloip-adsk Jan 28, 2025

HYDRA-1286 : Additional conditional compilation

49b45bf

debloip-adsk assigned debloip-adsk and unassigned debloip-adsk Jan 28, 2025

debloip-adsk commented Jan 28, 2025

View reviewed changes

debloip-adsk requested a review from lanierd-adsk January 28, 2025 22:47

lanierd-adsk requested changes Jan 29, 2025

View reviewed changes

debloip-adsk added 2 commits January 29, 2025 11:28

HYDRA-1286 : Rename ComputeSmoothNormals

5478876

HYDRA-1286 : Move transpose of inverse matrix out of loop

40f7d9d

HYDRA-1286 : Extract out utility functions

aefa92a

debloip-adsk assigned debloip-adsk and unassigned debloip-adsk Jan 29, 2025

debloip-adsk requested a review from lanierd-adsk January 29, 2025 17:17

lanierd-adsk reviewed Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HYDRA-1286 : Support displacement on GeomSubset wireframe highlights #242

HYDRA-1286 : Support displacement on GeomSubset wireframe highlights #242

debloip-adsk commented Jan 28, 2025

debloip-adsk Jan 28, 2025

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025

debloip-adsk Jan 28, 2025

debloip-adsk Jan 28, 2025

debloip-adsk Jan 28, 2025

lanierd-adsk left a comment

lanierd-adsk Jan 29, 2025

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025 •

edited

Loading

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025

lanierd-adsk Jan 30, 2025

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025

lanierd-adsk Jan 29, 2025

debloip-adsk Jan 29, 2025

debloip-adsk commented Jan 29, 2025

lanierd-adsk left a comment

lanierd-adsk Jan 30, 2025

HYDRA-1286 : Support displacement on GeomSubset wireframe highlights #242

Are you sure you want to change the base?

HYDRA-1286 : Support displacement on GeomSubset wireframe highlights #242

Conversation

debloip-adsk commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanierd-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debloip-adsk Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debloip-adsk commented Jan 29, 2025

lanierd-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debloip-adsk Jan 29, 2025 •

edited

Loading