Skip to content
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-927 - Expose complexity settings in globalRenderSettings #106

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

vlasovi
Copy link
Collaborator

@vlasovi vlasovi commented Mar 21, 2024

Implement global refinement level for USD primitives
Automated test for the feature

@vlasovi vlasovi self-assigned this Mar 21, 2024
@vlasovi vlasovi requested a review from lanierd-adsk March 21, 2024 19:39
@vlasovi
Copy link
Collaborator Author

vlasovi commented Mar 21, 2024

lib/mayaHydra/mayaPlugin/renderOverride.cpp Show resolved Hide resolved
lib/mayaHydra/mayaPlugin/renderOverride.cpp Show resolved Hide resolved
@@ -39,6 +39,7 @@ struct MayaHydraParams
float motionSampleStart = 0;
float motionSampleEnd = 0;
bool displaySmoothMeshes = true;
int refineLevel = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the refineLevel which is adapters, MayaHydraAdapter::GetDisplayStyle().refineLevel > 0)
Is what you do replaces this ? Or is this something else ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the added one is the global refine level, while the one in DisplayStyle is per-primitive, but it's not enabled yet due to MRenderItem was already refined. HdsiLegacyDisplayStyleOverrideSceneIndex can be used to override the per-primitive one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand MayaHydraAdapter is used only for Maya native prims. The new scene index is supposed to be used only for USD prims. I will add "addExcludedSceneRoot" to it in my next commit to exclude Maya prims

@@ -945,6 +949,10 @@ void MtohRenderOverride::_CreateSceneIndicesChainAfterMergingSceneIndex()
_sceneIndexRegistry.reset(new MayaHydraSceneIndexRegistry(_renderIndexProxy));
}

// Add display style scene index
_lastFilteringSceneIndexBeforeCustomFiltering = _displayStyleSceneIndex =
HdsiLegacyDisplayStyleOverrideSceneIndex::New(_lastFilteringSceneIndexBeforeCustomFiltering);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth checking the perf impact (switch to Hydra/First Frame) after introducing this SceneIndex. We expect, by default, with refine level is 0, the overhead should be trivial

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lilike-adsk With refine level set to 0, I haven't noticed any performance impact of this change. The FPS maybe degraded with refine levels more than 0, but this is expected.

vlasovi added 2 commits March 25, 2024 10:38
# Conflicts:
#	lib/mayaHydra/mayaPlugin/renderOverride.cpp
…bal refine level

- Maya native prims are excluded from gloabl refine level
- Also fixed a bug with WireframeSelectionHighlightSceneIndex where the function addExcludedSceneRoot was called with a wrong path
auto wfSi = TfDynamic_cast<Fvp::WireframeSelectionHighlightSceneIndexRefPtr>(Fvp::WireframeSelectionHighlightSceneIndex::New(_lastFilteringSceneIndexBeforeCustomFiltering, _selection));
wfSi->SetDisplayName("Flow Viewport Wireframe Selection Highlight Scene Index");

// At time of writing, wireframe selection highlighting of Maya native data
// is done by Maya at render item creation time, so avoid double wireframe
// selection highlighting.
wfSi->addExcludedSceneRoot(_ID);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk while debugging around I found that the call that you implemented to wfSi->addExcludedSceneRoot doesn't receive the proper path. In my case the exclude path was "/MayaHydraViewportRenderer/_MayaHydra_HdStormRendererPlugin_0000023A87F10B80" while all the Maya native primitives were at the path "/MayaHydraViewportRenderer/rprims" so essentially we were drawing the selection highlight two times for those. Here I fix the bug

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vlasovi thanks for the fix! Can we better document what _ID is supposed to be used for? Also, /MayaHydraViewportRenderer is duplicated at line 566. Can we factor this out?

@vlasovi vlasovi requested a review from lanierd-adsk March 25, 2024 19:58
@vlasovi vlasovi assigned vlasovi and unassigned vlasovi Mar 25, 2024
@vlasovi vlasovi assigned vlasovi and unassigned vlasovi Mar 26, 2024
lanierd-adsk
lanierd-adsk previously approved these changes Mar 26, 2024
@vlasovi vlasovi added the ready-for-merge Development process is finished, PR is ready for merge label Mar 26, 2024
@lilike-adsk lilike-adsk merged commit 456be15 into dev Mar 26, 2024
10 checks passed
@lilike-adsk lilike-adsk deleted the vlasovi/HYDRA-927 branch March 26, 2024 18:43
@bjorn-siegert-adsk bjorn-siegert-adsk added the enhancement New feature or request label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants