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.

Sign up for GitHub

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

Add support for EXT_ray_flags_primitive_culling. #2173

Merged
merged 1 commit into from
Apr 9, 2020
Merged

Add support for EXT_ray_flags_primitive_culling. #2173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions SPIRV/GlslangToSpv.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,10 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion,
builder.addExecutionMode(shaderEntry, spv::ExecutionModeXfb);
}

if (sourceExtensions.find("GL_EXT_ray_flags_primitive_culling") != sourceExtensions.end()) {
Copy link
Member

Choose a reason for hiding this comment

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

Are semantics actually changed by declaring the extension, or is there some feature that must get used to truly need the capability?

Copy link
Member

Choose a reason for hiding this comment

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

Generally, we are triggering capability need by use of the ability.

Copy link
Member

Choose a reason for hiding this comment

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

See for example TGlslangToSpvTraverser::TranslateBuiltInDecoration

Copy link
Contributor

Choose a reason for hiding this comment

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

RayFlag values 256 and 512 become valid when this capability is enabled, but there are more ways that you could get the values 256 and 512 than by just using the gl_RayFlagsSkipAABBEXT and gl_RayFlagsSkipTrianglesEXT constants.

Copy link
Member

Choose a reason for hiding this comment

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

So, on the one hand it sounds like we should be testing for emitting 256 and 512 as the reason to require the capability, but on the other hand I'm not clear if that's your position.

Speculating: If this can't be detected 100% by a feature use on translation, we also have Builder::postProcessFeatures() to detect what was needed after translation completed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ultimately, there is no guaranteed way to know at compile time if this capability is needed.
The "Ray Flags" argument to OpTraceRayKHR is any Id. No requirement for it to be a constant. It could be loaded from memory, or passed in from another stage (as is done with the IncomingRayFlagsKHR builtin). This ultimately needs run-time validation, so we believe we need a separate extension to enable the shader author to specifically opt-in to using the capability.

Copy link
Member

Choose a reason for hiding this comment

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

I see. It's a feature that the compiler cannot tell is being used, so the extension does nothing except signal a run-time value might appear. Have we done this much?

The way to semantically guard such a feature would be, for example, to have the #extension declare a keyword/type/declaration/etc. that syntactically declares what is in use. These could turn into modes in SPIR-V rather than capabilities. There are choices in all those steps, but the point is that it is quite possible and common to still have a syntactic or semantically statically visible sign of what feature is in use.

Saying it more shortly, the #extension line by itself should not be what changes value semantics, it should be what enables a language feature that says what happens to semantics.

Copy link
Member

Choose a reason for hiding this comment

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

On re-reading the GLSL extension, the dynamic part really does not come through. It is focused on providing the two built-ins, not on adding a new mode that becomes present whether or not the new built-ins are consumed.

I think this deserves better documentation, if not actual language semantics.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can improve the documentation in the GLSL extension. I don't know that we have precedent for anything like this, and I'm not sure how any modes or other declarations would really be an improvement, but can you add further suggestions in https://gitlab.khronos.org/vulkan/vulkan/issues/2073 and we can consider changing that for final?
Right now we are blocked on actually being able to test these features until something is done here.

builder.addCapability(spv::CapabilityRayTraversalPrimitiveCullingProvisionalKHR);
}

unsigned int mode;
switch (glslangIntermediate->getStage()) {
case EShLangVertex:
Expand Down
118 changes: 60 additions & 58 deletions Test/baseResults/rayQuery-allOps.comp.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
rayQuery-allOps.comp
// Module Version 10000
// Generated by (magic number): 80008
// Id's are bound by 257
// Id's are bound by 258

Capability Shader
Capability RayQueryProvisionalKHR
Capability RayTraversalPrimitiveCullingProvisionalKHR
Extension "SPV_KHR_ray_query"
1: ExtInstImport "GLSL.std.450"
MemoryModel Logical GLSL450
EntryPoint GLCompute 4 "main"
ExecutionMode 4 LocalSize 1 1 1
Source GLSL 460
SourceExtension "GL_EXT_ray_flags_primitive_culling"
SourceExtension "GL_EXT_ray_query"
SourceExtension "GL_NV_ray_tracing"
Name 4 "main"
Name 6 "doSomething("
Name 10 "Ray"
Expand All @@ -34,32 +35,32 @@ rayQuery-allOps.comp
Name 83 "_mat3x4"
Name 143 "t"
Name 156 "committedStatus"
Name 240 "o"
Name 242 "d"
Name 252 "Ray"
MemberName 252(Ray) 0 "pos"
MemberName 252(Ray) 1 "tmin"
MemberName 252(Ray) 2 "dir"
MemberName 252(Ray) 3 "tmax"
Name 254 "Rays"
MemberName 254(Rays) 0 "rays"
Name 256 ""
Name 241 "o"
Name 243 "d"
Name 253 "Ray"
MemberName 253(Ray) 0 "pos"
MemberName 253(Ray) 1 "tmin"
MemberName 253(Ray) 2 "dir"
MemberName 253(Ray) 3 "tmax"
Name 255 "Rays"
MemberName 255(Rays) 0 "rays"
Name 257 ""
MemberDecorate 15(Log) 0 Offset 0
MemberDecorate 15(Log) 1 Offset 4
Decorate 15(Log) BufferBlock
Decorate 17 DescriptorSet 0
Decorate 17 Binding 0
Decorate 50(rtas) DescriptorSet 0
Decorate 50(rtas) Binding 1
MemberDecorate 252(Ray) 0 Offset 0
MemberDecorate 252(Ray) 1 Offset 12
MemberDecorate 252(Ray) 2 Offset 16
MemberDecorate 252(Ray) 3 Offset 28
Decorate 253 ArrayStride 32
MemberDecorate 254(Rays) 0 Offset 0
Decorate 254(Rays) BufferBlock
Decorate 256 DescriptorSet 0
Decorate 256 Binding 2
MemberDecorate 253(Ray) 0 Offset 0
MemberDecorate 253(Ray) 1 Offset 12
MemberDecorate 253(Ray) 2 Offset 16
MemberDecorate 253(Ray) 3 Offset 28
Decorate 254 ArrayStride 32
MemberDecorate 255(Rays) 0 Offset 0
Decorate 255(Rays) BufferBlock
Decorate 257 DescriptorSet 0
Decorate 257 Binding 2
2: TypeVoid
3: TypeFunction 2
8: TypeFloat 32
Expand Down Expand Up @@ -104,11 +105,12 @@ rayQuery-allOps.comp
144: 8(float) Constant 1056964608
175: 14(int) Constant 1
198: 14(int) Constant 2
252(Ray): TypeStruct 9(fvec3) 8(float) 9(fvec3) 8(float)
253: TypeRuntimeArray 252(Ray)
254(Rays): TypeStruct 253
255: TypePointer Uniform 254(Rays)
256: 255(ptr) Variable Uniform
231: 14(int) Constant 256
253(Ray): TypeStruct 9(fvec3) 8(float) 9(fvec3) 8(float)
254: TypeRuntimeArray 253(Ray)
255(Rays): TypeStruct 254
256: TypePointer Uniform 255(Rays)
257: 256(ptr) Variable Uniform
4(main): 2 Function None 3
5: Label
43(ray): 25(ptr) Variable Function
Expand All @@ -118,8 +120,8 @@ rayQuery-allOps.comp
83(_mat3x4): 82(ptr) Variable Function
143(t): 35(ptr) Variable Function
156(committedStatus): 68(ptr) Variable Function
240(o): 29(ptr) Variable Function
242(d): 29(ptr) Variable Function
241(o): 29(ptr) Variable Function
243(d): 29(ptr) Variable Function
44: 10(Ray) FunctionCall 12(makeRayDesc()
Store 43(ray) 44
51: 48 Load 50(rtas)
Expand Down Expand Up @@ -375,36 +377,36 @@ rayQuery-allOps.comp
Branch 228
228: Label
230: 14(int) RayQueryGetRayFlagsKHR 47(rayQuery)
231: 66(bool) UGreaterThan 230 20
SelectionMerge 233 None
BranchConditional 231 232 233
232: Label
234: 2 FunctionCall 6(doSomething()
Branch 233
233: Label
235: 8(float) RayQueryGetRayTMinKHR 47(rayQuery)
236: 66(bool) FOrdGreaterThan 235 27
SelectionMerge 238 None
BranchConditional 236 237 238
237: Label
239: 2 FunctionCall 6(doSomething()
Branch 238
238: Label
241: 9(fvec3) RayQueryGetWorldRayOriginKHR 47(rayQuery)
Store 240(o) 241
243: 9(fvec3) RayQueryGetWorldRayDirectionKHR 47(rayQuery)
Store 242(d) 243
244: 35(ptr) AccessChain 240(o) 20
245: 8(float) Load 244
246: 35(ptr) AccessChain 242(d) 198
247: 8(float) Load 246
248: 66(bool) FOrdEqual 245 247
SelectionMerge 250 None
BranchConditional 248 249 250
249: Label
251: 2 FunctionCall 6(doSomething()
Branch 250
250: Label
232: 66(bool) UGreaterThan 230 231
SelectionMerge 234 None
BranchConditional 232 233 234
233: Label
235: 2 FunctionCall 6(doSomething()
Branch 234
234: Label
236: 8(float) RayQueryGetRayTMinKHR 47(rayQuery)
237: 66(bool) FOrdGreaterThan 236 27
SelectionMerge 239 None
BranchConditional 237 238 239
238: Label
240: 2 FunctionCall 6(doSomething()
Branch 239
239: Label
242: 9(fvec3) RayQueryGetWorldRayOriginKHR 47(rayQuery)
Store 241(o) 242
244: 9(fvec3) RayQueryGetWorldRayDirectionKHR 47(rayQuery)
Store 243(d) 244
245: 35(ptr) AccessChain 241(o) 20
246: 8(float) Load 245
247: 35(ptr) AccessChain 243(d) 198
248: 8(float) Load 247
249: 66(bool) FOrdEqual 246 248
SelectionMerge 251 None
BranchConditional 249 250 251
250: Label
252: 2 FunctionCall 6(doSomething()
Branch 251
251: Label
Return
FunctionEnd
6(doSomething(): 2 Function None 3
Expand Down
3 changes: 2 additions & 1 deletion Test/baseResults/spv.ext.ClosestHitShader_Errors.rchit.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ ERROR: 0:8: 'assign' : l-value required "payload" (cannot modify hitAttributeNV
ERROR: 0:9: 'reportIntersectionEXT' : no matching overloaded function found
ERROR: 0:10: 'terminateRayEXT' : no matching overloaded function found
ERROR: 0:11: 'ignoreIntersectionEXT' : no matching overloaded function found
ERROR: 4 compilation errors. No code generated.
ERROR: 0:12: 'gl_RayFlagsSkipAABBEXT' : required extension not requested: GL_EXT_ray_flags_primitive_culling
ERROR: 5 compilation errors. No code generated.


SPIR-V is not generated for failed compile or link
81 changes: 42 additions & 39 deletions Test/baseResults/spv.ext.RayGenShader.rgen.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
spv.ext.RayGenShader.rgen
// Module Version 10000
// Generated by (magic number): 80008
// Id's are bound by 57
// Id's are bound by 58

Capability RayTraversalPrimitiveCullingProvisionalKHR
Capability RayTracingProvisionalKHR
Extension "SPV_KHR_ray_tracing"
1: ExtInstImport "GLSL.std.450"
MemoryModel Logical GLSL450
EntryPoint RayGenerationKHR 4 "main" 11 21
Source GLSL 460
SourceExtension "GL_EXT_ray_flags_primitive_culling"
SourceExtension "GL_EXT_ray_tracing"
Name 4 "main"
Name 8 "lx"
Expand All @@ -18,25 +20,25 @@ spv.ext.RayGenShader.rgen
Name 21 "gl_LaunchSizeEXT"
Name 24 "sy"
Name 29 "accEXT0"
Name 37 "block"
MemberName 37(block) 0 "dir"
MemberName 37(block) 1 "origin"
Name 39 ""
Name 50 "accEXT1"
Name 53 "imageu"
Name 56 "payload"
Name 38 "block"
MemberName 38(block) 0 "dir"
MemberName 38(block) 1 "origin"
Name 40 ""
Name 51 "accEXT1"
Name 54 "imageu"
Name 57 "payload"
Decorate 11(gl_LaunchIDEXT) BuiltIn LaunchIdKHR
Decorate 21(gl_LaunchSizeEXT) BuiltIn LaunchSizeKHR
Decorate 29(accEXT0) DescriptorSet 0
Decorate 29(accEXT0) Binding 0
MemberDecorate 37(block) 0 Offset 0
MemberDecorate 37(block) 1 Offset 16
Decorate 37(block) BufferBlock
Decorate 50(accEXT1) DescriptorSet 0
Decorate 50(accEXT1) Binding 1
Decorate 53(imageu) DescriptorSet 0
Decorate 53(imageu) Binding 2
Decorate 56(payload) Location 0
MemberDecorate 38(block) 0 Offset 0
MemberDecorate 38(block) 1 Offset 16
Decorate 38(block) BufferBlock
Decorate 51(accEXT1) DescriptorSet 0
Decorate 51(accEXT1) Binding 1
Decorate 54(imageu) DescriptorSet 0
Decorate 54(imageu) Binding 2
Decorate 57(payload) Location 0
2: TypeVoid
3: TypeFunction 2
6: TypeInt 32 0
Expand All @@ -51,24 +53,25 @@ spv.ext.RayGenShader.rgen
27: TypeAccelerationStructureKHR
28: TypePointer UniformConstant 27
29(accEXT0): 28(ptr) Variable UniformConstant
35: TypeFloat 32
36: TypeVector 35(float) 3
37(block): TypeStruct 36(fvec3) 36(fvec3)
38: TypePointer ShaderRecordBufferKHR 37(block)
39: 38(ptr) Variable ShaderRecordBufferKHR
40: TypeInt 32 1
41: 40(int) Constant 1
42: TypePointer ShaderRecordBufferKHR 36(fvec3)
45: 35(float) Constant 1056964608
46: 40(int) Constant 0
49: 35(float) Constant 1061158912
50(accEXT1): 28(ptr) Variable UniformConstant
51: TypeImage 6(int) 2D nonsampled format:R32ui
52: TypePointer UniformConstant 51
53(imageu): 52(ptr) Variable UniformConstant
54: TypeVector 35(float) 4
55: TypePointer RayPayloadKHR 54(fvec4)
56(payload): 55(ptr) Variable RayPayloadKHR
35: 6(int) Constant 768
36: TypeFloat 32
37: TypeVector 36(float) 3
38(block): TypeStruct 37(fvec3) 37(fvec3)
39: TypePointer ShaderRecordBufferKHR 38(block)
40: 39(ptr) Variable ShaderRecordBufferKHR
41: TypeInt 32 1
42: 41(int) Constant 1
43: TypePointer ShaderRecordBufferKHR 37(fvec3)
46: 36(float) Constant 1056964608
47: 41(int) Constant 0
50: 36(float) Constant 1061158912
51(accEXT1): 28(ptr) Variable UniformConstant
52: TypeImage 6(int) 2D nonsampled format:R32ui
53: TypePointer UniformConstant 52
54(imageu): 53(ptr) Variable UniformConstant
55: TypeVector 36(float) 4
56: TypePointer RayPayloadKHR 55(fvec4)
57(payload): 56(ptr) Variable RayPayloadKHR
4(main): 2 Function None 3
5: Label
8(lx): 7(ptr) Variable Function
Expand All @@ -92,10 +95,10 @@ spv.ext.RayGenShader.rgen
32: 6(int) Load 16(ly)
33: 6(int) Load 20(sx)
34: 6(int) Load 24(sy)
43: 42(ptr) AccessChain 39 41
44: 36(fvec3) Load 43
47: 42(ptr) AccessChain 39 46
48: 36(fvec3) Load 47
TraceRayKHR 30 31 32 33 34 12 44 45 48 49 41
44: 43(ptr) AccessChain 40 42
45: 37(fvec3) Load 44
48: 43(ptr) AccessChain 40 47
49: 37(fvec3) Load 48
TraceRayKHR 30 31 32 33 34 35 45 46 49 50 42
Return
FunctionEnd
4 changes: 2 additions & 2 deletions Test/rayQuery-allOps.comp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#version 460
#extension GL_NV_ray_tracing : enable
#extension GL_EXT_ray_query : enable
#extension GL_EXT_ray_flags_primitive_culling : enable

struct Ray
{
Expand Down Expand Up @@ -192,7 +192,7 @@ void main()
doSomething();
}

if (rayQueryGetRayFlagsEXT(rayQuery) > 0)
if (rayQueryGetRayFlagsEXT(rayQuery) > gl_RayFlagsSkipTrianglesEXT)
{
doSomething();
}
Expand Down
1 change: 1 addition & 0 deletions Test/spv.ext.ClosestHitShader_Errors.rchit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ void main()
reportIntersectionEXT(1.0, 1U); // ERROR, unsupported builtin in stage
terminateRayEXT();
ignoreIntersectionEXT();
bool e1 = gl_IncomingRayFlagsEXT == gl_RayFlagsSkipAABBEXT;
}
3 changes: 2 additions & 1 deletion Test/spv.ext.RayGenShader.rgen
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#version 460
#extension GL_EXT_ray_tracing : enable
#extension GL_EXT_ray_flags_primitive_culling : enable
layout(binding = 0, set = 0) uniform accelerationStructureEXT accEXT0;
layout(binding = 1, set = 0) uniform accelerationStructureEXT accEXT1; // Unused
layout(binding = 2, r32ui) shadercallcoherent uniform uimage2D imageu;
Expand All @@ -16,5 +17,5 @@ void main()
uint ly = gl_LaunchIDEXT.y;
uint sx = gl_LaunchSizeEXT.x;
uint sy = gl_LaunchSizeEXT.y;
traceRayEXT(accEXT0, lx, ly, sx, sy, 0u, origin, 0.5f, dir, 0.75f, 1);
traceRayEXT(accEXT0, lx, ly, sx, sy, gl_RayFlagsSkipTrianglesEXT | gl_RayFlagsSkipAABBEXT, origin, 0.5f, dir, 0.75f, 1);
}
1 change: 1 addition & 0 deletions Test/spv.ext.RayGenShader_Errors.rgen
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void main()
mat4x3 e11 = gl_WorldToObjectEXT; // ERROR, unsupported builtin in stage
float e12 = gl_HitTEXT; // ERROR, unsupported builtin in stage
float e13 = gl_HitKindEXT; // ERROR, unsupported builtin in stage
int e14 = gl_RayFlagsSkipAABBEXT; // ERROR, unsupported builtin in stage
reportIntersectionEXT(1.0, 1U); // ERROR, unsupported builtin in stage
ignoreIntersectionEXT(); // ERROR, unsupported builtin in stage
terminateRayEXT(); // ERROR, unsupported builtin in stage
Expand Down
4 changes: 4 additions & 0 deletions glslang/MachineIndependent/Initialize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5457,6 +5457,8 @@ void TBuiltIns::initialize(int version, EProfile profile, const SpvVersion& spvV
"const uint gl_RayFlagsCullOpaqueEXT = 64U;"
"const uint gl_RayFlagsCullNoOpaqueNV = 128U;"
"const uint gl_RayFlagsCullNoOpaqueEXT = 128U;"
"const uint gl_RayFlagsSkipTrianglesEXT = 256U;"
"const uint gl_RayFlagsSkipAABBEXT = 512U;"
"const uint gl_HitKindFrontFacingTriangleEXT = 254U;"
"const uint gl_HitKindBackFacingTriangleEXT = 255U;"
"\n";
Expand Down Expand Up @@ -7572,6 +7574,8 @@ void TBuiltIns::identifyBuiltIns(int version, EProfile profile, const SpvVersion
symbolTable.setFunctionExtensions("rayQueryGetIntersectionWorldToObjectEXT", 1, &E_GL_EXT_ray_query);
symbolTable.setFunctionExtensions("rayQueryGetWorldRayOriginEXT", 1, &E_GL_EXT_ray_query);
symbolTable.setFunctionExtensions("rayQueryGetWorldRayDirectionEXT", 1, &E_GL_EXT_ray_query);
symbolTable.setVariableExtensions("gl_RayFlagsSkipAABBEXT", 1, &E_GL_EXT_ray_flags_primitive_culling);
symbolTable.setVariableExtensions("gl_RayFlagsSkipTrianglesEXT", 1, &E_GL_EXT_ray_flags_primitive_culling);
}

if ((profile != EEsProfile && version >= 130) ||
Expand Down
Loading