Repository: microsoft/terminal
File: src/renderer/atlas/AtlasEngine.api.cpp (C++)

Code before:

    // Point sizes are commonly treated at a 72 DPI scale
    // (including by OpenType), whereas DirectWrite uses 96 DPI.
    // Since we want the height in px we multiply by the display's DPI.
    const auto fontSizeInPx = std::ceil(requestedSize.Y / 72.0 * _api.dpi);

    const auto designUnitsPerPx = fontSizeInPx / static_cast<double>(metrics.designUnitsPerEm);
    const auto ascentInPx = static_cast<double>(metrics.ascent) * designUnitsPerPx;
    const auto descentInPx = static_cast<double>(metrics.descent) * designUnitsPerPx;
    const auto lineGapInPx = static_cast<double>(metrics.lineGap) * designUnitsPerPx;
    const auto advanceWidthInPx = static_cast<double>(glyphMetrics.advanceWidth) * designUnitsPerPx;
    const auto underlineOffsetInPx = static_cast<double>(-metrics.underlinePosition) * designUnitsPerPx;
    const auto underlineThicknessInPx = static_cast<double>(metrics.underlineThickness) * designUnitsPerPx;
    const auto strikethroughOffsetInPx = static_cast<double>(-metrics.strikethroughPosition) * designUnitsPerPx;
    const auto strikethroughThicknessInPx = static_cast<double>(metrics.strikethroughThickness) * designUnitsPerPx;
    const auto lineThicknessInPx = std::round(std::max(1.0, std::min(underlineThicknessInPx, strikethroughThicknessInPx)));

    const auto halfGapInPx = lineGapInPx / 2.0;
    const auto baseline = std::ceil(ascentInPx + halfGapInPx);
    const auto underlinePosInPx = std::ceil(baseline + underlineOffsetInPx - lineThicknessInPx / 2.0);
    const auto strikethroughPosInPx = std::round(baseline + strikethroughOffsetInPx - lineThicknessInPx / 2.0);
    const auto cellHeightViaDescent = std::ceil(baseline + descentInPx + halfGapInPx);
    // A double-underline is 3 lines tall due to a 1 line gap between the two lines.
    // This logic should be kept in sync with AtlasEngine::_updateConstantBuffer,
    // which calculates the offset/position of the lower line.
    const auto cellHeightViaUnderlines = underlinePosInPx + 3 * lineThicknessInPx;

    const auto cellWidth = gsl::narrow<u16>(std::ceil(advanceWidthInPx));
    const auto cellHeight = gsl::narrow<u16>(std::max(cellHeightViaDescent, cellHeightViaUnderlines));

    {
        til::size coordSize;
        coordSize.X = cellWidth;
        coordSize.Y = cellHeight;

        if (requestedSize.X == 0)
        {
            // The coordSizeUnscaled parameter to SetFromEngine is used for API functions like GetConsoleFontSize.
            // Since clients expect that settings the font height to Y yields back a font height of Y,
            // we're scaling the X relative/proportional to the actual cellWidth/cellHeight ratio.
            // The code below uses a poor form of integer rounding.
            requestedSize.X = (requestedSize.Y * cellWidth + cellHeight / 2) / cellHeight;
        }

        fontInfo.SetFromEngine(requestedFaceName, requestedFamily, requestedWeight, false, coordSize, requestedSize);
    }

    if (fontMetrics)
    {
        const auto lineThickness = gsl::narrow<u16>(lineThicknessInPx);
        const auto underlinePos = gsl::narrow<u16>(underlinePosInPx);

Review comment (suggestion):
With Consolas for instance the `cellHeightViaUnderlines` is usally larger than the `cellHeightViaDescent`. Without this code change here a double-underline would appear clipped off.

Code after:

    // Point sizes are commonly treated at a 72 DPI scale
    // (including by OpenType), whereas DirectWrite uses 96 DPI.
    // Since we want the height in px we multiply by the display's DPI.
    const auto fontSize = std::ceilf(requestedSize.Y / 72.0f * _api.dpi);

    const auto designUnitsPerPx = fontSize / static_cast<float>(metrics.designUnitsPerEm);
    const auto ascent = static_cast<float>(metrics.ascent) * designUnitsPerPx;
    const auto descent = static_cast<float>(metrics.descent) * designUnitsPerPx;
    const auto lineGap = static_cast<float>(metrics.lineGap) * designUnitsPerPx;
    const auto advanceWidth = static_cast<float>(glyphMetrics.advanceWidth) * designUnitsPerPx;
    const auto underlinePosition = static_cast<float>(-metrics.underlinePosition) * designUnitsPerPx;
    const auto underlineThickness = static_cast<float>(metrics.underlineThickness) * designUnitsPerPx;
    const auto strikethroughPosition = static_cast<float>(-metrics.strikethroughPosition) * designUnitsPerPx;
    const auto strikethroughThickness = static_cast<float>(metrics.strikethroughThickness) * designUnitsPerPx;

    const auto halfGap = lineGap / 2.0f;
    const auto baseline = std::ceilf(ascent + halfGap);
    const auto lineHeight = std::ceilf(baseline + descent + halfGap);
    const auto underlinePos = std::roundf(baseline + underlinePosition);
    const auto underlineWidth = std::max(1.0f, std::roundf(underlineThickness));
    const auto strikethroughPos = std::roundf(baseline + strikethroughPosition);
    const auto strikethroughWidth = std::max(1.0f, std::roundf(strikethroughThickness));
    const auto thinLineWidth = std::max(1.0f, std::roundf(underlineThickness / 2.0f));

    // For double underlines we loosely follow what Word does:
    // 1. The lines are half the width of an underline (= thinLineWidth)
    // 2. Ideally the bottom line is aligned with the bottom of the underline
    // 3. The top underline is vertically in the middle between baseline and ideal bottom underline
    // 4. If the top line gets too close to the baseline the underlines are shifted downwards
    // 5. The minimum gap between the two lines appears to be similar to Tex (1.2pt)
    // (Additional notes below.)

    // 2.
    auto doubleUnderlinePosBottom = underlinePos + underlineWidth - thinLineWidth;
    // 3. Since we don't align the center of our two lines, but rather the top borders
    //    we need to subtract half a line width from our center point.
    auto doubleUnderlinePosTop = std::roundf((baseline + doubleUnderlinePosBottom - thinLineWidth) / 2.0f);
    // 4.
    doubleUnderlinePosTop = std::max(doubleUnderlinePosTop, baseline + thinLineWidth);
    // 5. The gap is only the distance _between_ the lines, but we need the distance from the
    //    top border of the top and bottom lines, which includes an additional line width.
    const auto doubleUnderlineGap = std::max(1.0f, std::roundf(1.2f / 72.0f * _api.dpi));
    doubleUnderlinePosBottom = std::max(doubleUnderlinePosBottom, doubleUnderlinePosTop + doubleUnderlineGap + thinLineWidth);
    // Our cells can't overlap each other so we additionally clamp the bottom line to be inside the cell boundaries.
    doubleUnderlinePosBottom = std::min(doubleUnderlinePosBottom, lineHeight - thinLineWidth);

    const auto cellWidth = gsl::narrow<u16>(std::ceilf(advanceWidth));
    const auto cellHeight = gsl::narrow<u16>(lineHeight);

    {