Vulkan-ValidationLayers icon indicating copy to clipboard operation
Vulkan-ValidationLayers copied to clipboard

vkUpdateDescriptorSetWithTemplate missing validation on Acceleration structure descriptor information

Open MarkY-LunarG opened this issue 1 year ago • 2 comments

I created a sample in a fork of Sascha Willem's Vulkan samples and modified the RayTracingReflections sample to use Descriptor templates. When I did this, I was not passing in the acceleration structure data correctly, and validation did not throw any error.s. The new sample is called raytracingreflectionsdesctemplates.

To reproduce, download the source, and modify the file examples/raytracingreflectionsdesctemplates/raytracingreflectionsdesctemplates.cpp so that the following line is garbage instead of being set to topLevelAS.handle.

descriptorUpdateData.acceleration = topLevelAS.handle;

This should produce an error that the handle isn't valid. I'm not sure what the VUID should be (sorry).

MarkY-LunarG avatar Feb 07 '24 17:02 MarkY-LunarG

  1. where is this raytracingreflectionsdesctemplates example? Do you have a fork and branch to look at?
  2. I think the VU is VUID-vkUpdateDescriptorSetWithTemplate-pData-01685

spencer-lunarg avatar Feb 08 '24 13:02 spencer-lunarg

Weird, I thought I had put the source location in there...

[My Fork of Sascha's Vulkan samples)[https://github.com/MarkY-LunarG/Vulkan] Branch: marky_move_raytrace_reflect_to_desc_templates

MarkY-LunarG avatar Feb 08 '24 14:02 MarkY-LunarG

I believe this has been fixed, I am seeing VUID-vkCmdTraceRaysKHR-None-08114 if I modify the example as you have said @MarkY-LunarG . Do you want to verify?

ziga-lunarg avatar Dec 20 '24 21:12 ziga-lunarg

ya, I forgot I did rework a lot of this stuff implicitly when doing GPU-AV, so glad it seems to work... I guess what would be ideal is a test in VVL just say "this issue is closed"

spencer-lunarg avatar Dec 21 '24 00:12 spencer-lunarg

This is maybe not complete yet. I have a test below. The issue is that if you use regular vkUpdateDescriptorSets it will immediately report that the VkAccelerationStructureKHR is not valid, this is done in object_tracker_utils.cpp. However similar to #9069, the checks for vkUpdateDescriptorSetWithTemplateKHR, can only be done in CoreChecks, so validation for this would have to be moved. But moving object_tracker_utils code to core_checks does not go cleanly.

TEST_F(NegativeRayTracing, UpdateDescriptorSetsWithTemplateInvalidAccelerationStructure) {
    TEST_DESCRIPTION(
        "Setup a ray tracing pipeline (ray generation, miss and closest hit shaders) and acceleration structure, and trace one "
        "ray. Only call traceRay in the ray generation shader");

    SetTargetApiVersion(VK_API_VERSION_1_2);
    AddRequiredExtensions(VK_KHR_DESCRIPTOR_UPDATE_TEMPLATE_EXTENSION_NAME);
    AddRequiredFeature(vkt::Feature::rayTracingPipeline);
    AddRequiredFeature(vkt::Feature::accelerationStructure);
    AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
    RETURN_IF_SKIP(InitFrameworkForRayTracingTest());
    RETURN_IF_SKIP(InitState());

    vkt::rt::Pipeline pipeline(*this, m_device);

    // Set shaders

    const char *ray_gen = R"glsl(
        #version 460
        #extension GL_EXT_ray_tracing : require // Requires SPIR-V 1.5 (Vulkan 1.2)

        layout(binding = 0, set = 0) uniform accelerationStructureEXT tlas;

        layout(location = 0) rayPayloadEXT vec3 hit;

        void main() {
            traceRayEXT(tlas, gl_RayFlagsOpaqueEXT, 0xff, 0, 0, 0, vec3(0,0,1), 0.1, vec3(0,0,1), 1000.0, 0);
        }
    )glsl";
    pipeline.SetGlslRayGenShader(ray_gen);

    const char *miss = R"glsl(
        #version 460
        #extension GL_EXT_ray_tracing : require

        layout(location = 0) rayPayloadInEXT vec3 hit;

        void main() {
            hit = vec3(0.1, 0.2, 0.3);
        }
    )glsl";
    pipeline.AddGlslMissShader(miss);

    const char *closest_hit = R"glsl(
        #version 460
        #extension GL_EXT_ray_tracing : require

        layout(location = 0) rayPayloadInEXT vec3 hit;
        hitAttributeEXT vec2 baryCoord;

        void main() {
            const vec3 barycentricCoords = vec3(1.0f - baryCoord.x - baryCoord.y, baryCoord.x, baryCoord.y);
            hit = barycentricCoords;
        }
    )glsl";
    pipeline.AddGlslClosestHitShader(closest_hit);


    pipeline.AddBinding(VK_DESCRIPTOR_TYPE_ACCELERATION_STRUCTURE_KHR, 0);
    pipeline.CreateDescriptorSet();

    VkDescriptorUpdateTemplateEntry descriptor_update_template_entry;
    descriptor_update_template_entry.dstBinding = 0u;
    descriptor_update_template_entry.dstArrayElement = 0u;
    descriptor_update_template_entry.descriptorCount = 1u;
    descriptor_update_template_entry.descriptorType = VK_DESCRIPTOR_TYPE_ACCELERATION_STRUCTURE_KHR;
    descriptor_update_template_entry.offset = 0u;
    descriptor_update_template_entry.stride = 0u;

    VkDescriptorUpdateTemplateCreateInfo template_create_info = vku::InitStructHelper();
    template_create_info.pNext = nullptr;
    template_create_info.flags = 0;
    template_create_info.descriptorUpdateEntryCount = 1;
    template_create_info.pDescriptorUpdateEntries = &descriptor_update_template_entry;
    template_create_info.templateType = VK_DESCRIPTOR_UPDATE_TEMPLATE_TYPE_DESCRIPTOR_SET;
    template_create_info.descriptorSetLayout = pipeline.GetDescriptorSet().layout_;
    template_create_info.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS;
    template_create_info.pipelineLayout = VK_NULL_HANDLE;
    template_create_info.set = 0;

    vkt::DescriptorUpdateTemplate descriptorUpdateTemplate(*m_device, template_create_info);

    struct DescriptorUpdateData {
        VkAccelerationStructureKHR acceleration;
    } descriptorUpdateData;

    {
        vkt::as::BuildGeometryInfoKHR tlas(
            vkt::as::blueprint::BuildOnDeviceTopLevel(*m_device, *m_default_queue, m_command_buffer));
        descriptorUpdateData.acceleration = tlas.GetDstAS()->handle(); 
    }
    vk::UpdateDescriptorSetWithTemplateKHR(device(), pipeline.GetDescriptorSet().set_, descriptorUpdateTemplate,
                                           &descriptorUpdateData);

    // Build pipeline
    pipeline.Build();

    // Bind descriptor set, pipeline, and trace rays
    m_command_buffer.Begin();
    vk::CmdBindDescriptorSets(m_command_buffer, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR, pipeline.GetPipelineLayout(), 0, 1,
                              &pipeline.GetDescriptorSet().set_, 0, nullptr);
    vk::CmdBindPipeline(m_command_buffer, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR, pipeline.Handle());
    vkt::rt::TraceRaysSbt trace_rays_sbt = pipeline.GetTraceRaysSbt();
    vk::CmdTraceRaysKHR(m_command_buffer, &trace_rays_sbt.ray_gen_sbt, &trace_rays_sbt.miss_sbt, &trace_rays_sbt.hit_sbt,
                        &trace_rays_sbt.callable_sbt, 1, 1, 1);
    m_command_buffer.End();
    m_default_queue->Submit(m_command_buffer);
    m_device->Wait();
}

ziga-lunarg avatar Dec 22 '24 14:12 ziga-lunarg