Skip to content

Make the behavior of rendering/rendering_device/driver consistent with --rendering-device #109207

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make the behavior of rendering/rendering_device/driver consistent with --rendering-device #109207

wants to merge 1 commit into from
+3 −0

Conversation

scgm0
Copy link
Contributor

@scgm0 scgm0 commented Aug 1, 2025

Currently, the --rendering-device command supports direct switching between d3d12, vulkan, and opengl3, among others. Although the rendering/rendering_device/driver setting in the configuration file can also be set to opengl3, doing so may cause rendering bugs in child windows:
图片
Through examining the source code, I discovered that this occurs because when reading --rendering-device, if rendering_driver is set to opengl3, it automatically sets rendering_method to gl_compatibility. However, reading from the configuration file does not trigger this behavior, so I added it manually.
This is not the conventional usage of rendering/rendering_device/driver, but I'd like to allow users to switch rendering APIs via override.cfg in my project. Whether it's d3d12, vulkan, or opengl3, different users have reported rendering issues with each. Previously, users could only switch APIs using --rendering-device, which wasn't very convenient. Enabling rendering/rendering_device/driver to be properly set to opengl3 would be much more user-friendly (sparing users the need to simultaneously change both rendering/renderer/rendering_method and rendering/rendering_device/driver).

@scgm0 scgm0 requested a review from a team as a code owner August 1, 2025 19:18
@clayjohn clayjohn added this to the 4.x milestone Aug 1, 2025
Calinou
Calinou approved these changes Aug 1, 2025
View reviewed changes
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch from aaa4262 to e819f49 Compare August 3, 2025 17:57
@clayjohn
Copy link
Member

clayjohn commented Aug 4, 2025

Has anyone had trouble with this, or are we just making this change because we can?

To me to seems incredibly confusing to give the users the option to set a rendering driver that we know doesn't exist.

It's different with command line arguments because we can't give immediate feedback. With project settings, we can immediately tell the user that the value they input is invalid if the property accepts a string, or not provide the option if we can provide a fixed list.

@scgm0
Copy link
Contributor Author

scgm0 commented Aug 4, 2025

Has anyone had trouble with this, or are we just making this change because we can?

To me to seems incredibly confusing to give the users the option to set a rendering driver that we know doesn't exist.

It's different with command line arguments because we can't give immediate feedback. With project settings, we can immediately tell the user that the value they input is invalid if the property accepts a string, or not provide the option if we can provide a fixed list.

As I mentioned, it's not a conventional usage. In my own application, I provide a fixed list, but certain edge cases require users to manually modify the configuration file (for example, using d3d12 in wine—the application runs, but the screen remains entirely black, which doesn't trigger a fallback to the rendering driver). If users choose to edit the configuration file manually, they should know what they're doing, and even if they make a mistake, they still have a chance to correct it. (Note: The users here are not Godot users but rather users of the application I developed using Godot.)

Even without this PR, the valid options still include opengl3_x, just as RenderingServer.get_current_rendering_driver_name() would return opengl3_x. However, directly setting rendering/rendering_device/driver to opengl3_x would cause a bug due to an incorrect rendering/renderer/rendering_method.

Currently, it indeed lacks a check. I can add one—should I?

@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch from e819f49 to ac69f50 Compare August 6, 2025 13:15
@Calinou
Copy link
Member

Calinou commented Aug 6, 2025

or not provide the option if we can provide a fixed list.

Note that the engine currently doesn't provide this list for projects to use, but #109247 makes it possible.

@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch 3 times, most recently from dc65c51 to 500befd Compare August 12, 2025 18:40
@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch 4 times, most recently from c806140 to 098321b Compare August 19, 2025 22:51
@scgm0
Copy link
Contributor Author

scgm0 commented Aug 20, 2025 •
edited
Loading

@clayjohn How to promote the merging of this PR?

@clayjohn
Copy link
Member

@clayjohn How to promote the merging of this PR?

You can make a proposal and see if other people have the same use-case as you.

@scgm0
Copy link
Contributor Author

scgm0 commented Aug 20, 2025 •
edited
Loading

@clayjohn How to promote the merging of this PR?

You can make a proposal and see if other people have the same use-case as you.

However, I believe this PR also falls under bug fixes. Currently, rendering/rendering_device/driver allows users to freely switch between vulkan, d3d12, or opengl3_x. When set to opengl3_x, the main window can render normally, but because rendering/renderer/rendering_method does not change to gl_compatibility, it causes rendering issues with child windows.

@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch 3 times, most recently from 36ac3c6 to c132bc8 Compare August 30, 2025 10:36
@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch from c132bc8 to 8018c46 Compare September 2, 2025 07:50
@scgm0 scgm0 force-pushed the Fix-RenderingDevice-Opengl3 branch from 8018c46 to 6525267 Compare September 3, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Calinou Calinou Calinou approved these changes

Assignees
No one assigned
Labels
enhancement topic:core topic:rendering
Projects
None yet
Milestone
4.x
Development

Successfully merging this pull request may close these issues.
3 participants
@scgm0 @clayjohn @Calinou