AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.09k stars 357 forks source link

Add missing members to ShaderGlobals in rend_lib.h #1721

Closed tgrant-nv closed 1 year ago

tgrant-nv commented 1 year ago

Description

There is a separate definition of ShaderGlobals in testrender/cuda/rend_lib.h that has drifted away from the definition in include/OSL/shaderglobals.h. Two new members were added in #1702 which changed the byte offset of the output closure Ci, which leads to incorrect shader output since the offset is baked into the PTX.

Adding the new members fixes the shading issue. But this should be regarded as a short-term fix, since the two ShaderGlobals definitions have diverged in other ways that might be significant.

Tests

All OptiX tests passing with the exception of testoptix.optix.opt, which is a preexisting failure.

Checklist:

lgritz commented 1 year ago

should we have a static_assert somewhere to ensure that these structs don't drift apart inadvertently again?

tgrant-nv commented 1 year ago

There are some other differences that I'm not quite sure how to sort out. Such as shaderID being tacked on to the end of the CUDA version. The order of some members is different as well. It would definitely be nice to synchronize them and keep them in sync, though.

tgrant-nv commented 1 year ago

I'll reorder the CUDA version to match the "real" version.

tgrant-nv commented 1 year ago

It was just shadingStateUniform that was a bit out of place. I've rearranged the members so the layout matches shaderglobals.h, with the exception of shaderID. shaderID could probably be tucked into one of the existing pointees, but that would be a bit of a refactor.