Page MenuHome

Fix T76251: Animated procedural texture does not influence particle system
ClosedPublic

Authored by Luc Revardel (lrevardel) on Apr 30 2020, 11:00 AM.

Details

Summary

Dependancy missing while building depsgraph for particle systems.
fix: adding a relation texture->particles when texture has animation data.

Note: I don't have commit access, please proceed with the commit. Thanks

Diff Detail

Repository
rB Blender

Event Timeline

Luc Revardel (lrevardel) requested review of this revision.Apr 30 2020, 11:00 AM
Luc Revardel (lrevardel) created this revision.
Luc Revardel (lrevardel) retitled this revision from fix T76251: Animated procedural texture does not influence particle system to Fix T76251: Animated procedural texture does not influence particle system.Apr 30 2020, 11:30 AM

This seems a bit weird.

Mind set should be: the particle system depends on the texture. So relations builder shouldn't go deeper into texture to build relations for particles.

There seems already be relations from texture's ANIMATION to texture's GENERIC_DATABLOCK. This yields the following relations chain: Texture ANIMATION -> Texture GENERIC_DATABLOCK -> particle_settings_reset_key.

If i'm reading this patch correct, then what it does is that if texture is animated then there will be an explicit relation Texture GENERIC_DATABLOCK -> particle_settings_eval_key. Which seems to be redundant since there is already a relation from particle_settings_reset_key to particle_settings_eval_key.

source/blender/depsgraph/intern/builder/deg_builder_relations.cc
1883

Unused variable?

So relations builder shouldn't go deeper into texture to build relations for particles.

By this I assume you mean the builder should not inspect the texture at all, i.e should not call "check_id_has_anim_component(&mtex->tex->id)"

Note that removing the flag RELATION_FLAG_FLUSH_USER_EDIT_ONLY from:

add_relation(texture_key,
            particle_settings_reset_key,
            "Particle Texture",
            RELATION_FLAG_FLUSH_USER_EDIT_ONLY);

fixes the problem but has slow performances.
This flag seems to filter some dependency updates that we would expect to be propagated for this issue. That's why I thought another kind of dependency triggering particle_settings_eval should be added.

I'll check your comment in details, thanks for the time spent to explain.

Luc Revardel (lrevardel) updated this revision to Diff 24230.EditedApr 30 2020, 1:28 PM
  • Fix T76251: Animated procedural texture does not influence particle system

fixing Sergey's comment regarding unused variable.

Here's my understanding of the issue:
As mentioned by Sergey,there's already the relation below:
Texture_ANIMATION -> Texture_GENERIC_DATABLOCK -> particle_settings_reset_key

This chain should trigger in the case of T76251 issue, however it does not due to RELATION_FLAG_FLUSH_USER_EDIT_ONLY flag used to setup Texture_GENERIC_DATABLOCK -> particle_settings_reset_key.

So we have 2 options:
a/ remove RELATION_FLAG_FLUSH_USER_EDIT_ONLY usage: tested, works fine but is quite slow when playing animation
b/ add a new relation Texture_ANIMATION->particle_settings_eval_key to bypass particle_settings_reset_key - this is the patch attached (updated after Sergey comments on unused variable)

fps comparison using file & PC setup from T76251:
a: 1.4 fps
b: 4.8 fps

Ah, yes, the reset is only to happen on user edits, while eval should happen on regular texture update as well. Intuitively this should work, but particles code is hard to predict.

Does this P1358 fix the issue?

By this I assume you mean the builder should not inspect the texture at all, i.e should not call "check_id_has_anim_component(&mtex->tex->id)"

Particle builder should not check this. Texture builder is allowed to check the texture its building :)

Luc Revardel (lrevardel) marked an inline comment as done.EditedApr 30 2020, 2:55 PM

Does this P1358 fix the issue?

Yes, P1358 works pefectly, same fps as what I posted.

I thought we cannot add some kind of "parralel" path between texture and particle settings. Still a lot to understand there !

I hope the depsGraph documentation that you're currently working on (as I understand based on devs weekly report) will put some light upon all this ;-)

BTW, let me know your preferred approach to commit this, since I don't have commit access, you might want to commit the change directly.

  • Fix T76251: Animated procedural texture does not influence particle system

Updating diffs with Sergey's P1358 proposal.

This revision is now accepted and ready to land.Apr 30 2020, 3:33 PM

Seems safe enough. If it was a dependency between e.g. an object and a particle system it would be more tricky, but between a texture and a particle system is relatively safe I guess.