Thanks for the patch. As mentioned before, I think this patch is trying to do too much at once. Moving more stuff into files for specific nodes should be done separately from refactoring how nodes are defined in general.

For me the most interesting aspect of this patch is the ability to define the rna of nodes outside of makesrna. When you say "but can be augmented with conventional dna structs in node->storage if necessary", do you mean that this would then have to be done in makesrna again, or can it also be done in the node file?
I think it's reasonable to allow using id properties here, even though it would be nicer if we could still benefit from the compactness or dna structs (compared to id properties). However, that might be much harder to do, not sure.

I don't particularly like the dependence on the macros (like DECL_NODE_FUNC_OPTIONAL), they make it a bit annoying to see what is actually going on. Besides that I wonder if a node definition should be an instantiation of GeometryNodeDefinition, instead of having the class itself be the node definition (and therefore requiring all methods to be static).

In D12292#318921, @Jacques Lucke (JacquesLucke) wrote:

Moving more stuff into files for specific nodes should be done separately from refactoring how nodes are defined in general.

There is a C function node_make_runtime_type_ex that does the basic node type setup. The C++ class feature is built on top of that, but i can remove it for the time being. It's a bit more experimental, eventually i hope we could use actual classes to define types, like you would do in python. The RNA cpp codegen isn't quite good enough for that yet.

I don't particularly like the dependence on the macros (like DECL_NODE_FUNC_OPTIONAL), they make it a bit annoying to see what is actually going on. Besides that I wonder if a node definition should be an instantiation of GeometryNodeDefinition, instead of having the class itself be the node definition (and therefore requiring all methods to be static).

Yeah, the macros are part of the experimental C++ code, so not strictly necessary. Macros are needed in C++11 to test for existence of class members (see inline comments for details). This can't be fully templated and has to have a separate function for each member. In C++20 there are nicer features for that ("concepts"). Or we could just add defaults in the base class for everything expected, so a "T::<fieldname>" expression will never fail to compile.

Using true member functions instead of static functions won't be as easy, because an instance of the NodeDefinition classes isn't actually related to the bNode directly. The way that might work is along the lines of the C++ RNA generated classes (Cycles uses these for importing data). The "this" argument is a wrapper around the PointerRNA. I'm not sure how registered callbacks or property definitions could work there. In python classes property declarations are done with annotations now, but C++ lacks the introspection features for that. Would probably require some macros for decorating class members and register appropriate ID props, but that's a bigger project.

For me the most interesting aspect of this patch is the ability to define the rna of nodes outside of makesrna. When you say "but can be augmented with conventional dna structs in node->storage if necessary", do you mean that this would then have to be done in makesrna again, or can it also be done in the node file?
I think it's reasonable to allow using id properties here, even though it would be nicer if we could still benefit from the compactness or dna structs (compared to id properties). However, that might be much harder to do, not sure.

Yes, any structs that are not supported by ID properties have to be defined in the compile-time makesrna. And that only works by putting DNA/RNA into the respective modules again, undoing the advantage of localized node definitions. For nodes this is very rarely needed, e.g. the CurveMapping or ColorRamp nodes are rare exceptions. Those are ancient bits of data and they are not very complicated, so might even try and make an alternative with just ID props. In any case, all the old callbacks for storage alloc/copy/free are still there, just not needed for most nodes with simple properties.

Or we could just add defaults in the base class for everything expected, so a "T::<fieldname>" expression will never fail to compile.

Becomes a bit more tricky though: nullptr callbacks can be used internally to follow a different code path (if (ntype->somecallback) { ntype->somecallback(); } else { do_stuff; }), so having a dummy implementation for callbacks can break expected behavior. That's why having a thing that returns a function pointer or nullptr if undefined is so useful.

I've stripped down the code to bare minimum now, just with the base type setup function that creates a runtime RNA struct definition, which can then be augmented with ID properties.

The ellipse node isn't particularly useful, so i would want to remove it first. Any idea for adding an example somewhere? Should be convert one of the existing nodes to a runtime definition? Or just leave that out entirely and use runtime setup for a future node?

Thanks, that looks like a much more manageable patch already.

Instead of adding a new node you could e.g. update the Mesh Line node.

This is also a bit related to T75724: Extensible Architecture Refactoring. I wonder what @Brecht Van Lommel (brecht) thinks about using id properties in built-in nodes and whether he has thought about how RNA could be registered at runtime in the future.

Rather than ID properties, I think all node properties should actually be sockets. There can be restrictions on such sockets in that they may not be linkable or must be statically evaluated. But I think the aim should be to make every property on a node to be exposable in node groups or drivable through other nodes.

In general, I'd argue against replacing DNA by ID properties due them using a lot of memory, and if we want to optimize Blender to handle bigger scenes that is not a step in the right direction. Sockets already are quite big so compared to that it's not really an argument. But node graphs are not usually what is memory intensive in files, and hopefully some day we can move to a more compact node socket struct also.

Runtime RNA registration would be good but is complicated, not sure we want to go into that here.