Skip to content

Commit

Permalink
AtlasEngine: Fix ligature splitting for JetBrains Mono (#15810)
Browse files Browse the repository at this point in the history
Some fonts implement ligatures by replacing a string like "&&" with
a whitespace padding glyph, followed by the actual "&&" glyph which
has a 1 column advance width. In that case the algorithm in
`_drawTextOverlapSplit` will get confused because it strictly scans
the input from left to right, searching for color changes.
The initial color is the glyph's color and so it breaks for such fonts
because then the first split will retain the last column's color.

## Validation Steps Performed
* Use JetBrains Mono
* Print ``"`e[91m`&`e[96m&`e[m"``
* Red and blue `&` appear ✅

---------

Co-authored-by: Tushar Singh <tusharvickey1999@gmail.com>
Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
(cherry picked from commit a7a4490)
Service-Card-Id: 90153416
Service-Version: 1.18
  • Loading branch information
3 people committed Sep 22, 2023
1 parent 48b42db commit 2b5b574
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 6 deletions.
31 changes: 25 additions & 6 deletions src/renderer/atlas/BackendD3D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1086,11 +1086,15 @@ void BackendD3D::_drawText(RenderingPayload& p)
// < ! - -
void BackendD3D::_drawTextOverlapSplit(const RenderingPayload& p, u16 y)
{
const auto& originalQuad = _getLastQuad();
auto& originalQuad = _getLastQuad();

// If the current row has a non-default line rendition then every glyph is scaled up by 2x horizontally.
// This requires us to make some changes: For instance, if the ligature occupies columns 3, 4 and 5 (0-indexed)
// then we need to get the foreground colors from columns 2 and 4, because columns 0,1 2,3 4,5 6,7 and so on form pairs.
// A wide glyph would be a total of 4 actual columns wide! In other words, we need to properly round our clip rects and columns.
i32 columnAdvance = 1;
i32 columnAdvanceInPx{ p.s->font->cellSize.x };
i32 cellCount{ p.s->viewportCellCount.x };
i32 columnAdvanceInPx = p.s->font->cellSize.x;
i32 cellCount = p.s->viewportCellCount.x;

if (p.rows[y]->lineRendition != LineRendition::SingleWidth)
{
Expand All @@ -1104,12 +1108,27 @@ void BackendD3D::_drawTextOverlapSplit(const RenderingPayload& p, u16 y)
originalLeft = std::max(originalLeft, 0);
originalRight = std::min(originalRight, cellCount * columnAdvanceInPx);

auto column = originalLeft / columnAdvanceInPx + 1;
if (originalLeft >= originalRight)
{
return;
}

const auto colors = &p.foregroundBitmap[p.colorBitmapRowStride * y];

// As explained in the beginning, column and clipLeft should be in multiples of columnAdvance
// and columnAdvanceInPx respectively, because that's how line renditions work.
auto column = originalLeft / columnAdvanceInPx;
auto clipLeft = column * columnAdvanceInPx;
column *= columnAdvance;

const auto colors = &p.foregroundBitmap[p.colorBitmapRowStride * y];
auto lastFg = originalQuad.color;
// Our loop below will split the ligature by processing it from left to right.
// Some fonts however implement ligatures by replacing a string like "&&" with a whitespace padding glyph,
// followed by the actual "&&" glyph which has a 1 column advance width. In that case the originalQuad
// will have the .color of the 2nd column and not of the 1st one. We need to fix that here.
auto lastFg = colors[column];
originalQuad.color = lastFg;
column += columnAdvance;
clipLeft += columnAdvanceInPx;

// We must ensure to exit the loop while `column` is less than `cellCount.x`,
// otherwise we cause a potential out of bounds access into foregroundBitmap.
Expand Down
1 change: 1 addition & 0 deletions src/tools/RenderingTests/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <array>
#include <cassert>
#include <cstdio>

// Another variant of "defer" for C++.
namespace
Expand Down

0 comments on commit 2b5b574

Please sign in to comment.