Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge the PrintString functionality into AdaptDispatch #14640

Merged
6 commits merged into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 0 additions & 77 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1078,83 +1078,6 @@ Viewport Terminal::_GetVisibleViewport() const noexcept
size);
}

// Writes a string of text to the buffer, then moves the cursor (and viewport)
// in accordance with the written text.
// This method is our proverbial `WriteCharsLegacy`, and great care should be made to
// keep it minimal and orderly, lest it become WriteCharsLegacy2ElectricBoogaloo
// TODO: MSFT 21006766
// This needs to become stream logic on the buffer itself sooner rather than later
// because it's otherwise impossible to avoid the Electric Boogaloo-ness here.
// I had to make a bunch of hacks to get Japanese and emoji to work-ish.
void Terminal::_WriteBuffer(const std::wstring_view& stringView)
{
auto& cursor = _activeBuffer().GetCursor();

// Defer the cursor drawing while we are iterating the string, for a better performance.
// We can not waste time displaying a cursor event when we know more text is coming right behind it.
cursor.StartDeferDrawing();

for (size_t i = 0; i < stringView.size(); i++)
{
const auto wch = stringView.at(i);
const auto cursorPosBefore = cursor.GetPosition();
auto proposedCursorPosition = cursorPosBefore;

// TODO: MSFT 21006766
// This is not great but I need it demoable. Fix by making a buffer stream writer.
//
// If wch is a surrogate character we need to read 2 code units
// from the stringView to form a single code point.
const auto isSurrogate = wch >= 0xD800 && wch <= 0xDFFF;
const auto view = stringView.substr(i, isSurrogate ? 2 : 1);
const OutputCellIterator it{ view, _activeBuffer().GetCurrentAttributes() };
const auto end = _activeBuffer().Write(it);
const auto cellDistance = end.GetCellDistance(it);
const auto inputDistance = end.GetInputDistance(it);

if (inputDistance > 0)
{
// If "wch" was a surrogate character, we just consumed 2 code units above.
// -> Increment "i" by 1 in that case and thus by 2 in total in this iteration.
proposedCursorPosition.x += cellDistance;
i += gsl::narrow_cast<size_t>(inputDistance - 1);
}
else
{
// If _WriteBuffer() is called with a consecutive string longer than the viewport/buffer width
// the call to _buffer->Write() will refuse to write anything on the current line.
// GetInputDistance() thus returns 0, which would in turn cause i to be
// decremented by 1 below and force the outer loop to loop forever.
// This if() basically behaves as if "\r\n" had been encountered above and retries the write.
// With well behaving shells during normal operation this safeguard should normally not be encountered.
proposedCursorPosition.x = 0;
proposedCursorPosition.y++;

// Try the character again.
i--;

// If we write the last cell of the row here, TextBuffer::Write will
// mark this line as wrapped for us. If the next character we
// process is a newline, the Terminal::CursorLineFeed will unmark
// this line as wrapped.

// TODO: GH#780 - This should really be a _deferred_ newline. If
// the next character to come in is a newline or a cursor
// movement or anything, then we should _not_ wrap this line
// here.
}

_AdjustCursorPosition(proposedCursorPosition);
}

// Notify UIA of new text.
// It's important to do this here instead of in TextBuffer, because here you have access to the entire line of text,
// whereas TextBuffer writes it one character at a time via the OutputCellIterator.
_activeBuffer().TriggerNewTextNotification(stringView);

cursor.EndDeferDrawing();
}

void Terminal::_AdjustCursorPosition(const til::point proposedPosition)
{
#pragma warning(suppress : 26496) // cpp core checks wants this const but it's modified below.
Expand Down
5 changes: 1 addition & 4 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ class Microsoft::Terminal::Core::Terminal final :

#pragma region ITerminalApi
// These methods are defined in TerminalApi.cpp
void PrintString(const std::wstring_view string) override;
void ReturnResponse(const std::wstring_view response) override;
Microsoft::Console::VirtualTerminal::StateMachine& GetStateMachine() noexcept override;
TextBuffer& GetTextBuffer() noexcept override;
Expand All @@ -117,7 +116,7 @@ class Microsoft::Terminal::Core::Terminal final :
void SetScrollingRegion(const til::inclusive_rect& scrollMargins) noexcept override;
void WarningBell() override;
bool GetLineFeedMode() const noexcept override;
void LineFeed(const bool withReturn) override;
void LineFeed(const bool withReturn, const bool wrapForced) override;
void SetWindowTitle(const std::wstring_view title) override;
CursorType GetUserDefaultCursorStyle() const noexcept override;
bool ResizeWindow(const til::CoordType width, const til::CoordType height) noexcept override;
Expand Down Expand Up @@ -421,8 +420,6 @@ class Microsoft::Terminal::Core::Terminal final :
Microsoft::Console::Types::Viewport _GetMutableViewport() const noexcept;
Microsoft::Console::Types::Viewport _GetVisibleViewport() const noexcept;

void _WriteBuffer(const std::wstring_view& stringView);

void _AdjustCursorPosition(const til::point proposedPosition);

void _NotifyScrollEvent() noexcept;
Expand Down
14 changes: 4 additions & 10 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ TRACELOGGING_DEFINE_PROVIDER(g_hCTerminalCoreProvider,
(0x103ac8cf, 0x97d2, 0x51aa, 0xb3, 0xba, 0x5f, 0xfd, 0x55, 0x28, 0xfa, 0x5f),
TraceLoggingOptionMicrosoftTelemetry());

// Print puts the text in the buffer and moves the cursor
void Terminal::PrintString(const std::wstring_view string)
{
_WriteBuffer(string);
}

void Terminal::ReturnResponse(const std::wstring_view response)
{
if (_pfnWriteInput)
Expand Down Expand Up @@ -92,13 +86,13 @@ bool Terminal::GetLineFeedMode() const noexcept
return false;
}

void Terminal::LineFeed(const bool withReturn)
void Terminal::LineFeed(const bool withReturn, const bool wrapForced)
{
auto cursorPos = _activeBuffer().GetCursor().GetPosition();

// since we explicitly just moved down a row, clear the wrap status on the
// row we just came from
_activeBuffer().GetRowByOffset(cursorPos.y).SetWrapForced(false);
// If the line was forced to wrap, set the wrap status.
// When explicitly moving down a row, clear the wrap status.
_activeBuffer().GetRowByOffset(cursorPos.y).SetWrapForced(wrapForced);

cursorPos.y++;
if (withReturn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3207,7 +3207,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottom()

// GH#5839 -
// This test does expose a real bug when using WriteCharsLegacy to emit
// wrapped lines in conpty without WC_DELAY_EOL_WRAP. However, this fix has
// wrapped lines in conpty without delayed EOL wrap. However, this fix has
// not yet been made, so for now, we need to just skip the cases that cause
// this.
if (writingMethod == PrintWithWriteCharsLegacy && paintEachNewline == PaintEveryLine)
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void TerminalApiTest::PrintStringOfSurrogatePairs()
[](LPVOID data) -> DWORD {
const auto& baton = *reinterpret_cast<Baton*>(data);
Log::Comment(L"Writing data.");
baton.pTerm->PrintString(baton.text);
baton.pTerm->_stateMachine->ProcessString(baton.text);
Log::Comment(L"Setting event.");
SetEvent(baton.done);
return 0;
Expand Down
29 changes: 2 additions & 27 deletions src/host/_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;

auto lpString = pwchRealUnicode;

auto coordScreenBufferSize = screenInfo.GetBufferSize().Dimensions();
// In VT mode, the width at which we wrap is determined by the line rendition attribute.
if (WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING))
{
coordScreenBufferSize.width = textBuffer.GetLineWidth(CursorPosition.y);
}
const auto coordScreenBufferSize = screenInfo.GetBufferSize().Dimensions();

static constexpr til::CoordType LOCAL_BUFFER_SIZE = 1024;
WCHAR LocalBuffer[LOCAL_BUFFER_SIZE];
Expand All @@ -376,11 +371,6 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);

CursorPosition = cursor.GetPosition();
// In VT mode, we need to recalculate the width when moving to a new line.
if (WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING))
{
coordScreenBufferSize.width = textBuffer.GetLineWidth(CursorPosition.y);
}
}
}

Expand Down Expand Up @@ -570,22 +560,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine;
// WCL-NOTE: wrong place (typically inside another character).
CursorPosition.x = XPosition;

// enforce a delayed newline if we're about to pass the end and the WC_DELAY_EOL_WRAP flag is set.
if (WI_IsFlagSet(dwFlags, WC_DELAY_EOL_WRAP) && CursorPosition.x >= coordScreenBufferSize.width && fWrapAtEOL)
{
// Our cursor position as of this time is going to remain on the last position in this column.
CursorPosition.x = coordScreenBufferSize.width - 1;

// Update in the structures that we're still pointing to the last character in the row
cursor.SetPosition(CursorPosition);

// Record for the delay comparison that we're delaying on the last character in the row
cursor.DelayEOLWrap(CursorPosition);
}
else
{
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);
}
Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY);

// WCL-NOTE: If we have processed the entire input string during our "fast one-line print" handler,
// WCL-NOTE: we are done as there is nothing more to do. Neat!
Expand Down
2 changes: 1 addition & 1 deletion src/host/cmdline.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData);
#define WC_LIMIT_BACKSPACE 0x10
//#define WC_NONDESTRUCTIVE_TAB 0x20 - This is not needed anymore, because the VT code handles tabs internally now.
//#define WC_NEWLINE_SAVE_X 0x40 - This has been replaced with an output mode flag instead as it's line discipline behavior that may not necessarily be coupled with VT.
#define WC_DELAY_EOL_WRAP 0x80
//#define WC_DELAY_EOL_WRAP 0x80 - This is not needed anymore, because the AdaptDispatch class handles all VT output.

// Word delimiters
bool IsWordDelim(const wchar_t wch);
Expand Down
45 changes: 5 additions & 40 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,39 +24,6 @@ ConhostInternalGetSet::ConhostInternalGetSet(_In_ IIoProvider& io) :
{
}

// Routine Description:
// - Handles the print action from the state machine
// Arguments:
// - string - The string to be printed.
// Return Value:
// - <none>
void ConhostInternalGetSet::PrintString(const std::wstring_view string)
{
auto dwNumBytes = string.size() * sizeof(wchar_t);

auto& cursor = _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor();
if (!cursor.IsOn())
{
cursor.SetIsOn(true);
}

// Defer the cursor drawing while we are iterating the string, for a better performance.
// We can not waste time displaying a cursor event when we know more text is coming right behind it.
cursor.StartDeferDrawing();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/note: where'd this go

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh duh

The root cause was the fact that we called cursor.StartDeferDrawing()
before outputting the text, and this was something I had adopted in the
new implementation as well. But I've now removed that, and instead just
call cursor.SetIsOn(false). This seems to avoid the cursor droppings,
and hopefully still has similar performance benefits.

const auto ntstatus = WriteCharsLegacy(_io.GetActiveOutputBuffer(),
string.data(),
string.data(),
string.data(),
&dwNumBytes,
nullptr,
_io.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition().x,
WC_LIMIT_BACKSPACE | WC_DELAY_EOL_WRAP,
nullptr);
cursor.EndDeferDrawing();

THROW_IF_NTSTATUS_FAILED(ntstatus);
}

// - Sends a string response to the input stream of the console.
// - Used by various commands where the program attached would like a reply to one of the commands issued.
// - This will generate two "key presses" (one down, one up) for every character in the string and place them into the head of the console's input stream.
Expand Down Expand Up @@ -207,20 +174,18 @@ bool ConhostInternalGetSet::GetLineFeedMode() const
// - Performs a line feed, possibly preceded by carriage return.
// Arguments:
// - withReturn - Set to true if a carriage return should be performed as well.
// - wrapForced - Set to true is the line feed was the result of the line wrapping.
// Return Value:
// - <none>
void ConhostInternalGetSet::LineFeed(const bool withReturn)
void ConhostInternalGetSet::LineFeed(const bool withReturn, const bool wrapForced)
{
auto& screenInfo = _io.GetActiveOutputBuffer();
auto& textBuffer = screenInfo.GetTextBuffer();
auto cursorPosition = textBuffer.GetCursor().GetPosition();

// We turn the cursor on before an operation that might scroll the viewport, otherwise
// that can result in an old copy of the cursor being left behind on the screen.
textBuffer.GetCursor().SetIsOn(true);

// Since we are explicitly moving down a row, clear the wrap status on the row we're leaving
textBuffer.GetRowByOffset(cursorPosition.y).SetWrapForced(false);
// If the line was forced to wrap, set the wrap status.
// When explicitly moving down a row, clear the wrap status.
textBuffer.GetRowByOffset(cursorPosition.y).SetWrapForced(wrapForced);

cursorPosition.y += 1;
if (withReturn)
Expand Down
3 changes: 1 addition & 2 deletions src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
public:
ConhostInternalGetSet(_In_ Microsoft::Console::IIoProvider& io);

void PrintString(const std::wstring_view string) override;
void ReturnResponse(const std::wstring_view response) override;

Microsoft::Console::VirtualTerminal::StateMachine& GetStateMachine() override;
Expand All @@ -47,7 +46,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
void WarningBell() override;

bool GetLineFeedMode() const override;
void LineFeed(const bool withReturn) override;
void LineFeed(const bool withReturn, const bool wrapForced) override;

void SetWindowTitle(const std::wstring_view title) override;

Expand Down
3 changes: 1 addition & 2 deletions src/terminal/adapter/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ namespace Microsoft::Console::VirtualTerminal
ITerminalApi& operator=(const ITerminalApi&) = delete;
ITerminalApi& operator=(ITerminalApi&&) = delete;

virtual void PrintString(const std::wstring_view string) = 0;
virtual void ReturnResponse(const std::wstring_view response) = 0;

virtual StateMachine& GetStateMachine() = 0;
Expand All @@ -55,7 +54,7 @@ namespace Microsoft::Console::VirtualTerminal
virtual void SetScrollingRegion(const til::inclusive_rect& scrollMargins) = 0;
virtual void WarningBell() = 0;
virtual bool GetLineFeedMode() const = 0;
virtual void LineFeed(const bool withReturn) = 0;
virtual void LineFeed(const bool withReturn, const bool wrapForced) = 0;
virtual void SetWindowTitle(const std::wstring_view title) = 0;
virtual void UseAlternateScreenBuffer() = 0;
virtual void UseMainScreenBuffer() = 0;
Expand Down
Loading