AcademySoftwareFoundation / OpenShadingLanguage

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

Add type information for needed attributes. #1650

Closed curtisblack closed 1 year ago

curtisblack commented 1 year ago

Description

We can currently query which attribute names and scopes are requested in a shader. This PR extends that idea to also allow querying the attribute types. This now more closely matches the equivalent available queries for user data.

For example, this shader:

shader Shader
{
  color c;
  getattribute("user_color", c);

  string s;
  getattribute("user_string", s);
}

allows us to do this:

int nattr = 0;
if (shadingSys->getattribute(shaderRef.get(), "num_attributes_needed", nattr) && nattr)
{
  // this already works
  OIIO::ustring* names = nullptr;
  shadingSys->getattribute(shaderRef.get(), "attributes_needed", OSL::TypeDesc::PTR, &names);

  // this is the added feature
  OIIO::TypeDesc* types = nullptr;
  shadingSys->getattribute(shaderRef.get(), "attribute_types", OSL::TypeDesc::PTR, &types);

  // nattr: 2
  // names: ["user_color", "user_string"]
  // types: [OIIO::TypeColor, OIIO::TypeString]
}

The motivation behind this feature is to perform additional type validation at material compile time when determining what primvars can be provided to the shader.

One side-effect of this change to be aware of is that, as mentioned in the code comments, if the same name is requested multiple times with different types, it will now be reported multiple times. Hopefully this is not too much of an issue as the same behaviour also occurs when an attribute is requested in multiple scopes, or when user data is requested with multiple types.

Tests

I wasn't able to find any existing unit tests for the "attributes_needed" feature to use as a starting point, if they do already exist please let me know and I can make those changes. I did see a spot in testshade where it seemed appropriate to add a usage of this feature.

Checklist:

linux-foundation-easycla[bot] commented 1 year ago

CLA Signed

The committers listed above are authorized under a signed CLA.

lgritz commented 1 year ago

Looks like there are a few formatting issues to fix (look at the logs of the failed clang-format test), and also needs a DCO message attached to the commit. But the code LGTM.

lgritz commented 1 year ago

@curtisblack Ping! -- can you please push an update with the DCO and fixed formatting so I can merge this in time for the monthly patch release this week?