Skip to content

Commit

Permalink
Prevent flickering in nushell due to FTCS marks (microsoft#14677)
Browse files Browse the repository at this point in the history
Tl;dr: Conpty would flush a frame whenever it encountered a FTCS mark.
Combine that with the whole-line redrawing that nushell does, and the
Terminal would get the prompt in two frames instead of one, causing a
slight flickering. This fixes that by rendering the frame, but not
flushing to the pipe when we encounter one of these sequences.

Closes microsoft#13710 

A complication here: there are some sequences that we passthrough
_immediately_ when we encounter them. For example, `\x1b[ 2q`. we need
to also not flush when we encounter one of these sequences. nushell
emits one of these as a part of the prompt, and that would force the
buffered frame to get written _anyways_, before writing that to the
pipe.
  • Loading branch information
zadjii-msft authored May 11, 2023
1 parent b2dd7fa commit 6364450
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 12 deletions.
5 changes: 4 additions & 1 deletion src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2323,7 +2323,10 @@ void SCREEN_INFORMATION::SetTerminalConnection(_In_ VtEngine* const pTtyConnecti
if (pTtyConnection)
{
engine.SetTerminalConnection(pTtyConnection,
std::bind(&StateMachine::FlushToTerminal, _stateMachine.get()));
[&stateMachine = *_stateMachine]() -> bool {
ServiceLocator::LocateGlobals().pRender->NotifyPaintFrame();
return stateMachine.FlushToTerminal();
});
}
else
{
Expand Down
19 changes: 9 additions & 10 deletions src/renderer/vt/XtermEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,18 +526,17 @@ CATCH_RETURN();
RETURN_IF_FAILED(_fUseAsciiOnly ?
VtEngine::_WriteTerminalAscii(wstr) :
VtEngine::_WriteTerminalUtf8(wstr));
// GH#4106, GH#2011 - WriteTerminalW is only ever called by the
// GH#4106, GH#2011, GH#13710 - WriteTerminalW is only ever called by the
// StateMachine, when we've encountered a string we don't understand. When
// this happens, we usually don't actually trigger another frame, but we
// _do_ want this string to immediately be sent to the terminal. Since we
// only flush our buffer on actual frames, this means that strings we've
// decided to pass through would have gotten buffered here until the next
// actual frame is triggered.
//
// To fix this, flush here, so this string is sent to the connected terminal
// application.
// this happens, we will trigger a new frame in the renderer, and
// immediately buffer this wstr (representing the sequence we didn't
// understand). We won't immediately _Flush to the terminal - that might
// cause flickering (where we've buffered some state but not the whole
// "frame" as specified by the app). We'll just immediately buffer this
// sequence, and flush it when the render thread comes around to paint the
// frame normally.

return _Flush();
return S_OK;
}

// Method Description:
Expand Down
4 changes: 4 additions & 0 deletions src/renderer/vt/invalidate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ CATCH_RETURN();
_circled = circled;
}

// If we flushed for any reason other than circling (i.e, a sequence that we
// didn't understand), we don't need to push the buffer out on EndPaint.
_noFlushOnEnd = !circled;

_trace.TraceTriggerCircling(*pForcePaint);

return S_OK;
Expand Down
16 changes: 15 additions & 1 deletion src/renderer/vt/paint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,21 @@ using namespace Microsoft::Console::Types;
RETURN_IF_FAILED(_MoveCursor(_deferredCursorPos));
}

RETURN_IF_FAILED(_Flush());
// If this frame was triggered because we encountered a VT sequence which
// required the buffered state to get printed, we don't want to flush this
// frame to the pipe. That might result in us rendering half the output of a
// particular frame (as emitted by the client).
//
// Instead, we'll leave this frame in _buffer, and just keep appending to
// it as needed.
if (_noFlushOnEnd) [[unlikely]]
{
_noFlushOnEnd = false;
}
else
{
RETURN_IF_FAILED(_Flush());
}

return S_OK;
}
Expand Down
1 change: 1 addition & 0 deletions src/renderer/vt/vtrenderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ namespace Microsoft::Console::Render

bool _resizeQuirk{ false };
bool _passthrough{ false };
bool _noFlushOnEnd{ false };
std::optional<TextColor> _newBottomLineBG{ std::nullopt };

[[nodiscard]] HRESULT _WriteFill(const size_t n, const char c) noexcept;
Expand Down

0 comments on commit 6364450

Please sign in to comment.