Repository: microsoft/terminal
File: src/renderer/dx/DxFontRenderData.cpp (C++)

Code before:
// Arguments:
// - axes - the axes to update our map with
void DxFontRenderData::_SetAxes(const std::unordered_map<std::wstring_view, float>& axes)
{
    _axesVector.clear();
    _didUserSetAxes = false;
    _didUserSetItalic = false;

    // Update our axis map with the provided axes
    if (!axes.empty())
    {
#pragma warning(suppress : 26445) // the analyzer doesn't like reference to string_view
        for (const auto& [axis, value] : axes)
        {
            if (axis.length() == TAG_LENGTH)
            {
                const auto dwriteFontAxis = DWRITE_FONT_AXIS_VALUE{ DWRITE_MAKE_FONT_AXIS_TAG(til::at(axis, 0), til::at(axis, 1), til::at(axis, 2), til::at(axis, 3)), value };
                _axesVector.emplace_back(dwriteFontAxis);
                if (dwriteFontAxis.axisTag != DWRITE_FONT_AXIS_TAG_WEIGHT)
                {
                    _axesVectorWithoutWeight.emplace_back(dwriteFontAxis);
                }
                if (dwriteFontAxis.axisTag == DWRITE_FONT_AXIS_TAG_ITALIC && value == 1)
                {
                    _didUserSetItalic = true;
                }
            }
        }
        _didUserSetAxes = true;
    }
}

// Method Description:
// - Converts a DWRITE_FONT_STRETCH enum into the corresponding float value to
//   create a DWRITE_FONT_AXIS_VALUE with
// Arguments:
// - fontStretch: the old DWRITE_FONT_STRETCH enum to be converted into an axis value
// Return value:
// - The float value corresponding to the passed in fontStretch
float DxFontRenderData::_FontStretchToWidthAxisValue(DWRITE_FONT_STRETCH fontStretch) noexcept
{
    // 10 elements from DWRITE_FONT_STRETCH_UNDEFINED (0) to DWRITE_FONT_STRETCH_ULTRA_EXPANDED (9)
    static constexpr auto fontStretchEnumToVal = std::array{ 100.0f, 50.0f, 62.5f, 75.0f, 87.5f, 100.0f, 112.5f, 125.0f, 150.0f, 200.0f };

    if (gsl::narrow_cast<size_t>(fontStretch) > fontStretchEnumToVal.size())
    {
        fontStretch = DWRITE_FONT_STRETCH_NORMAL;
    }

    return til::at(fontStretchEnumToVal, fontStretch);
}

Review comment (performance):
Personally I'd shorten these things to
```cpp
_didUserSetItalic |= dwriteFontAxis.axisTag == DWRITE_FONT_AXIS_TAG_ITALIC && value == 1;
```

The benefits as I personally, subjectivelly perceive them:
* Easier to read.
* It compiles to what GCC/Clang would produce. MSVC always needs a bit of handholding when it comes to booleans.
* And a longer one...

CPUs appear to be really fast nowadays, but they're actually often just as dumb as they used to be. One of the problems is that the "predictor" for branches has a limited size. While an important branch like this has literally 0 performance impact on this code and is absolutely fine, it might displace actually important branches out of the limited branch predictor cache.

For instance I have a PR open which simply removes `std::deque` and improves performance by ~5% on my CPU. This is really weird actually, because if you benchmark `std::deque` in a small benchmark project you'll find that it's super fast. And it's only iterated about ~100/sec! Still... it's mere presence has a negative impact, despite being fast itself. CPUs are dumb.

That said I wouldn't mind merging it as is. 😄

Code after:
// Arguments:
// - axes - the axes to update our map with
void DxFontRenderData::_SetAxes(const std::unordered_map<std::wstring_view, float>& axes)
{
    // Clear out the old vector and booleans in case this is a hot reload
    _axesVector = std::vector<DWRITE_FONT_AXIS_VALUE>{};
    _didUserSetAxes = false;
    _didUserSetItalic = false;

    // Update our axis map with the provided axes
    if (!axes.empty())
    {
        // Store the weight aside: we will be creating a span of all the axes in the vector except the weight,
        // and then we will add the weight to the vector
        // We are doing this so that when the text attribute is bold, we can apply all the axes except the weight
        std::optional<DWRITE_FONT_AXIS_VALUE> weightAxis;

        // Since we are calling an 'emplace_back' after creating the span,
        // there is a chance a reallocation happens (if the vector needs to grow), which would make the span point to
        // deallocated memory. To avoid this, make sure to reserve enough memory in the vector.
        _axesVector.reserve(axes.size());

#pragma warning(suppress : 26445) // the analyzer doesn't like reference to string_view
        for (const auto& [axis, value] : axes)
        {
            if (axis.length() == TAG_LENGTH)
            {
                const auto dwriteFontAxis = DWRITE_FONT_AXIS_VALUE{ DWRITE_MAKE_FONT_AXIS_TAG(til::at(axis, 0), til::at(axis, 1), til::at(axis, 2), til::at(axis, 3)), value };
                if (dwriteFontAxis.axisTag != DWRITE_FONT_AXIS_TAG_WEIGHT)
                {
                    _axesVector.emplace_back(dwriteFontAxis);
                }
                else
                {
                    weightAxis = dwriteFontAxis;
                }
                _didUserSetItalic |= dwriteFontAxis.axisTag == DWRITE_FONT_AXIS_TAG_ITALIC && value == 1;
            }
        }

        // Make the span, which has all the axes except the weight
        _axesVectorWithoutWeight = gsl::make_span(_axesVector);

        // Add the weight axis to the vector if needed
        if (weightAxis)
        {
            _axesVector.emplace_back(weightAxis.value());
        }
        _didUserSetAxes = true;
    }
}