-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HYDRA-703 : Fix mesh adapter transform update #20
HYDRA-703 : Fix mesh adapter transform update #20
Conversation
204d7d1
to
85b5275
Compare
auto* adapter = reinterpret_cast<MayaHydraCameraAdapter*>(clientData); | ||
adapter->MarkDirty(HdCamera::DirtyTransform); | ||
adapter->InvalidateTransform(); | ||
adapter->MarkDirty(HdCamera::DirtyTransform); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were a couple places where we were dirtying the transform before invalidating it. I've unified all instances of this sequence of operations to first invalidate the transform and then dirty it.
@@ -210,9 +210,9 @@ void MayaHydraDagAdapter::CreateCallbacks() | |||
void MayaHydraDagAdapter::MarkDirty(HdDirtyBits dirtyBits) | |||
{ | |||
if (dirtyBits != 0) { | |||
GetSceneProducer()->GetRenderIndex().GetChangeTracker().MarkRprimDirty(GetID(), dirtyBits); | |||
GetSceneProducer()->MarkRprimDirty(GetID(), dirtyBits); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is where everything originates, as it is what caused (and now fixed) the issue for MeshAdapter; however, this line being the problem only highlighted a larger problem, which is that we were still using the Hydra 1.0 way of dirtying prims in some places in our Maya-native SceneIndex (and this is why this PR may be a bit larger than expected).
@@ -26,7 +26,7 @@ | |||
|
|||
PXR_NAMESPACE_OPEN_SCOPE | |||
|
|||
using SceneIndicesVector = std::vector<HdSceneIndexBasePtr>;//Be careful, these are not not Ref counted. Elements could become dangling | |||
using SceneIndicesVector = std::vector<HdSceneIndexBaseRefPtr>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this comment and discussion : #4 (comment). Doing this now because I added a utility observer class along with the unit test for this issue, and that class needs its observed SceneIndex to remain valid for its whole lifetime.
void MayaHydraSceneIndex::MarkRprimDirty(const SdfPath& id, HdDirtyBits dirtyBits) { | ||
_MarkPrimDirty(id, dirtyBits, HdDirtyBitsTranslator::RprimDirtyBitsToLocatorSet); | ||
} | ||
|
||
void MayaHydraSceneIndex::MarkSprimDirty(const SdfPath& id, HdDirtyBits dirtyBits) | ||
{ | ||
_MarkPrimDirty(id, dirtyBits, HdDirtyBitsTranslator::SprimDirtyBitsToLocatorSet); | ||
} | ||
|
||
void MayaHydraSceneIndex::MarkBprimDirty(const SdfPath& id, HdDirtyBits dirtyBits) | ||
{ | ||
_MarkPrimDirty(id, dirtyBits, HdDirtyBitsTranslator::BprimDirtyBitsToLocatorSet); | ||
} | ||
|
||
void MayaHydraSceneIndex::MarkInstancerDirty(const SdfPath& id, HdDirtyBits dirtyBits) | ||
{ | ||
_MarkPrimDirty(id, dirtyBits, HdDirtyBitsTranslator::InstancerDirtyBitsToLocatorSet); | ||
} | ||
|
||
void MayaHydraSceneIndex::_MarkPrimDirty( | ||
const SdfPath& id, | ||
HdDirtyBits dirtyBits, | ||
DirtyBitsToLocatorsFunc dirtyBitsToLocatorsFunc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broke up the previous MarkPrimDirty method into separate prim-category-specific methods : the reason for this is because there does not seem to be a reliable way to determine which DirtyBitsToLocatorSet method to use purely based off a prim's type. While this does look more like Hydra 1.0 in terms of API design, it also matches more closely how HdChangeTracker handles these calls for its own emulated scene index. When our different adapters all move to using Hydra 2.0 APIs, we can change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
…mesh-adapter-transform-update
This PR fixes an issue with MeshAdapters where their transform was not being dirtied properly, and thus not updated correctly in the viewport. Since this issue was not MeshAdapter-specific and only highlighted a larger-scale problem, I've fixed the problem in the other places where it appeared.