Skip to content

Comments

HYDRA-1975 - fix aov sharing crash#345

Merged
lilike-adsk merged 5 commits intodevfrom
lilike/HYDRA-1975/fix-aov-sharing-crash
Nov 26, 2025
Merged

HYDRA-1975 - fix aov sharing crash#345
lilike-adsk merged 5 commits intodevfrom
lilike/HYDRA-1975/fix-aov-sharing-crash

Conversation

@lilike-adsk
Copy link
Collaborator

Fix the AOV buffer sharing issues:

  1. Consume latest HVT which includes updated render buffer management changes
  2. Remove the call to UsdImagingGLEngine::GetRenderAovs() as it may interfere with the current renderer
  3. Remove the call to GetRenderBuffer(aovName) as the AOV may not have been created yet for first frame
  4. To match what HVT expects, if a non-color AOV was visualized previously, we only share that one

@lilike-adsk lilike-adsk self-assigned this Nov 17, 2025
}

// Can't rely on UsdImagingGLEngine::GetRenderAovs() as it may interfere with the current renderer
TfTokenVector _GetRenderAovs(HdRenderIndex* renderIndex)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy the implementation of UsdImagingGLEngine::GetRenderAovs() here, will ask HVT to see if it can provide one utility to do this query.

for (auto* instance : _allInstances) {
if (instance->_initializationSucceeded
&& passIndex < static_cast<int>(instance->_framePassesData.size())) {
UsdImagingGLEngine engine(
Copy link
Collaborator Author

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

@lilike-adsk lilike-adsk removed the request for review from lanierd-adsk November 17, 2025 20:21
Copy link
Collaborator

@lanierd-adsk lanierd-adsk left a comment

Choose a reason for hiding this comment

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

I don't think using a hardcoded list is correct, the render delegate are proposing custom AOVs that are not part of the hardcoded list we use.

// 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

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

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.

const TfToken aovName = TfToken(visibleAOVNames[visibleIdx].asChar());
// 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

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

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

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.

@lilike-adsk lilike-adsk merged commit 1ba2e3d into dev Nov 26, 2025
10 checks passed
@lilike-adsk lilike-adsk deleted the lilike/HYDRA-1975/fix-aov-sharing-crash branch November 26, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants