maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Improve the legacy color conversions (#6358)

Browse source 
This PR provides a faster algorithm for converting 8-bit and 24-bit
colors into the 4-bit legacy values that are required by the Win32
console APIs. It also fixes areas of the code that were incorrectly
using a simple 16-color conversion that didn't handle 8-bit and 24-bit
values.

The faster conversion algorithm should be an improvement for issues #783
and #3950.

One of the main points of this PR was to fix the
`ReadConsoleOutputAttribute` API, which was using a simplified legacy
color conversion (the original `TextAttribute:GetLegacyAttributes`
method), which could only handle values from the 16-color table. RGB
values, and colors from the 256-color table, would be mapped to
completely nonsensical values. This API has now been updated to use the
more correct `Settings::GenerateLegacyAttributes` method.

But there were also a couple of other places in the code that were using
`GetLegacyAttributes` when they really had no reason to be working with
legacy attributes at all. This could result in colors being downgraded
to 4-bit values (often badly, as explained above), when the code was
already perfectly capable of displaying the full 24-bits.

This included the fill colors in the IME composer (in `ConsoleImeInfo`),
and the construction of the highlighting colors in the color
search/selection handler (`Selection::_HandleColorSelection`). I also
got rid of some legacy attribute code in the `Popup` class, which was
originally intended to update colors below the popup when the settings
changed, but actually caused more problems than it solved.

The other major goal of this PR was to improve the performance of the
`GenerateLegacyAttributes` method, since the existing implementation
could be quite slow when dealing with RGB values.

The simple cases are handled much the same as they were before. For an
`IsDefault` color, we get the default index from the
`Settings::_wFillAttribute` field. For an `IsIndex16` color, the index
can just be returned as is.

For an `IsRgb` color, the RGB components are compressed down to 8 bits
(3 red, 3 green, 2 blue), simply by dropping the least significant bits.
This 8-bit value is then used to lookup a representative 16-color value
from a hard-coded table. An `IsIndex256` color is also converted with a
lookup table, just using the existing 8-bit index.

The RGB mapping table was calculated by taking each compressed 8-bit
color, and picking a entry from the _Campbell_ palette that best
approximated that color. This was done by looking at a range of 24-bit
colors that mapped to the 8-bit value, finding the best _Campbell_ match
for each of them (using a [CIEDE2000] color difference calculation), and
then the most common match became the index that the 8-bit value would
map to.

The 256-color table was just a simpler version of this process. For each
entry in the table, we take the default RGB palette value, and find it's
closest match in the _Campbell_ palette.

Because these tables are hard-coded, the results won't adjust to changes
in the palette. However, they should still produce reasonable results
for palettes that follow the standard ANSI color range. And since
they're only a very loose approximation of the colors anyway, the exact
value really isn't that important.

That said, I have tried to make sure that if you take an RGB value for a
particular index in a reasonable color scheme, then the legacy color
mapped from that value should ideally match the same index. This will
never be possible for all color schemes, but I have tweaked a few of the
table entries to improve the results for some of the common schemes.

One other point worth making regarding the hard-coded tables: even if we
wanted to take the active palette into account, that wouldn't actually
be possible over a conpty connection, because we can't easily know what
color scheme the client application is using. At least this way the
results in conhost are guaranteed to be the same as in the Windows
Terminal.

[CIEDE2000]: https://en.wikipedia.org/wiki/Color_difference#CIEDE2000

## Validation Steps Performed

This code still passes the `TextAttributeTests` that check the basic
`GetLegacyAttribute` behaviour and verify the all legacy attributes
roundtrip correctly. However, some of the values in the `RgbColorTests`
had to be updated, since we're now intentionally returning different
values as a result of the changes to the RGB conversion algorithm.

I haven't added additional unit tests, but I have done a lot of manual
testing to see how well the new algorithm works with a range of colors
and a variety of different color schemes. It's not perfect in every
situation, but I think it works well enough for the purpose it serves.

I've also confirmed that the issues reported in #5940 and #6247 are now
fixed by these changes. 

Closes #5940 
Closes #6247
This commit is contained in:
James Holderness 2020-06-08 20:05:06 +01:00 committed by GitHub
parent 4a1f2d3c09
commit ccea66710c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 138 additions and 213 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.github/actions/spell-check/expect/expect.txt vendored
View file

@ -1976,6 +1976,7 @@ robomac
roundtrip
ROWSTOSCROLL
RRF
RRRGGGBB
rtcore
RTEXT
rtf

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

@ -5,16 +5,26 @@
#include "TextAttribute.hpp"
#include "../../inc/conattrs.hpp"
// Routine Description:
// - Returns a WORD with legacy-style attributes for this textattribute.
// Parameters:
// - defaultAttributes: the attribute values to be used for default colors.
// Return value:
// - a WORD with legacy-style attributes for this textattribute.
WORD TextAttribute::GetLegacyAttributes(const WORD defaultAttributes) const noexcept
{
const BYTE fgIndex = _foreground.GetLegacyIndex(defaultAttributes & FG_ATTRS);
const BYTE bgIndex = _background.GetLegacyIndex((defaultAttributes & BG_ATTRS) >> 4);
const WORD metaAttrs = _wAttrLegacy & META_ATTRS;
const bool brighten = _foreground.IsIndex16() && IsBold();
return fgIndex | (bgIndex << 4) | metaAttrs | (brighten ? FOREGROUND_INTENSITY : 0);
}
bool TextAttribute::IsLegacy() const noexcept
{
return _foreground.IsLegacy() && _background.IsLegacy();
}
bool TextAttribute::IsHighColor() const noexcept
{
return _foreground.IsHighColor() || _background.IsHighColor();
}
// Arguments:
// - None
// Return Value:
@ -241,21 +251,6 @@ void TextAttribute::SetDefaultBackground() noexcept
_background = TextColor();
}
// Method Description:
// - Returns true if this attribute indicates its foreground is the "default"
// foreground. Its _rgbForeground will contain the actual value of the
// default foreground. If the default colors are ever changed, this method
// should be used to identify attributes with the default fg value, and
// update them accordingly.
// Arguments:
// - <none>
// Return Value:
// - true iff this attribute indicates it's the "default" foreground color.
bool TextAttribute::ForegroundIsDefault() const noexcept
{
return _foreground.IsDefault();
}
// Method Description:
// - Returns true if this attribute indicates its background is the "default"
// background. Its _rgbBackground will contain the actual value of the

35
src/buffer/out/TextAttribute.hpp
View file

@ -59,38 +59,7 @@ public:
{
}
WORD GetLegacyAttributes() const noexcept
{
const BYTE fg = (_foreground.GetIndex() & FG_ATTRS);
const BYTE bg = (_background.GetIndex() << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
const bool brighten = _foreground.IsIndex16() && IsBold();
return (fg | bg | meta) | (brighten ? FOREGROUND_INTENSITY : 0);
}
// Method Description:
// - Returns a WORD with legacy-style attributes for this textattribute.
// If either the foreground or background of this textattribute is not
// a legacy attribute, then instead use the provided default index as
// the value for that component.
// Arguments:
// - defaultFgIndex: the BYTE to use as the index for the foreground, should
// the foreground not be a legacy style attribute.
// - defaultBgIndex: the BYTE to use as the index for the background, should
// the background not be a legacy style attribute.
// Return Value:
// - a WORD with legacy-style attributes for this textattribute.
WORD GetLegacyAttributes(const BYTE defaultFgIndex,
const BYTE defaultBgIndex) const noexcept
{
const BYTE fgIndex = _foreground.IsLegacy() ? _foreground.GetIndex() : defaultFgIndex;
const BYTE bgIndex = _background.IsLegacy() ? _background.GetIndex() : defaultBgIndex;
const BYTE fg = (fgIndex & FG_ATTRS);
const BYTE bg = (bgIndex << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
const bool brighten = _foreground.IsIndex16() && IsBold();
return (fg | bg | meta) | (brighten ? FOREGROUND_INTENSITY : 0);
}
WORD GetLegacyAttributes(const WORD defaultAttributes = 0x07) const noexcept;
COLORREF CalculateRgbForeground(std::basic_string_view<COLORREF> colorTable,
COLORREF defaultFgColor,
@ -119,7 +88,6 @@ public:
friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept;
bool IsLegacy() const noexcept;
bool IsHighColor() const noexcept;
bool IsBold() const noexcept;
bool IsItalic() const noexcept;
bool IsBlinking() const noexcept;
@ -149,7 +117,6 @@ public:
void SetDefaultForeground() noexcept;
void SetDefaultBackground() noexcept;
bool ForegroundIsDefault() const noexcept;
bool BackgroundIsDefault() const noexcept;
void SetStandardErase() noexcept;

82
src/buffer/out/TextColor.cpp
View file

@ -4,16 +4,57 @@
#include "precomp.h"
#include "TextColor.h"
// clang-format off
// A table mapping 8-bit RGB colors, in the form RRRGGGBB,
// down to one of the 16 colors in the legacy palette.
constexpr std::array<BYTE, 256> CompressedRgbToIndex16 = {
0, 1, 1, 9, 0, 0, 1, 1, 2, 1, 1, 1, 2, 8, 1, 9,
2, 2, 3, 3, 2, 2, 11, 3, 10, 10, 11, 11, 10, 10, 10, 11,
0, 5, 1, 1, 0, 0, 1, 1, 8, 1, 1, 1, 2, 8, 1, 9,
2, 2, 3, 3, 2, 2, 11, 3, 10, 10, 10, 11, 10, 10, 10, 11,
5, 5, 5, 1, 4, 5, 1, 1, 8, 8, 1, 9, 2, 8, 9, 9,
2, 2, 3, 3, 2, 2, 11, 3, 10, 10, 11, 11, 10, 10, 10, 11,
4, 5, 5, 1, 4, 5, 5, 1, 8, 5, 5, 1, 8, 8, 9, 9,
2, 2, 8, 9, 10, 2, 11, 3, 10, 10, 11, 11, 10, 10, 10, 11,
4, 13, 5, 5, 4, 13, 5, 5, 4, 13, 13, 13, 6, 8, 13, 9,
6, 8, 8, 9, 10, 10, 11, 3, 10, 10, 11, 11, 10, 10, 10, 11,
4, 13, 13, 13, 4, 13, 13, 13, 4, 12, 13, 13, 6, 12, 13, 13,
6, 6, 8, 9, 6, 6, 7, 7, 10, 14, 14, 7, 10, 10, 14, 11,
4, 12, 13, 13, 4, 12, 13, 13, 4, 12, 13, 13, 6, 12, 12, 13,
6, 6, 12, 7, 6, 6, 7, 7, 6, 14, 14, 7, 14, 14, 14, 15,
12, 12, 13, 13, 12, 12, 13, 13, 12, 12, 12, 13, 12, 12, 12, 13,
6, 12, 12, 7, 6, 6, 7, 7, 6, 14, 14, 7, 14, 14, 14, 15
};
// A table mapping indexed colors from the 256-color palette,
// down to one of the 16 colors in the legacy palette.
constexpr std::array<BYTE, 256> Index256ToIndex16 = {
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
0, 1, 1, 1, 9, 9, 2, 1, 1, 1, 1, 1, 2, 2, 3, 3,
3, 3, 2, 2, 11, 11, 3, 3, 10, 10, 11, 11, 11, 11, 10, 10,
10, 10, 11, 11, 5, 5, 5, 5, 1, 1, 8, 8, 1, 1, 9, 9,
2, 2, 3, 3, 3, 3, 2, 2, 11, 11, 3, 3, 10, 10, 11, 11,
11, 11, 10, 10, 10, 10, 11, 11, 4, 13, 5, 5, 5, 5, 4, 13,
13, 13, 13, 13, 6, 8, 8, 8, 9, 9, 10, 10, 11, 11, 3, 3,
10, 10, 11, 11, 11, 11, 10, 10, 10, 10, 11, 11, 4, 13, 13, 13,
13, 13, 4, 12, 13, 13, 13, 13, 6, 6, 8, 8, 9, 9, 6, 6,
7, 7, 7, 7, 10, 14, 14, 14, 7, 7, 10, 10, 14, 14, 11, 11,
4, 12, 13, 13, 13, 13, 4, 12, 13, 13, 13, 13, 6, 6, 12, 12,
7, 7, 6, 6, 7, 7, 7, 7, 6, 14, 14, 14, 7, 7, 14, 14,
14, 14, 15, 15, 12, 12, 13, 13, 13, 13, 12, 12, 12, 12, 13, 13,
6, 12, 12, 12, 7, 7, 6, 6, 7, 7, 7, 7, 6, 14, 14, 14,
7, 7, 14, 14, 14, 14, 15, 15, 0, 0, 0, 0, 0, 0, 8, 8,
8, 8, 8, 8, 8, 8, 8, 8, 7, 7, 7, 7, 7, 7, 15, 15
};
// clang-format on
bool TextColor::IsLegacy() const noexcept
{
return IsIndex16() || (IsIndex256() && _index < 16);
}
bool TextColor::IsHighColor() const noexcept
{
return IsRgb() || (IsIndex256() && _index >= 16);
}
bool TextColor::IsIndex16() const noexcept
{
return _meta == ColorType::IsIndex16;
@ -135,6 +176,37 @@ COLORREF TextColor::GetColor(std::basic_string_view<COLORREF> colorTable,
}
}
// Method Description:
// - Return a legacy index value that best approximates this color.
// Arguments:
// - defaultIndex: The index to use for a default color.
// Return Value:
// - an index into the 16-color table
BYTE TextColor::GetLegacyIndex(const BYTE defaultIndex) const noexcept
{
if (IsDefault())
{
return defaultIndex;
}
else if (IsIndex16())
{
return GetIndex();
}
else if (IsIndex256())
{
return Index256ToIndex16.at(GetIndex());
}
else
{
// We compress the RGB down to an 8-bit value and use that to
// lookup a representative 16-color index from a hard-coded table.
const BYTE compressedRgb = (_red & 0b11100000) +
((_green >> 3) & 0b00011100) +
((_blue >> 6) & 0b00000011);
return CompressedRgbToIndex16.at(compressedRgb);
}
}
// Method Description:
// - Return a COLORREF containing our stored value. Will return garbage if this
//attribute is not a RGB attribute.

3
src/buffer/out/TextColor.h
View file

@ -78,7 +78,6 @@ public:
friend constexpr bool operator!=(const TextColor& a, const TextColor& b) noexcept;
bool IsLegacy() const noexcept;
bool IsHighColor() const noexcept;
bool IsIndex16() const noexcept;
bool IsIndex256() const noexcept;
bool IsDefault() const noexcept;
@ -92,6 +91,8 @@ public:
const COLORREF defaultColor,
const bool brighten) const noexcept;
BYTE GetLegacyIndex(const BYTE defaultIndex) const noexcept;
constexpr BYTE GetIndex() const noexcept
{
return _index;

17
src/host/cmdline.cpp
View file

@ -26,23 +26,6 @@
#pragma hdrstop
using Microsoft::Console::Interactivity::ServiceLocator;
// Routine Description:
// - This routine is called when the user changes the screen/popup colors.
// - It goes through the popup structures and changes the saved contents to reflect the new screen/popup colors.
void CommandLine::UpdatePopups(const TextAttribute& NewAttributes,
const TextAttribute& NewPopupAttributes,
const TextAttribute& OldAttributes,
const TextAttribute& OldPopupAttributes)
{
for (auto& popup : _popups)
{
try
{
popup->UpdateStoredColors(NewAttributes, NewPopupAttributes, OldAttributes, OldPopupAttributes);
}
CATCH_LOG();
}
}
// Routine Description:
// - This routine validates a string buffer and returns the pointers of where the strings start within the buffer.

5
src/host/cmdline.h
View file

@ -84,11 +84,6 @@ public:
bool HasPopup() const noexcept;
Popup& GetPopup();
void UpdatePopups(const TextAttribute& NewAttributes,
const TextAttribute& NewPopupAttributes,
const TextAttribute& OldAttributes,
const TextAttribute& OldPopupAttributes);
void EndCurrentPopup();
void EndAllPopups();

8
src/host/conareainfo.cpp
View file

@ -24,8 +24,8 @@ ConversionAreaBufferInfo::ConversionAreaBufferInfo(const COORD coordBufferSize)
ConversionAreaInfo::ConversionAreaInfo(const COORD bufferSize,
const COORD windowSize,
const CHAR_INFO fill,
const CHAR_INFO popupFill,
const TextAttribute& fill,
const TextAttribute& popupFill,
const FontInfo fontInfo) :
_caInfo{ bufferSize },
_isHidden{ true },
@ -37,8 +37,8 @@ ConversionAreaInfo::ConversionAreaInfo(const COORD bufferSize,
THROW_IF_NTSTATUS_FAILED(SCREEN_INFORMATION::CreateInstance(windowSize,
fontInfo,
bufferSize,
TextAttribute{ fill.Attributes },
TextAttribute{ popupFill.Attributes },
fill,
popupFill,
0,
&pNewScreen));

4
src/host/conareainfo.h
View file

@ -42,8 +42,8 @@ class ConversionAreaInfo final
public:
ConversionAreaInfo(const COORD bufferSize,
const COORD windowSize,
const CHAR_INFO fill,
const CHAR_INFO popupFill,
const TextAttribute& fill,
const TextAttribute& popupFill,
const FontInfo fontInfo);
~ConversionAreaInfo() = default;
ConversionAreaInfo(const ConversionAreaInfo&) = delete;

6
src/host/conimeinfo.cpp
View file

@ -146,11 +146,9 @@ void ConsoleImeInfo::ClearAllAreas()
const COORD windowSize = gci.GetActiveOutputBuffer().GetViewport().Dimensions();
CHAR_INFO fill;
fill.Attributes = gci.GetActiveOutputBuffer().GetAttributes().GetLegacyAttributes();
const TextAttribute fill = gci.GetActiveOutputBuffer().GetAttributes();
CHAR_INFO popupFill;
popupFill.Attributes = gci.GetActiveOutputBuffer().GetPopupAttributes()->GetLegacyAttributes();
const TextAttribute popupFill = gci.GetActiveOutputBuffer().GetPopupAttributes();
const FontInfo& fontInfo = gci.GetActiveOutputBuffer().GetCurrentFont();

20
src/host/ft_host/API_RgbColorTests.cpp
View file

@ -320,21 +320,21 @@ BOOL Validate256GridToLegacy(COORD cursorPosInitial)
// Verify some other locations in the table, that will be RGB->Legacy conversions.
VERIFY_ARE_EQUAL(GetGridAttrs(1, 1, rOutputBuffer, actualWriteSize), MakeAttribute(0x1, 0x1));
VERIFY_ARE_EQUAL(GetGridAttrs(2, 1, rOutputBuffer, actualWriteSize), MakeAttribute(0xB, 0xB));
VERIFY_ARE_EQUAL(GetGridAttrs(3, 3, rOutputBuffer, actualWriteSize), MakeAttribute(0xB, 0xB));
VERIFY_ARE_EQUAL(GetGridAttrs(2, 2, rOutputBuffer, actualWriteSize), MakeAttribute(0x2, 0x2));
VERIFY_ARE_EQUAL(GetGridAttrs(2, 3, rOutputBuffer, actualWriteSize), MakeAttribute(0x3, 0x3));
VERIFY_ARE_EQUAL(GetGridAttrs(3, 4, rOutputBuffer, actualWriteSize), MakeAttribute(0x4, 0x4));
VERIFY_ARE_EQUAL(GetGridAttrs(2, 1, rOutputBuffer, actualWriteSize), MakeAttribute(0x3, 0x3));
VERIFY_ARE_EQUAL(GetGridAttrs(5, 8, rOutputBuffer, actualWriteSize), MakeAttribute(0x4, 0x4));
VERIFY_ARE_EQUAL(GetGridAttrs(3, 5, rOutputBuffer, actualWriteSize), MakeAttribute(0x5, 0x5));
VERIFY_ARE_EQUAL(GetGridAttrs(4, 5, rOutputBuffer, actualWriteSize), MakeAttribute(0x9, 0x9));
VERIFY_ARE_EQUAL(GetGridAttrs(4, 6, rOutputBuffer, actualWriteSize), MakeAttribute(0x6, 0x6));
VERIFY_ARE_EQUAL(GetGridAttrs(4, 7, rOutputBuffer, actualWriteSize), MakeAttribute(0x7, 0x7));
VERIFY_ARE_EQUAL(GetGridAttrs(6, 8, rOutputBuffer, actualWriteSize), MakeAttribute(0x9, 0x9));
VERIFY_ARE_EQUAL(GetGridAttrs(8, 8, rOutputBuffer, actualWriteSize), MakeAttribute(0x6, 0x6));
VERIFY_ARE_EQUAL(GetGridAttrs(9, 8, rOutputBuffer, actualWriteSize), MakeAttribute(0x7, 0x7));
VERIFY_ARE_EQUAL(GetGridAttrs(3, 11, rOutputBuffer, actualWriteSize), MakeAttribute(0x8, 0x8));
VERIFY_ARE_EQUAL(GetGridAttrs(3, 12, rOutputBuffer, actualWriteSize), MakeAttribute(0x1, 0x1));
VERIFY_ARE_EQUAL(GetGridAttrs(4, 12, rOutputBuffer, actualWriteSize), MakeAttribute(0xA, 0xA));
VERIFY_ARE_EQUAL(GetGridAttrs(5, 12, rOutputBuffer, actualWriteSize), MakeAttribute(0xD, 0xD));
VERIFY_ARE_EQUAL(GetGridAttrs(10, 12, rOutputBuffer, actualWriteSize), MakeAttribute(0xE, 0xE));
VERIFY_ARE_EQUAL(GetGridAttrs(10, 13, rOutputBuffer, actualWriteSize), MakeAttribute(0xC, 0xC));
VERIFY_ARE_EQUAL(GetGridAttrs(11, 13, rOutputBuffer, actualWriteSize), MakeAttribute(0xF, 0xF));
VERIFY_ARE_EQUAL(GetGridAttrs(8, 1, rOutputBuffer, actualWriteSize), MakeAttribute(0xD, 0xD));
VERIFY_ARE_EQUAL(GetGridAttrs(12, 0, rOutputBuffer, actualWriteSize), MakeAttribute(0xE, 0xE));
VERIFY_ARE_EQUAL(GetGridAttrs(12, 4, rOutputBuffer, actualWriteSize), MakeAttribute(0xC, 0xC));
VERIFY_ARE_EQUAL(GetGridAttrs(12, 3, rOutputBuffer, actualWriteSize), MakeAttribute(0xF, 0xF));
// Greyscale ramp
VERIFY_ARE_EQUAL(GetGridAttrs(14, 8, rOutputBuffer, actualWriteSize), MakeAttribute(0x0, 0x0));

9
src/host/output.cpp
View file

@ -148,16 +148,19 @@ std::vector<WORD> ReadOutputAttributes(const SCREEN_INFORMATION& screenInfo,
// While we haven't read enough cells yet and the iterator is still valid (hasn't reached end of buffer)
while (amountRead < amountToRead && it)
{
const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const auto legacyAttributes = gci.GenerateLegacyAttributes(it->TextAttr());
// If the first thing we read is trailing, pad with a space.
// OR If the last thing we read is leading, pad with a space.
if ((amountRead == 0 && it->DbcsAttr().IsTrailing()) ||
(amountRead == (amountToRead - 1) && it->DbcsAttr().IsLeading()))
{
retVal.push_back(it->TextAttr().GetLegacyAttributes());
retVal.push_back(legacyAttributes);
}
else
{
retVal.push_back(it->TextAttr().GetLegacyAttributes() | it->DbcsAttr().GeneratePublicApiAttributeFormat());
retVal.push_back(legacyAttributes | it->DbcsAttr().GeneratePublicApiAttributeFormat());
}
amountRead++;
@ -384,7 +387,7 @@ void ScrollRegion(SCREEN_INFORMATION& screenInfo,
// However, if the character is null and we were given a null attribute (represented as legacy 0),
// then we'll just fill with spaces and whatever the buffer's default colors are.
if (fillCharGiven == UNICODE_NULL && fillAttrsGiven.IsLegacy() && fillAttrsGiven.GetLegacyAttributes() == 0)
if (fillCharGiven == UNICODE_NULL && fillAttrsGiven == TextAttribute{ 0 })
{
fillData = OutputCellIterator(UNICODE_SPACE, screenInfo.GetAttributes());
}

51
src/host/popup.cpp
View file

@ -32,7 +32,7 @@ Popup::Popup(SCREEN_INFORMATION& screenInfo, const COORD proposedSize) :
_screenInfo(screenInfo),
_userInputFunction(&Popup::_getUserInputInternal)
{
_attributes = *screenInfo.GetPopupAttributes();
_attributes = screenInfo.GetPopupAttributes();
const COORD size = _CalculateSize(screenInfo, proposedSize);
const COORD origin = _CalculateOrigin(screenInfo, size);
@ -175,55 +175,6 @@ void Popup::_DrawPrompt(const UINT id)
used));
}
// Routine Description:
// - Updates the colors of the backed up information inside this popup.
// - This is useful if user preferences change while a popup is displayed.
// Arguments:
// - newAttr - The new default color for text in the buffer
// - newPopupAttr - The new color for text in popups
// - oldAttr - The previous default color for text in the buffer
// - oldPopupAttr - The previous color for text in popups
void Popup::UpdateStoredColors(const TextAttribute& newAttr,
const TextAttribute& newPopupAttr,
const TextAttribute& oldAttr,
const TextAttribute& oldPopupAttr)
{
// We also want to find and replace the inversion of the popup colors in case there are highlights
const WORD wOldPopupLegacy = oldPopupAttr.GetLegacyAttributes();
const WORD wNewPopupLegacy = newPopupAttr.GetLegacyAttributes();
const WORD wOldPopupAttrInv = (WORD)(((wOldPopupLegacy << 4) & 0xf0) | ((wOldPopupLegacy >> 4) & 0x0f));
const WORD wNewPopupAttrInv = (WORD)(((wNewPopupLegacy << 4) & 0xf0) | ((wNewPopupLegacy >> 4) & 0x0f));
const TextAttribute oldPopupInv{ wOldPopupAttrInv };
const TextAttribute newPopupInv{ wNewPopupAttrInv };
// Walk through every row in the rectangle
for (size_t i = 0; i < _oldContents.Height(); i++)
{
auto row = _oldContents.GetRow(i);
// Walk through every cell
for (auto& cell : row)
{
auto& attr = cell.TextAttr();
if (attr == oldAttr)
{
attr = newAttr;
}
else if (attr == oldPopupAttr)
{
attr = newPopupAttr;
}
else if (attr == oldPopupInv)
{
attr = newPopupInv;
}
}
}
}
// Routine Description:
// - Cleans up a popup by restoring the stored buffer information to the region of
// the screen that the popup was covering and frees resources.

5
src/host/popup.h
View file

@ -39,11 +39,6 @@ public:
void Draw();
void UpdateStoredColors(const TextAttribute& newAttr,
const TextAttribute& newPopupAttr,
const TextAttribute& oldAttr,
const TextAttribute& oldPopupAttr);
void End();
SHORT Width() const noexcept;

14
src/host/screenInfo.cpp
View file

@ -1860,7 +1860,7 @@ const SCREEN_INFORMATION& SCREEN_INFORMATION::GetMainBuffer() const
existingFont,
WindowSize,
initAttributes,
*GetPopupAttributes(),
GetPopupAttributes(),
Cursor::CURSOR_SMALL_SIZE,
ppsiNewScreenBuffer);
if (NT_SUCCESS(Status))
@ -2017,9 +2017,9 @@ TextAttribute SCREEN_INFORMATION::GetAttributes() const
// <none>
// Return value:
// - This screen buffer's popup attributes
const TextAttribute* const SCREEN_INFORMATION::GetPopupAttributes() const
TextAttribute SCREEN_INFORMATION::GetPopupAttributes() const
{
return &_PopupAttributes;
return _PopupAttributes;
}
// Routine Description:
@ -2071,7 +2071,7 @@ void SCREEN_INFORMATION::SetDefaultAttributes(const TextAttribute& attributes,
CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const TextAttribute oldPrimaryAttributes = GetAttributes();
const TextAttribute oldPopupAttributes = *GetPopupAttributes();
const TextAttribute oldPopupAttributes = GetPopupAttributes();
// Quick return if we don't need to do anything.
if (oldPrimaryAttributes == attributes && oldPopupAttributes == popupAttributes)
@ -2082,12 +2082,6 @@ void SCREEN_INFORMATION::SetDefaultAttributes(const TextAttribute& attributes,
SetAttributes(attributes);
SetPopupAttributes(popupAttributes);
auto& commandLine = CommandLine::Instance();
if (commandLine.HasPopup())
{
commandLine.UpdatePopups(attributes, popupAttributes, oldPrimaryAttributes, oldPopupAttributes);
}
// Force repaint of entire viewport, unless we're in conpty mode. In that
// case, we don't really need to force a redraw of the entire screen just
// because the text attributes changed.

2
src/host/screenInfo.hpp
View file

@ -214,7 +214,7 @@ public:
const SCREEN_INFORMATION& GetActiveBuffer() const;
TextAttribute GetAttributes() const;
const TextAttribute* const GetPopupAttributes() const;
TextAttribute GetPopupAttributes() const;
void SetAttributes(const TextAttribute& attributes);
void SetPopupAttributes(const TextAttribute& popupAttributes);

13
src/host/selectionInput.cpp
View file

@ -660,17 +660,20 @@ bool Selection::_HandleColorSelection(const INPUT_KEY_INFO* const pInputKeyInfo)
// ALT+n => fg, CTRL+n => bg
if (fAltPressed || fCtrlPressed)
{
ULONG ulAttr = wVirtualKeyCode - '0' + 6;
TextAttribute selectionAttr;
const BYTE colorIndex = gsl::narrow_cast<BYTE>(wVirtualKeyCode - '0' + 6);
if (fCtrlPressed)
{
// Setting background color. Set fg color to black.
ulAttr <<= 4;
selectionAttr.SetIndexedBackground256(colorIndex);
selectionAttr.SetIndexedForeground256(0);
}
else
{
// Set foreground color. Maintain the current console bg color.
ulAttr |= gci.GetActiveOutputBuffer().GetAttributes().GetLegacyAttributes() & 0xf0;
selectionAttr = gci.GetActiveOutputBuffer().GetAttributes();
selectionAttr.SetIndexedForeground256(colorIndex);
}
// If shift was pressed as well, then this is actually a
@ -706,7 +709,7 @@ bool Selection::_HandleColorSelection(const INPUT_KEY_INFO* const pInputKeyInfo)
Search search(gci.renderData, str, Search::Direction::Forward, Search::Sensitivity::CaseInsensitive);
while (search.FindNext())
{
search.Color(TextAttribute{ static_cast<WORD>(ulAttr) });
search.Color(selectionAttr);
}
}
}
@ -714,7 +717,7 @@ bool Selection::_HandleColorSelection(const INPUT_KEY_INFO* const pInputKeyInfo)
}
else
{
ColorSelection(_srSelectionRect, TextAttribute{ static_cast<WORD>(ulAttr) });
ColorSelection(_srSelectionRect, selectionAttr);
ClearSelection();
}

39
src/host/settings.cpp
View file

@ -760,47 +760,14 @@ COLORREF Settings::GetColorTableEntry(const size_t index) const
// Routine Description:
// - Generates a legacy attribute from the given TextAttributes.
// This needs to be a method on the Settings because the generated index
// is dependent upon the particular values of the color table at the time of reading.
// is dependent upon the default fill attributes.
// Parameters:
// - attributes - The TextAttributes to generate a legacy attribute for.
// Return value:
// - A WORD representing the entry in the color table that most closely represents the given fullcolor attributes.
// - A WORD representing the legacy attributes that most closely represent the given fullcolor attributes.
WORD Settings::GenerateLegacyAttributes(const TextAttribute attributes) const
{
const WORD wLegacyOriginal = attributes.GetLegacyAttributes();
if (attributes.IsLegacy())
{
return wLegacyOriginal;
}
// We need to construct the legacy attributes manually
// First start with whatever our default legacy attributes are
BYTE fgIndex = static_cast<BYTE>((_wFillAttribute & FG_ATTRS));
BYTE bgIndex = static_cast<BYTE>((_wFillAttribute & BG_ATTRS) >> 4);
// If the attributes have any RGB components, we need to match that RGB
// color to a color table value.
if (attributes.IsHighColor())
{
// If the attribute doesn't have a "default" colored *ground, look up
// the nearest color table value for its *ground.
if (!attributes.ForegroundIsDefault())
{
const COLORREF rgbForeground = LookupForegroundColor(attributes);
fgIndex = static_cast<BYTE>(::FindNearestTableIndex(rgbForeground, Get16ColorTable()));
}
if (!attributes.BackgroundIsDefault())
{
const COLORREF rgbBackground = LookupBackgroundColor(attributes);
bgIndex = static_cast<BYTE>(::FindNearestTableIndex(rgbBackground, Get16ColorTable()));
}
}
// TextAttribute::GetLegacyAttributes(BYTE, BYTE) will use the legacy value
// it has if the component is a legacy index, otherwise it will use the
// provided byte for each index. In this way, we can provide a value to
// use should it not already have one.
const WORD wCompleteAttr = attributes.GetLegacyAttributes(fgIndex, bgIndex);
return wCompleteAttr;
return attributes.GetLegacyAttributes(_wFillAttribute);
}
COLORREF Settings::GetCursorColor() const noexcept

2
src/host/ut_host/ObjectTests.cpp
View file

@ -63,7 +63,7 @@ class ObjectTests
existingOutput.GetCurrentFont(),
existingOutput.GetBufferSize().Dimensions(),
existingOutput.GetAttributes(),
*existingOutput.GetPopupAttributes(),
existingOutput.GetPopupAttributes(),
existingOutput.GetTextBuffer().GetCursor().GetSize(),
&newOutput));