"id" attribute handling in meshes on merge by distance

If you set an id on vertices in GN with a Set ID node, and then use a Merge by Distance node (and maybe other meshes which cause attributes to be combined) the “id” attribute is handled like a default integer attribute. I.E. it is averaged.

For an “id” attribute this is undesirable imho. It would be better to do what the merge by distance point clouds code path does (pick the id of the first one).

I decided to make this a devtalk post instead of a bugreport as it not technically a bug, and changing it is not backwards compatible with meshes that happen to an attribute named “id”. Should I look into making the mesh code behave like the pointcloud code? I think I know where in the code this is handled…

I think it would make sense for meshes to handle IDs like the point cloud code. Since the attribute is a relatively recent addition and since averaging it isn’t very useful, I wouldn’t be concerned about backwards compatibility in this case.

For an implementation, it would probably be best to find a solution that didn’t depend on looking up the attribute with a string for every merged vertex. It would also be good to avoid averaging the ID attribute with the generic custom data mixing only to set it to the first value later.

I’ll have a look if I can find out where the averaging is handled and if I can handle id different on a layer level.

It’s probably out of the question but I want to drop it here anyway:

In a way it would be better if the id attribute would be it’s own type instead of just PROP_INT. that way you could give it it’s own LayerTypeInfo, with it’s own layer functions.

That would I think lead to cleaner code than checking for the string “id” in various places and making exceptions.

Edit
After some more looking at the code there are 3 possibilities:

  • Give ID layers their own type.
  • Check the name of the id layer on the interpolation of each value (not good, as well as ugly)
  • Lookup which PROP_INT layer is the “id” layer (if any) and pass it’s index to CustomData_interp to make an exception in the interpolation code (ugly)

IMO the middle one is definitely a bad plan. I don’t really like the last one either. I have no idea what the implications of adding a new type are.

I think it would be nice to make the attribute types a bit more advanced: They should have a ‘storage type’ (INT in this case) and a ‘semantic type’. ‘ID’. Where the semantic type (if set) could override certain functions of the base storage type. this could also be handy maybe for UV layers, To give them a storage type of FLOAT2 and a semantic type of UV.

Currently the design is that different behavior during intepolation is based on attribute names, and nothing else. I don’t think that adding a different storage type for different interpolation behavior is a good idea; that’s what we’re tying to move away from with the generic attribute system. Using separate types for everything just doesn’t scale well.

This code should be written to use a SoA paradigm anyway. That is, it should use a for each attribute: for each element style loop rather than for each element: for each attribute. That makes switching the interpolation behavior much easier, and makes it more generic.

However, I also like the idea of adding user controlled “interpolation types” to attributes, which is a bit like your “semantic type” idea. The alternative is adding options to every node about how to interpolate every one of the attributes, which is less reusable and much more clunky. Some use cases are:

  • Using this interpolation method for integer attributes
  • Marking 2D vectors as UVs (or even changing between different UV interpolation methods (see the subsurf modifier))
  • Marking vectors that should be transformed with the positions, or even marking vectors that should be transformed as normals.
  • Maybe using the same interpolation method we use for original indices (CD_ORIGINDEX).
2 Likes

this is what I have now. For whoever is interested.

https://developer.blender.org/D16248

It ads a simple way to override the interpolation function (or any LayerTypeInfo callback, for that matter) on a per-layer bases.

edit:

To add such a general interpolation override and reverse the nesting order of the loops is rather a bigger project. For which I currently don’t have time. So for now I abandon this project.