@Sergey Sharybin (sergey) I'm just uploading it here so that maybe you can take a quick look to see if something seems iffy to you. Otherwise I'll just continue tomorrow and figure out why it's failing.
The base pointer is no longer used to determine whether there is an OBJECT_BASE_FLAGS node; the has_node() function is now used to do this. This allows the code to create the relations properly the first time the object is seen, rather than waiting until it is (maybe) seen with a non-nullptr base pointer.
An alternative would be to always create the Entry → Exit relation, and create the Entry → OBJECT_BASE_FLAGS → Exit relations only when necessary. This would result in redundant relations, though, which the current approach avoids.
This is a bit confusing terminology. In the nodes builder the build_object_from_layer()is used from the loop which iterates bases.
Here you can read this as "build relations inside of object's FROM_LAYER component", but then the lac of "symmetry" with nodes builder requires extra brain cells.
Maybe build_object_from_layer_relations would solve the mystery? What do you think?
FROM_LAYER -> SYNCHRONIZATION seems to only happen when the object is build from base, and before this code was always run. Is this an intentional change?
|297–299 ↗||(On Diff #26286)|
Two things here.
First, I think we should stick to COMPONENT level communication as much as possible, leave picking up exact operation to the graph. Kind of hides specifics of topology from other areas. Additionally, this allows depsgraph to automatically pick exit operation for source of relation, and will pick up entry operation for destination of relation.
Second, is maybe this is the reason why object is invisible? If there is a driver on hide_viewport this code wires relation to the EXIT, which is too late since then FROM_LAYER function will not have driver evaluated yet.
Think we should tag OBJECT_BASE_FLAGS here, as this is what is being modified.
The fact that there is ENTRY/EXIT is kind of details. The way how update flush works is that the entire component is tagged anyway. Sticking to OBJECT_BASE_FLAGS allows us:
To elaborate a bit on the latter point. Having all those explicit ENTRY/EXIT and some other "utility" operations allows to simplify graph building process since you don't need to worry too much about "hey, if there is no FOO used in BAR, i should use node BAZ, otherwise i need to use node DOOMOP". However, these nodes are a burden for evaluation. So one might imagine that it's totally possible to write graph minimizer which will rewire some relations and will eliminate the no-op nodes. Exact algorithm is yet to be formalized, but I feel it's totally doable and is something we should implement eventually.
Same as above.
Even though it makes sense given the FROM_LAYER component, my brain keeps reading it as build_X_from_Y, and we're not building an object from a layer.
I agree that naming the function build_object_from_layer_relations is an improvement, so I'll do that and leave the overall naming for a future improvement.
This was an intentional change.
The code didn't always run. It was only run when base != nullptr. In other words, this happened:
So, in the old code the Base * is used to determine whether that FROM_LAYER → SYNCHRONIZATION needs to be added. Because of this, it needs to be called every time build_object() is called.
My new approach uses has_node(object_flags_key) to determine whether that relation needs to be added or not. This means that it can do the right thing right away, without having to rely on future calls that may or may not pass a Base *.
|297–299 ↗||(On Diff #26286)|
This was the cause of Spring loosing her hair. I didn't take into account the source parameter, so it would indeed always relate to the EXIT node. By sticking to component level communication, this is picked up properly automatically.
I was in doubt what to do here, so thanks for the clarification.
And yes, it would be fun to think of an algorithm to reduce the no-op nodes further.