Repository: microsoft/terminal
File: src/renderer/gdi/state.cpp (C++)

Code before:
                                    _lineMetrics.underlineWidth +
                                    std::lround(fontSize * 0.05);

    // However, we don't want the underline to extend past the bottom of the
    // cell, so we clamp the offset to fit just inside.
    const auto maxUnderlineOffset = Font.GetSize().height - _lineMetrics.underlineWidth;
    _lineMetrics.underlineOffset2 = std::min(_lineMetrics.underlineOffset2, maxUnderlineOffset);

    // But if the resulting gap isn't big enough even to register as a thicker
    // line, it's better to place the second line slightly above the first.
    if (_lineMetrics.underlineOffset2 < _lineMetrics.underlineOffset + _lineMetrics.gridlineWidth)
    {
        _lineMetrics.underlineOffset2 = _lineMetrics.underlineOffset - _lineMetrics.gridlineWidth;
    }

    const int strokeHalfWidth = std::lround(_lineMetrics.underlineWidth / 2.0);

    // Since we use GDI pen for drawing, the underline offset should point to
    // the center of the underline.
    _lineMetrics.underlineOffset += strokeHalfWidth;
    _lineMetrics.underlineOffset2 += strokeHalfWidth;

    // We want the underline to always be visible and remain within the cell
    // bottom, so we clamp the offset to fit just inside.
    _lineMetrics.underlineOffset = std::min(_lineMetrics.underlineOffset, maxUnderlineOffset);
    _lineMetrics.underlineOffset2 = std::min(_lineMetrics.underlineOffset2, maxUnderlineOffset);

    // Curlyline uses the gap between cell bottom and singly underline position
    // as the height of the wave's peak. The baseline for curly line is at the
    // middle of singly underline. The gap could be too big, so we also apply a
    // limit on the peak height.
    {
        const auto cellBottomGap = std::lround(Font.GetSize().height - _lineMetrics.underlineOffset - strokeHalfWidth);

        // get the max height for curly line peak. Max height is in `em` units.
        constexpr auto maxCurlyLinePeakHeightEm = 0.075f;
        const auto maxCurlyLinePeakHeight = std::lround(maxCurlyLinePeakHeightEm * fontSize);

        _lineMetrics.curlylinePeakHeight = std::clamp(cellBottomGap, 0L, maxCurlyLinePeakHeight);
    }

    // Now find the size of a 0 in this current font and save it for conversions done later.
    _coordFontLast = Font.GetSize();

    // Persist font for cleanup (and free existing if necessary)
    if (_hfont != nullptr)
    {
        LOG_HR_IF(E_FAIL, !(DeleteObject(_hfont)));
        _hfont = nullptr;
    }


Review comment (bug):
This still seems wrong to me. The offsets have already been clamped prior to this, so no additional clamping should have been necessary. The `strokeHalfWidth` adjustment should just be accounting for the difference between a `PatBlt` rendering and a `LineTo` rendering. If it was correct, then the `LineTo` rendering should exactly match where the `PatBlt` would have rendered without that adjustment. If it ends up out of bounds, then that's clearly not the case, so something must be wrong with the `strokeHalfWidth` calculation. Maybe a rounding issue? I don't know.

Worst case, if you can't get it to work, at least move the `strokeHalfWidth` adjustment higher up, prior to where we're doing the first clamping (and get rid of this second set of clamping). Because we've got special case handling for the double underline offset, which is potentially going to be broken if you clamp it a second time.

Code after:
                                    _lineMetrics.underlineWidth +
                                    std::lround(fontSize * 0.05);

    // However, we don't want the underline to extend past the bottom of the
    // cell, so we clamp the offset to fit just inside.
    const auto maxUnderlineOffset = Font.GetSize().height - _lineMetrics.underlineWidth;
    _lineMetrics.underlineOffset2 = std::min(_lineMetrics.underlineOffset2, maxUnderlineOffset);

    // But if the resulting gap isn't big enough even to register as a thicker
    // line, it's better to place the second line slightly above the first.
    if (_lineMetrics.underlineOffset2 < _lineMetrics.underlineOffset + _lineMetrics.gridlineWidth)
    {
        _lineMetrics.underlineOffset2 = _lineMetrics.underlineOffset - _lineMetrics.gridlineWidth;
    }

    // Since we use GDI pen for drawing, the underline offset should point to
    // the center of the underline.
    const auto underlineHalfWidth = gsl::narrow_cast<int>(std::floor(_lineMetrics.underlineWidth / 2.0f));
    _lineMetrics.underlineOffset += underlineHalfWidth;
    _lineMetrics.underlineOffset2 += underlineHalfWidth;

    // Curlyline is drawn with a desired height relative to the font size. The
    // baseline of curlyline is at the middle of singly underline. When there's
    // limited space to draw a curlyline, we apply a limit on the peak height.
    {
        // initialize curlyline peak height to a desired value. Clamp it to at
        // least 1.
        constexpr auto curlyLinePeakHeightEm = 0.075f;
        _lineMetrics.curlylinePeakHeight = gsl::narrow_cast<int>(std::max(1L, std::lround(curlyLinePeakHeightEm * fontSize)));

        // calc the limit we need to apply
        const auto maxDrawableCurlyLinePeakHeight = Font.GetSize().height - _lineMetrics.underlineOffset - _lineMetrics.underlineWidth;

        // if the limit is <= 0 (no height at all), stick with the desired height.
        // This is how we force a curlyline even when there's no space, though it
        // might be clipped at the bottom.
        if (maxDrawableCurlyLinePeakHeight > 0.0f)
        {
            _lineMetrics.curlylinePeakHeight = std::min(_lineMetrics.curlylinePeakHeight, maxDrawableCurlyLinePeakHeight);
        }
    }

    // Now find the size of a 0 in this current font and save it for conversions done later.
    _coordFontLast = Font.GetSize();

    // Persist font for cleanup (and free existing if necessary)
    if (_hfont != nullptr)
    {
        LOG_HR_IF(E_FAIL, !(DeleteObject(_hfont)));
        _hfont = nullptr;
    }