Skip to content

Enable virutal GPU for tests #42

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enable virutal GPU for tests #42

wants to merge 2 commits into from

Conversation

laforel
Copy link
Contributor

@laforel laforel commented Jul 28, 2025

Implements comprehensive virtual GPU support for running GPU-dependent tests in headless GitHub Actions Linux runners using Xvfb and Mesa software rendering.

  • Added Xvfb (X Virtual Framebuffer) setup for headless OpenGL rendering

  • Configured Mesa LLVMpipe software renderer with optimized settings

  • Set up proper environment variables (DISPLAY, LIBGL_ALWAYS_SOFTWARE, GALLIUM_DRIVER)

  • Fixed Linux RPATH configuration ($ORIGIN/ vs macOS @loader_path/)

  • Added --disable-new-dtags linker flags to force RPATH over RUNPATH

  • Ensured automatic OpenUSD library discovery without manual LD_LIBRARY_PATH

  • Created Linux-specific baseline images for all rendering tests

  • Consolidated platform-specific baseline strategy using _linux suffix

  • Added support for new tests: TestFramePasses_ClearDepthBuffer and TestFramePasses_ClearColorBuffer

  • Updated TestSearchEdges and TestSearchPoints with Linux-specific expected results

  • Fixed algorithmic differences between platforms in search functionality

  • Limited test execution to Linux runners only (unlimited resources)

  • Cleaned up redundant environment variable declarations

  • Maintained platform-agnostic CMake presets

@laforel laforel requested a review from Copilot July 28, 2025 13:55
Copilot

This comment was marked as outdated.

@laforel laforel force-pushed the laforel/gpu-tests branch 15 times, most recently from f2384dc to 4540cb9 Compare July 30, 2025 22:55
@autodesk-chorus
Copy link

Chorus detected one or more security issues with this pull request. See the Checks tab for more details.

As a reminder, please follow the secure code review process as part of the Secure Coding Non-Negotiable requirement.

@laforel laforel force-pushed the laforel/gpu-tests branch 7 times, most recently from c8dea80 to 8a11bf5 Compare July 31, 2025 11:04
Implements comprehensive virtual GPU support for running GPU-dependent tests in headless GitHub Actions Linux runners using Xvfb and Mesa software rendering.

- Added Xvfb (X Virtual Framebuffer) setup for headless OpenGL rendering
- Configured Mesa LLVMpipe software renderer with optimized settings
- Set up proper environment variables (DISPLAY, LIBGL_ALWAYS_SOFTWARE, GALLIUM_DRIVER)

- Fixed Linux RPATH configuration (`$ORIGIN/` vs macOS `@loader_path/`)
- Added `--disable-new-dtags` linker flags to force RPATH over RUNPATH
- Ensured automatic OpenUSD library discovery without manual LD_LIBRARY_PATH

- Created Linux-specific baseline images for all rendering tests
- Consolidated platform-specific baseline strategy using `_linux` suffix
- Added support for new tests: `TestFramePasses_ClearDepthBuffer` and `TestFramePasses_ClearColorBuffer`

- Updated `TestSearchEdges` and `TestSearchPoints` with Linux-specific expected results
- Fixed algorithmic differences between platforms in search functionality

- Limited test execution to Linux runners only (unlimited resources)
- Cleaned up redundant environment variable declarations
- Maintained platform-agnostic CMake presets
@laforel laforel force-pushed the laforel/gpu-tests branch from 8a11bf5 to be7ce79 Compare July 31, 2025 11:58
@laforel laforel requested a review from Copilot July 31, 2025 14:12
Copy link
Contributor

@hodoulp hodoulp left a comment

Choose a reason for hiding this comment

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

⚠️ Now I better understand what you did. That's an idea but the side-effect is that we have more baseline images for software GPU linux but we already have them i.e., hardware GPU linux. We have to stop and rethink about this challenge.

Copy link
Contributor

@Copilot 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 implements comprehensive virtual GPU support for running GPU-dependent tests in headless Linux CI environments, enabling platform-specific baseline testing with software rendering fallbacks.

  • Added Xvfb and Mesa software rendering setup for headless Linux testing
  • Fixed Linux RPATH configuration to use $ORIGIN/ instead of macOS @loader_path/
  • Created Linux-specific baseline images with _linux suffix for all rendering tests

Reviewed Changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/tests/testSearches.cpp Added software rendering detection logic for platform-specific test results
test/data/baselines/*_linux.png Added Linux-specific baseline images for rendering tests
test/RenderingFramework/TestHelpers.h Added software rendering detection function declaration
test/RenderingFramework/TestHelpers.cpp Implemented software rendering detection and Linux baseline logic
CMakeLists.txt Fixed Linux RPATH configuration with proper linker flags
.github/workflows/ci-steps.yaml Added virtual GPU setup and enabled Linux-only test execution

@@ -314,30 +316,41 @@ TEST(TestViewportToolbox, TestSearchEdges)
ASSERT_EQ(primState->pointIndices.size(), 0);
ASSERT_EQ(primState->pointColorIndices.size(), 0);

// Use helper function to detect software rendering
bool isSoftwareRendering = TestHelpers::isSoftwareRenderingEnabled();

static const std::vector<VtIntArray> results
#if defined(_WIN32) || defined(__linux__)
Copy link
Preview

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

[nitpick] The conditional compilation logic is becoming complex with nested runtime checks. Consider refactoring this into a helper function that returns the appropriate expected results based on platform and rendering mode to improve maintainability.

Copilot uses AI. Check for mistakes.

Comment on lines +112 to +116
const char* softwareRendering = getenv("LIBGL_ALWAYS_SOFTWARE");
if (softwareRendering != nullptr) {
// Validate the environment variable value for safety
std::string envValue(softwareRendering);
return (envValue == "1" || envValue == "true");
Copy link
Preview

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

Using getenv() can be unsafe in multi-threaded environments. Consider using a thread-safe alternative or ensuring this function is only called during single-threaded initialization phases.

Suggested change
const char* softwareRendering = getenv("LIBGL_ALWAYS_SOFTWARE");
if (softwareRendering != nullptr) {
// Validate the environment variable value for safety
std::string envValue(softwareRendering);
return (envValue == "1" || envValue == "true");
const std::string softwareRendering = getEnvVar("LIBGL_ALWAYS_SOFTWARE");
if (!softwareRendering.empty()) {
// Validate the environment variable value for safety
return (softwareRendering == "1" || softwareRendering == "true");

Copilot uses AI. Check for mistakes.

@@ -120,8 +139,15 @@ std::string HydraRendererContext::getFilename(
}
fullFilepath += "_ios";
#else
// macOS hardware GPU uses _osx baselines
Copy link
Preview

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

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

There is an extra trailing space after 'baselines' in the comment.

Suggested change
// macOS hardware GPU uses _osx baselines
// macOS hardware GPU uses _osx baselines

Copilot uses AI. Check for mistakes.

@hodoulp
Copy link
Contributor

hodoulp commented Jul 31, 2025

cc'ing @erikaharrison-adsk In order to use the same baseline images for Linux (software vs. hardware) & thinking that we only want to trap major errors can we investigate the idea of having a different threshold for swf Linux?
So we can use the existing Linux baseline images while waiting for the right long-term solution.

@laforel laforel force-pushed the laforel/gpu-tests branch from fb7a517 to 0714ce4 Compare July 31, 2025 18:31
@laforel laforel force-pushed the laforel/gpu-tests branch from 0714ce4 to 4fcebda Compare August 16, 2025 10:30
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