Skip to content
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

Always allow selecting any rendering driver in the settings, add "auto" option. #103026

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Feb 19, 2025

See #103006 (comment)

  • All rendering driver names can be selected in the project setting (for both compatibility and rendering device), regardless of the editor build config.
  • "auto" option is added (and set as default for some platforms) to allow auto selection of the first available driver (in the following order: metal, vulkan, d3d12).

@bruvzg
Copy link
Member Author

bruvzg commented Feb 19, 2025

Additionally removed _NOVAL from properties and added some info about values to the docs. We should probably check other _NOVAL properties as well.

@clayjohn
Copy link
Member

Maybe this would be more suitable for 4.5? Is there any reason to rush this and merge it in the RC phase?

@bruvzg
Copy link
Member Author

bruvzg commented Feb 19, 2025

Is there any reason to rush this and merge it in the RC phase?

Impossibility of picking iOS rendering driver when using Intel macs is an issue, but I'm not sure if it's something absolutely necessary for 4.4. Default value is OK in most cases, and absolute majority of iOS developers will use Apple Silicon macs.

@bruvzg
Copy link
Member Author

bruvzg commented Feb 19, 2025

We should probably check other _NOVAL properties as well.

Other _NOVAL properties:

  • 🟢 OK, _NOVAL use seems reasonable:
    • rendering/gl_compatibility/force_angle_on_devices - OK, we do not want a huge block list in the docs.
    • editor/script/search_in_file_extensions, different value for .NET version, so should be OK.
  • 🟡 OK, but no reason to use _NOVAL:
    • display/display_server/driver (and overrides), all values are selectable on all platforms. Default value is always default.
    • input_devices/pen_tablet/driver (and override), all values are selectable on all platforms. Default value is always empty string.
    • rendering/scaling_3d/mode (iOS and macOS overrides), all values are selectable on all platforms. Default value is always Bilinear.
  • 🔴 Can be an issue if you try to override the value:
    • audio/driver/driver, uses platform specific values, probably should be changed to auto + platform specific overrides as well.
    • gui/common/swap_cancel_ok, uses platform specific default value.

@bruvzg bruvzg modified the milestones: 4.4, 4.5 Feb 19, 2025
@akien-mga
Copy link
Member

I think for 4.4 we should change this to not be platform/driver specific:

		{
			Vector<String> driver_hints_arr;
#ifdef VULKAN_ENABLED
			driver_hints_arr.push_back("vulkan");
#endif
			driver_hints = String(",").join(driver_hints_arr);

#ifdef D3D12_ENABLED
			driver_hints_arr.push_back("d3d12");
#endif
			driver_hints_with_d3d12 = String(",").join(driver_hints_arr);

#ifdef METAL_ENABLED
			// Make metal the preferred and default driver.
			driver_hints_arr.insert(0, "metal");
#endif
			driver_hints_with_metal = String(",").join(driver_hints_arr);
		}

We should let any version of Godot choose any driver for the project, as it might run on a different device. If editing a macOS or iOS project on Windows, you should still be able to configure whether it should use Metal or Vulkan.

@bruvzg
Copy link
Member Author

bruvzg commented Feb 19, 2025

We should let any version of Godot choose any driver for the project, as it might run on a different device. If editing a macOS or iOS project on Windows, you should still be able to configure whether it should use Metal or Vulkan.

Apart for additional notes in the docs, this is the only thing this PR do. OpenGL part is the same, but only relevant if you build an editor w/o GL support, so not important.

@akien-mga akien-mga modified the milestones: 4.5, 4.4 Feb 19, 2025
@akien-mga akien-mga added the bug label Feb 19, 2025
@akien-mga
Copy link
Member

I think we might want this for 4.4 to solve the current weird platform-specific available issue for configuration.

I'll see if I can think of a more minimal subset that does not introduce "auto" to avoid changing config so late in the cycle, but from a quick look I'm not sure I see such subset, so we might want to just go all the way and merge this for 4.4.rc1, and document clearly that those settings are now "auto" by default.

@bruvzg
Copy link
Member Author

bruvzg commented Feb 19, 2025

I'll see if I can think of a more minimal subset that does not introduce "auto" to avoid changing config so late in the cycle, but from a quick look I'm not sure I see such subset, so we might want to just go all the way and merge this for 4.4.rc1, and document clearly that those settings are now "auto" by default.

Since it was not (and is not) saving default values, valid existing project configs should not change. If it was default, it's still default (with the same behavior and not saved), and if it was set to other value, it will stay with this value. So for most users it will be a minor editor UI change. The only affected cases are iOS config when using editor on Intel Mac, and custom builds with disabled renderers.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I think it's a good idea and solves the issue of not being able to select a supported driver for the target platform based on what the host editor platform supports.

See e.g. in the current master branch, from Linux I can't select d3d12 for Windows, and it wrongly shows that macOS and iOS default to vulkan (and metal can't be selected).

image

@clayjohn pointed out a caveat which is that if users self-compile Godot without support for some drivers (e.g. Windows editor build without d3d12), those users who be able to still select d3d12 and not being able to run the project in their editor build. We do have fallbacks enabled by default so it should fallback to vulkan, and worse case to OpenGL, so it's unlikely to be a widespread issue (they'd need to also disable fallbacks to really brick things - in which case they can just fix the project.godot manually).

@akien-mga akien-mga merged commit 413b794 into godotengine:master Feb 20, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

akien-mga added a commit to akien-mga/godot that referenced this pull request Feb 22, 2025
Simpler alternative to godotengine#103026 which avoids breaking compatibility.

Instead of introducing a new `auto` default value, we ensure that all
supported drivers are registered regardless of the editor's host platform,
and that the defaults are the intended ones.

This solves the following issues:
- macOS exports are meant to default to Metal in 4.4, but they would
  default to Vulkan if exported from Linux, Windows, or Android editors.
- Windows exports couldn't be made with Direct3D 12 from Linux, macOS, or
  Android editors, as the option couldn't be selected outside Windows.

Unlike godotengine#103026, it doesn't solve the issue of not always saving the
rendering drivers to `project.godot`, but now the defaults are at least
consistent between editor platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Bad
Development

Successfully merging this pull request may close these issues.

3 participants