Skip to content

Commit

Permalink
Replace WinRT clipboard API with Win32 for copying (#17006)
Browse files Browse the repository at this point in the history
In the spirit of #15360 this implements the copy part.
The problem is that we have an issue accessing the clipboard while
other applications continue to work just fine. The major difference
between us and the others is that we use the WinRT clipboard APIs.
So, the idea is that we just use the Win32 APIs instead.

The feel-good side-effect is that this is (no joke) 200-1000x faster,
but I suspect no one will notice the -3ms difference down to <0.01ms.

The objective effect however is that it just works.

This may resolve #16982.

## Validation Steps Performed
* Cycle through Text/HTML/RTF-only in the Interaction settings
* Paste the contents into Word each time
* Text is plain and HTML/RTF are colored ✅
  • Loading branch information
lhecker authored Apr 10, 2024
1 parent 20b0bed commit 5f3a857
Show file tree
Hide file tree
Showing 23 changed files with 113 additions and 135 deletions.
52 changes: 0 additions & 52 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1645,10 +1645,6 @@ namespace winrt::TerminalApp::implementation
{
term.RaiseNotice({ this, &TerminalPage::_ControlNoticeRaisedHandler });

// Add an event handler when the terminal's selection wants to be copied.
// When the text buffer data is retrieved, we'll copy the data into the Clipboard
term.CopyToClipboard({ this, &TerminalPage::_CopyToClipboardHandler });

// Add an event handler when the terminal wants to paste data from the Clipboard.
term.PasteFromClipboard({ this, &TerminalPage::_PasteFromClipboardHandler });

Expand Down Expand Up @@ -2556,54 +2552,6 @@ namespace winrt::TerminalApp::implementation
return dimension;
}

// Method Description:
// - Place `copiedData` into the clipboard as text. Triggered when a
// terminal control raises its CopyToClipboard event.
// Arguments:
// - copiedData: the new string content to place on the clipboard.
winrt::fire_and_forget TerminalPage::_CopyToClipboardHandler(const IInspectable /*sender*/,
const CopyToClipboardEventArgs copiedData)
{
co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::High);

auto dataPack = DataPackage();
dataPack.RequestedOperation(DataPackageOperation::Copy);

const auto copyFormats = copiedData.Formats() != nullptr ?
copiedData.Formats().Value() :
static_cast<CopyFormat>(0);

// copy text to dataPack
dataPack.SetText(copiedData.Text());

if (WI_IsFlagSet(copyFormats, CopyFormat::HTML))
{
// copy html to dataPack
const auto htmlData = copiedData.Html();
if (!htmlData.empty())
{
dataPack.SetHtmlFormat(htmlData);
}
}

if (WI_IsFlagSet(copyFormats, CopyFormat::RTF))
{
// copy rtf data to dataPack
const auto rtfData = copiedData.Rtf();
if (!rtfData.empty())
{
dataPack.SetRtf(rtfData);
}
}

try
{
Clipboard::SetContent(dataPack);
Clipboard::Flush();
}
CATCH_LOG();
}

static wil::unique_close_clipboard_call _openClipboard(HWND hwnd)
{
bool success = false;
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,6 @@ namespace winrt::TerminalApp::implementation
void _ScrollToBufferEdge(ScrollDirection scrollDirection);
void _SetAcceleratorForMenuItem(Windows::UI::Xaml::Controls::MenuFlyoutItem& menuItem, const winrt::Microsoft::Terminal::Control::KeyChord& keyChord);

winrt::fire_and_forget _CopyToClipboardHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::CopyToClipboardEventArgs copiedData);
winrt::fire_and_forget _PasteFromClipboardHandler(const IInspectable sender,
const Microsoft::Terminal::Control::PasteFromClipboardEventArgs eventArgs);

Expand Down
120 changes: 97 additions & 23 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1219,11 +1219,87 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_updateSelectionUI();
}

static wil::unique_close_clipboard_call _openClipboard(HWND hwnd)
{
bool success = false;

// OpenClipboard may fail to acquire the internal lock --> retry.
for (DWORD sleep = 10;; sleep *= 2)
{
if (OpenClipboard(hwnd))
{
success = true;
break;
}
// 10 iterations
if (sleep > 10000)
{
break;
}
Sleep(sleep);
}

return wil::unique_close_clipboard_call{ success };
}

static void _copyToClipboard(const UINT format, const void* src, const size_t bytes)
{
wil::unique_hglobal handle{ THROW_LAST_ERROR_IF_NULL(GlobalAlloc(GMEM_MOVEABLE, bytes)) };

const auto locked = GlobalLock(handle.get());
memcpy(locked, src, bytes);
GlobalUnlock(handle.get());

THROW_LAST_ERROR_IF_NULL(SetClipboardData(format, handle.get()));
handle.release();
}

static void _copyToClipboardRegisteredFormat(const wchar_t* format, const void* src, size_t bytes)
{
const auto id = RegisterClipboardFormatW(format);
if (!id)
{
LOG_LAST_ERROR();
return;
}
_copyToClipboard(id, src, bytes);
}

static void copyToClipboard(wil::zwstring_view text, std::string_view html, std::string_view rtf)
{
const auto clipboard = _openClipboard(nullptr);
if (!clipboard)
{
LOG_LAST_ERROR();
return;
}

EmptyClipboard();

if (!text.empty())
{
// As per: https://learn.microsoft.com/en-us/windows/win32/dataxchg/standard-clipboard-formats
// CF_UNICODETEXT: [...] A null character signals the end of the data.
// --> We add +1 to the length. This works because .c_str() is null-terminated.
_copyToClipboard(CF_UNICODETEXT, text.c_str(), (text.size() + 1) * sizeof(wchar_t));
}

if (!html.empty())
{
_copyToClipboardRegisteredFormat(L"HTML Format", html.data(), html.size());
}

if (!rtf.empty())
{
_copyToClipboardRegisteredFormat(L"Rich Text Format", rtf.data(), rtf.size());
}
}

// Called when the Terminal wants to set something to the clipboard, i.e.
// when an OSC 52 is emitted.
void ControlCore::_terminalCopyToClipboard(std::wstring_view wstr)
void ControlCore::_terminalCopyToClipboard(wil::zwstring_view wstr)
{
CopyToClipboard.raise(*this, winrt::make<implementation::CopyToClipboardEventArgs>(winrt::hstring{ wstr }));
copyToClipboard(wstr, {}, {});
}

// Method Description:
Expand All @@ -1236,31 +1312,29 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool ControlCore::CopySelectionToClipboard(bool singleLine,
const Windows::Foundation::IReference<CopyFormat>& formats)
{
const auto lock = _terminal->LockForWriting();

// no selection --> nothing to copy
if (!_terminal->IsSelectionActive())
::Microsoft::Terminal::Core::Terminal::TextCopyData payload;
{
return false;
}
const auto lock = _terminal->LockForWriting();

// use action's copyFormatting if it's present, else fallback to globally
// set copyFormatting.
const auto copyFormats = formats != nullptr ? formats.Value() : _settings->CopyFormatting();
// no selection --> nothing to copy
if (!_terminal->IsSelectionActive())
{
return false;
}

const auto copyHtml = WI_IsFlagSet(copyFormats, CopyFormat::HTML);
const auto copyRtf = WI_IsFlagSet(copyFormats, CopyFormat::RTF);
// use action's copyFormatting if it's present, else fallback to globally
// set copyFormatting.
const auto copyFormats = formats != nullptr ? formats.Value() : _settings->CopyFormatting();

// extract text from buffer
// RetrieveSelectedTextFromBuffer will lock while it's reading
const auto& [textData, htmlData, rtfData] = _terminal->RetrieveSelectedTextFromBuffer(singleLine, copyHtml, copyRtf);

// send data up for clipboard
CopyToClipboard.raise(*this,
winrt::make<CopyToClipboardEventArgs>(winrt::hstring{ textData },
winrt::to_hstring(htmlData),
winrt::to_hstring(rtfData),
copyFormats));
const auto copyHtml = WI_IsFlagSet(copyFormats, CopyFormat::HTML);
const auto copyRtf = WI_IsFlagSet(copyFormats, CopyFormat::RTF);

// extract text from buffer
// RetrieveSelectedTextFromBuffer will lock while it's reading
payload = _terminal->RetrieveSelectedTextFromBuffer(singleLine, copyHtml, copyRtf);
}

copyToClipboard(payload.plainText, payload.html, payload.rtf);
return true;
}

Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// clang-format off
til::typed_event<IInspectable, Control::FontSizeChangedArgs> FontSizeChanged;

til::typed_event<IInspectable, Control::CopyToClipboardEventArgs> CopyToClipboard;
til::typed_event<IInspectable, Control::TitleChangedEventArgs> TitleChanged;
til::typed_event<> WarningBell;
til::typed_event<> TabColorChanged;
Expand Down Expand Up @@ -371,7 +370,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _sendInputToConnection(std::wstring_view wstr);

#pragma region TerminalCoreCallbacks
void _terminalCopyToClipboard(std::wstring_view wstr);
void _terminalCopyToClipboard(wil::zwstring_view wstr);
void _terminalWarningBell();
void _terminalTitleChanged(std::wstring_view wstr);
void _terminalScrollPositionChanged(const int viewTop,
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ namespace Microsoft.Terminal.Control
Boolean ShouldShowSelectOutput();

// These events are called from some background thread
event Windows.Foundation.TypedEventHandler<Object, CopyToClipboardEventArgs> CopyToClipboard;
event Windows.Foundation.TypedEventHandler<Object, TitleChangedEventArgs> TitleChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> WarningBell;
event Windows.Foundation.TypedEventHandler<Object, Object> TabColorChanged;
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/EventArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "EventArgs.h"
#include "FontSizeChangedArgs.g.cpp"
#include "TitleChangedEventArgs.g.cpp"
#include "CopyToClipboardEventArgs.g.cpp"
#include "ContextMenuRequestedEventArgs.g.cpp"
#include "PasteFromClipboardEventArgs.g.cpp"
#include "OpenHyperlinkEventArgs.g.cpp"
Expand Down
28 changes: 0 additions & 28 deletions src/cascadia/TerminalControl/EventArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include "FontSizeChangedArgs.g.h"
#include "TitleChangedEventArgs.g.h"
#include "CopyToClipboardEventArgs.g.h"
#include "ContextMenuRequestedEventArgs.g.h"
#include "PasteFromClipboardEventArgs.g.h"
#include "OpenHyperlinkEventArgs.g.h"
Expand Down Expand Up @@ -47,33 +46,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
WINRT_PROPERTY(hstring, Title);
};

struct CopyToClipboardEventArgs : public CopyToClipboardEventArgsT<CopyToClipboardEventArgs>
{
public:
CopyToClipboardEventArgs(hstring text) :
_text(text),
_html(),
_rtf(),
_formats(static_cast<CopyFormat>(0)) {}

CopyToClipboardEventArgs(hstring text, hstring html, hstring rtf, Windows::Foundation::IReference<CopyFormat> formats) :
_text(text),
_html(html),
_rtf(rtf),
_formats(formats) {}

hstring Text() { return _text; };
hstring Html() { return _html; };
hstring Rtf() { return _rtf; };
Windows::Foundation::IReference<CopyFormat> Formats() { return _formats; };

private:
hstring _text;
hstring _html;
hstring _rtf;
Windows::Foundation::IReference<CopyFormat> _formats;
};

struct ContextMenuRequestedEventArgs : public ContextMenuRequestedEventArgsT<ContextMenuRequestedEventArgs>
{
public:
Expand Down
8 changes: 0 additions & 8 deletions src/cascadia/TerminalControl/EventArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@ namespace Microsoft.Terminal.Control
Int32 Height { get; };
}

runtimeclass CopyToClipboardEventArgs
{
String Text { get; };
String Html { get; };
String Rtf { get; };
Windows.Foundation.IReference<CopyFormat> Formats { get; };
}

runtimeclass ContextMenuRequestedEventArgs
{
Windows.Foundation.Point Position { get; };
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_revokers.ContextMenuRequested = _interactivity.ContextMenuRequested(winrt::auto_revoke, { get_weak(), &TermControl::_contextMenuHandler });

// "Bubbled" events - ones we want to handle, by raising our own event.
_revokers.CopyToClipboard = _core.CopyToClipboard(winrt::auto_revoke, { get_weak(), &TermControl::_bubbleCopyToClipboard });
_revokers.TitleChanged = _core.TitleChanged(winrt::auto_revoke, { get_weak(), &TermControl::_bubbleTitleChanged });
_revokers.TabColorChanged = _core.TabColorChanged(winrt::auto_revoke, { get_weak(), &TermControl::_bubbleTabColorChanged });
_revokers.TaskbarProgressChanged = _core.TaskbarProgressChanged(winrt::auto_revoke, { get_weak(), &TermControl::_bubbleSetTaskbarProgress });
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// UNDER NO CIRCUMSTANCES SHOULD YOU ADD A (PROJECTED_)FORWARDED_TYPED_EVENT HERE
// Those attach the handler to the core directly, and will explode if
// the core ever gets detached & reattached to another window.
BUBBLED_FORWARDED_TYPED_EVENT(CopyToClipboard, IInspectable, Control::CopyToClipboardEventArgs);
BUBBLED_FORWARDED_TYPED_EVENT(TitleChanged, IInspectable, Control::TitleChangedEventArgs);
BUBBLED_FORWARDED_TYPED_EVENT(TabColorChanged, IInspectable, IInspectable);
BUBBLED_FORWARDED_TYPED_EVENT(SetTaskbarProgress, IInspectable, IInspectable);
Expand Down Expand Up @@ -398,7 +397,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
Control::ControlCore::FoundMatch_revoker FoundMatch;
Control::ControlCore::UpdateSelectionMarkers_revoker UpdateSelectionMarkers;
Control::ControlCore::OpenHyperlink_revoker coreOpenHyperlink;
Control::ControlCore::CopyToClipboard_revoker CopyToClipboard;
Control::ControlCore::TitleChanged_revoker TitleChanged;
Control::ControlCore::TabColorChanged_revoker TabColorChanged;
Control::ControlCore::TaskbarProgressChanged_revoker TaskbarProgressChanged;
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ namespace Microsoft.Terminal.Control
Microsoft.Terminal.Control.IControlSettings Settings { get; };

event Windows.Foundation.TypedEventHandler<Object, TitleChangedEventArgs> TitleChanged;
event Windows.Foundation.TypedEventHandler<Object, CopyToClipboardEventArgs> CopyToClipboard;
event Windows.Foundation.TypedEventHandler<Object, PasteFromClipboardEventArgs> PasteFromClipboard;
event Windows.Foundation.TypedEventHandler<Object, OpenHyperlinkEventArgs> OpenHyperlink;
event Windows.Foundation.TypedEventHandler<Object, Object> SetTaskbarProgress;
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,7 @@ void Terminal::SetTitleChangedCallback(std::function<void(std::wstring_view)> pf
_pfnTitleChanged.swap(pfn);
}

void Terminal::SetCopyToClipboardCallback(std::function<void(std::wstring_view)> pfn) noexcept
void Terminal::SetCopyToClipboardCallback(std::function<void(wil::zwstring_view)> pfn) noexcept
{
_pfnCopyToClipboard.swap(pfn);
}
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Microsoft::Terminal::Core::Terminal final :
bool ResizeWindow(const til::CoordType width, const til::CoordType height) noexcept override;
void SetConsoleOutputCP(const unsigned int codepage) noexcept override;
unsigned int GetConsoleOutputCP() const noexcept override;
void CopyToClipboard(std::wstring_view content) override;
void CopyToClipboard(wil::zwstring_view content) override;
void SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::TaskbarState state, const size_t progress) override;
void SetWorkingDirectory(std::wstring_view uri) override;
void PlayMidiNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) override;
Expand Down Expand Up @@ -225,7 +225,7 @@ class Microsoft::Terminal::Core::Terminal final :
void SetWriteInputCallback(std::function<void(std::wstring_view)> pfn) noexcept;
void SetWarningBellCallback(std::function<void()> pfn) noexcept;
void SetTitleChangedCallback(std::function<void(std::wstring_view)> pfn) noexcept;
void SetCopyToClipboardCallback(std::function<void(std::wstring_view)> pfn) noexcept;
void SetCopyToClipboardCallback(std::function<void(wil::zwstring_view)> pfn) noexcept;
void SetScrollPositionChangedCallback(std::function<void(const int, const int, const int)> pfn) noexcept;
void SetCursorPositionChangedCallback(std::function<void()> pfn) noexcept;
void TaskbarProgressChangedCallback(std::function<void()> pfn) noexcept;
Expand Down Expand Up @@ -324,7 +324,7 @@ class Microsoft::Terminal::Core::Terminal final :
std::function<void(std::wstring_view)> _pfnWriteInput;
std::function<void()> _pfnWarningBell;
std::function<void(std::wstring_view)> _pfnTitleChanged;
std::function<void(std::wstring_view)> _pfnCopyToClipboard;
std::function<void(wil::zwstring_view)> _pfnCopyToClipboard;

// I've specifically put this instance here as it requires
// alignas(std::hardware_destructive_interference_size)
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ unsigned int Terminal::GetConsoleOutputCP() const noexcept
return CP_UTF8;
}

void Terminal::CopyToClipboard(std::wstring_view content)
void Terminal::CopyToClipboard(wil::zwstring_view content)
{
_pfnCopyToClipboard(content);
}
Expand Down
2 changes: 1 addition & 1 deletion src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ unsigned int ConhostInternalGetSet::GetConsoleOutputCP() const
// - content - the text to be copied.
// Return Value:
// - <none>
void ConhostInternalGetSet::CopyToClipboard(const std::wstring_view /*content*/)
void ConhostInternalGetSet::CopyToClipboard(const wil::zwstring_view /*content*/)
{
// TODO
}
Expand Down
2 changes: 1 addition & 1 deletion src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
void SetConsoleOutputCP(const unsigned int codepage) override;
unsigned int GetConsoleOutputCP() const override;

void CopyToClipboard(const std::wstring_view content) override;
void CopyToClipboard(const wil::zwstring_view content) override;
void SetTaskbarProgress(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::TaskbarState state, const size_t progress) override;
void SetWorkingDirectory(const std::wstring_view uri) override;
void PlayMidiNote(const int noteNumber, const int velocity, const std::chrono::microseconds duration) override;
Expand Down
Loading

0 comments on commit 5f3a857

Please sign in to comment.