Skip to content

Commit

Permalink
AtlasEngine: Minor bug fixes (#16219)
Browse files Browse the repository at this point in the history
This commit fixes 4 minor bugs:
* Forgot to set the maximum swap chain latency. Without it, it defaults
  to up to 3 frames of latency. We don't need this, because our renderer
  is simple and fast and is expected to draw frames within <1ms.
* ClearType treats the alpha channel as ignored, whereas custom shaders
  can manipulate the alpha channel freely. This meant that using both
  simultaneously would produce weird effects, like text having black
  background. We now force grayscale AA instead.
* The builtin retro shader should not be effected by the previous point.
* When the cbuffer is entirely unused in a custom shader, it has so far
  resulted in constant redraws. This happened because the D3D reflection
  `GetDesc` call will then return `E_FAIL` in this situation.
  The new code on the other hand will now assume that a failure
  to get the description is equal to the variable being unused.

Closes #15960

## Validation Steps Performed
* A custom passthrough shader works with grayscale and ClearType AA
  while also changing the opacity with Ctrl+Shift+Scroll ✅
* Same for the builtin retro shader, but ClearType works ✅
* The passthrough shader doesn't result in constant redrawing ✅

(cherry picked from commit 0289cb0)
Service-Card-Id: 90915276
Service-Version: 1.18
  • Loading branch information
lhecker authored and DHowett committed Nov 7, 2023
1 parent dc0cbf8 commit a3caf55
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 17 deletions.
14 changes: 7 additions & 7 deletions src/renderer/atlas/AtlasEngine.api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,20 +490,20 @@ void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept

void AtlasEngine::_resolveTransparencySettings() noexcept
{
// An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain().
// We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs.
const bool useAlpha = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty();
// If the user asks for ClearType, but also for a transparent background
// (which our ClearType shader doesn't simultaneously support)
// then we need to sneakily force the renderer to grayscale AA.
const auto antialiasingMode = _api.enableTransparentBackground && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode;
const bool enableTransparentBackground = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty() || _api.s->misc->useRetroTerminalEffect;
const auto antialiasingMode = useAlpha && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode;

if (antialiasingMode != _api.s->font->antialiasingMode || enableTransparentBackground != _api.s->target->enableTransparentBackground)
if (antialiasingMode != _api.s->font->antialiasingMode || useAlpha != _api.s->target->useAlpha)
{
const auto s = _api.s.write();
s->font.write()->antialiasingMode = antialiasingMode;
// An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain().
// We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs.
s->target.write()->enableTransparentBackground = enableTransparentBackground;
_api.backgroundOpaqueMixin = enableTransparentBackground ? 0x00000000 : 0xff000000;
s->target.write()->useAlpha = useAlpha;
_api.backgroundOpaqueMixin = useAlpha ? 0x00000000 : 0xff000000;
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/renderer/atlas/AtlasEngine.r.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ void AtlasEngine::_createSwapChain()
.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL,
// If our background is opaque we can enable "independent" flips by setting DXGI_ALPHA_MODE_IGNORE.
// As our swap chain won't have to compose with DWM anymore it reduces the display latency dramatically.
.AlphaMode = _p.s->target->enableTransparentBackground ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
.AlphaMode = _p.s->target->useAlpha ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
.Flags = swapChainFlags,
};

Expand Down Expand Up @@ -360,6 +360,8 @@ void AtlasEngine::_createSwapChain()
_p.swapChain.targetSize = _p.s->targetSize;
_p.swapChain.waitForPresentation = true;

LOG_IF_FAILED(_p.swapChain.swapChain->SetMaximumFrameLatency(1));

WaitUntilCanRender();

if (_p.swapChainChangedCallback)
Expand Down
20 changes: 12 additions & 8 deletions src/renderer/atlas/BackendD3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,30 +403,36 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
/* ppCode */ blob.addressof(),
/* ppErrorMsgs */ error.addressof());

// Unless we can determine otherwise, assume this shader requires evaluation every frame
_requiresContinuousRedraw = true;

if (SUCCEEDED(hr))
{
THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.put()));
THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.addressof()));

// Try to determine whether the shader uses the Time variable
wil::com_ptr<ID3D11ShaderReflection> reflector;
if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.put()))))
if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.addressof()))))
{
// Depending on the version of the d3dcompiler_*.dll, the next two functions either return nullptr
// on failure or an instance of CInvalidSRConstantBuffer or CInvalidSRVariable respectively,
// which cause GetDesc() to return E_FAIL. In other words, we have to assume that any failure in the
// next few lines indicates that the cbuffer is entirely unused (--> _requiresContinuousRedraw=false).
if (ID3D11ShaderReflectionConstantBuffer* constantBufferReflector = reflector->GetConstantBufferByIndex(0)) // shader buffer
{
if (ID3D11ShaderReflectionVariable* variableReflector = constantBufferReflector->GetVariableByIndex(0)) // time
{
D3D11_SHADER_VARIABLE_DESC variableDescriptor;
if (SUCCEEDED_LOG(variableReflector->GetDesc(&variableDescriptor)))
if (SUCCEEDED(variableReflector->GetDesc(&variableDescriptor)))
{
// only if time is used
_requiresContinuousRedraw = WI_IsFlagSet(variableDescriptor.uFlags, D3D_SVF_USED);
}
}
}
}
else
{
// Unless we can determine otherwise, assume this shader requires evaluation every frame
_requiresContinuousRedraw = true;
}
}
else
{
Expand All @@ -447,8 +453,6 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
else if (p.s->misc->useRetroTerminalEffect)
{
THROW_IF_FAILED(p.device->CreatePixelShader(&custom_shader_ps[0], sizeof(custom_shader_ps), nullptr, _customPixelShader.put()));
// We know the built-in retro shader doesn't require continuous redraw.
_requiresContinuousRedraw = false;
}

if (_customPixelShader)
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/atlas/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ namespace Microsoft::Console::Render::Atlas
struct TargetSettings
{
HWND hwnd = nullptr;
bool enableTransparentBackground = false;
bool useAlpha = false;
bool useSoftwareRendering = false;
};

Expand Down

0 comments on commit a3caf55

Please sign in to comment.