Skip to content

Commit

Permalink
Improve performance of scrollbar marks (#16006)
Browse files Browse the repository at this point in the history
This replaces the use of a `<Canvas>` with an `<Image>` for drawing
scrollbar marks. Otherwise, WinUI struggles with the up to ~9000 UI
elements as they get dirtied every time the scrollbar moves.
(FWIW 9000 is not a lot and it should not struggle with that.)

The `<Image>` element has the benefit that we can get hold of a CPU-side
bitmap which we can manually draw our marks into and then swap them into
the UI tree. It draws the same 9000 elements, but now WinUI doesn't
struggle anymore because only 1 element gets invalidated every time.

Closes #15955

## Validation Steps Performed
* Fill the buffer with "e"
* Searching for "e" fills the entire thumb range with white ✅
* ...doesn't lag when scrolling around ✅
* ...updates quickly when adding newlines at the end ✅
* Marks sort of align with their scroll position ✅
  • Loading branch information
lhecker authored Sep 20, 2023
1 parent 523edd7 commit b5333f6
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 63 deletions.
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

til::color ControlCore::ForegroundColor() const
{
const auto lock = _terminal->LockForReading();
return _terminal->GetRenderSettings().GetColorAlias(ColorAlias::DefaultForeground);
}

Expand Down
129 changes: 88 additions & 41 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,56 +296,102 @@ namespace winrt::Microsoft::Terminal::Control::implementation

if (_showMarksInScrollbar)
{
// Update scrollbar marks
ScrollBarCanvas().Children().Clear();
const auto marks{ _core.ScrollMarks() };
const auto fullHeight{ ScrollBarCanvas().ActualHeight() };
const auto totalBufferRows{ update.newMaximum + update.newViewportSize };

auto drawPip = [&](const auto row, const auto rightAlign, const auto& brush) {
Windows::UI::Xaml::Shapes::Rectangle r;
r.Fill(brush);
r.Width(16.0f / 3.0f); // pip width - 1/3rd of the scrollbar width.
r.Height(2);
const auto fractionalHeight = row / totalBufferRows;
const auto relativePos = fractionalHeight * fullHeight;
ScrollBarCanvas().Children().Append(r);
Windows::UI::Xaml::Controls::Canvas::SetTop(r, relativePos);
if (rightAlign)
const auto scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel();
const auto scrollBarWidthInDIP = scrollBar.ActualWidth();
const auto scrollBarHeightInDIP = scrollBar.ActualHeight();
const auto scrollBarWidthInPx = gsl::narrow_cast<int32_t>(lrint(scrollBarWidthInDIP * scaleFactor));
const auto scrollBarHeightInPx = gsl::narrow_cast<int32_t>(lrint(scrollBarHeightInDIP * scaleFactor));

const auto canvas = FindName(L"ScrollBarCanvas").as<Controls::Image>();
auto source = canvas.Source().try_as<Media::Imaging::WriteableBitmap>();

if (!source || scrollBarWidthInPx != source.PixelWidth() || scrollBarHeightInPx != source.PixelHeight())
{
source = Media::Imaging::WriteableBitmap{ scrollBarWidthInPx, scrollBarHeightInPx };
canvas.Source(source);
canvas.Width(scrollBarWidthInDIP);
canvas.Height(scrollBarHeightInDIP);
}

const auto buffer = source.PixelBuffer();
const auto data = buffer.data();
const auto stride = scrollBarWidthInPx * sizeof(til::color);

// The bitmap has the size of the entire scrollbar, but we want the marks to only show in the range the "thumb"
// (the scroll indicator) can move. That's why we need to add an offset to the start of the drawable bitmap area
// (to offset the decrease button) and subtract twice that (to offset the increase button as well).
//
// The WinUI standard scrollbar defines a Margin="2,0,2,0" for the "VerticalPanningThumb" and a Padding="0,4,0,0"
// for the "VerticalDecrementTemplate" (and similar for the increment), but it seems neither of those is correct,
// because a padding for 3 DIPs seem to be the exact right amount to add.
const auto increaseDecreaseButtonHeight = scrollBarWidthInPx + lround(3 * scaleFactor);
const auto drawableDataStart = data + stride * increaseDecreaseButtonHeight;
const auto drawableRange = scrollBarHeightInPx - 2 * increaseDecreaseButtonHeight;

// Protect the remaining code against negative offsets. This normally can't happen
// and this code just exists so it doesn't crash if I'm ever wrong about this.
// (The window has a min. size that ensures that there's always a scrollbar thumb.)
if (drawableRange < 0)
{
assert(false);
return;
}

// The scrollbar bitmap is divided into 3 evenly sized stripes:
// Left: Regular marks
// Center: nothing
// Right: Search marks
const auto pipWidth = (scrollBarWidthInPx + 1) / 3;
const auto pipHeight = lround(1 * scaleFactor);

const auto maxOffsetY = drawableRange - pipHeight;
const auto offsetScale = maxOffsetY / gsl::narrow_cast<float>(update.newMaximum + update.newViewportSize);
// A helper to turn a TextBuffer row offset into a bitmap offset.
const auto dataAt = [&](til::CoordType row) [[msvc::forceinline]] {
const auto y = std::clamp<long>(lrintf(row * offsetScale), 0, maxOffsetY);
return drawableDataStart + stride * y;
};
// A helper to draw a single pip (mark) at the given location.
const auto drawPip = [&](uint8_t* beg, til::color color) [[msvc::forceinline]] {
const auto end = beg + pipHeight * stride;
for (; beg < end; beg += stride)
{
Windows::UI::Xaml::Controls::Canvas::SetLeft(r, 16.0f * .66f);
// Coincidentally a til::color has the same RGBA format as the bitmap.
#pragma warning(suppress : 26490) // Don't use reinterpret_cast (type.1).
std::fill_n(reinterpret_cast<til::color*>(beg), pipWidth, color);
}
};

for (const auto m : marks)
memset(data, 0, buffer.Length());

if (const auto marks = _core.ScrollMarks())
{
Windows::UI::Xaml::Shapes::Rectangle r;
Media::SolidColorBrush brush{};
// Sneaky: technically, a mark doesn't need to have a color set,
// it might want to just use the color from the palette for that
// kind of mark. Fortunately, ControlCore is kind enough to
// pre-evaluate that for us, and shove the real value into the
// Color member, regardless if the mark has a literal value set.
brush.Color(static_cast<til::color>(m.Color.Color));
drawPip(m.Start.Y, false, brush);
for (const auto& m : marks)
{
const auto row = m.Start.Y;
const til::color color{ m.Color.Color };
const auto base = dataAt(row);
drawPip(base, color);
}
}

if (_searchBox)
if (_searchBox && _searchBox->Visibility() == Visibility::Visible)
{
const auto searchMatches{ _core.SearchResultRows() };
if (searchMatches &&
searchMatches.Size() > 0 &&
_searchBox->Visibility() == Visibility::Visible)
if (const auto searchMatches = _core.SearchResultRows())
{
const til::color fgColor{ _core.ForegroundColor() };
Media::SolidColorBrush searchMarkBrush{};
searchMarkBrush.Color(fgColor);
for (const auto m : searchMatches)
const til::color color{ _core.ForegroundColor() };
const auto rightAlignedOffset = (scrollBarWidthInPx - pipWidth) * sizeof(til::color);

for (const auto row : searchMatches)
{
drawPip(m, true, searchMarkBrush);
const auto base = dataAt(row) + rightAlignedOffset;
drawPip(base, color);
}
}
}

source.Invalidate();
canvas.Visibility(Visibility::Visible);
}
}

Expand Down Expand Up @@ -620,14 +666,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// achieve the intended effect.
ScrollBar().IndicatorMode(Controls::Primitives::ScrollingIndicatorMode::None);
ScrollBar().Visibility(Visibility::Collapsed);
ScrollMarksGrid().Visibility(Visibility::Collapsed);
}
else // (default or Visible)
{
// Default behavior
ScrollBar().IndicatorMode(Controls::Primitives::ScrollingIndicatorMode::MouseIndicator);
ScrollBar().Visibility(Visibility::Visible);
ScrollMarksGrid().Visibility(Visibility::Visible);
}

_interactivity.UpdateSettings();
Expand All @@ -640,8 +684,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

_showMarksInScrollbar = settings.ShowMarks();
// Clear out all the current marks
ScrollBarCanvas().Children().Clear();
// Hide all scrollbar marks since they might be disabled now.
if (const auto canvas = ScrollBarCanvas())
{
canvas.Visibility(Visibility::Collapsed);
}
// When we hot reload the settings, the core will send us a scrollbar
// update. If we enabled scrollbar marks, then great, when we handle
// that message, we'll redraw them.
Expand Down
30 changes: 8 additions & 22 deletions src/cascadia/TerminalControl/TermControl.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -1322,28 +1322,14 @@
ValueChanged="_ScrollbarChangeHandler"
ViewportSize="10" />

<Grid x:Name="ScrollMarksGrid"
Grid.Column="1"
Width="{StaticResource ScrollBarSize}"
HorizontalAlignment="Right"
VerticalAlignment="Stretch">

<Grid.RowDefinitions>
<RowDefinition Height="Auto" />
<RowDefinition Height="*" />
<RowDefinition Height="Auto" />
</Grid.RowDefinitions>

<Border Grid.Row="0"
Height="{StaticResource ScrollBarSize}" />
<Canvas x:Name="ScrollBarCanvas"
Grid.Row="1"
Width="{StaticResource ScrollBarSize}"
HorizontalAlignment="Right"
VerticalAlignment="Stretch" />
<Border Grid.Row="2"
Height="{StaticResource ScrollBarSize}" />
</Grid>
<Image x:Name="ScrollBarCanvas"
Grid.Column="1"
Width="{StaticResource ScrollBarSize}"
HorizontalAlignment="Right"
VerticalAlignment="Stretch"
x:Load="False"
IsHitTestVisible="False"
Visibility="Collapsed" />

</Grid>

Expand Down

0 comments on commit b5333f6

Please sign in to comment.