Skip to content

Commit

Permalink
WIP: Expose current TextAttributes from the underlying console API.
Browse files Browse the repository at this point in the history
This is not a polished PR that is ready to merge; it demonstrates a
direction for which I need to get buy-off from the team before pursuing
further.

I'm currently working on implementing the XTPUSHSGR/XTPOPSGR control
sequences (WIP PR [here](microsoft#1978), which requires saving a [stack of] text
attributes, and not just the legacy attributes, but full RGB colors,
etc.

My first instinct was to implement the "business logic" (the stack) in
the `AdaptDispatch` layer, but that will require getting the [full] text
attributes from the underlying console API. This is not a *terribly*
"invasive" change, but it is exposing new stuff at a layer boundary.

Put another way, this means pound-including the
"../buffer/out/TextAttribute.hpp" header in a few places outside of
"buffer/out", and is that okay, or does that header need to be kept
private and isolated?

So there are a few ways this could go:

1. You folks might say "ugh, no!", and:
   1. I could push (haha) the business logic of pushing and popping text
      attributes down another layer (so `AdaptDispatch` just forwards on
      the [PushGraphicsRendition](https://github.com/microsoft/terminal/blob/0af275b9cb68da14f38f05b2cdcbb35da99cb17c/src/terminal/adapter/adaptDispatchGraphics.cpp#L452) call to the lower layer, OR
   2. You suggest a different, cleaner way of exposing the text
      attributes.

OR

2. Maybe you think this general direction is fine, but maybe you have
   some particular requests that I do certain things differently (I
   totally understand being picky about stuff that cuts across API
   layers).

What do you think?

Related: PR microsoft#1978
Related: Issue microsoft#1796
  • Loading branch information
jazzdelightsme committed Jul 19, 2019
1 parent eae920e commit 14e3bc4
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 15 deletions.
8 changes: 7 additions & 1 deletion src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1651,7 +1651,7 @@ void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo,
// - pwAttributes - Pointer to space that will receive color attributes data
// Return Value:
// - STATUS_SUCCESS if we succeeded or STATUS_INVALID_PARAMETER for bad params (nullptr).
[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferAttributes(_In_ const SCREEN_INFORMATION& screenInfo, _Out_ WORD* const pwAttributes)
[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferLegacyAttributes(_In_ const SCREEN_INFORMATION& screenInfo, _Out_ WORD* const pwAttributes)
{
NTSTATUS Status = STATUS_SUCCESS;

Expand All @@ -1668,6 +1668,12 @@ void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo,
return Status;
}

void DoSrvPrivateGetConsoleScreenBufferAttributes(_In_ const SCREEN_INFORMATION& screenInfo, TextAttribute& attributes)
{
attributes = screenInfo.GetActiveBuffer().GetAttributes();
}


// Routine Description:
// - A private API call for forcing the renderer to repaint the screen. If the
// input screen buffer is not the active one, then just do nothing. We only
Expand Down
6 changes: 4 additions & 2 deletions src/host/getset.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Revision History:

#pragma once
#include "../inc/conattrs.hpp"
#include "../buffer/out/TextAttribute.hpp"
class SCREEN_INFORMATION;

void DoSrvPrivateSetLegacyAttributes(SCREEN_INFORMATION& screenInfo,
Expand Down Expand Up @@ -67,8 +68,9 @@ void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo,
void DoSrvSetCursorColor(SCREEN_INFORMATION& screenInfo,
const COLORREF cursorColor);

[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferAttributes(const SCREEN_INFORMATION& screenInfo,
_Out_ WORD* const pwAttributes);
[[nodiscard]] NTSTATUS DoSrvPrivateGetConsoleScreenBufferLegacyAttributes(const SCREEN_INFORMATION& screenInfo,
_Out_ WORD* const pwAttributes);
void DoSrvPrivateGetConsoleScreenBufferAttributes(_In_ const SCREEN_INFORMATION& screenInfo, TextAttribute& attributes);

void DoSrvPrivateRefreshWindow(const SCREEN_INFORMATION& screenInfo);

Expand Down
16 changes: 14 additions & 2 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,9 +629,21 @@ BOOL ConhostInternalGetSet::SetCursorStyle(const CursorType cursorType)
// - pwAttributes - Pointer to space to receive color attributes data
// Return Value:
// - TRUE if successful. FALSE otherwise.
BOOL ConhostInternalGetSet::PrivateGetConsoleScreenBufferAttributes(_Out_ WORD* const pwAttributes)
BOOL ConhostInternalGetSet::PrivateGetConsoleScreenBufferLegacyAttributes(_Out_ WORD* const pwAttributes)
{
return NT_SUCCESS(DoSrvPrivateGetConsoleScreenBufferAttributes(_io.GetActiveOutputBuffer(), pwAttributes));
return NT_SUCCESS(DoSrvPrivateGetConsoleScreenBufferLegacyAttributes(_io.GetActiveOutputBuffer(), pwAttributes));
}

// Routine Description:
// - Retrieves the default color attributes information for the active screen buffer.
// - This function is used to optimize SGR calls in lieu of calling GetConsoleScreenBufferInfoEx.
// Arguments:
// - pAttributes - Pointer to space to receive color attributes data
// Return Value:
// - TRUE if successful. FALSE otherwise.
void ConhostInternalGetSet::PrivateGetConsoleScreenBufferAttributes(_Out_ TextAttribute& attributes)
{
DoSrvPrivateGetConsoleScreenBufferAttributes(_io.GetActiveOutputBuffer(), attributes);
}

// Routine Description:
Expand Down
3 changes: 2 additions & 1 deletion src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
BOOL PrivateEnableAlternateScroll(const bool fEnabled) override;
BOOL PrivateEraseAll() override;

BOOL PrivateGetConsoleScreenBufferAttributes(_Out_ WORD* const pwAttributes) override;
BOOL PrivateGetConsoleScreenBufferLegacyAttributes(_Out_ WORD* const pwAttributes) override;
void PrivateGetConsoleScreenBufferAttributes(_Out_ TextAttribute& attributes) override;

BOOL PrivatePrependConsoleInput(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& events,
_Out_ size_t& eventsWritten) override;
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/adaptDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchType
// because it has to fill the Largest Window Size by asking the OS and wastes time memcpying colors and other data
// we do not need to resolve this Set Graphics Rendition request.
WORD attr;
bool fSuccess = !!_conApi->PrivateGetConsoleScreenBufferAttributes(&attr);
bool fSuccess = !!_conApi->PrivateGetConsoleScreenBufferLegacyAttributes(&attr);

if (fSuccess)
{
Expand Down
4 changes: 3 additions & 1 deletion src/terminal/adapter/conGetSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Author(s):

#include "..\..\types\inc\IInputEvent.hpp"
#include "..\..\inc\conattrs.hpp"
#include "..\..\buffer\out\TextAttribute.hpp"

#include <deque>
#include <memory>
Expand Down Expand Up @@ -87,7 +88,8 @@ namespace Microsoft::Console::VirtualTerminal
virtual BOOL PrivateEraseAll() = 0;
virtual BOOL SetCursorStyle(const CursorType cursorType) = 0;
virtual BOOL SetCursorColor(const COLORREF cursorColor) = 0;
virtual BOOL PrivateGetConsoleScreenBufferAttributes(_Out_ WORD* const pwAttributes) = 0;
virtual BOOL PrivateGetConsoleScreenBufferLegacyAttributes(_Out_ WORD* const pwAttributes) = 0;
virtual void PrivateGetConsoleScreenBufferAttributes(_Out_ TextAttribute& attributes) = 0;
virtual BOOL PrivatePrependConsoleInput(_Inout_ std::deque<std::unique_ptr<IInputEvent>>& events,
_Out_ size_t& eventsWritten) = 0;
virtual BOOL PrivateWriteConsoleControlInput(_In_ KeyEvent key) = 0;
Expand Down
22 changes: 15 additions & 7 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,16 +689,24 @@ class TestGetSet final : public ConGetSet
return _fSetCursorColorResult;
}

BOOL PrivateGetConsoleScreenBufferAttributes(_Out_ WORD* const pwAttributes) override
BOOL PrivateGetConsoleScreenBufferLegacyAttributes(_Out_ WORD* const pwAttributes) override
{
Log::Comment(L"PrivateGetConsoleScreenBufferAttributes MOCK returning data...");
Log::Comment(L"PrivateGetConsoleScreenBufferLegacyAttributes MOCK returning data...");

if (pwAttributes != nullptr && _fPrivateGetConsoleScreenBufferAttributesResult)
if (pwAttributes != nullptr && _fPrivateGetConsoleScreenBufferLegacyAttributesResult)
{
*pwAttributes = _wAttribute;
}

return _fPrivateGetConsoleScreenBufferAttributesResult;
return _fPrivateGetConsoleScreenBufferLegacyAttributesResult;
}

void PrivateGetConsoleScreenBufferAttributes(_Out_ TextAttribute& attributes) override
{
Log::Comment(L"PrivateGetConsoleScreenBufferAttributes MOCK returning data...");

// TODO
attributes = TextAttribute();
}

BOOL PrivateRefreshWindow() override
Expand Down Expand Up @@ -871,7 +879,7 @@ class TestGetSet final : public ConGetSet
_fPrivateWriteConsoleControlInputResult = TRUE;
_fScrollConsoleScreenBufferWResult = TRUE;
_fSetConsoleWindowInfoResult = TRUE;
_fPrivateGetConsoleScreenBufferAttributesResult = TRUE;
_fPrivateGetConsoleScreenBufferLegacyAttributesResult = TRUE;
_fMoveToBottomResult = true;

_PrepCharsBuffer(wch, wAttr);
Expand Down Expand Up @@ -1394,7 +1402,7 @@ class TestGetSet final : public ConGetSet
BOOL _fSetConsoleXtermTextAttributeResult = false;
BOOL _fSetConsoleRGBTextAttributeResult = false;
BOOL _fPrivateSetLegacyAttributesResult = false;
BOOL _fPrivateGetConsoleScreenBufferAttributesResult = false;
BOOL _fPrivateGetConsoleScreenBufferLegacyAttributesResult = false;
BOOL _fSetCursorStyleResult = false;
CursorType _ExpectedCursorStyle;
BOOL _fSetCursorColorResult = false;
Expand Down Expand Up @@ -2505,7 +2513,7 @@ class AdapterTest
Log::Comment(L"Test 2: Gracefully fail when getting buffer information fails.");

_testGetSet->PrepData();
_testGetSet->_fPrivateGetConsoleScreenBufferAttributesResult = FALSE;
_testGetSet->_fPrivateGetConsoleScreenBufferLegacyAttributesResult = FALSE;

VERIFY_IS_FALSE(_pDispatch->SetGraphicsRendition(rgOptions, cOptions));

Expand Down

0 comments on commit 14e3bc4

Please sign in to comment.