godot icon indicating copy to clipboard operation
godot copied to clipboard

Fix crash on startup for Vulkan projects on Android

Open syntaxerror247 opened this issue 9 months ago • 7 comments

Fixes a regression introduced by https://github.com/godotengine/godot/pull/103026 that caused crash on Android when running Vulkan projects.

Fixes https://github.com/godotengine/godot/issues/103156

syntaxerror247 avatar Feb 22 '25 03:02 syntaxerror247

Just noting another direct property check for the Windows exporter:

platform/windows/export/export_plugin.cpp
245:            include_d3d12_extra_libs = (String(GLOBAL_GET("rendering/rendering_device/driver.windows")) == "d3d12") && (String(GLOBAL_GET("rendering/renderer/rendering_method")) != "gl_compatibility");

This one should still be fine as d3d12 is not the default and thus not what auto configures on Windows... unless someone has compiled export templates without Vulkan support and relies on auto to automatically fallback to d3d12, where this would break. It's showing the limit of that system.


And for the record, direct references to the opengl driver also changed in #103026:

platform/windows/export/export_plugin.cpp
227:            include_angle_libs = (String(GLOBAL_GET("rendering/gl_compatibility/driver.windows")) == "opengl3_angle") && (String(GLOBAL_GET("rendering/renderer/rendering_method")) == "gl_compatibility");

platform/macos/export/export_plugin.cpp
1784:           include_angle_libs = String(GLOBAL_GET("rendering/gl_compatibility/driver.macos")) == "opengl3_angle";

Those should be fine for now, but if we were to make auto default to opengl3_angle on Windows or macOS, we'd also have a problem here. It's pretty brittle.

akien-mga avatar Feb 22 '25 22:02 akien-mga

These are OK for default templates, and currently we have no way to detect how a custom template was build. For custom templates, there are application/export_d3d12 and application/export_angle export options to force libraries to copy (usual custom builds won't have any libs to copy, so these checks won't do anything).

These check probably should not be there at all, since default templates do no include external libs. It was added at early stages of D3D12 and ANGLE support development when libraries were necessary.

bruvzg avatar Feb 22 '25 22:02 bruvzg

Also, it is documented to work only if a specific renderer value is set:

If set to [code]1[/code], ANGLE libraries are exported with the exported application. If set to [code]0[/code], ANGLE libraries are exported only if [member ProjectSettings.rendering/gl_compatibility/driver] is set to [code]"opengl3_angle"[/code].

If set to [code]1[/code], the Direct3D 12 runtime libraries (Agility SDK, PIX) are exported with the exported application. If set to [code]0[/code], Direct3D 12 libraries are exported only if [member ProjectSettings.rendering/rendering_device/driver] is set to [code]"d3d12"[/code].

bruvzg avatar Feb 22 '25 23:02 bruvzg

auto introduces a lot of uncertainty. From a user’s perspective, I would expect it to handle everything automatically.

IMO, rendering driver settings should be explicitly set by the user. Instead of relying on auto, we should define a clear default for each platform while allowing users to choose their preferred driver.

They could also remove any .mobile and .android override and just have rendering/rendering_device/driver as the sole source of truth.

Rather than treating settings like rendering/rendering_device/driver.android as an override, it should be a required setting. Users should have to set it explicitly rather than being able to remove it. Making it a proper setting would simplify platform-specific checks.

syntaxerror247 avatar Feb 22 '25 23:02 syntaxerror247

The problem is macOS, which have different defaults for Intel and ARM. I guess we can do driver.macos.x86_64 and driver.macos.arm64 as separate settings, but not sure if it's better.

Instead of a single driver it might be better to change the whole system to the priority list where users can only change the preferred order of the drivers, instead of picking one.

bruvzg avatar Feb 22 '25 23:02 bruvzg

See https://github.com/godotengine/godot/pull/103197 for now which should go back to a known working state and still fix the main issues we have.

akien-mga avatar Feb 23 '25 00:02 akien-mga

The problem is macOS, which have different defaults for Intel and ARM. I guess we can do driver.macos.x86_64 and driver.macos.arm64 as separate settings, but not sure if it's better.

In practice it shouldn't be a big issue though I think. metal for macOS just means "Metal on ARM and Vulkan on Intel". The only potential problem is that the automatic fallback to Vulkan on Intel depends on the fallback_to_vulkan setting, which users might disable by oversight, and end up bricking any support of Intel Macs. We should maybe just remove that check and always fallback without configuration (if users don't want to support Intel Macs, they can export a non-universal arm64 binary).

Instead of a single driver it might be better to change the whole system to the priority list where users can only change the preferred order of the drivers, instead of picking one.

Yeah for 4.5 I believe we could explore in that direction.

akien-mga avatar Feb 23 '25 00:02 akien-mga

Superseded by #103197 which includes it.

akien-mga avatar Feb 24 '25 14:02 akien-mga