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

Improve and generalize Hydra dependency graph #1158 #1159

Merged
merged 4 commits into from
May 16, 2022

Conversation

sebastienblor
Copy link
Collaborator

@sebastienblor sebastienblor commented May 13, 2022

Changes proposed in this pull request
This PR does quite a few changes on the previous "tracking" of node graphs. The previous system was a dependency graph, but hardcoded for specific use cases, and not fully reliable. It's important the the list of references & back-references is stored in a centralized place, so that every time something changes, all the lists are properly updated. In this case, the "node graph tracker" which was part of the meshes, was not updated when the render delegate changed its dependencies, and we could end up with crashes or refresh issues when shading nodes were deleted / reconnected / etc...

Here's a list of what this PR changes :

  • "node graph" is now generalized to "dependencies", as we just really want to track the dependencies between some prims and others, for proper refresh. Both use case for meshes and lights are now unified
  • node_graph_tracker is removed. Everything is now centralized in the render delegate
  • did some cleanup on the functions to track new dependencies, remove existing ones, etc...
  • ensure the light shaders are only updated when the light is "dirty"
  • removed the skydome attribute "shader" that we don't need to support for now
  • whe na Sprim is created, we need to check if there's already a link to its path (can happen when we delete or disconnect / reconnect)
  • when a node graph is deleted, we ensure we mark all its source prims as dirty, so that they re-evaluate and set their connections properly in arnold (we used to have dangling links to nodes that were destroyed)

Issues fixed in this pull request
Fixes #1158

@sebastienblor sebastienblor self-assigned this May 13, 2022
@sebastienblor sebastienblor marked this pull request as draft May 13, 2022 15:58
Copy link
Contributor

@jhodgson jhodgson left a comment

Choose a reason for hiding this comment

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

    HDARNOLD_API
    AtNode* HdArnoldRenderDelegate::GetSubdivDicingCamera(HdRenderIndex* renderIndex);

We need to remove HdArnoldRenderDelegate:: in render_delegate.h line 453.

The PR is working well and is fixing crashed deleting light filters.

[&](const SdfPath& p) { lightShaderPath = p; });

AtNode *color = nullptr;
AtNode *shader = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

AtNode *shader = nullptr; is unused

for (const auto& source : it->second) {
// for each source referencing the current target
// we need to remove the target from its list
auto &sourceIt = _sourceToTargetsMap.find(source);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a const reference const auto &sourceIt = _sourceToTargetsMap.find(source);

@sebastienblor sebastienblor marked this pull request as ready for review May 16, 2022 07:42
@sebastienblor
Copy link
Collaborator Author

Fixed the build issues in the last commit

@sebastienblor sebastienblor merged commit 0b0d118 into Autodesk:master May 16, 2022
@sebastienblor sebastienblor deleted the pr/1158 branch May 16, 2022 10:54
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.

Improve Hydra dependency graph
2 participants