Wrapper get_latency_timings of VK_NV_low_latency2 is incorrect

[VZout]

The get_latency_timings for this extension (added in #802) is a bit odd. it takes a single VkGetLatencyMarkerInfoNV as a pointer. and that struct has a number of timings. the count variable returns the size of ptimings NOT the amount of VkGetLatencyMarkerInfoNV's to pass in.

currently in ash the function signature is this:

    pub unsafe fn get_latency_timings(
        &self,
        swapchain: vk::SwapchainKHR,
        latency_marker_info: &mut [vk::GetLatencyMarkerInfoNV<'_>],
    )

But it shouldn't take a slice of latency_marker_info but instead a slice of VkLatencyTimingsFrameReportNV that is put inside the ptimings variable of GetLatencyMarkerInfoNV

This is my manual impl i wrote for testing:

            // get ptimings count.
            let mut marker = vk::GetLatencyMarkerInfoNV::default();
            let mut count = 0;
            (self
                .device
                .extensions()
                .low_latency2
                .fp()
                .get_latency_timings_nv)(
                self.device.extensions().low_latency2.device(),
                self.swapchain,
                &mut count,
                (&mut marker) as *mut _,
            );

            // alloc ptimings array initialized to default.
            let mut timings = vec![vk::LatencyTimingsFrameReportNV::default(); count as usize];
            marker.p_timings = timings.as_mut_ptr();

            // actually get the timings.
            if count > 0 {
                (self
                    .device
                    .extensions()
                    .low_latency2
                    .fp()
                    .get_latency_timings_nv)(
                    self.device.extensions().low_latency2.device(),
                    self.swapchain,
                    &mut count,
                    (&mut marker) as *mut _,
                );
            }

[MarijnS95]

That is quite possible because there was no way to test this extension (a driver update landed 2 days ago that enables it on my laptop 😥), and I believe the documentation wasn't up to snuff either.

Will check it out and come up with a solution, thanks for reporting!

[VZout]

Yeah the documentation is a bit tricky to read :/
Thanks!

[MarijnS95]

Yikes, this is a really dirty API: they should have just had the count value inside vk::GetLatencyMarkerInfoNV.

Likewise both vk::GetLatencyMarkerInfoNV and vk::LatencyTimingsFrameReportNV have sType/pNext, forcing us to be flexible and accept an argument as &mut.

EDIT: There's also no VkResult to signal INCOMPLETE, if count changes between runs...

[MarijnS95]

Aaah it's even worse! Because vk::GetLatencyMarkerInfoNV does not have a count member, our generator does not know that it's a pointer to an array and hence turns the builder function for it into a single element.

Let me file an issue upstream while typing in yet more workarounds for this poor API design :(

[repi]

glad you filed a upstream issue on it, I'll share it with our Nvidia contacts as well that we've been talking to about this (otherwise good) extension

[MarijnS95]

glad you filed a upstream issue on it, I'll share it with our Nvidia contacts as well that we've been talking to about this (otherwise good) extension

Thanks, all this collaboration paid off really quickly: KhronosGroup/Vulkan-Docs#2269 (comment)