Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ios fix #1950

Merged
merged 7 commits into from
Aug 11, 2022
Merged

Ios fix #1950

merged 7 commits into from
Aug 11, 2022

Conversation

hakolao
Copy link
Contributor

@hakolao hakolao commented Aug 9, 2022

As part of my ongoing desire to also build my current project as an iOS app I discovered that iOS seems to be broken.

I am assuming that not many have been running Vulkano on iOS. So this PR is a result of a bunch of figuring out how to get an iOS app to compile and run on my iPhone 8 plus.

Error 1:

error[E0782]: trait objects must include the `dyn` keyword
  --> /Users/okkohakola/Programming/Rust/vulkano/vulkano/src/library.rs:45:44
   |
45 |         fn def_loader_impl() -> Result<Box<Loader>, LoadingError> {

After fixing above by adding dyn

Error 2:

error: cannot find macro `statically_linked_vulkan_loader` in this scope
  --> /Users/okkohakola/Programming/Rust/vulkano/vulkano/src/library.rs:46:26
   |
46 |             let loader = statically_linked_vulkan_loader!();

After fixing above by crate::statically_linked_vulkan_loader

Errors 3:

error[E0053]: method `get_instance_proc_addr` has an incompatible type for trait
   --> /Users/okkohakola/Programming/Rust/vulkano/vulkano/src/library.rs:328:13
    |
46  |               let loader = crate::statically_linked_vulkan_loader!();
    |                            ----------------------------------------- in this macro invocation
...
328 | /             fn get_instance_proc_addr(
329 | |                 &self,
330 | |                 instance: ash::vk::Instance,
331 | |                 name: *const c_char,
332 | |             ) -> extern "system" fn() -> () {
    | |___________________________________________^ expected unsafe fn, found normal fn

46  |             let loader = crate::statically_linked_vulkan_loader!();
    |                          ----------------------------------------- in this macro invocation
...
332 |             ) -> extern "system" fn() -> () {
    |                  -------------------------- expected `extern "system" fn()` because of return type
333 |                 unsafe { vkGetInstanceProcAddr(instance, name) }
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found enum `Option`

After fixing that with

        unsafe impl Loader for StaticallyLinkedVulkanLoader {
            unsafe fn get_instance_proc_addr(
                &self,
                instance: ash::vk::Instance,
                name: *const c_char,
            ) -> ash::vk::PFN_vkVoidFunction {
                vkGetInstanceProcAddr(instance, name)
            }
        }

Errors 4

// Bunch of
144 |                     win.borrow().xlib_display().unwrap(),
    |                                  ^^^^^^^^^^^^ method not found in `&Window`

Fix that with

#[cfg(all(unix, not(target_os = "android"), not(target_os = "macos"), not(target_os = "ios")))]

Error 5

41 |     unsafe { winit_to_surface(instance, window) }
   |              ^^^^^^^^^^^^^^^^ not found in this scope

Already down the rabbit hole, make one for ios... how? ... Well my eventual investigation & debugging resulted in handling iOS and MacOS similarly to wgpu seen in this PR.

I've managed to get this working nicely with resizes etc. following wgpu's approach

If you have xcode and apple developer subscription, you should be able to run bevy_vulkano_ios

There is also a simple triangle example in this branch.

And lastly, to fix create_surface_from_handle, testing that in this branch

@Rua
Copy link
Contributor

Rua commented Aug 10, 2022

There's no regressions on my end, so in principle it could be merged. But ideally someone who can test this on iOS would check it first. Is there anyone?

@hakolao
Copy link
Contributor Author

hakolao commented Aug 10, 2022

Let me just try a couple more things today still @Rua just to be sure

@@ -179,6 +179,9 @@ pub(crate) unsafe fn get_metal_layer_macos(view: *mut std::ffi::c_void) -> *mut
let is_valid_layer: BOOL = msg_send![main_layer, isKindOfClass: class];
if is_valid_layer == NO {
let new_layer: *mut Object = msg_send![class, new];
let () = msg_send![new_layer, setEdgeAntialiasingMask: 0];
let () = msg_send![new_layer, setPresentsWithTransaction: false];
let () = msg_send![new_layer, removeAllAnimations];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added these just to make sure we're doing the same function calls master version is doing. Though I am unsure if these are necessary.

@hakolao
Copy link
Contributor Author

hakolao commented Aug 10, 2022

This is ready now

@Rua
Copy link
Contributor

Rua commented Aug 11, 2022

I'll merge it, hopefully we can get other users to test and report any bugs.

@Rua Rua merged commit 062b5e2 into vulkano-rs:master Aug 11, 2022
Rua added a commit that referenced this pull request Aug 11, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants