Skip to content

Commit 703e191

Browse files
committed
Revert "ConPTY: Emit DSR CPR on resize (#19089)"
This reverts commit c55aca5 because it is unaware of the VT state and may inject the DSR CPR while e.g. a DCS is going on.
1 parent e2f3e53 commit 703e191

File tree

8 files changed

+68
-93
lines changed

8 files changed

+68
-93
lines changed

src/host/PtySignalInputThread.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,6 @@ void PtySignalInputThread::_DoResizeWindow(const ResizeWindowData& data)
184184
}
185185

186186
_api.ResizeWindow(data.sx, data.sy);
187-
188-
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
189-
gci.GetVtIo()->RequestCursorPositionFromTerminal();
190187
}
191188

192189
void PtySignalInputThread::_DoClearBuffer(const bool keepCursorRow) const

src/host/VtInputThread.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ using namespace Microsoft::Console::VirtualTerminal;
2121
// - hPipe - a handle to the file representing the read end of the VT pipe.
2222
// - inheritCursor - a bool indicating if the state machine should expect a
2323
// cursor positioning sequence. See MSFT:15681311.
24-
VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, std::function<void()> capturedCPR) :
24+
VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor) :
2525
_hFile{ std::move(hPipe) }
2626
{
2727
THROW_HR_IF(E_HANDLE, _hFile.get() == INVALID_HANDLE_VALUE);
2828

2929
auto dispatch = std::make_unique<InteractDispatch>();
30-
auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), std::move(capturedCPR));
30+
auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), inheritCursor);
3131
_pInputStateMachine = std::make_unique<StateMachine>(std::move(engine));
3232
}
3333

@@ -185,7 +185,8 @@ void VtInputThread::_InputThread()
185185
return S_OK;
186186
}
187187

188-
InputStateMachineEngine& VtInputThread::GetInputStateMachineEngine() const noexcept
188+
til::enumset<DeviceAttribute, uint64_t> VtInputThread::WaitUntilDA1(DWORD timeout) const noexcept
189189
{
190-
return static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
190+
const auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
191+
return engine.WaitUntilDA1(timeout);
191192
}

src/host/VtInputThread.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,16 @@ namespace Microsoft::Console
2020
{
2121
namespace VirtualTerminal
2222
{
23-
class InputStateMachineEngine;
2423
enum class DeviceAttribute : uint64_t;
2524
}
2625

2726
class VtInputThread
2827
{
2928
public:
30-
VtInputThread(_In_ wil::unique_hfile hPipe, std::function<void()> capturedCPR = nullptr);
29+
VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor);
3130

3231
[[nodiscard]] HRESULT Start();
33-
VirtualTerminal::InputStateMachineEngine& GetInputStateMachineEngine() const noexcept;
32+
til::enumset<VirtualTerminal::DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
3433

3534
private:
3635
static DWORD WINAPI StaticVtInputThreadProc(_In_ LPVOID lpParameter);

src/host/VtIo.cpp

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "output.h" // CloseConsoleProcessState
1212
#include "../interactivity/inc/ServiceLocator.hpp"
1313
#include "../renderer/base/renderer.hpp"
14-
#include "../terminal/parser/InputStateMachineEngine.hpp"
1514
#include "../types/inc/CodepointWidthDetector.hpp"
1615
#include "../types/inc/utils.hpp"
1716

@@ -156,9 +155,7 @@ bool VtIo::IsUsingVt() const
156155
{
157156
if (IsValidHandle(_hInput.get()))
158157
{
159-
_pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput), [this]() {
160-
_cursorPositionReportReceived();
161-
});
158+
_pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput), _lookingForCursorPosition);
162159
}
163160
}
164161
CATCH_RETURN();
@@ -180,7 +177,6 @@ bool VtIo::IsUsingVt() const
180177
// wait for the DA1 response below and effectively wait for both.
181178
if (_lookingForCursorPosition)
182179
{
183-
_pVtInputThread->GetInputStateMachineEngine().CaptureNextCPR();
184180
writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR)
185181
}
186182

@@ -201,7 +197,7 @@ bool VtIo::IsUsingVt() const
201197
// Allow the input thread to momentarily gain the console lock.
202198
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
203199
const auto suspension = gci.SuspendLock();
204-
_deviceAttributes = _pVtInputThread->GetInputStateMachineEngine().WaitUntilDA1(3000);
200+
_deviceAttributes = _pVtInputThread->WaitUntilDA1(3000);
205201
}
206202
}
207203

@@ -252,35 +248,6 @@ void VtIo::Shutdown() noexcept
252248
CATCH_LOG();
253249
}
254250

255-
void VtIo::RequestCursorPositionFromTerminal()
256-
{
257-
if (_lookingForCursorPosition)
258-
{
259-
// By delaying sending another DSR CPR until we received a response to the previous one,
260-
// we debounce our requests to the terminal. We don't want to flood it unnecessarily.
261-
_scheduleAnotherCPR = true;
262-
return;
263-
}
264-
265-
_lookingForCursorPosition = true;
266-
_pVtInputThread->GetInputStateMachineEngine().CaptureNextCPR();
267-
268-
Writer writer{ this };
269-
writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR)
270-
writer.Submit();
271-
}
272-
273-
void VtIo::_cursorPositionReportReceived()
274-
{
275-
_lookingForCursorPosition = false;
276-
277-
if (_scheduleAnotherCPR)
278-
{
279-
_scheduleAnotherCPR = false;
280-
RequestCursorPositionFromTerminal();
281-
}
282-
}
283-
284251
void VtIo::SetDeviceAttributes(const til::enumset<DeviceAttribute, uint64_t> attributes) noexcept
285252
{
286253
_deviceAttributes = attributes;

src/host/VtIo.hpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ namespace Microsoft::Console::VirtualTerminal
6161
[[nodiscard]] HRESULT StartIfNeeded();
6262
void Shutdown() noexcept;
6363

64-
void RequestCursorPositionFromTerminal();
6564
void SetDeviceAttributes(til::enumset<DeviceAttribute, uint64_t> attributes) noexcept;
6665
til::enumset<DeviceAttribute, uint64_t> GetDeviceAttributes() const noexcept;
6766
void SendCloseEvent();
@@ -78,7 +77,7 @@ namespace Microsoft::Console::VirtualTerminal
7877
};
7978

8079
[[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, _In_opt_ const HANDLE SignalHandle);
81-
void _cursorPositionReportReceived();
80+
8281
void _uncork();
8382
void _flushNow();
8483

@@ -106,7 +105,6 @@ namespace Microsoft::Console::VirtualTerminal
106105

107106
State _state = State::Uninitialized;
108107
bool _lookingForCursorPosition = false;
109-
bool _scheduleAnotherCPR = false;
110108
bool _closeEventSent = false;
111109
int _corked = 0;
112110

src/terminal/parser/InputStateMachineEngine.cpp

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -89,24 +89,14 @@ static bool operator==(const Ss3ToVkey& pair, const Ss3ActionCodes code) noexcep
8989
return pair.action == code;
9090
}
9191

92-
InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, std::function<void()> capturedCPR) :
93-
_pDispatch{ std::move(pDispatch) },
94-
_capturedCPR{ std::move(capturedCPR) },
95-
_doubleClickTime{ std::chrono::milliseconds(GetDoubleClickTime()) }
92+
InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, const bool lookingForDSR) :
93+
_pDispatch(std::move(pDispatch)),
94+
_lookingForDSR(lookingForDSR),
95+
_doubleClickTime(std::chrono::milliseconds(GetDoubleClickTime()))
9696
{
9797
THROW_HR_IF_NULL(E_INVALIDARG, _pDispatch.get());
9898
}
9999

100-
IInteractDispatch& InputStateMachineEngine::GetDispatch() const noexcept
101-
{
102-
return *_pDispatch.get();
103-
}
104-
105-
void InputStateMachineEngine::CaptureNextCPR() noexcept
106-
{
107-
_lookingForCPR = true;
108-
}
109-
110100
til::enumset<DeviceAttribute, uint64_t> InputStateMachineEngine::WaitUntilDA1(DWORD timeout) const noexcept
111101
{
112102
uint64_t val = 0;
@@ -423,19 +413,13 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
423413
// The F3 case is special - it shares a code with the DeviceStatusResponse.
424414
// If we're looking for that response, then do that, and break out.
425415
// Else, fall though to the _GetCursorKeysModifierState handler.
426-
if (_lookingForCPR)
416+
if (_lookingForDSR)
427417
{
428-
_lookingForCPR = false;
429-
_capturedCPR();
430-
431-
const auto y = parameters.at(0).value();
432-
const auto x = parameters.at(1).value();
433-
434-
if (y > 0 && x > 0)
435-
{
436-
_pDispatch->MoveCursor(y, x);
437-
return true;
438-
}
418+
_pDispatch->MoveCursor(parameters.at(0), parameters.at(1));
419+
// Right now we're only looking for on initial cursor
420+
// position response. After that, only look for F3.
421+
_lookingForDSR = false;
422+
return true;
439423
}
440424
// Heuristic: If the hosting terminal used the win32 input mode, chances are high
441425
// that this is a CPR requested by the terminal application as opposed to a F3 key.
@@ -507,6 +491,10 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
507491

508492
_deviceAttributes.fetch_or(attributes.bits(), std::memory_order_relaxed);
509493
til::atomic_notify_all(_deviceAttributes);
494+
495+
// VtIo first sends a DSR CPR and then a DA1 request.
496+
// If we encountered a DA1 response here, the DSR request is definitely done now.
497+
_lookingForDSR = false;
510498
return true;
511499
}
512500
return false;

src/terminal/parser/InputStateMachineEngine.hpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,8 @@ namespace Microsoft::Console::VirtualTerminal
159159
class InputStateMachineEngine : public IStateMachineEngine
160160
{
161161
public:
162-
InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, std::function<void()> capturedCPR = nullptr);
162+
InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, const bool lookingForDSR = false);
163163

164-
IInteractDispatch& GetDispatch() const noexcept;
165-
void CaptureNextCPR() noexcept;
166164
til::enumset<DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
167165

168166
bool EncounteredWin32InputModeSequence() const noexcept override;
@@ -191,8 +189,7 @@ namespace Microsoft::Console::VirtualTerminal
191189
private:
192190
const std::unique_ptr<IInteractDispatch> _pDispatch;
193191
std::atomic<uint64_t> _deviceAttributes{ 0 };
194-
bool _lookingForCPR = false;
195-
std::function<void()> _capturedCPR;
192+
bool _lookingForDSR = false;
196193
bool _encounteredWin32InputModeSequence = false;
197194
bool _expectingStringTerminator = false;
198195
DWORD _mouseButtonState = 0;

0 commit comments

Comments
 (0)