Page MenuHome

Depsgraph: Implement 'ID_RECALC_GEOMETRY_DEFORM'
ClosedPublic

Authored by Germano Cavalcante (mano-wii) on Jun 14 2021, 4:18 PM.
Tags
None
Tokens
"Y So Serious" token, awarded by shader."Yellow Medal" token, awarded by franMarz."Like" token, awarded by campbellbarton."Love" token, awarded by silex."Love" token, awarded by gilberto_rodrigues."Love" token, awarded by Schiette."Mountain of Wealth" token, awarded by ace_dragon."Mountain of Wealth" token, awarded by jbakker."Like" token, awarded by xorrito."Yellow Medal" token, awarded by chironamo.

Details

Summary

During a mesh transformation in edit mode (Move, Rotate...), not the entire draw cache needs to be recreated.

But this occurs because DEG_id_tag_update(tc->obedit->data, ID_RECALC_GEOMETRY), chain a call to BKE_object_data_batch_cache_dirty_tag that tags all the batch cache to be redone

This patch proposes not tag dirty All, but only what participates in the deformation of geometry.

Depsgraph Changes:
Currently, the graph can be compared to this simplified one:


Where DEG_id_tag_update(id, ID_RECALC_GEOMETRY) triggers geom_eval_mesh or geom_eval_obj_init.
And DEG_id_tag_update(id, ID_RECALC_SELECT) triggers update_select or update_select_obj.

This patch proposes to separate "tag_dirty" from "geom_eval" and create a separate node for it (update_all in the image bellow).
In addition to adding an node to update_deform
The graph becomes something like this:

Benchmarking:

master:patch:
large_mesh_editing:Average: 16.727632 FPSAverage: 26.424897 FPS
rdata 9ms iter 26ms (frame 60ms)rdata 0ms iter 19ms (frame 38ms)
large_mesh_editing_ledge:Average: 17.761902 FPSAverage: 28.070558 FPS
rdata 9ms iter 24ms (frame 56ms)rdata 0ms iter 18ms (frame 36ms)
looptris_test:Average: 5.537827 FPSAverage: 5.456050 FPS
rdata 11ms iter 26ms (frame 169ms)rdata 11ms iter 28ms (frame 172ms)
subdiv_mesh_cage_and_final:Average: 2.095824 FPSAverage: 2.140402 FPS
rdata 7ms iter 21ms (frame 242ms)rdata 0ms iter 20ms (frame 237ms)
rdata 7ms iter 22ms (frame 233ms)rdata 0ms iter 21ms (frame 227ms)
subdiv_mesh_final_only:Average: 6.626541 FPSAverage: 7.974115 FPS
rdata 3ms iter 13ms (frame 145ms)rdata 0ms iter 10ms (frame 122ms)
subdiv_mesh_final_only_ledge:Average: 6.590914 FPSAverage: 7.978224 FPS
rdata 3ms iter 13ms (frame 143ms)rdata 0ms iter 10ms (frame 121ms)

Diff Detail

Repository
rB Blender
Branch
arcpatch-D11599 (branched from master)
Build Status
Buildable 15355
Build 15355: arc lint + arc unit

Event Timeline

Germano Cavalcante (mano-wii) requested review of this revision.Jun 14 2021, 4:18 PM
Germano Cavalcante (mano-wii) created this revision.

Some might say that the graph is too complex and difficult to maintain.

Some complexity is expected for best performance.

The way ID_RECALC_GEOMETRY_DEFORM skips CoW doesn't seem to be conventional.

Yeah, unfortunately, it is not. We can add non-cow-dependent geometry component, with the operation responsible for ID_RECALC_GEOMETRY_DEFORM in there, and make current geoemtry exit operation dependent on that one. Think it could work!

source/blender/draw/intern/draw_cache_impl_mesh.c
760

Is it something to be resolved before commit?

source/blender/makesdna/DNA_ID.h
612–614

Add explanation what is it for and things like that.

Germano Cavalcante (mano-wii) marked 2 inline comments as done.
  • Add code comments

We can add non-cow-dependent geometry component, with the operation responsible for ID_RECALC_GEOMETRY_DEFORM in there, and make current geoemtry exit operation dependent on that one.

I thought about it, I didn't implement it because I was in doubt in case it can be used for objects outside of edit mode and with CoW.
(But in this case as the mesh eval is freed together with batch cache anyway).
I'm also not sure how geometry-dependent objects behave in this case.

I will study this idea.

source/blender/draw/intern/draw_cache_impl_mesh.c
760
Germano Cavalcante (mano-wii) planned changes to this revision.Jun 14 2021, 6:53 PM

I found a small error in the graph

  • Isolate the operation responsible for ID_RECALC_GEOMETRY_DEFORM in its own component

Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)

Maybe it's just me, but I find it a bit confusing that there are nodes that are called "tag xxx". In the scope of the depsgraph, it's the tagging that causes the evaluation, but with these nodes it's the evaluation that causes the tagging. I know that the latter tagging is for the GPU batches and not for the depsgraph, but that's far from obvious when your brain is engaged in "understanding the depsgraph" mode. Maybe it's better to name them after what the effect should be? Like "update gpu batch" or something like that? The fact that that happens via tagging could be seen as an implementation detail.

I'm also in doubt about NodeType::GEOMETRY_NO_COW. For starters, there is no NodeType::GEOMETRY_COW. Furthermore, I'm more in favour of positive naming that express what something is for, rather than what it's not for. Wouldn't NodeType::GEOMETRY_EDIT_MODE be more appropriate?

source/blender/blenkernel/intern/object_update.c
352–382

I think this change should be separated from the rest of the patch, as it's just a non-functional change.

source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
1494

"prior actual" → "prior to actual"

source/blender/depsgraph/intern/node/deg_node_operation.h
103

Typo: DRITYDIRTY

Sybren A. Stüvel (sybren) requested changes to this revision.Jun 18 2021, 2:52 PM
This revision now requires changes to proceed.Jun 18 2021, 2:52 PM
Germano Cavalcante (mano-wii) planned changes to this revision.Jun 18 2021, 3:36 PM

I think it's best to wait for a decision on the D11337: Depsgraph: remove redundant mesh data duplication in edit-mode first.
With that patch, it will no longer be necessary to create the new component (GEOMETRY_NO_COW).

Germano Cavalcante (mano-wii) marked 3 inline comments as done.
  • Rename GEOMETRY_NO_COW --> GEOMETRY_SHARED
  • Fix typos
  • Rename _TAG_DIRTY_ --> _UPDATE_

On second thought, D11337 doesn't directly affect this patch, so I believe that, if accepted, it can land early.
If something later changes in the behavior of the GEOMETRY component, the removal of the GEOMETRY_SHARED component may be considered.

  • Cleanup: Remove redundant assignments of open_node
  • Resolve TODO in BKE_MESH_BATCH_DIRTY_DEFORM
  • Rebase on master
  • Remove GEOMETRY_SHARED component
  • Rename 'GEOMETRY_EVAL_UPDATE_DEFORM' --> 'GEOMETRY_EVAL_DEFORM'

Benchmarking:

master:patch:
large_mesh_editing:Average: 16.727632 FPSAverage: 26.424897 FPS
rdata 9ms iter 26ms (frame 60ms)rdata 0ms iter 19ms (frame 38ms)
large_mesh_editing_ledge:Average: 17.761902 FPSAverage: 28.070558 FPS
rdata 9ms iter 24ms (frame 56ms)rdata 0ms iter 18ms (frame 36ms)
looptris_test:Average: 5.537827 FPSAverage: 5.456050 FPS
rdata 11ms iter 26ms (frame 169ms)rdata 11ms iter 28ms (frame 172ms)
subdiv_mesh_cage_and_final:Average: 2.095824 FPSAverage: 2.140402 FPS
rdata 7ms iter 21ms (frame 242ms)rdata 0ms iter 20ms (frame 237ms)
rdata 7ms iter 22ms (frame 233ms)rdata 0ms iter 21ms (frame 227ms)
subdiv_mesh_final_only:Average: 6.626541 FPSAverage: 7.974115 FPS
rdata 3ms iter 13ms (frame 145ms)rdata 0ms iter 10ms (frame 122ms)
subdiv_mesh_final_only_ledge:Average: 6.590914 FPSAverage: 7.978224 FPS
rdata 3ms iter 13ms (frame 143ms)rdata 0ms iter 10ms (frame 121ms)

As we can see, there is a considerable improvement in the performance of files that work with mesh editing. (large_mesh_editing.blend, large_mesh_editing_ledge.blend)
But for some reason, there is a regression in the looptris_test.blend (which theoretically shouldn't be affected).
(Is it because tessellation is multitreaded and with this patch depsgraph dedicates a thread to the batch dirty tag?)
I'm not sure it's worth investigating as the penalty isn't considerable.

  • Remove leftover from GEOMETRY_SHARED
  • Rebase on master

I checked the tests again.

Profiling: (Best and worst result)

master:PATCH:
large_mesh_editing:Average: 17.137613 FPSAverage: 25.408552 FPS
rdata 8ms iter 26ms (frame 58ms)rdata 0ms iter 21ms (frame 39ms)
looptris_test:Average: 5.577005 FPSAverage: 5.500189 FPS
rdata 11ms iter 27ms (frame 168ms)rdata 11ms iter 27ms (frame 171ms)

For experimentation, and in order to solve the worst case, I tested a new graph where the "tag_dirty" operator is called in the nodes "geom_eval_mesh_init" and "geom_eval_obj_init":

The result is better but, to work, the component "GEOMETRY" has to be added to the list of "special component where we do not want all operations to be tagged."
Although it does not seem problematic, I prefer to avoid even greater changes.

master:Test Graph:
large_mesh_editing:Average: 17.137613 FPSAverage: 26.049423 FPS
rdata 8ms iter 26ms (frame 58ms)rdata 0ms iter 20ms (frame 38ms)
looptris_test:Average: 5.577005 FPSAverage: 5.524559 FPS
rdata 11ms iter 27ms (frame 168ms)rdata 12ms iter 27ms (frame 169ms)

I don't quite understand the graphs. What do the "start" and "end" nodes represent? Do they mean the same thing in different graphs? I don't know much about geometry batches, so my brain has to work overtime to just follow what's being said. Having to mentally reshuffle the graphs to compare them, as each graph puts things in a different place, makes this process even harder.

source/blender/blenkernel/intern/object_update.c
400

What is this switch for? Is it here because it needs expanding for other datablock types in the future? Or will only mesh datablocks be ever supported by this function?

source/blender/depsgraph/intern/node/deg_node_operation.h
104

updatesupdate

106

doesdo

106

The comment above talked about "deform batches", this comment talks about just "batches". Which "batches" are referred to here?

Germano Cavalcante (mano-wii) marked 3 inline comments as done.
  • Add fallback for BKE_object_data_eval_batch_cache_deform_tag
  • Improve comments

What do the "start" and "end" nodes represent? Do they mean the same thing in different graphs?

The graph basically represents the Depsgraph in a simplified way. I used them to study solutions. In fact it needed more clarification for review.
The "Start" points to the nodes that start the chain of operations. For example, it points to geom_eval_mesh_deform that is equivalent to GEOMETRY_EVAL_DEFORM() in the Depsgraph. So when DEG_id_tag_update is called with the parameter ID_RECALC_GEOMETRY_DEFORM, the node equivalent to geom_eval_mesh_deform is tagged and every chain of nodes it points to is executed.
The "End" indicates that there are nodes in front but it does not need to be represented.

source/blender/blenkernel/intern/object_update.c
400

It may be supported for other datablock types in the future, but is currently only implemented for meshes.
Added a fallback for the other datablocks.

Here's a little context of what currently exists (without the patch).


The cut of the graph shown below shows how the depsgraph behaves for updating the GEOMETRY component.
Note that there is no node to "update" (tag dirty) the batch cache.
This is because the BKE_object_batch_cache_dirty_tag is called along with the GEOMETRY_EVAL() operation.
(The only batch cache related node is GEOMETRY_SELECT_UPDATE()).


However, for optimization, we need to do something more granular, and not update the entire batch cache when the geometry has just been deformed.
So we need to remove the BKE_object_batch_cache_dirty_tag from GEOMETRY EVAL() and add another node that only updates part of the batch cache.
So, in the new graph, the component that already has GEOMETRY_SELECT_UPDATE() now has two new nodes: BATCH_UPDATE_DEFORM() and BATCH_UPDATE_ALL(). One node does not depend on the other and are never called together

  • BATCH_UPDATE_DEFORM() --> operation BKE_object_data_eval_batch_cache_deform_tag
  • BATCH_UPDATE_ALL() --> operation BKE_object_data_eval_batch_cache_dirty_tag

With the patch, if we call DEG_id_tag_update(mesh, ID_RECALC_GEOMETRY_DEFORM), the new node GEOMETRY_EVAL_DEFORM will be triggered and skip the BKE_object_batch_cache_dirty_tag (BATCH_UPDATE_ALL) operation.

Thanks for the clear explanation. +1 for more granularity.

LGTM. I didn't go through all the new depsgraph relations mentally, because I'm not that familiar with the geometry batch system anyway. I'll trust that the patch has been thoroughly tested.

This revision is now accepted and ready to land.Jul 12 2021, 5:22 PM