Skip to content

Commit ff22ca5

Browse files
Qmoonyclaude
andcommitted
AtlasEngine: Enable RTL text shaping via bidi analysis (#20156)
Previously, IDWriteTextAnalyzer::GetGlyphs and GetGlyphPlacements were called with isRightToLeft=0 for all text, which prevented DirectWrite from performing Arabic contextual shaping (cursive joining) and produced glyphs in LTR order for RTL text. This change: - Calls AnalyzeBidi() to determine text direction per run - Passes the correct isRightToLeft value to GetGlyphs/GetGlyphPlacements - Reverses the resulting RTL glyph ordering to LTR for the backends - Builds ShapedRow directly for RTL runs, bypassing the column loop which assumes ascending clusterMap values Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent c334f91 commit ff22ca5

5 files changed

Lines changed: 91 additions & 7 deletions

File tree

src/renderer/atlas/AtlasEngine.cpp

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,13 +1032,34 @@ void AtlasEngine::_mapCharacters(const wchar_t* text, const u32 textLength, u32*
10321032
void AtlasEngine::_mapComplex(IDWriteFontFace2* mappedFontFace, u32 idx, u32 length, ShapedRow& row)
10331033
{
10341034
_api.analysisResults.clear();
1035+
_api.bidiResults.clear();
10351036

10361037
TextAnalysisSource analysisSource{ _p.userLocaleName.c_str(), _api.bufferLine.data(), gsl::narrow<UINT32>(_api.bufferLine.size()) };
1037-
TextAnalysisSink analysisSink{ _api.analysisResults };
1038+
TextAnalysisSink analysisSink{ _api.analysisResults, _api.bidiResults };
10381039
THROW_IF_FAILED(_p.textAnalyzer->AnalyzeScript(&analysisSource, idx, length, &analysisSink));
1040+
THROW_IF_FAILED(_p.textAnalyzer->AnalyzeBidi(&analysisSource, idx, length, &analysisSink));
10391041

10401042
for (const auto& a : _api.analysisResults)
10411043
{
1044+
// Determine if this script run is RTL by intersecting with bidi runs.
1045+
// If all overlapping bidi runs agree on direction → use that.
1046+
// If mixed (RTL+LTR within the same script run) → fall back to LTR.
1047+
int direction = -1;
1048+
for (const auto& b : _api.bidiResults)
1049+
{
1050+
if (b.textPosition + b.textLength <= a.textPosition) continue;
1051+
if (b.textPosition >= a.textPosition + a.textLength) continue;
1052+
const int bdir = (b.resolvedLevel % 2 != 0) ? 1 : 0;
1053+
if (direction == -1)
1054+
direction = bdir;
1055+
else if (direction != bdir)
1056+
{
1057+
direction = 0;
1058+
break;
1059+
}
1060+
}
1061+
const BOOL isRTL = (direction == 1);
1062+
10421063
u32 actualGlyphCount = 0;
10431064

10441065
#pragma warning(push)
@@ -1076,7 +1097,7 @@ void AtlasEngine::_mapComplex(IDWriteFontFace2* mappedFontFace, u32 idx, u32 len
10761097
/* textLength */ a.textLength,
10771098
/* fontFace */ mappedFontFace,
10781099
/* isSideways */ false,
1079-
/* isRightToLeft */ 0,
1100+
/* isRightToLeft */ isRTL,
10801101
/* scriptAnalysis */ &a.analysis,
10811102
/* localeName */ _p.userLocaleName.c_str(),
10821103
/* numberSubstitution */ nullptr,
@@ -1128,7 +1149,7 @@ void AtlasEngine::_mapComplex(IDWriteFontFace2* mappedFontFace, u32 idx, u32 len
11281149
/* fontFace */ mappedFontFace,
11291150
/* fontEmSize */ _p.s->font->fontSize,
11301151
/* isSideways */ false,
1131-
/* isRightToLeft */ 0,
1152+
/* isRightToLeft */ isRTL,
11321153
/* scriptAnalysis */ &a.analysis,
11331154
/* localeName */ _p.userLocaleName.c_str(),
11341155
/* features */ &features,
@@ -1139,6 +1160,56 @@ void AtlasEngine::_mapComplex(IDWriteFontFace2* mappedFontFace, u32 idx, u32 len
11391160

11401161
_api.clusterMap[a.textLength] = gsl::narrow_cast<u16>(actualGlyphCount);
11411162

1163+
// For RTL runs, DirectWrite returns glyphs in right-to-left visual order
1164+
// with advances going right-to-left. Reverse to LTR order so backends draw
1165+
// glyphs correctly without per-direction logic.
1166+
if (isRTL)
1167+
{
1168+
const auto N = actualGlyphCount;
1169+
1170+
std::reverse(_api.glyphIndices.data(), _api.glyphIndices.data() + N);
1171+
if (N > 1)
1172+
{
1173+
std::reverse(_api.glyphAdvances.data(), _api.glyphAdvances.data() + N - 1);
1174+
}
1175+
std::reverse(_api.glyphOffsets.data(), _api.glyphOffsets.data() + N);
1176+
for (size_t i = 0; i < N; ++i)
1177+
{
1178+
_api.glyphOffsets[i].advanceOffset = -_api.glyphOffsets[i].advanceOffset;
1179+
}
1180+
1181+
// Build row output directly. The normal column loop below assumes ascending
1182+
// clusterMap values and LTR column ordering, both of which fail for RTL.
1183+
const auto shift = gsl::narrow_cast<u8>(row.lineRendition != LineRendition::SingleWidth);
1184+
const auto rowColors = _p.foregroundBitmap.begin() + _p.colorBitmapRowStride * _api.lastPaintBufferLineCoord.y;
1185+
1186+
// Compute total expected advance for the RTL run and adjust last glyph.
1187+
const auto colStart = _api.bufferLineColumn[a.textPosition];
1188+
const auto colEnd = _api.bufferLineColumn[a.textPosition + a.textLength];
1189+
const auto expectedTotal = static_cast<f32>((colEnd - colStart) * _p.s->font->cellSize.x);
1190+
f32 actualTotal = 0;
1191+
for (size_t i = 0; i < N; ++i)
1192+
actualTotal += _api.glyphAdvances[i];
1193+
if (N > 0)
1194+
_api.glyphAdvances[N - 1] += expectedTotal - actualTotal;
1195+
1196+
// After reversal, LTR glyph i corresponds to text position (textLength - 1 - i)
1197+
// in the RTL range, due to 1:1 clusterMap mapping before reversal.
1198+
for (size_t i = 0; i < N; ++i)
1199+
{
1200+
const auto textIdx = a.textPosition + a.textLength - 1 - i;
1201+
const auto col = _api.bufferLineColumn[textIdx];
1202+
row.colors.emplace_back(rowColors[static_cast<size_t>(col) << shift]);
1203+
}
1204+
1205+
row.glyphIndices.insert(row.glyphIndices.end(), _api.glyphIndices.begin(), _api.glyphIndices.begin() + N);
1206+
row.glyphAdvances.insert(row.glyphAdvances.end(), _api.glyphAdvances.begin(), _api.glyphAdvances.begin() + N);
1207+
row.glyphOffsets.insert(row.glyphOffsets.end(), _api.glyphOffsets.begin(), _api.glyphOffsets.begin() + N);
1208+
1209+
// RTL run fully handled. Skip the normal column loop below.
1210+
continue;
1211+
}
1212+
11421213
const auto shift = gsl::narrow_cast<u8>(row.lineRendition != LineRendition::SingleWidth);
11431214
const auto colors = _p.foregroundBitmap.begin() + _p.colorBitmapRowStride * _api.lastPaintBufferLineCoord.y;
11441215
auto prevCluster = _api.clusterMap[0];

src/renderer/atlas/AtlasEngine.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ namespace Microsoft::Console::Render::Atlas
144144

145145
std::array<Buffer<DWRITE_FONT_AXIS_VALUE>, 4> textFormatAxes;
146146
std::vector<TextAnalysisSinkResult> analysisResults;
147+
std::vector<BidiRun> bidiResults;
147148
Buffer<u16> clusterMap;
148149
Buffer<DWRITE_SHAPING_TEXT_PROPERTIES> textProps;
149150
Buffer<u16> glyphIndices;

src/renderer/atlas/DWriteTextAnalysis.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ HRESULT TextAnalysisSource::GetNumberSubstitution(UINT32 textPosition, UINT32* t
104104
return E_NOTIMPL;
105105
}
106106

107-
TextAnalysisSink::TextAnalysisSink(std::vector<TextAnalysisSinkResult>& results) noexcept :
108-
_results{ results }
107+
TextAnalysisSink::TextAnalysisSink(std::vector<TextAnalysisSinkResult>& results, std::vector<BidiRun>& bidiResults) noexcept :
108+
_results{ results },
109+
_bidiResults{ bidiResults }
109110
{
110111
}
111112

@@ -167,9 +168,12 @@ HRESULT TextAnalysisSink::SetLineBreakpoints(UINT32 textPosition, UINT32 textLen
167168
}
168169

169170
HRESULT TextAnalysisSink::SetBidiLevel(UINT32 textPosition, UINT32 textLength, UINT8 explicitLevel, UINT8 resolvedLevel) noexcept
171+
try
170172
{
171-
return E_NOTIMPL;
173+
_bidiResults.emplace_back(textPosition, textLength, resolvedLevel);
174+
return S_OK;
172175
}
176+
CATCH_RETURN()
173177

174178
HRESULT TextAnalysisSink::SetNumberSubstitution(UINT32 textPosition, UINT32 textLength, IDWriteNumberSubstitution* numberSubstitution) noexcept
175179
{

src/renderer/atlas/DWriteTextAnalysis.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace Microsoft::Console::Render::Atlas
3434

3535
struct TextAnalysisSink final : IDWriteTextAnalysisSink
3636
{
37-
TextAnalysisSink(std::vector<TextAnalysisSinkResult>& results) noexcept;
37+
TextAnalysisSink(std::vector<TextAnalysisSinkResult>& results, std::vector<BidiRun>& bidiResults) noexcept;
3838
#ifndef NDEBUG
3939
~TextAnalysisSink();
4040
#endif
@@ -49,6 +49,7 @@ namespace Microsoft::Console::Render::Atlas
4949

5050
private:
5151
std::vector<TextAnalysisSinkResult>& _results;
52+
std::vector<BidiRun>& _bidiResults;
5253
#ifndef NDEBUG
5354
ULONG _refCount = 1;
5455
#endif

src/renderer/atlas/common.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,13 @@ namespace Microsoft::Console::Render::Atlas
315315
DWRITE_SCRIPT_ANALYSIS analysis;
316316
};
317317

318+
struct BidiRun
319+
{
320+
u32 textPosition;
321+
u32 textLength;
322+
u8 resolvedLevel;
323+
};
324+
318325
enum class GraphicsAPI
319326
{
320327
Automatic,

0 commit comments

Comments
 (0)