Skip to content
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

Relax nvrhi push constant limits to permit platform-specific runtime checks #6

Closed
wants to merge 33 commits into from

Conversation

SRSaunders
Copy link

@SRSaunders SRSaunders commented Oct 12, 2023

Expand platform push constant limits to 256 bytes on Windows and Linux, and 4096 bytes on macOS. Application can then determine and enforce application-specific and GPU-specific limits at runtime.

stephenap07 and others added 30 commits January 5, 2022 12:43
It's only supported in DX12 so far.
CMakeLists fix for NVRHI shader compiler to support AppleClang
It's only supported in DX12 so far.
@asemarafa
Copy link

asemarafa commented Oct 13, 2023

I encountered a build error in m1 mac, in vulkan-resource-bindings.cpp related to the hash_combine function. The error is as follows:


error: no matching function for call to 'hash_combine'
note: candidate function template not viable: no known conversion from 'uint64_t' (aka 'unsigned long long') to 'size_t &' (aka 'unsigned long &') for 1st argument

I've made some adjustments to resolve this issue

diff --git a/rtxmu b/rtxmu
index c27b13e..799a918 160000
--- a/rtxmu
+++ b/rtxmu
@@ -1 +1 @@
-Subproject commit c27b13ee5ff5966ac368df553f3b8b5d1272d855
+Subproject commit 799a918af94000d22828125d46aefd6ecd947e8d
diff --git a/src/vulkan/vulkan-resource-bindings.cpp b/src/vulkan/vulkan-resource-bindings.cpp
index 3b5b044..9af80b8 100644
--- a/src/vulkan/vulkan-resource-bindings.cpp
+++ b/src/vulkan/vulkan-resource-bindings.cpp
@@ -453,9 +453,11 @@ namespace nvrhi::vulkan
                 const auto range = binding.range.resolve(buffer->desc);
 
                 uint64_t viewInfoHash = 0;
-                nvrhi::hash_combine(viewInfoHash, range.byteOffset);
-                nvrhi::hash_combine(viewInfoHash, range.byteSize);
-                nvrhi::hash_combine(viewInfoHash, (uint64_t)vkformat);
+		size_t seed = static_cast<size_t>(viewInfoHash);
+		nvrhi::hash_combine(seed, range.byteOffset);
+		nvrhi::hash_combine(seed, range.byteSize);
+		nvrhi::hash_combine(seed, (uint64_t)vkformat);
+		viewInfoHash = static_cast<uint64_t>(seed);
 
                 const auto& bufferViewFound = buffer->viewCache.find(viewInfoHash);
                 auto& bufferViewRef = (bufferViewFound != buffer->viewCache.end()) ? bufferViewFound->second : buffer->viewCache[viewInfoHash];
@@ -772,9 +774,11 @@ namespace nvrhi::vulkan
 
                     const auto range = binding.range.resolve(buffer->desc);
                     uint64_t viewInfoHash = 0;
-                    nvrhi::hash_combine(viewInfoHash, range.byteOffset);
-                    nvrhi::hash_combine(viewInfoHash, range.byteSize);
-                    nvrhi::hash_combine(viewInfoHash, (uint64_t)vkformat);
+		    size_t seed = static_cast<size_t>(viewInfoHash);
+		    nvrhi::hash_combine(seed, range.byteOffset);
+		    nvrhi::hash_combine(seed, range.byteSize);
+		    nvrhi::hash_combine(seed, (uint64_t)vkformat);
+		    viewInfoHash = static_cast<uint64_t>(seed);
 
                     const auto& bufferViewFound = buffer->viewCache.find(viewInfoHash);
                     auto& bufferViewRef = (bufferViewFound != buffer->viewCache.end()) ? bufferViewFound->second : buffer->viewCache[viewInfoHash];
diff --git a/thirdparty/Vulkan-Headers b/thirdparty/Vulkan-Headers
index 00671c6..0193e15 160000
--- a/thirdparty/Vulkan-Headers
+++ b/thirdparty/Vulkan-Headers
@@ -1 +1 @@
-Subproject commit 00671c64ba5c488ade22ad572a0ef81d5e64c803
+Subproject commit 0193e158bc9f4d17e3c3a61c9311a0439ed5572d


Thanks!

@SRSaunders
Copy link
Author

SRSaunders commented Oct 13, 2023

Sorry about that. If you are using Robert's nvrhi head branch I should have mentioned that you also need to apply #4. This solves the same problem. I hope you didn't spend too much time on it.

FYI - @RobertBeckebans has not yet merged these PRs since the release version of the game currently works off a detached head at commit 1cbc9e9. Perhaps he will update this soon to include improvements from Nvidia's upstream branch and these local changes.

@SRSaunders
Copy link
Author

Rebased onto updated nvrhi main...

@SRSaunders SRSaunders deleted the pushconst-limits branch February 27, 2024 15:23
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.

4 participants