VK_EXT_legacy_vertex_attributes extension is not validated correctly

[Mr-222]

Problem Statement

As the Vulkan Documentation states , with the extension enabled, vertex attributes can be loaded from arbitrary buffer alignments. It allows binding a vertex buffer at offset=7 for a user with dynamic vertex input active. However, after enabling this extension, validation layer will still complain about the alignment issue.

Here is a simple example that you can use to reproduce this bug, I modified based on vertex_dynamic_state vulkan sample, I call vkCmdBindVertexBuffers with offset=7.

    /**
     *     @fn void VertexDynamicState::draw_created_model(VkCommandBuffer commandBuffer)
     *     @brief Drawing created model using indexing buffer
     */
    void VertexDynamicState::draw_created_model(VkCommandBuffer commandBuffer)
    {
        //VkDeviceSize offsets[1] = {0};
        VkDeviceSize offsets[1] = {7};
        vkCmdBindVertexBuffers(commandBuffer, 0, 1, cube.vertices->get(), offsets);
        vkCmdBindIndexBuffer(commandBuffer, cube.indices->get_handle(), 0, VK_INDEX_TYPE_UINT32);
        vkCmdDrawIndexed(commandBuffer, cube.index_count, 1, 0, 0, 0);
    }
    
    template<typename T>
    std::vector<uint8_t> add_nonsense_data_front(T array, int arraySizeBytes, int offsetBytes)
    {
        std::vector<uint8_t> newArray(arraySizeBytes + offsetBytes);
        for (int i = 0; i < offsetBytes; ++i)
            newArray[i] = static_cast<uint8_t>(i);
    
        memcpy(newArray.data() + offsetBytes, array.data(), arraySizeBytes);
    
        return newArray;
    }
    
    /**
     *  @fn void VertexDynamicState::model_data_creation()
     *  @brief Generate vertex input data for simple cube (position and normal vectors)
     */
    void VertexDynamicState::model_data_creation()
    {
        constexpr uint32_t                     vertex_count = 8;
        std::array<SampleVertex, vertex_count> vertices;
    
        vertices[0].pos = {0.0f, 0.0f, 0.0f};
        vertices[1].pos = {1.0f, 0.0f, 0.0f};
        vertices[2].pos = {1.0f, 1.0f, 0.0f};
        vertices[3].pos = {0.0f, 1.0f, 0.0f};
        vertices[4].pos = {0.0f, 0.0f, 1.0f};
        vertices[5].pos = {1.0f, 0.0f, 1.0f};
        vertices[6].pos = {1.0f, 1.0f, 1.0f};
        vertices[7].pos = {0.0f, 1.0f, 1.0f};
    
        /* Normalized normal vectors for each face of cube */
        glm::vec3 Xp = {1.0, 0.0, 0.0};
        glm::vec3 Xm = {-1.0, 0.0, 0.0};
        glm::vec3 Yp = {0.0, 1.0, 0.0};
        glm::vec3 Ym = {0.0, -1.0, 0.0};
        glm::vec3 Zp = {0.0, 0.0, 1.0};
        glm::vec3 Zm = {0.0, 0.0, -1.0};
    
        /* Normalized normal vectors for each vertex (created by sum of corresponding faces) */
        vertices[0].normal = glm::normalize(Xm + Ym + Zm);
        vertices[1].normal = glm::normalize(Xp + Ym + Zm);
        vertices[2].normal = glm::normalize(Xp + Yp + Zm);
        vertices[3].normal = glm::normalize(Xm + Yp + Zm);
        vertices[4].normal = glm::normalize(Xm + Ym + Zp);
        vertices[5].normal = glm::normalize(Xp + Ym + Zp);
        vertices[6].normal = glm::normalize(Xp + Yp + Zp);
        vertices[7].normal = glm::normalize(Xm + Yp + Zp);
    
        /* Scaling and position transform */
        for (uint8_t i = 0; i < vertex_count; i++)
        {
            vertices[i].pos *= glm::vec3(10.0f, 10.0f, 10.0f);
            vertices[i].pos -= glm::vec3(5.0f, 20.0f, 5.0f);
        }
    
        constexpr uint32_t index_count        = 36;
        uint32_t           vertex_buffer_size = vertex_count * sizeof(SampleVertex);
        uint32_t           index_buffer_size  = index_count * sizeof(uint32_t);
        cube.index_count                      = index_count;
    
        /* Array with vertices indexes for corresponding triangles */
        std::array<uint32_t, index_count> indices{0, 4, 3,
                                                  4, 7, 3,
                                                  0, 3, 2,
                                                  0, 2, 1,
                                                  1, 2, 6,
                                                  6, 5, 1,
                                                  5, 6, 7,
                                                  7, 4, 5,
                                                  0, 1, 5,
                                                  5, 4, 0,
                                                  3, 7, 6,
                                                  6, 2, 3};
    
        // Test legacy vertex attributes extension by adding 7 nonsense bytes data at the front of the vertex buffer
        vertex_buffer_size += 7;
        std::vector<uint8_t> newVertices    = add_nonsense_data_front(vertices, vertex_buffer_size - 7, 7);
        vkb::core::BufferC   vertex_staging = vkb::core::BufferC::create_staging_buffer(get_device(), newVertices);
    
        //vkb::core::BufferC vertex_staging = vkb::core::BufferC::create_staging_buffer(get_device(), vertices);
        vkb::core::BufferC index_staging  = vkb::core::BufferC::create_staging_buffer(get_device(), indices);
    
        cube.vertices = std::make_unique<vkb::core::BufferC>(get_device(),
                                                             vertex_buffer_size,
                                                             VK_BUFFER_USAGE_VERTEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT,
                                                             VMA_MEMORY_USAGE_GPU_ONLY);
    
        cube.indices = std::make_unique<vkb::core::BufferC>(get_device(),
                                                            index_buffer_size,
                                                            VK_BUFFER_USAGE_INDEX_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT,
                                                            VMA_MEMORY_USAGE_GPU_ONLY);
    
        /* Copy from staging buffers */
        VkCommandBuffer copy_command = get_device().create_command_buffer(VK_COMMAND_BUFFER_LEVEL_PRIMARY, true);
    
        VkBufferCopy copy_region = {};
    
        copy_region.size = vertex_buffer_size;
        vkCmdCopyBuffer(
            copy_command,
            vertex_staging.get_handle(),
            cube.vertices->get_handle(),
            1,
            &copy_region);
    
        copy_region.size = index_buffer_size;
        vkCmdCopyBuffer(
            copy_command,
            index_staging.get_handle(),
            cube.indices->get_handle(),
            1,
            &copy_region);
    
        get_device().flush_command_buffer(copy_command, queue, true);
    }
    
    VertexDynamicState::VertexDynamicState()
    {
        title = "Vertex Dynamic State";
    
        set_api_version(VK_API_VERSION_1_3);
        add_instance_extension(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME);
        add_device_extension(VK_EXT_VERTEX_INPUT_DYNAMIC_STATE_EXTENSION_NAME);
        add_device_extension(VK_EXT_LEGACY_VERTEX_ATTRIBUTES_EXTENSION_NAME);
    }
    
    void VertexDynamicState::request_gpu_features(vkb::PhysicalDevice &gpu)
    {
        /* Enable extension features required by this sample
        These are passed to device creation via a pNext structure chain */
        auto &requested_vertex_input_features = gpu.request_extension_features<VkPhysicalDeviceVertexInputDynamicStateFeaturesEXT>(VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VERTEX_INPUT_DYNAMIC_STATE_FEATURES_EXT);
        requested_vertex_input_features.vertexInputDynamicState = VK_TRUE;
    
        //gpu.get_mutable_requested_features().robustBufferAccess = VK_TRUE;
    
        auto &requested_legacy_vertex_attributes_features = gpu.request_extension_features<VkPhysicalDeviceLegacyVertexAttributesFeaturesEXT>(VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_LEGACY_VERTEX_ATTRIBUTES_FEATURES_EXT);
        requested_legacy_vertex_attributes_features.legacyVertexAttributes = VK_TRUE;
    
        if (gpu.get_features().samplerAnisotropy)
        {
            gpu.get_mutable_requested_features().samplerAnisotropy = true;
        }
    }

After that, the validation layer would complain:
[error] 615493573 - VUID-vkCmdDrawIndexed-None-02721: Validation Error: [ VUID-vkCmdDrawIndexed-None-02721 ] Object 0: handle = 0x7323f50000000048, type = VK_OBJECT_TYPE_BUFFER; Object 1: handle = 0x72303f0000000052, type = VK_OBJECT_TYPE_PIPELINE; | MessageID = 0x24afafc5 | vkCmdDrawIndexed():  Format VK_FORMAT_R32G32B32_SFLOAT has an alignment of 4 but the alignment of attribAddress (43) is not aligned in pVertexAttributeDescriptions[0](binding=0 location=0) where attribAddress = vertex buffer offset (7) + binding stride (36) + attribute offset (0). The Vulkan spec states: If robustBufferAccess is not enabled, and that pipeline was created without enabling VK_PIPELINE_ROBUSTNESS_BUFFER_BEHAVIOR_ROBUST_BUFFER_ACCESS_EXT for vertexInputs, then for a given vertex buffer binding, any attribute data fetched must be entirely contained within the corresponding vertex buffer binding, as described in Vertex Input Description (https://vulkan.lunarg.com/doc/view/1.3.290.0/windows/1.3-extensions/vkspec.html#VUID-vkCmdDrawIndexed-None-02721)

Possible cause

In layers/cc_pipeline_graphics.cpp, line 4104, function ValidateDrawPipelineVertexAttribute doesn't check if feature VkPhysicalDeviceLegacyVertexAttributesFeaturesEXT is enabled, it judges the address of vertex attribute aligned or not. If not, log an error.

bool CoreChecks::ValidateDrawPipelineVertexAttribute(const vvl::CommandBuffer &cb_state, const vvl::Pipeline &pipeline,
                                                     const vvl::DrawDispatchVuid &vuid) const {
    ...
    // Verify vertex attribute address alignment
    for (uint32_t i = 0; i < vertex_attribute_descriptions.size(); i++) {
        ...

        if (SafeModulo(attrib_address, vtx_attrib_req_alignment) != 0) {
            const LogObjectList objlist(buffer_state->Handle(), pipeline.Handle());
            skip |= LogError(vuid.vertex_binding_attribute_02721, objlist, vuid.loc(),
                             "Format %s has an alignment of %" PRIu64 " but the alignment of attribAddress (%" PRIu64
                             ") is not aligned in pVertexAttributeDescriptions[%" PRIu32
                             "]"
                             "(binding=%" PRIu32 " location=%" PRIu32 ") where attribAddress = vertex buffer offset (%" PRIu64
                             ") + binding stride (%" PRIu64 ") + attribute offset (%" PRIu64 ").",
                             string_VkFormat(attribute_format), vtx_attrib_req_alignment, attrib_address, i, vertex_binding,
                             attribute_description.location, vertex_buffer_offset, vertex_buffer_stride, attribute_offset);
        }
    }
    return skip;
}

[spencer-lunarg]

thanks for reporting, I added this in #7978

I will have time next week to come back around and take a look at this so we can have a fix by the next SDK release!

[Mr-222]

Reformatted the codeblock in my description to make it clear. Take your time, I'm in no hurry at all about this. Thank you Spencer!