Repository: google/filament
File: filament/backend/src/webgpu/WebGPUHandles.cpp (C++)

Code before:
        case ElementType::USHORT2: return integer ? VertexFormat::Uint16x2 : VertexFormat::Float32x2;
        case ElementType::HALF2: return VertexFormat::Float16x2;
        case ElementType::FLOAT2: return VertexFormat::Float32x2;
        // Three Component Types
        case ElementType::BYTE3: return VertexFormat::Sint8x4;    // NOT MINSPEC
        case ElementType::UBYTE3: return VertexFormat::Uint8x4;   // NOT MINSPEC
        case ElementType::SHORT3: return VertexFormat::Sint16x4;  // NOT MINSPEC
        case ElementType::USHORT3: return VertexFormat::Uint16x4; // NOT MINSPEC
        case ElementType::HALF3: return VertexFormat::Float16x4;  // NOT MINSPEC
        case ElementType::FLOAT3: return VertexFormat::Float32x3;
        // Four Component Types
        case ElementType::BYTE4: return integer ? VertexFormat::Sint8x4 : VertexFormat::Float32x4;
        case ElementType::UBYTE4: return integer ? VertexFormat::Uint8x4 : VertexFormat::Float32x4;
        case ElementType::SHORT4: return integer ? VertexFormat::Sint16x4 : VertexFormat::Float32x4;
        case ElementType::USHORT4: return integer ? VertexFormat::Uint16x4 : VertexFormat::Float32x4;
        case ElementType::HALF4: return VertexFormat::Float16x4;
        case ElementType::FLOAT4: return VertexFormat::Float32x4;
    }
}

} // namespace

namespace filament::backend {

WGPUVertexBufferInfo::WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
        AttributeArray const& attributes)
    : HwVertexBufferInfo(bufferCount, attributeCount),
      mVertexBufferLayout(bufferCount),
      mAttributes(bufferCount) {
    for (uint32_t attribIndex = 0; attribIndex < attributes.size(); attribIndex++) {
        Attribute attrib = attributes[attribIndex];
        // Ignore the attributes which are not bind to vertex buffers.
        if (attrib.buffer == Attribute::BUFFER_UNUSED) {
            continue;
        }

        assert_invariant(attrib.buffer < bufferCount);
        bool const isInteger = attrib.flags & Attribute::FLAG_INTEGER_TARGET;
        bool const isNormalized = attrib.flags & Attribute::FLAG_NORMALIZED;
        wgpu::VertexFormat vertexFormat = getVertexFormat(attrib.type, isNormalized, isInteger);

        mAttributes[attrib.buffer].push_back({
            .format = vertexFormat,
            .offset = attrib.offset,
            .shaderLocation = attribIndex,
        });
        mVertexBufferLayout[attrib.buffer] = {
            .arrayStride = attrib.stride,
        };
    }


Review comment (bug):
@sidreesshah I have been studying this code and working with in the scope of what I am doing, which is building a render pipeline. I think this implementation may not be getting it right.

The idea is only 2 collections of are needed, the attributes and the buffer layouts, as each attribute belongs to exactly 1 (or 0) buffer layout. Thus, I sort the vertex attributes first by buffer index, then by offset, so that they all fit nicely in one collection. Then, I build up the vertex buffer layouts by addressing where each contiguous groups of attributes for that buffer start, count the attributes, and add up the strides for the overall stride of of the vertex buffer layout. At least, this model is what makes sense to me. Each attribute can belong to one buffer. There can only be so many attributes. Each buffer can have multiple attributes. The unused attributes are again ignored.

This is what I have:
```cpp
struct WGPUVertexBufferInfo : public HwVertexBufferInfo {
    WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
            AttributeArray const& attributes);

    std::array<wgpu::VertexAttribute, MAX_VERTEX_ATTRIBUTE_COUNT> vertexAttributes = {};
    std::array<wgpu::VertexBufferLayout, MAX_VERTEX_BUFFER_COUNT> vertexBufferLayouts = {};
};
```

```cpp
WGPUVertexBufferInfo::WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
        AttributeArray const& attributes)
    : HwVertexBufferInfo(bufferCount, attributeCount) {
    if (bufferCount == 0 || attributeCount == 0) {
        HwVertexBufferInfo::bufferCount = 0;
        HwVertexBufferInfo::attributeCount = 0;
        return;
    }
    assert_invariant(attributes.size() >= attributeCount);
    assert_invariant(attributeCount <= MAX_VERTEX_ATTRIBUTE_COUNT);
    assert_invariant(bufferCount <= MAX_VERTEX_BUFFER_COUNT);
    // sort attributes first by buffer index, then by offset and keep original index
    std::array<std::pair<Attribute const*, uint8_t>, MAX_VERTEX_ATTRIBUTE_COUNT>
            attributesWithIndex{};
    for (size_t i = 0; i < attributes.size(); i++) {
        attributesWithIndex[i] = std::pair{ &attributes[i], i };
    }
    std::sort(attributesWithIndex.begin(), attributesWithIndex.end(),
            [](auto const& first, auto const& second) {
                auto firstAttr = first.first;
                auto secondAttr = second.first;
                if (firstAttr->buffer < secondAttr->buffer) {
                    return true;// buffer index in increasing order
                } else if (firstAttr->buffer > secondAttr->buffer) {
                    return false;
                }
                // same buffer index...
                if (firstAttr->offset < secondAttr->offset) {
                    return true;// offsets in increasing order by buffer
                } else if (firstAttr->offset > secondAttr->offset) {
                    return false;
                } else if (firstAttr->buffer == Attribute::BUFFER_UNUSED) {
                    return true;// don't care
                } else {
                    assert_invariant(false);// should not be possible to have multiple attributes
                                            // with the same value buffer index and offset
                    return true;
                }
            });
    uint8_t bufferIndex = 0;
    // make sure the first sorted attribute starts at buffer 0
    assert_invariant(attributesWithIndex[0].first->buffer == bufferIndex);
    for (size_t sortedIndex = 0; sortedIndex < attributes.size(); sortedIndex++) {
        const auto [attribute, attrIndex] = attributesWithIndex[sortedIndex];
        if (attribute->buffer == Attribute::BUFFER_UNUSED) {
            HwVertexBufferInfo::attributeCount = sortedIndex;
            break;
        }
        wgpu::VertexAttribute& vAttribute = vertexAttributes[sortedIndex];
        if (attribute->buffer > bufferIndex) {
            bufferIndex++;
            // make sure each buffer index increases by 1
            assert_invariant(bufferIndex == attribute->buffer);
        }
        vAttribute.format =
                getVertexFormat(attribute->type, attribute->flags & Attribute::FLAG_NORMALIZED,
                        attribute->flags & Attribute::FLAG_INTEGER_TARGET);
        vAttribute.offset = attribute->offset;
        vAttribute.shaderLocation = attrIndex;
        wgpu::VertexBufferLayout& vbLayout = vertexBufferLayouts[attribute->buffer];
        if (vbLayout.attributes == nullptr) {
            vbLayout.attributes = &vAttribute;
            vbLayout.attributeCount = 0;
            vbLayout.arrayStride = 0;
            vbLayout.stepMode = wgpu::VertexStepMode::Vertex;
        }
        vbLayout.attributeCount++;
        vbLayout.arrayStride += attribute->stride;
    }
    HwVertexBufferInfo::bufferCount = bufferIndex + 1;
}
```


Code after:
        case ElementType::SHORT2: return integer ? VertexFormat::Sint16x2 : VertexFormat::Float32x2;
        case ElementType::USHORT2: return integer ? VertexFormat::Uint16x2 : VertexFormat::Float32x2;
        case ElementType::HALF2: return VertexFormat::Float16x2;
        case ElementType::FLOAT2: return VertexFormat::Float32x2;
        // Three Component Types
        case ElementType::BYTE3: return VertexFormat::Sint8x4;    // NOT MINSPEC
        case ElementType::UBYTE3: return VertexFormat::Uint8x4;   // NOT MINSPEC
        case ElementType::SHORT3: return VertexFormat::Sint16x4;  // NOT MINSPEC
        case ElementType::USHORT3: return VertexFormat::Uint16x4; // NOT MINSPEC
        case ElementType::HALF3: return VertexFormat::Float16x4;  // NOT MINSPEC
        case ElementType::FLOAT3: return VertexFormat::Float32x3;
        // Four Component Types
        case ElementType::BYTE4: return integer ? VertexFormat::Sint8x4 : VertexFormat::Float32x4;
        case ElementType::UBYTE4: return integer ? VertexFormat::Uint8x4 : VertexFormat::Float32x4;
        case ElementType::SHORT4: return integer ? VertexFormat::Sint16x4 : VertexFormat::Float32x4;
        case ElementType::USHORT4: return integer ? VertexFormat::Uint16x4 : VertexFormat::Float32x4;
        case ElementType::HALF4: return VertexFormat::Float16x4;
        case ElementType::FLOAT4: return VertexFormat::Float32x4;
    }
}

}// namespace

namespace filament::backend {

WGPUVertexBufferInfo::WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
        AttributeArray const& attributes)
    : HwVertexBufferInfo(bufferCount, attributeCount),
      mVertexBufferLayout(bufferCount),
      mAttributes(bufferCount) {
    assert_invariant(attributeCount > 0);
    assert_invariant(bufferCount > 0);
    for (uint32_t attribIndex = 0; attribIndex < attributes.size(); attribIndex++) {
        Attribute const& attrib = attributes[attribIndex];
        // Ignore the attributes which are not bind to vertex buffers.
        if (attrib.buffer == Attribute::BUFFER_UNUSED) {
            continue;
        }

        assert_invariant(attrib.buffer < bufferCount);
        bool const isInteger = attrib.flags & Attribute::FLAG_INTEGER_TARGET;
        bool const isNormalized = attrib.flags & Attribute::FLAG_NORMALIZED;
        wgpu::VertexFormat vertexFormat = getVertexFormat(attrib.type, isNormalized, isInteger);

        // Attributes are sequential per buffer
        mAttributes[attrib.buffer].push_back({
            .format = vertexFormat,
            .offset = attrib.offset,
            .shaderLocation = static_cast<uint32_t>(mAttributes[attrib.buffer].size()),
        });