AGPHVT-39 : Add a clear depth param in FramePass and clear the aov ta… by lanierd-adsk · Pull Request #18 · Autodesk/hydra-viewport-toolbox

lanierd-adsk · 2025-06-30T09:00:38Z

…sk cache cache , tentative in adding a unit test

Copilot

Pull Request Overview

This PR adds a clear depth parameter in the FramePass workflow and integrates support to clear the AOV task cache accordingly, along with an accompanying unit test to validate the behavior. The key changes include:

Adding new parameters (backgroundDepth and clearBackgroundDepth) to FramePassParams.
Modifying FramePass and RenderBufferManager to support clearing the depth buffer.
Introducing a new unit test in testFramePasses.cpp to verify multi-viewport behavior with depth clearing.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/tests/testFramePasses.cpp	New unit test for multi-viewport clear depth handling.
source/engine/renderBufferManager.cpp	Added ResetRenderOutputClear to clear the AOV task cache.
source/engine/framePass.cpp	Updated to set clear depth values based on new FramePassParams.
include/hvt/engine/framePass.h	Introduced backgroundDepth and clearBackgroundDepth parameters.
CMakeLists.txt	Minor fix in a comment regarding vcpkg dependency options.

Comments suppressed due to low confidence (3)

test/tests/testFramePasses.cpp:28

[nitpick] Confirm that the inclusion of 'framePassUtils.h' is necessary for the test since its functions do not appear to be directly used.

#include <hvt/engine/framePassUtils.h>

include/hvt/engine/framePass.h:130

[nitpick] Consider adding a doc comment to describe the purpose and usage of backgroundDepth for clarity.

    float backgroundDepth {1.0f};

include/hvt/engine/framePass.h:132

[nitpick] Consider documenting clearBackgroundDepth to explain when and why the depth buffer should be cleared.

    bool clearBackgroundDepth { false };

Copilot · 2025-06-30T09:01:03Z

+    // Renders 10 times (i.e., arbitrary number to guarantee best result).
+    int frameCount = 10;


[nitpick] Consider replacing the magic number '10' with a named constant to improve maintainability.

Suggested change

// Renders 10 times (i.e., arbitrary number to guarantee best result).

int frameCount = 10;

// Renders a default number of times to guarantee best result.

const int DEFAULT_RENDER_FRAME_COUNT = 10;

int frameCount = DEFAULT_RENDER_FRAME_COUNT;

sebastienberube-adsk

Changes to clear the depth buffer and to reset the clear parameters make sense and fix a known issue with multiple frame passes.

We might update the RenderBufferManager API to improve and clarify how the buffers are cleared at some point in the future, but this iteration does the trick for now.

sebastienberube-adsk · 2025-07-03T01:43:13Z

    void SetViewportRenderOutput(const TfToken& name, HdRenderBuffer* aovBuffer,
        HdRenderBuffer* depthBuffer, const SdfPath& controllerId);

+    /// Resets the clear values


[Minor]
End sentence with "."

sebastienberube-adsk · 2025-07-03T01:50:00Z

+        _bufferManager->SetRenderOutputClearColor(
+            HdAovTokens->depth, VtValue(_passParams.backgroundDepth));
+    }
+


I am thinking maybe ResetRenderOutputClear() could be made public in RenderBufferManager, and be called here (ln 318) before setting the color and depth clear color.

sebastienberube-adsk · 2025-07-03T02:13:59Z

-    bool clearBackground { true };
+    float backgroundDepth {1.0f};
+    bool clearBackground{ true };
+    bool clearBackgroundDepth { false };


NOTE: At one point, we might want to rename clearBackground to clearBackgroundColor.
Not needed for this PR, but as already mentioned by @lanierd-adsk, it would be clearer and more consistent.

@sebastienberube-adsk Could you create a Jira ticket with all your comments and assign it to @sm-adsk ?

hodoulp · 2025-07-04T13:14:53Z

+            hvt::FramePassParams& params = framePass1.sceneFramePass->params();
+
+            params.renderBufferSize = GfVec2i(width, height);
+            // To display on the left part of the viewport.


@sebastienberube-adsk Visually the model display seems to be on the right side of the viewport! Did I miss something?

hodoulp · 2025-07-04T13:30:49Z

@sebastienberube-adsk When running on macOS with XCode, the new unit test generates lot of errors on the console:

[ RUN      ] TestViewportToolbox.TestFramePasses_MultiViewportsClearDepth
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for color."
[]: Error issued : /Users/hodoulp/dev/agp/hvt_publ_1/source/engine/framePassUtils.cpp - HVT_NS_v0_25_03::CreateRenderBufferProxy(136): "The frame pass is not initialized for depth."
[       OK ] TestViewportToolbox.TestFramePasses_MultiViewportsClearDepth (217 ms)

AGPHVT-39 : Add a clear depth param in FramePass and clear the aov ta…

4b44ba0

…sk cache cache , tentative in adding a unit test

lanierd-adsk requested review from Copilot and sebastienberube-adsk June 30, 2025 09:00

lanierd-adsk self-assigned this Jun 30, 2025

Copilot AI reviewed Jun 30, 2025

View reviewed changes

lanierd-adsk marked this pull request as draft June 30, 2025 09:01

erikaharrison-adsk added 2 commits July 2, 2025 18:50

Add baselines

71ed043

Comment tweak, clang-format

19af646

sebastienberube-adsk approved these changes Jul 3, 2025

View reviewed changes

sebastienberube-adsk marked this pull request as ready for review July 3, 2025 01:54

sebastienberube-adsk reviewed Jul 3, 2025

View reviewed changes

sebastienberube-adsk merged commit 8f90658 into main Jul 3, 2025
10 checks passed

sebastienberube-adsk deleted the lanierd/AGPHVT-39 branch July 3, 2025 20:18

hodoulp reviewed Jul 4, 2025

View reviewed changes

hodoulp mentioned this pull request Jul 18, 2025

OGSMOD-7744 - Fix utest wrong mgt of shared buffers #32

Merged

Navigation Menu

Uh oh!

AGPHVT-39 : Add a clear depth param in FramePass and clear the aov ta… - #18

AGPHVT-39 : Add a clear depth param in FramePass and clear the aov ta…#18
sebastienberube-adsk merged 3 commits into
mainfrom
lanierd/AGPHVT-39

lanierd-adsk commented Jun 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 30, 2025

Uh oh!

sebastienberube-adsk left a comment

Uh oh!

sebastienberube-adsk Jul 3, 2025

Uh oh!

sebastienberube-adsk Jul 3, 2025

Uh oh!

sebastienberube-adsk Jul 3, 2025

Uh oh!

hodoulp Jul 4, 2025

Uh oh!

Uh oh!

hodoulp Jul 4, 2025

Uh oh!

hodoulp commented Jul 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		// Renders 10 times (i.e., arbitrary number to guarantee best result).
		int frameCount = 10;

-    // Renders 10 times (i.e., arbitrary number to guarantee best result).
-    int frameCount = 10;
+    // Renders a default number of times to guarantee best result.
+    const int DEFAULT_RENDER_FRAME_COUNT = 10;
+    int frameCount = DEFAULT_RENDER_FRAME_COUNT;

Uh oh!

Conversation

lanierd-adsk commented Jun 30, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

sebastienberube-adsk left a comment

Choose a reason for hiding this comment

Uh oh!

sebastienberube-adsk Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

sebastienberube-adsk Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

sebastienberube-adsk Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

hodoulp Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hodoulp Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

hodoulp commented Jul 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants