extensions: Provide new_from_instance() fallback for Instance fun…

…ctions

This is a minimal, semver-compatible backport of #734 to the
`0.37-stable` branch, warning Ash users of the problem outlined below
while the issue is properly being solved in the next breaking Ash
release (by separating `Instance` and `Device` functions in the
generator to avert this problem entirely while also always providing
optimal `Device`-specific functions for extension wrappers that are
currently already loading _everything_ via `Instance` to forgo the
problem).

As discovered and detailed in #727 a few extension wrappers were loading
and calling `Instance` functions via `Device` and
`get_device_proc_addr()` which is [defined] to only return non-`NULL`
function pointers for `Device` functions.  Those wrapper functions will
always call into Ash's panicking NULL-stub functions as the desired
`Instance` function could not be loaded.

Deprecate the `new()` functions for extension wrappers that were doing
this, while pointing the reader to `new_from_instance()` and explaining
in the docs what function will always `panic!()` when the struct was
loaded using `new()` instead.

This function always takes a raw `vk::Device` directly to fill `handle`
(rather than `ash::Device` to retrieve `handle()` from), allowing users
to pass `vk::Device::null()` when they do intend to load this extension
wrapper just for calling the `Instance` function.

[defined]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetDeviceProcAddr.html#_description

Changelog.md

@@ -15,6 +15,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added missing `Device::get_device_queue2()` wrapper (#736)
 - Exposed `FramebufferCreateInfoBuilder::attachment_count()` builder for `vk::FramebufferCreateFlags::IMAGELESS` (#747)
 
+### Replaced
+
+- Replace `new()` with `new_with_instance()` (via deprecation) on the following extensions to stop panicking in the listed wrapper functions: (#744)
+  - `VK_KHR_swapchain`: `get_physical_device_present_rectangles()`
+  - `VK_KHR_device_group`: `get_physical_device_present_rectangles()`
+  - `VK_EXT_full_screen_exclusive`: `get_physical_device_surface_present_modes2()`
+- `VK_KHR_device_group_creation`: Replaced `device()` with `instance()` (via deprecation) because it was returning `vk::Instance` (#744)
+- `VK_KHR_device_group_creation`: Replaced `new()` with `new_()` (via deprecation) to take `Entry` by borrow instead of move (#744)
+
 ## [0.37.2] - 2022-01-11
 
 ### Added
@@ -51,7 +60,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added `VK_EXT_extended_dynamic_state3` device extension (#671)
 - Added `VK_EXT_descriptor_buffer` instance extension (#679)
 
-
 ### Fixed
 
 - `VK_KHR_ray_tracing_pipeline`: Set the buffer length in `get_ray_tracing_capture_replay_shader_group_handles` so it no longer always returns an empty `Vec` (#658)
@@ -295,7 +303,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### 0.29.0
 
-- -Breaking-: Removed Display impl for flags. The Debug impl now reports flags by name.
+- *Breaking*: Removed Display impl for flags. The Debug impl now reports flags by name.
 - Functions now have a doc comment that links to the Vulkan spec
 - Entry has a new method called `try_enumerate_instance_version` which can be used in a 1.0 context.
 - The generator now uses `BTreeMap` for better diffs.
@@ -304,9 +312,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Switched to a new [changelog](https://keepachangelog.com/en/1.0.0/) format
 - Fixed a build issue on ARM.
-- -Breaking- Arrays are now passed by reference.
+- *Breaking*: Arrays are now passed by reference.
 - Builders are now marked as `#[transparent]`.
-- -Breaking-  Renamed `.next(..)` to `push_next`. `push_next` is only available on structs that are passed directly. Additionally `push_next` only accepts structs that can be inserted into the pointer chain. Read the readme for more information.
+- *Breaking*: Renamed `.next(..)` to `push_next`. `push_next` is only available on structs that are passed directly. Additionally `push_next` only accepts structs that can be inserted into the pointer chain. Read the readme for more information.
 - New -experimental- extensions. Those do not follow the semver rules and can be removed at any time.
 - Added `AmdGpaInterface` extension.
 
@@ -371,7 +379,7 @@ flags: vk::CommandPoolCreateFlags::RESET_COMMAND_BUFFER_BIT,
 - `map_memory` now returns a void ptr
 
 - `ash::util::Align` is a helper struct that
-can write to aligned memory.
+  can write to aligned memory.
 
 [Unreleased]: https://github.com/MaikKlein/ash/compare/0.37.2...HEAD
 [0.37.2]: https://github.com/MaikKlein/ash/releases/tag/0.37.2

ash/src/extensions/ext/full_screen_exclusive.rs

@@ -1,16 +1,24 @@
 use crate::prelude::*;
 use crate::vk;
-use crate::{Device, Instance};
+use crate::{Device, Entry, Instance};
 use std::ffi::CStr;
 use std::mem;
 
+/// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_EXT_full_screen_exclusive.html>
 #[derive(Clone)]
 pub struct FullScreenExclusive {
     handle: vk::Device,
     fp: vk::ExtFullScreenExclusiveFn,
 }
 
 impl FullScreenExclusive {
+    /// [`Instance`] functions cannot be loaded from a [`Device`] and will always panic when called:
+    /// - [`Self::get_physical_device_surface_present_modes2()`]
+    ///
+    /// Load this struct using an [`Instance`] instead via [`Self::new_from_instance()`], or
+    /// suppress the deprecation warning if the above [`Instance`] function is not called.
+    /// This will be solved in the next breaking `ash` release.
+    #[deprecated = "`Instance` functions loaded via this function will always panic. Load via `new_from_instance()` instead."]
     pub fn new(instance: &Instance, device: &Device) -> Self {
         let handle = device.handle();
         let fp = vk::ExtFullScreenExclusiveFn::load(|name| unsafe {
@@ -19,6 +27,19 @@ impl FullScreenExclusive {
         Self { handle, fp }
     }
 
+    /// Loads all functions on the [`Instance`] instead of [`Device`]. This incurs an extra
+    /// dispatch table for [`Device`] functions but also allows the [`Instance`] function to be
+    /// loaded instead of always panicking. See also [`Self::new()`] for more details.
+    ///
+    /// It is okay to pass [`vk::Device::null()`] when this struct is only used to call the
+    /// [`Instance`] function.
+    pub fn new_from_instance(entry: &Entry, instance: &Instance, device: vk::Device) -> Self {
+        let fp = vk::ExtFullScreenExclusiveFn::load(|name| unsafe {
+            mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
+        });
+        Self { handle: device, fp }
+    }
+
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkAcquireFullScreenExclusiveModeEXT.html>
     #[inline]
     pub unsafe fn acquire_full_screen_exclusive_mode(
@@ -29,6 +50,10 @@ impl FullScreenExclusive {
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkGetPhysicalDeviceSurfacePresentModes2EXT.html>
+    ///
+    /// # Warning
+    ///
+    /// Function will always panic unless this struct is loaded via [`Self::new_from_instance()`].
     #[inline]
     pub unsafe fn get_physical_device_surface_present_modes2(
         &self,

ash/src/extensions/khr/device_group.rs

@@ -2,7 +2,7 @@
 use super::Swapchain;
 use crate::prelude::*;
 use crate::vk;
-use crate::{Device, Instance};
+use crate::{Device, Entry, Instance};
 use std::ffi::CStr;
 use std::mem;
 
@@ -14,6 +14,13 @@ pub struct DeviceGroup {
 }
 
 impl DeviceGroup {
+    /// [`Instance`] functions cannot be loaded from a [`Device`] and will always panic when called:
+    /// - [`Self::get_physical_device_present_rectangles()`]
+    ///
+    /// Load this struct using an [`Instance`] instead via [`Self::new_from_instance()`], or
+    /// suppress the deprecation warning if the above [`Instance`] function is not called.
+    /// This will be solved in the next breaking `ash` release.
+    #[deprecated = "`Instance` functions loaded via this function will always panic. Load via `new_from_instance()` instead."]
     pub fn new(instance: &Instance, device: &Device) -> Self {
         let handle = device.handle();
         let fp = vk::KhrDeviceGroupFn::load(|name| unsafe {
@@ -22,6 +29,19 @@ impl DeviceGroup {
         Self { handle, fp }
     }
 
+    /// Loads all functions on the [`Instance`] instead of [`Device`]. This incurs an extra
+    /// dispatch table for [`Device`] functions but also allows the [`Instance`] function to be
+    /// loaded instead of always panicking. See also [`Self::new()`] for more details.
+    ///
+    /// It is okay to pass [`vk::Device::null()`] when this struct is only used to call the
+    /// [`Instance`] function.
+    pub fn new_from_instance(entry: &Entry, instance: &Instance, device: vk::Device) -> Self {
+        let fp = vk::KhrDeviceGroupFn::load(|name| unsafe {
+            mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
+        });
+        Self { handle: device, fp }
+    }
+
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkGetDeviceGroupPeerMemoryFeaturesKHR.html>
     #[inline]
     pub unsafe fn get_device_group_peer_memory_features(
@@ -112,6 +132,10 @@ impl DeviceGroup {
     ///
     /// [Vulkan 1.1]: https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_VERSION_1_1.html
     /// [`VK_KHR_surface`]: https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_KHR_surface.html
+    ///
+    /// # Warning
+    ///
+    /// Function will always panic unless this struct is loaded via [`Self::new_from_instance()`].
     #[inline]
     pub unsafe fn get_physical_device_present_rectangles(
         &self,

ash/src/extensions/khr/device_group_creation.rs

@@ -13,7 +13,13 @@ pub struct DeviceGroupCreation {
 }
 
 impl DeviceGroupCreation {
+    /// Call [`Self::new_()`] with a borrow of `Entry` instead.
+    #[deprecated = "Unnecessarily requires a move of `Entry`. Call `new_()` with a borrow instead."]
     pub fn new(entry: Entry, instance: &Instance) -> Self {
+        Self::new_(&entry, instance)
+    }
+
+    pub fn new_(entry: &Entry, instance: &Instance) -> Self {
         let handle = instance.handle();
         let fp = vk::KhrDeviceGroupCreationFn::load(|name| unsafe {
             mem::transmute(entry.get_instance_proc_addr(handle, name.as_ptr()))
@@ -59,8 +65,14 @@ impl DeviceGroupCreation {
         &self.fp
     }
 
+    #[deprecated = "typo: this function is called `device()`, but returns an `Instance`."]
     #[inline]
     pub fn device(&self) -> vk::Instance {
         self.handle
     }
+
+    #[inline]
+    pub fn instance(&self) -> vk::Instance {
+        self.handle
+    }
 }

ash/src/extensions/khr/swapchain.rs

@@ -3,17 +3,25 @@ use super::DeviceGroup;
 use crate::prelude::*;
 use crate::vk;
 use crate::RawPtr;
-use crate::{Device, Instance};
+use crate::{Device, Entry, Instance};
 use std::ffi::CStr;
 use std::mem;
 
+/// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_KHR_swapchain.html>
 #[derive(Clone)]
 pub struct Swapchain {
     handle: vk::Device,
     fp: vk::KhrSwapchainFn,
 }
 
 impl Swapchain {
+    /// [`Instance`] functions cannot be loaded from a [`Device`] and will always panic when called:
+    /// - [`Self::get_physical_device_present_rectangles()`]
+    ///
+    /// Load this struct using an [`Instance`] instead via [`Self::new_from_instance()`], or
+    /// suppress the deprecation warning if the above [`Instance`] function is not called.
+    /// This will be solved in the next breaking `ash` release.
+    #[deprecated = "`Instance` functions loaded via this function will always panic. Load via `new_from_instance()` instead."]
     pub fn new(instance: &Instance, device: &Device) -> Self {
         let handle = device.handle();
         let fp = vk::KhrSwapchainFn::load(|name| unsafe {
@@ -22,6 +30,19 @@ impl Swapchain {
         Self { handle, fp }
     }
 
+    /// Loads all functions on the [`Instance`] instead of [`Device`]. This incurs an extra
+    /// dispatch table for [`Device`] functions but also allows the [`Instance`] function to be
+    /// loaded instead of always panicking. See also [`Self::new()`] for more details.
+    ///
+    /// It is okay to pass [`vk::Device::null()`] when this struct is only used to call the
+    /// [`Instance`] function.
+    pub fn new_from_instance(entry: &Entry, instance: &Instance, device: vk::Device) -> Self {
+        let fp = vk::KhrSwapchainFn::load(|name| unsafe {
+            mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
+        });
+        Self { handle: device, fp }
+    }
+
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCreateSwapchainKHR.html>
     #[inline]
     pub unsafe fn create_swapchain(
@@ -153,6 +174,10 @@ impl Swapchain {
     ///
     /// [Vulkan 1.1]: https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_VERSION_1_1.html
     /// [`VK_KHR_surface`]: https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/VK_KHR_surface.html
+    ///
+    /// # Warning
+    ///
+    /// Function will always panic unless this struct is loaded via [`Self::new_from_instance()`].
     #[inline]
     pub unsafe fn get_physical_device_present_rectangles(
         &self,

examples/src/lib.rs

@@ -381,7 +381,7 @@ impl ExampleBase {
                 .cloned()
                 .find(|&mode| mode == vk::PresentModeKHR::MAILBOX)
                 .unwrap_or(vk::PresentModeKHR::FIFO);
-            let swapchain_loader = Swapchain::new(&instance, &device);
+            let swapchain_loader = Swapchain::new_from_instance(&entry, &instance, device.handle());
 
             let swapchain_create_info = vk::SwapchainCreateInfoKHR::builder()
                 .surface(surface)