Fix crash on startup for Vulkan projects on Android

[syntaxerror247]

Fixes a regression introduced by #103026 that caused crash on Android when running Vulkan projects.

Fixes #103156

[bruvzg]

Tested Android editor, seems to be working fine:

There's another direct property check (only controls export warning and was missing rendering_method check):

diff --git a/platform/ios/export/export_plugin.cpp b/platform/ios/export/export_plugin.cpp
index f253e11ec5..ca637bfdb0 100644
--- a/platform/ios/export/export_plugin.cpp
+++ b/platform/ios/export/export_plugin.cpp
@@ -2540,7 +2540,9 @@ bool EditorExportPlatformIOS::has_valid_export_configuration(const Ref<EditorExp
 		}
 	}
 
-	if (GLOBAL_GET("rendering/rendering_device/driver.ios") == "metal") {
+	String rendering_method = GLOBAL_GET("rendering/renderer/rendering_method.mobile");
+	String rendering_driver = GLOBAL_GET("rendering/rendering_device/driver.ios");
+	if ((rendering_method == "forward_plus" || rendering_method == "mobile") && (rendering_driver == "metal" || rendering_driver == "auto")) {
 		float version = p_preset->get("application/min_ios_version").operator String().to_float();
 		if (version < 14.0) {
 			err += TTR("Metal renderer require iOS 14+.") + "\n";

[akien-mga]

Nice catch. This should also fix an unreported bug of not being able to export a gl_compatibility project to iOS < 14 due to the default override for the (unused in this case) RenderingDevice driver being metal.

[akien-mga]

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.

[bruvzg]

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.

[akien-mga]

This is still a bit unsafe btw, as there are other potential overrides that may conflict, e.g. a user could choose to set rendering/renderer/rendering_method.android or rendering/rendering_device/driver.mobile. It's not the default overrides we register but they can be added from the project settings. Not sure which one would take precedence.

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

This isn't necessarily something that can be fixed here, I'm just pointing out how brittle our system is.

[akien-mga]

On the other hand, one thing we could simplify here: Vulkan is the only possible driver for RenderingDevice-based rendering methods. Or conversely, it's only if it's using GL Compatibility that it's not going to use Vulkan.

So all this could be simplified to:

bool EditorExportPlatformAndroid::_uses_vulkan() {
	return GLOBAL_GET("rendering/renderer/rendering_method.mobile") != "gl_compatibility";
}

Still with the above caveat that there could be a .android override or no override if users added/removed them, so this could still break.

I assume the code was written as an explicit check of Vulkan being enabled instead of Vulkan not being enabled to future proof / not make assumptions on future available rendering methods and drivers, which is probably a good idea.

[bruvzg]

I think I have old PR to resolve this kind of overrides on export.

[bruvzg]

The non-standard override is relevant for any project setting, not just these.

[bruvzg]

#71542 export plugin code is obviously outdated (I'll update it for 4.5), but core part should be OK.

[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].

[syntaxerror247]

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.

[bruvzg]

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.

[akien-mga]

See #103197 for now which should go back to a known working state and still fix the main issues we have.

[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.