Skip to content

Commit 0289cb0

Browse files
authored
AtlasEngine: Minor bug fixes (#16219)
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 ✅
1 parent 19efcfe commit 0289cb0

File tree

4 files changed

+23
-17
lines changed

4 files changed

+23
-17
lines changed

src/renderer/atlas/AtlasEngine.api.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -490,20 +490,20 @@ void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept
490490

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

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

src/renderer/atlas/AtlasEngine.r.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ void AtlasEngine::_createSwapChain()
329329
.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL,
330330
// If our background is opaque we can enable "independent" flips by setting DXGI_ALPHA_MODE_IGNORE.
331331
// As our swap chain won't have to compose with DWM anymore it reduces the display latency dramatically.
332-
.AlphaMode = _p.s->target->enableTransparentBackground ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
332+
.AlphaMode = _p.s->target->useAlpha ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE,
333333
.Flags = swapChainFlags,
334334
};
335335

@@ -360,6 +360,8 @@ void AtlasEngine::_createSwapChain()
360360
_p.swapChain.targetSize = _p.s->targetSize;
361361
_p.swapChain.waitForPresentation = true;
362362

363+
LOG_IF_FAILED(_p.swapChain.swapChain->SetMaximumFrameLatency(1));
364+
363365
WaitUntilCanRender();
364366

365367
if (_p.swapChainChangedCallback)

src/renderer/atlas/BackendD3D.cpp

+12-8
Original file line numberDiff line numberDiff line change
@@ -403,30 +403,36 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
403403
/* ppCode */ blob.addressof(),
404404
/* ppErrorMsgs */ error.addressof());
405405

406-
// Unless we can determine otherwise, assume this shader requires evaluation every frame
407-
_requiresContinuousRedraw = true;
408-
409406
if (SUCCEEDED(hr))
410407
{
411-
THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.put()));
408+
THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.addressof()));
412409

413410
// Try to determine whether the shader uses the Time variable
414411
wil::com_ptr<ID3D11ShaderReflection> reflector;
415-
if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.put()))))
412+
if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.addressof()))))
416413
{
414+
// Depending on the version of the d3dcompiler_*.dll, the next two functions either return nullptr
415+
// on failure or an instance of CInvalidSRConstantBuffer or CInvalidSRVariable respectively,
416+
// which cause GetDesc() to return E_FAIL. In other words, we have to assume that any failure in the
417+
// next few lines indicates that the cbuffer is entirely unused (--> _requiresContinuousRedraw=false).
417418
if (ID3D11ShaderReflectionConstantBuffer* constantBufferReflector = reflector->GetConstantBufferByIndex(0)) // shader buffer
418419
{
419420
if (ID3D11ShaderReflectionVariable* variableReflector = constantBufferReflector->GetVariableByIndex(0)) // time
420421
{
421422
D3D11_SHADER_VARIABLE_DESC variableDescriptor;
422-
if (SUCCEEDED_LOG(variableReflector->GetDesc(&variableDescriptor)))
423+
if (SUCCEEDED(variableReflector->GetDesc(&variableDescriptor)))
423424
{
424425
// only if time is used
425426
_requiresContinuousRedraw = WI_IsFlagSet(variableDescriptor.uFlags, D3D_SVF_USED);
426427
}
427428
}
428429
}
429430
}
431+
else
432+
{
433+
// Unless we can determine otherwise, assume this shader requires evaluation every frame
434+
_requiresContinuousRedraw = true;
435+
}
430436
}
431437
else
432438
{
@@ -447,8 +453,6 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p)
447453
else if (p.s->misc->useRetroTerminalEffect)
448454
{
449455
THROW_IF_FAILED(p.device->CreatePixelShader(&custom_shader_ps[0], sizeof(custom_shader_ps), nullptr, _customPixelShader.put()));
450-
// We know the built-in retro shader doesn't require continuous redraw.
451-
_requiresContinuousRedraw = false;
452456
}
453457

454458
if (_customPixelShader)

src/renderer/atlas/common.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ namespace Microsoft::Console::Render::Atlas
313313
struct TargetSettings
314314
{
315315
HWND hwnd = nullptr;
316-
bool enableTransparentBackground = false;
316+
bool useAlpha = false;
317317
bool useSoftwareRendering = false;
318318
};
319319

0 commit comments

Comments
 (0)