Is the depth estimate in the AABBPruner::buildStep function inaccurate?

[allengx]

I understand that the mBuilder.mNbPrimitives are not the same as the number of leaf nodes：

else if(mProgress==BUILD_INIT)
{

    // - Assume new tree with n leaves is perfectly-balanced
    // - Compute the depth of perfectly-balanced tree with n leaves
    // - Estimate number of working units for the new tree
        // ...
    const PxU32 depth = Ps::ilog2(mBuilder.mNbPrimitives);  // Note: This is the depth without counting the leaf layer
    // ...
}

Here seems to assume that the AABBTreeBuildParams.mLimit = 1。

AABBTreeBuildParams(PxU32 limit = 1, PxU32 nb_prims = 0, const PxBounds3* boxes = NULL) :
                mLimit(limit), mNbPrimitives(nb_prims), mAABBArray(boxes), mCache(NULL) {}

but the actual use is NB_OBJECTS_PER_NODE it is 4：

#define NB_OBJECTS_PER_NODE 4

bool AABBPruner::prepareBuild()
{
    PX_PROFILE_ZONE("SceneQuery.prepareBuild", mContextID);

    PX_ASSERT(mIncrementalRebuild);
    if(mNeedsNewTree)
    {
        if(mProgress==BUILD_NOT_STARTED)
        {
                        // ....
            mBuilder.mLimit         = NB_OBJECTS_PER_NODE;
                        // ...
        }
    }
    else
        return false;

    return true;
}

I don't know how much of an impact this will have on productivity, because I haven't done systematic testing. But consider adding mLimit to the depth calculation as well.

[PierreTerdiman]

It is indeed just an estimate, and by nature it is not accurate indeed. Not just because of the number of objects per leaf, but also because the code "assumes new tree with n leaves is perfectly balanced" - which is typically not the case.

We could try to make it more accurate but it basically would not change anything. The estimate only affects how many frames it takes to rebuild a new tree, when used in conjunction with the "dynamicTreeRebuildRateHint" user-defined parameter. Users can already tweak this the way they need, and changing that code would silently break those existing tweaks. That would not be a good move...