Paths

Table of Contentst

Fix T82242: creating particle influence textures does not set up DEG relation immediately
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Oct 30 2020, 12:14 PM.

Details

Reviewers

Sergey Sharybin (sergey)
Campbell Barton (campbellbarton)
Brecht Van Lommel (brecht)

Maniphest Tasks

T82242: Particle System Influence procedural textures [e.g. on hair length] dont update in realtime immediately [depsgraph relation will work only after certain updates]

Commits

rB3b57927651fe: Fix T82242: creating particle influence textures does not set up DEG

Summary

Texture and ParticleSettings have a DEG relation, but
DEG_relations_tag_update was not called when the texture changed.
This lead to no updates when e.g. texture size changes, relation only
went into full effect after save/reload or adding/removing keyframes.

Two places were additional relation tagging is needed:

ParticleSettings active_texture changes
ParticleSettingsTextureSlot texture changes

Diff Detail

Repository

rB Blender

Branch

T82242 (branched from master)

Build Status

Buildable 11007
Build 11007: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Oct 30 2020, 12:14 PM

Philipp Oeser (lichtwerk) created this revision.

Philipp Oeser (lichtwerk) added projects: Nodes & Physics, Dependency Graph.Oct 30 2020, 12:14 PM

Harbormaster completed remote builds in B11002: Diff 30538.Oct 30 2020, 12:15 PM

It doesn't sound correct to me. rna_TextureSlot_update is used for updates of, say, blend type or texture offset. Such properties should not cause relations update as they do not affect relations, but affect the way how evaluation goes.

It also not clear to me why EOL area keeps draining time. @Dalai Felinto (dfelinto), @Brecht Van Lommel (brecht) the policy was only to look into recent regressions, was there change to this policy?

This revision now requires changes to proceed.Oct 30 2020, 12:24 PM

only update relations for the TextureSlot when a new texture is created

Harbormaster completed remote builds in B11007: Diff 30544.Oct 30 2020, 12:46 PM

In D9393#232945, @Sergey Sharybin (sergey) wrote:

It doesn't sound correct to me. rna_TextureSlot_update is used for updates of, say, blend type or texture offset. Such properties should not cause relations update as they do not affect relations, but affect the way how evaluation goes.

Corrected

It also not clear to me why EOL area keeps draining time. @Dalai Felinto (dfelinto), @Brecht Van Lommel (brecht) the policy was only to look into recent regressions, was there change to this policy?

Yep, that is true, could leave my hands off of it fully, but, eh, cough, D9056: Hair: Shape-preserving length brush is the same area, no?

This might be acceptable for now. But please get a second opinion here.

Yep, that is true, could leave my hands off of it fully, but, eh, cough, D9056: Hair: Shape-preserving length brush is the same area, no?

Well, grooming tools are all about moving points in space. Those are not likely to change the way they do it depending on particle system implementation. Besides, it was an experiment which is not directly aimed to master branch and doesn't drain other people's time.

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

Brecht Van Lommel (brecht) added inline comments.Oct 30 2020, 3:41 PM

source/blender/editors/render/render_shading.c
841	I don't think this has to be for `RNA_ParticleSettingsTextureSlot` only? If you assign an ID pointer, that means a relations update. The same can be added for the new material and new world operators as well.

move tagging back to (specialized) RNA callback - this is more general, no?

Harbormaster completed remote builds in B11163: Diff 30843.Nov 6 2020, 6:47 PM

Sorry, really dont want this to be a time-sink for review, just thought this would be the better solution?

I don't think this has to be for RNA_ParticleSettingsTextureSlot only?

This is now more general

If you assign an ID pointer, that means a relations update. The same can be added for the new material and new world operators as well.

Since the RNA callback are called here: for Materials, rna_MaterialSlot_update already does relation tagging

source/blender/editors/render/render_shading.c
841	check if this should be included in the corresponding RNA_property_update rna_MaterialSlot_update

Please ignore the reply to Brechts inline comment (not sure what happened there, just some internal notes...)

@Sergey Sharybin (sergey): do I still have green light for this solution? (thx for having a look in advance)

From quickly skimping through the code seems fine. yes.
But could worth having send pair of eyes from Campbell or Brecht.

Philipp Oeser (lichtwerk) added a reviewer: Brecht Van Lommel (brecht).Nov 12 2020, 12:15 PM

@Brecht Van Lommel (brecht): think this can go in?

Yes, it's fine.

Closed by commit rB3b57927651fe: Fix T82242: creating particle influence textures does not set up DEG (authored by Philipp Oeser (lichtwerk)). · Explain WhyDec 9 2020, 10:02 PM

This revision was automatically updated to reflect the committed changes.

Philipp Oeser (lichtwerk) added a commit: rB3b57927651fe: Fix T82242: creating particle influence textures does not set up DEG.

Revision Contents
Changeset List

Path

Size

source/

blender/

editors/

render/

render_shading.c

4 lines

makesrna/

intern/

rna_particle.c

2 lines

Diff 30544

View Options

source/blender/editors/render/render_shading.c