Skip to content

Commit

Permalink
Render row-by-row instead of invalidating entire screen (#5185)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Adjusts DirectX renderer to use `til::bitmap` to track invalidation
regions. Uses special modification to invalidate a row-at-a-time to
ensure ligatures and NxM glyphs continue to work.

## References
Likely helps #1064

## PR Checklist
* [x] Closes #778
* [x] I work here.
* [x] Manual testing performed. See Performance traces in #778.
* [x] Automated tests for `til` changes.
* [x] Am core contributor. And discussed with @DHowett-MSFT.

## Detailed Description of the Pull Request / Additional comments
- Applies `til::bitmap` as the new invalidation scheme inside the
  DirectX renderer and updates all entrypoints for collecting
  invalidation data to coalesce into this structure.
- Semi-permanently routes all invalidations through a helper method
  `_InvalidateRectangle` that will expand any invalidation to cover the
  entire line. This ensures that ligatures and NxM glyphs will continue
  to render appropriately while still allowing us to dramatically reduce
  the number of lines drawn overall. In the future, we may come up with
  a tighter solution than line-by-line invalidation and can modify this
  helper method appropriately at that later date to further scope the
  invalid region.
- Ensures that the `experimental.retroTerminalEffects` feature continues
  to invalidate the entire display on start of frame as the shader is
  applied at the end of the frame composition and will stack on itself
  in an amusing fashion when we only redraw part of the display.
- Moves many member variables inside the DirectX renderer into the new
  `til::size`, `til::point`, and `til::rectangle` methods to facilitate
  easier management and mathematical operations. Consequently adds
  `try/catch` blocks around many of the already-existing `noexcept`
  methods to deal with mathematical or casting failures now detected by
  using the support classes.
- Corrects `TerminalCore` redraw triggers to appropriately communicate
  scrolling circumstances to the renderer so it can optimize the draw
  regions appropriately.
- Fixes an issue in the base `Renderer` that was causing overlapping
  scroll regions due to behavior of `Viewport::TrimToViewport` modifying
  the local. This fix is "good enough" for now and should go away when
  `Viewport` is fully migrated to `til::rectangle`.
- Adds multiplication and division operators to `til::rectangle` and
  supporting tests. These operates will help scale back and forth
  between a cell-based rectangle and a pixel-based rectangle. They take
  special care to ensure that a pixel rectangle being divided downward
  back to cells will expand (with the ceiling division methods) to cover
  a full cell when even one pixel inside the cell is touched (as is how
  a redraw would have to occur).
- Blocks off trace logging of invalid regions if no one is listening to
  optimize performance.
- Restores full usage of `IDXGISwapChain1::Present1` to accurately and
  fully communicate dirty and scroll regions to the underlying DirectX
  framework. This additional information allows the framework to
  optimize drawing between frames by eliminating data transfer of
  regions that aren't modified and shuffling frames in place. See
  [Remarks](https://docs.microsoft.com/en-us/windows/win32/api/dxgi1_2/nf-dxgi1_2-idxgiswapchain1-present1#remarks)
  for more details.
- Updates `til::bitmap` set methods to use more optimized versions of
  the setters on the `dynamic_bitset<>` that can bulk fill bits as the
  existing algorithm was noticeably slow after applying the
  "expand-to-row" helper to the DirectX renderer invalidation.
- All `til` import hierarchy is now handled in the parent `til.h` file
  and not in the child files to prevent circular imports from happening.
  We don't expect the import of any individual library file, only the
  base one. So this should be OK for now.

## Validation Steps Performed
- Ran `cmatrix`, `cmatrix -u0`, and `cacafire` after changes were made.
- Made a bunch of ligatures with `Cascadia Code` in the Terminal
  before/after the changes and confirmed they still ligate.
- Ran `dir` in Powershell and fixed the scrolling issues
- Clicked all over the place and dragged to make sure selection works.
- Checked retro terminal effect manually with Powershell.
  • Loading branch information
miniksa authored Apr 13, 2020
1 parent ffbdfe3 commit 79684bf
Show file tree
Hide file tree
Showing 17 changed files with 762 additions and 431 deletions.
5 changes: 5 additions & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

if (point.Properties().IsLeftButtonPressed())
{
auto lock = _terminal->LockForWriting();

const auto cursorPosition = point.Position();
const auto terminalPosition = _GetTerminalPosition(cursorPosition);

Expand Down Expand Up @@ -979,6 +981,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
_lastMouseClickTimestamp = point.Timestamp();
_lastMouseClickPos = cursorPosition;
}

_renderer->TriggerSelection();
}
else if (point.Properties().IsRightButtonPressed())
Expand Down Expand Up @@ -1037,6 +1040,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

if (point.Properties().IsLeftButtonPressed())
{
auto lock = _terminal->LockForWriting();

const auto cursorPosition = point.Position();

if (_singleClickTouchdownPos)
Expand Down
15 changes: 13 additions & 2 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,11 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)

if (notifyScroll)
{
_buffer->GetRenderTarget().TriggerRedrawAll();
// We have to report the delta here because we might have circled the text buffer.
// That didn't change the viewport and therefore the TriggerScroll(void)
// method can't detect the delta on its own.
COORD delta{ 0, -gsl::narrow<SHORT>(newRows) };
_buffer->GetRenderTarget().TriggerScroll(&delta);
_NotifyScrollEvent();
}

Expand All @@ -789,13 +793,20 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition)

void Terminal::UserScrollViewport(const int viewTop)
{
// we're going to modify state here that the renderer could be reading.
auto lock = LockForWriting();

const auto clampedNewTop = std::max(0, viewTop);
const auto realTop = ViewStartIndex();
const auto newDelta = realTop - clampedNewTop;
// if viewTop > realTop, we want the offset to be 0.

_scrollOffset = std::max(0, newDelta);
_buffer->GetRenderTarget().TriggerRedrawAll();

// We can use the void variant of TriggerScroll here because
// we adjusted the viewport so it can detect the difference
// from the previous frame drawn.
_buffer->GetRenderTarget().TriggerScroll();
}

int Terminal::GetScrollOffset() noexcept
Expand Down
2 changes: 1 addition & 1 deletion src/inc/til.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#include "til/some.h"
#include "til/size.h"
#include "til/point.h"
#include "til/rectangle.h"
#include "til/operators.h"
#include "til/rectangle.h"
#include "til/bitmap.h"
#include "til/u8u16convert.h"

Expand Down
11 changes: 8 additions & 3 deletions src/inc/til/bitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt));
_runs.reset(); // reset cached runs on any non-const method

til::at(_bits, _rc.index_of(pt)) = true;
_bits.set(_rc.index_of(pt));

_dirty |= til::rectangle{ pt };
}
Expand All @@ -283,9 +283,9 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc));
_runs.reset(); // reset cached runs on any non-const method

for (const auto pt : rc)
for (auto row = rc.top(); row < rc.bottom(); ++row)
{
til::at(_bits, _rc.index_of(pt)) = true;
_bits.set(_rc.index_of(til::point{ rc.left(), row }), rc.width(), true);
}

_dirty |= rc;
Expand Down Expand Up @@ -378,6 +378,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
return _dirty == _rc;
}

constexpr til::size size() const noexcept
{
return _sz;
}

std::wstring to_string() const
{
std::wstringstream wss;
Expand Down
4 changes: 0 additions & 4 deletions src/inc/til/operators.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@

#pragma once

#include "rectangle.h"
#include "size.h"
#include "bitmap.h"

namespace til // Terminal Implementation Library. Also: "Today I Learned"
{
// Operators go here when they involve two headers that can't/don't include each other.
Expand Down
13 changes: 13 additions & 0 deletions src/inc/til/point.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
return *this;
}

template<typename TilMath>
point scale(TilMath, const float scale) const
{
struct
{
float x, y;
} pt;
THROW_HR_IF(E_ABORT, !base::CheckMul(scale, _x).AssignIfValid(&pt.x));
THROW_HR_IF(E_ABORT, !base::CheckMul(scale, _y).AssignIfValid(&pt.y));

return til::point(TilMath(), pt);
}

point operator/(const point& other) const
{
ptrdiff_t x;
Expand Down
52 changes: 48 additions & 4 deletions src/inc/til/rectangle.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@

#pragma once

#include "point.h"
#include "size.h"
#include "some.h"

#ifdef UNIT_TESTING
class RectangleTests;
#endif
Expand Down Expand Up @@ -179,6 +175,22 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
{
}

// This template will convert to rectangle from anything that has a Left, Top, Right, and Bottom field that are floating-point;
// a math type is required.
template<typename TilMath, typename TOther>
constexpr rectangle(TilMath, const TOther& other, std::enable_if_t<std::is_floating_point_v<decltype(std::declval<TOther>().Left)> && std::is_floating_point_v<decltype(std::declval<TOther>().Top)> && std::is_floating_point_v<decltype(std::declval<TOther>().Right)> && std::is_floating_point_v<decltype(std::declval<TOther>().Bottom)>, int> /*sentinel*/ = 0) :
rectangle(til::point{ TilMath::template cast<ptrdiff_t>(other.Left), TilMath::template cast<ptrdiff_t>(other.Top) }, til::point{ TilMath::template cast<ptrdiff_t>(other.Right), TilMath::template cast<ptrdiff_t>(other.Bottom) })
{
}

// This template will convert to rectangle from anything that has a left, top, right, and bottom field that are floating-point;
// a math type is required.
template<typename TilMath, typename TOther>
constexpr rectangle(TilMath, const TOther& other, std::enable_if_t<std::is_floating_point_v<decltype(std::declval<TOther>().left)> && std::is_floating_point_v<decltype(std::declval<TOther>().top)> && std::is_floating_point_v<decltype(std::declval<TOther>().right)> && std::is_floating_point_v<decltype(std::declval<TOther>().bottom)>, int> /*sentinel*/ = 0) :
rectangle(til::point{ TilMath::template cast<ptrdiff_t>(other.left), TilMath::template cast<ptrdiff_t>(other.top) }, til::point{ TilMath::template cast<ptrdiff_t>(other.right), TilMath::template cast<ptrdiff_t>(other.bottom) })
{
}

constexpr bool operator==(const rectangle& other) const noexcept
{
return _topLeft == other._topLeft &&
Expand Down Expand Up @@ -636,6 +648,38 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
return *this;
}

// scale_up will scale the entire rectangle up by the size factor
// This includes moving the origin.
rectangle scale_up(const size& size) const
{
const auto topLeft = _topLeft * size;
const auto bottomRight = _bottomRight * size;
return til::rectangle{ topLeft, bottomRight };
}

// scale_down will scale the entire rectangle down by the size factor,
// but rounds the bottom-right corner out.
// This includes moving the origin.
rectangle scale_down(const size& size) const
{
auto topLeft = _topLeft;
auto bottomRight = _bottomRight;
topLeft = topLeft / size;

// Move bottom right point into a size
// Use size specialization of divide_ceil to round up against the size given.
// Add leading addition to point to convert it back into a point.
bottomRight = til::point{} + til::size{ right(), bottom() }.divide_ceil(size);

return til::rectangle{ topLeft, bottomRight };
}

template<typename TilMath>
rectangle scale(TilMath, const float scale) const
{
return til::rectangle{ _topLeft.scale(TilMath{}, scale), _bottomRight.scale(TilMath{}, scale) };
}

#pragma endregion

constexpr ptrdiff_t top() const noexcept
Expand Down
13 changes: 13 additions & 0 deletions src/inc/til/size.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
return size{ width, height };
}

template<typename TilMath>
size scale(TilMath, const float scale) const
{
struct
{
float Width, Height;
} sz;
THROW_HR_IF(E_ABORT, !base::CheckMul(scale, _width).AssignIfValid(&sz.Width));
THROW_HR_IF(E_ABORT, !base::CheckMul(scale, _height).AssignIfValid(&sz.Height));

return til::size(TilMath(), sz);
}

size operator/(const size& other) const
{
ptrdiff_t width;
Expand Down
75 changes: 68 additions & 7 deletions src/renderer/base/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,22 @@ void Renderer::TriggerSelection()
// Get selection rectangles
const auto rects = _GetSelectionRects();

// Restrict all previous selection rectangles to inside the current viewport bounds
for (auto& sr : _previousSelection)
{
// Make the exclusive SMALL_RECT into a til::rectangle.
til::rectangle rc{ Viewport::FromExclusive(sr).ToInclusive() };

// Make a viewport representing the coordinates that are currently presentable.
const til::rectangle viewport{ til::size{ _pData->GetViewport().Dimensions() } };

// Intersect them so we only invalidate things that are still visible.
rc &= viewport;

// Convert back into the exclusive SMALL_RECT and store in the vector.
sr = Viewport::FromInclusive(rc).ToExclusive();
}

std::for_each(_rgpEngines.begin(), _rgpEngines.end(), [&](IRenderEngine* const pEngine) {
LOG_IF_FAILED(pEngine->InvalidateSelection(_previousSelection));
LOG_IF_FAILED(pEngine->InvalidateSelection(rects));
Expand Down Expand Up @@ -330,13 +346,26 @@ bool Renderer::_CheckViewportAndScroll()
coordDelta.X = srOldViewport.Left - srNewViewport.Left;
coordDelta.Y = srOldViewport.Top - srNewViewport.Top;

std::for_each(_rgpEngines.begin(), _rgpEngines.end(), [&](IRenderEngine* const pEngine) {
LOG_IF_FAILED(pEngine->UpdateViewport(srNewViewport));
LOG_IF_FAILED(pEngine->InvalidateScroll(&coordDelta));
});
for (auto engine : _rgpEngines)
{
LOG_IF_FAILED(engine->UpdateViewport(srNewViewport));
}

_srViewportPrevious = srNewViewport;

return coordDelta.X != 0 || coordDelta.Y != 0;
if (coordDelta.X != 0 || coordDelta.Y != 0)
{
for (auto engine : _rgpEngines)
{
LOG_IF_FAILED(engine->InvalidateScroll(&coordDelta));
}

_ScrollPreviousSelection(coordDelta);

return true;
}

return false;
}

// Routine Description:
Expand Down Expand Up @@ -369,6 +398,8 @@ void Renderer::TriggerScroll(const COORD* const pcoordDelta)
LOG_IF_FAILED(pEngine->InvalidateScroll(pcoordDelta));
});

_ScrollPreviousSelection(*pcoordDelta);

_NotifyPaintFrame();
}

Expand Down Expand Up @@ -927,10 +958,13 @@ void Renderer::_PaintSelection(_In_ IRenderEngine* const pEngine)
{
for (auto dirtyRect : dirtyAreas)
{
// Make a copy as `TrimToViewport` will manipulate it and
// can destroy it for the next dirtyRect to test against.
auto rectCopy = rect;
Viewport dirtyView = Viewport::FromInclusive(dirtyRect);
if (dirtyView.TrimToViewport(&rect))
if (dirtyView.TrimToViewport(&rectCopy))
{
LOG_IF_FAILED(pEngine->PaintSelection(rect));
LOG_IF_FAILED(pEngine->PaintSelection(rectCopy));
}
}
}
Expand Down Expand Up @@ -1002,6 +1036,33 @@ std::vector<SMALL_RECT> Renderer::_GetSelectionRects() const
return result;
}

// Method Description:
// - Offsets all of the selection rectangles we might be holding onto
// as the previously selected area. If the whole viewport scrolls,
// we need to scroll these areas also to ensure they're invalidated
// properly when the selection further changes.
// Arguments:
// - delta - The scroll delta
// Return Value:
// - <none> - Updates internal state instead.
void Renderer::_ScrollPreviousSelection(const til::point delta)
{
if (delta != til::point{ 0, 0 })
{
for (auto& sr : _previousSelection)
{
// Get a rectangle representing this piece of the selection.
til::rectangle rc = Viewport::FromExclusive(sr).ToInclusive();

// Offset the entire existing rectangle by the delta.
rc += delta;

// Store it back into the vector.
sr = Viewport::FromInclusive(rc).ToExclusive();
}
}
}

// Method Description:
// - Adds another Render engine to this renderer. Future rendering calls will
// also be sent to the new renderer.
Expand Down
1 change: 1 addition & 0 deletions src/renderer/base/renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ namespace Microsoft::Console::Render
SMALL_RECT _srViewportPrevious;

std::vector<SMALL_RECT> _GetSelectionRects() const;
void _ScrollPreviousSelection(const til::point delta);
std::vector<SMALL_RECT> _previousSelection;

[[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine);
Expand Down
Loading

1 comment on commit 79684bf

@github-actions
Copy link

Choose a reason for hiding this comment

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

New misspellings found, please review:

  • aditionally
  • algorithim
  • aslo
  • charater
  • commadline
  • ertical
  • Esentially
  • gitlab
  • Hackathon
  • pallete
  • particualr
  • plit
  • recieve
  • thoguh
  • validator
To accept these changes, run the following commands
remove_obsolete_words=$(mktemp)
echo '#!/usr/bin/perl -ni
my $re=join "|", qw('"
aaaaa
aaaaaa
aaaaaaaaaaaaaaaaaaa
ABCDEFGHIJKLMNOPQRSTQ
ABCDEFGHIJPQRST
ABCDEFGHIJPQRSTQ
APPLOGIC
BBBB
BBBBBBBBBBBBBBBBBBBBBBBBBB
CCCC
DDDD
EEEE
ffff
ggggg
gggggg
ggggggg
LLLLLLLL
QQQQ
RRRRRRR
Spc
unicodechar
usr
XChars
xxxx
YChars
"');
next if /^($re)(?:$| .*)/;
print;' > $remove_obsolete_words
chmod +x $remove_obsolete_words
for file in .github/actions/spell-check/whitelist/alphabet.txt .github/actions/spell-check/whitelist/web.txt .github/actions/spell-check/whitelist/whitelist.txt; do $remove_obsolete_words $file; done
rm $remove_obsolete_words
(
echo "
aditionally
algorithim
applogic
aslo
charater
commadline
ertical
Esentially
gitlab
Hackathon
pallete
particualr
plit
recieve
thoguh
validator
"
) | sort -u -f | perl -ne 'next unless /./; print' > new_whitelist.txt && mv new_whitelist.txt '.github/actions/spell-check/whitelist/79684bf82177dfa52bd1eca84476a2950d765bb9.txt'
✏️ Contributor please read this
  • If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
  • If you need to use a specific token in one place and it shouldn't generally be used, you can
    add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Please sign in to comment.