Refactor TypeDesc and StructTypeDesc storage

[ld-kerley]

This PR moves the TypeDesc storage away from static storage to a model where these objects are stored inside the GenContext object.

The TypeDesc objects are registered in the GenContext object, potentially per document load.

Struct unit tests added for validation.

Outstanding questions:

• Decide where the struct test should live.
  • Leave it in stdlib?
  • Create a new folder to test MaterialX features separately from the standard nodes?
• Keep the TypeDescStorage class as a separate object type or fold its contents inside the GenContext class itself - which is the only place its currently used. Now folded inside GenContext class
• Does registering a TypeDesc need to be made more thread safe, related, make a decision about what the behavior should be if the same type is registered more than once.
  • Just skip the registration if the type name is already registered.
  • Register the newly provided type in place of whatever type is already registered.
  • Compare the existing registered type to the new type and raise an error if they differ.

I'll take a pass over the documentation once the code has been reviewed.

[niklasharrysson]

With the change to store type descriptions locally in GenContext, a lot of the API calls have been updated to pass in the context. Currently it's a bit random how it's passed in. Maybe the API would look more consistent if the context was always passed as first argument to the calls that need this?

[ld-kerley]

I'm happy to conform that, I was wary of making too many API changes all at once. Perhaps we get this update to land - and then I can take a look at a separate PR that makes the API feel more consistent. I think there's also scope to look at passing GenContext as a const GenContext& in more places too perhaps.

[ld-kerley]

Interested to hear from @jstone-lucasfilm what his preference is here. Defer API changes - or roll them all in to the same PR?

[niklasharrysson]

I was mainly referring to just the API functions that were changed here anyway. Not to change this in all API. That could definitely wait for a later PR.

But this was just an idea for consistency. Perhaps it becomes less consistent anyway because of how it is passed in other places in the API. It’s just coding style and not a big issue.

[ld-kerley]

I do like the idea of making it consistent as a secondary PR.

[niklasharrysson]

@ld-kerley This looks like a great direction to go, to remove the static storage of the type descriptions. I've added some initial comments above. Thanks!

[ld-kerley]

While we're considering larger scale changes - the other thing that would remove another slight "wort", would be if we switched the TypeDesc storage from an unordered_map to a container that retains order.

Before the structs it was unimportant, but because we're not allowing a struct to use another previously defined struct, it's important to retain the order in which the types are registered - at least the structs.

We could just go simple and use vector<TypeDesc>. I know we do lookup by name, but find() on a vector of approx 30 elements is not crazy. That or we could use map or maybe even tsl::ordered_map (https://github.com/Tessil/ordered-map) which I've used in other projects and found to be nicely performant - header only.

[niklasharrysson]

While we're considering larger scale changes - the other thing that would remove another slight "wort", would be if we switched the TypeDesc storage from an unordered_map to a container that retains order.

Before the structs it was unimportant, but because we're not allowing a struct to use another previously defined struct, it's important to retain the order in which the types are registered - at least the structs.

We could just go simple and use vector<TypeDesc>. I know we do lookup by name, but find() on a vector of approx 30 elements is not crazy. That or we could use map or maybe even tsl::ordered_map (https://github.com/Tessil/ordered-map) which I've used in other projects and found to be nicely performant - header only.

Since the GenContext is a big class and we normally have only a single instance of this per thread, I think it's fine to store extra data here, if needed for better performance. So we could store both a map and vector for the type descriptions, one for quick access by name and one for ordered access.

[jstone-lucasfilm]

Thanks for the great proposal and discussion, @ld-kerley, and I believe we can close this now that #2176 has landed in main.