starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-05-06 20:34:29 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Move the common render settings into a shared class (#12127)

Browse Source 
## Summary of the Pull Request

This PR moves the color table and related render settings, which are common to both conhost and Windows Terminal, into a shared class that can be accessed directly from the renderer. This avoids the overhead of having to look up these properties via the `IRenderData` interface, which relies on inefficient virtual function calls.

This also introduces the concept of color aliases, which determine the position in the color table that colors like the default foreground and background are stored. This allows the option of mapping them to one of the standard 16 colors, or to have their own separate table entries.

## References

This is a continuation of the color table refactoring started in #11602 and #11784. The color alias functionality is a prerequisite for supporting a default bold color as proposed in #11939. The color aliases could also be a way for us to replace the PowerShell color quirk for #6807.

## PR Checklist
* [x] Closes #12002
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

In addition to the color table, this new `RenderSettings` class manages the blinking state, the code for adjusting indistinguishable colors, and various boolean properties used in the color calculations. These boolean properties are now stored in a `til::enumset` so they can all be managed through a single `SetRenderMode` API, and easily extended with additional modes that we're likely to need in the future.

In Windows Terminal we have an instance of `RenderSettings` stored in the `Terminal` class, and in conhost it's stored in the `Settings` class. In both cases, a reference to this class is passed to the `Renderer` constructor, so it now has direct access to that data. The renderer can then pass that reference to the render engines where it's needed in the `UpdateDrawingBrushes` method.

This means the renderer no longer needs the `IRenderData` interface to access the `GetAttributeColors`, `GetCursorColor`, or `IsScreenReversed` methods, so those have now been removed. We still need access to `GetAttributeColors` in certain accessibility code, though, so I've kept that method in the `IUIAData` interface, but the implementation just forwards to the `RenderSettings` class.

The implementation of the `RenderSettings::GetAttributeColors` method is loosely based on the original `Terminal` code, only the `CalculateRgbColors` call has now been incorporated directly into the code. This let us deduplicate some bits that were previously repeated in the section for adjusting indistinguishable colors. The last steps, where we calculate the alpha components, have now been split in to a separate `GetAttributeColorsWithAlpha` method, since that's typically not needed.

## Validation Steps Performed

There were quite a lot changes needed in the unit tests, but they're mostly straightforward replacements of one method call with another.

In the `TextAttributeTests`, where we were previously testing the `CalculateRgbColors` method, we're now running those tests though `RenderSettings::GetAttributeColors`, which incorporates the same functionality. The only complication is when testing the `IntenseIsBright` option, that needs to be set with an additional `SetRenderMode` call where previously it was just a parameter on `CalculateRgbColors`.

In the `ScreenBufferTests` and `TextBufferTests`, calls to `LookupAttributeColors` have again been replaced by the `RenderSettings::GetAttributeColors` method, which serves the same purpose, and calls to `IsScreenReversed` have been replaced with an appropriate `GetRenderMode` call. In the `VtRendererTests`, all the calls to `UpdateDrawingBrushes` now just need to be passed a reference to a `RenderSettings` instance.
This commit is contained in:
James Holderness
2022-01-13 21:10:09 +00:00
committed by GitHub
parent b3fab518f8
commit 62c95b5017
79 changed files with 898 additions and 920 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
src/buffer/out/TextAttribute.cpp
View File

@@ -124,41 +124,6 @@ bool TextAttribute::IsLegacy() const noexcept
return _foreground.IsLegacy() && _background.IsLegacy();
}
// Routine Description:
// - Calculates rgb colors based off of current color table and active modification attributes.
// Arguments:
// - colorTable: the current color table rgb values.
// - defaultFgIndex: the color table index of the default foreground color.
// - defaultBgIndex: the color table index of the default background color.
// - reverseScreenMode: true if the screen mode is reversed.
// - blinkingIsFaint: true if blinking should be interpreted as faint. (defaults to false)
// - boldIsBright: true if "bold" should be interpreted as bright. (defaults to true)
// Return Value:
// - the foreground and background colors that should be displayed.
std::pair<COLORREF, COLORREF> TextAttribute::CalculateRgbColors(const std::array<COLORREF, TextColor::TABLE_SIZE>& colorTable,
const size_t defaultFgIndex,
const size_t defaultBgIndex,
const bool reverseScreenMode,
const bool blinkingIsFaint,
const bool boldIsBright) const noexcept
{
auto fg = _foreground.GetColor(colorTable, defaultFgIndex, boldIsBright && IsBold());
auto bg = _background.GetColor(colorTable, defaultBgIndex);
if (IsFaint() || (IsBlinking() && blinkingIsFaint))
{
fg = (fg >> 1) & 0x7F7F7F; // Divide foreground color components by two.
}
if (IsReverseVideo() ^ reverseScreenMode)
{
std::swap(fg, bg);
}
if (IsInvisible())
{
fg = bg;
}
return { fg, bg };
}
// Method description:
// - Tells us whether the text is a hyperlink or not
// Return value: