HYDRA-324: update proxy shape scene index to support interpret pick result #3028
Conversation
perrauo-adsk
commented
Apr 18, 2023
•
perrauo-adsk
commented
- Update proxy shape scene index to support interpret pick result being developed internally.
| return TfCreateRefPtr(new MayaUsdProxyShapeSceneIndex( | ||
| // Flatten transforms, visibility, purpose, model, and material | ||
| // bindings over hierarchies. | ||
| HdFlatteningSceneIndex::New( |
There was a problem hiding this comment.
I am concerned about this. The New() method is no longer a constructor of just a MayaUsdProxyShapeSceneIndex: it always has a HdFlatteningSceneIndex that comes along with it, and there is no way of doing otherwise. Separation of concerns should apply here.
| // Disable UfePath instances, passing in -1 to indicate this | ||
| MayaUsd::ufe::usdPathToUfePathSegment(path, -1) }); |
There was a problem hiding this comment.
All instances is the default, please remove the -1 argument (especially if you're not using the proper defined constant).
| MayaUsdProxyShapeBase* proxyShape) | ||
| : ParentClass(inputSceneIndex) | ||
| , _usdImagingStageSceneIndex(inputSceneIndex) | ||
| , _usdImagingStageSceneIndex(usdImagingStageSceneIndex) |
There was a problem hiding this comment.
As far as I can tell, this is passed in as a argument to support a temporary workaround, and init on first use of _usdImagingStageSceneIndex. However, why is it the responsibility of the MayaUsdProxyShapeSceneIndex to set the stage in Populate() of something that is arguably external to it? Can't we set the stage onto the _usdImagingStageSceneIndex upon its construction? A proxy shape node's stage won't change.
There was a problem hiding this comment.
No the MayaUsdProxyShapeSceneIndex also listens for changes:
void MayaUsdProxyShapeSceneIndex::ObjectsChanged(
const MayaUsdProxyStageObjectsChangedNotice& notice)
{
_usdImagingStageSceneIndex->ApplyPendingUpdates();
}
There was a problem hiding this comment.
Also please note, that changing the inner working of MayaUsdProxySHapeSceneIndex is out of scope for these changes. The intent is simply to change data source involve, and implementing the interpretDataSource. Changing the ::New is already a bit of a stretch and came as a result of needing to expose the proxyshapesceneindex as top level scene index.
| @@ -25,12 +25,19 @@ | |||
| #include <mayaUsd/nodes/proxyShapeBase.h> | |||
|
|
|||
| #include <pxr/base/tf/refPtr.h> | |||
| #include <pxr/imaging/hd/flatteningSceneIndex.h> | |||
There was a problem hiding this comment.
Could forward declare HdFlatteningSceneIndex.
| #include <pxr/imaging/hd/renderIndex.h> | ||
| #include <pxr/imaging/hd/sceneIndexPlugin.h> | ||
| #include <pxr/imaging/hd/sceneIndexPluginRegistry.h> | ||
| #include <pxr/usd/sdf/path.h> | ||
| #include <pxr/usdImaging/usdImaging/stageSceneIndex.h> | ||
|
|
||
| #if WANT_UFE_BUILD | ||
| #include <ufe/path.h> |
There was a problem hiding this comment.
Could be forward declared.
| #include <ufe/sceneItem.h> | ||
| #include <ufe/selectionNotification.h> |
…uo/HYDRA-324/revise-proxy-shape-scene-index-to-match-new-scene-index-api-2
perrauo-adsk
added
the
ready-for-merge
seando-adsk
deleted the
perrauo/HYDRA-324/revise-proxy-shape-scene-index-to-match-new-scene-index-api-2
branch
perrauo-adsk
restored the
perrauo/HYDRA-324/revise-proxy-shape-scene-index-to-match-new-scene-index-api-2
branch
seando-adsk
deleted the
perrauo/HYDRA-324/revise-proxy-shape-scene-index-to-match-new-scene-index-api-2
branch
