Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use grayscale AA always on non-opaque backgrounds #5277

Merged
9 commits merged into from
Apr 24, 2020
24 changes: 23 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,23 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
RootGrid().Background(acrylic);
}

// GH#5098: Inform the engine of the new opacity of the default text background.
if (_renderEngine)
{
_renderEngine->SetDefaultTextBackgroundOpacity(::base::saturated_cast<float>(_settings.TintOpacity()));
}
}
else
{
Media::SolidColorBrush solidColor{};
RootGrid().Background(solidColor);

// GH#5098: Inform the engine of the new opacity of the default text background.
if (_renderEngine)
{
_renderEngine->SetDefaultTextBackgroundOpacity(1.0f);
}
}

if (!_settings.BackgroundImage().empty())
Expand Down Expand Up @@ -1278,14 +1290,24 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
try
{
auto acrylicBrush = RootGrid().Background().as<Media::AcrylicBrush>();
acrylicBrush.TintOpacity(acrylicBrush.TintOpacity() + effectiveDelta);
_settings.TintOpacity(acrylicBrush.TintOpacity() + effectiveDelta);
acrylicBrush.TintOpacity(_settings.TintOpacity());

if (acrylicBrush.TintOpacity() == 1.0)
{
_settings.UseAcrylic(false);
_InitializeBackgroundBrush();
uint32_t bg = _settings.DefaultBackground();
_BackgroundColorChanged(bg);
}
else
{
// GH#5098: Inform the engine of the new opacity of the default text background.
if (_renderEngine)
{
_renderEngine->SetDefaultTextBackgroundOpacity(::base::saturated_cast<float>(_settings.TintOpacity()));
}
}
}
CATCH_LOG();
}
Expand Down
37 changes: 37 additions & 0 deletions src/renderer/dx/CustomTextRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,43 @@ using namespace Microsoft::Console::Render;

d2dContext->FillRectangle(rect, drawingContext->backgroundBrush);

// GH#5098: If we're rendering with cleartype text, we need to always render
// onto an opaque background. If our background _isn't_ opaque, then we need
// to use grayscale AA for this run of text.
//
// We can force grayscale AA for just this run of text by pushing a new
// layer onto the d2d context. We'll only need to do this for cleartype
// text, when our eventual background isn't actually opaque. See
// DxEngine::PaintBufferLine and DxEngine::UpdateDrawingBrushes for more
// details.
//
// DANGER: Layers slow us down. Only do this in the specific case where
// someone has chosen the slower ClearType antialiasing (versus the faster
// grayscale antialiasing).

// First, create the scope_exit to pop the layer. If we don't need the
// layer, we'll just gracefully release it.
auto popLayer = wil::scope_exit([&d2dContext]() noexcept {
d2dContext->PopLayer();
});

if (drawingContext->forceGrayscaleAA)
{
// Mysteriously, D2D1_LAYER_OPTIONS_INITIALIZE_FOR_CLEARTYPE actually
// gets us the behavior we want, which is grayscale.
d2dContext->PushLayer(D2D1::LayerParameters(rect,
nullptr,
D2D1_ANTIALIAS_MODE_ALIASED,
D2D1::IdentityMatrix(),
1.0,
nullptr,
D2D1_LAYER_OPTIONS_INITIALIZE_FOR_CLEARTYPE),
nullptr);
}
else
{
popLayer.release();
}
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
// Now go onto drawing the text.

// First check if we want a color font and try to extract color emoji first.
Expand Down
3 changes: 3 additions & 0 deletions src/renderer/dx/CustomTextRenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace Microsoft::Console::Render
DrawingContext(ID2D1RenderTarget* renderTarget,
ID2D1Brush* foregroundBrush,
ID2D1Brush* backgroundBrush,
bool forceGrayscaleAA,
IDWriteFactory* dwriteFactory,
const DWRITE_LINE_SPACING spacing,
const D2D_SIZE_F cellSize,
Expand All @@ -20,6 +21,7 @@ namespace Microsoft::Console::Render
this->renderTarget = renderTarget;
this->foregroundBrush = foregroundBrush;
this->backgroundBrush = backgroundBrush;
this->forceGrayscaleAA = forceGrayscaleAA;
this->dwriteFactory = dwriteFactory;
this->spacing = spacing;
this->cellSize = cellSize;
Expand All @@ -29,6 +31,7 @@ namespace Microsoft::Console::Render
ID2D1RenderTarget* renderTarget;
ID2D1Brush* foregroundBrush;
ID2D1Brush* backgroundBrush;
bool forceGrayscaleAA;
IDWriteFactory* dwriteFactory;
DWRITE_LINE_SPACING spacing;
D2D_SIZE_F cellSize;
Expand Down
62 changes: 60 additions & 2 deletions src/renderer/dx/DxRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ DxEngine::DxEngine() :
_haveDeviceResources{ false },
_retroTerminalEffects{ false },
_antialiasingMode{ D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE },
_defaultTextBackgroundOpacity{ 1.0f },
_hwndTarget{ static_cast<HWND>(INVALID_HANDLE_VALUE) },
_sizeTarget{},
_dpi{ USER_DEFAULT_SCREEN_DPI },
Expand Down Expand Up @@ -1237,10 +1238,36 @@ try
DWRITE_LINE_SPACING spacing;
RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline));

// GH#5098: If we're rendering with cleartype text, we need to always
// render onto an opaque background. If our background's opacity is
// 1.0f, that's great, we can use that. Otherwise, we need to force the
// text renderer to render this text in grayscale. In
// UpdateDrawingBrushes, we'll set the backgroundColor's a channel to
// 1.0 if we're in cleartype mode and the background's opacity is 1.0.
// Otherwise, at this point, the _backgroundColor's alpha is <1.0.
//
// Currently, only text with the default background color uses an alpha
// of 0, every other background uses 1.0
//
// DANGER: Layers slow us down. Only do this in the specific case where
// someone has chosen the slower ClearType antialiasing (versus the faster
// grayscale antialiasing)
const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE;
const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f;
// Another way of naming "bgIsDefault" is "bgHasTransparency"
const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) &&
(_backgroundColor.r == _defaultBackgroundColor.r) &&
(_backgroundColor.g == _defaultBackgroundColor.g) &&
(_backgroundColor.b == _defaultBackgroundColor.b);
const bool forceGrayscaleAA = usingCleartype &&
usingTransparency &&
bgIsDefault;

// Assemble the drawing context information
DrawingContext context(_d2dRenderTarget.Get(),
_d2dBrushForeground.Get(),
_d2dBrushBackground.Get(),
forceGrayscaleAA,
_dwriteFactory.Get(),
spacing,
_glyphCell,
Expand Down Expand Up @@ -1527,8 +1554,19 @@ CATCH_RETURN()
const ExtendedAttributes /*extendedAttrs*/,
bool const isSettingDefaultBrushes) noexcept
{
_foregroundColor = _ColorFFromColorRef(colorForeground);
_backgroundColor = _ColorFFromColorRef(colorBackground);
// GH#5098: If we're rendering with cleartype text, we need to always render
// onto an opaque background. If our background's opacity is 1.0f, that's
// great, we can actually use cleartype in that case. In that scenario
// (cleartype && opacity == 1.0), we'll force the opacity bits of the
// COLORREF to 0xff so we draw as cleartype. In any other case, leave the
// opacity bits unchanged. PaintBufferLine will later do some logic to
// determine if we should paint the text as grayscale or not.
const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE;
const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f;
const bool forceOpaqueBG = usingCleartype && !usingTransparency;

_foregroundColor = _ColorFFromColorRef(OPACITY_OPAQUE | colorForeground);
_backgroundColor = _ColorFFromColorRef((forceOpaqueBG ? OPACITY_OPAQUE : 0) | colorBackground);

_d2dBrushForeground->SetColor(_foregroundColor);
_d2dBrushBackground->SetColor(_backgroundColor);
Expand Down Expand Up @@ -2130,3 +2168,23 @@ void DxEngine::SetAntialiasingMode(const D2D1_TEXT_ANTIALIAS_MODE antialiasingMo
{
_antialiasingMode = antialiasingMode;
}

// Method Description:
// - Update our tracker of the opacity of our background. We can only
// effectively render cleartype text onto fully-opaque backgrounds. If we're
// rendering onto a transparent surface (like acrylic), then cleartype won't
// work correctly, and will actually just additively blend with the
// background. This is here to support GH#5098.
// Arguments:
// - opacity: the new opacity of our background, on [0.0f, 1.0f]
// Return Value:
// - <none>
void DxEngine::SetDefaultTextBackgroundOpacity(const float opacity) noexcept
{
_defaultTextBackgroundOpacity = opacity;

// Make sure we redraw all the cells, to update whether they're actually
// drawn with cleartype or not.
// We don't terribly care if this fails.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why don't we care?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not worth crashing over if it does fail. It'll just make a graphical glitch... is my most likely candidate for a reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's definitely the reason

LOG_IF_FAILED(InvalidateAll());
}
3 changes: 3 additions & 0 deletions src/renderer/dx/DxRenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ namespace Microsoft::Console::Render

void SetSelectionBackground(const COLORREF color) noexcept;
void SetAntialiasingMode(const D2D1_TEXT_ANTIALIAS_MODE antialiasingMode) noexcept;
void SetDefaultTextBackgroundOpacity(const float opacity) noexcept;

protected:
[[nodiscard]] HRESULT _DoUpdateTitle(_In_ const std::wstring& newTitle) noexcept override;
Expand Down Expand Up @@ -189,6 +190,8 @@ namespace Microsoft::Console::Render

D2D1_TEXT_ANTIALIAS_MODE _antialiasingMode;

float _defaultTextBackgroundOpacity;

// DirectX constant buffers need to be a multiple of 16; align to pad the size.
__declspec(align(16)) struct
{
Expand Down