-
Notifications
You must be signed in to change notification settings - Fork 194
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
VK_KHR_swapchain is broken on Vulkan 1.0 #307
Comments
I am not sure if that is an ash bug. The spec says |
@MaikKlein good point. It does seem allowed according to this wording. I guess the question is, who prints this then:
|
That's from android's vulkan loader and should also return a nullptr. |
Thanks @msiglreith ! |
If its from android's vulkan loader, why is it prefixed with |
Possibly, but ash follows the spec here. Not sure how hard it would be to change that. I don't know if there is a way of querying the version of an instance at runtime, otherwise you'd have to pass in the spec version every time you want to create an extension, which doesn't seem that nice either. I guess technically we could carry the version around in an instance, but that just feels hacky just to make android happy here. But still a possibility.
Maybe it is coming from the validation layer? |
The vulkan backend forwards debug messages, which are generated by the loader here, into |
After thinking about it for a while, I think the situation can be improved. As in only load function pointers for
pub struct Swapchain {
handle: vk::Device,
swapchain_fn_1_0: vk::KhrSwapchainFn_1_0,
swapchain_fn_1_1: vk::KhrSwapchainFn_1_1,
} Ash already does that internally for As to your bug report: This is not a blocker for you right? It is just an incorrect error log from the vulkan loader on android. You can even see it has names such as |
It's our only clue so far on why Android fails to wok, according to the gfx-rs/wgpu-rs#313 (comment) , so might as well be a blocker. As for filing this upstream, I don't do Android development yet, and not even sure where exactly to file it. (https://github.com/KhronosGroup/Vulkan-Loader doesn't seem to have the code @msiglreith linked to). Will look into it... |
While I don't know if this is a breaking bug, there is at least one breaking bug in the stack somewhere as the downstream issue shows. It could be this one, or it could be
I don't know. If anyone can test the downstream issue on a diffrent phne, preferrabily not running android 9, it would be great. |
I get the same However I have no problem rendering on Android with ash on Android 10. |
Would you mind sharing the code so I can test it as well? |
I don't think my employer would like that 😉 |
Just chiming in here to say that the https://cs.android.com/android/platform/superproject/+/master:frameworks/native/libs/ui/GraphicBufferMapper.cpp;l=53-59 Even the SM8150 SoC from qcom on the latest Android 10 tag uses [email protected]: |
Doesn't work 😞 It doesn't even compile, instead throwing a linker error with the folloving note:
|
I think you didn't read the documentation of https://github.com/rust-windowing/android-ndk-rs properly. Using it correctly should solve those issues. Straight up compiling the ash example for android won't work without changing the code. Here is a deprecated repo with some extra documentation: https://github.com/rust-windowing/android-rs-glue |
Well, I changed the code, but it still doesn't work. Here's my modified version:
And here is the modified Cargo.toml:
I upgraded winit and did a couple minor tweaks to remove the dependency on the deprecated android-glue, and the ndk-glue path points to an unmodified clone of the master branch. I also installed cargo-apk from said clone, so there shouldn't be any issues. However, it still doesn't work. Here's the error message:
I had fixed a very similar bug with the loop statement, but that trick apparently doesn't work with ash. |
ash_window::enumerate_required_extensions(&window)? currently panics, requires rust-windowing/winit#1624 or manually specifying the extensions. Furthemore, the creation of the surface must be moved into the event loop: Event::Resumed => {
// destroy surface if already have some
// ANativeWindow handle only valid between Resumed and Suspended
// might change when getting a new Resumed event -> destroy old one
surface = ash_window::create_surface(&entry, &instance, &window, None)?;
println!("surface: {:?}", surface);
}
Event::Suspended => /* destroy surface here */ Then everything should work for this example at least (: |
It worked! Well, almost. There is still no output to the screen, and touching it gives an ANR. But, the reason for that is winit doesn't poll the android event queue, meaning inputs go unhandled. I'll probably look into that some other day, and if its not implemented on the master branch open an issue. EDIT:
And here is the output :
The |
|
Doing the necessary steps listed above (patching winit, ndk-* versions, ndk::main, cdylib, surface/swapchain creation on Resumed, etc.) the wgpu example should also run. A few more wgpu related parts needed to adjusted as well: surface format was not supported and compatible surface needs to be changed to None (on Android presentation is always supported for all devices). The error message
is still there and can be ignored as mentioned in my opinion. Funny enough, touch input handling also works once drawing something on screen.. |
I figured out that recently as well, cant wait to get wgpu working! |
@msiglreith Any updates on this? Trying to make the same thing work but facing VK and similar issues. Is there a working example of running wgpu on android? |
@msiglreith Thank you a lot! I was able to run your example on my Mi Mix 2S phone. Also, I tried to run it on the Pixel_3_API_29 emulator but got Vulkan errors. I think it's just because it's not installed on my mac and not specified in the emulator settings but is it possible to make wgpu fallback to GLES in this kind of situation? |
This looks like an old issue that I think we can close (as the downstream In addition this Vulkan 1.1 dependency was mentioned in the function docs when a high-level wrapper was finally added in #629, just a few days short of two years after this issue was filed. Then we very recently discovered a major flaw in that PR and a few other extension wrapper loaders: #727 - in short you cannot load instance-level functions like Will close this and track that issue in #727. Separately we are thinking about making loader functions fail on |
See https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VK_KHR_swapchain.html
Also downstream issue in gfx-rs/wgpu-rs#313 (comment)
Basically, looks like Ash requests an address of
vkGetPhysicalDevicePresentRectanglesKHR
even when on Vulkan 1.0 instance, where it doesn't exist.This is a blocker for gfx/wgpu on Android today.
The text was updated successfully, but these errors were encountered: