[Bf-committers] Shrinkwrap constraint

Joshua Leung aligorith at gmail.com
Wed Dec 17 00:49:13 CET 2008


Here is a little review based on constraints system requirements:
1) shrinkwrap_new_data() is not needed. MEM_callocN() is used to allocate
the data anyway, and since those values are all 0, it should be fine
without. Only define this callback if there's any custom settings (i.e.
non-zero values) which need to be set for the constraint by default.

2) Currently this is missing an entry to the Ctrl-Alt-C (add constraint)
menu. See editconstraint.c -> addconstraint(). Make sure you add the entry
in a similar order to the other menu and only for relevant type of target is
selected (I guess this would be for meshes only, right?)

3) Type menu - capitialise all the entries in that menu - each item should
begin with capital letter. Probably that menu string can be declared const
in the code too...

4) Distance to target option works in a rather odd fashion. In the quick
test I was doing (object shrinkwrapped to grid with some quick PET hills),
this setting only seems to offset the object downwards. It would be more
useful if negative values were allowed too, so that object/bone could be
moved to sit on top of the ground surface, instead of sinking into it.

5) Adding this constraint to a bone will clear the rotation/scale of the
bone. This is not acceptable. Seeing as you're just modifying the location
now, perhaps it would be better to just copy the location component of the
'target' matrix (i.e. result of shrinkwrap) to the 'owner' matrix in the
shrinkwrap_evaluate() callback.

Alternatively (or in addition), it might be useful to explore having an
option to use the vector along which the owner was moved to lie on the grid,
to define the rotation of the owner. This could be useful for having a bone,
etc. to align with the normals of the target mesh (i.e. when using this
constraint to keep eyelid control bones glued to an eyeball).

6) Small formatting nitpicks - with comments, use the /* */ style comments
instead of //. I generally reserve // for TODO/FIXME type of temporary
comments that should be addressed ASAP.


Anyways, overall the default shrinkwrapping behaviour is great! It works out
of the box like a treat, so if you just fix the issues noted here, it should
be great to go in.   :-)

+1 from me

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.blender.org/pipermail/bf-committers/attachments/20081217/52576940/attachment-0001.htm 

More information about the Bf-committers mailing list