Skip to content

Commit

Permalink
hal: cargo feature to allow using VK_GOOGLE_display_timing unsafely (
Browse files Browse the repository at this point in the history
…gfx-rs#6149)

* Expose the raw swapchain from a vulkan `Surface`

* Allow setting the present timing information on hal Vulkan

* Fix clippy without the feature enabled

* CHANGELOG

* Revert inadvertently formatted Cargo.toml

* Move display timing to a feature

* Update the changelog

* Whitespace and doc wording tweaks

* Apply suggestions from code review

Co-authored-by: Marijn Suijten <marijns95@gmail.com>

* Revert inadvertent formatting changes again

* Remove unused qualification

Co-authored-by: Marijn Suijten <marijns95@gmail.com>

* Address review feedback

* Fix flow of sentence and follow intra-doc-link

* Add more docs to `set_next_present_time`, and rename

* Also rename `next_present_times`

* Apply suggestions from code review

Co-authored-by: Marijn Suijten <marijns95@gmail.com>

---------

Co-authored-by: Marijn Suijten <marijns95@gmail.com>
  • Loading branch information
DJMcNab and MarijnS95 authored Aug 27, 2024
1 parent 070f760 commit 685c213
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134).

* Support constant evaluation for `firstLeadingBit` and `firstTrailingBit` numeric built-ins in WGSL. Front-ends that translate to these built-ins also benefit from constant evaluation. By @ErichDonGubler in [#5101](https://github.com/gfx-rs/wgpu/pull/5101).

#### Vulkan

- Allow using [VK_GOOGLE_display_timing](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_GOOGLE_display_timing.html) unsafely with the `VULKAN_GOOGLE_DISPLAY_TIMING` feature. By @DJMcNab in [#6149](https://github.com/gfx-rs/wgpu/pull/6149)

### Bug Fixes

- Fix incorrect hlsl image output type conversion. By @atlv24 in [#6123](https://github.com/gfx-rs/wgpu/pull/6123)
Expand Down
12 changes: 11 additions & 1 deletion wgpu-hal/src/vulkan/adapter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::conv;

use ash::{amd, ext, khr, vk};
use ash::{amd, ext, google, khr, vk};
use parking_lot::Mutex;

use std::{collections::BTreeMap, ffi::CStr, sync::Arc};
Expand Down Expand Up @@ -771,6 +771,11 @@ impl PhysicalDeviceFeatures {
);
}

features.set(
F::VULKAN_GOOGLE_DISPLAY_TIMING,
caps.supports_extension(google::display_timing::NAME),
);

(features, dl_flags)
}

Expand Down Expand Up @@ -1004,6 +1009,11 @@ impl PhysicalDeviceProperties {
extensions.push(khr::shader_atomic_int64::NAME);
}

// Require VK_GOOGLE_display_timing if the associated feature was requested
if requested_features.contains(wgt::Features::VULKAN_GOOGLE_DISPLAY_TIMING) {
extensions.push(google::display_timing::NAME);
}

extensions
}

Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ impl super::Device {
view_formats: wgt_view_formats,
surface_semaphores,
next_semaphore_index: 0,
next_present_time: None,
})
}

Expand Down
65 changes: 65 additions & 0 deletions wgpu-hal/src/vulkan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,13 @@ struct Swapchain {
/// index as the image index, but we need to specify the semaphore as an argument
/// to the acquire_next_image function which is what tells us which image to use.
next_semaphore_index: usize,
/// The present timing information which will be set in the next call to [`present()`](crate::Queue::present()).
///
/// # Safety
///
/// This must only be set if [`wgt::Features::VULKAN_GOOGLE_DISPLAY_TIMING`] is enabled, and
/// so the VK_GOOGLE_display_timing extension is present.
next_present_time: Option<vk::PresentTimeGOOGLE>,
}

impl Swapchain {
Expand All @@ -375,6 +382,47 @@ pub struct Surface {
swapchain: RwLock<Option<Swapchain>>,
}

impl Surface {
/// Get the raw Vulkan swapchain associated with this surface.
///
/// Returns [`None`] if the surface is not configured.
pub fn raw_swapchain(&self) -> Option<vk::SwapchainKHR> {
let read = self.swapchain.read();
read.as_ref().map(|it| it.raw)
}

/// Set the present timing information which will be used for the next [presentation](crate::Queue::present) of this surface,
/// using [VK_GOOGLE_display_timing].
///
/// This can be used to give an id to presentations, for future use of `VkPastPresentationTimingGOOGLE`.
/// Note that `wgpu-hal` does *not* provide a way to use that API - you should manually access this through `ash`.
///
/// This can also be used to add a "not before" timestamp to the presentation.
///
/// The exact semantics of the fields are also documented in the [specification](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkPresentTimeGOOGLE.html) for the extension.
///
/// # Panics
///
/// - If the surface hasn't been configured.
/// - If the device doesn't [support present timing](wgt::Features::VULKAN_GOOGLE_DISPLAY_TIMING).
///
/// [VK_GOOGLE_display_timing]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_GOOGLE_display_timing.html
#[track_caller]
pub fn set_next_present_time(&self, present_timing: vk::PresentTimeGOOGLE) {
let mut swapchain = self.swapchain.write();
let swapchain = swapchain
.as_mut()
.expect("Surface should have been configured");
let features = wgt::Features::VULKAN_GOOGLE_DISPLAY_TIMING;
if swapchain.device.features.contains(features) {
swapchain.next_present_time = Some(present_timing);
} else {
// Ideally we'd use something like `device.required_features` here, but that's in `wgpu-core`, which we are a dependency of
panic!("Tried to set display timing properties without the corresponding feature ({features:?}) enabled.");
}
}
}

#[derive(Debug)]
pub struct SurfaceTexture {
index: u32,
Expand Down Expand Up @@ -1158,6 +1206,23 @@ impl crate::Queue for Queue {
.image_indices(&image_indices)
.wait_semaphores(swapchain_semaphores.get_present_wait_semaphores());

let mut display_timing;
let present_times;
let vk_info = if let Some(present_time) = ssc.next_present_time.take() {
debug_assert!(
ssc.device
.features
.contains(wgt::Features::VULKAN_GOOGLE_DISPLAY_TIMING),
"`next_present_times` should only be set if `VULKAN_GOOGLE_DISPLAY_TIMING` is enabled"
);
present_times = [present_time];
display_timing = vk::PresentTimesInfoGOOGLE::default().times(&present_times);
// SAFETY: We know that VK_GOOGLE_display_timing is present because of the safety contract on `next_present_times`.
vk_info.push_next(&mut display_timing)
} else {
vk_info
};

let suboptimal = {
profiling::scope!("vkQueuePresentKHR");
unsafe { self.swapchain_fn.queue_present(self.raw, &vk_info) }.map_err(|error| {
Expand Down
16 changes: 16 additions & 0 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,22 @@ bitflags::bitflags! {
///
/// This is a native only feature.
const SHADER_INT64_ATOMIC_ALL_OPS = 1 << 61;
/// Allows using the [VK_GOOGLE_display_timing] Vulkan extension.
///
/// This is used for frame pacing to reduce latency, and is generally only available on Android.
///
/// This feature does not have a `wgpu`-level API, and so users of wgpu wishing
/// to use this functionality must access it using various `as_hal` functions,
/// primarily [`Surface::as_hal`], to then use.
///
/// Supported platforms:
/// - Vulkan (with [VK_GOOGLE_display_timing])
///
/// This is a native only feature.
///
/// [VK_GOOGLE_display_timing]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_GOOGLE_display_timing.html
/// [`Surface::as_hal`]: https://docs.rs/wgpu/latest/wgpu/struct.Surface.html#method.as_hal
const VULKAN_GOOGLE_DISPLAY_TIMING = 1 << 62;
}
}

Expand Down

0 comments on commit 685c213

Please sign in to comment.