Skip to content

Commit 72660ba

Browse files
committed
atlas: display underlines in the right color when they're highlighted
This pull request resolves a longstanding issue (which probably nobody but me cared about :P) where gridlines and underlines did not change color to match the selection or search highlight foreground. During brush preparation, we stash the underline color; during line rendering we store it in a third bitmap and during rendering in each backend we read the colors out of the appropriate bitmap. The gridlines come from the foreground bitmap and the underlines from the new underline bitmap. I've updated each instance of append*Line to break the line-to-render up by color span. We are, therefore, no longer passing the stroke color in each gridline payload. The original console renderer supports painting the gridlines in different colors for each half of a 2-cell glyph. That functionality no longer works in Atlas. To fix it, we would need to move underline _span_ preparation into PaintBufferLine (so that we could apply the right colors to the bitmap mid-cell.) Known Issues ------------ - D2D only; dotted lines which are broken by a highlight have doubled dots (because we draw them as individual line segments and without a global geometry like curly lines.) - Atlas (all); grid line colors can no longer change in the middle of a 2-cell glyph. Tested By --------- RenderingTests
1 parent 83b9569 commit 72660ba

File tree

5 files changed

+122
-65
lines changed

5 files changed

+122
-65
lines changed

src/renderer/atlas/AtlasEngine.cpp

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ try
203203
auto dst = _p.colorBitmap.data() + dstOffset;
204204
const auto bytes = count * sizeof(u32);
205205

206-
for (size_t i = 0; i < 2; ++i)
206+
for (size_t i = 0; i < 3; ++i)
207207
{
208208
// Avoid bumping the colorBitmapGeneration unless necessary. This approx. further halves
209209
// the (already small) GPU load. This could easily be replaced with some custom SIMD
@@ -353,7 +353,7 @@ CATCH_RETURN()
353353
return S_OK;
354354
}
355355

356-
void AtlasEngine::_fillColorBitmap(const size_t y, const size_t x1, const size_t x2, const u32 fgColor, const u32 bgColor) noexcept
356+
void AtlasEngine::_fillColorBitmap(const size_t y, const size_t x1, const size_t x2, const u32 fgColor, const u32 bgColor, const u32 ulColor) noexcept
357357
{
358358
const auto bitmap = _p.colorBitmap.begin() + _p.colorBitmapRowStride * y;
359359
const auto shift = gsl::narrow_cast<u8>(_p.rows[y]->lineRendition != LineRendition::SingleWidth);
@@ -363,10 +363,11 @@ void AtlasEngine::_fillColorBitmap(const size_t y, const size_t x1, const size_t
363363
const u32 colors[] = {
364364
u32ColorPremultiply(bgColor),
365365
fgColor,
366+
ulColor,
366367
};
367368

368369
// This fills the color in the background bitmap, and then in the foreground bitmap.
369-
for (size_t i = 0; i < 2; ++i)
370+
for (size_t i = 0; i < 3; ++i)
370371
{
371372
const auto color = colors[i];
372373

@@ -428,7 +429,7 @@ try
428429
{
429430
const auto isFinalRow = y == hiEnd.y;
430431
const auto end = isFinalRow ? std::min(hiEnd.x, x2) : x2;
431-
_fillColorBitmap(row, x1, end, fgColor, bgColor);
432+
_fillColorBitmap(row, x1, end, fgColor, bgColor, fgColor);
432433

433434
// Return early if we couldn't paint the whole region (either this was not the last row, or
434435
// it was the last row but the highlight ends outside of our x range.)
@@ -451,7 +452,7 @@ try
451452
const auto isEndInside = y == hiEnd.y && hiEnd.x <= x2;
452453
if (isStartInside && isEndInside)
453454
{
454-
_fillColorBitmap(row, hiStart.x, static_cast<size_t>(hiEnd.x), fgColor, bgColor);
455+
_fillColorBitmap(row, hiStart.x, static_cast<size_t>(hiEnd.x), fgColor, bgColor, fgColor);
455456
++it;
456457
}
457458
else
@@ -460,7 +461,7 @@ try
460461
if (isStartInside)
461462
{
462463
const auto start = std::max(x1, hiStart.x);
463-
_fillColorBitmap(y, start, x2, fgColor, bgColor);
464+
_fillColorBitmap(y, start, x2, fgColor, bgColor, fgColor);
464465
}
465466

466467
break;
@@ -517,7 +518,7 @@ try
517518
}
518519

519520
// Apply the current foreground and background colors to the cells
520-
_fillColorBitmap(y, x, columnEnd, _api.currentForeground, _api.currentBackground);
521+
_fillColorBitmap(y, x, columnEnd, _api.currentForeground, _api.currentBackground, _api.currentUnderlineColor);
521522

522523
// Apply the highlighting colors to the highlighted cells
523524
RETURN_IF_FAILED(_drawHighlighted(_api.searchHighlights, y, x, columnEnd, highlightFg, highlightBg));
@@ -532,14 +533,14 @@ CATCH_RETURN()
532533
[[nodiscard]] HRESULT AtlasEngine::PaintBufferGridLines(const GridLineSet lines, const COLORREF gridlineColor, const COLORREF underlineColor, const size_t cchLine, const til::point coordTarget) noexcept
533534
try
534535
{
536+
UNREFERENCED_PARAMETER(gridlineColor);
537+
UNREFERENCED_PARAMETER(underlineColor);
535538
const auto shift = gsl::narrow_cast<u8>(_api.lineRendition != LineRendition::SingleWidth);
536539
const auto x = std::max(0, coordTarget.x - (_api.viewportOffset.x >> shift));
537540
const auto y = gsl::narrow_cast<u16>(clamp<til::CoordType>(coordTarget.y, 0, _p.s->viewportCellCount.y - 1));
538541
const auto from = gsl::narrow_cast<u16>(clamp<til::CoordType>(x << shift, 0, _p.s->viewportCellCount.x - 1));
539542
const auto to = gsl::narrow_cast<u16>(clamp<size_t>((x + cchLine) << shift, from, _p.s->viewportCellCount.x));
540-
const auto glColor = gsl::narrow_cast<u32>(gridlineColor) | 0xff000000;
541-
const auto ulColor = gsl::narrow_cast<u32>(underlineColor) | 0xff000000;
542-
_p.rows[y]->gridLineRanges.emplace_back(lines, glColor, ulColor, from, to);
543+
_p.rows[y]->gridLineRanges.emplace_back(lines, from, to);
543544
return S_OK;
544545
}
545546
CATCH_RETURN()
@@ -650,7 +651,9 @@ CATCH_RETURN()
650651
try
651652
{
652653
auto [fg, bg] = renderSettings.GetAttributeColorsWithAlpha(textAttributes);
654+
auto ul = renderSettings.GetAttributeUnderlineColor(textAttributes);
653655
fg |= 0xff000000;
656+
ul |= 0xff000000;
654657
bg |= _api.backgroundOpaqueMixin;
655658

656659
if (!isSettingDefaultBrushes)
@@ -666,6 +669,7 @@ try
666669

667670
_api.currentBackground = gsl::narrow_cast<u32>(bg);
668671
_api.currentForeground = gsl::narrow_cast<u32>(fg);
672+
_api.currentUnderlineColor = gsl::narrow_cast<u32>(ul);
669673
_api.attributes = attributes;
670674
}
671675
else
@@ -680,6 +684,11 @@ try
680684
{
681685
_api.s.write()->misc.write()->foregroundColor = fg;
682686
}
687+
688+
if (textAttributes.GetUnderlineColor().IsDefault() && fg != _api.s->misc->underlineColor)
689+
{
690+
_api.s.write()->misc.write()->underlineColor = ul;
691+
}
683692
}
684693

685694
return S_OK;
@@ -791,9 +800,10 @@ void AtlasEngine::_recreateCellCountDependentResources()
791800
// so we round up to multiple of 8 because 8 * sizeof(u32) == 32.
792801
_p.colorBitmapRowStride = alignForward<size_t>(_p.s->viewportCellCount.x, 8);
793802
_p.colorBitmapDepthStride = _p.colorBitmapRowStride * _p.s->viewportCellCount.y;
794-
_p.colorBitmap = Buffer<u32, 32>(_p.colorBitmapDepthStride * 2);
803+
_p.colorBitmap = Buffer<u32, 32>(_p.colorBitmapDepthStride * 3);
795804
_p.backgroundBitmap = { _p.colorBitmap.data(), _p.colorBitmapDepthStride };
796805
_p.foregroundBitmap = { _p.colorBitmap.data() + _p.colorBitmapDepthStride, _p.colorBitmapDepthStride };
806+
_p.underlineBitmap = { _p.colorBitmap.data() + (_p.colorBitmapDepthStride << 1), _p.colorBitmapDepthStride };
797807

798808
memset(_p.colorBitmap.data(), 0, _p.colorBitmap.size() * sizeof(u32));
799809

src/renderer/atlas/AtlasEngine.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ namespace Microsoft::Console::Render::Atlas
8989
void _mapCharacters(const wchar_t* text, u32 textLength, u32* mappedLength, IDWriteFontFace2** mappedFontFace) const;
9090
void _mapComplex(IDWriteFontFace2* mappedFontFace, u32 idx, u32 length, ShapedRow& row);
9191
ATLAS_ATTR_COLD void _mapReplacementCharacter(u32 from, u32 to, ShapedRow& row);
92-
void _fillColorBitmap(const size_t y, const size_t x1, const size_t x2, const u32 fgColor, const u32 bgColor) noexcept;
92+
void _fillColorBitmap(const size_t y, const size_t x1, const size_t x2, const u32 fgColor, const u32 bgColor, const u32 ulColor) noexcept;
9393
[[nodiscard]] HRESULT _drawHighlighted(std::span<const til::point_span>& highlights, const u16 row, const u16 begX, const u16 endX, const u32 fgColor, const u32 bgColor) noexcept;
9494

9595
// AtlasEngine.api.cpp
@@ -162,6 +162,7 @@ namespace Microsoft::Console::Render::Atlas
162162
u32 backgroundOpaqueMixin = 0xff000000;
163163
u32 currentBackground = 0;
164164
u32 currentForeground = 0;
165+
u32 currentUnderlineColor = 0;
165166
FontRelevantAttributes attributes = FontRelevantAttributes::None;
166167
u16x2 lastPaintBufferLineCoord{};
167168
// UpdateHyperlinkHoveredId()

src/renderer/atlas/BackendD2D.cpp

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -619,38 +619,52 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
619619
const auto cellCenter = row->lineRendition == LineRendition::DoubleHeightTop ? rowBottom : rowTop;
620620
const auto scaleHorizontal = row->lineRendition != LineRendition::SingleWidth ? 0.5f : 1.0f;
621621
const auto scaledCellWidth = cellWidth * scaleHorizontal;
622+
const auto horizontalShift = static_cast<u8>(row->lineRendition != LineRendition::SingleWidth);
622623

623624
const auto appendVerticalLines = [&](const GridLineRange& r, FontDecorationPosition pos) {
624625
const auto from = r.from * scaledCellWidth;
625626
const auto to = r.to * scaledCellWidth;
626627
auto x = from + pos.position;
627628

629+
const auto colors = &p.foregroundBitmap[p.colorBitmapRowStride * y];
630+
628631
D2D1_POINT_2F point0{ 0, cellCenter };
629632
D2D1_POINT_2F point1{ 0, cellCenter + cellHeight };
630-
const auto brush = _brushWithColor(r.gridlineColor);
631633
const f32 w = pos.height;
632634
const f32 hw = w * 0.5f;
633635

634-
for (; x < to; x += cellWidth)
636+
auto c = r.from;
637+
for (; x < to; x += cellWidth, c += 1 << horizontalShift)
635638
{
639+
const auto brush = _brushWithColor(colors[c]);
636640
const auto centerX = x + hw;
637641
point0.x = centerX;
638642
point1.x = centerX;
639643
_renderTarget->DrawLine(point0, point1, brush, w, nullptr);
640644
}
641645
};
642-
const auto appendHorizontalLine = [&](const GridLineRange& r, FontDecorationPosition pos, ID2D1StrokeStyle* strokeStyle, const u32 color) {
643-
const auto from = r.from * scaledCellWidth;
644-
const auto to = r.to * scaledCellWidth;
646+
const auto appendHorizontalLine = [&](const GridLineRange& r, FontDecorationPosition pos, ID2D1StrokeStyle* strokeStyle, const std::span<const u32>& colorBitmap) {
647+
const auto colors = &colorBitmap[p.colorBitmapRowStride * y];
645648

646-
const auto brush = _brushWithColor(color);
647649
const f32 w = pos.height;
648650
const f32 centerY = cellCenter + pos.position + w * 0.5f;
649-
const D2D1_POINT_2F point0{ from, centerY };
650-
const D2D1_POINT_2F point1{ to, centerY };
651-
_renderTarget->DrawLine(point0, point1, brush, w, strokeStyle);
651+
652+
for (auto from = r.from; from < r.to;)
653+
{
654+
const auto start = colors[from];
655+
u16 run = 1u;
656+
for (; colors[from + run] == start && run < (r.to - from); ++run)
657+
;
658+
659+
const auto brush = _brushWithColor(start);
660+
const D2D1_POINT_2F point0{ from * scaledCellWidth, centerY };
661+
const D2D1_POINT_2F point1{ (from + run) * scaledCellWidth, centerY };
662+
_renderTarget->DrawLine(point0, point1, brush, w, strokeStyle);
663+
664+
from += run;
665+
}
652666
};
653-
const auto appendCurlyLine = [&](const GridLineRange& r) {
667+
const auto appendCurlyLine = [&](const GridLineRange& r, const std::span<const u32>& colorBitmap) {
654668
const auto& font = *p.s->font;
655669

656670
const auto duTop = static_cast<f32>(font.doubleUnderline[0].position);
@@ -672,11 +686,16 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
672686
const auto step = roundf(0.5f * height);
673687
const auto period = 4.0f * step;
674688

675-
const auto from = r.from * scaledCellWidth;
676-
const auto to = r.to * scaledCellWidth;
689+
const auto colors = &colorBitmap[p.colorBitmapRowStride * y];
690+
691+
// Calculate the wave over the entire region to be underlined, even if
692+
// it has multiple colors in it. That way, when we clip it to render each
693+
// color, it is seamless.
694+
const auto fullSpanFrom = r.from * scaledCellWidth;
695+
const auto fullSpanTo = r.to * scaledCellWidth;
677696
// Align the start of the wave to the nearest preceding period boundary.
678697
// This ensures that the wave is continuous across color and cell changes.
679-
auto x = floorf(from / period) * period;
698+
auto x = floorf(fullSpanFrom / period) * period;
680699

681700
wil::com_ptr<ID2D1PathGeometry> geometry;
682701
THROW_IF_FAILED(p.d2dFactory->CreatePathGeometry(geometry.addressof()));
@@ -686,7 +705,7 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
686705

687706
// This adds complete periods of the wave until we reach the end of the range.
688707
sink->BeginFigure({ x, center }, D2D1_FIGURE_BEGIN_HOLLOW);
689-
for (D2D1_QUADRATIC_BEZIER_SEGMENT segment; x < to;)
708+
for (D2D1_QUADRATIC_BEZIER_SEGMENT segment; x < fullSpanTo;)
690709
{
691710
x += step;
692711
segment.point1.x = x;
@@ -708,11 +727,21 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
708727

709728
THROW_IF_FAILED(sink->Close());
710729

711-
const auto brush = _brushWithColor(r.underlineColor);
712-
const D2D1_RECT_F clipRect{ from, rowTop, to, rowBottom };
713-
_renderTarget->PushAxisAlignedClip(&clipRect, D2D1_ANTIALIAS_MODE_ALIASED);
714-
_renderTarget->DrawGeometry(geometry.get(), brush, duHeight, nullptr);
715-
_renderTarget->PopAxisAlignedClip();
730+
for (auto from = r.from; from < r.to;)
731+
{
732+
const auto start = colors[from];
733+
u16 run = 1u;
734+
for (; colors[from + run] == start && run < (r.to - from); ++run)
735+
;
736+
737+
const auto brush = _brushWithColor(start);
738+
const D2D1_RECT_F clipRect{ (from * scaledCellWidth), rowTop, (from + run) * scaledCellWidth, rowBottom };
739+
_renderTarget->PushAxisAlignedClip(&clipRect, D2D1_ANTIALIAS_MODE_ALIASED);
740+
_renderTarget->DrawGeometry(geometry.get(), brush, duHeight, nullptr);
741+
_renderTarget->PopAxisAlignedClip();
742+
743+
from += run;
744+
}
716745
};
717746

718747
for (const auto& r : row->gridLineRanges)
@@ -730,20 +759,20 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
730759
}
731760
if (r.lines.test(GridLines::Top))
732761
{
733-
appendHorizontalLine(r, p.s->font->gridTop, nullptr, r.gridlineColor);
762+
appendHorizontalLine(r, p.s->font->gridTop, nullptr, p.foregroundBitmap);
734763
}
735764
if (r.lines.test(GridLines::Bottom))
736765
{
737-
appendHorizontalLine(r, p.s->font->gridBottom, nullptr, r.gridlineColor);
766+
appendHorizontalLine(r, p.s->font->gridBottom, nullptr, p.foregroundBitmap);
738767
}
739768
if (r.lines.test(GridLines::Strikethrough))
740769
{
741-
appendHorizontalLine(r, p.s->font->strikethrough, nullptr, r.gridlineColor);
770+
appendHorizontalLine(r, p.s->font->strikethrough, nullptr, p.foregroundBitmap);
742771
}
743772

744773
if (r.lines.test(GridLines::Underline))
745774
{
746-
appendHorizontalLine(r, p.s->font->underline, nullptr, r.underlineColor);
775+
appendHorizontalLine(r, p.s->font->underline, nullptr, p.underlineBitmap);
747776
}
748777
else if (r.lines.any(GridLines::DottedUnderline, GridLines::HyperlinkUnderline))
749778
{
@@ -753,7 +782,7 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
753782
static constexpr FLOAT dashes[2]{ 1, 1 };
754783
THROW_IF_FAILED(p.d2dFactory->CreateStrokeStyle(&props, &dashes[0], 2, _dottedStrokeStyle.addressof()));
755784
}
756-
appendHorizontalLine(r, p.s->font->underline, _dottedStrokeStyle.get(), r.underlineColor);
785+
appendHorizontalLine(r, p.s->font->underline, _dottedStrokeStyle.get(), p.underlineBitmap);
757786
}
758787
else if (r.lines.test(GridLines::DashedUnderline))
759788
{
@@ -763,17 +792,17 @@ void BackendD2D::_drawGridlineRow(const RenderingPayload& p, const ShapedRow* ro
763792
static constexpr FLOAT dashes[2]{ 2, 2 };
764793
THROW_IF_FAILED(p.d2dFactory->CreateStrokeStyle(&props, &dashes[0], 2, _dashedStrokeStyle.addressof()));
765794
}
766-
appendHorizontalLine(r, p.s->font->underline, _dashedStrokeStyle.get(), r.underlineColor);
795+
appendHorizontalLine(r, p.s->font->underline, _dashedStrokeStyle.get(), p.underlineBitmap);
767796
}
768797
else if (r.lines.test(GridLines::CurlyUnderline))
769798
{
770-
appendCurlyLine(r);
799+
appendCurlyLine(r, p.underlineBitmap);
771800
}
772801
else if (r.lines.test(GridLines::DoubleUnderline))
773802
{
774803
for (const auto pos : p.s->font->doubleUnderline)
775804
{
776-
appendHorizontalLine(r, pos, nullptr, r.underlineColor);
805+
appendHorizontalLine(r, pos, nullptr, p.underlineBitmap);
777806
}
778807
}
779808
}

0 commit comments

Comments
 (0)