Skip to content

Commit

Permalink
mAYBE
Browse files Browse the repository at this point in the history
  • Loading branch information
leonMSFT committed Apr 8, 2020
1 parent 7574e9c commit 5fcbac7
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 27 deletions.
19 changes: 13 additions & 6 deletions src/buffer/out/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ Cursor::Cursor(const ULONG ulSize, TextBuffer& parentBuffer) noexcept :
_ulSize(ulSize),
_cursorType(CursorType::Legacy),
_fUseColor(false),
_color(s_InvertCursorColor)
_color(s_InvertCursorColor),
_iDeferCounter{ 0 }
{
}

Expand Down Expand Up @@ -166,7 +167,7 @@ void Cursor::_RedrawCursor() noexcept
// (Conversion areas have cursors to mark the insertion point internally, but the user's actual cursor is the one on the primary screen buffer.)
if (IsOn() && !IsConversionArea())
{
if (_fDeferCursorRedraw)
if (_iDeferCounter > 0)
{
_fHaveDeferredCursorRedraw = true;
}
Expand Down Expand Up @@ -281,6 +282,7 @@ void Cursor::CopyProperties(const Cursor& OtherCursor) noexcept

_fDeferCursorRedraw = OtherCursor._fDeferCursorRedraw;
_fHaveDeferredCursorRedraw = OtherCursor._fHaveDeferredCursorRedraw;
_iDeferCounter = OtherCursor._iDeferCounter;

// Size will be handled separately in the resize operation.
//_ulSize = OtherCursor._ulSize;
Expand Down Expand Up @@ -312,17 +314,22 @@ bool Cursor::IsDelayedEOLWrap() const noexcept

void Cursor::StartDeferDrawing() noexcept
{
_fDeferCursorRedraw = true;
++_iDeferCounter;
}

void Cursor::EndDeferDrawing() noexcept
{
if (_fHaveDeferredCursorRedraw)
// Nobody has started a defer drawing.
if (_iDeferCounter == 0)
{
_RedrawCursorAlways();
return;
}

_fDeferCursorRedraw = FALSE;
--_iDeferCounter;
if (_iDeferCounter == 0)
{
_RedrawCursorAlways();
}
}

const CursorType Cursor::GetType() const noexcept
Expand Down
1 change: 1 addition & 0 deletions src/buffer/out/cursor.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class Cursor final
bool _fDelayedEolWrap; // don't wrap at EOL till the next char comes in.
COORD _coordDelayedAt; // coordinate the EOL wrap was delayed at.

int _iDeferCounter;
bool _fDeferCursorRedraw; // whether we should defer redrawing the cursor or not
bool _fHaveDeferredCursorRedraw; // have we been asked to redraw the cursor while it was being deferred?

Expand Down
36 changes: 19 additions & 17 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,6 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
{
const auto oldDimensions = _mutableViewport.Dimensions();

if (!_buffer->GetSize().IsInBounds(_buffer->GetCursor().GetPosition()))
{
OutputDebugString(L"UserResize not in bounds\n");
}

if (viewportSize == oldDimensions)
{
return S_FALSE;
Expand All @@ -196,6 +191,23 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
// bottom in the new buffer as well. Track that case now.
const bool originalOffsetWasZero = _scrollOffset == 0;

// Defer the cursor drawing here right before the Reflow call
// because we only want the renderer to redraw the cursor right
// after we swap out the old buffer with the new buffer from Reflow.
// This swap happens down below, and so we'll EndDeferDrawing right
// after the swap.
// What would happen is that at the end of Reflow, we would call
// EndDeferDrawing, causing the renderer to go and attempt to redraw
// the cursor. However there's a race condition where, sometimes it
// would check for the _buffer properties before we swap out the
// buffer with the new one. This would make it crash if the old
// buffer's cursor position was out of bounds.
// Now that StartDeferDrawing uses a counter and will only redraw
// when the counter reaches 0, this StartDeferDrawing call here will
// force the renderer to redraw at least until this particular
// deferral is met with its corresponding EndDeferral down below.
_buffer->GetCursor().StartDeferDrawing();

// First allocate a new text buffer to take the place of the current one.
std::unique_ptr<TextBuffer> newTextBuffer;
try
Expand Down Expand Up @@ -335,6 +347,8 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting

_buffer.swap(newTextBuffer);

_buffer->GetCursor().EndDeferDrawing();

// GH#3494: Maintain scrollbar position during resize
// Make sure that we don't scroll past the mutableViewport at the bottom of the buffer
newVisibleTop = std::min(newVisibleTop, _mutableViewport.Top());
Expand Down Expand Up @@ -716,11 +730,6 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView)
// -> Increment "i" by 1 in that case and thus by 2 in total in this iteration.
proposedCursorPosition.X += gsl::narrow<SHORT>(cellDistance);

if (!_buffer->GetSize().IsInBounds(proposedCursorPosition))
{
OutputDebugString(L"InDist > 0 not in Bounds\n");
}

i += inputDistance - 1;
}
else
Expand All @@ -737,8 +746,6 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView)
// Try the character again.
i--;

OutputDebugString(L"Reset to 0\n");

// 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
Expand All @@ -753,11 +760,6 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView)
_AdjustCursorPosition(proposedCursorPosition);
}

if (!_buffer->GetSize().IsInBounds(_buffer->GetCursor().GetPosition()))
{
OutputDebugString(L"Still not in Bounds\n");
}

cursor.EndDeferDrawing();
}

Expand Down
11 changes: 7 additions & 4 deletions src/renderer/base/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,13 @@ void Renderer::TriggerRedrawCursor(const COORD* const pcoord)
{
LOG_IF_FAILED(pEngine->InvalidateCursor(&updateCoord));

if (!_pData->GetTextBuffer().GetSize().IsInBounds(_pData->GetTextBuffer().GetCursor().GetPosition()))
{
char buf[2000];
sprintf_s(buf, "TriggerRedraw: bufferX: %d, cursorpos: %d, updateCoord: %d\n", gsl::narrow_cast<int>(_pData->GetTextBuffer().GetSize().RightExclusive()), gsl::narrow_cast<int>(_pData->GetTextBuffer().GetCursor().GetPosition().X), updateCoord.X);
OutputDebugStringA(buf);
}

// Double-wide cursors need to invalidate the right half as well.
if (_pData->IsCursorDoubleWidth())
{
Expand Down Expand Up @@ -832,10 +839,6 @@ void Renderer::_PaintCursor(_In_ IRenderEngine* const pEngine)
// Draw it within the viewport
LOG_IF_FAILED(pEngine->PaintCursor(options));
}
else
{
OutputDebugString(L"PaintCursor NOT In Bounds \n");
}
}
}

Expand Down

1 comment on commit 5fcbac7

@github-actions
Copy link

Choose a reason for hiding this comment

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

New misspellings found, please review:

  • cursorpos
To accept these changes, run the following commands
remove_obsolete_words=$(mktemp)
echo '#!/usr/bin/perl -ni
my $re=join "|", qw('"
APPLOGIC
Impl
Spc
trackpads
unicodechar
ve
XY
"');
next if /^($re)(?:$| .*)/;
print;' > $remove_obsolete_words
chmod +x $remove_obsolete_words
for file in .github/actions/spell-check/whitelist/alphabet.txt .github/actions/spell-check/whitelist/web.txt .github/actions/spell-check/whitelist/whitelist.txt; do $remove_obsolete_words $file; done
rm $remove_obsolete_words
(
echo "
applogic
cursorpos
impl
xy
"
) | sort -u -f | perl -ne 'next unless /./; print' > new_whitelist.txt && mv new_whitelist.txt '.github/actions/spell-check/whitelist/5fcbac7a3505240cc2d4c30e833e181a1dcf0596.txt'
✏️ Contributor please read this
  • If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
  • If you need to use a specific token in one place and it shouldn't generally be used, you can
    add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Please sign in to comment.