Page MenuHome

Armature: add B-Bone lengthwise scaling and custom handle scaling options.
ClosedPublic

Authored by Alexander Gavrilov (angavrilov) on Dec 16 2020, 11:58 AM.

Details

Summary

In addition to the base bone transformation itself, B-Bones have
controls that affect transformation of its segments. For rotation
the features are quite complete, allowing to both reorient the
Bezier handles via properties, and to control them using custom
handle bones. However for scaling there are two deficiencies.

First, there are only X and Y scale factors (actually X and Z),
while lengthwise all segments have the same scaling. The ease
option merely affects the shape of the curve, and does not cause
actual scaling.

Second, scaling can only be controlled via properties, thus
requiring up to 6 drivers per joint between B-Bones to transfer
scaling factors from the handle bone. This is very inefficient.

This patch addresses these deficiencies by adding length scale
inputs, and providing toggles to apply custom handle local scale
channels to the now four scale-related properties.

The two length scale inputs control the ratio between the lengths
of the start and end segments of the bone: although for convenience
two inputs are provided, the whole chain is still uniformly scaled
to fit the curve.

To accommodate the third scale value, the scale channels are
converted from two scalars into a vector. The old Y becomes Z;
the Curve In/Out properties are also renamed to match. Old
files are properly versioned, including animation & drivers.

A Scale Easing option is provided to multiply the easing value
by the Y scale factors to synchronize them - this produces a
natural scaling effect where both the shape of the curve and
the scale is affected.

The second issue is addressed by providing toggles for each handle
that multiply each of the X, Y, Z and Ease values by the matching
Local Scale channel of the handle bone, thus replacing trivial drivers.
The Scale Easing option has no effect on this process since it's easy
to just enable both Length and Ease buttons.


UI screenshot:

UPDATE: "Unscaled Segments" removed.

Diff Detail

Repository
rB Blender
Branch
temp-angavrilov-bbone-scale (branched from master)
Build Status
Buildable 15203
Build 15203: arc lint + arc unit

Event Timeline

Alexander Gavrilov (angavrilov) requested review of this revision.Dec 16 2020, 11:58 AM
Alexander Gavrilov (angavrilov) created this revision.

OBSOLETE Simple test file:

Addressing these deficiencies was previously requested by @Juan Pablo Bouza (jpbouza), and this patch, along with D9493, D9469 and D9626, is required for the new Rigify face rigging system currently being developed in cooperation with @Ivan Cappiello (icappiello). One of the latest WIP generated result files is here, which can be tested using a branch build that includes these patches.

I couldn't build this(issues on my side I think) but I used your branch to test.

The behaviour is definitely nice! Here's me playing around with it:

I also modified your example file a bit for whoever will do code review to have a nicer time playing around with it themselves:


Here are some of the things that I think this file can demonstrate:

  • The "Scale Out" values use drivers, whereas the "Scale In" values use the new toggles to get the same result in a nicer way (since this set-up is so commonly desired, it shouldn't require a driver) So these two setups are equivalent (which is nice!): 1, 2
  • For many of the new options to be noticable, it's easiest if you start by scaling the control bones on their local Y axis.
  • Interesting that the legacy un-scaled segments option gives nicer results at a low segment count: Legacy, Scaled
  • Disabling Ease Scaling but enabling Length Scaling gives this really cool result that was not possible before!

The added complexity will benefit from some clear documentation, but I like the tooltips.
I also tested opening some Sprite Fright rigs and animations, everything seems to be undisturbed, yay!

Some concerns:

  1. As discussed in another patch, I wonder if the naming debacle regarding X/Y/Z could be resolved here so that eg. bbone_scaleinx, bbone_scaliny, bbone_scalein_len turns into bbone_scalein[0]. This makes the backward compatibility break clear and understandable for addon developers, and fixes the terrible mistake of naming these properties with the wrong axes a very long time ago. I know there were some technical questions regarding whether this kind of versioning is possible, I hope that can be figured out.
  1. For the UI, I wonder if the correlation between the bbone_handle_scale_start array values and the regular bbone "slider" values could be better communicated by putting them next to each other (terrible mockup). This UI layout is used in some places in Blender like the Stretch constraint, but it makes a bit more sense there since it's just enabling the slider that is right next to the checkbox, which is not the case here. I don't feel that strongly about this, but I thought I'd throw this out there for consideration.
  1. Also, I think they should be getting grayed out or hidden unless the handle type is set to Tangent or Relative.

Reworked the patch to rename Y to Z and convert scale to a 3 component vector.

Actual old files are updated by versioning, including animation and drivers, but all test files previously made for this patch are now broken.

New simple test file:

Test saved in 2.83 for checking that versioning works:

Alexander Gavrilov (angavrilov) edited the summary of this revision. (Show Details)

Tested again, good job on getting the versioning working, Sprite Fright characters seem to continue functioning exactly as before.

Looks good to go for code review as far as I'm concerned! :)

Sybren A. Stüvel (sybren) requested changes to this revision.EditedJun 10 2021, 5:12 PM

I think this patch does a few things at once, and even though they're useful, I think they should be split up:

  • Correct the "BBone Y-axis is actually the Z axis" mess,
  • move flags from flag to bbone_flag in DNA, and
  • introduce new scaling options.

For the Y=Z axis mess, I completely agree to get this fixed. Maybe we could even have a transition period, in which the old Y-property is mapped in RNA to the Z-property so that Python scripts can be gradually ported instead of breaking all at once.

PS: Having these changes all in one patch does make it nice to see the final result + the changes that are necessary to make it happen in a clean way, so this is not a sneer at how the patch is presented, just a note at how we should proceed.

source/blender/blenkernel/intern/armature.c
970

!! is useful in JavaScript, but doesn't mean anything in C. If you want to convert to a boolean, (bone->bbone_flag & BBONE_SCALE_SEGMENTS) != 0 is clearer.

source/blender/makesrna/intern/rna_armature.c
780

Properties should be positive, so setting a property to true should enable something, not disable it.

783–784

This should explain the option better (regardless of the positive/negative flip); currently it doesn't really explain why you would choose to enable or disable this option in an objective way. As @Demeter Dzadik (Mets) also noted, also in new files there could be uses for this option in certain low-segment-count cases.

1091

The BBone sides are referred to as "in" and "out" in other bits of the UI, and this should be consistent. Personally I wouldn't mind (and @Demeter Dzadik (Mets) also) to rename the others to "start" and "end" also, but I'm sure tutorial/book makers will hate us for it. The simplest would be to just name this "In Handle Scale" and "Out Handle Scale".

This revision now requires changes to proceed.Jun 10 2021, 5:12 PM

I think this patch does a few things at once, and even though they're useful, I think they should be split up:

  • Correct the "BBone Y-axis is actually the Z axis" mess,
  • move flags from flag' to bbone_flag` in DNA, and
  • introduce new scaling options.

It is technically very inadvisable to split Y->Z rename and adding the third scaling channel, because it would mean pointless temporary existence of bbone_scaleinz in drivers etc. Separating the new toggles could be feasible.

source/blender/makesrna/intern/rna_armature.c
1091

Wrong. If you look at the properties above, they all use 'start handle' wording. Your comment may be reasonable for the tooltip, i.e. 'scale in channels', but not when referring to the handles themselves.

  • Removed the Unscaled Segments option to simplify review (keeping in branch to submit as a separate patch later)
  • Split into 3 commits in a branch (DNA+Versioning, Y Scale implementation, Toggles)
  • Made the new boolean toggles animatable.
  • Tweaked tooltips to use 'Scale In' but 'start handle' wording, consistent with adjacent options.

It is technically very inadvisable to split Y->Z rename and adding the third scaling channel, because it would mean pointless temporary existence of bbone_scaleinz in drivers etc.

I'm thinking about some way to keep some form of backward compatibility. Not sure if I'm a big fan myself, though.

  • bbone_scalein: 3D vector that's to be used in all scripts/drivers/etc. from now on.
  • bbone_scaleinx and bbone_scaleinz: alias for bbone_scalein.x resp. bbone_scalein.z
  • bbone_scaleiny: alias for bbone_scaleinz for backward-compatibility

This would make it possible for script authors (which includes riggers) to gradually port their stuff to Blender 3.0. The downside is that bbone_scaleiny and bbone_scalein.y actually have different semantics, and I'm not sure if I find that acceptable.

If you have better ideas for maintaining backward compatibility, I'm all ears. I do find it important that my colleages at the Blender Animation Studio can keep working. If we commit this patch, pretty much all the team need to avoid updating Blender, then @Demeter Dzadik (Mets) has to race to make the changes to all the rigs, before the animators (and anyone else working with animated scenes) can get an update to Blender again.

Separating the new toggles could be feasible.

👍

source/blender/makesrna/intern/rna_armature.c
1091

It's all "Curve In/Out", "Roll In/Out", "Scale In/Out", and "Ease In/Out", but then when it's about the handles it's "Start/End Handle". Probably not something to address in this patch, though.

This would make it possible for script authors (which includes riggers) to gradually port their stuff to Blender 3.0. The downside is that bbone_scaleiny and bbone_scalein.y actually have different semantics, and I'm not sure if I find that acceptable.

If you have better ideas for maintaining backward compatibility, I'm all ears. I do find it important that my colleages at the Blender Animation Studio can keep working. If we commit this patch, pretty much all the team need to avoid updating Blender, then @Demeter Dzadik (Mets) has to race to make the changes to all the rigs, before the animators (and anyone else working with animated scenes) can get an update to Blender again.

I highly doubt any scripts that aren't rig generators like Rigify would touch the fields. Rigs would use drivers, and the versioning code updates them. I expect addons to need updating, but not already generated rigs.

To put in writing a quick chat I just had with the doktor:
+1, the only case where this would break rigs is if they rely on adding custom functions to driver namespace, and then that function relies on the old PyAPI paths.
At that point it's still just a Python API compatibility break, not really a rigging system compatibility break. And I think Python developers will welcome this change and be happy to adjust their code (which we have to do with every major release anyways!).

That’s good news!

This revision is now accepted and ready to land.Jun 17 2021, 4:16 PM