OVDB-136: Add AttributeSet::Info convenience class

[danrbailey]

This new class resolves a number of issues:

• It collates information that lives in the attribute set descriptor (the names, types and order of attributes) with information that lives solely on the attribute arrays (flag state, strides, etc). This is a frequent source of complexity as it involves navigating both data structures simultaneously to fully understand the array data.
• Allows for turning the existing AttributeSet constructors into delegating constructors and adds a new constructor that contains all the logic for initializing the new AttributeSet so this lives in one place.
• Provides a way to steal attribute array data from an existing attribute set during construction, as an intentional design decision means that there are no methods for resizing attribute arrays once created.
• Offers a clean mechanism for merging many AttributeSets into one - you create AttributeSet::Info classes out of each and merge those (as they contain all required information to do so), then create a new AttributesSet from the final AttributeSet::Info. @Idclip note - this method is purely a placeholder right now, it needs #631 to be merged to share this portion.

(@richhones @kubaroth)

[kaarrot]

Hey Dan, I'm taking another look at the list of members of the AttributeSet::Array struct and was wondering if adding following would require much work?

• type
• isUniform
• codec
• shared EDIT: Perhaps I'm missing that the above ones can be accessed with a descriptor?

[danrbailey]

Hey Dan, I'm taking another look at the list of members of the AttributeSet::Array struct and was wondering if adding following would require much work?

• type
• isUniform
• codec
• shared EDIT: Perhaps I'm missing that the above ones can be accessed with a descriptor?

The type and codec values can be retrieved from the descriptor - const NamePair& Descriptor::type(size_t pos) const. Think Nick already commented to that effect, but the first value is the value type, the second is the codec.

Whether the array is shared or not, you can retrieve from the attribute set - AttributeSet::isShared(size_t pos). However, for the print binary, I'm not sure it's necessary to include this because it'll never be true - there's no concept of instancing when the grids are deserialized so an attribute array couldn't be shared across multiple leaf nodes or grids.

That leaves whether an array is uniform or not. I hesitated a little on whether to include this, but decided not to. The primary purpose of this new data structure is for attribute set construction and for this purpose it's redundant. Arrays should always be constructed as uniform and then expanded when first populated (either implicitly through attempting to set values or explicitly). Providing a new mechanism for producing a non-uniform array would be unnecessarily confusing I think. I don't think this feature is out of place in the print binary so that would mean having to iterate over all the arrays, apologies I didn't think of this when I first proposed that this class would alleviate this.

[kaarrot]

Thanks for clarification.

[danrbailey]

I don't believe there to be any more feedback needing addressed for this PR. @Idclip - if you're happy can you approve please?

[danrbailey]

I don't believe there to be any more feedback needing addressed for this PR. @Idclip - if you're happy can you approve please?

@Idclip - can you have a last look over my changes please? I believe I've addressed your feedback now.

[richhones]

I finally got round to looking at this following it being raised at the last TSC meeting. I think that similar to @kubaroth 's comments asking for the type and codec and @Idclip 's comments about storing the Descriptor::Ptr that I think this maybe doesn't go far enough of being properly convenient for the general case. It could go further to simplify the AttributeSet/Descriptor stuff - my suggestion would be to remove the Descriptor from the Info class and store the type and codec information in the Array struct. Alternatively if we don't want to do that I would just put this internal to the merging code. This API is already pretty confusing and think that the inclusion of the Descriptor in this convenience class doesn't mean this class helps very much. If you still have to know to query the Descriptor for types and codecs is there a benefit to using the Info class over just using the AttributeSet itself.

I imagine in merging you want to merge Infos and so Descriptors need to be merged in the current setup so removing them from this struct would simplify that process completely no?