Merge remote-tracking branch 'origin/main' into dev/migrie/interactivity-projection-000

This commit is contained in:
Mike Griese 2021-05-12 12:26:46 -05:00
commit 086710aaba
33 changed files with 341 additions and 96 deletions

View file

@ -2358,6 +2358,7 @@ testmddefinition
testmode
testname
testnameprefix
testnetv
TESTNULL
testpass
testpasses

View file

@ -20,6 +20,12 @@ jobs:
parameters:
additionalBuildArguments: ${{ parameters.additionalBuildArguments }}
# It appears that the Component Governance build task that gets automatically injected stopped working
# when we renamed our main branch.
- task: ms.vss-governance-buildtask.governance-build-task-component-detection.ComponentGovernanceComponentDetection@0
displayName: 'Component Detection'
condition: and(succeededOrFailed(), not(eq(variables['Build.Reason'], 'PullRequest')))
- template: helix-runtests-job.yml
parameters:
name: 'RunTestsInHelix'

View file

@ -62,7 +62,8 @@ namespace winrt::TerminalApp::implementation
// - <none>
void SettingsTab::_MakeTabViewItem()
{
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
TabBase::_MakeTabViewItem();
Title(RS_(L"SettingsTab"));
TabViewItem().Header(winrt::box_value(Title()));
}

View file

@ -31,7 +31,7 @@ namespace winrt::TerminalApp::implementation
void Focus(winrt::Windows::UI::Xaml::FocusState focusState) override;
private:
void _MakeTabViewItem();
void _MakeTabViewItem() override;
winrt::fire_and_forget _CreateIcon();
};
}

View file

@ -46,9 +46,17 @@ namespace winrt::TerminalApp::implementation
auto weakThis{ get_weak() };
// Build the menu
Controls::MenuFlyout newTabFlyout;
_AppendCloseMenuItems(newTabFlyout);
TabViewItem().ContextFlyout(newTabFlyout);
Controls::MenuFlyout contextMenuFlyout;
// GH#5750 - When the context menu is dismissed with ESC, toss the focus
// back to our control.
contextMenuFlyout.Closed([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_RequestFocusActiveControlHandlers();
}
});
_AppendCloseMenuItems(contextMenuFlyout);
TabViewItem().ContextFlyout(contextMenuFlyout);
}
// Method Description:
@ -223,4 +231,24 @@ namespace winrt::TerminalApp::implementation
toolTip.Content(textBlock);
WUX::Controls::ToolTipService::SetToolTip(TabViewItem(), toolTip);
}
// Method Description:
// - Initializes a TabViewItem for this Tab instance.
// Arguments:
// - <none>
// Return Value:
// - <none>
void TabBase::_MakeTabViewItem()
{
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
// GH#3609 If the tab was tapped, and no one else was around to handle
// it, then ask our parent to toss focus into the active control.
TabViewItem().Tapped([weakThis{ get_weak() }](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_RequestFocusActiveControlHandlers();
}
});
}
}

View file

@ -25,6 +25,8 @@ namespace winrt::TerminalApp::implementation
void UpdateTabViewIndex(const uint32_t idx, const uint32_t numTabs);
void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap);
WINRT_CALLBACK(RequestFocusActiveControl, winrt::delegate<void()>);
WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
WINRT_CALLBACK(CloseRequested, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
@ -52,6 +54,8 @@ namespace winrt::TerminalApp::implementation
virtual void _CreateContextMenu();
virtual winrt::hstring _CreateToolTipTitle();
virtual void _MakeTabViewItem();
void _AppendCloseMenuItems(winrt::Windows::UI::Xaml::Controls::MenuFlyout flyout);
void _EnableCloseMenuItems();
void _CloseTabsAfter();

View file

@ -215,7 +215,9 @@ namespace winrt::TerminalApp::implementation
}
});
newTabImpl->TabRenamerDeactivated([weakThis{ get_weak() }](auto&& /*s*/, auto&& /*e*/) {
// The tab might want us to toss focus into the control, especially when
// transient UIs (like the context menu, or the renamer) are dismissed.
newTabImpl->RequestFocusActiveControl([weakThis{ get_weak() }]() {
if (const auto page{ weakThis.get() })
{
page->_FocusCurrentTab(false);

View file

@ -53,6 +53,15 @@ namespace winrt::TerminalApp::implementation
}
});
// GH#9162 - when the header is done renaming, ask for focus to be
// tossed back to the control, rather into ourselves.
_headerControl.RenameEnded([weakThis = get_weak()](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_RequestFocusActiveControlHandlers();
}
});
_UpdateHeaderControlMaxWidth();
// Use our header control as the TabViewItem's header
@ -83,7 +92,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalTab::_MakeTabViewItem()
{
TabViewItem(::winrt::MUX::Controls::TabViewItem{});
TabBase::_MakeTabViewItem();
TabViewItem().DoubleTapped([weakThis = get_weak()](auto&& /*s*/, auto&& /*e*/) {
if (auto tab{ weakThis.get() })
@ -878,14 +887,23 @@ namespace winrt::TerminalApp::implementation
}
// Build the menu
Controls::MenuFlyout newTabFlyout;
Controls::MenuFlyout contextMenuFlyout;
Controls::MenuFlyoutSeparator menuSeparator;
newTabFlyout.Items().Append(chooseColorMenuItem);
newTabFlyout.Items().Append(renameTabMenuItem);
newTabFlyout.Items().Append(duplicateTabMenuItem);
newTabFlyout.Items().Append(menuSeparator);
_AppendCloseMenuItems(newTabFlyout);
TabViewItem().ContextFlyout(newTabFlyout);
contextMenuFlyout.Items().Append(chooseColorMenuItem);
contextMenuFlyout.Items().Append(renameTabMenuItem);
contextMenuFlyout.Items().Append(duplicateTabMenuItem);
contextMenuFlyout.Items().Append(menuSeparator);
// GH#5750 - When the context menu is dismissed with ESC, toss the focus
// back to our control.
contextMenuFlyout.Closed([weakThis](auto&&, auto&&) {
if (auto tab{ weakThis.get() })
{
tab->_RequestFocusActiveControlHandlers();
}
});
_AppendCloseMenuItems(contextMenuFlyout);
TabViewItem().ContextFlyout(contextMenuFlyout);
}
// Method Description:

View file

@ -91,7 +91,6 @@ namespace winrt::TerminalApp::implementation
DECLARE_EVENT(ColorCleared, _colorCleared, winrt::delegate<>);
DECLARE_EVENT(TabRaiseVisualBell, _TabRaiseVisualBellHandlers, winrt::delegate<>);
DECLARE_EVENT(DuplicateRequested, _DuplicateRequestedHandlers, winrt::delegate<>);
FORWARDED_TYPED_EVENT(TabRenamerDeactivated, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, (&_headerControl), RenameEnded);
TYPED_EVENT(TaskbarProgressChanged, IInspectable, IInspectable);
private:
@ -120,7 +119,7 @@ namespace winrt::TerminalApp::implementation
std::optional<Windows::UI::Xaml::DispatcherTimer> _bellIndicatorTimer;
void _BellIndicatorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
void _MakeTabViewItem();
void _MakeTabViewItem() override;
winrt::fire_and_forget _UpdateHeaderControlMaxWidth();

View file

@ -101,27 +101,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
Close();
// Before destroying this instance we must ensure that we destroy the _renderer
// before the _renderEngine, as well as calling _renderer->TriggerTeardown().
// _renderEngine will be destroyed naturally after this ~destructor() returns.
decltype(_renderer) renderer;
if (_renderer)
{
// GH#8734:
// We lock the terminal here to make sure it isn't still being
// used in the connection thread before we destroy the renderer.
// However, we must unlock it again prior to triggering the
// teardown, to avoid the render thread being deadlocked. The
// renderer may be waiting to acquire the terminal lock, while
// we're waiting for the renderer to finish.
auto lock = _terminal->LockForWriting();
_renderer.swap(renderer);
}
if (renderer)
{
renderer->TriggerTeardown();
_renderer->TriggerTeardown();
}
}

View file

@ -176,8 +176,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
std::unique_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr };
std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr };
// NOTE: _renderEngine must be ordered before _renderer.
//
// As _renderer has a dependency on _renderEngine (through a raw pointer)
// we must ensure the _renderer is deallocated first.
// (C++ class members are destroyed in reverse order.)
std::unique_ptr<::Microsoft::Console::Render::DxEngine> _renderEngine{ nullptr };
std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr };
IControlSettings _settings{ nullptr };

View file

@ -87,12 +87,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TYPED_EVENT(ScrollPositionChanged, IInspectable, Control::ScrollPositionChangedArgs);
private:
// NOTE: _uiaEngine must be ordered before _core.
//
// ControlCore::AttachUiaEngine receives a IRenderEngine as a raw pointer, which we own.
// We must ensure that we first destroy the ControlCore before the UiaEngine instance
// in order to safely resolve this unsafe pointer dependency. Otherwise a deallocated
// IRenderEngine is accessed when ControlCore calls Renderer::TriggerTeardown.
// (C++ class members are destroyed in reverse order.)
std::unique_ptr<::Microsoft::Console::Render::UiaEngine> _uiaEngine;
winrt::com_ptr<ControlCore> _core{ nullptr };
unsigned int _rowsToScroll;
double _internalScrollbarPosition{ 0.0 };
std::unique_ptr<::Microsoft::Console::Render::UiaEngine> _uiaEngine;
// If this is set, then we assume we are in the middle of panning the
// viewport via touch input.
std::optional<til::point> _touchAnchor;

View file

@ -134,17 +134,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
Control::ControlCore _core{ nullptr };
Control::ControlInteractivity _interactivity{ nullptr };
bool _initializedTerminal;
winrt::com_ptr<SearchBoxControl> _searchBox;
IControlSettings _settings;
bool _focused;
std::atomic<bool> _closing;
bool _focused;
bool _initializedTerminal;
std::shared_ptr<ThrottledFunc<>> _tsfTryRedrawCanvas;
std::shared_ptr<ThrottledFunc<>> _updatePatternLocations;
std::shared_ptr<ThrottledFunc<>> _playWarningBell;
struct ScrollBarUpdate

View file

@ -0,0 +1,100 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
#include "precomp.h"
class MultipleInflightMessageTests
{
BEGIN_TEST_CLASS(MultipleInflightMessageTests)
END_TEST_CLASS()
// This test is intended to make sure that we do not regress after the _handlePostCharInputLoop fix in OpenConsole:c0ab9cb5b
TEST_METHOD(WriteWhileReadingInputCrash)
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") // Don't pollute other tests by isolating our potential crash and buffer resizing to this test.
END_TEST_METHOD_PROPERTIES()
using namespace std::string_view_literals;
const auto inputHandle = GetStdInputHandle();
const auto outputHandle = GetStdOutputHandle();
DWORD originalConsoleMode{};
VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleMode(inputHandle, &originalConsoleMode));
auto restoreMode{ wil::scope_exit([=]() {
SetConsoleMode(inputHandle, originalConsoleMode);
}) };
VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleMode(inputHandle, ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT));
// Prime the console with some fake input records.
const std::array<INPUT_RECORD, 4> inputRecords{
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ TRUE, 1, 'h', 0, L'h', 0 } },
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ FALSE, 1, 'h', 0, L'h', 0 } },
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ TRUE, 1, 'i', 0, L'i', 0 } },
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ FALSE, 1, 'i', 0, L'i', 0 } },
};
DWORD written{};
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleInput(inputHandle, inputRecords.data(), static_cast<DWORD>(inputRecords.size()), &written));
VERIFY_ARE_EQUAL(inputRecords.size(), written);
// !!! LOAD BEARING !!!
// This buffer needs to be large enough to force API_MSG to heap allocate (!)
std::array<wchar_t, 129> buffer;
DWORD read{};
bool readerThreadLaunched{ false };
std::condition_variable readerThreadLaunchCV;
std::mutex readerThreadLaunchMutex;
std::unique_lock lock{ readerThreadLaunchMutex };
std::thread readerThread{ [&]() {
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};
{
std::scoped_lock lock{ readerThreadLaunchMutex };
readerThreadLaunched = true;
}
readerThreadLaunchCV.notify_all();
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleW(inputHandle, buffer.data(), static_cast<DWORD>(buffer.size()), &read, nullptr));
} };
// CV wait should allow readerThread to progress
readerThreadLaunchCV.wait(lock, [&]() { return readerThreadLaunched; });
// should not progress until readerThreadLaunched is set.
Sleep(50); // Yeah, it's not science.
std::thread writerThread{ [&]() {
WEX::TestExecution::DisableVerifyExceptions disableVerifyExceptions{};
DWORD temp{};
// !!! LOAD BEARING !!!
// This buffer must be large enough to trigger a *re-allocation* in the API message handler.
std::array<wchar_t, 4096> anEvenLargerBuffer;
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(outputHandle, anEvenLargerBuffer.data(), static_cast<DWORD>(anEvenLargerBuffer.size()), COORD{ 1, 1 }, &temp)); // has payload (output buffer)
VERIFY_ARE_EQUAL(4096u, temp);
const std::array<INPUT_RECORD, 2> inputRecords{
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ TRUE, 1, '1', 0, L'!', SHIFT_PRESSED } },
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ FALSE, 1, '1', 0, L'!', SHIFT_PRESSED } },
};
DWORD written{};
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleInputW(inputHandle, inputRecords.data(), static_cast<DWORD>(inputRecords.size()), &written));
VERIFY_ARE_EQUAL(inputRecords.size(), written);
// !!! LOAD BEARING !!!
// It is (apparently) important that this come in two different writes.
const std::array<INPUT_RECORD, 2> inputRecords2{
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ TRUE, 1, VK_RETURN, 0, L'\r', 0 } },
INPUT_RECORD{ KEY_EVENT, KEY_EVENT_RECORD{ FALSE, 1, VK_RETURN, 0, L'\r', 0 } },
};
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleInputW(inputHandle, inputRecords2.data(), static_cast<DWORD>(inputRecords2.size()), &written));
VERIFY_ARE_EQUAL(inputRecords2.size(), written);
} };
writerThread.join();
readerThread.join();
VERIFY_ARE_EQUAL(L"hi!\r"sv, (std::wstring_view{ buffer.data(), read }));
}
};

View file

@ -19,6 +19,7 @@
<ClCompile Include="API_FontTests.cpp" />
<ClCompile Include="API_InputTests.cpp" />
<ClCompile Include="API_ModeTests.cpp" />
<ClCompile Include="API_MultipleInflightMessageTests.cpp" />
<ClCompile Include="API_OutputTests.cpp" />
<ClCompile Include="API_RgbColorTests.cpp" />
<ClCompile Include="API_TitleTests.cpp" />

View file

@ -84,6 +84,9 @@
<ClCompile Include="API_FillOutputTests.cpp">
<Filter>Source Files\API</Filter>
</ClCompile>
<ClCompile Include="API_MultipleInflightMessageTests.cpp">
<Filter>Source Files\API</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="Common.hpp">

View file

@ -32,6 +32,7 @@ SOURCES = \
API_FontTests.cpp \
API_InputTests.cpp \
API_ModeTests.cpp \
API_MultipleInflightMessageTests.cpp \
API_OutputTests.cpp \
API_RgbColorTests.cpp \
API_TitleTests.cpp \

View file

@ -20,7 +20,7 @@
}
],
"RemoteFiles": [ ],
"Packages": [ "Microsoft.Console.Tools.Nihilist" ]
"Packages": [ "Microsoft.Console.Tools.Nihilist", "microsoft-windows-test-testnetv5-0" ]
},
"Logs": [ ],
"Plugins": [ ],

View file

@ -36,6 +36,7 @@ public:
ReadData& operator=(const ReadData&) & = delete;
ReadData& operator=(ReadData&&) & = delete;
virtual void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) = 0;
virtual bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -1197,3 +1197,12 @@ void COOKED_READ_DATA::SavePendingInput(const size_t index, const bool multiline
}
return STATUS_SUCCESS;
}
void COOKED_READ_DATA::MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer)
{
// See the comment in WaitBlock.cpp for more information.
if (_userBuffer == reinterpret_cast<const wchar_t*>(oldBuffer))
{
_userBuffer = reinterpret_cast<wchar_t*>(newBuffer);
}
}

View file

@ -48,6 +48,8 @@ public:
bool AtEol() const noexcept;
void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) override;
bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -190,3 +190,8 @@ bool DirectReadData::Notify(const WaitTerminationReason TerminationReason,
}
return retVal;
}
void DirectReadData::MigrateUserBuffersOnTransitionToBackgroundWait(const void* /*oldBuffer*/, void* /*newBuffer*/)
{
// Direct read doesn't hold API message buffers
}

View file

@ -40,6 +40,7 @@ public:
~DirectReadData() override;
void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) override;
bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -218,3 +218,12 @@ bool RAW_READ_DATA::Notify(const WaitTerminationReason TerminationReason,
}
return RetVal;
}
void RAW_READ_DATA::MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer)
{
// See the comment in WaitBlock.cpp for more information.
if (_BufPtr == reinterpret_cast<const wchar_t*>(oldBuffer))
{
_BufPtr = reinterpret_cast<wchar_t*>(newBuffer);
}
}

View file

@ -37,6 +37,7 @@ public:
RAW_READ_DATA(RAW_READ_DATA&&) = default;
void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) override;
bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -189,3 +189,8 @@ bool WriteData::Notify(const WaitTerminationReason TerminationReason,
*pReplyStatus = Status;
return true;
}
void WriteData::MigrateUserBuffersOnTransitionToBackgroundWait(const void* /*oldBuffer*/, void* /*newBuffer*/)
{
// WriteData does not hold API message buffers across a wait
}

View file

@ -36,6 +36,7 @@ public:
void SetUtf8ConsumedCharacters(const size_t cchUtf8Consumed);
void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) override;
bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -27,7 +27,8 @@ CustomTextLayout::CustomTextLayout(gsl::not_null<DxFontRenderData*> const fontRe
_runs{},
_breakpoints{},
_runIndex{ 0 },
_width{ gsl::narrow_cast<size_t>(fontRenderData->GlyphCell().width()) }
_width{ gsl::narrow_cast<size_t>(fontRenderData->GlyphCell().width()) },
_isEntireTextSimple{ false }
{
_localeName.resize(gsl::narrow_cast<size_t>(fontRenderData->DefaultTextFormat()->GetLocaleNameLength()) + 1); // +1 for null
THROW_IF_FAILED(fontRenderData->DefaultTextFormat()->GetLocaleName(_localeName.data(), gsl::narrow<UINT32>(_localeName.size())));
@ -45,6 +46,7 @@ try
_runs.clear();
_breakpoints.clear();
_runIndex = 0;
_isEntireTextSimple = false;
_textClusterColumns.clear();
_text.clear();
_glyphScaleCorrections.clear();
@ -101,6 +103,7 @@ CATCH_RETURN()
_formatInUse = _fontRenderData->DefaultTextFormat().Get();
_fontInUse = _fontRenderData->DefaultFontFace().Get();
RETURN_IF_FAILED(_AnalyzeTextComplexity());
RETURN_IF_FAILED(_AnalyzeRuns());
RETURN_IF_FAILED(_ShapeGlyphRuns());
@ -135,6 +138,7 @@ CATCH_RETURN()
_formatInUse = drawingContext->useItalicFont ? _fontRenderData->ItalicTextFormat().Get() : _fontRenderData->DefaultTextFormat().Get();
_fontInUse = drawingContext->useItalicFont ? _fontRenderData->ItalicFontFace().Get() : _fontRenderData->DefaultFontFace().Get();
RETURN_IF_FAILED(_AnalyzeTextComplexity());
RETURN_IF_FAILED(_AnalyzeRuns());
RETURN_IF_FAILED(_ShapeGlyphRuns());
RETURN_IF_FAILED(_CorrectGlyphRuns());
@ -150,9 +154,8 @@ CATCH_RETURN()
// Routine Description:
// - Uses the internal text information and the analyzers/font information from construction
// to determine the complexity of the text. During the process we break the text into initial
// runs based on their complexity. This allows us to further optimize the layout process
// of simple runs.
// to determine the complexity of the text. If the text is determined to be entirely simple,
// we'll have more chances to optimize the layout process.
// Arguments:
// - <none> - Uses internal state
// Return Value:
@ -167,30 +170,21 @@ CATCH_RETURN()
UINT32 uiLengthRead = 0;
// Start from the beginning.
UINT32 pos = 0;
const UINT32 glyphStart = 0;
_glyphIndices.resize(textLength);
while (pos < textLength)
{
const HRESULT hr = _fontRenderData->Analyzer()->GetTextComplexity(
&_text.at(pos),
textLength,
_fontInUse,
&isTextSimple,
&uiLengthRead,
&_glyphIndices.at(pos));
const HRESULT hr = _fontRenderData->Analyzer()->GetTextComplexity(
_text.c_str(),
textLength,
_fontInUse,
&isTextSimple,
&uiLengthRead,
&_glyphIndices.at(glyphStart));
RETURN_IF_FAILED(hr);
_SetCurrentRun(pos);
_SplitCurrentRun(pos);
pos += std::max(uiLengthRead, 1u);
while (uiLengthRead > 0)
{
auto& run = _FetchNextRun(uiLengthRead);
run.isTextSimple = isTextSimple;
}
}
RETURN_IF_FAILED(hr);
_isEntireTextSimple = isTextSimple && uiLengthRead == textLength;
}
CATCH_RETURN();
return S_OK;
@ -224,26 +218,15 @@ CATCH_RETURN()
// Allocate enough room to have one breakpoint per code unit.
_breakpoints.resize(_text.size());
RETURN_IF_FAILED(_AnalyzeTextComplexity());
std::vector<std::pair<UINT32, UINT32>> complexRanges;
for (auto& run : _runs)
{
if (!run.isTextSimple)
{
complexRanges.push_back(std::make_pair(run.textStart, run.textLength));
}
}
for (auto& range : complexRanges)
if (!_isEntireTextSimple)
{
// Call each of the analyzers in sequence, recording their results.
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeLineBreakpoints(this, range.first, range.second, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeBidi(this, range.first, range.second, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeScript(this, range.first, range.second, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeNumberSubstitution(this, range.first, range.second, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeLineBreakpoints(this, 0, textLength, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeBidi(this, 0, textLength, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeScript(this, 0, textLength, this));
RETURN_IF_FAILED(_fontRenderData->Analyzer()->AnalyzeNumberSubstitution(this, 0, textLength, this));
// Perform our custom font fallback analyzer that mimics the pattern of the real analyzers.
RETURN_IF_FAILED(_AnalyzeFontFallback(this, range.first, range.second));
RETURN_IF_FAILED(_AnalyzeFontFallback(this, 0, textLength));
}
// Ensure that a font face is attached to every run
@ -283,7 +266,6 @@ CATCH_RETURN()
_glyphOffsets.resize(estimatedGlyphCount);
_glyphAdvances.resize(estimatedGlyphCount);
_glyphClusters.resize(textLength);
_glyphDesignUnitAdvances.resize(textLength);
UINT32 glyphStart = 0;
@ -357,7 +339,7 @@ CATCH_RETURN()
_glyphIndices.resize(totalGlyphsArrayCount);
}
if (run.isTextSimple)
if (_isEntireTextSimple)
{
// When the entire text is simple, we can skip GetGlyphs and directly retrieve glyph indices and
// advances(in font design unit). With the help of font metrics, we can calculate the actual glyph
@ -366,6 +348,10 @@ CATCH_RETURN()
DWRITE_FONT_METRICS1 metrics;
run.fontFace->GetMetrics(&metrics);
// With simple text, there's only one run. The actual glyph count is the same as textLength.
_glyphDesignUnitAdvances.resize(textLength);
_glyphAdvances.resize(textLength);
USHORT designUnitsPerEm = metrics.designUnitsPerEm;
RETURN_IF_FAILED(_fontInUse->GetDesignGlyphAdvances(
@ -374,14 +360,14 @@ CATCH_RETURN()
&_glyphDesignUnitAdvances.at(glyphStart),
run.isSideways));
for (UINT32 i = glyphStart; i < glyphStart + textLength; i++)
for (size_t i = glyphStart; i < _glyphAdvances.size(); i++)
{
_glyphAdvances.at(i) = (float)_glyphDesignUnitAdvances.at(i) / designUnitsPerEm * _formatInUse->GetFontSize() * run.fontScale;
}
// Set all the clusters as sequential. In a simple run, we're going 1 to 1.
// Fill the clusters sequentially from 0 to N-1.
std::iota(_glyphClusters.begin() + glyphStart, _glyphClusters.begin() + glyphStart + textLength, gsl::narrow_cast<unsigned short>(0));
std::iota(_glyphClusters.begin(), _glyphClusters.end(), gsl::narrow_cast<unsigned short>(0));
run.glyphCount = textLength;
glyphStart += textLength;
@ -486,6 +472,12 @@ CATCH_RETURN()
{
try
{
// For simple text, there is no need to correct runs.
if (_isEntireTextSimple)
{
return S_OK;
}
// Correct each run separately. This is needed whenever script, locale,
// or reading direction changes.
for (UINT32 runIndex = 0; runIndex < _runs.size(); ++runIndex)
@ -625,11 +617,6 @@ try
return S_FALSE; // Nothing to do..
}
if (run.isTextSimple)
{
return S_OK; // No need to correct run.
}
// We're going to walk through and check for advances that don't match the space that we expect to give out.
// Glyph Indices represents the number inside the selected font where the glyph image/paths are found.

View file

@ -76,7 +76,6 @@ namespace Microsoft::Console::Render
glyphCount(),
bidiLevel(),
script(),
isTextSimple(),
isNumberSubstituted(),
isSideways(),
fontFace{ nullptr },
@ -91,7 +90,6 @@ namespace Microsoft::Console::Render
UINT32 glyphCount; // number of glyphs associated with this run of text
DWRITE_SCRIPT_ANALYSIS script;
UINT8 bidiLevel;
bool isTextSimple;
bool isNumberSubstituted;
bool isSideways;
::Microsoft::WRL::ComPtr<IDWriteFontFace1> fontFace;
@ -178,6 +176,10 @@ namespace Microsoft::Console::Render
UINT32 _runIndex;
// Glyph shaping results
// Whether the entire text is determined to be simple and does not require full script shaping.
bool _isEntireTextSimple;
std::vector<DWRITE_GLYPH_OFFSET> _glyphOffsets;
// Clusters are complicated. They're in respect to each individual run.

View file

@ -194,3 +194,22 @@ void _CONSOLE_API_MSG::SetReplyInformation(const ULONG_PTR pInformation)
{
Complete.IoStatus.Information = pInformation;
}
void _CONSOLE_API_MSG::UpdateUserBufferPointers()
{
// There are some instances where an API message may get copied.
// Because it is infeasible to write a copy constructor for this class
// without rewriting large swaths of conhost (because of the unnamed union)
// we have chosen to introduce a "post-copy" step.
// This makes sure the buffers in State are in sync with the actual
// buffers in the object.
if (State.InputBuffer)
{
State.InputBuffer = _inputBuffer.data();
}
if (State.OutputBuffer)
{
State.OutputBuffer = _outputBuffer.data();
}
}

View file

@ -82,6 +82,13 @@ public:
void SetReplyStatus(const NTSTATUS Status);
void SetReplyInformation(const ULONG_PTR pInformation);
// MSFT-33127449, GH#9692
// We are not writing a copy constructor for this class
// so as to scope a fix as narrowly as possible to the
// "invalid user buffer" crash.
// TODO GH#10076: remove this.
void UpdateUserBufferPointers();
// DO NOT PUT MORE FIELDS DOWN HERE.
// The tail end of this structure will have a console driver packet
// copied over it and it will overwrite any fields declared here.

View file

@ -39,6 +39,8 @@ public:
{
}
virtual void MigrateUserBuffersOnTransitionToBackgroundWait(const void* oldBuffer, void* newBuffer) = 0;
virtual bool Notify(const WaitTerminationReason TerminationReason,
const bool fIsUnicode,
_Out_ NTSTATUS* const pReplyStatus,

View file

@ -32,6 +32,44 @@ ConsoleWaitBlock::ConsoleWaitBlock(_In_ ConsoleWaitQueue* const pProcessQueue,
{
_WaitReplyMessage = *pWaitReplyMessage;
// MSFT-33127449, GH#9692
// Until there's a "Wait", there's only one API message inflight at a time. In our
// quest for performance, we put that single API message in charge of its own
// buffer management- instead of allocating buffers on the heap and deleting them
// later (storing pointers to them at the far corners of the earth), it would
// instead allocate them from small internal pools (if possible) and only heap
// allocate (transparently) if necessary. The pointers flung to the corners of the
// earth would be pointers (1) back into the API_MSG or (2) to a heap block owned
// by boost::small_vector.
//
// It took us months to realize that those bare pointers were being held by
// COOKED_READ and RAW_READ and not actually being updated when the API message was
// _copied_ as it was shuffled off to the background to become a "Wait" message.
//
// It turns out that it's trivially possible to crash the console by sending two
// API calls -- one that waits and one that completes immediately -- when the
// waiting message or the "wait completer" has a bunch of dangling pointers in it.
// Oops.
//
// Here, we fix up the message's internal pointers (in lieu of giving it a proper
// copy constructor; see GH#10076) and then tell the wait completion routine (which
// is going to be a COOKED_READ, RAW_READ, DirectRead or WriteData) about the new
// buffer location.
//
// This is a scoped fix that should be replaced (TODO GH#10076) with a final one
// after Ask mode.
_WaitReplyMessage.UpdateUserBufferPointers();
if (pWaitReplyMessage->State.InputBuffer)
{
_pWaiter->MigrateUserBuffersOnTransitionToBackgroundWait(pWaitReplyMessage->State.InputBuffer, _WaitReplyMessage.State.InputBuffer);
}
if (pWaitReplyMessage->State.OutputBuffer)
{
_pWaiter->MigrateUserBuffersOnTransitionToBackgroundWait(pWaitReplyMessage->State.OutputBuffer, _WaitReplyMessage.State.OutputBuffer);
}
// We will write the original message back (with updated out parameters/payload) when the request is finally serviced.
if (pWaitReplyMessage->Complete.Write.Data != nullptr)
{