Split rendering driver project setting into `rendering_method` and `rendering/*/driver` #65541

clayjohn · 2022-09-08T20:19:40Z

Closes: godotengine/godot-proposals#4261

This also exposes OpenGL to the project creation menu and re-exposes the renderer switch button in the top right hand corner of the editor.

Things to consider during review:

Organization of the project settings. I organized the project settings as suggested in Change the naming of rendering methods and drivers godot-proposals#4261, but it feels a little awkward.
Using enums instead of strings for renderer name and rendering driver. In 3.x we had a VideoDriver enum which we used throughout the engine instead of exposing strings directly. I think we should consider having a RenderingDriver enum and a RendererName enum which we pass around to avoid doing risky string operations all over the engine.

I blindly made some changes to the iOS and Android platforms, would be nice if someone from each team can confirm that the changes look okay.

Screenshots of new project settings:

when using gl_compatibility

when using rendering device based renderers

Geometror · 2022-09-08T21:04:16Z

Great work! However, I wonder if it's worth it to reintroduce the the renderer switch button in the menu bar. This is something which will be changed very rarely during development for that it takes quite some space.

clayjohn · 2022-09-08T21:11:13Z

Great work! However, I wonder if it's worth it to reintroduce the the renderer switch button in the menu bar. This is something which will be changed very rarely during development for that it takes quite some space.

I was thinking the same thing. Especially since the new renderer names are much longer than GLES3/GLES2 (as they were in 3.x). I was thinking I would enable it here and then let the UI/UX people fix/remove it as they see fit.

aaronfranke · 2022-09-08T21:24:30Z

Why hide the rendering API from the user? Why is GLES3 just called "Compatibility"? I think it would make sense to let users know that it's GLES3. Same for the Vulkan ones, I don't see a reason to hide that it's Vulkan.

HeadClot · 2022-09-08T22:07:12Z

Why hide the rendering API from the user? Why is GLES3 just called "Compatibility"? I think it would make sense to let users know that it's GLES3. Same for the Vulkan ones, I don't see a reason to hide that it's Vulkan.

I am with aaronfranke on this. I think that hiding the api name from user will lead to confusion. At least put the API in question in the bullet points below clustered, mobile, and Compatibility. My 2 cents.

Calinou · 2022-09-09T00:08:08Z

Why hide the rendering API from the user? Why is GLES3 just called "Compatibility"? I think it would make sense to let users know that it's GLES3. Same for the Vulkan ones, I don't see a reason to hide that it's Vulkan.

In 4.0, the aim is to put less emphasis on the rendering driver (Vulkan, Direct3D, …) and more emphasis on the rendering backend, which is ultimately what matters more from a feature perspective. Different drivers can be used for a same rendering backend.

The Clustered and Mobile backends will be usable both with Vulkan and Direct3D 12 depending on the target hardware and user preference¹. The Compatibility backend will be usable both with OpenGL directly and with ANGLE (for GPUs where OpenGL support is problematic).

At the end of the day, what matters the most is which features are supported by the rendering backend you're using. This is what will potentially limit what you can do in your project, along with various performance and usability caveats. I'd say that in 2022, knowing which graphics API is used behind the scenes is an implementation detail. It will still be printed on startup (and can be shown in the About dialog), but I think this is a sensible move overall.

As I understand it, Vulkan will remain the defaulton Windows, but there will be a project setting to use Direct3D 12 on Windows instead. This would allow you to benefit from windowed gaming optimizations and Auto HDR on Windows 11. ↩

aaronfranke · 2022-09-09T01:13:15Z

@Calinou I disagree, it's extremely relevant, not an implementation detail.

Graphics APIs are something that every game developer has some knowledge about, and even many gamers know about the existence of Vulkan, OpenGL, and DirectX.
The choice of graphics API is directly related to the hardware it can run on. If somebody knows what specific devices they want to support, they can look up which APIs those devices support.

platform/android/export/export_plugin.cpp

m4gr3d · 2022-09-09T04:10:44Z

I blindly made some changes to the iOS and Android platforms, would be nice if someone from each team can confirm that the changes look okay.

@clayjohn The Android changes looks good.

clayjohn · 2022-09-09T05:25:19Z

Why hide the rendering API from the user? Why is GLES3 just called "Compatibility"? I think it would make sense to let users know that it's GLES3. Same for the Vulkan ones, I don't see a reason to hide that it's Vulkan.

Its not hidden. OpenGL3 is the driver, not the renderer. The choice of driver is still presented to the user. On top of that, Users have never been able to explicitly set the Rendering API. The GLES3 driver mostly confirmed to OpenGL ES 3.0 like the name implies, but on web it is limited to the set of features in the WebGL specification and on Desktop it is limited by OpenGL 3.3. This PR lets us actually expose the selection of rendering API to the user.

_Edit: and that's without mentioning the fact that the GLES3 renderer in Godot 3.x relied on extensions which were not part of basic OpenGL 3,3 / OpenGL 3.0 / WebGL2. So it couldn't run on many devices that advertised OpenGL ES 3.0 support. Again, this PR makes the situation better in that regard, not worse. _

BastiaanOlij · 2022-09-09T05:42:52Z

Sorry @clayjohn , bit late to the discussion and about to potentially throw a monkey wrench into it :)

Looking to the (near) future and the wish to make the engine more extendable with more implementations, either full renderers like the GLES3 one (possibly a raytracing backend that was discussed), different drivers such as our DirectX backend that works with the RD renderer as a replacement/alternative to Vulkan, or additional RD renderers such as the planned deferred renderer, having these selections hardcoded the way that we do will become a nightmare.

I wonder if we need some way to register the available options and make the selection on the projection creation window and dropdown selection more generic?

YuriSizov · 2022-09-09T10:15:41Z

I don't see harm in printing the supported driver/API in the hint text for each option. That's definitely not something that would stand in the way of this PR, just a bit about how we present the information to the user.

Options and how they separate target platforms make sense to me. The names may need to be bikeshed, but it's not too critical.

clayjohn · 2022-09-09T16:33:21Z

Notes from September 9th meeting:
~~1. Change compatibility to gl_compatibility~~
~~2. Have two codepaths one for compatibility and one for rendering_device~~
~~3. Rename clustered to forward+~~

clayjohn · 2022-09-12T21:11:47Z

Updated to:

remove GL_Compatibility renderer from project selection (will expose in another PR)
remove renderer selection from upper-right hand corner of editor (needs more discussion how this should be exposed)
rename "clustered" renderer to "forward_plus"
rename "compatibility" renderer to "gl_compatibility"

Should now be ready for review by @reduz

clayjohn · 2022-09-12T21:18:59Z

@aaronfranke @HeadClot I have removed the change to the project manager and I have updated the top post to highlight how this PR allows users to directly choose their graphics API for selected rather than before where it was fixed based on renderer choice.

Please let me know if you find this more suitable. To be perfectly frank, I don't fully understand your opposition to this PR in the first. We can't name the mobile renderer "Vulkan Mobile" because it may be running over Metal, WebGPU, D3D12, or Vulkan. One thing we could do is expose renderer choice and driver choice in the project manager so users can explicitly choose the renderer they want to us and the graphics API that they want to target.

aaronfranke · 2022-09-12T22:39:30Z

@clayjohn Good points. With Metal, as far as I know it's using MoltenVK, which is Vulkan on top of Metal, but I guess maybe this isn't the case for D3D12 and WebGPU. Anyway, the menus you presented make a lot of sense to me, so the driver name is still shown, just not in the project manager. How about something like this?

clayjohn · 2022-09-12T23:33:44Z

@clayjohn Good points.With Metal, as far as I know it's using MoltenVK, which is Vulkan on top of Metal, but I guess maybe this isn't the case for D3D12 and WebGPU. Anyway, the menus you presented make a lot of sense to me, so the driver name is still shown, just not in the project manager. How about something like this?

That seems fine to me. Although to be clear, I have reverted the project manager changes in this PR and I won't make further changes in this PR. Those changes will have to be made in another PR. This PR is just about updating project settings now. I understand that reduz wants to make a proposal for the Renderer/Driver selection in the Project Manager.

reduz · 2022-09-14T08:50:32Z

Looks good to me, we may need to clean up main at some point, but can be done at another time. Need agreement from @BastiaanOlij about renaming the renderer scene class to rendering method.

BastiaanOlij

Looks good to me, I'm happy with the rename :)

clayjohn · 2022-09-15T20:29:45Z

Rebased, so should be ready to merge! @akien-mga

One thing I shouldv'e highlighted in the original post is that projects made in an earlier 4.0 version will have the project setting warning label appear on older projects in the ProjectManager. The warning can be safely ignored, but we will want to flag it in the next couple of blog posts:

akien-mga · 2022-09-16T08:48:20Z

One thing I shouldv'e highlighted in the original post is that projects made in an earlier 4.0 version will have the project setting warning label appear on older projects in the ProjectManager. The warning can be safely ignored, but we will want to flag it in the next couple of blog posts:

To avoid confusion, maybe we can keep the old names temporarily for compatibility, and add some code to actually remove them from project.godot when opening in beta 2+?

doc/classes/ProjectSettings.xml

core/config/project_settings.cpp

doc/classes/ProjectSettings.xml

akien-mga · 2022-09-16T08:52:19Z

Looks fine overall, should be good to merge once the remaining comments are solved.

clayjohn · 2022-09-16T14:53:14Z

Looks fine overall, should be good to merge once the remaining comments are solved.

Thanks for checking! Pushed a fix with your suggested changes

core/config/project_settings.cpp

editor/project_manager.cpp

…ng_driver. To differentiate between a driver (e.g. Vulkan or D3D12) and a renderer (e.g. clustered or mobile renderer).

clayjohn · 2022-09-19T17:27:06Z

@akien-mga @aaronfranke Thank you both for reviewing! All of your comments should be addressed now

akien-mga · 2022-09-20T07:46:25Z

Thanks!

clayjohn added enhancement topic:core topic:rendering labels Sep 8, 2022

clayjohn added this to the 4.0 milestone Sep 8, 2022

clayjohn requested review from BastiaanOlij and reduz September 8, 2022 20:19

clayjohn requested review from a team as code owners September 8, 2022 20:19

m4gr3d reviewed Sep 9, 2022

View reviewed changes

platform/android/export/export_plugin.cpp Show resolved Hide resolved

clayjohn force-pushed the renderer-setting branch from 38b5fc5 to 3f08a3c Compare September 12, 2022 21:09

BastiaanOlij approved these changes Sep 14, 2022

View reviewed changes

clayjohn force-pushed the renderer-setting branch from 5d8d45f to d253a42 Compare September 15, 2022 20:28

akien-mga reviewed Sep 16, 2022

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

akien-mga reviewed Sep 16, 2022

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

akien-mga reviewed Sep 16, 2022

View reviewed changes

core/config/project_settings.cpp Outdated Show resolved Hide resolved

akien-mga reviewed Sep 16, 2022

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

clayjohn force-pushed the renderer-setting branch from d253a42 to e19837c Compare September 16, 2022 14:52

aaronfranke requested changes Sep 16, 2022

View reviewed changes

core/config/project_settings.cpp Outdated Show resolved Hide resolved

editor/project_manager.cpp Outdated Show resolved Hide resolved

Split rendering driver project setting into renderer_name and renderi…

4a1c7de

…ng_driver. To differentiate between a driver (e.g. Vulkan or D3D12) and a renderer (e.g. clustered or mobile renderer).

clayjohn force-pushed the renderer-setting branch from e19837c to 4a1c7de Compare September 19, 2022 17:26

aaronfranke approved these changes Sep 19, 2022

View reviewed changes

akien-mga approved these changes Sep 20, 2022

View reviewed changes

akien-mga merged commit 7da5322 into godotengine:master Sep 20, 2022

clayjohn deleted the renderer-setting branch September 20, 2022 14:44

akien-mga mentioned this pull request Sep 28, 2022

Project supported features: Ignore old 'Vulkan Clustered/Forward' names #66570

Merged

akien-mga changed the title ~~Split rendering driver project setting into renderer_name and rendering_driver~~ Split rendering driver project setting into rendering_method and rendering/*/driver Sep 29, 2022

akien-mga added the breaks compat label Sep 29, 2022

clayjohn mentioned this pull request Sep 30, 2022

Use mobile as the default rendering method on mobile when Vulkan is supported #66637

Merged

This was referenced Oct 6, 2022

Polish rendering driver refactor further #67000

Merged

Polish rendering driver refactor further (take 2) #67244

Merged

akien-mga mentioned this pull request Dec 2, 2022

Add an editor option to copy system info to clipboard #65902

Merged

RedMser mentioned this pull request Dec 9, 2022

[TRACKER] TODOs in class documentation #65576

Closed

6 tasks

FyiurAmron mentioned this pull request Oct 4, 2023

[macOS/Windows] Add optional ANGLE backed OpenGL renderer support (runtime backend selection). #72831

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split rendering driver project setting into `rendering_method` and `rendering/*/driver` #65541

Split rendering driver project setting into `rendering_method` and `rendering/*/driver` #65541

clayjohn commented Sep 8, 2022 •

edited

Loading

Geometror commented Sep 8, 2022

clayjohn commented Sep 8, 2022

aaronfranke commented Sep 8, 2022 •

edited

Loading

HeadClot commented Sep 8, 2022 •

edited

Loading

Calinou commented Sep 9, 2022 •

edited

Loading

aaronfranke commented Sep 9, 2022

m4gr3d commented Sep 9, 2022

clayjohn commented Sep 9, 2022 •

edited

Loading

BastiaanOlij commented Sep 9, 2022

YuriSizov commented Sep 9, 2022

clayjohn commented Sep 9, 2022 •

edited

Loading

clayjohn commented Sep 12, 2022

clayjohn commented Sep 12, 2022

aaronfranke commented Sep 12, 2022 •

edited

Loading

clayjohn commented Sep 12, 2022

reduz commented Sep 14, 2022

BastiaanOlij left a comment

clayjohn commented Sep 15, 2022

akien-mga commented Sep 16, 2022

akien-mga commented Sep 16, 2022

clayjohn commented Sep 16, 2022

clayjohn commented Sep 19, 2022

akien-mga commented Sep 20, 2022

Split rendering driver project setting into rendering_method and rendering/*/driver #65541

Split rendering driver project setting into rendering_method and rendering/*/driver #65541

Conversation

clayjohn commented Sep 8, 2022 • edited Loading

Geometror commented Sep 8, 2022

clayjohn commented Sep 8, 2022

aaronfranke commented Sep 8, 2022 • edited Loading

HeadClot commented Sep 8, 2022 • edited Loading

Calinou commented Sep 9, 2022 • edited Loading

Footnotes

aaronfranke commented Sep 9, 2022

m4gr3d commented Sep 9, 2022

clayjohn commented Sep 9, 2022 • edited Loading

BastiaanOlij commented Sep 9, 2022

YuriSizov commented Sep 9, 2022

clayjohn commented Sep 9, 2022 • edited Loading

clayjohn commented Sep 12, 2022

clayjohn commented Sep 12, 2022

aaronfranke commented Sep 12, 2022 • edited Loading

clayjohn commented Sep 12, 2022

reduz commented Sep 14, 2022

BastiaanOlij left a comment

Choose a reason for hiding this comment

clayjohn commented Sep 15, 2022

akien-mga commented Sep 16, 2022

akien-mga commented Sep 16, 2022

clayjohn commented Sep 16, 2022

clayjohn commented Sep 19, 2022

akien-mga commented Sep 20, 2022

Split rendering driver project setting into `rendering_method` and `rendering/*/driver` #65541

Split rendering driver project setting into `rendering_method` and `rendering/*/driver` #65541

clayjohn commented Sep 8, 2022 •

edited

Loading

aaronfranke commented Sep 8, 2022 •

edited

Loading

HeadClot commented Sep 8, 2022 •

edited

Loading

Calinou commented Sep 9, 2022 •

edited

Loading

clayjohn commented Sep 9, 2022 •

edited

Loading

clayjohn commented Sep 9, 2022 •

edited

Loading

aaronfranke commented Sep 12, 2022 •

edited

Loading