Improve vulkan capability detection on Android #72780
Conversation
| @@ -252,6 +252,7 @@ static const char *APK_ASSETS_DIRECTORY = "res://android/build/assets"; | |||
| static const char *AAB_ASSETS_DIRECTORY = "res://android/build/assetPacks/installTime/src/main/assets"; | |||
|
|
|||
| static const int DEFAULT_MIN_SDK_VERSION = 21; // Should match the value in 'platform/android/java/app/config.gradle#minSdk' | |||
| static const int VULKAN_MIN_SDK_VERSION = 24; | |||
There was a problem hiding this comment.
Is this the minimum Andriod SDK version? is there a point of still supporting 21-23 through OpenGL with Godot 4 or should we just make 24 the minimum? I'm not sure of the numbers here.
There was a problem hiding this comment.
We discussed this at length before and agreed that we should support as low as 21 using the OpenGL renderer.
| return false; | ||
| } | ||
|
|
||
| return Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && packageManager.hasSystemFeature(PackageManager.FEATURE_VULKAN_HARDWARE_LEVEL, 1); |
There was a problem hiding this comment.
What version are we comparing here?
There was a problem hiding this comment.
We're comparing the device SDK version, and this follows a similar logic as with the export logic:
If the device SDK version < Android N (which corresponds to API 24), we consider the device as not meeting the vulkan requirements.
- Add runtime check and abort when the device doesn't meet the requirements for vulkan support - Add filters to the AndroidManifest when exporting with a vulkan renderer
m4gr3d
force-pushed
the
add_vulkan_filter_when_necessary
branch
from
b8ded30 to
034fd15
Compare
| String current_renderer = GLOBAL_GET("rendering/renderer/rendering_method.mobile"); | ||
| bool has_vulkan = current_renderer == "forward_plus" || current_renderer == "mobile"; | ||
| if (has_vulkan) { |
There was a problem hiding this comment.
I think the better check would be to query rendering/rendering_device/driver.android, which would be "vulkan".
Currently that's the only available driver for RenderingDevice-based rendering methods (forward_plus and mobile), but in theory there could be other implementations in the future (e.g. a Metal implementation for macOS/iOS, and Direct3D 12 for Windows). For Android, it's unlikely to happen, but who knows, maybe there will be some OpenGL ES 3.2-based high end RenderingDevice API in the future. Or WebGPU-native or something.
There was a problem hiding this comment.
Though actually, you'd need to check both settings probably - rendering/rendering_device/driver.android is always defined, but only relevant if the selected rendering method is a RenderingDevice-based one... and I don't think there's an easy way to know that aside from checking whether it's "forward_plus" or "mobile".
So the "safest" way might be:
| String current_renderer = GLOBAL_GET("rendering/renderer/rendering_method.mobile"); | |
| bool has_vulkan = current_renderer == "forward_plus" || current_renderer == "mobile"; | |
| if (has_vulkan) { | |
| String current_renderer = GLOBAL_GET("rendering/renderer/rendering_method.mobile"); | |
| bool has_vulkan = (current_renderer == "forward_plus" || current_renderer == "mobile") && GLOBAL_GET("rendering/rendering_device/driver.android") == "vulkan"; | |
| if (has_vulkan) { |
Also relevant for these checks: #71542 (CC @bruvzg).
The current code could possibly fail in situations where a user defines rendering/renderer/rendering_method.android to supersede the mobile one (not sure which one would take precedence are both are supported features). Not sure who would want to do that but in theory one could want to use a different rendering method on Android and iOS and thus go for the platform-specific overrides instead of the mobile one.
I'm not suggesting to try to handle all this now though, it's just food for thought that we might need to make this more robust in the future.
|
Thanks! |
Add runtime check and abort when the device doesn't meet the requirements for vulkan support
Add filters to the AndroidManifest when exporting with a vulkan renderer