Skip to content
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

Open
wants to merge 2 commits into
base: emulator2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Expand Up @@ -22,7 +22,10 @@ To build the project with natives for your platform run
./gradlew build
```
in the project root directory.

### macOS
Blaze4D now (mostly) works on macOS! Building and running follows the same steps as on Windows or Linux.
The only difference is that you need to set the `VK_INSTANCE_LAYERS` environment variable to `VK_LAYER_KHRONOS_synchronization2` because moltenVK doesn't natively support it.
### Running
To run the game with the mod use any of the 3 run targets:
- `./gradlew runClient`
- `./gradlew runClientWithValidation` - Enables validation layers
Expand Down
19 changes: 17 additions & 2 deletions core/natives/src/device/init.rs
Expand Up @@ -4,6 +4,7 @@ use std::os::raw::c_char;
use std::sync::Arc;

use ash::vk;
use ash::vk::{DeviceCreateFlags, PhysicalDeviceFeatures2, PhysicalDevicePortabilitySubsetFeaturesKHR};
Copy link
Member

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 of DeviceCreateFlags use vk::DeviceCreateFlags

use bumpalo::Bump;
use vk_profiles_rs::{vp, VulkanProfiles};

Expand Down Expand Up @@ -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);}
Copy link
Member

Choose a reason for hiding this comment

The 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);
Copy link
Member

Choose a reason for hiding this comment

The 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 configure_device

let mut flags = vp::DeviceCreateFlagBits::MERGE_EXTENSIONS | vp::DeviceCreateFlagBits::OVERRIDE_FEATURES;
if config.disable_robustness {
flags |= vp::DeviceCreateFlagBits::DISABLE_ROBUST_ACCESS;
Expand Down Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The 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) {
Expand Down
16 changes: 15 additions & 1 deletion core/natives/src/instance/init.rs
Expand Up @@ -5,6 +5,7 @@ use std::str::Utf8Error;
use std::sync::Arc;

use ash::vk;
use ash::vk::InstanceCreateFlags;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use vk:: prefix here as well


use vk_profiles_rs::vp;
use crate::{BUILD_INFO, CRATE_NAME};
Expand Down Expand Up @@ -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");
Copy link
Member

Choose a reason for hiding this comment

The 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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ::empty() its not a functional difference but a bit clearer imo.

};

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| {
Expand Down