[Bf-codereview] Pepper to trunk merge (issue 4934047)

aligorith at gmail.com aligorith at gmail.com
Mon Aug 29 05:03:05 CEST 2011


http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/anim_sys.c
File source/blender/blenkernel/intern/anim_sys.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/anim_sys.c#newcode1171
source/blender/blenkernel/intern/anim_sys.c:1171: if
(RNA_struct_is_a(new_ptr.type, &RNA_PoseBone)) {
On 2011/08/23 18:39:45, ideasman42 wrote:
> for speed in this case I think you'd be better just to use a
comparison
> (new_ptr.type == &RNA_PoseBone) since we know PoseBone's aren't
subclassed.

> Since for non posebone types it will search up their type's bases.
Good point. Will do

http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/depsgraph.c
File source/blender/blenkernel/intern/depsgraph.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/depsgraph.c#newcode2068
source/blender/blenkernel/intern/depsgraph.c:2068: if
(adt->drivers.first)
On 2011/08/24 14:28:30, brechtvl wrote:
> This seems like a workaround for missing dependency graph relations?
The driver
> should add a relation and then the update flags should get flushed.
They will be
> missing if the object is being driven by a non-object datablock, but
if that's
> what this is intended to work around, perhaps that should be clarified
in the
> comment.
Indeed, this is a workaround for missing relations from all manner of
non-object datablocks. In this particular case, it was just the
current-frame in scene which people often used, but as with many things
like this, it extended to everything else.

Will clarify the comment

http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/fmodifier.c
File source/blender/blenkernel/intern/fmodifier.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/blenkernel/intern/fmodifier.c#newcode1212
source/blender/blenkernel/intern/fmodifier.c:1212: * NOTE: this is
really just a hack so that we don't need to version patch old files ;)
On 2011/08/24 14:28:30, brechtvl wrote:
> I think this should just be version patched, and the use_influence
property
> removed.
I considered that, but to cover each of the places where these can live
and update them involves quite a bit of effort, which IMO isn't worth
it.

Furthermore, this was assumed to not be frequently used enough that this
setting would be a hassle (or new FModifiers could always get created
with this enabled).

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/animation/drivers.c
File source/blender/editors/animation/drivers.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/animation/drivers.c#newcode386
source/blender/editors/animation/drivers.c:386: /* flags - on a
per-relevant-flag basis */
On 2011/08/23 18:39:45, ideasman42 wrote:
> shouldn't this comment be removed?

Yep. Will do

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/space_nla/nla_draw.c
File source/blender/editors/space_nla/nla_draw.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/space_nla/nla_draw.c#newcode474
source/blender/editors/space_nla/nla_draw.c:474: if (strip->flag &
NLASTRIP_FLAG_REVERSE)
On 2011/08/23 18:39:45, ideasman42 wrote:
> should be removed/commented or made into different strings.

Huh?

IIRC, I was still playing around with representations of reversed vs
normal order here and left this lying around. While not a biggie, I'll
probably just remove this.

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_access.c
File source/blender/makesrna/intern/rna_access.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_access.c#newcode1424
source/blender/makesrna/intern/rna_access.c:1424: void
RNA_property_update_cache_add(PointerRNA *ptr, PropertyRNA *prop)
On 2011/08/23 18:39:45, ideasman42 wrote:
> IMHO this should eventually cache based on unique pointer triplets:
>    (ptr.id.data, ptr.data, update_func)
"Eventually", but only if we ever find ourselves with such many such
update functions to justify needing to store the different ptr.data's.

But if and when this happens, we'd probably want to be able to flag the
update functions as requiring id only or both, so that we can optimise
for all those current cases where update only reads id pointer to tag
depsgraph on that ID.

http://codereview.appspot.com/4934047/


More information about the Bf-codereview mailing list