extensions/nv/low_latency2: Use count parameter for p_timings "array" member

[MarijnS95]

Fixes #814, CC @repi @VZout

As it turns out this extension has a rather suboptimal layout where the vk::GetLatencyMarkerInfoNV struct is not aware that its own p_timings field is an array, because it does not have a count field. Instead the length of this array is passed around via the pTimingCount pointer parameter of get_latency_timings().

A couple things are needed to set this straight:

1. The struct builder must be adjusted to accept a slice (without storing the length anywhere...);
2. The get_latency_timings() wrapper function should no longer accept a mutable slice of vk::GetLatencyMarkerInfoNV, but instead accept a single object that contains a pointer to the array (and p_next...);
3. This function must have a separate parameter to pass the length of the p_timings slice mentioned at 1., as we cannot reconstruct it from vk::GetLatencyMarkerInfoNV nor via its p_timings pointer;
4. The mutable struct and initialized p_timings array must still be passed fully by the user because both vk::GetLatencyMarkerInfoNV and vk::LatencyTimingsFrameReportNV have a p_next field that the user could initialize to point to some extension structure that they desire to be filled in. This info might also be relevant in the _len() query;
5. Asserts have been put in place to ensure correct use of this API with regards to p_timings being (not) NULL for querying the count or actual values, respectively.

[MarijnS95]

If it wasn't for p_next in vk::GetLatencyMarkerInfoNV we could have hidden away the construction of that type. And if it weren't for p_next in vk::LatencyTimingsFrameReportNV we could have done the allocation in a helper function and hid all this away and just return a Vec<vk::LatencyTimingsFrameReportNV<'_>>...

[MarijnS95]

It seems all lifetime-fu has escaped me; I can no longer use the slice passed as timings here even after self is dropped. Notably this is the first and only mutable slice that contains objects with a lifetime parameter.

let mut info = vk::GetLatencyMarkerInfoNV::default();
let latency_timings = base
    .low_latency2_loader
    .get_latency_timings_len(base.swapchain, &mut info);
dbg!(latency_timings);
if latency_timings != 0 {
    let mut timings = vec![vk::LatencyTimingsFrameReportNV::default(); latency_timings];
    {
        // Creating a new object just for show
        let mut info = vk::GetLatencyMarkerInfoNV::default().timings(&mut timings);
        base.low_latency2_loader.get_latency_timings(
            base.swapchain,
            latency_timings,
            &mut info,
        );
        // `info` should be dropped here, removing the mutable borrow on `timings`?
    }
    dbg!(&timings); // Error: `timings` is still borrowed in `.timings(&mut timings);` above?
}

error[E0502]: cannot borrow `timings` as immutable because it is also borrowed as mutable
   --> examples/src/bin/texture.rs:800:22
    |
793 |                     let mut info = vk::GetLatencyMarkerInfoNV::default().timings(&mut timings);
    |                                                                                  ------------ mutable borrow occurs here
...
800 |                 dbg!(&timings);
    |                      ^^^^^^^^
    |                      |
    |                      immutable borrow occurs here
    |                      mutable borrow later used here

[Ralith]

I think by constructing an &'a mut [LatencyTimingsFrameReportNV<'a>], you're saying that the slice is uniquely borrowed for (&'a mut) a lifetime that outlives it (T<'a>). Does it work better if you do fn timings<'b: 'a>(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'b>]) -> Self?

I think this might also be what you get implicitly if you write LatencyTimingsFrameReportNV<'_>.

[MarijnS95]

@Ralith that is indeed the first thing I tried, making sense that 'b is a lifetime that is bigger / lives longer than 'a (because LatencyTimingsFrameReportNV and the things that it borrows can live longer than the slice and struct GetLatencyMarkerInfoNV it is eventually stored within / referenced from).

Unfortunately the compiler is not having it:

pub fn timings<'b: 'a>(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'b>]) -> Self {
    self.p_timings = if timings.is_empty() {
        std::ptr::null_mut()
    } else {
        timings.as_mut_ptr()
    };
    self
}

Results in:

error: lifetime may not live long enough
     --> ash/src/vk/definitions.rs:51910:13
      |
51892 | impl<'a> GetLatencyMarkerInfoNV<'a> {
      |      -- lifetime `'a` defined here
...
51906 |     pub fn timings<'b: 'a>(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'b>]) -> Self {
      |                    -- lifetime `'b` defined here
...
51910 |             timings.as_mut_ptr()
      |             ^^^^^^^^^^^^^^^^^^^^ argument requires that `'a` must outlive `'b`
      |
      = help: consider adding the following bound: `'a: 'b`
      = note: requirement occurs because of a mutable reference to `[vk::definitions::LatencyTimingsFrameReportNV<'_>]`
      = note: mutable references are invariant over their type parameter
      = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance

Writing the bound the other way around results in the same error as before. And it doesn't make much sense for 'a to outlive 'b when 'a is referring to a thing of shorter lifetime 'b?

[MarijnS95]

Let's hold off merging these changes for a little bit until upstream fixes land in vk.xml: KhronosGroup/Vulkan-Docs#2269 (comment)

[MarijnS95]

The extension API definition has been fixed upstream (pCount is moved from being a function argument to a field on VkGetLatencyMarkerInfoNV), and our generator now "correctly" (see above regarding the lifetime...) generates a slice-setter. The changes are pulled in via a regular Vulkan spec-update PR: #816

[Ralith]

Awesome work!