KhronosGroup / GLSL

GLSL Shading Language Specification and Extensions
Other
337 stars 98 forks source link

GLSL_EXT_ray_query #149

Closed seanbaxter closed 2 years ago

seanbaxter commented 3 years ago

Can you add verbiage to GLSL_EXT_ray_query that describes (and really emphasize) that the rayQuery and accelerationStructures are handled very differently? The latter is always passed by value. But rayQuery seems to be passed by pointer/reference. This is mentioned in SPV_KHR_ray_query, but no where in GLSL_EXT_ray_query, and the SPIR-V spec still has no language on this extension.

dgkoch commented 3 years ago

SPIR-V spec still has no language on this extension.

The core SPIR-V spec doesn't get language for extensions until they are made core. Prior to that, just the tokens are listed as reserved, etc.

@seanbaxter if you want to draft a suggestion it might help this to go faster.

@Tobski would you be able to propose something for this?

seanbaxter commented 3 years ago

For these extensions I read the GLSL_* documents. All it shows is this: bool rayQueryProceedEXT(rayQueryEXT q);

How would someone look at that and know that it's pass-by-address? If the unified spec had parameter docs I wouldn't have had this problem. Same thing with the traceRayEXT instruction changing the type of its payload parameter--there's no entry in the unified spec. Maybe there is a philosophical reason not to put the text in there, but it's creating inaccuracy in programming.

Tobski commented 3 years ago

I mean the issue here is that for the purposes of GLSL, you don't care whether it's pass by address or pass by object - it's not a distinction we make in the GLSL spec, and has no semantic effect. So that's why we didn't document it anywhere in the GLSL spec. We hide those implementations details in GLSLang (or should be hiding them - if not then that's a bug...)

If you're using the GLSL spec as a reference for implementing your own mappings (which I can kind of understand why you would) then sure, I can see how that information would be useful.

I can add a note to the SPIR-V mapping section noting the difference, but that's about the only place I'd be comfortable putting it.

Tobski commented 2 years ago

This one slipped off my radar for way too long, sorry - fix now in https://github.com/KhronosGroup/GLSL/pull/189