LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.03k stars 135 forks source link

LLGL::CreateShader cannot be set to a target of Shader Model 6.0 or above. #93

Closed gdianaty closed 1 year ago

gdianaty commented 1 year ago

The Problem

LLGL's D3D12 backend currently lacks support for the DirectX Shader Compiler (DXC) API, but currently uses the Effect Compiler Tool (FXC) API. As a result, LLGL's built-in shader compiler interface (CodeFile/CodeString) is unable to compile beyond Shader Model 5.0.

The (Proposed) Solution

My proposal to allow D3D12Shader to compile SM6 is located at my branch of the repo: https://github.com/gdianaty/LLGL.

Some key points (both strong and weak) of my proposal are:

This is an issue and not a PR because I feel that this is a proposal and not appropriate for PR status, even under review. I also hope other folks may be able to point out potential solutions that I am missing at this time.

LukasBanana commented 1 year ago

Hi, your proposal looks really great!

The only thing I would change is to strengthen the robustness of IsProfileDxcAppropriate even if it's just validating there's no early NUL-terminator '\0'. Other than that, parsing the target profile string is probably the best way to check for the SM version since that's what both FXC and DXC consume anyway. Having that said, you also have to be aware of different profile name lengths, e.g. vs_6_0 vs lib_6_0 although the latter one is not yet supported in LLGL.

Let me take a closer look at your fork tomorrow but feel free to open a PR.

gdianaty commented 1 year ago

Thank you! I was really unsure of the appropriateness of some of my changes to LLGL (as I assumed the lack of DXC support was an intentional move for ease-of-use).

I had not yet considered lib_6_0, but that is an excellent point! Perhaps a more fitting (and slightly less C-flavored) approach would be to work forwards through the string until 1st underscore and then atoi'ing it? That should increase robustness in several ways.

LukasBanana commented 1 year ago

I intended to keep LLGL low on external dependencies indeed, but compiling SM6 HLSL from source would be a nice addition nonetheless as long as the dependency to DXC remains optional.

Using atoi sounds good. I am leaning more and more towards C functions anyway.

gdianaty commented 1 year ago

A few edits made to my branch since yesterday evening:

LukasBanana commented 1 year ago

I opened a pull request to merge your changes into #94 and for further discussions. This way it's a bit easier to make comments on specific source locations.

LukasBanana commented 1 year ago

This proposal has been integrated into master branch and some adjustments have been made in 51fdefb.