-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable portability features on macOS #73
base: emulator2
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ use std::os::raw::c_char; | |
use std::sync::Arc; | ||
|
||
use ash::vk; | ||
use ash::vk::{DeviceCreateFlags, PhysicalDeviceFeatures2, PhysicalDevicePortabilitySubsetFeaturesKHR}; | ||
use bumpalo::Bump; | ||
use vk_profiles_rs::{vp, VulkanProfiles}; | ||
|
||
|
@@ -97,8 +98,13 @@ pub fn create_device(config: DeviceCreateConfig, instance: Arc<InstanceContext>) | |
); | ||
} | ||
|
||
let device_create_info = device_create_info.queue_create_infos(queue_create_infos.as_slice()); | ||
|
||
let mut portability_subset_features = PhysicalDevicePortabilitySubsetFeaturesKHR::default(); | ||
#[cfg(target_os = "macos")] { | ||
let mut dummy_features = PhysicalDeviceFeatures2::builder().push_next(&mut portability_subset_features); | ||
unsafe {instance.vk().get_physical_device_features2(physical_device, &mut dummy_features);} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem with this is that if macos ever does add a first party vulkan implementation or if mvk doesn't need portability subset any more this will break. You should query if the extension is supported by the device and only then add it. |
||
} | ||
let device_create_info = device_create_info.queue_create_infos(queue_create_infos.as_slice()) | ||
.push_next(&mut portability_subset_features); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will break any system that doesn't support portability subset. Also this should be done in |
||
let mut flags = vp::DeviceCreateFlagBits::MERGE_EXTENSIONS | vp::DeviceCreateFlagBits::OVERRIDE_FEATURES; | ||
if config.disable_robustness { | ||
flags |= vp::DeviceCreateFlagBits::DISABLE_ROBUST_ACCESS; | ||
|
@@ -374,6 +380,15 @@ fn configure_device(device: &mut DeviceConfigurator) -> Result<Option<DeviceConf | |
} | ||
device.add_extension(&push_descriptor_name); | ||
|
||
let request_portability = cfg!(target_os="macos"); | ||
if request_portability { | ||
let portability_subset_name = CString::new("VK_KHR_portability_subset").unwrap(); | ||
if !device.is_extension_supported(&portability_subset_name) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a device doesn't have portability subset that doesn't mean that it won't work but that it doesn't need portability subset. So instead of failing if the extension is missing you should instead only query the extension if it is present. |
||
log::info!("Physical device (moltenVK) {:?} does not support VK_KHR_portability_subset", device.get_name()); | ||
return Ok(None); | ||
} | ||
device.add_extension(&portability_subset_name); | ||
} | ||
let maintenance_4_name = CString::new("VK_KHR_maintenance4").unwrap(); | ||
let mut maintenance4; | ||
if !device.is_extension_supported(&maintenance_4_name) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use std::str::Utf8Error; | |
use std::sync::Arc; | ||
|
||
use ash::vk; | ||
use ash::vk::InstanceCreateFlags; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
|
||
use vk_profiles_rs::vp; | ||
use crate::{BUILD_INFO, CRATE_NAME}; | ||
|
@@ -134,10 +135,23 @@ pub fn create_instance(config: InstanceCreateConfig) -> Result<Arc<InstanceConte | |
.engine_version(vk::make_api_version(0, BUILD_INFO.version_major, BUILD_INFO.version_minor, BUILD_INFO.version_patch)) | ||
.api_version(max_api_version.into()); | ||
|
||
// this has to be outside of the if statement because it is prematurely dropped otherwise | ||
let portability_name = CString::new("VK_KHR_portability_enumeration").unwrap(); | ||
let request_portability = cfg!(target_os="macos"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of checking for macos and then forcing the extension it would be better to check if the extension is present and then enable it no matter the platform. |
||
// Request portability extensions/portability enumeration bit for moltenVK | ||
let instance_create_flags = if request_portability { | ||
required_extensions_str.push(portability_name.as_c_str().as_ptr()); | ||
InstanceCreateFlags::ENUMERATE_PORTABILITY_KHR | ||
} else { | ||
InstanceCreateFlags::default() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
}; | ||
|
||
let mut instance_create_info = vk::InstanceCreateInfo::builder() | ||
.application_info(&application_info) | ||
.enabled_layer_names(required_layers.as_slice()) | ||
.enabled_extension_names(required_extensions_str.as_slice()); | ||
.enabled_extension_names(required_extensions_str.as_slice()) | ||
.flags(instance_create_flags); | ||
|
||
|
||
let debug_messengers = config.debug_messengers.into_boxed_slice(); | ||
let mut debug_messenger_create_infos: Vec<_> = debug_messengers.iter().map(|messenger| { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the
vk::
prefix for all ash structures and don't import them directly. So instead ofDeviceCreateFlags
usevk::DeviceCreateFlags