Repository: google/filament
File: filament/src/details/VertexBuffer.cpp (C++)

Code before:
        // use dummy buffers.
        // - A complication is that backends need to know if an attribute is declared as float or
        // integer in the shader, regardless of if the attribute is enabled or not in the
        // VertexBuffer (e.g. the morphing attributes could be disabled because we're only using
        // skinning).
        // - Another complication is that the SKN variant is selected by the renderable
        // (as opposed to the RenderPrimitive), so it's possible and valid for a primitive
        // that isn't skinned nor morphed to be rendered with the SKN variant (morphing/skinning
        // will then be disabled dynamically).
        //
        // Because of that we need to set FLAG_INTEGER_TARGET on all attributes that we know are
        // integer in the shader and the bottom line is that BONE_INDICES always needs to be set to
        // FLAG_INTEGER_TARGET.
        mAttributes[BONE_INDICES].flags |= Attribute::FLAG_INTEGER_TARGET;
    }

    FEngine::DriverApi& driver = engine.getDriverApi();

    mVertexBufferInfoHandle = engine.getVertexBufferInfoFactory().create(driver,
            mBufferCount, mDeclaredAttributes.count(), mAttributes);

    mHandle = driver.createVertexBuffer(mVertexCount, mVertexBufferInfoHandle,
            utils::ImmutableCString{ builder.getName() });

    auto shouldCreateBuffer = [this](size_t attributeIndex) {
        if (mAdvancedSkinningEnabled) {
            // For advanced skinning mode, only relevant buffers (BONE_INDICES & BONE_WEIGHTS) are
            // created. We already manually handled the data for those specific buffers above.
            if (attributeIndex == BONE_INDICES || attributeIndex == BONE_WEIGHTS) {
                return true;
            }
        }
        if (!mBufferObjectsEnabled) {
            return mDeclaredAttributes[attributeIndex] &&
                    mAttributes[attributeIndex].buffer != Attribute::BUFFER_UNUSED;
        }
        return false;
    };

    // calculate buffer sizes
    size_t bufferSizes[MAX_VERTEX_BUFFER_COUNT] = {};
    #pragma nounroll
    for (size_t i = 0, n = mAttributes.size(); i < n; ++i) {
        if (!shouldCreateBuffer(i)) {
            continue;
        }
        const uint32_t offset = mAttributes[i].offset;
        const uint8_t stride = mAttributes[i].stride;
        const uint8_t slot = mAttributes[i].buffer;
        const size_t end = offset + mVertexCount * stride;
        assert_invariant(slot < MAX_VERTEX_BUFFER_COUNT);

Review comment (suggestion):
this doesn't seem like it's equivalent to the previous?  Shouldn't it be
```
if (mBufferObjectsEnabled && mAdvancedSkinningEnabled) 
```

Code after:
        // use dummy buffers.
        // - A complication is that backends need to know if an attribute is declared as float or
        // integer in the shader, regardless of if the attribute is enabled or not in the
        // VertexBuffer (e.g. the morphing attributes could be disabled because we're only using
        // skinning).
        // - Another complication is that the SKN variant is selected by the renderable
        // (as opposed to the RenderPrimitive), so it's possible and valid for a primitive
        // that isn't skinned nor morphed to be rendered with the SKN variant (morphing/skinning
        // will then be disabled dynamically).
        //
        // Because of that we need to set FLAG_INTEGER_TARGET on all attributes that we know are
        // integer in the shader and the bottom line is that BONE_INDICES always needs to be set to
        // FLAG_INTEGER_TARGET.
        mAttributes[BONE_INDICES].flags |= Attribute::FLAG_INTEGER_TARGET;
    }

    FEngine::DriverApi& driver = engine.getDriverApi();

    mVertexBufferInfoHandle = engine.getVertexBufferInfoFactory().create(driver,
            mBufferCount, mDeclaredAttributes.count(), mAttributes);

    mHandle = driver.createVertexBuffer(mVertexCount, mVertexBufferInfoHandle,
            utils::ImmutableCString{ builder.getName() });

    // calculate buffer sizes
    size_t bufferSizes[MAX_VERTEX_BUFFER_COUNT] = {};

    auto shouldCreateBuffer = [this](size_t attributeIndex) {
        const uint8_t slot = mAttributes[attributeIndex].buffer;
        return mDeclaredAttributes[attributeIndex] && slot != Attribute::BUFFER_UNUSED &&
                !mBufferObjects[slot];
    };
    auto updateBufferSize = [&bufferSizes, this](size_t attributeIndex) {
        const uint32_t offset = mAttributes[attributeIndex].offset;
        const uint8_t stride = mAttributes[attributeIndex].stride;
        const uint8_t slot = mAttributes[attributeIndex].buffer;
        const size_t end = offset + mVertexCount * stride;
        assert_invariant(slot < MAX_VERTEX_BUFFER_COUNT);
        bufferSizes[slot] = std::max(bufferSizes[slot], end);
    };

    if (!mBufferObjectsEnabled) {
        #pragma nounroll
        for (size_t i = 0, n = mAttributes.size(); i < n; ++i) {
            if (shouldCreateBuffer(i)) {
                updateBufferSize(i);
            }
        }
    } else if (mAdvancedSkinningEnabled) {
        // For advanced skinning mode, only relevant buffers (BONE_INDICES & BONE_WEIGHTS) are
        // created. We manually populated the relevant attributes for those buffers above.