Modernizing/generalizing SetProperty operation

@Bob_Obara @tj.corona @johnt @Haocheng_Liu

The discussion started on SMTK MR 1592 has once again brought up the property system used on model and mesh. Is it something worth moving to the smtk::resource::Resource (including smtk::attribute::Resource) so that free-form properties can be set system-wide? It would simplify the SetProperty operation and could provide applications with a fair amount of flexibility for filtering components that might be complementary to attribute categories.

If we can encapsulate it to a single class (accessible via something like resource->Properties() and not have a ton of API in smtk::resource::Resource itself) I think its a great idea. We would also have to write JSON bindings for it. Also, since the code is in a pretty foundational section of SMTK, I would suggest that one of SMTK’s full-time developers (@Bob_Obara, @dcthomp, @Haocheng_Liu, @johnt or @tj.corona) implement it.

@tj.corona smtk::resource::Properties sounds like a fine idea. I could take a pass at implementing it if there is enough buy-in; it really shouldn’t be much effort.

Just a thought: if you use a template-heavy API, we would not need to differentiate between int, double and string. We also wouldn’t be limited to storing just those three things; we could assign/retrieve anything that is serializable.

@tj.corona Are you thinking of something like this?

// A concrete type we can use to own references:
class propmapbase { };

// The actual storage for any serializable type V
template<typename V> class propmap
  : public std::map<UUID, map<std::string, V>>
  , public propmapbase
{ };

// The thing that owns all the property maps.
// We could also make it a map from strings to
// propmapbase pointers and use the string to
// encode the function the map served.
class properties
  : public std::set<std::shared_ptr<propmapbase>>
{
  public:
    template<typename V> propmap<V>& get();
    template<typename V> const propmap<V>& get() const;
    template<typename V> clear();
};

My only concern about this design is that I can imagine myself abusing it with all kinds of std containers as time elapsing…(Especially when we can use c++17!) Should we use some TMP technique to predefine some allowable type ?
Ex

template<typename... Types>
struct List
{};

template<typename... ArgTypes>
struct ListTagBase
{
    using list = List<ArgTypes...>;
};

struct CommonTypes: ListTagBase<int, double, float>
{
};

I think since the goal of the property system is to allow free form storage, we should not be restrictive by whitelisting what may be stored or not.

  1. What abuses are you worried about?
  2. I can imagine wanting some properties to avoid serialization. That functionality could/should be provided by propmapbase:
    class propmapbase
    {
      public:
        propmapbase(bool serializable = true);
        bool serializable() const { return m_serializable; }
    
      protected:
        bool m_serializable;
    };
    
  3. We could expand the property system to include other things in the model system (Entity records, tessellation, etc.) if we wanted to. Would you consider that abuse? I think it would be nifty and could potentially eliminate a lot of code.

I take it back. After a second thought, I think you are right as the free form storage opens a new world to SMTK. See my comments below.
Edit: Still I prefer something at compile time(So we have a control of what users can use and developers are free to expand it in their own session).

+1. For instance, now I store rgg materials as string properties - Using string "materials’’ I can fetch a list of material names and using each material name then can I fetch its representation as a string property. Alternatively, it will be nice to store materials as its own native type(Ex Material class) and it also applies to rgg pins, ducts, assemblies. It saves the cost of serialization/deserialization of strings. Instead of being limited by string property, I can now use Pin/Duct/Assembly property directly.

Exactly this:

I don’t think we should use this system to store tessellation. It would be better to have something that is enforced by the compiler, rather than convention, for a concept that is ubiquitous across resources (there’s a “magic string” smell here).

If we were to restrict ourselves to things that were serializable to (and, thus, stored as) a string, it would result in a concise API and prevent abuse of this storage mechanism (for example, the RGG session currently avoids all of model’s infrastructure using property strings; this works for RGG where there is no clear mapping to BREPs, but it is difficult to enforce as a pattern).

I think storing properties as strings informs consumers of their purpose: while you could store whatever you wanted by providing a serialization routine, you should use properties for storing properties, not things. Additionally, if serialization were implicit, then persistency would be automatic (a nice bonus).

I was more thinking of templating the accessors to avoid findString(), findDouble(), findInt(). It’s a cool idea to template the entire storage of a property, but it could definitely be abused.

I really like the concept of a compile-time description of things that can be associated with a resource (hooray compile-time checks!), but I can see the utility of a runtime-defined property system as well. Also, as I mentioned above, I think a compile-time list of properties enforces a pattern that is going to be a bear to rein in once it is used to do things we didn’t originally intend.

FWIW, I am not sure there is any demand for free form storage.

  • As a reference point, I’ll bet that Simmetrix’ GeomSim hasn’t provided persistent properties of any type in its nearly 20 years of existence, so we might be solving a problem that doesn’t exist in the market.
  • I don’t think we should allow, for example, storing one resource as a property on another resource.
  • I have a bigger use case for ephemeral properties, for example, assigning material numbers to model entities at the beginning of export operations. But there are alternatives that make this a low priority requirement.

Having said all that, I can live with free-form properties as long as we add some syntactic sugar so people like me can easily use string keys and perhaps our current types – int, double, string – plus maybe json.

The horror! The horror!

I think it would be pretty easy to get ephemeral properties for free if they can also be persistent. If we are going to write .sbt files describing filtering routines that filter on properties, you will likely want properties to become persistent!

I completely agree in in terms of not making this too general purpose. If you want to model complex data with constraints thats what the attribute resource was designed for. This should be a simple POD-like class that maybe uses a dictionary to store its contents. It should be persistent though we may want to be able to “tag” some information as non-persistent so it wouldn’t be saved.

I have a branch with an initial sketch of what the property class would look like here:

https://gitlab.kitware.com/dcthomp/smtk/commits/resource-property

There are a few things that would be needed to start using it:

  • Switch the existing json I/O to use it. Note that currently smtk::model properties are all lumped into a single map from UUIDs to all properties (of all types) which does not exactly mirror this structure in memory (but which is probably more efficient space-wise since long UUID strings are not repeated).
  • Switch the smtk::model and smtk::mesh APIs to use this for double/int/string properties.

Neither is a huge amount of work.

This is an awesome start! I have a couple of nits (for example, I don’t see where/how a property index is used), but I think this is great.

The implementation looks really beautiful! Some tiny nits from me: raw pointer and using of new:slight_smile: .