Autodesk: Fix HdSt tests failing under HgiVulkan with Lavapipe #3170

erikaharrison-adsk · 2024-07-17T20:10:52Z

Description of Change(s)

⚠ Note: This PR is for feature-hgi-vulkan branch.

Introduction

The goal of this PR is to fix various issue with HgiVulkan to enable Lavapipe (software rasterizer) compatibility. This was done by fixing bugs found when running the HdSt tests. Lavapipe is a conformant Vulkan 1.3 implementation: all issues encountered are actually Vulkan API usage problems, and could very well be encountered on actual hardware device. No special code is required for supporting Lavapipe as long as HgiVulkan uses the Vulkan API correctly.

Most fixes fall into two categories:

Device limits are ignored, mostly max memory size and alignment requirements
Tests are OpenGL specific and should explicitly disable Vulkan

Breakdown of changes

testHgiVulkan
- Problem: assumes Vulkan debugging is enabled
  - Fix: add HGI_ENABLE_VULKAN=1 and HGIVULKAN_DEBUG=1 to pxr_register_test ENV argument
testHdStBufferAggregation
- Problem: assumes that VkPhysicalDeviceLimits::minUniformBufferOffsetAlignment is 256
  - Fix: use HgiCapabilities::GetUniformBufferOffsetAlignment()
- Problem: can generate more than 2GB of log files on failure
  - Fix: limit log to first 100 incorrect values in the huge buffer test
testHdStBufferArray
- Problem: output is device dependent (uboSize depends on VkPhysicalDeviceLimits::minUniformBufferOffsetAlignment) and diff'd against a constant baseline
  - Fix: ignore uboSize in diff. ssboSize is also ignored for the same reason as uboSize (depends on minStorageBufferOffsetAlignment)
testHdStBarAllocationLimit
- Problem: assumes that VkPhysicalDeviceLimits::maxStorageBufferRange is 2^30 (default value of setting HD_MAX_VBO_SIZE)
  - Fix: use the smallest of HgiCapabilities::GetMaxShaderStorageBlockSize() and HD_MAX_VBO_SIZE
testHdStBasicDrawing
- Problem: geometry shaders with more than VkPhysicalDeviceLimits::maxGeometryInputComponents input locations are being generated.
  - Fix: Inputs are passed as arrays of int, consuming one location per int. Instead pack them into ivec4 (for array sizes smaller or equal to 4, which is the max right now for quads), to pass up to 4 values per location.
testHdStBufferAggregation
- Problem: assumes that VkPhysicalDeviceLimits::maxStorageBufferRange is 2^30 (default value of setting HD_MAX_VBO_SIZE) when checking the vboRelocated performance counter (expects it to be 9)
  - Fix: use the smallest of HgiCapabilities::GetMaxShaderStorageBlockSize() and HD_MAX_VBO_SIZE to compute the correct expected value of vboRelocated.
testHdStCodeGen
- Problem: generated Vulkan shader results are different from the OpenGL baseline.
  - NOTE: this problem was fixed by another commit before this PR could be submitted. The PR retains some changes since they remove a lot of CMake script duplication.
  - Fix: Create GL and Vulkan variants of the tests so they can use a different baseline each.
testHdStDrawItemsCache
- Problem: is consuming command buffers faster than Lavapipe can execute them, so it reaches the maximum allowed inflight of 64. Normally buffers would be reset by _EndFrameSync after submission (or by EndFrame if it gets called), but since this sync is non-blocking, and Lavapipe is slow, it's called too early and does nothing (because all the buffers are still executing). And since all these calls must be on the main thread, it deadlocks: main thread is waiting for more command buffers, but those require calling _EndFrameSync to be made available, which it can't do because it's blocked...
  - Fix: Be smarter about allocating in-flight bits: search for the next available bit. If no in-flight bits are available, then update the existing command buffers in-flight status to release ones no longer in use, instead of just waiting (which is a deadlock).
testHdStGL
- Problem: OpenGL specific test
  - Fix: Disable Vulkan
testHdStGLSL
- Problem: OpenGL specific test
  - Fix: Disable Vulkan
testHdStHWFaceCulling
- Problem: output is device dependent (uboSize depends on VkPhysicalDeviceLimits::minUniformBufferOffsetAlignment) and diff'd against a constant baseline
  - Fix: ignore uboSize in diff. ssboSize is also ignored for the same reason as uboSize (depends on minStorageBufferOffsetAlignment)
testHdStPrimvars
- Problem: output is device dependent (uboSize depends on VkPhysicalDeviceLimits::minUniformBufferOffsetAlignment) and diff'd against a constant baseline
  - Fix: ignore uboSize in diff. ssboSize is also ignored for the same reason as uboSize (depends on minStorageBufferOffsetAlignment)

Fixes Issue(s)

N/A

I have verified that all unit tests pass with the proposed changes

I have submitted a signed Contributor License Agreement

jesschimein · 2024-07-18T19:01:02Z

Filed as internal issue #USD-9861

jesschimein · 2024-07-18T19:01:03Z

/AzurePipelines run

azure-pipelines · 2024-07-18T19:01:14Z

Azure Pipelines successfully started running 1 pipeline(s).

jesschimein · 2024-07-22T16:38:59Z

/AzurePipelines run

azure-pipelines · 2024-07-22T16:39:09Z

Azure Pipelines successfully started running 1 pipeline(s).

…to fix failing HdSt tests when using Lavapipe. However, this change fixes a hang in many situations when using HgiVulkan, not just for Lavapipe. From PR description: testHdStDrawItemsCache - Problem: is consuming command buffers faster than Lavapipe can execute them, so it reaches the maximum allowed inflight of 64. Normally buffers would be reset by _EndFrameSync after submission (or by EndFrame if it gets called), but since this sync is non-blocking, and Lavapipe is slow, it's called too early and does nothing (because all the buffers are still executing). And since all these calls must be on the main thread, it deadlocks: main thread is waiting for more command buffers, but those require calling _EndFrameSync to be made available, which it can't do because it's blocked... - Fix: Be smarter about allocating in-flight bits: search for the next available bit. If no in-flight bits are available, then update the existing command buffers in-flight status to release ones no longer in use, instead of just waiting (which is a deadlock). See #3170. (Internal change: 2336072)

DDoS · 2024-09-06T18:44:54Z

pxr/imaging/hdSt/codeGen.cpp

@@ -4611,6 +4718,8 @@ HdSt_CodeGen::_GenerateDrawingCoord(
                                  /*name=*/name,
                                  /*dataType=*/_tokens->_int);
        }
+
+        _PlumbInterstageElements();


Turns out this if (!_hasCS) { ... _PlumbInterstageElements(); } block should be moved after the static const std::vector<std::string> drawingCoordParams { ... } block near the top of the function. Otherwise the code at line 4433 doesn't find the mapping. It's also missing the "vs_" prefix.

jesschimein · 2024-09-10T16:03:43Z

/AzurePipelines run

azure-pipelines · 2024-09-10T16:03:54Z

Azure Pipelines successfully started running 1 pipeline(s).

clach · 2024-09-12T22:21:13Z

pxr/imaging/hgiVulkan/device.cpp

@@ -69,7 +69,8 @@ _SupportsPresentation(
                vkGetInstanceProcAddr(
                    vkInstance,
                    "vkGetPhysicalDeviceXlibPresentationSupportKHR");
-        return vkGetPhysicalDeviceXlibPresentationSupportKHR(
+        return vkGetPhysicalDeviceXlibPresentationSupportKHR &&


Just FYI, this gives me a compilation error: the address of 'VkBool32 vkGetPhysicalDeviceXlibPresentationSupportKHR(VkPhysicalDevice, uint32_t, Display*, VisualID)' will never be NULL

That's interesting, I actually observed this being null, when I used Lavapipe headless (didn't build it with X support). Is it a warning as an error or an actual error?

It's a warning as an error. Do you know if there's some define we can use to branch this?

~~Maybe the WSI macros? https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/WSIheaders.html~~
Nevermind, we're already using those. I might need to retest this to understand what's going wrong better.

vkGetInstanceProcAddr can return null, so I don't understand why the compiler would warn here.

DDoS · 2024-10-02T16:48:04Z

pxr/imaging/hdSt/codeGen.cpp

-        std::string const index = std::to_string(i);
-        ss << "  " << outputPrefix << "instanceCoordsI" << index << outArraySize
-           << " = " << "dc.instanceCoords[" << index << "]" << ";\n";
+        const auto [groupName, component] = _GetDrawingCoordMapping("instanceIndexI" + std::to_string(i));


Bug: "instanceIndexI" should be "instanceCoordsI"

jesschimein · 2024-10-02T17:34:49Z

/AzurePipelines run

azure-pipelines · 2024-10-02T17:34:59Z

Azure Pipelines successfully started running 1 pipeline(s).

failing HdSt tests when using Lavapipe. From PR description: testHdStBasicDrawing Problem: geometry shaders with more than VkPhysicalDeviceLimits::maxGeometryInputComponents input locations are being generated. Fix: Inputs are passed as arrays of int, consuming one location per int. Instead pack them into ivec4 (for array sizes smaller or equal to 4, which is the max right now for quads), to pass up to 4 values per location. See #3170. (Internal change: 2344215)

Removes CMake script duplication for GL and Vulkan versions of testHdStCodeGen. Adds some missing test directories. See github pull request #3170. (Internal change: 2345394)

erikaharrison-adsk force-pushed the adsk/feature/fix-vk-hdst-test branch from 01a7ae9 to 5a9c960 Compare July 19, 2024 15:57

DDoS reviewed Sep 6, 2024

View reviewed changes

DDoS force-pushed the adsk/feature/fix-vk-hdst-test branch 2 times, most recently from bb4fc08 to cfe2c61 Compare September 10, 2024 14:43

clach reviewed Sep 12, 2024

View reviewed changes

DDoS reviewed Oct 2, 2024

View reviewed changes

Fix hdSt Vulkan tests under Lavapipe

f1aee83

DDoS force-pushed the adsk/feature/fix-vk-hdst-test branch from cfe2c61 to f1aee83 Compare October 2, 2024 17:06

pixar-oss closed this in ab98317 Jan 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autodesk: Fix HdSt tests failing under HgiVulkan with Lavapipe #3170

Autodesk: Fix HdSt tests failing under HgiVulkan with Lavapipe #3170

erikaharrison-adsk commented Jul 17, 2024

jesschimein commented Jul 18, 2024

jesschimein commented Jul 18, 2024

azure-pipelines bot commented Jul 18, 2024

jesschimein commented Jul 22, 2024

azure-pipelines bot commented Jul 22, 2024

DDoS Sep 6, 2024

jesschimein commented Sep 10, 2024

azure-pipelines bot commented Sep 10, 2024

clach Sep 12, 2024

DDoS Sep 12, 2024

clach Sep 16, 2024

DDoS Sep 16, 2024 •

edited

Loading

DDoS Sep 16, 2024

DDoS Oct 2, 2024

jesschimein commented Oct 2, 2024

azure-pipelines bot commented Oct 2, 2024

Autodesk: Fix HdSt tests failing under HgiVulkan with Lavapipe #3170

Autodesk: Fix HdSt tests failing under HgiVulkan with Lavapipe #3170

Conversation

erikaharrison-adsk commented Jul 17, 2024

Description of Change(s)

Introduction

Breakdown of changes

Fixes Issue(s)

jesschimein commented Jul 18, 2024

jesschimein commented Jul 18, 2024

azure-pipelines bot commented Jul 18, 2024

jesschimein commented Jul 22, 2024

azure-pipelines bot commented Jul 22, 2024

DDoS Sep 6, 2024

Choose a reason for hiding this comment

jesschimein commented Sep 10, 2024

azure-pipelines bot commented Sep 10, 2024

clach Sep 12, 2024

Choose a reason for hiding this comment

DDoS Sep 12, 2024

Choose a reason for hiding this comment

clach Sep 16, 2024

Choose a reason for hiding this comment

DDoS Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

DDoS Sep 16, 2024

Choose a reason for hiding this comment

DDoS Oct 2, 2024

Choose a reason for hiding this comment

jesschimein commented Oct 2, 2024

azure-pipelines bot commented Oct 2, 2024

DDoS Sep 16, 2024 •

edited

Loading