Details

Reviewers

Commits

rBd040e1da4f61: Constraints: introduce wrapper functions to access target lists.

Summary

Instead of directly accessing constraint-specific callbacks
in code all over blender, introduce two wrappers to retrieve
and free the target list.

This incidentally revealed a place within the Collada exporter
in BCAnimationSampler.cpp that didn't clean up after retrieving
the targets, resulting in a small memory leak. Fixing this should
be the only functional change in this commit.

This was split off from D9732.

Diff Detail

Repository: rB Blender

Event Timeline

Alexander Gavrilov (angavrilov) requested review of this revision.Jan 16 2022, 7:50 PM

Alexander Gavrilov (angavrilov) created this revision.

Harbormaster completed remote builds in B20020: Diff 47107.Jan 16 2022, 7:51 PM

Brad Clark (RiggingDojo) awarded a token.Jan 24 2022, 4:35 PM

Brad Clark (RiggingDojo) added a subscriber: Brad Clark (RiggingDojo).

Rebased and fixed conflict.

Harbormaster completed remote builds in B20407: Diff 48083.Feb 8 2022, 12:33 PM

This is some nice cleanup, I agree that it's desirable to have functions for this.

source/blender/blenkernel/BKE_constraint.h
315	Document the return value.
315	Document that `r_list` is appended to by this function, and not cleared.
320	Document what type should be in `list`.
320	Name `list` after the thing in there. The fact that it's a `ListBase` already communicates that this is a list.
320	I'm not a fan of boolean parameters, and even less of inverted boolean parameters. This should at least document what happens when it's true/false. It would be even better to just have two separate functions, one for copying and one for not, if that's in any way doable. They can then get their own documentation.
source/blender/blenkernel/intern/constraint.c
6198	Flip the condition, return early. Same for the other ones, so that the happy flow is going straight down instead of indenting more and more.

This revision now requires changes to proceed.Feb 18 2022, 2:47 PM

Updated.

Harbormaster completed remote builds in B20683: Diff 48601.Feb 21 2022, 7:45 PM

Alexander Gavrilov (angavrilov) marked 5 inline comments as done.Feb 21 2022, 7:47 PM

Alexander Gavrilov (angavrilov) added inline comments.

source/blender/blenkernel/BKE_constraint.h
315	Actually, there is no reason not to clear it - it's just an artifact of how things worked before.
320	The parameter is directly inherited from the original callback, so parameters in the calls to the wrapper look the same.

Rebased.

Harbormaster completed remote builds in B22410: Diff 52212.Jun 3 2022, 1:30 PM

Two tiny changes, no need to re-review after this.

source/blender/blenkernel/intern/constraint.c
6195–6197	Use `BLI_listbase_clear()`.
6204–6211	Simplify: /* Constraint-specific targets. */ if (cti->get_constraint_targets == NULL) { return 0; } return cti->get_constraint_targets(con, r_targets);

This revision is now accepted and ready to land.Jun 3 2022, 2:38 PM

Alexander Gavrilov (angavrilov) marked an inline comment as done.Jun 3 2022, 3:15 PM

Alexander Gavrilov (angavrilov) added inline comments.

source/blender/blenkernel/intern/constraint.c
6204–6211	It's written this way so a further planned patch (as mentioned this is a part of D9732 split off to simplify review, and I will immediately rebase that one after committing) can simply insert some code without refactoring it again.

Closed by commit rBd040e1da4f61: Constraints: introduce wrapper functions to access target lists. (authored by Alexander Gavrilov (angavrilov)). · Explain WhyJun 3 2022, 3:19 PM

This revision was automatically updated to reflect the committed changes.

Alexander Gavrilov (angavrilov) added a commit: rBd040e1da4f61: Constraints: introduce wrapper functions to access target lists..

Alexander Gavrilov (angavrilov) mentioned this in D9732: Constraints: apply small fixes for custom space handling..Jun 3 2022, 4:43 PM

Constraints: introduce wrapper functions to access target lists.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 52222

source/blender/blenkernel/BKE_constraint.h

source/blender/blenkernel/intern/action.c

source/blender/blenkernel/intern/constraint.c

source/blender/blenkernel/intern/object.cc

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

source/blender/draw/engines/overlay/overlay_extra.c

source/blender/editors/armature/armature_add.c

source/blender/editors/armature/armature_naming.c

source/blender/editors/armature/armature_relations.c

source/blender/editors/armature/pose_select.c

source/blender/editors/object/object_constraint.c

source/blender/io/collada/BCAnimationSampler.cpp

source/blender/io/collada/SceneExporter.cpp

source/blender/makesrna/intern/rna_constraint.c

Constraints: introduce wrapper functions to access target lists.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 52222

source/blender/blenkernel/BKE_constraint.h

source/blender/blenkernel/intern/action.c

source/blender/blenkernel/intern/constraint.c

source/blender/blenkernel/intern/object.cc

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

source/blender/draw/engines/overlay/overlay_extra.c

source/blender/editors/armature/armature_add.c

source/blender/editors/armature/armature_naming.c

source/blender/editors/armature/armature_relations.c

source/blender/editors/armature/pose_select.c

source/blender/editors/object/object_constraint.c

source/blender/io/collada/BCAnimationSampler.cpp

source/blender/io/collada/SceneExporter.cpp

source/blender/makesrna/intern/rna_constraint.c

Constraints: introduce wrapper functions to access target lists.
ClosedPublic
Actions

Revision Contents
Changeset List