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.

Sign up for GitHub

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

Introduce VTInt to represent VT parameters #12207

Merged
2 commits merged into from
Apr 20, 2022
Merged

Introduce VTInt to represent VT parameters #12207

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87263bb
Introduce VTInt to represent VT parameters
lhecker Apr 19, 2022
75a02fc
Fix AuditMode errors
lhecker Apr 20, 2022
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
10 changes: 5 additions & 5 deletions src/cascadia/TerminalCore/ITerminalApi.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ namespace Microsoft::Terminal::Core
virtual void SetTextAttributes(const TextAttribute& attrs) = 0;

virtual Microsoft::Console::Types::Viewport GetBufferSize() = 0;
virtual void SetCursorPosition(short x, short y) = 0;
virtual COORD GetCursorPosition() = 0;
virtual void SetCursorPosition(til::point pos) = 0;
virtual til::point GetCursorPosition() = 0;
virtual void SetCursorVisibility(const bool visible) = 0;
virtual void CursorLineFeed(const bool withReturn) = 0;
virtual void EnableCursorBlinking(const bool enable) = 0;

virtual void DeleteCharacter(const size_t count) = 0;
virtual void InsertCharacter(const size_t count) = 0;
virtual void EraseCharacters(const size_t numChars) = 0;
virtual void DeleteCharacter(const til::CoordType count) = 0;
virtual void InsertCharacter(const til::CoordType count) = 0;
virtual void EraseCharacters(const til::CoordType numChars) = 0;
virtual bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) = 0;
virtual bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) = 0;

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/Terminal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ bool Terminal::SendMouseEvent(const COORD viewportPos, const unsigned int uiButt
#pragma warning(suppress : 26496) // analysis can't tell we're assigning through a reference below
auto clampedPos{ viewportPos };
_GetMutableViewport().ToOrigin().Clamp(clampedPos);
return _terminalInput->HandleMouse(clampedPos, uiButton, GET_KEYSTATE_WPARAM(states.Value()), wheelDelta, state);
return _terminalInput->HandleMouse(til::point{ clampedPos }, uiButton, GET_KEYSTATE_WPARAM(states.Value()), wheelDelta, state);
}

// Method Description:
Expand Down
10 changes: 5 additions & 5 deletions src/cascadia/TerminalCore/Terminal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ class Microsoft::Terminal::Core::Terminal final :
TextAttribute GetTextAttributes() const override;
void SetTextAttributes(const TextAttribute& attrs) override;
Microsoft::Console::Types::Viewport GetBufferSize() override;
void SetCursorPosition(short x, short y) override;
COORD GetCursorPosition() override;
void SetCursorPosition(til::point pos) override;
til::point GetCursorPosition() override;
void SetCursorVisibility(const bool visible) override;
void EnableCursorBlinking(const bool enable) override;
void CursorLineFeed(const bool withReturn) override;
void DeleteCharacter(const size_t count) override;
void InsertCharacter(const size_t count) override;
void EraseCharacters(const size_t numChars) override;
void DeleteCharacter(const til::CoordType count) override;
void InsertCharacter(const til::CoordType count) override;
void EraseCharacters(const til::CoordType numChars) override;
bool EraseInLine(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override;
bool EraseInDisplay(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::EraseType eraseType) override;
void WarningBell() override;
Expand Down
61 changes: 23 additions & 38 deletions src/cascadia/TerminalCore/TerminalApi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,22 @@ Viewport Terminal::GetBufferSize()
return _activeBuffer().GetSize();
}

void Terminal::SetCursorPosition(short x, short y)
void Terminal::SetCursorPosition(til::point pos)
{
const auto viewport = _GetMutableViewport();
const auto viewOrigin = viewport.Origin();
const short absoluteX = viewOrigin.X + x;
const short absoluteY = viewOrigin.Y + y;
COORD newPos{ absoluteX, absoluteY };
const til::point viewOrigin{ viewport.Origin() };
auto newPos = til::unwrap_coord(viewOrigin + pos);
Copy link
Member Author

Choose a reason for hiding this comment

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

This commit contains quite a number of explicit wrapping and unwrapping of COORD/SMALL_RECT into til::point/til::rect, or explicit narrowings into SHORTs, because only a part of the project was converted to the til helpers. In a future PR these changes will be undone, since barely anything will use COORD/SMALL_RECT then.

viewport.Clamp(newPos);
_activeBuffer().GetCursor().SetPosition(newPos);
}

COORD Terminal::GetCursorPosition()
til::point Terminal::GetCursorPosition()
{
const auto absoluteCursorPos = _activeBuffer().GetCursor().GetPosition();
const til::point absoluteCursorPos{ _activeBuffer().GetCursor().GetPosition() };
const auto viewport = _GetMutableViewport();
const auto viewOrigin = viewport.Origin();
const short relativeX = absoluteCursorPos.X - viewOrigin.X;
const short relativeY = absoluteCursorPos.Y - viewOrigin.Y;
COORD newPos{ relativeX, relativeY };

const til::point viewOrigin{ viewport.Origin() };
// TODO assert that the coord is > (0, 0) && <(view.W, view.H)
return newPos;
return absoluteCursorPos - viewOrigin;
}

// Method Description:
Expand Down Expand Up @@ -97,21 +91,17 @@ void Terminal::CursorLineFeed(const bool withReturn)
// - count, the number of characters to delete
// Return value:
// - <none>
void Terminal::DeleteCharacter(const size_t count)
void Terminal::DeleteCharacter(const til::CoordType count)
{
SHORT dist;
THROW_IF_FAILED(SizeTToShort(count, &dist));
const auto cursorPos = _activeBuffer().GetCursor().GetPosition();
const auto copyToPos = cursorPos;
const COORD copyFromPos{ cursorPos.X + dist, cursorPos.Y };
const til::point copyFromPos{ cursorPos.X + count, cursorPos.Y };
const auto viewport = _GetMutableViewport();

const auto sourceWidth = viewport.RightExclusive() - copyFromPos.X;
SHORT width;
THROW_IF_FAILED(UIntToShort(sourceWidth, &width));

// Get a rectangle of the source
auto source = Viewport::FromDimensions(copyFromPos, width, 1);
auto source = Viewport::FromDimensions(til::unwrap_coord(copyFromPos), gsl::narrow<short>(sourceWidth), 1);

// Get a rectangle of the target
const auto target = Viewport::FromDimensions(copyToPos, source.Dimensions());
Expand All @@ -137,28 +127,23 @@ void Terminal::DeleteCharacter(const size_t count)
// - count, the number of spaces to insert
// Return value:
// - <none>
void Terminal::InsertCharacter(const size_t count)
void Terminal::InsertCharacter(const til::CoordType count)
{
// NOTE: the code below is _extremely_ similar to DeleteCharacter
// We will want to use this same logic and implement a helper function instead
// that does the 'move a region from here to there' operation
// TODO: GitHub issue #2163
SHORT dist;
THROW_IF_FAILED(SizeTToShort(count, &dist));
const auto cursorPos = _activeBuffer().GetCursor().GetPosition();
const auto copyFromPos = cursorPos;
const COORD copyToPos{ cursorPos.X + dist, cursorPos.Y };
const til::point copyToPos{ cursorPos.X + count, cursorPos.Y };
const auto viewport = _GetMutableViewport();
const auto sourceWidth = viewport.RightExclusive() - copyFromPos.X;
SHORT width;
THROW_IF_FAILED(UIntToShort(sourceWidth, &width));

// Get a rectangle of the source
auto source = Viewport::FromDimensions(copyFromPos, width, 1);
const auto sourceOrigin = source.Origin();
auto source = Viewport::FromDimensions(copyFromPos, gsl::narrow<short>(sourceWidth), 1);

// Get a rectangle of the target
const auto target = Viewport::FromDimensions(copyToPos, source.Dimensions());
const auto target = Viewport::FromDimensions(til::unwrap_coord(copyToPos), source.Dimensions());
const auto walkDirection = Viewport::DetermineWalkDirection(source, target);

auto sourcePos = source.GetWalkOrigin(walkDirection);
Expand All @@ -170,16 +155,16 @@ void Terminal::InsertCharacter(const size_t count)
const auto data = OutputCell(*(_activeBuffer().GetCellDataAt(sourcePos)));
_activeBuffer().Write(OutputCellIterator({ &data, 1 }), targetPos);
} while (source.WalkInBounds(sourcePos, walkDirection) && target.WalkInBounds(targetPos, walkDirection));
const auto eraseIter = OutputCellIterator(UNICODE_SPACE, _activeBuffer().GetCurrentAttributes(), dist);
const auto eraseIter = OutputCellIterator(UNICODE_SPACE, _activeBuffer().GetCurrentAttributes(), count);
_activeBuffer().Write(eraseIter, cursorPos);
}

void Terminal::EraseCharacters(const size_t numChars)
void Terminal::EraseCharacters(const til::CoordType numChars)
{
const auto absoluteCursorPos = _activeBuffer().GetCursor().GetPosition();
const auto viewport = _GetMutableViewport();
const short distanceToRight = viewport.RightExclusive() - absoluteCursorPos.X;
const short fillLimit = std::min(static_cast<short>(numChars), distanceToRight);
const auto distanceToRight = viewport.RightExclusive() - absoluteCursorPos.X;
const auto fillLimit = std::min(numChars, distanceToRight);
const auto eraseIter = OutputCellIterator(UNICODE_SPACE, _activeBuffer().GetCurrentAttributes(), fillLimit);
_activeBuffer().Write(eraseIter, absoluteCursorPos);
}
Expand All @@ -198,7 +183,7 @@ bool Terminal::EraseInLine(const ::Microsoft::Console::VirtualTerminal::Dispatch
{
const auto cursorPos = _activeBuffer().GetCursor().GetPosition();
const auto viewport = _GetMutableViewport();
COORD startPos = { 0 };
til::point startPos;
startPos.Y = cursorPos.Y;
// nlength determines the number of spaces we need to write
DWORD nlength = 0;
Expand All @@ -224,7 +209,7 @@ bool Terminal::EraseInLine(const ::Microsoft::Console::VirtualTerminal::Dispatch
const auto eraseIter = OutputCellIterator(UNICODE_SPACE, _activeBuffer().GetCurrentAttributes(), nlength);

// Explicitly turn off end-of-line wrap-flag-setting when erasing cells.
_activeBuffer().Write(eraseIter, startPos, false);
_activeBuffer().Write(eraseIter, til::unwrap_coord(startPos), false);
return true;
}

Expand Down Expand Up @@ -275,7 +260,7 @@ bool Terminal::EraseInDisplay(const DispatchTypes::EraseType eraseType)
short sNewTop = coordLastChar.Y + 1;

// Increment the circular buffer only if the new location of the viewport would be 'below' the buffer
const short delta = (sNewTop + viewport.Height()) - (_activeBuffer().GetSize().Height());
const auto delta = (sNewTop + viewport.Height()) - (_activeBuffer().GetSize().Height());
for (auto i = 0; i < delta; i++)
{
_activeBuffer().IncrementCircularBuffer();
Expand All @@ -297,7 +282,7 @@ bool Terminal::EraseInDisplay(const DispatchTypes::EraseType eraseType)
// and we have to make sure we erase that text
const auto eraseStart = viewport.Height();
const auto eraseEnd = _activeBuffer().GetLastNonSpaceCharacter(viewport).Y;
for (SHORT i = eraseStart; i <= eraseEnd; i++)
for (auto i = eraseStart; i <= eraseEnd; i++)
{
_activeBuffer().GetRowByOffset(i).Reset(_activeBuffer().GetCurrentAttributes());
}
Expand All @@ -316,7 +301,7 @@ bool Terminal::EraseInDisplay(const DispatchTypes::EraseType eraseType)
// Move the viewport, adjust the scroll bar if needed, and restore the old cursor position
_mutableViewport = Viewport::FromExclusive(newWin);
Terminal::_NotifyScrollEvent();
SetCursorPosition(relativeCursor.X, relativeCursor.Y);
SetCursorPosition(til::point{ relativeCursor });

return true;
}
Expand Down
52 changes: 20 additions & 32 deletions src/cascadia/TerminalCore/TerminalDispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,10 @@ void TerminalDispatch::PrintString(const std::wstring_view string)
_terminalApi.PrintString(string);
}

bool TerminalDispatch::CursorPosition(const size_t line,
const size_t column)
bool TerminalDispatch::CursorPosition(VTInt line,
VTInt column)
{
SHORT x{ 0 };
SHORT y{ 0 };

THROW_IF_FAILED(SizeTToShort(column, &x));
THROW_IF_FAILED(SizeTToShort(line, &y));

THROW_IF_FAILED(ShortSub(x, 1, &x));
THROW_IF_FAILED(ShortSub(y, 1, &y));

_terminalApi.SetCursorPosition(x, y);
_terminalApi.SetCursorPosition({ column - 1, line - 1 });
return true;
}

Expand All @@ -57,27 +48,24 @@ bool TerminalDispatch::EnableCursorBlinking(const bool enable)
return true;
}

bool TerminalDispatch::CursorForward(const size_t distance)
bool TerminalDispatch::CursorForward(const VTInt distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X + gsl::narrow<short>(distance), cursorPos.Y };
_terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
_terminalApi.SetCursorPosition({ cursorPos.X + distance, cursorPos.Y });
return true;
}

bool TerminalDispatch::CursorBackward(const size_t distance)
bool TerminalDispatch::CursorBackward(const VTInt distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X - gsl::narrow<short>(distance), cursorPos.Y };
_terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
_terminalApi.SetCursorPosition({ cursorPos.X - distance, cursorPos.Y });
return true;
}

bool TerminalDispatch::CursorUp(const size_t distance)
bool TerminalDispatch::CursorUp(const VTInt distance)
{
const auto cursorPos = _terminalApi.GetCursorPosition();
const COORD newCursorPos{ cursorPos.X, cursorPos.Y + gsl::narrow<short>(distance) };
_terminalApi.SetCursorPosition(newCursorPos.X, newCursorPos.Y);
_terminalApi.SetCursorPosition({ cursorPos.X, cursorPos.Y + distance });
return true;
}

Expand All @@ -99,7 +87,7 @@ bool TerminalDispatch::LineFeed(const DispatchTypes::LineFeedType lineFeedType)
}
}

bool TerminalDispatch::EraseCharacters(const size_t numChars)
bool TerminalDispatch::EraseCharacters(const VTInt numChars)
{
_terminalApi.EraseCharacters(numChars);
return true;
Expand All @@ -114,7 +102,7 @@ bool TerminalDispatch::WarningBell()
bool TerminalDispatch::CarriageReturn()
{
const auto cursorPos = _terminalApi.GetCursorPosition();
_terminalApi.SetCursorPosition(0, cursorPos.Y);
_terminalApi.SetCursorPosition({ 0, cursorPos.Y });
return true;
}

Expand All @@ -134,13 +122,13 @@ bool TerminalDispatch::HorizontalTabSet()
return true;
}

bool TerminalDispatch::ForwardTab(const size_t numTabs)
bool TerminalDispatch::ForwardTab(const VTInt numTabs)
{
const auto width = _terminalApi.GetBufferSize().Dimensions().X;
const auto cursorPosition = _terminalApi.GetCursorPosition();
auto column = cursorPosition.X;
const auto row = cursorPosition.Y;
auto tabsPerformed = 0u;
VTInt tabsPerformed = 0;
_InitTabStopsForWidth(width);
while (column + 1 < width && tabsPerformed < numTabs)
{
Expand All @@ -151,17 +139,17 @@ bool TerminalDispatch::ForwardTab(const size_t numTabs)
}
}

_terminalApi.SetCursorPosition(column, row);
_terminalApi.SetCursorPosition({ column, row });
return true;
}

bool TerminalDispatch::BackwardsTab(const size_t numTabs)
bool TerminalDispatch::BackwardsTab(const VTInt numTabs)
{
const auto width = _terminalApi.GetBufferSize().Dimensions().X;
const auto cursorPosition = _terminalApi.GetCursorPosition();
auto column = cursorPosition.X;
const auto row = cursorPosition.Y;
auto tabsPerformed = 0u;
VTInt tabsPerformed = 0;
_InitTabStopsForWidth(width);
while (column > 0 && tabsPerformed < numTabs)
{
Expand All @@ -172,7 +160,7 @@ bool TerminalDispatch::BackwardsTab(const size_t numTabs)
}
}

_terminalApi.SetCursorPosition(column, row);
_terminalApi.SetCursorPosition({ column, row });
return true;
}

Expand Down Expand Up @@ -266,7 +254,7 @@ bool TerminalDispatch::EraseInLine(const DispatchTypes::EraseType eraseType)
// - count, the number of characters to delete
// Return Value:
// - True.
bool TerminalDispatch::DeleteCharacter(const size_t count)
bool TerminalDispatch::DeleteCharacter(const VTInt count)
{
_terminalApi.DeleteCharacter(count);
return true;
Expand All @@ -278,7 +266,7 @@ bool TerminalDispatch::DeleteCharacter(const size_t count)
// - count, the number of spaces to add
// Return Value:
// - True.
bool TerminalDispatch::InsertCharacter(const size_t count)
bool TerminalDispatch::InsertCharacter(const VTInt count)
{
_terminalApi.InsertCharacter(count);
return true;
Expand Down Expand Up @@ -803,7 +791,7 @@ bool TerminalDispatch::CursorSaveState()
// TODO GH#3849: When de-duplicating this, the AdaptDispatch version of this
// is more elaborate.
const auto attributes = _terminalApi.GetTextAttributes();
COORD coordCursor = _terminalApi.GetCursorPosition();
const auto coordCursor = _terminalApi.GetCursorPosition();
// The cursor is given to us by the API as relative to current viewport top.

// VT is also 1 based, not 0 based, so correct by 1.
Expand Down
Loading