-
Notifications
You must be signed in to change notification settings - Fork 0
AGPHVT-82 : Improve buffer transitions and content copying between frame passes. #85
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
Changes from all commits
134d34a
5b52ab7
6a1610c
9b1c94c
207dccf
c5e494d
3b4769b
114e579
a07bd8c
6a5fd3f
dd96c7c
54584c3
ffc155e
1a8dfeb
cc61ac8
0268550
b2c2f74
f58d4b0
049f4a8
44974b5
ecd489b
3c81144
4d3a12a
374ae01
0288104
e991466
20363c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -246,20 +246,32 @@ std::tuple<SdfPathVector, SdfPathVector> FramePass::CreatePresetTasks(PresetTask | |||||
|
|
||||||
| if (!IsStormRenderDelegate(GetRenderIndex()) && _bufferManager->IsAovSupported()) | ||||||
| { | ||||||
| // Initialize the AOV system to render color, depth and ID buffers. | ||||||
| _bufferManager->SetRenderOutputs( | ||||||
| { HdAovTokens->color, HdAovTokens->depth, HdAovTokens->primId, HdAovTokens->elementId, | ||||||
| HdAovTokens->instanceId }, | ||||||
| {}, | ||||||
| _passParams.renderParams.viewport); // NOTE: this is the non-adjusted viewport. | ||||||
|
|
||||||
|
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. I never understood the exact reason why initializing AOV buffers was necessary when using a renderer other than Storm. I can see similar code being executed with the TaskController, from the constructor: Since I do not understand why this line had been added to the TaskController, I am slightly worried removing it here might break something. However, on the other hand, I can see that In conclusion, removing |
||||||
| // Set the buffer paths for use with the selection and picking tasks. | ||||||
| _selectionHelper->SetVisualizeAOV(_bufferManager->GetViewportAov()); | ||||||
| } | ||||||
|
|
||||||
| return { taskIds, renderTaskIds }; | ||||||
| } | ||||||
|
|
||||||
| hvt::RenderBufferBindings FramePass::GetRenderBufferBindingsForNextPass( | ||||||
| std::vector<pxr::TfToken> const& aovs, bool copyContents) | ||||||
| { | ||||||
| std::string renderName = GetRenderIndex()->GetRenderDelegate()->GetRendererDisplayName(); | ||||||
|
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.
Suggested change
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. Actually, a TfToken would be preferable here I think. The TfToken for the rendererName could be stored in the FramePass of the RenderBufferManager in FramePass::Initialize. |
||||||
|
|
||||||
| hvt::RenderBufferBindings inputAOVs; | ||||||
| for (auto& aov : aovs) | ||||||
| { | ||||||
| pxr::HgiTextureHandle aovTexture; | ||||||
| if (copyContents) | ||||||
| aovTexture = GetRenderTexture(aov); | ||||||
|
|
||||||
| pxr::HdRenderBuffer* aovBuffer = GetRenderBuffer(aov); | ||||||
| inputAOVs.push_back({ aov, aovTexture, aovBuffer, renderName }); | ||||||
| } | ||||||
|
|
||||||
| return inputAOVs; | ||||||
| } | ||||||
|
|
||||||
| void FramePass::UpdateScene(UsdTimeCode /*frame*/) {} | ||||||
|
|
||||||
| HdTaskSharedPtrVector FramePass::GetRenderTasks(RenderBufferBindings const& inputAOVs) | ||||||
|
|
||||||
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.
Suggestion: use a TfToken for the renderName instead of a string.
Since TfToken creation is somewhat expensive, it could be created once in FramePass::Initialize(), then stored as a FramePass or RenderBufferManager member variable and assigned in the RenderBufferBinding without having to deal with the string representation after FramePass::Initialize().
Comparisons and assignments of TfTokens are cheap, once converted from a string.
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.
Yeah, could be. This is not executed in a deep or large loop so there will be no real performance improvement. I wish the name from Hydra was a token to begin with, or there was a way to get the more accurate name "HdStormRenderDelegate". We could store it in the frame pass, but I would like to make this change in a second PR not this one.