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

Code before:

struct WGPUBufferObject;

// VertexBufferInfo contains layout info for Vertex Buffer based on WebGPU structs. In WebGPU each
// VertexBufferLayout is associated with a single vertex buffer. So number of mVertexBufferLayout
// is equal to bufferCount. Each VertexBufferLayout can contain multiple VertexAttribute. Bind index
// of vertex buffer is implicitly calculated by the position of VertexBufferLayout in an array.
struct WGPUVertexBufferInfo : public HwVertexBufferInfo {

    WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
            AttributeArray const& attributes);
    inline  wgpu::VertexBufferLayout const* getVertexBufferLayout() const {
        return mVertexBufferLayout.data();
    }
    inline uint32_t getVertexBufferLayoutSize() const {
        return mVertexBufferLayout.size();
    }

    inline wgpu::VertexAttribute const* getVertexAttributeForIndex(uint32_t index) const {
        return mAttributes[index].data();
    }
    inline uint32_t getVertexAttributeSize(uint32_t index) const {
        return mAttributes[index].size();
    }
private:
    std::vector<wgpu::VertexBufferLayout> mVertexBufferLayout;
    std::vector<std::vector<wgpu::VertexAttribute>> mAttributes;
};

struct WGPUVertexBuffer : public HwVertexBuffer {
    WGPUVertexBuffer(wgpu::Device const &device, uint32_t vextexCount, uint32_t bufferCount,
                     Handle<WGPUVertexBufferInfo> vbih);

    void setBuffer(WGPUBufferObject *bufferObject, uint32_t index);

    Handle<WGPUVertexBufferInfo> vbih;
    utils::FixedCapacityVector<wgpu::Buffer> buffers;
};

struct WGPUIndexBuffer : public HwIndexBuffer {
    WGPUIndexBuffer(wgpu::Device const &device, uint8_t elementSize,
                    uint32_t indexCount);

    wgpu::Buffer buffer;
};

// TODO: Currently WGPUBufferObject is not used by WebGPU for useful task.
// Update the struct when used by WebGPU driver.
struct WGPUBufferObject : HwBufferObject {
    WGPUBufferObject(BufferObjectBinding bindingType, uint32_t byteCount);


Review comment (performance):
I don't think we should use a `std::vector`, as we don't expect these to dynamically change size over time, right? I would expect them to be immutable, in which case, we should either use `FixedCapacityVector<...>` or `std::array` _(or even just a C-style array)_. `FixedCapacityVector` has the disadvantage of requiring another heap allocation, which is always a risk _(locking etc. and leading up to 50ms initializations in rare cases, as mentioned Mathias)_, but, of course, the advantage of more efficient memory usage. `std::array` is a less efficient memory usage option, but avoids heap allocations. The array option is not bad if we know there is a relatively low limit on the possible maximum size of the array. See examples, such as [filament/backend/src/vulkan/VulkanPipelineCache.h ~line 157](https://github.com/google/filament/blob/21dd1319dfd350adb8bba9990e5779d1b3639a44/filament/backend/src/vulkan/VulkanPipelineCache.h#L157). We know we have a `MAX_VERTEX_ATTRIBUTE_COUNT` _(16)_ and `MAX_VERTEX_BUFFER_COUNT` _(16)_ _(defined in [filament/backend/include/backend/DriverEnums.h ~line 99](https://github.com/google/filament/blob/21dd1319dfd350adb8bba9990e5779d1b3639a44/filament/backend/include/backend/DriverEnums.h#L99-L101))_. So, which route we take is not clear to me. We can go one way, see how it goes, and always change. But, my point is that `std::vector` or even `FixedCapacityVector` is not necessarily the clearly correct choice here.


Code after:
namespace filament::backend {

class WGPUProgram final : public HwProgram {
public:
    WGPUProgram(wgpu::Device&, Program&);

    wgpu::ShaderModule vertexShaderModule = nullptr;
    wgpu::ShaderModule fragmentShaderModule = nullptr;
    wgpu::ShaderModule computeShaderModule = nullptr;
    std::vector<wgpu::ConstantEntry> constants;
};

struct WGPUBufferObject;

// VertexBufferInfo contains layout info for Vertex Buffer based on WebGPU structs. In WebGPU each
// VertexBufferLayout is associated with a single vertex buffer. So number of mVertexBufferLayout
// is equal to bufferCount. Each VertexBufferLayout can contain multiple VertexAttribute. Bind index
// of vertex buffer is implicitly calculated by the position of VertexBufferLayout in an array.
class WGPUVertexBufferInfo : public HwVertexBufferInfo {
public:
    WGPUVertexBufferInfo(uint8_t bufferCount, uint8_t attributeCount,
            AttributeArray const& attributes);
    inline  wgpu::VertexBufferLayout const* getVertexBufferLayout() const {
        return mVertexBufferLayout.data();
    }

    inline uint32_t getVertexBufferLayoutSize() const {
        return mVertexBufferLayout.size();
    }

    inline wgpu::VertexAttribute const* getVertexAttributeForIndex(uint32_t index) const {
        assert_invariant(index < mAttributes.size());
        return mAttributes[index].data();
    }

    inline uint32_t getVertexAttributeSize(uint32_t index) const {
        assert_invariant(index < mAttributes.size());
        return mAttributes[index].size();
    }

private:
    // TODO: can we do better in terms on heap management.
    std::vector<wgpu::VertexBufferLayout> mVertexBufferLayout {};
    std::vector<std::vector<wgpu::VertexAttribute>> mAttributes {};
};

struct WGPUVertexBuffer : public HwVertexBuffer {
    WGPUVertexBuffer(wgpu::Device const &device, uint32_t vertexCount, uint32_t bufferCount,
                     Handle<HwVertexBufferInfo> vbih);

    Handle<HwVertexBufferInfo> vbih;