Friday, March 8, 2013

The Power of Open Source - Visual Keying Bugfix

While checking my email this morning I was reminded of the power of open source...




Like a malfunctioning Jack-in-the-Box or the Ancient Mariner's albatross, every few months or so, there's always a new report about Visual Keying failing in some or other way.

In the earlier incarnations of this feature, this was perhaps understandable, as back then it tried to implement its own logic for these things. For example, IIRC it once calculated various inverses manually (which works ok in some cases, but quickly gets messy when making sure all the different combinations of non-inherited rotations/scaling/locations/parenting that went on). Many tweaks later, at one stage it was trying to use the wrong matrix as the localspace estimate (For reference: pchan->chanmat is NOT a matrix representing the bone's transform properties once pose calculations have finished; it's only when evaluating constraints that that's the case. Instead, pchan->chanmat actually stores the "complete" deforms which are needed for driving B-Bone calculations and armature deforms).

After many of these back and forth fixup attempts, I eventually replaced this with the same underlying mechanism used by the constraints system for converting matrices from one space to another. Basically, if even this system doesn't work, then the constraints shouldn't either (unless there was another "chanmat" snarfu hiding under the carpet waiting to pounce).

By and large, this system works as expected. For most constraints, it works, though IK chains sometimes pose some problems (namely because the act of inserting keyframes and performing the relevant conversions somehow ends up invariably changing the results of the solver due to different initial conditions for the solving process). Having said that, it was finally starting to settle down a bit at last, or so it seemed...

Recently, we got a pair of fresh new reports. Visual Keying, in particular rotations, was broken. Buggered. Completely missing in action. With some trepidation, I opened up the reports... "Ack! Not again! :-("

As usual, I went through the usual checks:
- Was it an error in the space conversions? Err... it's the exact same method used by constraints, which now uses the exact same core computations used to calculate the pose transforms in the first place...
- Was it an error with how we were extracting the rotations/transforms? Err... nope. There's only one way to do that, and that's what we're doing...
- Could it be those space conversions again? Nah... can't be...
- Hmm... the suggested py hacks apparently do the trick, but they smell suspiciously of calculating inverses which only work in a few "normal" cases only....
- Maybe it's the space conversions again...
- Or maybe it's the transform extractions...
- ... while (!insane) keep_running_in_circles();

---

Fast forward a few weeks.

After nearly giving up hope of finding any suitable fix for this problem, I found the message above in my inbox. Eh? A shadowed variable?! Surely not...

Lo and behold, it was there in the source, staring at me when I finally got back to my development computer and checked. How could it be (* 1)? Why hadn't the compiler picked up on it like it sometimes does (*)? Argh!

(* 1) My guess is that this crept in during a little refactor someone else did here a few releases ago to introduce another feature. A telltale sign was the clobbered whitespace in a few places I remember fixing earlier.

(* 2) It seems that's the way with many of these bugs which compilers sometimes pick up on and but oftentimes don't (even with all these warnings/errors turned on already), and end up causing often unexpected results:
- Shadowed vars - I've seen the warnings a few times, but in this case, there wasn't any hint of a warning for that file.
- Missing includes - Oops! How'd you like a nice little segfault when clicking right in that spot there to activate this obscure function you've been hacking the past hour...


Anyways, testing the fix, it quickly became obvious that the problems were now gone. Cheers to Josef Meier for detecting this bug!

No comments:

Post a Comment