Skip to content

Commit

Permalink
Display useful pixel shader compilation errors (#17436)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
blitzRahul authored Jun 21, 2024
1 parent 34d4dc5 commit fa40733
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 11 deletions.
7 changes: 4 additions & 3 deletions src/cascadia/TerminalControl/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ Please either install the missing font or choose another one.</value>
<comment>{0} is a file name</comment>
</data>
<data name="PixelShaderCompileFailed" xml:space="preserve">
<value>Unable to compile the specified pixel shader.</value>
<value>Pixel shader failed to compile: {0}</value>
<comment>{0} is the error message generated by the compiler</comment>
</data>
<data name="UnexpectedRendererError" xml:space="preserve">
<value>Renderer encountered an unexpected error: {0}</value>
Expand All @@ -223,7 +224,7 @@ Please either install the missing font or choose another one.</value>
</data>
<data name="RendererErrorOther" xml:space="preserve">
<value>Renderer encountered an unexpected error: {0:#010x} {1}</value>
<comment>{Locked="{0:#010x}","{1}"} {0:#010x} is a placeholder for a Windows error code (e.g. 0x88985002). {1} is the corresponding message.</comment>
<comment>{Locked="{0:#010x}","{1}"} {0:#010x} is a placeholder for a Windows error code (e.g. 0x88985002). {1} is the corresponding message. {2} is the filename.</comment>
</data>
<data name="TermControlReadOnly" xml:space="preserve">
<value>Read-only mode is enabled.</value>
Expand Down Expand Up @@ -320,4 +321,4 @@ Please either install the missing font or choose another one.</value>
<value>Suggested input: {0}</value>
<comment>{Locked="{0}"} {0} will be replaced with a string of input that is suggested for the user to input</comment>
</data>
</root>
</root>
11 changes: 9 additions & 2 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
message = winrt::hstring{ fmt::format(std::wstring_view{ RS_(L"PixelShaderNotFound") }, parameter) };
break;
case D2DERR_SHADER_COMPILE_FAILED:
message = winrt::hstring{ fmt::format(std::wstring_view{ RS_(L"PixelShaderCompileFailed") }) };
message = winrt::hstring{ fmt::format(std::wstring_view{ RS_(L"PixelShaderCompileFailed") }, parameter) };
break;
case DWRITE_E_NOFONT:
message = winrt::hstring{ fmt::format(std::wstring_view{ RS_(L"RendererErrorFontNotFound") }, parameter) };
Expand All @@ -1175,7 +1175,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
wchar_t buf[512];
const auto len = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, hr, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), &buf[0], ARRAYSIZE(buf), nullptr);
const std::wstring_view msg{ &buf[0], len };
message = winrt::hstring{ fmt::format(std::wstring_view{ RS_(L"RendererErrorOther") }, hr, msg) };
std::wstring resourceString = RS_(L"RendererErrorOther").c_str();
//conditional message construction
std::wstring partialMessage = fmt::format(std::wstring_view{ resourceString }, hr, msg);
if (!parameter.empty())
{
fmt::format_to(std::back_inserter(partialMessage), LR"( "{0}")", parameter);
}
message = winrt::hstring{ partialMessage };
break;
}
}
Expand Down
19 changes: 13 additions & 6 deletions src/renderer/atlas/BackendD3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "dwrite.h"
#include "wic.h"
#include "../../types/inc/ColorFix.hpp"
#include "../../types/inc/convert.hpp"

#if ATLAS_DEBUG_SHOW_DIRTY || ATLAS_DEBUG_COLORIZE_GLYPH_ATLAS
#include <til/colorbrewer.h>
Expand Down Expand Up @@ -451,15 +452,21 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
{
if (error)
{
LOG_HR_MSG(hr, "%.*hs", static_cast<int>(error->GetBufferSize()), static_cast<char*>(error->GetBufferPointer()));
if (p.warningCallback)
{
//to handle compile time errors
const std::string_view errMsgStrView{ static_cast<const char*>(error->GetBufferPointer()), error->GetBufferSize() };
const auto errMsgWstring = ConvertToW(CP_ACP, errMsgStrView);
p.warningCallback(D2DERR_SHADER_COMPILE_FAILED, errMsgWstring);
}
}
else
{
LOG_HR(hr);
}
if (p.warningCallback)
{
p.warningCallback(D2DERR_SHADER_COMPILE_FAILED, p.s->misc->customPixelShaderPath);
if (p.warningCallback)
{
//to handle errors such as file not found, path not found, access denied
p.warningCallback(hr, p.s->misc->customPixelShaderPath);
}
}
}

Expand Down

0 comments on commit fa40733

Please sign in to comment.