Skip to content

Commit

Permalink
Refactor TerminalDispatch (graphics) to match AdaptDispatch (#6728)
Browse files Browse the repository at this point in the history
This is essentially a rewrite of the
`TerminalDispatch::SetGraphicsRendition` method, bringing it into closer
alignment with the `AdaptDispatch` implementation, simplifying the
`ITerminalApi` interface, and making the code easier to extend. It adds
support for a number of attributes which weren't previously implemented.

REFERENCES

* This is a mirror of the `AdaptDispatch` refactoring in PR #5758.
* The closer alignment with `AdaptDispatch` is a small step towards
  solving issue #3849.
* The newly supported attributes should help a little with issues #5461
  (italics) and #6205 (strike-through).

DETAILS

I've literally copied and pasted the `SetGraphicsRendition`
implementation from `AdaptDispatch` into `TerminalDispatch`, with only
few minor changes:

* The `SetTextAttribute` and `GetTextAttribute` calls are slightly
  different in the `TerminalDispatch` version, since they don't return a
  pointless `success` value, and in the case of the getter, the
  `TextAttribute` is returned directly instead of by reference.
  Ultimately I'd like to move the `AdaptDispatch` code towards that way
  of doing things too, but I'd like to deal with that later as part of a
  wider refactoring of the `ConGetSet` interface.
* The `SetIndexedForeground256` and `SetIndexedBackground256` calls
  required the color indices to be remapped in the `AdaptDispatch`
  implementation, because the conhost color table is in a different
  order to the XTerm standard. `TerminalDispatch` doesn't have that
  problem, so doesn't require the mapping.
* The index color constants used in the 16-color `SetIndexedForeground`
  and `SetIndexedBackground` calls are also slightly different for the
  same reason.

VALIDATION

I cherry-picked this code on top of the #6506 and #6698 PRs, since
that's only way to really get the different color formats passed-through
to the terminal. I then ran a bunch of manual tests with various color
coverage scripts that I have, and confirmed that all the different color
formats were being rendered as expected.

Closes #6725
  • Loading branch information
j4james authored Jul 1, 2020
1 parent ddbe370 commit 6b43ace
Show file tree
Hide file tree
Showing 5 changed files with 229 additions and 372 deletions.
12 changes: 3 additions & 9 deletions src/cascadia/TerminalCore/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#pragma once

#include "../../terminal/adapter/DispatchTypes.hpp"
#include "../../buffer/out/TextAttribute.hpp"

namespace Microsoft::Terminal::Core
{
Expand All @@ -18,15 +19,8 @@ namespace Microsoft::Terminal::Core
virtual bool PrintString(std::wstring_view string) noexcept = 0;
virtual bool ExecuteChar(wchar_t wch) noexcept = 0;

virtual bool SetTextToDefaults(bool foreground, bool background) noexcept = 0;
virtual bool SetTextForegroundIndex(BYTE colorIndex) noexcept = 0;
virtual bool SetTextBackgroundIndex(BYTE colorIndex) noexcept = 0;
virtual bool SetTextForegroundIndex256(BYTE colorIndex) noexcept = 0;
virtual bool SetTextBackgroundIndex256(BYTE colorIndex) noexcept = 0;
virtual bool SetTextRgbColor(COLORREF color, bool foreground) noexcept = 0;
virtual bool BoldText(bool boldOn) noexcept = 0;
virtual bool UnderlineText(bool underlineOn) noexcept = 0;
virtual bool ReverseText(bool reversed) noexcept = 0;
virtual TextAttribute GetTextAttributes() const noexcept = 0;
virtual void SetTextAttributes(const TextAttribute& attrs) noexcept = 0;

virtual bool SetCursorPosition(short x, short y) noexcept = 0;
virtual COORD GetCursorPosition() noexcept = 0;
Expand Down
11 changes: 2 additions & 9 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,8 @@ class Microsoft::Terminal::Core::Terminal final :
// These methods are defined in TerminalApi.cpp
bool PrintString(std::wstring_view stringView) noexcept override;
bool ExecuteChar(wchar_t wch) noexcept override;
bool SetTextToDefaults(bool foreground, bool background) noexcept override;
bool SetTextForegroundIndex(BYTE colorIndex) noexcept override;
bool SetTextBackgroundIndex(BYTE colorIndex) noexcept override;
bool SetTextForegroundIndex256(BYTE colorIndex) noexcept override;
bool SetTextBackgroundIndex256(BYTE colorIndex) noexcept override;
bool SetTextRgbColor(COLORREF color, bool foreground) noexcept override;
bool BoldText(bool boldOn) noexcept override;
bool UnderlineText(bool underlineOn) noexcept override;
bool ReverseText(bool reversed) noexcept override;
TextAttribute GetTextAttributes() const noexcept override;
void SetTextAttributes(const TextAttribute& attrs) noexcept override;
bool SetCursorPosition(short x, short y) noexcept override;
COORD GetCursorPosition() noexcept override;
bool SetCursorVisibility(const bool visible) noexcept override;
Expand Down
75 changes: 3 additions & 72 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,83 +26,14 @@ try
}
CATCH_LOG_RETURN_FALSE()

bool Terminal::SetTextToDefaults(bool foreground, bool background) noexcept
TextAttribute Terminal::GetTextAttributes() const noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
if (foreground)
{
attrs.SetDefaultForeground();
}
if (background)
{
attrs.SetDefaultBackground();
}
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextForegroundIndex(BYTE colorIndex) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedForeground(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextBackgroundIndex(BYTE colorIndex) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedBackground(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextForegroundIndex256(BYTE colorIndex) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedForeground256(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
return _buffer->GetCurrentAttributes();
}

bool Terminal::SetTextBackgroundIndex256(BYTE colorIndex) noexcept
void Terminal::SetTextAttributes(const TextAttribute& attrs) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedBackground256(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextRgbColor(COLORREF color, bool foreground) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetColor(color, foreground);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::BoldText(bool boldOn) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetBold(boldOn);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::UnderlineText(bool underlineOn) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetUnderline(underlineOn);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::ReverseText(bool reversed) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetReverseVideo(reversed);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetCursorPosition(short x, short y) noexcept
Expand Down
8 changes: 3 additions & 5 deletions src/cascadia/TerminalCore/TerminalDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,9 @@ class TerminalDispatch : public Microsoft::Console::VirtualTerminal::TermDispatc
private:
::Microsoft::Terminal::Core::ITerminalApi& _terminalApi;

bool _SetRgbColorsHelper(const std::basic_string_view<::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions> options,
size_t& optionsConsumed) noexcept;
bool _SetBoldColorHelper(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions opt) noexcept;
bool _SetDefaultColorHelper(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions opt) noexcept;
void _SetGraphicsOptionHelper(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions opt) noexcept;
size_t _SetRgbColorsHelper(const std::basic_string_view<::Microsoft::Console::VirtualTerminal::DispatchTypes::GraphicsOptions> options,
TextAttribute& attr,
const bool isForeground) noexcept;

bool _SetResetPrivateModes(const std::basic_string_view<::Microsoft::Console::VirtualTerminal::DispatchTypes::PrivateModeParams> params, const bool enable) noexcept;
bool _PrivateModeParamsHelper(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::PrivateModeParams param, const bool enable) noexcept;
Expand Down
Loading

0 comments on commit 6b43ace

Please sign in to comment.