Display useful pixel shader compilation errors

[blitzRahul]

More descriptive warnings are triggered when custom pixel shader compilation fails.

If D3DCompileFromFile fails and the compiler generates an error message- the message is converted to a wstring and is sent as a parameter when calling p.warningCallback.
Changes were made to resources.resw and TermControl.cpp to accommodate this.

Validation Steps Performed

I tested the following errors that may be encountered while developing a custom pixel shader:

1. Compile time errors
2. File not found error
3. Path not found error
4. Access denied error

Fixes #17435

TAEF tests passed:
Summary: Total=294, Passed=294, Failed=0, Blocked=0, Not Run=0, Skipped=0

[blitzRahul]

@microsoft-github-policy-service agree

[DHowett]

Thanks so much for doing this! It will be a huge quality of life improvement for shader authors. Just a quick question :)

[DHowett]

Is there a chance that parameter will be empty for non-shader-related Renderer errors? If so, should we conditionally add the filename or parameter to the message, when we construct it?

[blitzRahul]

Hello Dustin! I stared at the code for a while, I think it is safe to conclude that parameter won't be empty for non-shader-related Renderer errors.

BackendD3D.cpp Line 453 to 470

In the event that the compiler returns an error (faulty shader code), parameter = compiler related error message, else parameter = file path. Unless D3DCompileFromFile function, line 409 BackendD3D.cpp returns an invalid hr, we should not have problems.

TermControl.cpp _RendererWarning function

I am new to this project, infact this is my first contribution; I have a lot of code analysis to do before I can make claims/conclusions with resonable confidene. So, let me know if I'm missing something here : )

Edit: I had a quick question. What about other languages? It would be nice to have native language speakers edit the respective Resource.resw files; reason being that online translators aren't exactly accurate.

[DHowett]

Sorry, I missed your edit! Don't worry too much about other languages - the automatic translation service should come through and automatically update the messages.

Your analysis makes sense of when and where the filename/parameters come from, as well. I appreciate it!

[DHowett]

Oh, I think I found what I was worried about!

There is one more call to p.warningCallback in AtlasEngine.r.cpp:

    if (_p.warningCallback)
    {
        try
        {
            _p.warningCallback(hr, {});
        }
        CATCH_LOG()
    }

If the render engine fails for a non-shader-related reason, like DirectWrite ran out of memory or the graphics hardware did not respond in time, I think we will display an error message with empty quotes in it:

/!\ Error                                                                 [X]

The renderer encountered an unexpected error: 0x80070057 Invalid Parameter ""

                                                                       [ OK ]

[blitzRahul]

I see the issue now! I made a commit to fix it, is the implementation fine?

[DHowett]

Suggested change

	if (!parameter.empty())
	{
	resourceString += L" \"{2}\"";
	message = winrt::hstring{ fmt::format(std::wstring_view{ resourceString }, hr, msg, parameter) };
	}
	else
	{
	message = winrt::hstring{ fmt::format(std::wstring_view{ resourceString }, hr, msg) };
	}
	std::wstring partialMessage = fmt::format(std::wstring_view{ resourceString }, hr, msg);
	if (!parameter.empty())
	{
	fmt::format_to(std::back_inserter(partialMessage), FMT_COMPILE(LR"( "{2}")"), parameter);
	}
	message = winrt::hstring{ partialMessage };

How does this sound? It simplifies the logic (there is only one format call that uses the resource string) and reduces the dependency on the number of format specifiers in the resource (e.g. we don't need to be as careful when we change the resource string)

[blitzRahul]

It made sense; learnt something new today! Thank you for the review!
Edit: also next time I'll stick to referencing code via text and not screenshots, I imagine it makes a little bit of a mess.

[DHowett]

Thank you for bearing with my questions! I really appreciate you contributing this fix - this problem has annoyed me too 🙂

[DHowett]

Pulled the screenshots out of the commit message body:

Appropriate warnings were generated
Compile time error:

File not found:

Access denied: