Skip to content

[DEBUG] b2ValidateNoEnlarged asserts after destroying a body that was enlarged in the previous step #100

Description

@ctzcs

[DEBUG] b2ValidateNoEnlarged asserts after destroying a body that was enlarged in the previous step

Summary

In DEBUG builds, b2Solve's b2ValidateNoEnlarged check can assert even though nothing is actually wrong: a proxy that was enlarged in step N and destroyed before step N+1 leaves orphaned b2_enlargedNode flags on its ancestor tree nodes, and no tree rebuild ever runs to clear them.

In gameplay terms this is "kill an enemy that is still flying from a knockback" — the dying body's proxy is destroyed while its enlarged flag is pending.

Root cause

The enlarged-flag lifecycle has a hole:

  1. b2FinalizeBodiesTask marks a fast/outgrown shape enlargedAABB = true; at the end of b2Solve, b2BroadPhase_EnlargeProxy sets b2_enlargedNode on the leaf's ancestors (propagated to the root) and buffers the move.
  2. The flags are cleared only by the tree rebuild (b2UpdateTreesTaskb2BroadPhase_RebuildTrees), which is enqueued from b2UpdateBroadPhasePairs — and only when moveArray.count > 0.
  3. b2BroadPhase_DestroyProxy calls b2UnBufferMove, removing the destroyed proxy's move entry. b2DynamicTree_DestroyProxy/b2RemoveLeaf recompute ancestor AABBs/heights but never their flags, so the orphaned b2_enlargedNode survives on the ancestors (the removed leaf's own parent is freed with it, so a shallow tree may not show this — the tree needs a few bodies for a grandparent to carry the flag).
  4. Next step: empty move buffer → b2UpdateBroadPhasePairs returns early → no rebuild → b2Solveb2ValidateNoEnlarged finds the stale flag → assert.

Note the assert fires on a flag that is otherwise benign: the tree still works, the flag merely means "this subtree wants a rebuild", and any later rebuild self-heals it. Upstream C Box2D only runs this validation under the opt-in B2_VALIDATE; the port runs it under plain DEBUG, which is why the port crashes where C wouldn't.

Minimal repro (DEBUG build)

using Box2D.NET;
using static Box2D.NET.B2Bodies;
using static Box2D.NET.B2Geometries;
using static Box2D.NET.B2Shapes;
using static Box2D.NET.B2Types;
using static Box2D.NET.B2Worlds;

var def = b2DefaultWorldDef();
def.gravity = new B2Vec2(0f, 0f);   // zero-G: still bodies never re-enlarge
                                    // (a floor makes resting bodies fidget past the
                                    // speculative margin every step, masking the hole)
def.enableContinuous = false;
var world = b2CreateWorld(def);

var bodies = new B2BodyId[8];       // enough bodies for a tree deep enough that the
for (int i = 0; i < bodies.Length; i++) // destroyed leaf's grandparent survives
{
    var bd = b2DefaultBodyDef();
    bd.type = B2BodyType.b2_dynamicBody;
    bd.position = new B2Vec2(i * 2f, 0f);
    bd.enableSleep = false;
    bodies[i] = b2CreateBody(world, bd);
    var sd = b2DefaultShapeDef();
    sd.density = 1f;
    b2CreatePolygonShape(bodies[i], sd, b2MakeBox(0.5f, 0.5f));
}

for (int i = 0; i < 10; i++) b2World_Step(world, 1f / 60f, 4);
b2Body_SetLinearVelocity(bodies[0], new B2Vec2(60f, 0f));
b2World_Step(world, 1f / 60f, 4);  // bodies[0] outgrows its fat AABB: enlarged + buffered
b2DestroyBody(bodies[0]);          // un-buffers the move; ancestor flags orphaned
b2World_Step(world, 1f / 60f, 4);  // empty move buffer → no rebuild → assert in
                                   // b2DynamicTree_ValidateNoEnlarged (B2DynamicTrees.cs)

Observed: System.InvalidOperationException: b2DynamicTree_ValidateNoEnlarged() B2DynamicTrees.cs on the step after the destroy.

Suggested fix

The enlarged flag always propagates to the root, so the root read answers "does any tree still owe a rebuild?" — two flag reads per otherwise-empty step:

// B2DynamicTrees.cs
/// True while any node carries the enlarged flag (it propagates to the root, so the
/// root read answers for the whole tree). b2RemoveLeaf recomputes ancestor AABBs but
/// never their flags, so a proxy destroyed between steps can orphan the flag.
internal static bool b2DynamicTree_HasEnlargedRoot(B2DynamicTree tree) =>
    tree.root != B2_NULL_INDEX &&
    (tree.nodes[tree.root].flags & (ushort)B2TreeNodeFlags.b2_enlargedNode) != 0;
// B2BroadPhases.cs, in b2UpdateBroadPhasePairs' moveCount == 0 early return:
if (moveCount == 0)
{
    // A proxy enlarged last step and destroyed before this one takes its move entry
    // with it; the enlarged flag orphaned on its ancestors is only cleared by a
    // rebuild. Without this, the next b2ValidateNoEnlarged in b2Solve trips.
    if (b2DynamicTree_HasEnlargedRoot(bp.trees[(int)B2BodyType.b2_dynamicBody]) ||
        b2DynamicTree_HasEnlargedRoot(bp.trees[(int)B2BodyType.b2_kinematicBody]))
        b2BroadPhase_RebuildTrees(bp);
    return;
}

With this patch the repro above passes, and the validator stays alive to catch real problems. An alternative would be gating the b2ValidateNoEnlarged calls behind an opt-in define (matching upstream C's B2_VALIDATE), but that silences the check instead of restoring the invariant.

Happy to open a PR with the patch if you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions