D3D12: Fix shader model check, initialization error handling

[KeyboardDanni]

Partial fix for #98207 and #98848

The D3D12 initialization code was checking whether the calls to CheckFeatureSupport() succeeded, but not the actual returned data, so it would continue initializing even if the Shader Model of the device was too old.

Additionally, if this code were to throw an initialization error, Godot would crash trying to uninitialize partially initialized data, due either to missing checks or bad init/deinit order.

This is only a partial fix because it doesn't automatically fallback to GL, but it at least displays a friendly error dialog (with instructions on switching renderer) instead of crashing, which is better than nothing.

In order for the automatic fallback to work, the Shader Model version check would have to be moved into the ContextDriver, but that requires the ID3D12Device which lives in (and is initialized by) the DeviceDriver.

I also bumped the minimum required Shader Model version from 6.0 to 6.1 per this comment.

[KeyboardDanni]

cc @DarioSamo , looking for your input on Shader Model version.

[DarioSamo]

The suggested change makes sense, but there seems to be unrelated changes polluting the commit a bit. Are they intentional? It might be best to move them to a separate PR if they are.

[KeyboardDanni]

Not unrelated. They're required in order for the D3D12 driver to uninitialize without Godot crashing.

[DarioSamo]

Not unrelated. They're required in order for the D3D12 driver to uninitialize without Godot crashing.

Wouldn't some of these imply any driver can cause the engine to crash if it fails to initialize then, not just D3D12? It seems odd for it to be able to reach some of the code that was changed in the first place if that happens.

[KeyboardDanni]

Wouldn't some of these imply any driver can cause the engine to crash if it fails to initialize then, not just D3D12? It seems odd for it to be able to reach some of the code that was changed in the first place if that happens.

Some of the stuff was getting initialized in the wrong order. This just fixes that so we don't have partial frames initialization causing the cleanup code to think there's resources that aren't there.

This stuff could probably use null checks, but I'm unsure if that's by design for performance reasons.

[DarioSamo]

The GLSL change makes sense to me, but I think we should probably investigate a bit deeper into why it can possibly reach the code that was modified for RenderingDevice. It's just a bit of a small code smell to me that that happens in the first place. I'll check in a bit.

[KeyboardDanni]

I've been testing this by commenting out shader_capabilities.shader_model = shader_model.HighestShaderModel; on line 5830. That's how I arrived at the changes to RenderingDevice.

I think in general, renderer backend init/deinit could use a bit of refactoring. I'm just unsure of the best way to do that as someone who's relatively new to this aspect of the codebase.

[DarioSamo]

This line should be wrapped with if frames not empty check based on the render graph's own state instead.

godot/servers/rendering/rendering_device_graph.cpp

Line 1578 in e0603ae

_wait_for_secondary_command_buffer_tasks();

[Repiteo]

@KeyboardDanni Have you gotten the chance to integrate Dario's suggestions?

[KeyboardDanni]

@Repiteo Requested change has been made.

[DarioSamo]

Changes look good to me, but I'm a bit suspect about the SM 6.1 change.

As per this page, the additions to SM 6.1 seem to be:

[SV_ViewID](https://github.com/microsoft/DirectXShaderCompiler/wiki/SV_ViewID). This feature enables instancing of the graphics pipeline by "view", in a manner that is orthogonal to draw instancing.
[SV_Barycentrics](https://github.com/microsoft/DirectXShaderCompiler/wiki/SV_Barycentrics). This is a new system-generated value available in pixel shaders, used for example to perform interpolation over small or unaligned values like a few bits from a 32-bit value.

ViewID is only relevant for multiview (XR), so unless it is enabled, it won't get compiled. And I don't think there's a GLSL construct for barycentrics. I'm not sure the provided comment is good enough of a justification to bump the minimum SM required unless we get something that proves it necessary.

[KeyboardDanni]

Could someone gently prod the CI? It failed on web build with a 503 trying to get emsdk.