Force LTR / logical order for text in GdiEngine (#12722)

Some applications like `vim -H` implement their own BiDi reordering.
Previously we used `PolyTextOutW` which supported such arrangements,
but with a0527a1 and the switch to `ExtTextOutW` we broke such applications.
This commit restores the old behavior by reimplementing the basics
of `ExtTextOutW`'s internal workings while enforcing LTR ordering.

## Validation Steps Performed
* Create a text file with "ץחסק פחופפסנ חס קוח ז׳חסש ץקקטק פחטסץ"
  Viewing the text file with `vim -H` presents the contents as expected 
* Printing enwik8 is as fast as before 
* Font fallback for various eastern scripts in enwik8 works as expected 
* `DECDWL` double-width sequences 
* Horizontal scrolling (apart from producing expected artifacts) 

Closes #12294
This commit is contained in:
Leonard Hecker 2022-03-22 00:32:45 +01:00 committed by GitHub
parent 855e1360c0
commit d97d9f0fcf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 70 additions and 22 deletions

View File

@ -237,9 +237,9 @@ charlespetzold
charset
CHARSETINFO
chcp
Checkin
checkbox
checkboxes
Checkin
chh
Childitem
chk
@ -917,6 +917,7 @@ getwriter
GFEh
Gfun
gfx
GGI
GHIJK
GHIJKL
GHIJKLM
@ -1094,6 +1095,7 @@ ifndef
IFont
ifstream
IGNOREEND
IGNORELANGUAGE
IHigh
IHosted
iid
@ -1463,9 +1465,9 @@ mscorlib
msctf
msctls
msdata
MSDL
msdn
msft
MSDL
MSGCMDLINEF
MSGF
MSGFILTER
@ -2239,6 +2241,7 @@ srect
srv
srvinit
srvpipe
ssa
ssh
sstream
stackoverflow
@ -2565,6 +2568,7 @@ USESHOWWINDOW
USESIZE
USESTDHANDLES
ushort
usp
USRDLL
utf
utils

View File

@ -91,6 +91,8 @@ namespace Microsoft::Console::Render
const int nIndex,
const LONG dwNewLong) noexcept;
static bool FontHasWesternScript(HDC hdc);
bool _fPaintStarted;
til::rect _invalidCharacters;
@ -138,13 +140,15 @@ namespace Microsoft::Console::Render
COLORREF _lastFg;
COLORREF _lastBg;
enum class FontType : size_t
enum class FontType : uint8_t
{
Undefined,
Default,
Italic,
Soft
};
FontType _lastFontType;
bool _fontHasWesternScript = false;
XFORM _currentLineTransform;
LineRendition _currentLineRendition;

View File

@ -6,7 +6,7 @@
<RootNamespace>gdi</RootNamespace>
<ProjectName>RendererGdi</ProjectName>
<TargetName>ConRenderGdi</TargetName>
<ConfigurationType>StaticLibrary</ConfigurationType>
<ConfigurationType>StaticLibrary</ConfigurationType>
</PropertyGroup>
<Import Project="$(SolutionDir)src\common.build.pre.props" />
<ItemGroup>
@ -22,6 +22,11 @@
<ClInclude Include="..\gdirenderer.hpp" />
<ClInclude Include="..\precomp.h" />
</ItemGroup>
<ItemDefinitionGroup>
<Lib>
<AdditionalDependencies>usp10.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Lib>
</ItemDefinitionGroup>
<!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->
<Import Project="$(SolutionDir)src\common.build.post.props" />
</Project>

View File

@ -11,6 +11,15 @@
using namespace Microsoft::Console::Render;
// This is an excerpt of GDI's FontHasWesternScript() as
// used by InternalTextOut() which is part of ExtTextOutW().
bool GdiEngine::FontHasWesternScript(HDC hdc)
{
WORD glyphs[4];
return (GetGlyphIndicesW(hdc, L"dMr\"", 4, glyphs, GGI_MARK_NONEXISTING_GLYPHS) == 4) &&
(glyphs[0] != 0xFFFF && glyphs[1] != 0xFFFF && glyphs[2] != 0xFFFF && glyphs[3] != 0xFFFF);
}
// Routine Description:
// - Prepares internal structures for a painting operation.
// Arguments:
@ -54,6 +63,8 @@ using namespace Microsoft::Console::Render;
_debugContext = GetDC(_debugWindow);
#endif
_lastFontType = FontType::Undefined;
return S_OK;
}
@ -445,10 +456,42 @@ using namespace Microsoft::Console::Render;
for (size_t i = 0; i != _cPolyText; ++i)
{
const auto& t = _pPolyText[i];
if (!ExtTextOutW(_hdcMemoryContext, t.x, t.y, t.uiFlags, &t.rcl, t.lpstr, t.n, t.pdx))
// The following if/else replicates the essentials of how ExtTextOutW() without ETO_IGNORELANGUAGE works.
// See InternalTextOut().
//
// Unlike the original, we don't check for `GetTextCharacterExtra(hdc) != 0`,
// because we don't ever call SetTextCharacterExtra() anyways.
//
// GH#12294:
// Additionally we set ss.fOverrideDirection to TRUE, because we need to present RTL
// text in logical order in order to be compatible with applications like `vim -H`.
if (_fontHasWesternScript && ScriptIsComplex(t.lpstr, t.n, SIC_COMPLEX) == S_FALSE)
{
hr = E_FAIL;
break;
if (!ExtTextOutW(_hdcMemoryContext, t.x, t.y, t.uiFlags | ETO_IGNORELANGUAGE, &t.rcl, t.lpstr, t.n, t.pdx))
{
hr = E_FAIL;
break;
}
}
else
{
SCRIPT_STATE ss{};
ss.fOverrideDirection = TRUE;
SCRIPT_STRING_ANALYSIS ssa;
hr = ScriptStringAnalyse(_hdcMemoryContext, t.lpstr, t.n, 0, -1, SSA_GLYPHS | SSA_FALLBACK, 0, nullptr, &ss, t.pdx, nullptr, nullptr, &ssa);
if (FAILED(hr))
{
break;
}
hr = ScriptStringOut(ssa, t.x, t.y, t.uiFlags, &t.rcl, 0, 0, FALSE);
std::ignore = ScriptStringFree(&ssa);
if (FAILED(hr))
{
break;
}
}
}

View File

@ -16,8 +16,9 @@ Abstract:
// This includes support libraries from the CRT, STL, WIL, and GSL
#include "LibraryIncludes.h"
#include <windows.h>
#include <Windows.h>
#include <windowsx.h>
#include <usp10.h>
#ifndef _NTSTATUS_DEFINED
#define _NTSTATUS_DEFINED

View File

@ -29,7 +29,7 @@ GdiEngine::GdiEngine() :
_fInvalidRectUsed(false),
_lastFg(INVALID_COLOR),
_lastBg(INVALID_COLOR),
_lastFontType(FontType::Default),
_lastFontType(FontType::Undefined),
_currentLineTransform(IDENTITY_XFORM),
_currentLineRendition(LineRendition::SingleWidth),
_fPaintStarted(false),
@ -142,15 +142,6 @@ GdiEngine::~GdiEngine()
_hwndTargetWindow = hwnd;
_hdcMemoryContext = hdcNewMemoryContext;
// If we have a font, apply it to the context.
if (nullptr != _hfont)
{
LOG_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, _hfont));
}
// Record the fact that the selected font is the default.
_lastFontType = FontType::Default;
if (nullptr != hdcRealWindow)
{
LOG_HR_IF(E_FAIL, !(ReleaseDC(_hwndTargetWindow, hdcRealWindow)));
@ -327,6 +318,7 @@ GdiEngine::~GdiEngine()
break;
}
_lastFontType = fontType;
_fontHasWesternScript = FontHasWesternScript(_hdcMemoryContext);
}
return S_OK;
@ -348,9 +340,6 @@ GdiEngine::~GdiEngine()
// Select into DC
RETURN_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, hFont.get()));
// Record the fact that the selected font is the default.
_lastFontType = FontType::Default;
// Save off the font metrics for various other calculations
RETURN_HR_IF(E_FAIL, !(GetTextMetricsW(_hdcMemoryContext, &_tmFontMetrics)));
@ -456,7 +445,9 @@ GdiEngine::~GdiEngine()
const SIZE cellSize,
const size_t centeringHint) noexcept
{
// If the soft font is currently selected, replace it with the default font.
// If we previously called SelectFont(_hdcMemoryContext, _softFont), it will
// still hold a reference to the _softFont object we're planning to overwrite.
// --> First revert back to the standard _hfont, lest we have dangling pointers.
if (_lastFontType == FontType::Soft)
{
RETURN_HR_IF_NULL(E_FAIL, SelectFont(_hdcMemoryContext, _hfont));