Naming of Wrapped DNA Types [Proposal]

Technical Feedback
1

We have a couple of DNA types that are wrapped with C++ types improving code ergonomics. The issue is that the correlation between the DNA and wrapper type name is not standardized and can sometimes be confusing. This document proposes a simple naming scheme to solve this.

This was also discussed a bit in a recent admins meeting but without a clear result yet, so any feedback is welcome.

Existing Examples

Here are a few examples of wrapped types that we used in Blender already. Generally, the DNA type has a wrap() method that casts it to the corresponding wrapper type.

DNA Name Wrapper Name
::CurvesGeometry blender::bke::CurvesGeometry
::GreasePencilLayer blender::bke::greasepencil::Layer
::ActionSlot blender::animrig::Slot

There are two main cases currently:

  1. The DNA and wrapper types have the same name but they are in different namespaces. In this case it’s more obvious that they are the same, but whenever the type name is used, it’s very context dependend what type is meant. For example, if CurvesGeometry is used in the blender::bke namespace, it refers to the wrapper type, but if it is used in blender::nodes it refers to the DNA type. This is especially confusing to people getting into a new area in Blender, because it’s a problem that usually does not exist.
  2. The DNA and wrapper type have very different names making the relation between them less obvious.

Proposal

The proposed naming scheme is fairly straight forward: Use the same name for both types except that the DNA type has a “DNA” suffix.

The suffix is added on the DNA types instead of the wrapper types, because the wrapper types are expected to be used way more often. The DNA types are generally only used for file reading and writing.

We’d use DNA_STRUCT_RENAME to make sure that this has no impact on file compatibility.

The examples above would be changed as follows.

DNA Name Wrapper Name
::CurvesGeometryDNA blender::bke::CurvesGeometry
::GreasePencilLayerDNA blender::bke::greasepencil::GreasePencilLayer
::ActionSlotDNA blender::animrig::ActionSlot

Discussion

This solves the two mentioned problems:

  1. The types never have exactly the same name anymore, so it’s never ambiguous which of these types we refer to. For example, CurvesGeometry always refers to the wrapper type whereas CurvesGeometryDNA always refers to the DNA type.
  2. The types have very similar names, making it obvious that they relate to each other.

The main downside is that the new wrapper names are a bit more verbose (Layer vs. GreasePencilLayer). Personally, I feel like this kind of verbosity is fine since it seems to result in more obvious local code.

Names like Layer and Slot are very common and while it’s nice to use such short names, they are also context dependend. By that I mean that you have to be aware of what context/namespace you are in. They are also not very searchable.

We could consider to add all of these wrapper types to the global or blender:: namespace to avoid the redundancy when refering to them from other namespaces. The DNA types are in global namespace already anyway. However, that can also be considered separately.

3

Personally I like the idea of having a DNA suffix to disambiguate names when the DNA and wrapped name would otherwise be the same. But I’m not really convinced that it’s necessary for cases like greasepencil::Layer (personally I’d like that to be written as gp::Layer if length is a concern). Otherwise we end up in a situation where we’re adding a “DNA” suffix to potentially very many DNA types just because that’s the rule.

I do wonder how this would relate to just moving every DNA type to a blender::dna namespace. We could start doing that with the types that have DNA wrappers. I think that would be a good option, especially as DNA is less of the default source for type definitions used at runtime and moves more toward just being a file format.

4

Is there a guideline for when to use a wrapper? To me it’s not clear why some types have a wrapper, and some have methods in the DNA headers. What are we moving towards?

If the idea is to move everything to a wrapper eventually, I think it would help to have a convention like:

  • Wrappers are in the blender:: namespace, DNA is in blender::dna:: namespace.
  • Wrappers are defined in BKE_foo_types.hh corresponding to DNA_foo_types.h

Then it would be possible to do an automated refactor for all DNA types.Generating the contents of BKE_foo_types.h and replacing DNA_ by BKE_ includes almost everywhere.

1 Like
5

There isn’t a clear guideline yet, but would be good to have that indeed.

I believe so far Hans and I mainly used wrapper types when we wanted to be able to use the types independent of DNA, thus they need to have proper copy/move constructors and destructors. On existing types, we generally just added methods to the existing types because that’s the most straight forward way to do it.

Overall I’m fine with moving the DNA types to blender::dna. Then the suffix might also not be necessary because basically all code would have to use dna::TypeName to refer to the type.

Do you mean directly into blender:: or in some sub-namespace like Hans suggested or are you not sure yet?

6

For the existing reasons about when wrappers are used, I’d second what Jacques said. We’ve been reluctant to add C++ object semantics (copy & move constructors, etc.) to existing DNA types.

Longer term I hope we don’t inherit from DNA types at all, and instead just use them as a serialization structs only created when reading and writing. That’s getting way out of scope for this task but I think it gives some context to the use of a dna namespace.

7

I’d also prefer to have a dna:: namespace, for the reasons mentioned above, but also because it would avoid renaming all of our DNA structs (this is possible of course, but IMHO we should keep DNA renames to the strict minimum, only when really necessary). DNA renames have a cost in complexity, data obfuscation (as in blendfiles, old orig names remain in use forever), etc.

If we want to generalize wrappers usage, we’ll also have to think about RNA. Right now it relies mostly on DNA data definition, any other type of data backend requires quite a lot of binding code that has to be written by hand. This is not really sustainable on a wider scale. And keeping RNA working at DNA level when everything else uses C++ wrappers is also probably not a good thing.

8

In Grease Pencil code Layer and Drawing are used so often, I can’t say I’m thrilled by the idea it will become GreasePencilLayer and GreasePencilDrawing everywhere. It’s not a bit verbose, but quite verbose. And for auto-complete, to name something practical, it won’t be fun.

GreasePencilDrawingReference is already about one third of a line of code.

And what to think about function declarations like:

void grease_pencil_layer_parent_clear(bke::greasepencil::GreasePencilLayer &layer)

Also a bit much, if you ask me.

One would almost say: let’s abbreviate it then to GPLayer and GPDrawing, which would be funny, because that’s more or less circling back to the names used in GPv2.

1 Like
9

I meant the blender:: namespace, because then it might be straightforward to do an automatic refactor to generate wrapper types for every existing DNA struct, and use it throughout the code. And only have the DNA structs as base classes and makesdna.

We’d also have to wrap the entire blender code in the blender:: namespace, but we should done that anyway.

When we add or change conventions like this, I’d prefer it we do refactors to make the entire code consistent with them if possible. Currently a type can be in:

  • ::
  • blender::
  • blender::foo
  • blender::bke
  • blender::bke::foo

And which one depends more on legacy reasons than logical organization.

10

I think Brecht pretty much summarised my own thoughts on this topic.

Not sure what are the expected outcomes from the discussion. To me it seems after we agree on the direction we’d need to spend time and uniformly implement it for all DNA types. We need to bring this area back in consistent state.