a bunch of fixes

Author: dooly123

Basis Server/BasisNetworkCore/BasisNetworkVersion.cs

@@ -78,6 +78,24 @@ public class BasisNetworkVersion
         //     derived from the channel (worth 1.6pp on keyframes, zero on deltas, and it would make
         //     the decoder depend on reproducing the serializer's exact byte geometry).
         //     Wire-incompatible in both directions: a v49 peer reads the [n] byte as a length.
-        public static ushort ServerVersion = 50;
+        // 51: three of the four bone slots that quantized their smallest-three components into a
+        //     range narrower than InvSqrt2 — UpperChest (0.50), Shoulders (0.50), Feet (0.60) —
+        //     go to the full range. A narrowed range is a hard clamp at 2*asin(range), and clamping
+        //     a component does not shorten the rotation, it changes it: the decoder rebuilds the
+        //     dropped component from the three that survived. Measured with both ends in perfect
+        //     agreement and no loss anywhere, a 0.50 slot is 20° wrong at 80° and 30° wrong at 90°,
+        //     and a 0.60 foot is 16° wrong at 90°. Those are the only slots in the rig that were
+        //     narrowed and the only ones that could be visibly wrong, which is why the symptom read
+        //     as "feet point the wrong way" rather than as general pose noise.
+        //     Same field layout, same widths, same packet size — only the SCALE of three components
+        //     per affected slot changes, so a v50 peer decodes those bones ~18% short and this is
+        //     wire-incompatible in both directions.
+        //     Precision given up at High: 0.06° -> 0.06° on the 12-bit slots, i.e. nothing; a win or
+        //     a wash at every lower tier too. Toes are deliberately NOT widened — at 5/3/3/2 bits
+        //     their error is quantization-dominated, so stretching the same codes over 1.4x the
+        //     range costs more than the clamp does below High. They still carry a 30° cliff and want
+        //     a per-quality range table rather than this one shared one.
+        //     See the MAX_COMPONENT note in BasisBoneRotationCompression for the full table.
+        public static ushort ServerVersion = 51;
     }
 }

Basis Server/BasisNetworkCore/Compression/BasisBoneRotationCompression.cs

@@ -172,13 +172,40 @@ static BasisBoneRotationCompression()
         /// Values derived from max anatomical rotation, computing sin(maxAngle/2)
         /// for the largest possible remaining component, plus safety margin.
         /// Full InvSqrt2 used for any joint that can approach or exceed 90° from T-pose.
+        ///
+        /// <para><b>A range narrower than InvSqrt2 is a CLIFF, not a graceful loss of precision.</b>
+        /// While the rotation angle stays under 90° the dropped component is w, so the three that
+        /// survive are the vector part and every one of them is bounded by sin(angle/2). A range of
+        /// r therefore encodes exactly up to 2*asin(r) and CLAMPS above it — and clamping a component
+        /// does not shorten the rotation, it changes it, because the decoder rebuilds the dropped
+        /// component as sqrt(1 - a² - b² - c²) from whatever survived. Measured worst case with
+        /// sender and receiver in perfect agreement, no packet loss and no interpolation:
+        ///
+        /// <code>
+        ///   range 0.50 (clamps at 60°):  10° error at 70°, 20° at 80°, 30° at 90°
+        ///   range 0.60 (clamps at 74°):   6° error at 80°, 16° at 90°
+        /// </code>
+        ///
+        /// Past ~100° the error falls away again as a vector component overtakes w and gets dropped
+        /// instead, so the damage is a band just above the clamp — which is exactly where an ankle
+        /// sits when the toes are pointed, and where a clavicle sits on a shrug.</para>
+        ///
+        /// <para>Feet, shoulders, upper chest and toes were the only slots in the rig carrying a
+        /// narrowed range, and they were the only slots that could be visibly wrong on the wire. The
+        /// anatomical ROM figures they were derived from are sound; what they missed is that the
+        /// encoded quantity is the delta from the AVATAR'S OWN rest pose, which does not sit at
+        /// anatomical neutral on every rig, so the real budget needed is the ROM plus whatever the
+        /// bind pose already spent. There is no headroom to be had at 0.50. Feet, shoulders and
+        /// upper chest are InvSqrt2 now, which costs 0.06° -> 0.06° at High (i.e. nothing) and is a
+        /// win or a wash at every lower tier. Toes stay narrow — see the note on that entry.</para>
         /// </summary>
         public static readonly float[] MAX_COMPONENT = new float[]
         {
             // 3-DOF body (9): Spine, Chest, UpperChest, Neck, Head, UpperArms, UpperLegs
             InvSqrt2,               // Spine         full (deep backbend/fold can exceed 90° combined)
             InvSqrt2,               // Chest         full
-            0.50f,                  // UpperChest    thoracic limit ~58° → 1.41x
+            InvSqrt2,               // UpperChest    was 0.50 (~58° thoracic limit): clamped at 60°,
+                                    //               30° wrong at 90°. Costs 0.00° at High to widen.
             InvSqrt2,               // Neck          full (extreme head tilt)
             InvSqrt2,               // Head          full
             InvSqrt2, InvSqrt2,     // UpperArms     full (shoulder has ~180° ROM)
@@ -189,12 +216,27 @@ static BasisBoneRotationCompression()
             InvSqrt2, InvSqrt2,     // LowerLegs     full (knee 150°)
 
             // 2-DOF extremities (6): Shoulders, Hands, Feet
-            0.50f, 0.50f,           // Shoulders     clavicle max ~58° (shrug+protract) → 1.41x
+            InvSqrt2, InvSqrt2,     // Shoulders     was 0.50 (~58° clavicle): a shrug+protract lands
+                                    //               in the clamp band. 30° wrong at 90°.
             InvSqrt2, InvSqrt2,     // Hands         full (wrist can circle ~90°)
-            0.60f, 0.60f,           // Feet          ankle max ~70° combined → 1.18x
+            InvSqrt2, InvSqrt2,     // Feet          was 0.60 (~70° ankle): pointed toes, kneeling and
+                                    //               tiptoe all exceed it. 16° wrong at 90°.
 
             // toes (2) — eyes/jaw excluded (driven by face system)
-            0.50f, 0.50f,           // Toes          ~58° curl → 1.41x
+            0.50f, 0.50f,           // Toes          KEPT NARROW, and it is a genuine trade rather
+                                    //               than an oversight. Toes get 5/3/3/2 bits, so
+                                    //               unlike every slot above them their error is
+                                    //               dominated by quantization, not by the clamp.
+                                    //               Widening to InvSqrt2 helps only at High
+                                    //               (worst 30.0° -> 7.4°) and hurts everywhere else
+                                    //               (Medium/Low 3 bits: 15.9° -> 32.9° in-band;
+                                    //               VeryLow 2 bits: 38.4° -> 97.7°), because
+                                    //               stretching the same 4 or 8 codes over 1.4x the
+                                    //               range costs more than the cliff it removes.
+                                    //               Fixing this properly means a per-QUALITY range
+                                    //               table (full range where bits are plentiful,
+                                    //               narrow where they are not), the same way
+                                    //               GetBpcTable already varies by quality.
 
             // finger proximal (10): curl ~90° + spread ~25° → combined ~95°
             // At 95°: axis=0.74, w=0.68. After dropping axis, remaining max=0.68
@@ -338,16 +380,11 @@ public static int ConvertToSize(BasisAvatarBitPacking.BitQuality q)
             return BasisAvatarBitPacking.PositionBytes(q) + RotationBytes(q) + TailBytes + EndEffectorBytes(q);
         }
 
-        public static int ComputeBitOffsets(byte[] bpc, int[] outBitOffsets)
-        {
-            int pos = 0;
-            for (int i = 0; i < bpc.Length; i++)
-            {
-                outBitOffsets[i] = pos;
-                pos += 2 + 3 * bpc[i];
-            }
-            return pos;
-        }
+        // ComputeBitOffsets lived here: it laid out all 51 bone slots as 2 + 3*bpc apiece, which was
+        // the wire format until v47 moved the thirty finger joints to ten curl/splay channels. It had
+        // no callers left but its own test, and that test failed because it compared its total
+        // against RotationBytes, which follows the real layout. BuildRotationFieldOffsets is the
+        // version that models the wire as it is; use that.
 
         // ────────────────────────────────────────────────────────────
         //  Smallest-Three Encode / Decode (pure floats, no Unity types)

Basis Server/BasisServerTests/CorePrimitiveCompressionTests.cs

@@ -178,6 +178,71 @@ public void SmallestThree_OutOfRangeComponents_ClampToMaxRange()
         Assert.True(Math.Abs(norm - 1.0) < 1e-3);
     }
 
+    /// <summary>
+    /// The clamp above is correct behaviour for an explicitly narrow range, but on the RIG it is a
+    /// defect: a bone whose MAX_COMPONENT is narrower than the motion it actually sees does not lose
+    /// precision gracefully, it reconstructs a different rotation, because the decoder rebuilds the
+    /// dropped component from whatever survived the clamp. UpperChest, the shoulders and the feet
+    /// each carried a narrowed range and each was 16-30° wrong in the band just above its clamp
+    /// (v51). This pins the whole High rig against that returning: every slot must reproduce every
+    /// rotation angle to within its own quantization step.
+    ///
+    /// Toes are excluded deliberately and are the one slot still narrow — see the note on the
+    /// MAX_COMPONENT entry for why widening them costs more than it buys below High.
+    /// </summary>
+    [Fact]
+    public void SmallestThree_EveryHighSlot_TracksRotationAcrossItsFullAngleRange()
+    {
+        byte[] bpc = BasisBoneRotationCompression.GetBpcTable(BitQuality.High);
+        float[] maxComp = BasisBoneRotationCompression.MAX_COMPONENT;
+        int[] toeSlots = { 19, 20 };
+
+        // Fixed seed: the axis set must be dense enough to find a clamp but must not be tuned to it.
+        var rng = new Random(20260809);
+
+        for (int slot = 0; slot < BasisBoneRotationCompression.WireBoneSlotCount; slot++)
+        {
+            if (Array.IndexOf(toeSlots, slot) >= 0) continue;
+
+            // A component step is 2*range/(2^bpc - 1); the angular error it produces is a small
+            // multiple of that in radians. 12x leaves ~4x headroom over the measured worst case
+            // (0.06° at High) while still being ~100x under the 16-30° a clamp produces.
+            double step = 2.0 * maxComp[slot] / ((1 << bpc[slot]) - 1);
+            double tolDeg = 12.0 * step * 180.0 / Math.PI;
+
+            for (int deg = 0; deg <= 180; deg += 5)
+            {
+                for (int i = 0; i < 400; i++)
+                {
+                    // Uniform axis on the sphere — a clamp is a property of the component that ends
+                    // up largest, so the axis has to be swept, not just the angle.
+                    double z0 = 2 * rng.NextDouble() - 1;
+                    double r = Math.Sqrt(Math.Max(0, 1 - z0 * z0));
+                    double phi = 2 * Math.PI * rng.NextDouble();
+                    float ax = (float)(r * Math.Cos(phi)), ay = (float)(r * Math.Sin(phi)), az = (float)z0;
+
+                    double half = deg * Math.PI / 180.0 * 0.5;
+                    float s = (float)Math.Sin(half);
+                    float qx = ax * s, qy = ay * s, qz = az * s, qw = (float)Math.Cos(half);
+
+                    ulong packed = BasisBoneRotationCompression.EncodeSmallestThree(qx, qy, qz, qw, bpc[slot], maxComp[slot]);
+                    BasisBoneRotationCompression.DecodeSmallestThree(packed, bpc[slot],
+                        out float dx, out float dy, out float dz, out float dw, maxComp[slot]);
+
+                    double dot = Math.Min(1.0, Math.Abs((double)qx * dx + (double)qy * dy + (double)qz * dz + (double)qw * dw));
+                    double errDeg = 2.0 * Math.Acos(dot) * 180.0 / Math.PI;
+
+                    Assert.True(errDeg <= tolDeg,
+                        $"slot {slot} (bpc {bpc[slot]}, range {maxComp[slot]}) is {errDeg:F2}° off at " +
+                        $"{deg}° about ({ax:F3}, {ay:F3}, {az:F3}); tolerance {tolDeg:F2}°. A narrowed " +
+                        $"MAX_COMPONENT clamps at " +
+                        $"{2 * Math.Asin(Math.Min(1f, maxComp[slot])) * 180 / Math.PI:F1}° and reconstructs " +
+                        $"a DIFFERENT rotation above it, not a less precise one.");
+                }
+            }
+        }
+    }
+
     // ────────────────────────────────────────────────────────────
     //  Bitstream
     // ────────────────────────────────────────────────────────────
@@ -247,34 +312,41 @@ public void WriteBits_IsLsbFirst_AndLeavesNeighborsUntouched()
     //  Bone tables and packet sizing
     // ────────────────────────────────────────────────────────────
 
+    /// <summary>
+    /// Replaces ComputeBitOffsets_MatchesRotationBytes_ForAllQualities, which pinned a helper that
+    /// laid out all 51 bone slots as 2 + 3*bpc — the wire format until v47 moved the thirty finger
+    /// joints to ten curl/splay channels. The helper had no callers but that test and the assertion
+    /// was simply false (Medium: 78 bytes claimed against the real 52). Both are gone; this pins the
+    /// same invariant against BuildRotationFieldOffsets, which is what the channel map actually uses.
+    /// </summary>
     [Fact]
-    public void ComputeBitOffsets_MatchesRotationBytes_ForAllQualities()
+    public void RotationFieldOffsets_AreContiguous_AndMatchRotationBytes_ForAllQualities()
     {
         foreach (var q in AllQualities)
         {
-            byte[] bpc = BasisBoneRotationCompression.GetBpcTable(q);
-            var offsets = new int[bpc.Length];
-            int totalBits = BasisBoneRotationCompression.ComputeBitOffsets(bpc, offsets);
+            int[] widths = BasisBoneRotationCompression.BuildRotationFieldWidths(q);
+            Assert.Equal(BasisBoneRotationCompression.RotationFieldCount, widths.Length);
+
+            var offsets = new int[BasisBoneRotationCompression.RotationFieldCount];
+            int totalBits = BasisBoneRotationCompression.BuildRotationFieldOffsets(q, offsets);
 
-            Assert.Equal(0, offsets[0]);
+            // Offsets must tile the region exactly: no gaps, no overlap.
             int expected = 0;
-            for (int i = 0; i < bpc.Length; i++)
+            for (int i = 0; i < widths.Length; i++)
             {
                 Assert.Equal(expected, offsets[i]);
-                expected += 2 + 3 * bpc[i];
+                expected += widths[i];
             }
             Assert.Equal(expected, totalBits);
-
-            // RotationBytes is NOT this total. ComputeBitOffsets walks all 51 legacy bone slots;
-            // since v47 the wire carries 21 of them plus a 10-channel finger block, so the wire size
-            // comes from the rotation FIELD widths instead. (Sizing anything off the 51-slot total
-            // overruns every sub-High payload — it is larger than the payload itself.)
-            var fieldWidths = BasisBoneRotationCompression.BuildRotationFieldWidths(q);
-            int wireBits = 0;
-            foreach (int w in fieldWidths) wireBits += w;
-            Assert.Equal(wireBits, BasisBoneRotationCompression.RotationBits(q));
-            Assert.Equal((wireBits + 7) >> 3, BasisBoneRotationCompression.RotationBytes(q));
-            Assert.True(totalBits > wireBits, "legacy 51-slot total should exceed the v47 wire size");
+            Assert.Equal(totalBits, BasisBoneRotationCompression.RotationBits(q));
+            Assert.Equal((totalBits + 7) >> 3, BasisBoneRotationCompression.RotationBytes(q));
+
+            // The explicit bone slots come first, then one field per finger channel.
+            for (int slot = 0; slot < BasisBoneRotationCompression.WireBoneSlotCount; slot++)
+                Assert.Equal(2 + 3 * BasisBoneRotationCompression.GetBpcTable(q)[slot], widths[slot]);
+            for (int f = 0; f < BasisBoneRotationCompression.FingerChannelCount; f++)
+                Assert.Equal(BasisBoneRotationCompression.FingerFieldWidth(q),
+                    widths[BasisBoneRotationCompression.WireBoneSlotCount + f]);
         }
     }