Compare commits

...

2 Commits

Author SHA1 Message Date
Dustin Howett
b746661809 HAX: conpty flags 2019-11-30 10:50:03 -08:00
Dustin Howett
50dcfe1919 Replace CodepointWidthDetector's runtime table with a static one
This commit request replaces CodepointWidthDetector's
dynamically-generated map with a static constexpr one that's compiled
into the binary.

It also almost totally removes the notion of an `Invalid` width. We
definitely had gaps in our character coverage where we'd report a
character as invalid, but we'd then flatten that down to `Narrow` when
asked. By combining the not-present state and the narrow state, we get
to save a significant chunk of data.

I've tested this by feeding it all 0x10FFFF codepoints (and then some)
and making sure they 100% match the old code's outputs.

|------------------------------|---------------|----------------|
| Metric                       | Then          | Now            |
|------------------------------|---------------|----------------|
| disk space                   | 56k (`.text`) | 3k (`.rdata`)  |
| runtime memory (allocations) | 1088          | 0              |
| runtime memory (bytes)       | 51k           | ~0             |
| memory behavior              | not shared    | fully shared   |
| lookup time                  | ~31ns         | ~9ns           |
| first hit penalty            | ~170000ns     | 0ns            |
| lines of code                | 1088          | 285            |
| clarity                      | extreme       | slightly worse |
|------------------------------|---------------|----------------|

I also took a moment and cleaned up a stray boolean that we didn't need.
2019-08-08 15:51:05 -07:00
15 changed files with 390 additions and 1240 deletions

View File

@@ -20,6 +20,7 @@ const std::wstring_view ConsoleArguments::HEIGHT_ARG = L"--height";
const std::wstring_view ConsoleArguments::INHERIT_CURSOR_ARG = L"--inheritcursor";
const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature";
const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty";
const std::wstring_view ConsoleArguments::NARROW_CHARS_ARG = L"--narrow";
std::wstring EscapeArgument(std::wstring_view ac)
{
@@ -106,6 +107,7 @@ ConsoleArguments::ConsoleArguments(const std::wstring& commandline,
{
_clientCommandline = L"";
_vtMode = L"";
_vtNarrowChars = false;
_headless = false;
_createServerHandle = true;
_serverHandle = 0;
@@ -130,6 +132,7 @@ ConsoleArguments& ConsoleArguments::operator=(const ConsoleArguments& other)
_vtInHandle = other._vtInHandle;
_vtOutHandle = other._vtOutHandle;
_vtMode = other._vtMode;
_vtNarrowChars = other._vtNarrowChars;
_headless = other._headless;
_createServerHandle = other._createServerHandle;
_serverHandle = other._serverHandle;
@@ -475,7 +478,13 @@ void ConsoleArguments::s_ConsumeArg(_Inout_ std::vector<std::wstring>& args, _In
}
else if (arg == INHERIT_CURSOR_ARG)
{
_inheritCursor = true;
WI_SetFlag(_vtOptions, VtOption::InheritCursor);
s_ConsumeArg(args, i);
hr = S_OK;
}
else if (arg == NARROW_CHARS_ARG)
{
WI_SetFlag(_vtOptions, VtOption::AmbiguousCharactersNarrow);
s_ConsumeArg(args, i);
hr = S_OK;
}
@@ -592,6 +601,11 @@ std::wstring ConsoleArguments::GetVtMode() const
return _vtMode;
}
bool ConsoleArguments::GetVtNarrowChars() const
{
return _vtNarrowChars;
}
bool ConsoleArguments::GetForceV1() const
{
return _forceV1;

View File

@@ -45,11 +45,11 @@ public:
std::wstring GetClientCommandline() const;
std::wstring GetVtMode() const;
bool GetVtNarrowChars() const;
bool GetForceV1() const;
short GetWidth() const;
short GetHeight() const;
bool GetInheritCursor() const;
void SetExpectedSize(COORD dimensions) noexcept;
@@ -66,6 +66,7 @@ public:
static const std::wstring_view INHERIT_CURSOR_ARG;
static const std::wstring_view FEATURE_ARG;
static const std::wstring_view FEATURE_PTY_ARG;
static const std::wstring_view NARROW_CHARS_ARG;
private:
#ifdef UNIT_TESTING
@@ -82,12 +83,13 @@ private:
const bool createServerHandle,
const DWORD serverHandle,
const DWORD signalHandle,
const bool inheritCursor) :
const VtOption vtOptions) :
_commandline(commandline),
_clientCommandline(clientCommandline),
_vtInHandle(vtInHandle),
_vtOutHandle(vtOutHandle),
_vtMode(vtMode),
_vtOptions(vtOptions),
_width(width),
_height(height),
_forceV1(forceV1),
@@ -95,7 +97,6 @@ private:
_createServerHandle(createServerHandle),
_serverHandle(serverHandle),
_signalHandle(signalHandle),
_inheritCursor(inheritCursor),
_recievedEarlySizeChange{ false },
_originalWidth{ -1 },
_originalHeight{ -1 }
@@ -111,7 +112,9 @@ private:
HANDLE _vtOutHandle;
// Settings that dictate how ConPTY behaves
std::wstring _vtMode;
VtOption _vtOptions;
bool _forceV1;
bool _headless;
@@ -122,7 +125,6 @@ private:
bool _createServerHandle;
DWORD _serverHandle;
DWORD _signalHandle;
bool _inheritCursor;
bool _recievedEarlySizeChange;
short _originalWidth;
@@ -174,7 +176,7 @@ namespace WEX
L"Server Handle: '0x%x'\r\n"
L"Use Signal Handle: '%ws'\r\n"
L"Signal Handle: '0x%x'\r\n",
L"Inherit Cursor: '%ws'\r\n",
L"VT Options: '0x%x'\r\n",
ci.GetClientCommandline().c_str(),
s_ToBoolString(ci.HasVtHandles()),
ci.GetVtInHandle(),
@@ -188,7 +190,7 @@ namespace WEX
ci.GetServerHandle(),
s_ToBoolString(ci.HasSignalHandle()),
ci.GetSignalHandle(),
s_ToBoolString(ci.GetInheritCursor()));
ci.GetVtOptions());
}
private:
@@ -217,7 +219,7 @@ namespace WEX
expected.GetServerHandle() == actual.GetServerHandle() &&
expected.HasSignalHandle() == actual.HasSignalHandle() &&
expected.GetSignalHandle() == actual.GetSignalHandle() &&
expected.GetInheritCursor() == actual.GetInheritCursor();
expected.GetVtOptions() == actual.GetVtOptions();
}
static bool AreSame(const ConsoleArguments& expected, const ConsoleArguments& actual)
@@ -242,7 +244,7 @@ namespace WEX
!object.ShouldCreateServerHandle() &&
object.GetServerHandle() == 0 &&
(object.GetSignalHandle() == 0 || object.GetSignalHandle() == INVALID_HANDLE_VALUE) &&
!object.GetInheritCursor();
object.GetVtOptions() == VtOption::None;
}
};
}

View File

@@ -72,12 +72,13 @@ VtIo::VtIo() :
[[nodiscard]] HRESULT VtIo::Initialize(const ConsoleArguments* const pArgs)
{
_lookingForCursorPosition = pArgs->GetInheritCursor();
auto vtOptions = pArgs->GetVtOptions();
_lookingForCursorPosition = WI_IsFlagSet(vtOptions, VtOption::InheritCursor);
// If we were already given VT handles, set up the VT IO engine to use those.
if (pArgs->InConptyMode())
{
return _Initialize(pArgs->GetVtInHandle(), pArgs->GetVtOutHandle(), pArgs->GetVtMode(), pArgs->GetSignalHandle());
return _Initialize(pArgs->GetVtInHandle(), pArgs->GetVtOutHandle(), pArgs->GetVtMode(), vtOptions, pArgs->GetSignalHandle());
}
// Didn't need to initialize if we didn't have VT stuff. It's still OK, but report we did nothing.
else
@@ -107,6 +108,7 @@ VtIo::VtIo() :
[[nodiscard]] HRESULT VtIo::_Initialize(const HANDLE InHandle,
const HANDLE OutHandle,
const std::wstring& VtMode,
const VtOption Options,
_In_opt_ const HANDLE SignalHandle)
{
FAIL_FAST_IF_MSG(_initialized, "Someone attempted to double-_Initialize VtIo");
@@ -116,6 +118,9 @@ VtIo::VtIo() :
_hInput.reset(InHandle);
_hOutput.reset(OutHandle);
_hSignal.reset(SignalHandle);
_options = Options;
WI_SetFlagIf(_options, VtOption::ForceAsciiOnly, _IoMode == VtIoMode::XTERM_ASCII);
// The only way we're initialized is if the args said we're in conpty mode.
// If the args say so, then at least one of in, out, or signal was specified
@@ -161,30 +166,25 @@ VtIo::VtIo() :
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()));
static_cast<WORD>(gci.GetColorTableSize()),
_options);
break;
case VtIoMode::XTERM:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()),
false);
break;
case VtIoMode::XTERM_ASCII:
_pVtRenderEngine = std::make_unique<XtermEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()),
true);
_options);
break;
case VtIoMode::WIN_TELNET:
_pVtRenderEngine = std::make_unique<WinTelnetEngine>(std::move(_hOutput),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()));
static_cast<WORD>(gci.GetColorTableSize()),
_options);
break;
default:
return E_FAIL;

View File

@@ -13,6 +13,15 @@ class ConsoleArguments;
namespace Microsoft::Console::VirtualTerminal
{
enum class VtOption
{
None = 0x0,
InheritCursor = 0x1,
ForceAsciiOnly = 0x2,
AmbiguousCharactersNarrow = 0x4,
};
DEFINE_ENUM_FLAG_OPERATORS(VtOption);
class VtIo : public Microsoft::Console::ITerminalOwner
{
public:
@@ -46,6 +55,7 @@ namespace Microsoft::Console::VirtualTerminal
// After CreateAndStartSignalThread is called, this will be invalid.
wil::unique_hfile _hSignal;
VtIoMode _IoMode;
VtOption _options;
bool _initialized;
bool _objectsCreated;
@@ -57,7 +67,7 @@ namespace Microsoft::Console::VirtualTerminal
std::unique_ptr<Microsoft::Console::VtInputThread> _pVtInputThread;
std::unique_ptr<Microsoft::Console::PtySignalInputThread> _pPtySignalInputThread;
[[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, const std::wstring& VtMode, _In_opt_ const HANDLE SignalHandle);
[[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, const std::wstring& VtMode, const VtOption Options, _In_opt_ const HANDLE SignalHandle);
void _ShutdownIfNeeded();

View File

@@ -32,32 +32,12 @@ class CodepointWidthDetectorTests
{
TEST_CLASS(CodepointWidthDetectorTests);
TEST_METHOD(CodepointWidthDetectDefersMapPopulation)
{
CodepointWidthDetector widthDetector;
VERIFY_IS_TRUE(widthDetector._map.empty());
widthDetector.IsWide(UNICODE_SPACE);
VERIFY_IS_TRUE(widthDetector._map.empty());
// now force checking
widthDetector.GetWidth(emoji);
VERIFY_IS_FALSE(widthDetector._map.empty());
}
TEST_METHOD(CanLookUpEmoji)
{
CodepointWidthDetector widthDetector;
VERIFY_IS_TRUE(widthDetector.IsWide(emoji));
}
TEST_METHOD(TestUnicodeRangeCompare)
{
CodepointWidthDetector::UnicodeRangeCompare compare;
// test comparing 2 search terms
CodepointWidthDetector::UnicodeRange a{ 0x10 };
CodepointWidthDetector::UnicodeRange b{ 0x15 };
VERIFY_IS_TRUE(static_cast<bool>(compare(a, b)));
}
TEST_METHOD(CanExtractCodepoint)
{
CodepointWidthDetector widthDetector;

View File

@@ -13,8 +13,9 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe,
const IDefaultColorProvider& colorProvider,
const Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable) :
VtEngine(std::move(hPipe), colorProvider, initialViewport),
const WORD cColorTable,
const VtOption vtOptions) :
VtEngine(std::move(hPipe), colorProvider, initialViewport, vtOptions),
_ColorTable(ColorTable),
_cColorTable(cColorTable)
{

View File

@@ -27,7 +27,8 @@ namespace Microsoft::Console::Render
const Microsoft::Console::IDefaultColorProvider& colorProvider,
const Microsoft::Console::Types::Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable);
const WORD cColorTable
const VtOption vtOptions);
virtual ~WinTelnetEngine() override = default;
[[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground,

View File

@@ -12,8 +12,9 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
const IDefaultColorProvider& colorProvider,
const Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable) :
XtermEngine(std::move(hPipe), colorProvider, initialViewport, ColorTable, cColorTable, false)
const WORD cColorTable,
const VtOption vtOptions) :
XtermEngine(std::move(hPipe), colorProvider, initialViewport, ColorTable, cColorTable, vtOptions)
{
}

View File

@@ -27,7 +27,8 @@ namespace Microsoft::Console::Render
const Microsoft::Console::IDefaultColorProvider& colorProvider,
const Microsoft::Console::Types::Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable);
const WORD cColorTable,
const VtOption vtOptions);
virtual ~Xterm256Engine() override = default;

View File

@@ -14,11 +14,11 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
const Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable,
const bool fUseAsciiOnly) :
VtEngine(std::move(hPipe), colorProvider, initialViewport),
const VtOption vtOptions) :
VtEngine(std::move(hPipe), colorProvider, initialViewport, vtOptions),
_ColorTable(ColorTable),
_cColorTable(cColorTable),
_fUseAsciiOnly(fUseAsciiOnly),
_fUseAsciiOnly(WI_IsFlagSet(vtOptions, VtOption::ForceAsciiOnly)),
_previousLineWrapped(false),
_usingUnderLine(false),
_needToDisableCursor(false)

View File

@@ -32,7 +32,7 @@ namespace Microsoft::Console::Render
const Microsoft::Console::Types::Viewport initialViewport,
_In_reads_(cColorTable) const COLORREF* const ColorTable,
const WORD cColorTable,
const bool fUseAsciiOnly);
const VtOption vtOptions);
virtual ~XtermEngine() override = default;

View File

@@ -38,6 +38,13 @@ SMALL_RECT VtEngine::GetDirtyRectInChars()
[[nodiscard]] HRESULT VtEngine::IsGlyphWideByFont(const std::wstring_view /*glyph*/, _Out_ bool* const pResult) noexcept
{
*pResult = false;
// If we've been told to report ambiguous-width characters as narrow, the <false> we stored in
// pResult is valid.
if (WI_IsFlagSet(_vtOptions, VtOption::AmbiguousCharactersNarrow))
{
return S_OK;
}
return S_FALSE;
}

View File

@@ -35,7 +35,8 @@ namespace Microsoft::Console::Render
VtEngine(_In_ wil::unique_hfile hPipe,
const Microsoft::Console::IDefaultColorProvider& colorProvider,
const Microsoft::Console::Types::Viewport initialViewport);
const Microsoft::Console::Types::Viewport initialViewport,
const VtOption vtOptions);
virtual ~VtEngine() override = default;
@@ -129,6 +130,8 @@ namespace Microsoft::Console::Render
bool _newBottomLine;
COORD _deferredCursorPos;
VtOption _vtOptions;
bool _pipeBroken;
HRESULT _exitResult;
Microsoft::Console::ITerminalOwner* _terminalOwner;

File diff suppressed because it is too large Load Diff

View File

@@ -22,79 +22,6 @@ static_assert(sizeof(unsigned int) == sizeof(wchar_t) * 2,
// use to measure the width of a codepoint
class CodepointWidthDetector final
{
protected:
// used to store range data in CodepointWidthDetector's internal map
class UnicodeRange final
{
public:
UnicodeRange(const unsigned int lowerBound,
const unsigned int upperBound) :
_lowerBound{ lowerBound },
_upperBound{ upperBound },
_isBounds{ true }
{
}
UnicodeRange(const unsigned int searchTerm) :
_lowerBound{ searchTerm },
_upperBound{ searchTerm },
_isBounds{ false }
{
}
bool IsBounds() const noexcept
{
return _isBounds;
}
unsigned int LowerBound() const
{
FAIL_FAST_IF(!_isBounds);
return _lowerBound;
}
unsigned int UpperBound() const
{
FAIL_FAST_IF(!_isBounds);
return _upperBound;
}
unsigned int SearchTerm() const
{
FAIL_FAST_IF(_isBounds);
return _lowerBound;
}
private:
unsigned int _lowerBound;
unsigned int _upperBound;
bool _isBounds;
};
// used for comparing if we've found the range that a searching UnicodeRange falls into
struct UnicodeRangeCompare final
{
bool operator()(const UnicodeRange& a, const UnicodeRange& b) const
{
if (!a.IsBounds() && b.IsBounds())
{
return a.SearchTerm() < b.LowerBound();
}
else if (a.IsBounds() && !b.IsBounds())
{
return a.UpperBound() < b.SearchTerm();
}
else if (a.IsBounds() && b.IsBounds())
{
return a.LowerBound() < b.LowerBound();
}
else
{
return a.SearchTerm() < b.SearchTerm();
}
}
};
public:
CodepointWidthDetector() = default;
CodepointWidthDetector(const CodepointWidthDetector&) = delete;
@@ -115,11 +42,8 @@ public:
private:
bool _lookupIsWide(const std::wstring_view glyph) const noexcept;
bool _checkFallbackViaCache(const std::wstring_view glyph) const;
unsigned int _extractCodepoint(const std::wstring_view glyph) const noexcept;
void _populateUnicodeSearchMap();
static unsigned int _extractCodepoint(const std::wstring_view glyph) noexcept;
mutable std::map<std::wstring, bool> _fallbackCache;
std::map<UnicodeRange, CodepointWidth, UnicodeRangeCompare> _map;
std::function<bool(std::wstring_view)> _pfnFallbackMethod;
bool _hasFallback = false;
};