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

Update rendering driver name on fallbacks. Fix rendering driver/method in the editor system info. #95887

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Aug 21, 2024

  • Fixes wrong names used in HelpCopy System Info (was using project settings, instead of real current driver/method).
  • Add missing Direct3D 12 to the list.
  • Set saved driver name if it was changed by fallback

Might fix some issues with shaders when D3D12 ↔ Vulkan fallback is used, since saved name is used to set defines:

builder.append(String("#define RENDER_DRIVER_") + OS::get_singleton()->get_current_rendering_driver_name().to_upper() + "\n");

Fixes #96380

}
if (driver_name == "vulkan") {
driver_name = "Vulkan";
} else if (driver_name == "d3d12") {
driver_name = "Direct3D 12";
} else if (driver_name == "opengl3_angle") {
Copy link
Member

Choose a reason for hiding this comment

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

If we're making a special case for opengl3_angle, we might as well do one for opengl3_es too.

Copy link
Member Author

@bruvzg bruvzg Sep 1, 2024

Choose a reason for hiding this comment

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

I guess it this case, we should change opengl3 to return OpenGL 3 instead of GLES3 (but this won't be correct in case of web and mobile, which always use ES).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to return OpenGL 3 or OpenGL ES 3 based on actually used backend.

@akien-mga akien-mga added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 3, 2024
@akien-mga akien-mga merged commit 262c8da into godotengine:master Sep 3, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member

This breaks compiling with MSVC, it seems to introduce bad declares by introducing platform code, suspect platform/windows/platform_gl.h

Trying to create a fix

@bruvzg
Copy link
Member Author

bruvzg commented Sep 3, 2024

This breaks compiling with MSVC, it seems to introduce bad declares by introducing platform code, suspect platform/windows/platform_gl.h

Strange, CI runs MSVC and passed.

@AThousandShips
Copy link
Member

AThousandShips commented Sep 3, 2024

Starting with a simple undefine series but might be something that can be configured for the include to block the defines, can alternatively build editor_node.cpp separately in scu

I think the errors only occur in SCU builds, and are introduced by the inclusion of the rasterizer in editor_node.cpp

@bruvzg
Copy link
Member Author

bruvzg commented Sep 3, 2024

simple undefine series but might be something that can be configured for the include to block the defines, can alternatively build editor_node.cpp separately in scu

Or we can add another internal OS method like set_current_rendering_method() and remove the include, actual value is set in the same init code that sets set_current_rendering_method.

@AThousandShips
Copy link
Member

Sounds like a more coherent solution, making a PR to split up the SCU as a band-aid while that solution can be implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy System Info shows incorrect rendering method when overriden on command line
3 participants