Skip to content

Comments

HYDRA-2047 : Remove non-HVT code path#376

Merged
debloip-adsk merged 6 commits intodevfrom
debloip/HYDRA-2047/renderoverride-cleanup
Jan 26, 2026
Merged

HYDRA-2047 : Remove non-HVT code path#376
debloip-adsk merged 6 commits intodevfrom
debloip/HYDRA-2047/renderoverride-cleanup

Conversation

@debloip-adsk
Copy link
Collaborator

  • Remove all non-HVT code paths (which includes the entire SelectionTask, as it was only used in the non-HVT path)
  • Remove HVT flags from CMake setup
  • Remove a dead function (_NeedToRecreateTheSceneIndicesChain) and its dependent code

@debloip-adsk debloip-adsk self-assigned this Jan 21, 2026
@debloip-adsk debloip-adsk removed their assignment Jan 21, 2026
@debloip-adsk debloip-adsk self-assigned this Jan 22, 2026
@debloip-adsk debloip-adsk requested review from lanierd-adsk and ppt-adsk and removed request for ppt-adsk January 22, 2026 21:44
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.

Thanks for doing all this cleanup. Only a few question to be sure what is being removed is no longer necessary.

_CreateSceneIndicesChainAfterMergingSceneIndex(drawContext);

const Fvp::InformationInterface::RenderViewDesc hydraViewportInformation(panelNameStr, true);
manager.AddRenderViewData(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see this function call any longer. Is that on purpose ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, since _NeedToRecreateTheSceneIndicesChain always returned false we could never get into this branch path

}

auto& manager = Fvp::RenderViewDataManager::Get();
if (_NeedToRecreateTheSceneIndicesChain(currentDisplayStyle)){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this unused ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, if you look at this function's implementation, it always returned false and had a comment saying to remove it at some point

TF_DEBUG(MAYAHYDRALIB_RENDEROVERRIDE_SCENE_INDEX_CHAIN_MGMT)
.Msg("Re-using existing scene index chain to render %s\n", panelNameStr.c_str());

#ifdef MAYA_HAS_VIEW_SELECTED_OBJECT_API
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this no longer necessary ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's still there, just moved up a bit due to the removed code, see line 1081 in the updated file

@lanierd-adsk lanierd-adsk requested a review from Copilot January 23, 2026 09:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes all non-HVT (Hydra Viewport Toolbox) code paths from the codebase, streamlining the implementation to use HVT exclusively. The changes eliminate conditional compilation branches, remove unused code including the entire SelectionTask, and clean up CMake configuration.

Changes:

  • Removed VIEWPORT_TOOLBOX preprocessor conditionals and associated non-HVT code paths throughout the codebase
  • Deleted the SelectionTask class and related files that were only used in non-HVT mode
  • Removed the _NeedToRecreateTheSceneIndicesChain function and its dependent code
  • Updated CMake build configuration to remove HVT-related conditional compilation flags

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/lib/mayaUsd/render/mayaToHydra/CMakeLists.txt Unconditionally added testFramePasses.py test; removed BUILD_WITH_VIEWPORT_TOOLBOX conditional
lib/mayaHydra/mayaPlugin/renderOverride.h Removed VIEWPORT_TOOLBOX preprocessor guards from declarations
lib/mayaHydra/mayaPlugin/renderOverride.cpp Removed all non-HVT code paths and VIEWPORT_TOOLBOX conditionals; deleted _NeedToRecreateTheSceneIndicesChain function
lib/mayaHydra/mayaPlugin/plugin.cpp Removed VIEWPORT_TOOLBOX guards from command registration
lib/mayaHydra/mayaPlugin/CMakeLists.txt Made renderRegionCommand.cpp and setVisibleFramePassesCommand.cpp unconditional source files
lib/flowViewport/selection/fvpSelectionTask.h Deleted entire file (SelectionTask header)
lib/flowViewport/selection/fvpSelectionTask.cpp Deleted entire file (SelectionTask implementation)
lib/flowViewport/selection/CMakeLists.txt Removed fvpSelectionTask source and header files
lib/flowViewport/sceneIndex/fvpPassFilteringSceneIndex.h Removed VIEWPORT_TOOLBOX preprocessor guards
lib/flowViewport/sceneIndex/fvpPassFilteringSceneIndex.cpp Removed VIEWPORT_TOOLBOX preprocessor guards
lib/flowViewport/imageWriter/fvpTextureBufferWriter.cpp Removed non-HVT code path using HdEngine
lib/flowViewport/imageWriter/fvpRenderBufferWriter.cpp Removed non-HVT code path using HdxTaskController
lib/flowViewport/fvpFramePassData.h Removed VIEWPORT_TOOLBOX preprocessor guards
lib/flowViewport/fvpFramePassData.cpp Removed VIEWPORT_TOOLBOX preprocessor guards
lib/flowViewport/CMakeLists.txt Made VIEWPORTTOOLBOX libraries unconditional dependencies
lib/CMakeLists.txt Removed ENABLE_VIEWPORT_TOOLBOX conditional around HVT subdirectory
cmake/compiler_config.cmake Removed VIEWPORT_TOOLBOX preprocessor definition
CMakeLists.txt Removed ENABLE_VIEWPORT_TOOLBOX build option
Comments suppressed due to low confidence (1)

lib/mayaHydra/mayaPlugin/renderOverride.cpp:1

  • There is a duplicate #ifdef VIEWPORT_TOOLBOX guard on line 1681 within the function body. This appears to be a leftover from the refactoring. Remove the duplicate preprocessor directive.
//

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lanierd-adsk lanierd-adsk self-requested a review January 23, 2026 16:35
@debloip-adsk debloip-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jan 26, 2026
@debloip-adsk debloip-adsk merged commit 60b596d into dev Jan 26, 2026
10 checks passed
@debloip-adsk debloip-adsk deleted the debloip/HYDRA-2047/renderoverride-cleanup branch January 26, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge Development process is finished, PR is ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants