Page MenuHome

Fix T66667 Adding empty mesh cache modifier to a mesh degrades playback performance significantly even if modifier display disabled
Needs RevisionPublic

Authored by Sybren A. Stüvel (sybren) on Jul 11 2019, 3:12 PM.

Details

Summary

This fixes T66667: Adding empty mesh cache modifier to a mesh degrades playback performance

The root cause of the issue is that the modifier's dependsOnTime() function would return true when the filepath was empty, which would trigger a rebuild of the draw cache every time it was redrawn in the viewport.

The modifier's dependsOnTime() function is only called when rebuilding the dependency graph. This necessitates the call to rna_Modifier_dependency_update when the filepath property is updated; without it, animated data from the mesh cache won't run.

Diff Detail

Repository
rB Blender
Branch
wip-sybren-T66667 (branched from master)
Build Status
Buildable 4027
Build 4027: arc lint + arc unit

Event Timeline

source/blender/modifiers/intern/MOD_meshcache.c
66

I am not sure what is this about.

If property is animated it is up to the dependency graph to consider property as dependent on time.

Is this about check for file path needed to be done in dependsOnTime() instead of isDisabled() ? Still wouldn't explain why mcmd->factor is so special and how it will affect on the result though.

source/blender/modifiers/intern/MOD_meshcache.c
66

You're right, I probably was overly careful.

An alternative approach would be to change BKE_object_modifier_use_time() so that it also calls modifier_isEnabled() and only calls dependsOnTime() when the modifier is actually enabled.

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)
  • Moved isEnabled check into BKE_object_modifier_use_time()

BKE_object_modifier_use_time() now also checks whether the modifier
is enabled, and only then enabled considers its 'depends on time'
flag.

@Bastien Montagne (mont29) @Sergey Sharybin (sergey) now that master is open again, may I poke you for another round of review?

Sergey Sharybin (sergey) requested changes to this revision.Jul 30 2019, 10:55 AM

Changing modifiers and constraint enabled state does not tag relations for update. This means you can not check for enabled modifiers in BKE_object_modifier_use_time(): if modifier which is dependent on time is disabled in .blend file, it will not properly update when you enable it in the interface.

This issue goes deeper than this change and i am not sure it really worth solving a corner case when an empty mesh cache modifier is used (which almost never happens in real setups) n the cost of making it even slower to toggle modifiers enabled state.

More correct approach would be to get rid of dependsOnTime(), and rely on actual relations. For example, in lots of cases dependsOnTime() is to be replaced with a relation between texture and geometry component on the object. If the texture depends on time, it is to have relation between time and texture node in the dependency graph.
Caching modifiers can do similar thing (for example Time -> Cache -> Geometry), or have more explicit relation done by the dependnecy graph from time directly to the geometry.
That would make this time relations mapping to the dependency graph more naturally.

If this approach doesn't work, we can as well introduce the callback which will tell whether modifier is effective based on its settings, but not based on its enabled flags.

This revision now requires changes to proceed.Jul 30 2019, 10:55 AM