Skip to content

HYDRA-1975 - fix aov sharing crash #345

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.

Sign up for GitHub

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

Merged
lilike-adsk merged 5 commits into from
Nov 26, 2025
Merged

HYDRA-1975 - fix aov sharing crash #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d20373
fix aov sharing crash
lilike-adsk Nov 17, 2025
8802517
polish a bit and update test case
lilike-adsk Nov 19, 2025
9a16062
revert the change on the command
lilike-adsk Nov 19, 2025
a4271fe
merge with latest dev
lilike-adsk Nov 21, 2025
f5bb82b
sync the crash fix from HVT
lilike-adsk Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/hydra-viewport-toolbox
Open in desktop
Submodule hydra-viewport-toolbox updated 4 files
+1 −0 include/hvt/engine/taskManager.h
+16 −1 source/engine/framePass.cpp
+6 −1 source/engine/renderBufferManager.cpp
+10 −3 source/engine/taskManager.cpp
54 changes: 42 additions & 12 deletions lib/mayaHydra/mayaPlugin/renderOverride.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,28 @@ TfTokenVector MtohRenderOverride::GetAvailableFramePassAovs(int passIndex)
for (auto* instance : _allInstances) {
if (instance->_initializationSucceeded
&& passIndex < static_cast<int>(instance->_framePassesData.size())) {
UsdImagingGLEngine engine(
Copy link
Collaborator Author

@lilike-adsk lilike-adsk Nov 17, 2025

Choose a reason for hiding this comment

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

Reuse same hgi/hgiDriver in a new engine may interfere with the existing engine

instance->_hgiDriver, instance->_framePassesData[passIndex]->_rendererName);
auto currAovs = engine.GetRendererAovs();
// Can't rely on UsdImagingGLEngine::GetRenderAovs() as creating a temp UsdImagingGLEngine with same hgi
// may interfere with the current renderer, just copy the same implementation here
TfTokenVector currAovs;
const auto renderIndex = instance->renderIndex(passIndex);
if (renderIndex && renderIndex->IsBprimTypeSupported(HdPrimTypeTokens->renderBuffer)) {

static const TfToken candidates[] = { HdAovTokens->primId,
HdAovTokens->depth,
HdAovTokens->normal,
#if PXR_VERSION > 2411
HdAovTokens->Neye,
#endif
HdAovTokensMakePrimvar(TfToken("st")) };

currAovs = { HdAovTokens->color };
for (auto const& aov : candidates) {
if (renderIndex->GetRenderDelegate()->GetDefaultAovDescriptor(aov).format
!= HdFormatInvalid) {
currAovs.push_back(aov);
}
}
}
aovs.insert(aovs.end(), currAovs.begin(), currAovs.end());
}
}
Expand Down Expand Up @@ -970,7 +989,7 @@ MStatus MtohRenderOverride::Render(
const MIntArray& framePassesVisible =
MayaHydraSetVisibleFramePasses::getVisibleFramePasses();
int numVisibleFramePasses = framePassesVisible.length();
const MStringArray& visibleAOVNames = MayaHydraSetVisibleFramePasses::getAovNames();
const MStringArray& visibleAOVNames = MayaHydraSetVisibleFramePasses::getAovNames();
const int numFramePasses = _GetNumFramePasses();
if (numVisibleFramePasses > numFramePasses) {
numVisibleFramePasses
Expand Down Expand Up @@ -998,8 +1017,13 @@ MStatus MtohRenderOverride::Render(
currentPass->params().enablePresentation = isLastVisiblePass;

// Set the AOV to visualize for the current pass if it exists
const TfToken aovName = TfToken(visibleAOVNames[visibleIdx].asChar());
const bool aovNameExists = currentPass->GetRenderBuffer(aovName) != nullptr;
const TfToken aovName = TfToken(visibleAOVNames[visibleIdx].asChar());
Copy link
Collaborator

@lanierd-adsk lanierd-adsk Nov 25, 2025

Choose a reason for hiding this comment

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

This code is wrong in the sense that you only have hardcoded AOVs :
HdAovTokens->primId,
HdAovTokens->depth,
HdAovTokens->normal,
HdAovTokens->Neye,
HdAovTokensMakePrimvar(TfToken("st"))

But the render delegate , like Arnold, has some other AOVs available that the user may want to use for display. I don't think we should check in the visibleAOVs name if it exists sibnce this list is hardcoded.

Copy link
Collaborator Author

@lilike-adsk lilike-adsk Nov 25, 2025

Choose a reason for hiding this comment

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

This is the same code used in UsdImagingGLEngine::GetRenderAovs() , yes, a better way is required in the future, before that, we have to copy that piece of code here as using UsdImagingGLEngine::GetRenderAovs() will affect the current rendering.

// Can't rely on GetRenderBuffer(aovName) here as the AOV may not have been created yet
const auto renderDelegate = _GetRenderDelegate(actualPassIndex);
const bool aovNameExists
Copy link
Collaborator

@lanierd-adsk lanierd-adsk Nov 25, 2025

Choose a reason for hiding this comment

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

Here we are doing the same test as in visibleAOVNames, if it is in AOVNames then this will always be true since we tested for the same thing in MtohRenderOverride::GetAvailableFramePassAovs. But I don't think we should use this hardcoded list.

Copy link
Collaborator Author

@lilike-adsk lilike-adsk Nov 25, 2025

Choose a reason for hiding this comment

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

We still need to validate if the AOV is supported by a renderer here, as user may specify a non-supported AOV by the commad and we didn't validate the user input.

Copy link
Collaborator

@lanierd-adsk lanierd-adsk Nov 25, 2025

Choose a reason for hiding this comment

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

ok thanks. I was confused and thought we needed to have the diaplsy aov be part of the hardcoded list which is not the case.

= renderDelegate ?
renderDelegate->GetDefaultAovDescriptor(aovName).format != HdFormatInvalid
: false;
currentPass->params().visualizeAOV
= (aovNameExists)
? aovName
Expand Down Expand Up @@ -1031,12 +1055,18 @@ MStatus MtohRenderOverride::Render(
const int previousPassIndex = (visibleIdx > 0) ?framePassesVisible[visibleIdx - 1] : 0;
hvt::FramePassPtr& previousPass = _GetFramePass(previousPassIndex);
if (previousPass) {
hvt::RenderBufferBindings inputAOVs = previousPass->GetRenderBufferBindingsForNextPass(
{ PXR_NS::HdAovTokens->color, PXR_NS::HdAovTokens->depth }
);

HdTaskSharedPtrVector passTasks = currentPass->GetRenderTasks(inputAOVs);

std::vector<TfToken> inputAOVs;
const auto& preVisualizedAOV = previousPass->params().visualizeAOV;
// If a non-color AOV was visualized previously, HVT expects to share only that one
if (preVisualizedAOV == HdAovTokens->color) {
inputAOVs = { HdAovTokens->color, HdAovTokens->depth };
}
else {
inputAOVs = { preVisualizedAOV };
}
const hvt::RenderBufferBindings aovBindings = previousPass->GetRenderBufferBindingsForNextPass(inputAOVs);
HdTaskSharedPtrVector passTasks = currentPass->GetRenderTasks(aovBindings);

/*Debug code left here if needed later
hvt::FramePass& framePassToDebug = *currentPass;
std::ostringstream content;
Expand Down