Skip to content

Commit

Permalink
Restore the DECCTR color table report over conpty (#13227)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

Up to now we haven't supported passing `DCS` sequences over conpty, so
any `DCS` operations would just be ignored in Windows Terminal. This PR
introduces a mechanism whereby we can selectively pass through
operations that could reasonably be handled by the connected terminal
without interfering with the regular conpty renderer. For now this is
just used to support restoring the `DECCTR` color table report.

## References

Support for `DECCTR` was originally added to conhost in PR #13139.

## PR Checklist
* [x] Closes #13223
* [x] CLA signed.
* [x] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not
checked, I'm ready to accept this work might be rejected in favor of a
different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

The way this works is we have a helper method in `AdaptDispatch` that
`DCS` operations can use to create a passthrough `StringHandler` for the
incoming data instead of their usual handler. To make this passthrough
process more efficient, the handler buffers the data before forwarding
it to conpty.

However, it's important that we aren't holding back data if output is
paused before the string terminator, so we need to flush the buffer
whenever we reach the end of the current write operation. This is
achieved by querying a new method in the `StateMachine` that lets us
know if we're currently dealing with the last character.

Another issue that came up was with the way the `StateMachine` caches
sequences that it might later need to forward to conpty. In the case of
string sequences like `DCS`, we don't want the actual string data cached
here, because that will just waste memory, and can also result in the
data being mistakenly output. So I've now disabled that caching when
we're in any of the string processing states.

## Validation Steps Performed

I've manually confirmed that the `DECCTR` sequence can now update the
color table in Windows Terminal. I've also added a new unit test in
`ConptyRoundtripTests` to verify the sequence is being passed through
successfully.
  • Loading branch information
j4james authored Jun 10, 2022
1 parent b22684e commit c754f4d
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 2 deletions.
38 changes: 38 additions & 0 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final

TEST_METHOD(PassthroughCursorShapeImmediately);

TEST_METHOD(PassthroughDECCTR);

TEST_METHOD(TestWrappingALongString);
TEST_METHOD(TestAdvancedWrapping);
TEST_METHOD(TestExactWrappingWithoutSpaces);
Expand Down Expand Up @@ -1206,6 +1208,42 @@ void ConptyRoundtripTests::PassthroughHardReset()
}
}

void ConptyRoundtripTests::PassthroughDECCTR()
{
Log::Comment(L"Update the color table with DECCTR. This should immediately be flushed to the Terminal.");

auto& g = ServiceLocator::LocateGlobals();
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto& hostSm = si.GetStateMachine();
auto& termRenderSettings = term->GetRenderSettings();

_flushFirstFrame();

_logConpty = true;

// We're going to update color table entries 101 through 103, so we start by
// initializing those entries to black in the Terminal render settings.
termRenderSettings.SetColorTableEntry(101, RGB(0, 0, 0));
termRenderSettings.SetColorTableEntry(102, RGB(0, 0, 0));
termRenderSettings.SetColorTableEntry(103, RGB(0, 0, 0));

// DECCTR is expected to arrive in two parts. The control sequence comes
// first, and the string content follows in the second packet.
expectedOutput.push_back("\x1bP2$p");
expectedOutput.push_back("101;2;100;0;0/102;2;0;100;0/103;2;0;0;100\x1b\\");

// Send the control sequence to the host.
hostSm.ProcessString(L"\x1bP2$p101;2;100;0;0/102;2;0;100;0/103;2;0;0;100\x1b\\");

// Verify that the color table entries have been updated in the Terminal.
VERIFY_ARE_EQUAL(RGB(255, 0, 0), termRenderSettings.GetColorTableEntry(101));
VERIFY_ARE_EQUAL(RGB(0, 255, 0), termRenderSettings.GetColorTableEntry(102));
VERIFY_ARE_EQUAL(RGB(0, 0, 255), termRenderSettings.GetColorTableEntry(103));

termRenderSettings.ResetColorTable();
}

void ConptyRoundtripTests::OutputWrappedLinesAtTopOfBuffer()
{
Log::Comment(
Expand Down
51 changes: 51 additions & 0 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2672,6 +2672,13 @@ ITermDispatch::StringHandler AdaptDispatch::RestoreTerminalState(const DispatchT
// - a function to parse the report data.
ITermDispatch::StringHandler AdaptDispatch::_RestoreColorTable()
{
// If we're a conpty, we create a passthrough string handler to forward the
// color report to the connected terminal.
if (_api.IsConsolePty())
{
return _CreatePassthroughHandler();
}

return [this, parameter = VTInt{}, parameters = std::vector<VTParameter>{}](const auto ch) mutable {
if (ch >= L'0' && ch <= L'9')
{
Expand Down Expand Up @@ -2884,3 +2891,47 @@ bool AdaptDispatch::PlaySounds(const VTParameters parameters)
return true;
});
}

// Routine Description:
// - Helper method to create a string handler that can be used to pass through
// DCS sequences when in conpty mode.
// Arguments:
// - <none>
// Return value:
// - a function to receive the data or nullptr if the initial flush fails
ITermDispatch::StringHandler AdaptDispatch::_CreatePassthroughHandler()
{
// Before we pass through any more data, we need to flush the current frame
// first, otherwise it can end up arriving out of sync.
_renderer.TriggerFlush(false);
// Then we need to flush the sequence introducer and parameters that have
// already been parsed by the state machine.
auto& stateMachine = _api.GetStateMachine();
if (stateMachine.FlushToTerminal())
{
// And finally we create a StringHandler to receive the rest of the
// sequence data, and pass it through to the connected terminal.
auto& engine = stateMachine.Engine();
return [&, buffer = std::wstring{}](const auto ch) mutable {
// To make things more efficient, we buffer the string data before
// passing it through, only flushing if the buffer gets too large,
// or we're dealing with the last character in the current output
// fragment, or we've reached the end of the string.
const auto endOfString = ch == AsciiChars::ESC;
buffer += ch;
if (buffer.length() >= 4096 || stateMachine.IsProcessingLastCharacter() || endOfString)
{
// The end of the string is signaled with an escape, but for it
// to be a valid string terminator we need to add a backslash.
if (endOfString)
{
buffer += L'\\';
}
engine.ActionPassThroughString(buffer);
buffer.clear();
}
return !endOfString;
};
}
return nullptr;
}
2 changes: 2 additions & 0 deletions src/terminal/adapter/adaptDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ namespace Microsoft::Console::VirtualTerminal
void _ReportSGRSetting() const;
void _ReportDECSTBMSetting();

StringHandler _CreatePassthroughHandler();

std::vector<bool> _tabStopColumns;
bool _initDefaultTabStops = true;

Expand Down
26 changes: 24 additions & 2 deletions src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,7 @@ void StateMachine::_EnterDcsParam() noexcept
void StateMachine::_EnterDcsIgnore() noexcept
{
_state = VTStates::DcsIgnore;
_cachedSequence.reset();
_trace.TraceStateChange(L"DcsIgnore");
}

Expand Down Expand Up @@ -950,6 +951,7 @@ void StateMachine::_EnterDcsIntermediate() noexcept
void StateMachine::_EnterDcsPassThrough() noexcept
{
_state = VTStates::DcsPassThrough;
_cachedSequence.reset();
_trace.TraceStateChange(L"DcsPassThrough");
}

Expand All @@ -966,6 +968,7 @@ void StateMachine::_EnterDcsPassThrough() noexcept
void StateMachine::_EnterSosPmApcString() noexcept
{
_state = VTStates::SosPmApcString;
_cachedSequence.reset();
_trace.TraceStateChange(L"SosPmApcString");
}

Expand Down Expand Up @@ -1843,6 +1846,8 @@ void StateMachine::ProcessString(const std::wstring_view string)

if (_processingIndividually)
{
// Note whether we're dealing with the last character in the buffer.
_processingLastCharacter = (current + 1 >= string.size());
// If we're processing characters individually, send it to the state machine.
ProcessCharacter(til::at(string, current));
++current;
Expand Down Expand Up @@ -1921,6 +1926,7 @@ void StateMachine::ProcessString(const std::wstring_view string)
{
// Reset our state, and put all but the last char in again.
ResetState();
_processingLastCharacter = false;
// Chars to flush are [pwchSequenceStart, pwchCurr)
auto wchIter = run.cbegin();
while (wchIter < run.cend() - 1)
Expand All @@ -1929,6 +1935,7 @@ void StateMachine::ProcessString(const std::wstring_view string)
wchIter++;
}
// Manually execute the last char [pwchCurr]
_processingLastCharacter = true;
switch (_state)
{
case VTStates::Ground:
Expand Down Expand Up @@ -1958,11 +1965,13 @@ void StateMachine::ProcessString(const std::wstring_view string)
// after dispatching the characters
_EnterGround();
}
else
else if (_state != VTStates::SosPmApcString && _state != VTStates::DcsPassThrough && _state != VTStates::DcsIgnore)
{
// If the engine doesn't require flushing at the end of the string, we
// want to cache the partial sequence in case we have to flush the whole
// thing to the terminal later.
// thing to the terminal later. There is no need to do this if we've
// reached one of the string processing states, though, since that data
// will be dealt with as soon as it is received.
if (!_cachedSequence)
{
_cachedSequence.emplace(std::wstring{});
Expand All @@ -1974,6 +1983,19 @@ void StateMachine::ProcessString(const std::wstring_view string)
}
}

// Routine Description:
// - Determines whether the character being processed is the last in the
// current output fragment, or there are more still to come. Other parts
// of the framework can use this information to work more efficiently.
// Arguments:
// - <none>
// Return Value:
// - True if we're processing the last character. False if not.
bool StateMachine::IsProcessingLastCharacter() const noexcept
{
return _processingLastCharacter;
}

// Routine Description:
// - Wherever the state machine is, whatever it's going, go back to ground.
// This is used by conhost to "jiggle the handle" - when VT support is
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/parser/stateMachine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace Microsoft::Console::VirtualTerminal

void ProcessCharacter(const wchar_t wch);
void ProcessString(const std::wstring_view string);
bool IsProcessingLastCharacter() const noexcept;

void ResetState() noexcept;

Expand Down Expand Up @@ -199,5 +200,6 @@ namespace Microsoft::Console::VirtualTerminal
// This is tracked per state machine instance so that separate calls to Process*
// can start and finish a sequence.
bool _processingIndividually;
bool _processingLastCharacter;
};
}

0 comments on commit c754f4d

Please sign in to comment.