Page MenuHome
Differential D6609

Fix T72213: F-Curve animation does not update FreeStyle properties
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Jan 17 2020, 1:53 PM.

Details

Reviewers
Sergey Sharybin (sergey)
Brecht Van Lommel (brecht)
Commits
rB9f66062da73e: Fix T72213: F-Curve animation does not update FreeStyle properties
Summary

FreeStyle line styles were not part of the dependency graph, and blacklisted from the Copy-on-Write system. As a result, animated FreeStyle properties would not be updated by the animation system, resulting in T72213. There was an explicit call to run the animation system on the original datablocks, but that was (for good reasons) removed in D5394.

This commit adds the FreeStyleLineStyle datablocks to the dependency graph and allows them to be handled by the CoW system. As a result

  • the UI now updates properly when properties are animated, and
  • animated property values are actually used when rendering.

@Sergey Sharybin (sergey), @Brecht Van Lommel (brecht), do you think this is safe for 2.82?

Diff Detail

Repository
rB Blender
Branch
temp-sybren-T72213-freestyle-anim-depsgraph (branched from master)
Build Status
Buildable 6302
Build 6302: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) created this revision.Jan 17 2020, 1:53 PM
Harbormaster completed remote builds in B6301: Diff 20930.Jan 17 2020, 1:53 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 20932.Jan 17 2020, 2:08 PM
  • Moved code around + updated comment, no functional changes
Harbormaster completed remote builds in B6302: Diff 20932.Jan 17 2020, 2:08 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 20934.Jan 17 2020, 2:12 PM
  • Some more cleanup of the patch
Harbormaster completed remote builds in B6303: Diff 20934.Jan 17 2020, 2:12 PM
Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Jan 17 2020, 2:12 PM
Brecht Van Lommel (brecht) requested changes to this revision.Jan 17 2020, 4:37 PM

Seems fine overall.

source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
84

Linestyles are datablocks and so could be used by multiple line sets. This is missing code to ensure the datablock is built only once, see similar functions like build_texture() that have:

if (built_map_.checkIsBuiltAndTag(texture)) {
  return;
}
source/blender/depsgraph/intern/builder/deg_builder_relations_view_layer.cc
85

Same comment as above.

This revision now requires changes to proceed.Jan 17 2020, 4:37 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 20941.Jan 17 2020, 4:48 PM
  • Added built_map_.checkIsBuiltAndTag() checks
Harbormaster completed remote builds in B6306: Diff 20941.Jan 17 2020, 4:48 PM
Sybren A. Stüvel (sybren) marked 2 inline comments as done.Jan 17 2020, 4:49 PM
Sergey Sharybin (sergey) requested changes to this revision.Jan 17 2020, 4:53 PM

The change is also missing tweak to ID_TYPE_IS_COW.

Think it's best to finally remove duplicated logic and apply P1222 together with your patch.

source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc
83–88

Linestyle building should be own function, and ID_LS is to be handled in build_id().

This revision now requires changes to proceed.Jan 17 2020, 4:53 PM
Sybren A. Stüvel (sybren) marked an inline comment as done.Jan 17 2020, 5:17 PM
Sybren A. Stüvel (sybren) updated this revision to Diff 20944.Jan 17 2020, 5:17 PM
Harbormaster completed remote builds in B6308: Diff 20944.Jan 17 2020, 5:17 PM
Sergey Sharybin (sergey) accepted this revision.Jan 20 2020, 10:35 AM

Looks good to me now.

I think it's safe for 2.82. If Brecht can have second unbiased look at changes that'd be fantastic :)

Brecht Van Lommel (brecht) accepted this revision.Jan 20 2020, 2:39 PM
This revision is now accepted and ready to land.Jan 20 2020, 2:39 PM
Closed by commit rB9f66062da73e: Fix T72213: F-Curve animation does not update FreeStyle properties (authored by Sybren A. Stüvel (sybren)). · Explain WhyJan 21 2020, 12:34 PM
This revision was automatically updated to reflect the committed changes.
Sybren A. Stüvel (sybren) added a commit: rB9f66062da73e: Fix T72213: F-Curve animation does not update FreeStyle properties.

Revision Contents
Changeset List

Diff 20932

View Options

source/blender/depsgraph/intern/builder/deg_builder_nodes.h

Loading...
View Options

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

Loading...
View Options

source/blender/depsgraph/intern/builder/deg_builder_nodes_view_layer.cc

Loading...
View Options

source/blender/depsgraph/intern/builder/deg_builder_relations.h

Loading...
View Options

source/blender/depsgraph/intern/builder/deg_builder_relations_view_layer.cc

Loading...
View Options

source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc

Loading...