Skip to content

Commit

Permalink
Eliminate the IRenderTarget interface (#12568)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

The purpose of the `IRenderTarget` interface was to support the concept
of multiple buffers in conhost. When a text buffer needed to trigger a
redraw, the render target implementation would be responsible for
deciding whether to forward that redraw to the renderer, depending on
whether the buffer was active or not.

This PR instead introduces a flag in the `TextBuffer` class to track
whether it is active or not, and it can then simply check the flag to
decide whether it needs to trigger a redraw or not. That way it can work
with the `Renderer` directly, and we can avoid a bunch of virtual calls
for the various redraw methods.

## PR Checklist
* [x] Closes #12551
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12551

## Detailed Description of the Pull Request / Additional comments

Anywhere that had previously been getting an `IRenderTarget` from the
`TextBuffer` class in order to trigger a redraw, will now just call the
`TriggerRedraw` method on the `TextBuffer` class itself, since that will
handle the active check which used to be the responsibility of the
render target. All the redraw methods that were in `IRenderTarget` are
now exposed in `TextBuffer` instead.

For this to work, though, the `Renderer` needed to be available before
the `TextBuffer` could be constructed, which required a change in the
conhost initialization order. In the `ConsoleInitializeConnectInfo`
function, I had to move the `Renderer` initialization up so it could be
constructed before the call to `SetUpConsole` (which initializes the
buffer). Once both are ready, the `Renderer::EnablePainting` method is
called to start the render thread.

The other catch is that the renderer used to setup its initial viewport
in the constructor, but with the new initialization order, the viewport
size would not be known at that point. I've addressed that problem by
moving the viewport initialization into the `EnablePainting` method,
since that will only be called after the buffer is setup.

## Validation Steps Performed

The changes in architecture required a number of tweaks to the unit
tests. Some were using a dummy `IRenderTarget` implementation which now
needed to be replaced with a full `Renderer` (albeit with mostly null
parameters). In the case of the scroll test, a mock `IRenderTarget` was
used to track the scroll delta, which now had to be replaced with a mock
`RenderEngine` instead.

Some tests previously relied on having just a `TextBuffer` but without a
`Renderer`, and now they require both. And tests that were constructing
the `TextBuffer` and `Renderer` themselves had to be updated to use the
new initialization order, i.e. with the renderer constructed first.

Semantically, though, the tests still essentially work the same way as
they did before, and they all still pass.
  • Loading branch information
j4james authored Mar 14, 2022
1 parent bf551db commit ce6ce50
Show file tree
Hide file tree
Showing 45 changed files with 435 additions and 535 deletions.
2 changes: 1 addition & 1 deletion src/buffer/out/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void Cursor::_RedrawCursorAlways() noexcept
{
try
{
_parentBuffer.GetRenderTarget().TriggerRedrawCursor(&_cPosition);
_parentBuffer.TriggerRedrawCursor(_cPosition);
}
CATCH_LOG();
}
Expand Down
100 changes: 77 additions & 23 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "textBuffer.hpp"
#include "CharRow.hpp"

#include "../renderer/base/renderer.hpp"
#include "../types/inc/utils.hpp"
#include "../types/inc/convert.hpp"
#include "../../types/inc/Utf16Parser.hpp"
Expand All @@ -21,23 +22,26 @@ using PointTree = interval_tree::IntervalTree<til::point, size_t>;
// Routine Description:
// - Creates a new instance of TextBuffer
// Arguments:
// - fontInfo - The font to use for this text buffer as specified in the global font cache
// - screenBufferSize - The X by Y dimensions of the new screen buffer
// - fill - Uses the .Attributes property to decide which default color to apply to all text in this buffer
// - defaultAttributes - The attributes with which the buffer will be initialized
// - cursorSize - The height of the cursor within this buffer
// - isActiveBuffer - Whether this is the currently active buffer
// - renderer - The renderer to use for triggering a redraw
// Return Value:
// - constructed object
// Note: may throw exception
TextBuffer::TextBuffer(const COORD screenBufferSize,
const TextAttribute defaultAttributes,
const UINT cursorSize,
Microsoft::Console::Render::IRenderTarget& renderTarget) :
const bool isActiveBuffer,
Microsoft::Console::Render::Renderer& renderer) :
_firstRow{ 0 },
_currentAttributes{ defaultAttributes },
_cursor{ cursorSize, *this },
_storage{},
_unicodeStorage{},
_renderTarget{ renderTarget },
_isActiveBuffer{ isActiveBuffer },
_renderer{ renderer },
_size{},
_currentHyperlinkId{ 1 },
_currentPatternId{ 0 }
Expand Down Expand Up @@ -387,7 +391,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
// Take the cell distance written and notify that it needs to be repainted.
const auto written = newIt.GetCellDistance(givenIt);
const Viewport paint = Viewport::FromDimensions(target, { gsl::narrow<SHORT>(written), 1 });
_NotifyPaint(paint);
TriggerRedraw(paint);

return newIt;
}
Expand Down Expand Up @@ -556,7 +560,10 @@ bool TextBuffer::IncrementCircularBuffer(const bool inVtMode)
{
// FirstRow is at any given point in time the array index in the circular buffer that corresponds
// to the logical position 0 in the window (cursor coordinates and all other coordinates).
_renderTarget.TriggerCircling();
if (_isActiveBuffer)
{
_renderer.TriggerCircling();
}

// Prune hyperlinks to delete obsolete references
_PruneHyperlinks();
Expand Down Expand Up @@ -825,7 +832,7 @@ void TextBuffer::SetCurrentLineRendition(const LineRendition lineRendition)
// We also need to make sure the cursor is clamped within the new width.
GetCursor().SetPosition(ClampPositionWithinLine(cursorPosition));
}
_NotifyPaint(Viewport::FromDimensions({ 0, rowIndex }, { GetSize().Width(), 1 }));
TriggerRedraw(Viewport::FromDimensions({ 0, rowIndex }, { GetSize().Width(), 1 }));
}
}

Expand Down Expand Up @@ -953,6 +960,69 @@ UnicodeStorage& TextBuffer::GetUnicodeStorage() noexcept
return _unicodeStorage;
}

void TextBuffer::SetAsActiveBuffer(const bool isActiveBuffer) noexcept
{
_isActiveBuffer = isActiveBuffer;
}

bool TextBuffer::IsActiveBuffer() const noexcept
{
return _isActiveBuffer;
}

Microsoft::Console::Render::Renderer& TextBuffer::GetRenderer() noexcept
{
return _renderer;
}

void TextBuffer::TriggerRedraw(const Viewport& viewport)
{
if (_isActiveBuffer)
{
_renderer.TriggerRedraw(viewport);
}
}

void TextBuffer::TriggerRedrawCursor(const COORD position)
{
if (_isActiveBuffer)
{
_renderer.TriggerRedrawCursor(&position);
}
}

void TextBuffer::TriggerRedrawAll()
{
if (_isActiveBuffer)
{
_renderer.TriggerRedrawAll();
}
}

void TextBuffer::TriggerScroll()
{
if (_isActiveBuffer)
{
_renderer.TriggerScroll();
}
}

void TextBuffer::TriggerScroll(const COORD delta)
{
if (_isActiveBuffer)
{
_renderer.TriggerScroll(&delta);
}
}

void TextBuffer::TriggerNewTextNotification(const std::wstring_view newText)
{
if (_isActiveBuffer)
{
_renderer.TriggerNewTextNotification(newText);
}
}

// Routine Description:
// - Method to help refresh all the Row IDs after manipulating the row
// by shuffling pointers around.
Expand Down Expand Up @@ -989,11 +1059,6 @@ void TextBuffer::_RefreshRowIDs(std::optional<SHORT> newRowWidth)
_unicodeStorage.Remap(rowMap, newRowWidth);
}

void TextBuffer::_NotifyPaint(const Viewport& viewport) const
{
_renderTarget.TriggerRedraw(viewport);
}

// Routine Description:
// - Retrieves the first row from the underlying buffer.
// Arguments:
Expand Down Expand Up @@ -1026,17 +1091,6 @@ ROW& TextBuffer::_GetPrevRowNoWrap(const ROW& Row)
return _storage.at(prevRowIndex);
}

// Method Description:
// - Retrieves this buffer's current render target.
// Arguments:
// - <none>
// Return Value:
// - This buffer's current render target.
Microsoft::Console::Render::IRenderTarget& TextBuffer::GetRenderTarget() noexcept
{
return _renderTarget;
}

// Method Description:
// - get delimiter class for buffer cell position
// - used for double click selection and uia word navigation
Expand Down
27 changes: 20 additions & 7 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,19 @@ filling in the last row, and updating the screen.
#include "../buffer/out/textBufferCellIterator.hpp"
#include "../buffer/out/textBufferTextIterator.hpp"

#include "../renderer/inc/IRenderTarget.hpp"
namespace Microsoft::Console::Render
{
class Renderer;
}

class TextBuffer final
{
public:
TextBuffer(const COORD screenBufferSize,
const TextAttribute defaultAttributes,
const UINT cursorSize,
Microsoft::Console::Render::IRenderTarget& renderTarget);
const bool isActiveBuffer,
Microsoft::Console::Render::Renderer& renderer);
TextBuffer(const TextBuffer& a) = delete;

// Used for duplicating properties to another text buffer
Expand Down Expand Up @@ -139,7 +143,17 @@ class TextBuffer final
const UnicodeStorage& GetUnicodeStorage() const noexcept;
UnicodeStorage& GetUnicodeStorage() noexcept;

Microsoft::Console::Render::IRenderTarget& GetRenderTarget() noexcept;
void SetAsActiveBuffer(const bool isActiveBuffer) noexcept;
bool IsActiveBuffer() const noexcept;

Microsoft::Console::Render::Renderer& GetRenderer() noexcept;

void TriggerRedraw(const Microsoft::Console::Types::Viewport& viewport);
void TriggerRedrawCursor(const COORD position);
void TriggerRedrawAll();
void TriggerScroll();
void TriggerScroll(const COORD delta);
void TriggerNewTextNotification(const std::wstring_view newText);

const COORD GetWordStart(const COORD target, const std::wstring_view wordDelimiters, bool accessibilityMode = false, std::optional<til::point> limitOptional = std::nullopt) const;
const COORD GetWordEnd(const COORD target, const std::wstring_view wordDelimiters, bool accessibilityMode = false, std::optional<til::point> limitOptional = std::nullopt) const;
Expand Down Expand Up @@ -213,23 +227,22 @@ class TextBuffer final
// storage location for glyphs that can't fit into the buffer normally
UnicodeStorage _unicodeStorage;

bool _isActiveBuffer;
Microsoft::Console::Render::Renderer& _renderer;

std::unordered_map<uint16_t, std::wstring> _hyperlinkMap;
std::unordered_map<std::wstring, uint16_t> _hyperlinkCustomIdMap;
uint16_t _currentHyperlinkId;

void _RefreshRowIDs(std::optional<SHORT> newRowWidth);

Microsoft::Console::Render::IRenderTarget& _renderTarget;

void _SetFirstRowIndex(const SHORT FirstRowIndex) noexcept;

COORD _GetPreviousFromCursor() const;

void _SetWrapOnCurrentRow();
void _AdjustWrapOnCurrentRow(const bool fSet);

void _NotifyPaint(const Microsoft::Console::Types::Viewport& viewport) const;

// Assist with maintaining proper buffer state for Double Byte character sequences
bool _PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute);
bool _AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribute);
Expand Down
10 changes: 5 additions & 5 deletions src/buffer/out/ut_textbuffer/ReflowTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "../../inc/consoletaeftemplates.hpp"

#include "../textBuffer.hpp"
#include "../../renderer/inc/DummyRenderTarget.hpp"
#include "../../renderer/inc/DummyRenderer.hpp"
#include "../../types/inc/Utf16Parser.hpp"
#include "../../types/inc/GlyphWidth.hpp"

Expand Down Expand Up @@ -732,10 +732,10 @@ class ReflowTests
{
TEST_CLASS(ReflowTests);

static DummyRenderTarget target;
static DummyRenderer renderer;
static std::unique_ptr<TextBuffer> _textBufferFromTestBuffer(const TestBuffer& testBuffer)
{
auto buffer = std::make_unique<TextBuffer>(testBuffer.size, TextAttribute{ 0x7 }, 0, target);
auto buffer = std::make_unique<TextBuffer>(testBuffer.size, TextAttribute{ 0x7 }, 0, false, renderer);

size_t i{};
for (const auto& testRow : testBuffer.rows)
Expand Down Expand Up @@ -773,7 +773,7 @@ class ReflowTests

static std::unique_ptr<TextBuffer> _textBufferByReflowingTextBuffer(TextBuffer& originalBuffer, const COORD newSize)
{
auto buffer = std::make_unique<TextBuffer>(newSize, TextAttribute{ 0x7 }, 0, target);
auto buffer = std::make_unique<TextBuffer>(newSize, TextAttribute{ 0x7 }, 0, false, renderer);
TextBuffer::Reflow(originalBuffer, *buffer, std::nullopt, std::nullopt);
return buffer;
}
Expand Down Expand Up @@ -853,4 +853,4 @@ class ReflowTests
}
};

DummyRenderTarget ReflowTests::target{};
DummyRenderer ReflowTests::renderer{};
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1397,9 +1397,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();

auto& renderTarget = *_renderer;
auto& renderSettings = _terminal->GetRenderSettings();
renderSettings.ToggleBlinkRendition(renderTarget);
renderSettings.ToggleBlinkRendition(*_renderer);
}

void ControlCore::BlinkCursor()
Expand Down
Loading

0 comments on commit ce6ce50

Please sign in to comment.