Repository: microsoft/terminal
File: src/renderer/base/RenderSettings.cpp (C++)

Code before:

// Routine Description:
// - Calculates the RGB colors of a given text attribute, using the current
//   color table configuration and active render settings.
// Arguments:
// - attr - The TextAttribute to retrieve the colors for.
// Return Value:
// - The color values of the attribute's foreground and background.
std::pair<COLORREF, COLORREF> RenderSettings::GetAttributeColors(const TextAttribute& attr) const noexcept
{
    _blinkIsInUse = _blinkIsInUse || attr.IsBlinking();

    const auto fgTextColor = attr.GetForeground();
    const auto bgTextColor = attr.GetBackground();

    const auto defaultFgIndex = GetColorAliasIndex(ColorAlias::DefaultForeground);
    const auto defaultBgIndex = GetColorAliasIndex(ColorAlias::DefaultBackground);

    const auto brightenFg = attr.IsBold() && GetRenderMode(Mode::IntenseIsBright);
    const auto dimFg = attr.IsFaint() || (_blinkShouldBeFaint && attr.IsBlinking());
    const auto swapFgAndBg = attr.IsReverseVideo() ^ GetRenderMode(Mode::ScreenReversed);

    // We want to nudge the foreground color to make it more perceivable only for the
    // default color pairs within the color table
    if (GetRenderMode(Mode::DistinguishableColors) &&
        Feature_AdjustIndistinguishableText::IsEnabled() &&
        !dimFg &&
        (fgTextColor.IsDefault() || fgTextColor.IsLegacy()) &&
        (bgTextColor.IsDefault() || bgTextColor.IsLegacy()))
    {
        const auto bgIndex = bgTextColor.IsDefault() ? AdjustedBgIndex : bgTextColor.GetIndex();
        auto fgIndex = fgTextColor.IsDefault() ? AdjustedFgIndex : fgTextColor.GetIndex();

        if (fgTextColor.IsIndex16() && (fgIndex < 8) && brightenFg)
        {
            // There is a special case for bold here - we need to get the bright version of the foreground color
            fgIndex += 8;
        }

        if (swapFgAndBg)
        {
            const auto fg = _adjustedForegroundColors[fgIndex][bgIndex];
            const auto bg = fgTextColor.GetColor(_colorTable, defaultFgIndex);
            return { fg, bg };
        }
        else
        {
            const auto fg = _adjustedForegroundColors[bgIndex][fgIndex];
            const auto bg = bgTextColor.GetColor(_colorTable, defaultBgIndex);
            return { fg, bg };
        }

Review comment (performance):
I believe these two lines should be swapped.

In practice it really won't make a difference here (because a compiler _should_ be smart enough to figure out that `IsEnabled()` equals `false` and that the entire if condition can be optimized away), but in theory at least there's no guarantee that the compiler will always be smart enough to optimize it away. Additionally this only works well if the previous call(s) are inlineable and have no side effects (allowing it to be optimized away).

That's why we should try to move conditions that have the highest chance of being predictable to the beginning of a boolean expression.

Code after:

// Routine Description:
// - Calculates the RGB colors of a given text attribute, using the current
//   color table configuration and active render settings.
// Arguments:
// - attr - The TextAttribute to retrieve the colors for.
// Return Value:
// - The color values of the attribute's foreground and background.
std::pair<COLORREF, COLORREF> RenderSettings::GetAttributeColors(const TextAttribute& attr) const noexcept
{
    _blinkIsInUse = _blinkIsInUse || attr.IsBlinking();

    const auto fgTextColor = attr.GetForeground();
    const auto bgTextColor = attr.GetBackground();

    const auto defaultFgIndex = GetColorAliasIndex(ColorAlias::DefaultForeground);
    const auto defaultBgIndex = GetColorAliasIndex(ColorAlias::DefaultBackground);

    const auto brightenFg = attr.IsBold() && GetRenderMode(Mode::IntenseIsBright);
    const auto dimFg = attr.IsFaint() || (_blinkShouldBeFaint && attr.IsBlinking());
    const auto swapFgAndBg = attr.IsReverseVideo() ^ GetRenderMode(Mode::ScreenReversed);

    // We want to nudge the foreground color to make it more perceivable only for the
    // default color pairs within the color table
    if (Feature_AdjustIndistinguishableText::IsEnabled() &&
        GetRenderMode(Mode::DistinguishableColors) &&
        !dimFg &&
        (fgTextColor.IsDefault() || fgTextColor.IsLegacy()) &&
        (bgTextColor.IsDefault() || bgTextColor.IsLegacy()))
    {
        const auto bgIndex = bgTextColor.IsDefault() ? AdjustedBgIndex : bgTextColor.GetIndex();
        auto fgIndex = fgTextColor.IsDefault() ? AdjustedFgIndex : fgTextColor.GetIndex();

        if (fgTextColor.IsIndex16() && (fgIndex < 8) && brightenFg)
        {
            // There is a special case for bold here - we need to get the bright version of the foreground color
            fgIndex += 8;
        }

        if (swapFgAndBg)
        {
            const auto fg = _adjustedForegroundColors[fgIndex][bgIndex];
            const auto bg = fgTextColor.GetColor(_colorTable, defaultFgIndex);
            return { fg, bg };
        }
        else
        {
            const auto fg = _adjustedForegroundColors[bgIndex][fgIndex];
            const auto bg = bgTextColor.GetColor(_colorTable, defaultBgIndex);
            return { fg, bg };
        }