Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial refactor of IRenderEngine interface #10615

Closed
wants to merge 11 commits into from
Closed

Initial refactor of IRenderEngine interface #10615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5a83be5
Initial refactor of IRenderEngine interface
skyline75489 Jul 11, 2021
e4b6add
No PatternId code in conhost
skyline75489 Jul 11, 2021
24ea604
Fix conhost overlay
skyline75489 Jul 11, 2021
8c9b6a1
Rm _CalculateRenderDataForDirtyLine
skyline75489 Jul 11, 2021
25a3d3c
Fix IME overlay in conhost
skyline75489 Jul 11, 2021
84f0128
Implement selection
skyline75489 Jul 12, 2021
643d788
Revert "Implement selection"
skyline75489 Jul 13, 2021
5429418
Merge branch 'main' into chesterliu/dev/re-renderer
skyline75489 Jul 13, 2021
f71d285
Another try
skyline75489 Jul 14, 2021
622f4fb
Merge branch 'main' into chesterliu/dev/re-renderer
skyline75489 Jul 22, 2021
13d386a
code format & spelling
skyline75489 Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_lastHoveredId = newId;
_lastHoveredInterval = newInterval;
_renderEngine->UpdateHyperlinkHoveredId(newId);
_renderer->UpdateLastHoveredInterval(newInterval);
_renderEngine->UpdateLastHoveredInterval(newInterval);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic example of how this PR decouples renderer.cpp & DxEngine.

Only DxEngine actually needs UpdateLastHoveredInterval. UpdateLastHoveredInterval never should be inside renderer.cpp.

_renderer->TriggerRedrawAll();
}

Expand Down
36 changes: 18 additions & 18 deletions src/host/ut_host/VtRendererTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ void VtRendererTest::Xterm256TestInvalidate()
Log::Comment(NoThrowString().Format(
L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences"));
COORD scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one down, only top line is invalid. ----"));
Expand All @@ -257,7 +257,7 @@ void VtRendererTest::Xterm256TestInvalidate()
});

scrollDelta = { 0, 3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));

TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
Expand All @@ -282,7 +282,7 @@ void VtRendererTest::Xterm256TestInvalidate()
});

scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one up, only bottom line is invalid. ----"));
Expand All @@ -299,7 +299,7 @@ void VtRendererTest::Xterm256TestInvalidate()
});

scrollDelta = { 0, -3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three up, only bottom 3 lines are invalid. ----"));
Expand Down Expand Up @@ -327,9 +327,9 @@ void VtRendererTest::Xterm256TestInvalidate()
L"Multiple scrolls are coalesced"));

scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
scrollDelta = { 0, 2 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
Expand All @@ -354,11 +354,11 @@ void VtRendererTest::Xterm256TestInvalidate()
});

scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
Log::Comment(engine->_invalidMap.to_string().c_str());

scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
Log::Comment(engine->_invalidMap.to_string().c_str());

qExpectedInput.push_back("\x1b[2J");
Expand Down Expand Up @@ -903,7 +903,7 @@ void VtRendererTest::XtermTestInvalidate()
Log::Comment(NoThrowString().Format(
L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences"));
COORD scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one down, only top line is invalid. ----"));
Expand All @@ -920,7 +920,7 @@ void VtRendererTest::XtermTestInvalidate()
});

scrollDelta = { 0, 3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
Expand All @@ -944,7 +944,7 @@ void VtRendererTest::XtermTestInvalidate()
});

scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled one up, only bottom line is invalid. ----"));
Expand All @@ -961,7 +961,7 @@ void VtRendererTest::XtermTestInvalidate()
});

scrollDelta = { 0, -3 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three up, only bottom 3 lines are invalid. ----"));
Expand Down Expand Up @@ -989,9 +989,9 @@ void VtRendererTest::XtermTestInvalidate()
L"Multiple scrolls are coalesced"));

scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
scrollDelta = { 0, 2 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
TestPaint(*engine, [&]() {
Log::Comment(NoThrowString().Format(
L"---- Scrolled three down, only top 3 lines are invalid. ----"));
Expand All @@ -1016,11 +1016,11 @@ void VtRendererTest::XtermTestInvalidate()
});

scrollDelta = { 0, 1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
Log::Comment(engine->_invalidMap.to_string().c_str());

scrollDelta = { 0, -1 };
VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta));
VERIFY_SUCCEEDED(engine->TriggerScroll(&scrollDelta));
Log::Comment(engine->_invalidMap.to_string().c_str());

qExpectedInput.push_back("\x1b[2J");
Expand Down Expand Up @@ -1417,7 +1417,7 @@ void VtRendererTest::TestResize()
// The renderer (in Renderer@_PaintFrameForEngine..._CheckViewportAndScroll)
// will manually call UpdateViewport once before actually painting the
// first frame. Replicate that behavior here
VERIFY_SUCCEEDED(engine->UpdateViewport(view.ToInclusive()));
engine->UpdateViewport(view.ToInclusive());

TestPaint(*engine, [&]() {
VERIFY_IS_FALSE(engine->_firstPaint);
Expand All @@ -1429,7 +1429,7 @@ void VtRendererTest::TestResize()
const auto newView = Viewport::FromDimensions({ 0, 0 }, { 120, 30 });
qExpectedInput.push_back("\x1b[8;30;120t");

VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive()));
engine->UpdateViewport(newView.ToInclusive());

TestPaint(*engine, [&]() {
VERIFY_IS_TRUE(engine->_invalidMap.all());
Expand Down
30 changes: 0 additions & 30 deletions src/interactivity/onecore/BgfxEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid
return S_FALSE;
}

[[nodiscard]] HRESULT BgfxEngine::ScrollFrame() noexcept
{
return S_OK;
}

[[nodiscard]] HRESULT BgfxEngine::PaintBackground() noexcept
{
PVOID OldRunBase;
Expand Down Expand Up @@ -166,19 +161,6 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid
CATCH_RETURN();
}

[[nodiscard]] HRESULT BgfxEngine::PaintBufferGridLines(GridLines const /*lines*/,
COLORREF const /*color*/,
size_t const /*cchLine*/,
COORD const /*coordTarget*/) noexcept
{
return S_OK;
}

[[nodiscard]] HRESULT BgfxEngine::PaintSelection(const SMALL_RECT /*rect*/) noexcept
{
return S_OK;
}

[[nodiscard]] HRESULT BgfxEngine::PaintCursor(const CursorOptions& options) noexcept
{
// TODO: MSFT: 11448021 - Modify BGFX to support rendering full-width
Expand Down Expand Up @@ -257,15 +239,3 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid
*pResult = false;
return S_OK;
}

// Method Description:
// - Updates the window's title string.
// Does nothing for BGFX.
// Arguments:
// - newTitle: the new string to use for the title of the window
// Return Value:
// - S_OK
[[nodiscard]] HRESULT BgfxEngine::_DoUpdateTitle(_In_ const std::wstring_view /*newTitle*/) noexcept
{
return S_OK;
}
3 changes: 0 additions & 3 deletions src/interactivity/onecore/BgfxEngine.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ namespace Microsoft::Console::Render
[[nodiscard]] HRESULT GetFontSize(_Out_ COORD* const pFontSize) noexcept override;
[[nodiscard]] HRESULT IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept override;

protected:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you forget to remove some of them from this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can't compile Bgfx & WDDM. I don't know if it can compile or not. So I just left it as it is.

[[nodiscard]] HRESULT _DoUpdateTitle(_In_ const std::wstring_view newTitle) noexcept override;

private:
ULONG_PTR _sharedViewBase;
SIZE_T _runLength;
Expand Down
Loading