-
Notifications
You must be signed in to change notification settings - Fork 5
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,17 +82,17 @@ void _TransformNodeDirty(MObject& node, MPlug& plug, void* clientData) | |
// that dirty as well... | ||
if (adapter->IsVisible(false)) { | ||
// Transform can change while dag path is hidden. | ||
adapter->MarkDirty(HdChangeTracker::DirtyVisibility | HdChangeTracker::DirtyTransform); | ||
adapter->InvalidateTransform(); | ||
adapter->MarkDirty(HdChangeTracker::DirtyVisibility | HdChangeTracker::DirtyTransform); | ||
} else { | ||
adapter->MarkDirty(HdChangeTracker::DirtyVisibility); | ||
} | ||
// We use IsVisible(checkDirty=false) because we need to make sure we | ||
// DON'T update visibility from within this callback, since the change | ||
// has't propagated yet | ||
} else if (adapter->IsVisible(false)) { | ||
adapter->MarkDirty(HdChangeTracker::DirtyTransform); | ||
adapter->InvalidateTransform(); | ||
adapter->MarkDirty(HdChangeTracker::DirtyTransform); | ||
} | ||
} | ||
|
||
|
@@ -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 commentThe 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). |
||
if (IsInstanced()) { | ||
GetSceneProducer()->GetRenderIndex().GetChangeTracker().MarkInstancerDirty(GetInstancerID(), dirtyBits); | ||
GetSceneProducer()->MarkInstancerDirty(GetInstancerID(), dirtyBits); | ||
} | ||
if (dirtyBits & HdChangeTracker::DirtyVisibility) { | ||
_visibilityDirty = true; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe 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. |
||
|
||
/// In order to access this interface, call the function GetMayaHydraLibInterface() | ||
class MayaHydraLibInterface | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -580,7 +580,7 @@ void MayaHydraSceneIndex::SetDefaultLight(const GlfSimpleLight& light) | |
_mayaDefaultLight.SetDiffuse(light.GetDiffuse()); | ||
_mayaDefaultLight.SetSpecular(light.GetSpecular()); | ||
_mayaDefaultLight.SetPosition(light.GetPosition()); | ||
MarkPrimDirty(_mayaDefaultLightPath, HdLight::DirtyParams); | ||
MarkSprimDirty(_mayaDefaultLightPath, HdLight::DirtyParams); | ||
} | ||
} | ||
|
||
|
@@ -943,18 +943,33 @@ void MayaHydraSceneIndex::_AddPrimAncestors(const SdfPath& path) | |
|
||
} | ||
|
||
void MayaHydraSceneIndex::MarkPrimDirty(const SdfPath& id, HdDirtyBits dirtyBits) | ||
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) | ||
Comment on lines
+946
to
+968
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
{ | ||
// Dispatch based on prim type. | ||
HdSceneIndexPrim prim = GetPrim(id); | ||
HdDataSourceLocatorSet locators; | ||
if (HdPrimTypeIsGprim(prim.primType)) { | ||
HdDirtyBitsTranslator::RprimDirtyBitsToLocatorSet(prim.primType, dirtyBits, &locators); | ||
} | ||
else { | ||
HdDirtyBitsTranslator::SprimDirtyBitsToLocatorSet(prim.primType, dirtyBits, &locators); | ||
} | ||
|
||
dirtyBitsToLocatorsFunc(prim.primType, dirtyBits, &locators); | ||
if (!locators.IsEmpty()) { | ||
DirtyPrims({ {id, locators} }); | ||
} | ||
|
@@ -1254,11 +1269,10 @@ void MayaHydraSceneIndex::RecreateAdapter(const SdfPath& id, const MObject& obj) | |
}, | ||
_materialAdapters)) { | ||
auto& renderIndex = GetRenderIndex(); | ||
auto& changeTracker = renderIndex.GetChangeTracker(); | ||
for (const auto& rprimId : renderIndex.GetRprimIds()) { | ||
const auto* rprim = renderIndex.GetRprim(rprimId); | ||
if (rprim != nullptr && rprim->GetMaterialId() == id) { | ||
changeTracker.MarkRprimDirty(rprimId, HdChangeTracker::DirtyMaterialId); | ||
MarkRprimDirty(rprimId, HdChangeTracker::DirtyMaterialId); | ||
} | ||
} | ||
if (MObjectHandle(obj).isValid()) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
// Copyright 2023 Autodesk | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
// | ||
|
||
#include "testUtils.h" | ||
|
||
#include <pxr/imaging/hd/tokens.h> | ||
#include <pxr/imaging/hd/xformSchema.h> | ||
|
||
#include <maya/MGlobal.h> | ||
|
||
#include <gtest/gtest.h> | ||
|
||
PXR_NAMESPACE_USING_DIRECTIVE | ||
|
||
namespace { | ||
const std::string kCubeName = "testCube"; | ||
} // namespace | ||
|
||
TEST(MeshAdapterTransform, testDirtying) | ||
{ | ||
// Setup notifications accumulator for the first terminal scene index | ||
const SceneIndicesVector& sceneIndices = GetTerminalSceneIndices(); | ||
ASSERT_GT(sceneIndices.size(), static_cast<size_t>(0)); | ||
SceneIndexNotificationsAccumulator notifsAccumulator(sceneIndices.front()); | ||
|
||
// The test cube should still be selected from the Python driver | ||
MGlobal::executeCommand("move 3 5 8"); | ||
|
||
// Check if the cube mesh prim had its xform dirtied | ||
bool cubeXformWasDirtied = false; | ||
for (const auto& dirtiedPrimEntry : notifsAccumulator.GetDirtiedPrimEntries()) { | ||
HdSceneIndexPrim prim | ||
= notifsAccumulator.GetObservedSceneIndex()->GetPrim(dirtiedPrimEntry.primPath); | ||
|
||
cubeXformWasDirtied = dirtiedPrimEntry.primPath.GetName() == kCubeName + "Shape" | ||
&& prim.primType == HdPrimTypeTokens->mesh | ||
&& dirtiedPrimEntry.dirtyLocators.Contains(HdXformSchema::GetDefaultLocator()); | ||
|
||
if (cubeXformWasDirtied) { | ||
break; | ||
} | ||
} | ||
EXPECT_TRUE(cubeXformWasDirtied); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# Copyright 2023 Autodesk | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
import maya.cmds as cmds | ||
import fixturesUtils | ||
import mtohUtils | ||
from testUtils import PluginLoaded | ||
|
||
class TestMeshAdapterTransform(mtohUtils.MayaHydraBaseTestCase): | ||
# MayaHydraBaseTestCase.setUpClass requirement. | ||
_file = __file__ | ||
|
||
def setupScene(self): | ||
self.setHdStormRenderer() | ||
cmds.polyCube(name="testCube") | ||
cmds.refresh() # Refresh to create the MeshAdapter and its mesh prim at the origin | ||
|
||
def test_Dirtying(self): | ||
self.setupScene() | ||
with PluginLoaded('mayaHydraCppTests'): | ||
cmds.mayaHydraCppTest(f="MeshAdapterTransform.testDirtying") | ||
|
||
if __name__ == '__main__': | ||
fixturesUtils.runTests(globals()) |
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.