From 7e8f0398c3c851c96189b1ef72f1c82feda7510e Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 23 Mar 2020 09:55:01 -0700 Subject: [PATCH 01/29] Start moving dx renderer onto bitmap. --- src/inc/til/point.h | 24 ++ src/renderer/dx/DxRenderer.cpp | 398 ++++++++++----------------------- src/renderer/dx/DxRenderer.hpp | 22 +- src/renderer/dx/precomp.h | 4 + 4 files changed, 147 insertions(+), 301 deletions(-) diff --git a/src/inc/til/point.h b/src/inc/til/point.h index c7643b05286..39a465f073a 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -107,6 +107,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator+=(const point& other) + { + *this = *this + other; + return *this; + } + point operator-(const point& other) const { ptrdiff_t x; @@ -118,6 +124,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator-=(const point& other) + { + *this = *this - other; + return *this; + } + point operator*(const point& other) const { ptrdiff_t x; @@ -129,6 +141,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator*=(const point& other) + { + *this = *this * other; + return *this; + } + point operator/(const point& other) const { ptrdiff_t x; @@ -140,6 +158,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return point{ x, y }; } + point& operator/=(const point& other) + { + *this = *this / other; + return *this; + } + constexpr ptrdiff_t x() const noexcept { return _x; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index d5fed132ab5..cf411554bf1 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -65,9 +65,8 @@ using namespace Microsoft::Console::Types; // TODO GH 2683: The default constructor should not throw. DxEngine::DxEngine() : RenderEngineBase(), - _isInvalidUsed{ false }, - _invalidRect{ 0 }, - _invalidScroll{ 0 }, + _invalidMap{}, + _invalidScroll{}, _presentParams{ 0 }, _presentReady{ false }, _presentScroll{ 0 }, @@ -75,16 +74,16 @@ DxEngine::DxEngine() : _presentOffset{ 0 }, _isEnabled{ false }, _isPainting{ false }, - _displaySizePixels{ 0 }, + _displaySizePixels{}, _foregroundColor{ 0 }, _backgroundColor{ 0 }, _selectionBackground{}, - _glyphCell{ 0 }, + _glyphCell{}, _haveDeviceResources{ false }, _retroTerminalEffects{ false }, _antialiasingMode{ D2D1_TEXT_ANTIALIAS_MODE_GRAYSCALE }, _hwndTarget{ static_cast(INVALID_HANDLE_VALUE) }, - _sizeTarget{ 0 }, + _sizeTarget{}, _dpi{ USER_DEFAULT_SCREEN_DPI }, _scale{ 1.0f }, _chainMode{ SwapChainMode::ForComposition }, @@ -238,8 +237,8 @@ HRESULT DxEngine::_SetupTerminalEffects() // Setup the viewport. D3D11_VIEWPORT vp; - vp.Width = static_cast(_displaySizePixels.cx); - vp.Height = static_cast(_displaySizePixels.cy); + vp.Width = _displaySizePixels.width(); + vp.Height = _displaySizePixels.height(); vp.MinDepth = 0.0f; vp.MaxDepth = 1.0f; vp.TopLeftX = 0; @@ -424,8 +423,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept { switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { // use the HWND's dimensions for the swap chain dimensions. RECT rect = { 0 }; RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); @@ -454,11 +452,10 @@ void DxEngine::_ComputePixelShaderSettings() noexcept break; } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { // Use the given target size for compositions. - SwapChainDesc.Width = _displaySizePixels.cx; - SwapChainDesc.Height = _displaySizePixels.cy; + SwapChainDesc.Width = _displaySizePixels.width(); + SwapChainDesc.Height = _displaySizePixels.height(); // We're doing advanced composition pretty much for the purpose of pretty alpha, so turn it on. SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED; @@ -628,8 +625,8 @@ void DxEngine::_ReleaseDeviceResources() noexcept return _dwriteFactory->CreateTextLayout(string, gsl::narrow(stringLength), _dwriteTextFormat.Get(), - gsl::narrow(_displaySizePixels.cx), - _glyphCell.cy != 0 ? _glyphCell.cy : gsl::narrow(_displaySizePixels.cy), + _displaySizePixels.width(), + _glyphCell.height() != 0 ? _glyphCell.height() : _displaySizePixels.height(), ppTextLayout); } @@ -686,7 +683,8 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() { RETURN_HR_IF_NULL(E_INVALIDARG, psrRegion); - _InvalidOr(*psrRegion); + _invalidMap.set(*psrRegion); + return S_OK; } @@ -700,8 +698,9 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() { RETURN_HR_IF_NULL(E_INVALIDARG, pcoordCursor); - const SMALL_RECT sr = Microsoft::Console::Types::Viewport::FromCoord(*pcoordCursor).ToInclusive(); - return Invalidate(&sr); + _invalidMap.set(*pcoordCursor); + + return S_OK; } // Routine Description: @@ -712,6 +711,7 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() // - S_OK [[nodiscard]] HRESULT DxEngine::InvalidateSystem(const RECT* const prcDirtyClient) noexcept { + // TODO: this is not right. RETURN_HR_IF_NULL(E_INVALIDARG, prcDirtyClient); _InvalidOr(*prcDirtyClient); @@ -743,50 +743,24 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::InvalidateScroll(const COORD* const pcoordDelta) noexcept +try { - if (pcoordDelta->X != 0 || pcoordDelta->Y != 0) - { - try - { - POINT delta = { 0 }; - delta.x = pcoordDelta->X * _glyphCell.cx; - delta.y = pcoordDelta->Y * _glyphCell.cy; - - _InvalidOffset(delta); - - _invalidScroll.cx += delta.x; - _invalidScroll.cy += delta.y; + const til::point deltaCells{ *pcoordDelta }; - // Add the revealed portion of the screen from the scroll to the invalid area. - const RECT display = _GetDisplayRect(); - RECT reveal = display; - - // X delta first - OffsetRect(&reveal, delta.x, 0); - IntersectRect(&reveal, &reveal, &display); - SubtractRect(&reveal, &display, &reveal); - - if (!IsRectEmpty(&reveal)) - { - _InvalidOr(reveal); - } + if (deltaCells != til::point{ 0, 0 }) + { + const til::point deltaPixels = deltaCells * _glyphCell; - // Y delta second (subtract rect won't work if you move both) - reveal = display; - OffsetRect(&reveal, 0, delta.y); - IntersectRect(&reveal, &reveal, &display); - SubtractRect(&reveal, &display, &reveal); + // Shift the contents of the map and fill in revealed area. + _invalidMap.translate(deltaCells, true); - if (!IsRectEmpty(&reveal)) - { - _InvalidOr(reveal); - } - } - CATCH_RETURN(); + // TODO: should we just maintain it all in cells? + _invalidScroll += deltaPixels; } return S_OK; } +CATCH_RETURN(); // Routine Description: // - Invalidates the entire window area @@ -795,12 +769,12 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::InvalidateAll() noexcept +try { - const RECT screen = _GetDisplayRect(); - _InvalidOr(screen); - + _invalidMap.set_all(); return S_OK; } +CATCH_RETURN(); // Routine Description: // - This currently has no effect in this renderer. @@ -822,23 +796,17 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() // - // Return Value: // - X by Y area in pixels of the surface -[[nodiscard]] SIZE DxEngine::_GetClientSize() const noexcept +[[nodiscard]] til::size DxEngine::_GetClientSize() const noexcept { switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { RECT clientRect = { 0 }; LOG_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &clientRect)); - SIZE clientSize = { 0 }; - clientSize.cx = clientRect.right - clientRect.left; - clientSize.cy = clientRect.bottom - clientRect.top; - - return clientSize; + return til::rectangle{ clientRect }.size(); } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { SIZE size = _sizeTarget; size.cx = static_cast(size.cx * _scale); size.cy = static_cast(size.cy * _scale); @@ -865,90 +833,6 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept cellsToPixels.bottom *= fontSize.cy; } -// Routine Description: -// - Retrieves a rectangle representation of the pixel size of the -// surface we are drawing on -// Arguments: -// - -// Return Value; -// - Origin-placed rectangle representing the pixel size of the surface -[[nodiscard]] RECT DxEngine::_GetDisplayRect() const noexcept -{ - return { 0, 0, _displaySizePixels.cx, _displaySizePixels.cy }; -} - -// Routine Description: -// - Helper to shift the existing dirty rectangle by a pixel offset -// and crop it to still be within the bounds of the display surface -// Arguments: -// - delta - Adjustment distance in pixels -// - -Y is up, Y is down, -X is left, X is right. -// Return Value: -// - -void DxEngine::_InvalidOffset(POINT delta) -{ - if (_isInvalidUsed) - { - // Copy the existing invalid rect - RECT invalidNew = _invalidRect; - - // Offset it to the new position - THROW_IF_WIN32_BOOL_FALSE(OffsetRect(&invalidNew, delta.x, delta.y)); - - // Get the rect representing the display - const RECT rectScreen = _GetDisplayRect(); - - // Ensure that the new invalid rectangle is still on the display - IntersectRect(&invalidNew, &invalidNew, &rectScreen); - - _invalidRect = invalidNew; - } -} - -// Routine description: -// - Adds the given character rectangle to the total dirty region -// - Will scale internally to pixels based on the current font. -// Arguments: -// - sr - character rectangle -// Return Value: -// - -void DxEngine::_InvalidOr(SMALL_RECT sr) noexcept -{ - RECT region; - region.left = sr.Left; - region.top = sr.Top; - region.right = sr.Right; - region.bottom = sr.Bottom; - _ScaleByFont(region, _glyphCell); - - region.right += _glyphCell.cx; - region.bottom += _glyphCell.cy; - - _InvalidOr(region); -} - -// Routine Description: -// - Adds the given pixel rectangle to the total dirty region -// Arguments: -// - rc - Dirty pixel rectangle -// Return Value: -// - -void DxEngine::_InvalidOr(RECT rc) noexcept -{ - if (_isInvalidUsed) - { - UnionRect(&_invalidRect, &_invalidRect, &rc); - - const RECT rcScreen = _GetDisplayRect(); - IntersectRect(&_invalidRect, &_invalidRect, &rcScreen); - } - else - { - _invalidRect = rc; - _isInvalidUsed = true; - } -} - // Routine Description: // - This is unused by this renderer. // Arguments: @@ -971,24 +855,23 @@ void DxEngine::_InvalidOr(RECT rc) noexcept // - Any DirectX error, a memory error, etc. [[nodiscard]] HRESULT DxEngine::StartPaint() noexcept { - FAIL_FAST_IF_FAILED(InvalidateAll()); RETURN_HR_IF(E_NOT_VALID_STATE, _isPainting); // invalid to start a paint while painting. -#pragma warning(suppress : 26477 26485 26494 26482 26446 26447) // We don't control TraceLoggingWrite - TraceLoggingWrite(g_hDxRenderProvider, - "Invalid", - TraceLoggingInt32(_invalidRect.bottom - _invalidRect.top, "InvalidHeight"), - TraceLoggingInt32((_invalidRect.bottom - _invalidRect.top) / _glyphCell.cy, "InvalidHeightChars"), - TraceLoggingInt32(_invalidRect.right - _invalidRect.left, "InvalidWidth"), - TraceLoggingInt32((_invalidRect.right - _invalidRect.left) / _glyphCell.cx, "InvalidWidthChars"), - TraceLoggingInt32(_invalidRect.left, "InvalidX"), - TraceLoggingInt32(_invalidRect.left / _glyphCell.cx, "InvalidXChars"), - TraceLoggingInt32(_invalidRect.top, "InvalidY"), - TraceLoggingInt32(_invalidRect.top / _glyphCell.cy, "InvalidYChars"), - TraceLoggingInt32(_invalidScroll.cx, "ScrollWidth"), - TraceLoggingInt32(_invalidScroll.cx / _glyphCell.cx, "ScrollWidthChars"), - TraceLoggingInt32(_invalidScroll.cy, "ScrollHeight"), - TraceLoggingInt32(_invalidScroll.cy / _glyphCell.cy, "ScrollHeightChars")); + //#pragma warning(suppress : 26477 26485 26494 26482 26446 26447) // We don't control TraceLoggingWrite + // TraceLoggingWrite(g_hDxRenderProvider, + // "Invalid", + // TraceLoggingInt32(_invalidRect.bottom - _invalidRect.top, "InvalidHeight"), + // TraceLoggingInt32((_invalidRect.bottom - _invalidRect.top) / _glyphCell.cy, "InvalidHeightChars"), + // TraceLoggingInt32(_invalidRect.right - _invalidRect.left, "InvalidWidth"), + // TraceLoggingInt32((_invalidRect.right - _invalidRect.left) / _glyphCell.cx, "InvalidWidthChars"), + // TraceLoggingInt32(_invalidRect.left, "InvalidX"), + // TraceLoggingInt32(_invalidRect.left / _glyphCell.cx, "InvalidXChars"), + // TraceLoggingInt32(_invalidRect.top, "InvalidY"), + // TraceLoggingInt32(_invalidRect.top / _glyphCell.cy, "InvalidYChars"), + // TraceLoggingInt32(_invalidScroll.cx, "ScrollWidth"), + // TraceLoggingInt32(_invalidScroll.cx / _glyphCell.cx, "ScrollWidthChars"), + // TraceLoggingInt32(_invalidScroll.cy, "ScrollHeight"), + // TraceLoggingInt32(_invalidScroll.cy / _glyphCell.cy, "ScrollHeightChars")); if (_isEnabled) { @@ -999,8 +882,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept { RETURN_IF_FAILED(_CreateDeviceResources(true)); } - else if (_displaySizePixels.cy != clientSize.cy || - _displaySizePixels.cx != clientSize.cx) + else if (_displaySizePixels != clientSize) { // OK, we're going to play a dangerous game here for the sake of optimizing resize // First, set up a complete clear of all device resources if something goes terribly wrong. @@ -1013,7 +895,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept _d2dRenderTarget.Reset(); // Change the buffer size and recreate the render target (and surface) - RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.cx, clientSize.cy, DXGI_FORMAT_B8G8R8A8_UNORM, 0)); + RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width(), clientSize.height(), DXGI_FORMAT_B8G8R8A8_UNORM, 0)); RETURN_IF_FAILED(_PrepareRenderTarget()); // OK we made it past the parts that can cause errors. We can release our failure handler. @@ -1052,7 +934,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept if (SUCCEEDED(hr)) { - if (_invalidScroll.cy != 0 || _invalidScroll.cx != 0) + if (_invalidScroll != til::point{ 0, 0 }) { _presentDirty = _invalidRect; @@ -1083,10 +965,9 @@ void DxEngine::_InvalidOr(RECT rc) noexcept } } - _invalidRect = { 0 }; - _isInvalidUsed = false; + _invalidMap.reset_all(); - _invalidScroll = { 0 }; + _invalidScroll = {}; return hr; } @@ -1191,20 +1072,13 @@ void DxEngine::_InvalidOr(RECT rc) noexcept // - S_OK [[nodiscard]] HRESULT DxEngine::PaintBackground() noexcept { - switch (_chainMode) - { - case SwapChainMode::ForHwnd: - _d2dRenderTarget->FillRectangle(D2D1::RectF(static_cast(_invalidRect.left), - static_cast(_invalidRect.top), - static_cast(_invalidRect.right), - static_cast(_invalidRect.bottom)), - _d2dBrushBackground.Get()); - break; - case SwapChainMode::ForComposition: - D2D1_COLOR_F nothing = { 0 }; + D2D1_COLOR_F nothing = { 0 }; + for (const auto rect : _invalidMap.runs()) + { + _d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); _d2dRenderTarget->Clear(nothing); - break; + _d2dRenderTarget->PopAxisAlignedClip(); } return S_OK; @@ -1226,9 +1100,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept try { // Calculate positioning of our origin. - D2D1_POINT_2F origin; - origin.x = static_cast(coord.X * _glyphCell.cx); - origin.y = static_cast(coord.Y * _glyphCell.cy); + D2D1_POINT_2F origin = til::point{ coord } * _glyphCell; // Create the text layout CustomTextLayout layout(_dwriteFactory.Get(), @@ -1236,7 +1108,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept _dwriteTextFormat.Get(), _dwriteFontFace.Get(), clusters, - _glyphCell.cx); + _glyphCell.width()); // Get the baseline for this font as that's where we draw from DWRITE_LINE_SPACING spacing; @@ -1248,7 +1120,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept _d2dBrushBackground.Get(), _dwriteFactory.Get(), spacing, - D2D1::SizeF(gsl::narrow(_glyphCell.cx), gsl::narrow(_glyphCell.cy)), + _glyphCell, D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); // Layout then render the text @@ -1279,10 +1151,8 @@ void DxEngine::_InvalidOr(RECT rc) noexcept _d2dBrushForeground->SetColor(_ColorFFromColorRef(color)); - const auto font = _GetFontSize(); - D2D_POINT_2F target; - target.x = static_cast(coordTarget.X) * font.X; - target.y = static_cast(coordTarget.Y) * font.Y; + const auto font = _glyphCell; + D2D_POINT_2F target = til::point{ coordTarget } * font; D2D_POINT_2F start = { 0 }; D2D_POINT_2F end = { 0 }; @@ -1295,7 +1165,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept if (lines & GridLines::Top) { end = start; - end.x += font.X; + end.x += font.width(); _d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); } @@ -1303,7 +1173,7 @@ void DxEngine::_InvalidOr(RECT rc) noexcept if (lines & GridLines::Left) { end = start; - end.y += font.Y; + end.y += font.height(); _d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); } @@ -1316,28 +1186,28 @@ void DxEngine::_InvalidOr(RECT rc) noexcept // The top right corner inclusive is at 7,0 which is X (0) + Font Height (8) - 1 = 7. // 0.5 pixel offset for crisp lines; -0.5 on the Y to fit _in_ the cell, not outside it. - start = { target.x + 0.5f, target.y + font.Y - 0.5f }; + start = { target.x + 0.5f, target.y + font.height() - 0.5f }; if (lines & GridLines::Bottom) { end = start; - end.x += font.X - 1.f; + end.x += font.width() - 1.f; _d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); } - start = { target.x + font.X - 0.5f, target.y + 0.5f }; + start = { target.x + font.width() - 0.5f, target.y + 0.5f }; if (lines & GridLines::Right) { end = start; - end.y += font.Y - 1.f; + end.y += font.height() - 1.f; _d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get()); } // Move to the next character in this run. - target.x += font.X; + target.x += font.width(); } return S_OK; @@ -1356,17 +1226,16 @@ void DxEngine::_InvalidOr(RECT rc) noexcept _d2dBrushForeground->SetColor(_selectionBackground); const auto resetColorOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); - RECT pixels; + const til::rectangle cells{ rect }; + + /*RECT pixels; pixels.left = rect.Left * _glyphCell.cx; pixels.top = rect.Top * _glyphCell.cy; pixels.right = rect.Right * _glyphCell.cx; - pixels.bottom = rect.Bottom * _glyphCell.cy; + pixels.bottom = rect.Bottom * _glyphCell.cy;*/ + const til::rectangle pixels = cells * _glyphCell; - D2D1_RECT_F draw = { 0 }; - draw.left = static_cast(pixels.left); - draw.top = static_cast(pixels.top); - draw.right = static_cast(pixels.right); - draw.bottom = static_cast(pixels.bottom); + const D2D1_RECT_F draw = pixels; _d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get()); @@ -1395,51 +1264,41 @@ enum class CursorPaintType return S_FALSE; } // Create rectangular block representing where the cursor can fill. - D2D1_RECT_F rect = { 0 }; - rect.left = static_cast(options.coordCursor.X * _glyphCell.cx); - rect.top = static_cast(options.coordCursor.Y * _glyphCell.cy); - rect.right = static_cast(rect.left + _glyphCell.cx); - rect.bottom = static_cast(rect.top + _glyphCell.cy); + D2D1_RECT_F rect = til::rectangle{ options.coordCursor } * _glyphCell; // If we're double-width, make it one extra glyph wider if (options.fIsDoubleWidth) { - rect.right += _glyphCell.cx; + rect.right += _glyphCell.width(); } CursorPaintType paintType = CursorPaintType::Fill; switch (options.cursorType) { - case CursorType::Legacy: - { + case CursorType::Legacy: { // Enforce min/max cursor height ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent); - - ulHeight = gsl::narrow((_glyphCell.cy * ulHeight) / 100); + ulHeight = (_glyphCell.height() * ulHeight) / 100; rect.top = rect.bottom - ulHeight; break; } - case CursorType::VerticalBar: - { + case CursorType::VerticalBar: { // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. // It's either the left + the proposed width from the ease of access setting, or // it's the right edge of the block cursor as a maximum. rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); break; } - case CursorType::Underscore: - { + case CursorType::Underscore: { rect.top = rect.bottom - 1; break; } - case CursorType::EmptyBox: - { + case CursorType::EmptyBox: { paintType = CursorPaintType::Outline; break; } - case CursorType::FullBox: - { + case CursorType::FullBox: { break; } default: @@ -1456,13 +1315,11 @@ enum class CursorPaintType switch (paintType) { - case CursorPaintType::Fill: - { + case CursorPaintType::Fill: { _d2dRenderTarget->FillRectangle(rect, brush.Get()); break; } - case CursorPaintType::Outline: - { + case CursorPaintType::Outline: { // DrawRectangle in straddles physical pixels in an attempt to draw a line // between them. To avoid this, bump the rectangle around by half the stroke width. rect.top += 0.5f; @@ -1576,6 +1433,7 @@ CATCH_RETURN() // Return Value: // - S_OK or relevant DirectX error [[nodiscard]] HRESULT DxEngine::UpdateFont(const FontInfoDesired& pfiFontInfoDesired, FontInfo& fiFontInfo) noexcept +try { RETURN_IF_FAILED(_GetProposedFont(pfiFontInfoDesired, fiFontInfo, @@ -1584,22 +1442,16 @@ CATCH_RETURN() _dwriteTextAnalyzer, _dwriteFontFace)); - try - { - const auto size = fiFontInfo.GetSize(); - - _glyphCell.cx = size.X; - _glyphCell.cy = size.Y; - } - CATCH_RETURN(); + _glyphCell = fiFontInfo.GetSize(); return S_OK; } +CATCH_RETURN(); [[nodiscard]] Viewport DxEngine::GetViewportInCharacters(const Viewport& viewInPixels) noexcept { - const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / _glyphCell.cx); - const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / _glyphCell.cy); + const short widthInChars = gsl::narrow_cast(viewInPixels.Width() / _glyphCell.width()); + const short heightInChars = gsl::narrow_cast(viewInPixels.Height() / _glyphCell.height()); return Viewport::FromDimensions(viewInPixels.Origin(), { widthInChars, heightInChars }); } @@ -1688,29 +1540,7 @@ float DxEngine::GetScaling() const noexcept // - Rectangle describing dirty area in characters. [[nodiscard]] std::vector DxEngine::GetDirtyArea() { - SMALL_RECT r; - r.Top = gsl::narrow(floor(_invalidRect.top / _glyphCell.cy)); - r.Left = gsl::narrow(floor(_invalidRect.left / _glyphCell.cx)); - r.Bottom = gsl::narrow(floor(_invalidRect.bottom / _glyphCell.cy)); - r.Right = gsl::narrow(floor(_invalidRect.right / _glyphCell.cx)); - - // Exclusive to inclusive - r.Bottom--; - r.Right--; - - return { r }; -} - -// Routine Description: -// - Gets COORD packed with shorts of each glyph (character) cell's -// height and width. -// Arguments: -// - -// Return Value: -// - Nearest integer short x and y values for each cell. -[[nodiscard]] COORD DxEngine::_GetFontSize() const noexcept -{ - return { gsl::narrow(_glyphCell.cx), gsl::narrow(_glyphCell.cy) }; + return _invalidMap.runs(); } // Routine Description: @@ -1720,10 +1550,12 @@ float DxEngine::GetScaling() const noexcept // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::GetFontSize(_Out_ COORD* const pFontSize) noexcept +try { - *pFontSize = _GetFontSize(); + *pFontSize = _glyphCell; return S_OK; } +CATCH_RETURN(); // Routine Description: // - Currently unused by this renderer. @@ -1733,30 +1565,28 @@ float DxEngine::GetScaling() const noexcept // Return Value: // - S_OK or relevant DirectWrite error. [[nodiscard]] HRESULT DxEngine::IsGlyphWideByFont(const std::wstring_view glyph, _Out_ bool* const pResult) noexcept +try { RETURN_HR_IF_NULL(E_INVALIDARG, pResult); - try - { - const Cluster cluster(glyph, 0); // columns don't matter, we're doing analysis not layout. + const Cluster cluster(glyph, 0); // columns don't matter, we're doing analysis not layout. - // Create the text layout - CustomTextLayout layout(_dwriteFactory.Get(), - _dwriteTextAnalyzer.Get(), - _dwriteTextFormat.Get(), - _dwriteFontFace.Get(), - { &cluster, 1 }, - _glyphCell.cx); + // Create the text layout + CustomTextLayout layout(_dwriteFactory.Get(), + _dwriteTextAnalyzer.Get(), + _dwriteTextFormat.Get(), + _dwriteFontFace.Get(), + { &cluster, 1 }, + _glyphCell.width()); - UINT32 columns = 0; - RETURN_IF_FAILED(layout.GetColumns(&columns)); + UINT32 columns = 0; + RETURN_IF_FAILED(layout.GetColumns(&columns)); - *pResult = columns != 1; - } - CATCH_RETURN(); + *pResult = columns != 1; return S_OK; } +CATCH_RETURN(); // Method Description: // - Updates the window's title string. @@ -2130,12 +1960,10 @@ float DxEngine::GetScaling() const noexcept switch (_chainMode) { - case SwapChainMode::ForHwnd: - { + case SwapChainMode::ForHwnd: { return D2D1::ColorF(rgb); } - case SwapChainMode::ForComposition: - { + case SwapChainMode::ForComposition: { // Get the A value we've snuck into the highest byte const BYTE a = ((color >> 24) & 0xFF); const float aFloat = a / 255.0f; diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index b8521d9b1ee..20550e9f31f 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -121,7 +121,7 @@ namespace Microsoft::Console::Render SwapChainMode _chainMode; HWND _hwndTarget; - SIZE _sizeTarget; + til::size _sizeTarget; int _dpi; float _scale; @@ -130,8 +130,8 @@ namespace Microsoft::Console::Render bool _isEnabled; bool _isPainting; - SIZE _displaySizePixels; - SIZE _glyphCell; + til::size _displaySizePixels; + til::size _glyphCell; D2D1_COLOR_F _defaultForegroundColor; D2D1_COLOR_F _defaultBackgroundColor; @@ -140,16 +140,8 @@ namespace Microsoft::Console::Render D2D1_COLOR_F _backgroundColor; D2D1_COLOR_F _selectionBackground; - [[nodiscard]] RECT _GetDisplayRect() const noexcept; - - bool _isInvalidUsed; - RECT _invalidRect; - SIZE _invalidScroll; - - void _InvalidOr(SMALL_RECT sr) noexcept; - void _InvalidOr(RECT rc) noexcept; - - void _InvalidOffset(POINT pt); + til::bitmap _invalidMap; + til::point _invalidScroll; bool _presentReady; RECT _presentDirty; @@ -244,9 +236,7 @@ namespace Microsoft::Console::Render ::Microsoft::WRL::ComPtr& textAnalyzer, ::Microsoft::WRL::ComPtr& fontFace) const noexcept; - [[nodiscard]] COORD _GetFontSize() const noexcept; - - [[nodiscard]] SIZE _GetClientSize() const noexcept; + [[nodiscard]] til::size _GetClientSize() const noexcept; [[nodiscard]] D2D1_COLOR_F _ColorFFromColorRef(const COLORREF color) noexcept; diff --git a/src/renderer/dx/precomp.h b/src/renderer/dx/precomp.h index 858413f4b40..df389166ab2 100644 --- a/src/renderer/dx/precomp.h +++ b/src/renderer/dx/precomp.h @@ -4,6 +4,7 @@ #pragma once // This includes support libraries from the CRT, STL, WIL, and GSL +#define BLOCK_TIL // We want to include it later, after DX. #include "LibraryIncludes.h" #include @@ -34,4 +35,7 @@ #include #include +// Re-include TIL at the bottom to gain DX superpowers. +#include "til.h" + #pragma hdrstop From b46b5d0f6f0e09ce874c9a1097f7c24dc5cd6c85 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 23 Mar 2020 14:31:12 -0700 Subject: [PATCH 02/29] it works! --- src/cascadia/TerminalCore/Terminal.cpp | 1 + src/inc/til.h | 2 +- src/inc/til/operators.h | 4 -- src/inc/til/rectangle.h | 30 +++++++++-- src/renderer/dx/DxRenderer.cpp | 69 ++++++++++++-------------- src/renderer/dx/precomp.h | 1 + 6 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 36daae54fce..8228abb8b65 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -705,6 +705,7 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition) if (notifyScroll) { + // TODO: don't do this, thanks migrie _buffer->GetRenderTarget().TriggerRedrawAll(); _NotifyScrollEvent(); } diff --git a/src/inc/til.h b/src/inc/til.h index 5204284a004..796a0b880b1 100644 --- a/src/inc/til.h +++ b/src/inc/til.h @@ -10,8 +10,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" diff --git a/src/inc/til/operators.h b/src/inc/til/operators.h index 109cd3e3576..7e18e33498a 100644 --- a/src/inc/til/operators.h +++ b/src/inc/til/operators.h @@ -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. diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 19f684f589f..e3d8013d6db 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -3,10 +3,6 @@ #pragma once -#include "point.h" -#include "size.h" -#include "some.h" - #ifdef UNIT_TESTING class RectangleTests; #endif @@ -636,6 +632,32 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return *this; } + // MUL will scale the entire rectangle up by the size factor + rectangle operator*(const size& size) + { + auto topLeft = _topLeft; + auto bottomRight = _bottomRight; + topLeft = topLeft * size; + bottomRight = bottomRight * size; + return til::rectangle{ topLeft, bottomRight }; + } + + // DIV will scale the entire rectangle down by the size factor, + // but rounds the bottom-right corner out. + rectangle operator/(const size& size) + { + 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 }; + } + #pragma endregion constexpr ptrdiff_t top() const noexcept diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index cf411554bf1..1877a85ce5d 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -625,8 +625,8 @@ void DxEngine::_ReleaseDeviceResources() noexcept return _dwriteFactory->CreateTextLayout(string, gsl::narrow(stringLength), _dwriteTextFormat.Get(), - _displaySizePixels.width(), - _glyphCell.height() != 0 ? _glyphCell.height() : _displaySizePixels.height(), + _displaySizePixels.width(), + _glyphCell.height() != 0 ? _glyphCell.height() : _displaySizePixels.height(), ppTextLayout); } @@ -647,9 +647,7 @@ void DxEngine::_ReleaseDeviceResources() noexcept [[nodiscard]] HRESULT DxEngine::SetWindowSize(const SIZE Pixels) noexcept { _sizeTarget = Pixels; - - RETURN_IF_FAILED(InvalidateAll()); - + _invalidMap.resize(_sizeTarget / _glyphCell, true); return S_OK; } @@ -683,7 +681,7 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() { RETURN_HR_IF_NULL(E_INVALIDARG, psrRegion); - _invalidMap.set(*psrRegion); + _invalidMap.set(Viewport::FromExclusive(*psrRegion).ToInclusive()); return S_OK; } @@ -710,14 +708,17 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::InvalidateSystem(const RECT* const prcDirtyClient) noexcept +try { - // TODO: this is not right. RETURN_HR_IF_NULL(E_INVALIDARG, prcDirtyClient); - _InvalidOr(*prcDirtyClient); + // Dirty client is in pixels. Use divide specialization against glyph factor to make conversion + // to cells. + _invalidMap.set(til::rectangle{ *prcDirtyClient } / _glyphCell); return S_OK; } +CATCH_RETURN(); // Routine Description: // - Invalidates a series of character rectangles @@ -857,21 +858,17 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept { RETURN_HR_IF(E_NOT_VALID_STATE, _isPainting); // invalid to start a paint while painting. - //#pragma warning(suppress : 26477 26485 26494 26482 26446 26447) // We don't control TraceLoggingWrite - // TraceLoggingWrite(g_hDxRenderProvider, - // "Invalid", - // TraceLoggingInt32(_invalidRect.bottom - _invalidRect.top, "InvalidHeight"), - // TraceLoggingInt32((_invalidRect.bottom - _invalidRect.top) / _glyphCell.cy, "InvalidHeightChars"), - // TraceLoggingInt32(_invalidRect.right - _invalidRect.left, "InvalidWidth"), - // TraceLoggingInt32((_invalidRect.right - _invalidRect.left) / _glyphCell.cx, "InvalidWidthChars"), - // TraceLoggingInt32(_invalidRect.left, "InvalidX"), - // TraceLoggingInt32(_invalidRect.left / _glyphCell.cx, "InvalidXChars"), - // TraceLoggingInt32(_invalidRect.top, "InvalidY"), - // TraceLoggingInt32(_invalidRect.top / _glyphCell.cy, "InvalidYChars"), - // TraceLoggingInt32(_invalidScroll.cx, "ScrollWidth"), - // TraceLoggingInt32(_invalidScroll.cx / _glyphCell.cx, "ScrollWidthChars"), - // TraceLoggingInt32(_invalidScroll.cy, "ScrollHeight"), - // TraceLoggingInt32(_invalidScroll.cy / _glyphCell.cy, "ScrollHeightChars")); + if (TraceLoggingProviderEnabled(g_hDxRenderProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + const auto invalidatedStr = _invalidMap.to_string(); + const auto invalidated = invalidatedStr.c_str(); + +#pragma warning(suppress : 26477 26485 26494 26482 26446 26447) // We don't control TraceLoggingWrite + TraceLoggingWrite(g_hDxRenderProvider, + "Invalid", + TraceLoggingWideString(invalidated), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } if (_isEnabled) { @@ -895,7 +892,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept _d2dRenderTarget.Reset(); // Change the buffer size and recreate the render target (and surface) - RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width(), clientSize.height(), DXGI_FORMAT_B8G8R8A8_UNORM, 0)); + RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width(), clientSize.height(), DXGI_FORMAT_B8G8R8A8_UNORM, 0)); RETURN_IF_FAILED(_PrepareRenderTarget()); // OK we made it past the parts that can cause errors. We can release our failure handler. @@ -934,7 +931,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept if (SUCCEEDED(hr)) { - if (_invalidScroll != til::point{ 0, 0 }) + /*if (_invalidScroll != til::point{ 0, 0 }) { _presentDirty = _invalidRect; @@ -954,7 +951,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept _presentParams.pScrollRect = nullptr; _presentParams.pScrollOffset = nullptr; } - } + }*/ _presentReady = true; } @@ -1074,12 +1071,17 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept { D2D1_COLOR_F nothing = { 0 }; + // Runs are counts of cells. + // Use a transform by the size of one cell to convert cells-to-pixels + // as we clear. + _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell)); for (const auto rect : _invalidMap.runs()) { _d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); _d2dRenderTarget->Clear(nothing); _d2dRenderTarget->PopAxisAlignedClip(); } + _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Identity()); return S_OK; } @@ -1226,16 +1228,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept _d2dBrushForeground->SetColor(_selectionBackground); const auto resetColorOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); - const til::rectangle cells{ rect }; - - /*RECT pixels; - pixels.left = rect.Left * _glyphCell.cx; - pixels.top = rect.Top * _glyphCell.cy; - pixels.right = rect.Right * _glyphCell.cx; - pixels.bottom = rect.Bottom * _glyphCell.cy;*/ - const til::rectangle pixels = cells * _glyphCell; - - const D2D1_RECT_F draw = pixels; + const D2D1_RECT_F draw = til::rectangle{ rect } *_glyphCell; _d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get()); @@ -1263,8 +1256,8 @@ enum class CursorPaintType { return S_FALSE; } - // Create rectangular block representing where the cursor can fill. - D2D1_RECT_F rect = til::rectangle{ options.coordCursor } * _glyphCell; + // Create rectangular block representing where the cursor can fill.s + D2D1_RECT_F rect = til::rectangle{ til::point{options.coordCursor} } *_glyphCell; // If we're double-width, make it one extra glyph wider if (options.fIsDoubleWidth) diff --git a/src/renderer/dx/precomp.h b/src/renderer/dx/precomp.h index df389166ab2..d77791ddc34 100644 --- a/src/renderer/dx/precomp.h +++ b/src/renderer/dx/precomp.h @@ -8,6 +8,7 @@ #include "LibraryIncludes.h" #include +#include #include "..\host\conddkrefs.h" #include From 78c1bc10f7680f3ada2e3ccdf317322e550afdcb Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 23 Mar 2020 15:26:10 -0700 Subject: [PATCH 03/29] try to get present1 working and get mad because std::vector is bad. --- src/inc/til/bitmap.h | 3 ++- src/renderer/dx/DxRenderer.cpp | 40 +++++++++++++++++++++++----------- src/renderer/dx/DxRenderer.hpp | 2 +- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 2070ee7a643..1f8cd624d61 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -281,7 +281,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" for (const auto pt : rc) { - til::at(_bits, _rc.index_of(pt)) = true; + auto idx = _rc.index_of(pt); + til::at(_bits, idx) = true; } _dirty |= rc; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 1877a85ce5d..e3abfe11cbe 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -369,7 +369,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept // You can find out how to install it here: // https://docs.microsoft.com/en-us/windows/uwp/gaming/use-the-directx-runtime-and-visual-studio-graphics-diagnostic-features // clang-format on - // D3D11_CREATE_DEVICE_DEBUG | + D3D11_CREATE_DEVICE_DEBUG | D3D11_CREATE_DEVICE_SINGLETHREADED; const std::array FeatureLevels{ D3D_FEATURE_LEVEL_11_1, @@ -918,6 +918,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept // Return Value: // - Any DirectX error, a memory error, etc. [[nodiscard]] HRESULT DxEngine::EndPaint() noexcept +try { RETURN_HR_IF(E_INVALIDARG, !_isPainting); // invalid to end paint when we're not painting @@ -931,27 +932,39 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept if (SUCCEEDED(hr)) { - /*if (_invalidScroll != til::point{ 0, 0 }) + if (_invalidScroll != til::point{ 0, 0 }) { - _presentDirty = _invalidRect; + // Copy `til::rectangles` into RECT map. + _presentDirty.assign(_invalidMap.begin(), _invalidMap.end()); + + // The scroll rect is the entire screen minus the revealed areas. + // Get the entire screen into a rectangle. + til::rectangle scrollArea{ _displaySizePixels }; + + // Reduce the size of the rectangle by the scroll. + scrollArea -= til::size{} - _invalidScroll; - const RECT display = _GetDisplayRect(); - SubtractRect(&_presentScroll, &display, &_presentDirty); - _presentOffset.x = _invalidScroll.cx; - _presentOffset.y = _invalidScroll.cy; + // Assign the area to the present storage + _presentScroll = scrollArea; - _presentParams.DirtyRectsCount = 1; - _presentParams.pDirtyRects = &_presentDirty; + // Pass the offset. + _presentOffset = _invalidScroll; + + // Now fill up the parameters structure from the member variables. + _presentParams.DirtyRectsCount = gsl::narrow(_presentDirty.size()); + _presentParams.pDirtyRects = _presentDirty.data(); _presentParams.pScrollOffset = &_presentOffset; _presentParams.pScrollRect = &_presentScroll; + // The scroll rect will be empty if we scrolled >= 1 full screen size. + // Present1 doesn't like that. So clear it out. Everything will be dirty anyway. if (IsRectEmpty(&_presentScroll)) { _presentParams.pScrollRect = nullptr; _presentParams.pScrollOffset = nullptr; } - }*/ + } _presentReady = true; } @@ -968,6 +981,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept return hr; } +CATCH_RETURN() // Routine Description: // - Copies the front surface of the swap chain (the one being displayed) @@ -1019,8 +1033,8 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept { HRESULT hr = S_OK; - hr = _dxgiSwapChain->Present(1, 0); - /*hr = _dxgiSwapChain->Present1(1, 0, &_presentParams);*/ + /*hr = _dxgiSwapChain->Present(1, 0);*/ + hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); if (FAILED(hr)) { @@ -1039,7 +1053,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept RETURN_IF_FAILED(_CopyFrontToBack()); _presentReady = false; - _presentDirty = { 0 }; + _presentDirty.clear(); _presentOffset = { 0 }; _presentScroll = { 0 }; _presentParams = { 0 }; diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 20550e9f31f..d319ae8fe65 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -144,7 +144,7 @@ namespace Microsoft::Console::Render til::point _invalidScroll; bool _presentReady; - RECT _presentDirty; + std::vector _presentDirty; RECT _presentScroll; POINT _presentOffset; DXGI_PRESENT_PARAMETERS _presentParams; From 777524a00f80c532ed45af2a022bb163a1923a1d Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 30 Mar 2020 15:27:28 -0700 Subject: [PATCH 04/29] Change invalidation to full rows to restore ligature drawing. --- src/inc/til/bitmap.h | 12 ++++++++---- src/renderer/dx/DxRenderer.cpp | 19 ++++++++++++++++--- src/renderer/dx/DxRenderer.hpp | 3 +++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index 4ba766a8d13..acafbaf1560 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -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 }; } @@ -283,10 +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) { - auto idx = _rc.index_of(pt); - til::at(_bits, idx) = true; + _bits.set(_rc.index_of(til::point{ rc.left(), row }), rc.width(), true); } _dirty |= rc; @@ -379,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; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index de69f3316e5..b0c04877fcb 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -65,6 +65,7 @@ using namespace Microsoft::Console::Types; // TODO GH 2683: The default constructor should not throw. DxEngine::DxEngine() : RenderEngineBase(), + _invalidateFullRows{ true }, _invalidMap{}, _invalidScroll{}, _presentParams{ 0 }, @@ -676,6 +677,18 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() return _dxgiSwapChain; } +til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const +{ + if (_invalidateFullRows) + { + return til::rectangle{ til::point{static_cast(0), rc.top()}, til::size{_invalidMap.size().width(), rc.height()} }; + } + else + { + return rc; + } +} + // Routine Description: // - Invalidates a rectangle described in characters // Arguments: @@ -686,7 +699,7 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() { RETURN_HR_IF_NULL(E_INVALIDARG, psrRegion); - _invalidMap.set(Viewport::FromExclusive(*psrRegion).ToInclusive()); + _invalidMap.set(_InvalidToFullRow(Viewport::FromExclusive(*psrRegion).ToInclusive())); return S_OK; } @@ -701,7 +714,7 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() { RETURN_HR_IF_NULL(E_INVALIDARG, pcoordCursor); - _invalidMap.set(*pcoordCursor); + _invalidMap.set(_InvalidToFullRow(til::rectangle{ *pcoordCursor, til::size{1, 1} })); return S_OK; } @@ -719,7 +732,7 @@ try // Dirty client is in pixels. Use divide specialization against glyph factor to make conversion // to cells. - _invalidMap.set(til::rectangle{ *prcDirtyClient } / _glyphCell); + _invalidMap.set(_InvalidToFullRow(til::rectangle{ *prcDirtyClient } / _glyphCell)); return S_OK; } diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index d319ae8fe65..4024b8e03b2 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -140,6 +140,7 @@ namespace Microsoft::Console::Render D2D1_COLOR_F _backgroundColor; D2D1_COLOR_F _selectionBackground; + bool _invalidateFullRows; til::bitmap _invalidMap; til::point _invalidScroll; @@ -238,6 +239,8 @@ namespace Microsoft::Console::Render [[nodiscard]] til::size _GetClientSize() const noexcept; + til::rectangle _InvalidToFullRow(const til::rectangle& rc) const; + [[nodiscard]] D2D1_COLOR_F _ColorFFromColorRef(const COLORREF color) noexcept; // Routine Description: From a15af9b3fd7c023fd380eb76b6dd2cdc34df63c0 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 30 Mar 2020 16:20:03 -0700 Subject: [PATCH 05/29] Add tests for til and round out the operators. --- src/inc/til/rectangle.h | 16 +++- src/til/ut_til/BitmapTests.cpp | 8 ++ src/til/ut_til/RectangleTests.cpp | 118 ++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index e3d8013d6db..f16f3c2db2d 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -633,7 +633,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // MUL will scale the entire rectangle up by the size factor - rectangle operator*(const size& size) + rectangle operator*(const size& size) const { auto topLeft = _topLeft; auto bottomRight = _bottomRight; @@ -642,9 +642,15 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return til::rectangle{ topLeft, bottomRight }; } + rectangle& operator*=(const size& size) + { + *this = *this * size; + return *this; + } + // DIV will scale the entire rectangle down by the size factor, // but rounds the bottom-right corner out. - rectangle operator/(const size& size) + rectangle operator/(const size& size) const { auto topLeft = _topLeft; auto bottomRight = _bottomRight; @@ -658,6 +664,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return til::rectangle{ topLeft, bottomRight }; } + rectangle& operator/=(const size& size) + { + *this = *this / size; + return *this; + } + #pragma endregion constexpr ptrdiff_t top() const noexcept diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index 648b32c7340..cb560d240aa 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -844,6 +844,14 @@ class BitmapTests VERIFY_IS_FALSE(bitmap.all()); } + TEST_METHOD(Size) + { + til::size sz{ 5, 10 }; + til::bitmap map{ sz }; + + VERIFY_ARE_EQUAL(sz, map.size()); + } + TEST_METHOD(Runs) { // This map --> Those runs diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 82aa5a69a75..e9c7e91311f 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -797,6 +797,124 @@ class RectangleTests } } + TEST_METHOD(MultiplicationSize) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Multiply by size to scale from cells to pixels"); + { + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; + const auto actual = start * scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Multiply by size with width way too big."); + { + const til::size scale{ std::numeric_limits().max(), static_cast(7) }; + + auto fn = [&]() { + const auto actual = start * scale; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"3.) Multiply by size with height way too big."); + { + const til::size scale{ static_cast(3), std::numeric_limits().max() }; + + auto fn = [&]() { + const auto actual = start * scale; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(MultiplicationSizeInplace) + { + const til::rectangle start{ 10, 20, 30, 40 }; + + Log::Comment(L"1.) Multiply by size to scale from cells to pixels"); + { + const til::size scale{ 3, 7 }; + const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; + auto actual = start; + actual *= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"2.) Multiply by size with width way too big."); + { + const til::size scale{ std::numeric_limits().max(), static_cast(7) }; + + auto fn = [&]() { + auto actual = start; + actual *= scale; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + + Log::Comment(L"3.) Multiply by size with height way too big."); + { + const til::size scale{ static_cast(3), std::numeric_limits().max() }; + + auto fn = [&]() { + auto actual = start; + actual *= scale; + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + + TEST_METHOD(DivisionSize) + { + const til::rectangle start{ 10, 20, 29, 40 }; + + Log::Comment(L"0.) Division by size to scale from pixels to cells"); + { + const til::size scale{ 3, 7 }; + + // Division is special. The top and left round down. + // The bottom and right round up. This is to ensure that the cells + // the smaller rectangle represents fully cover all the pixels + // of the larger rectangle. + // L: 10 / 3 = 3.333 --> round down --> 3 + // T: 20 / 7 = 2.857 --> round down --> 2 + // R: 29 / 3 = 9.667 --> round up ----> 10 + // B: 40 / 7 = 5.714 --> round up ----> 6 + const til::rectangle expected{ 3, 2, 10, 6 }; + const auto actual = start / scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + + TEST_METHOD(DivisionSizeInplace) + { + const til::rectangle start{ 10, 20, 29, 40 }; + + Log::Comment(L"0.) Division by size to scale from pixels to cells"); + { + const til::size scale{ 3, 7 }; + + // Division is special. The top and left round down. + // The bottom and right round up. This is to ensure that the cells + // the smaller rectangle represents fully cover all the pixels + // of the larger rectangle. + // L: 10 / 3 = 3.333 --> round down --> 3 + // T: 20 / 7 = 2.857 --> round down --> 2 + // R: 29 / 3 = 9.667 --> round up ----> 10 + // B: 40 / 7 = 5.714 --> round up ----> 6 + const til::rectangle expected{ 3, 2, 10, 6 }; + auto actual = start; + actual /= scale; + VERIFY_ARE_EQUAL(expected, actual); + } + } + TEST_METHOD(Top) { const til::rectangle rc{ 5, 10, 15, 20 }; From ac01c0bd8f5e3b8f2d4aa39639d25cfa72119002 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 30 Mar 2020 16:54:11 -0700 Subject: [PATCH 06/29] code format --- src/inc/til/rectangle.h | 6 ++-- src/renderer/dx/DxRenderer.cpp | 57 ++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index f16f3c2db2d..ec6ea185587 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -635,10 +635,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // MUL will scale the entire rectangle up by the size factor rectangle operator*(const size& size) const { - auto topLeft = _topLeft; - auto bottomRight = _bottomRight; - topLeft = topLeft * size; - bottomRight = bottomRight * size; + const auto topLeft = _topLeft * size; + const auto bottomRight = _bottomRight * size; return til::rectangle{ topLeft, bottomRight }; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index b0c04877fcb..b3b3e076991 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -370,7 +370,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept // You can find out how to install it here: // https://docs.microsoft.com/en-us/windows/uwp/gaming/use-the-directx-runtime-and-visual-studio-graphics-diagnostic-features // clang-format on - D3D11_CREATE_DEVICE_DEBUG | + // D3D11_CREATE_DEVICE_DEBUG | D3D11_CREATE_DEVICE_SINGLETHREADED; const std::array FeatureLevels{ D3D_FEATURE_LEVEL_11_1, @@ -424,7 +424,8 @@ void DxEngine::_ComputePixelShaderSettings() noexcept { switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { // use the HWND's dimensions for the swap chain dimensions. RECT rect = { 0 }; RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); @@ -453,7 +454,8 @@ void DxEngine::_ComputePixelShaderSettings() noexcept break; } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { // Use the given target size for compositions. SwapChainDesc.Width = _displaySizePixels.width(); SwapChainDesc.Height = _displaySizePixels.height(); @@ -681,7 +683,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const { if (_invalidateFullRows) { - return til::rectangle{ til::point{static_cast(0), rc.top()}, til::size{_invalidMap.size().width(), rc.height()} }; + return til::rectangle{ til::point{ static_cast(0), rc.top() }, til::size{ _invalidMap.size().width(), rc.height() } }; } else { @@ -714,7 +716,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const { RETURN_HR_IF_NULL(E_INVALIDARG, pcoordCursor); - _invalidMap.set(_InvalidToFullRow(til::rectangle{ *pcoordCursor, til::size{1, 1} })); + _invalidMap.set(_InvalidToFullRow(til::rectangle{ *pcoordCursor, til::size{ 1, 1 } })); return S_OK; } @@ -819,13 +821,15 @@ CATCH_RETURN(); { switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { RECT clientRect = { 0 }; LOG_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &clientRect)); return til::rectangle{ clientRect }.size(); } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { SIZE size = _sizeTarget; size.cx = static_cast(size.cx * _scale); size.cy = static_cast(size.cy * _scale); @@ -1051,7 +1055,6 @@ CATCH_RETURN() { HRESULT hr = S_OK; - /*hr = _dxgiSwapChain->Present(1, 0);*/ hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); if (FAILED(hr)) @@ -1109,6 +1112,11 @@ CATCH_RETURN() _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell)); for (const auto rect : _invalidMap.runs()) { + // Use aliased. + // For graphics reasons, it'll look better because it will ensure that + // the edges are cut nice and sharp (not blended by anti-aliasing). + // For performance reasons, it takes a lot less work to not + // do anti-alias blending. _d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); _d2dRenderTarget->Clear(nothing); _d2dRenderTarget->PopAxisAlignedClip(); @@ -1260,7 +1268,7 @@ CATCH_RETURN() _d2dBrushForeground->SetColor(_selectionBackground); const auto resetColorOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); - const D2D1_RECT_F draw = til::rectangle{ rect } *_glyphCell; + const D2D1_RECT_F draw = til::rectangle{ rect } * _glyphCell; _d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get()); @@ -1288,8 +1296,8 @@ enum class CursorPaintType { return S_FALSE; } - // Create rectangular block representing where the cursor can fill.s - D2D1_RECT_F rect = til::rectangle{ til::point{options.coordCursor} } *_glyphCell; + // Create rectangular block representing where the cursor can fill. + D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } } * _glyphCell; // If we're double-width, make it one extra glyph wider if (options.fIsDoubleWidth) @@ -1301,29 +1309,34 @@ enum class CursorPaintType switch (options.cursorType) { - case CursorType::Legacy: { + case CursorType::Legacy: + { // Enforce min/max cursor height ULONG ulHeight = std::clamp(options.ulCursorHeightPercent, s_ulMinCursorHeightPercent, s_ulMaxCursorHeightPercent); ulHeight = (_glyphCell.height() * ulHeight) / 100; rect.top = rect.bottom - ulHeight; break; } - case CursorType::VerticalBar: { + case CursorType::VerticalBar: + { // It can't be wider than one cell or we'll have problems in invalidation, so restrict here. // It's either the left + the proposed width from the ease of access setting, or // it's the right edge of the block cursor as a maximum. rect.right = std::min(rect.right, rect.left + options.cursorPixelWidth); break; } - case CursorType::Underscore: { + case CursorType::Underscore: + { rect.top = rect.bottom - 1; break; } - case CursorType::EmptyBox: { + case CursorType::EmptyBox: + { paintType = CursorPaintType::Outline; break; } - case CursorType::FullBox: { + case CursorType::FullBox: + { break; } default: @@ -1340,11 +1353,13 @@ enum class CursorPaintType switch (paintType) { - case CursorPaintType::Fill: { + case CursorPaintType::Fill: + { _d2dRenderTarget->FillRectangle(rect, brush.Get()); break; } - case CursorPaintType::Outline: { + case CursorPaintType::Outline: + { // DrawRectangle in straddles physical pixels in an attempt to draw a line // between them. To avoid this, bump the rectangle around by half the stroke width. rect.top += 0.5f; @@ -1985,10 +2000,12 @@ CATCH_RETURN(); switch (_chainMode) { - case SwapChainMode::ForHwnd: { + case SwapChainMode::ForHwnd: + { return D2D1::ColorF(rgb); } - case SwapChainMode::ForComposition: { + case SwapChainMode::ForComposition: + { // Get the A value we've snuck into the highest byte const BYTE a = ((color >> 24) & 0xFF); const float aFloat = a / 255.0f; From c4789836d5ae26bae5375fad37a3043f9913afe5 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 30 Mar 2020 17:09:21 -0700 Subject: [PATCH 07/29] code format and audit mode. --- src/renderer/dx/DxRenderer.cpp | 246 +++++++++++++++++---------------- src/renderer/dx/DxRenderer.hpp | 2 +- 2 files changed, 130 insertions(+), 118 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index b3b3e076991..959eeb468ef 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -349,6 +349,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept // Return Value: // - Could be any DirectX/D3D/D2D/DXGI/DWrite error or memory issue. [[nodiscard]] HRESULT DxEngine::_CreateDeviceResources(const bool createSwapChain) noexcept +try { if (_haveDeviceResources) { @@ -420,72 +421,68 @@ void DxEngine::_ComputePixelShaderSettings() noexcept SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_UNSPECIFIED; SwapChainDesc.Scaling = DXGI_SCALING_NONE; - try + switch (_chainMode) { - switch (_chainMode) - { - case SwapChainMode::ForHwnd: - { - // use the HWND's dimensions for the swap chain dimensions. - RECT rect = { 0 }; - RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); - - SwapChainDesc.Width = rect.right - rect.left; - SwapChainDesc.Height = rect.bottom - rect.top; - - // We can't do alpha for HWNDs. Set to ignore. It will fail otherwise. - SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_IGNORE; - const auto createSwapChainResult = _dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(), - _hwndTarget, - &SwapChainDesc, - nullptr, - nullptr, - &_dxgiSwapChain); - if (FAILED(createSwapChainResult)) - { - SwapChainDesc.Scaling = DXGI_SCALING_STRETCH; - RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(), - _hwndTarget, - &SwapChainDesc, - nullptr, - nullptr, - &_dxgiSwapChain)); - } - - break; - } - case SwapChainMode::ForComposition: + case SwapChainMode::ForHwnd: + { + // use the HWND's dimensions for the swap chain dimensions. + RECT rect = { 0 }; + RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect)); + + SwapChainDesc.Width = rect.right - rect.left; + SwapChainDesc.Height = rect.bottom - rect.top; + + // We can't do alpha for HWNDs. Set to ignore. It will fail otherwise. + SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_IGNORE; + const auto createSwapChainResult = _dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(), + _hwndTarget, + &SwapChainDesc, + nullptr, + nullptr, + &_dxgiSwapChain); + if (FAILED(createSwapChainResult)) { - // Use the given target size for compositions. - SwapChainDesc.Width = _displaySizePixels.width(); - SwapChainDesc.Height = _displaySizePixels.height(); - - // We're doing advanced composition pretty much for the purpose of pretty alpha, so turn it on. - SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED; - // It's 100% required to use scaling mode stretch for composition. There is no other choice. SwapChainDesc.Scaling = DXGI_SCALING_STRETCH; - - RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForComposition(_d3dDevice.Get(), - &SwapChainDesc, - nullptr, - &_dxgiSwapChain)); - break; - } - default: - THROW_HR(E_NOTIMPL); + RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(), + _hwndTarget, + &SwapChainDesc, + nullptr, + nullptr, + &_dxgiSwapChain)); } - if (_retroTerminalEffects) + break; + } + case SwapChainMode::ForComposition: + { + // Use the given target size for compositions. + SwapChainDesc.Width = _displaySizePixels.width(); + SwapChainDesc.Height = _displaySizePixels.height(); + + // We're doing advanced composition pretty much for the purpose of pretty alpha, so turn it on. + SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED; + // It's 100% required to use scaling mode stretch for composition. There is no other choice. + SwapChainDesc.Scaling = DXGI_SCALING_STRETCH; + + RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForComposition(_d3dDevice.Get(), + &SwapChainDesc, + nullptr, + &_dxgiSwapChain)); + break; + } + default: + THROW_HR(E_NOTIMPL); + } + + if (_retroTerminalEffects) + { + const HRESULT hr = _SetupTerminalEffects(); + if (FAILED(hr)) { - const HRESULT hr = _SetupTerminalEffects(); - if (FAILED(hr)) - { - _retroTerminalEffects = false; - LOG_HR_MSG(hr, "Failed to setup terminal effects. Disabling."); - } + _retroTerminalEffects = false; + LOG_HR_MSG(hr, "Failed to setup terminal effects. Disabling."); } } - CATCH_RETURN(); // With a new swap chain, mark the entire thing as invalid. RETURN_IF_FAILED(InvalidateAll()); @@ -515,6 +512,7 @@ void DxEngine::_ComputePixelShaderSettings() noexcept return S_OK; } +CATCH_RETURN(); [[nodiscard]] HRESULT DxEngine::_PrepareRenderTarget() noexcept { @@ -629,6 +627,7 @@ void DxEngine::_ReleaseDeviceResources() noexcept _In_reads_(stringLength) PCWCHAR string, _In_ size_t stringLength, _Out_ IDWriteTextLayout** ppTextLayout) noexcept +try { return _dwriteFactory->CreateTextLayout(string, gsl::narrow(stringLength), @@ -637,6 +636,7 @@ void DxEngine::_ReleaseDeviceResources() noexcept _glyphCell.height() != 0 ? _glyphCell.height() : _displaySizePixels.height(), ppTextLayout); } +CATCH_RETURN() // Routine Description: // - Sets the target window handle for our display pipeline @@ -653,11 +653,13 @@ void DxEngine::_ReleaseDeviceResources() noexcept } [[nodiscard]] HRESULT DxEngine::SetWindowSize(const SIZE Pixels) noexcept +try { _sizeTarget = Pixels; _invalidMap.resize(_sizeTarget / _glyphCell, true); return S_OK; } +CATCH_RETURN(); void DxEngine::SetCallback(std::function pfn) { @@ -698,6 +700,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::Invalidate(const SMALL_RECT* const psrRegion) noexcept +try { RETURN_HR_IF_NULL(E_INVALIDARG, psrRegion); @@ -705,6 +708,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const return S_OK; } +CATCH_RETURN() // Routine Description: // - Invalidates one specific character coordinate @@ -713,6 +717,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::InvalidateCursor(const COORD* const pcoordCursor) noexcept +try { RETURN_HR_IF_NULL(E_INVALIDARG, pcoordCursor); @@ -720,6 +725,7 @@ til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const return S_OK; } +CATCH_RETURN() // Routine Description: // - Invalidates a rectangle describing a pixel area on the display @@ -766,6 +772,8 @@ CATCH_RETURN(); [[nodiscard]] HRESULT DxEngine::InvalidateScroll(const COORD* const pcoordDelta) noexcept try { + RETURN_HR_IF(E_INVALIDARG, !pcoordDelta); + const til::point deltaCells{ *pcoordDelta }; if (deltaCells != til::point{ 0, 0 }) @@ -817,7 +825,7 @@ CATCH_RETURN(); // - // Return Value: // - X by Y area in pixels of the surface -[[nodiscard]] til::size DxEngine::_GetClientSize() const noexcept +[[nodiscard]] til::size DxEngine::_GetClientSize() const { switch (_chainMode) { @@ -877,6 +885,7 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept // Return Value: // - Any DirectX error, a memory error, etc. [[nodiscard]] HRESULT DxEngine::StartPaint() noexcept +try { RETURN_HR_IF(E_NOT_VALID_STATE, _isPainting); // invalid to start a paint while painting. @@ -894,44 +903,41 @@ void _ScaleByFont(RECT& cellsToPixels, SIZE fontSize) noexcept if (_isEnabled) { - try + const auto clientSize = _GetClientSize(); + if (!_haveDeviceResources) { - const auto clientSize = _GetClientSize(); - if (!_haveDeviceResources) - { - RETURN_IF_FAILED(_CreateDeviceResources(true)); - } - else if (_displaySizePixels != clientSize) - { - // OK, we're going to play a dangerous game here for the sake of optimizing resize - // First, set up a complete clear of all device resources if something goes terribly wrong. - auto resetDeviceResourcesOnFailure = wil::scope_exit([&]() noexcept { - _ReleaseDeviceResources(); - }); + RETURN_IF_FAILED(_CreateDeviceResources(true)); + } + else if (_displaySizePixels != clientSize) + { + // OK, we're going to play a dangerous game here for the sake of optimizing resize + // First, set up a complete clear of all device resources if something goes terribly wrong. + auto resetDeviceResourcesOnFailure = wil::scope_exit([&]() noexcept { + _ReleaseDeviceResources(); + }); - // Now let go of a few of the device resources that get in the way of resizing buffers in the swap chain - _dxgiSurface.Reset(); - _d2dRenderTarget.Reset(); + // Now let go of a few of the device resources that get in the way of resizing buffers in the swap chain + _dxgiSurface.Reset(); + _d2dRenderTarget.Reset(); - // Change the buffer size and recreate the render target (and surface) - RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width(), clientSize.height(), DXGI_FORMAT_B8G8R8A8_UNORM, 0)); - RETURN_IF_FAILED(_PrepareRenderTarget()); + // Change the buffer size and recreate the render target (and surface) + RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width(), clientSize.height(), DXGI_FORMAT_B8G8R8A8_UNORM, 0)); + RETURN_IF_FAILED(_PrepareRenderTarget()); - // OK we made it past the parts that can cause errors. We can release our failure handler. - resetDeviceResourcesOnFailure.release(); + // OK we made it past the parts that can cause errors. We can release our failure handler. + resetDeviceResourcesOnFailure.release(); - // And persist the new size. - _displaySizePixels = clientSize; - } - - _d2dRenderTarget->BeginDraw(); - _isPainting = true; + // And persist the new size. + _displaySizePixels = clientSize; } - CATCH_RETURN(); + + _d2dRenderTarget->BeginDraw(); + _isPainting = true; } return S_OK; } +CATCH_RETURN() // Routine Description: // - Ends batch drawing and captures any state necessary for presentation @@ -1103,6 +1109,7 @@ CATCH_RETURN() // Return Value: // - S_OK [[nodiscard]] HRESULT DxEngine::PaintBackground() noexcept +try { D2D1_COLOR_F nothing = { 0 }; @@ -1125,6 +1132,7 @@ CATCH_RETURN() return S_OK; } +CATCH_RETURN() // Routine Description: // - Places one line of text onto the screen at the given position @@ -1138,40 +1146,38 @@ CATCH_RETURN() COORD const coord, const bool /*trimLeft*/, const bool /*lineWrapped*/) noexcept +try { - try - { - // Calculate positioning of our origin. - D2D1_POINT_2F origin = til::point{ coord } * _glyphCell; - - // Create the text layout - CustomTextLayout layout(_dwriteFactory.Get(), - _dwriteTextAnalyzer.Get(), - _dwriteTextFormat.Get(), - _dwriteFontFace.Get(), - clusters, - _glyphCell.width()); - - // Get the baseline for this font as that's where we draw from - DWRITE_LINE_SPACING spacing; - RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); - - // Assemble the drawing context information - DrawingContext context(_d2dRenderTarget.Get(), - _d2dBrushForeground.Get(), - _d2dBrushBackground.Get(), - _dwriteFactory.Get(), - spacing, - _glyphCell, - D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); - - // Layout then render the text - RETURN_IF_FAILED(layout.Draw(&context, _customRenderer.Get(), origin.x, origin.y)); - } - CATCH_RETURN(); + // Calculate positioning of our origin. + const D2D1_POINT_2F origin = til::point{ coord } * _glyphCell; + + // Create the text layout + CustomTextLayout layout(_dwriteFactory.Get(), + _dwriteTextAnalyzer.Get(), + _dwriteTextFormat.Get(), + _dwriteFontFace.Get(), + clusters, + _glyphCell.width()); + + // Get the baseline for this font as that's where we draw from + DWRITE_LINE_SPACING spacing; + RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline)); + + // Assemble the drawing context information + DrawingContext context(_d2dRenderTarget.Get(), + _d2dBrushForeground.Get(), + _d2dBrushBackground.Get(), + _dwriteFactory.Get(), + spacing, + _glyphCell, + D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT); + + // Layout then render the text + RETURN_IF_FAILED(layout.Draw(&context, _customRenderer.Get(), origin.x, origin.y)); return S_OK; } +CATCH_RETURN() // Routine Description: // - Paints lines around cells (draws in pieces of the grid) @@ -1187,6 +1193,7 @@ CATCH_RETURN() COLORREF const color, size_t const cchLine, COORD const coordTarget) noexcept +try { const auto existingColor = _d2dBrushForeground->GetColor(); const auto restoreBrushOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); @@ -1254,6 +1261,7 @@ CATCH_RETURN() return S_OK; } +CATCH_RETURN() // Routine Description: // - Paints an overlay highlight on a portion of the frame to represent selected text @@ -1262,6 +1270,7 @@ CATCH_RETURN() // Return Value: // - S_OK or relevant DirectX error. [[nodiscard]] HRESULT DxEngine::PaintSelection(const SMALL_RECT rect) noexcept +try { const auto existingColor = _d2dBrushForeground->GetColor(); @@ -1274,6 +1283,7 @@ CATCH_RETURN() return S_OK; } +CATCH_RETURN() // Helper to choose which Direct2D method to use when drawing the cursor rectangle enum class CursorPaintType @@ -1290,6 +1300,7 @@ enum class CursorPaintType // Return Value: // - S_OK or relevant DirectX error. [[nodiscard]] HRESULT DxEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +try { // if the cursor is off, do nothing - it should not be visible. if (!options.isOn) @@ -1376,6 +1387,7 @@ enum class CursorPaintType return S_OK; } +CATCH_RETURN() // Routine Description: // - Paint terminal effects. diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 4024b8e03b2..b1ca9f6ae27 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -237,7 +237,7 @@ namespace Microsoft::Console::Render ::Microsoft::WRL::ComPtr& textAnalyzer, ::Microsoft::WRL::ComPtr& fontFace) const noexcept; - [[nodiscard]] til::size _GetClientSize() const noexcept; + [[nodiscard]] til::size _GetClientSize() const; til::rectangle _InvalidToFullRow(const til::rectangle& rc) const; From f39b352b608ad854bf689d9e7e2ae77e7a4fbb7a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 1 Apr 2020 10:39:43 -0700 Subject: [PATCH 08/29] Dustin's suggestion about helper for invalidating rectangles . --- src/renderer/dx/DxRenderer.cpp | 18 +++++++++--------- src/renderer/dx/DxRenderer.hpp | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 959eeb468ef..82a08ad059b 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -681,16 +681,16 @@ Microsoft::WRL::ComPtr DxEngine::GetSwapChain() return _dxgiSwapChain; } -til::rectangle DxEngine::_InvalidToFullRow(const til::rectangle& rc) const +void DxEngine::_InvalidateRectangle(const til::rectangle& rc) { + auto invalidate = rc; + if (_invalidateFullRows) { - return til::rectangle{ til::point{ static_cast(0), rc.top() }, til::size{ _invalidMap.size().width(), rc.height() } }; - } - else - { - return rc; + invalidate = til::rectangle{ til::point{ static_cast(0), rc.top() }, til::size{ _invalidMap.size().width(), rc.height() } }; } + + _invalidMap.set(invalidate); } // Routine Description: @@ -704,7 +704,7 @@ try { RETURN_HR_IF_NULL(E_INVALIDARG, psrRegion); - _invalidMap.set(_InvalidToFullRow(Viewport::FromExclusive(*psrRegion).ToInclusive())); + _InvalidateRectangle(Viewport::FromExclusive(*psrRegion).ToInclusive()); return S_OK; } @@ -721,7 +721,7 @@ try { RETURN_HR_IF_NULL(E_INVALIDARG, pcoordCursor); - _invalidMap.set(_InvalidToFullRow(til::rectangle{ *pcoordCursor, til::size{ 1, 1 } })); + _InvalidateRectangle(til::rectangle{ *pcoordCursor, til::size{ 1, 1 } }); return S_OK; } @@ -740,7 +740,7 @@ try // Dirty client is in pixels. Use divide specialization against glyph factor to make conversion // to cells. - _invalidMap.set(_InvalidToFullRow(til::rectangle{ *prcDirtyClient } / _glyphCell)); + _InvalidateRectangle(til::rectangle{ *prcDirtyClient } / _glyphCell); return S_OK; } diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index b1ca9f6ae27..d7d06c8afc9 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -239,7 +239,7 @@ namespace Microsoft::Console::Render [[nodiscard]] til::size _GetClientSize() const; - til::rectangle _InvalidToFullRow(const til::rectangle& rc) const; + void _InvalidateRectangle(const til::rectangle& rc); [[nodiscard]] D2D1_COLOR_F _ColorFFromColorRef(const COLORREF color) noexcept; From 0d863c2ed190969b1eb56707030e10e17c711206 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 1 Apr 2020 11:35:44 -0700 Subject: [PATCH 09/29] Fix selection issues. --- src/renderer/base/renderer.cpp | 7 +++++-- src/renderer/dx/DxRenderer.cpp | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index dfe4bdd3a6d..5701dade926 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -927,10 +927,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)); } } } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 82a08ad059b..929a325ef06 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1277,7 +1277,7 @@ try _d2dBrushForeground->SetColor(_selectionBackground); const auto resetColorOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); - const D2D1_RECT_F draw = til::rectangle{ rect } * _glyphCell; + const D2D1_RECT_F draw = til::rectangle{ Viewport::FromExclusive(rect).ToInclusive() } * _glyphCell; _d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get()); From ea8e379b06aaebabc62c36c7cad8c2b9b0828f3a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 1 Apr 2020 12:40:06 -0700 Subject: [PATCH 10/29] Restore scrolling by appropriately scaling dirty rectangles from cells to pixels. --- src/renderer/dx/DxRenderer.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 929a325ef06..3c02a87db0f 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -965,6 +965,11 @@ try // Copy `til::rectangles` into RECT map. _presentDirty.assign(_invalidMap.begin(), _invalidMap.end()); + // Scale all dirty rectangles into pixels + std::transform(_presentDirty.begin(), _presentDirty.end(), _presentDirty.begin(), [&](til::rectangle rc) { + return rc * _glyphCell; + }); + // The scroll rect is the entire screen minus the revealed areas. // Get the entire screen into a rectangle. til::rectangle scrollArea{ _displaySizePixels }; From 1d72e6b47ca561b4172327e2a041d1467eaca373 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 1 Apr 2020 14:16:14 -0700 Subject: [PATCH 11/29] Don't redraw everything. We're competent enough at scrolling in the DX renderer now to know what the signal means and figure it out. --- src/cascadia/TerminalCore/Terminal.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index bdcb0fbe444..cd832a75ccf 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -701,8 +701,7 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition) if (notifyScroll) { - // TODO: don't do this, thanks migrie - _buffer->GetRenderTarget().TriggerRedrawAll(); + _buffer->GetRenderTarget().TriggerScroll(); _NotifyScrollEvent(); } } @@ -715,7 +714,7 @@ void Terminal::UserScrollViewport(const int viewTop) // if viewTop > realTop, we want the offset to be 0. _scrollOffset = std::max(0, newDelta); - _buffer->GetRenderTarget().TriggerRedrawAll(); + _buffer->GetRenderTarget().TriggerScroll(); } int Terminal::GetScrollOffset() noexcept From 1d51d8608b9210fa383a1a6bd401e2bd9f882e62 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 3 Apr 2020 10:39:06 -0700 Subject: [PATCH 12/29] Report the delta when scroll buffer circles as there is no implicit viewport change. Add comments to the TriggerScroll calls to explain why each was chosen. --- src/cascadia/TerminalCore/Terminal.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index cd832a75ccf..880418546a2 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -701,7 +701,11 @@ void Terminal::_AdjustCursorPosition(const COORD proposedPosition) if (notifyScroll) { - _buffer->GetRenderTarget().TriggerScroll(); + // 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(newRows) }; + _buffer->GetRenderTarget().TriggerScroll(&delta); _NotifyScrollEvent(); } } @@ -714,6 +718,10 @@ void Terminal::UserScrollViewport(const int viewTop) // if viewTop > realTop, we want the offset to be 0. _scrollOffset = std::max(0, newDelta); + + // 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(); } From b8c66ddb07f890bba49aa155c99a9bd4aa2784dc Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 3 Apr 2020 10:43:12 -0700 Subject: [PATCH 13/29] Invalidate all with retro terminal effects experimental feature turned on as the shader is applied at a late stage in the pipeline and causes an unexpected graphics effect with differential drawing. --- src/renderer/dx/DxRenderer.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 3c02a87db0f..bcb0aeb8c5b 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -889,6 +889,15 @@ try { RETURN_HR_IF(E_NOT_VALID_STATE, _isPainting); // invalid to start a paint while painting. + // If retro terminal effects are on, we must invalidate everything for them to draw correctly. + // Yes, this will further impact the performance of retro terminal effects. + // But we're talking about running the entire display pipeline through a shader for + // cosmetic effect, so performance isn't likely the top concern with this feature. + if (_retroTerminalEffects) + { + _invalidMap.set_all(); + } + if (TraceLoggingProviderEnabled(g_hDxRenderProvider, WINEVENT_LEVEL_VERBOSE, 0)) { const auto invalidatedStr = _invalidMap.to_string(); From 1e5e4ebde8c485d6d967c84a19b0106c25988d85 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 3 Apr 2020 15:17:37 -0700 Subject: [PATCH 14/29] Don't use incremental drawing parameters on the first frame. Just use the old Present method. --- src/renderer/dx/DxRenderer.cpp | 19 ++++++++++++++++++- src/renderer/dx/DxRenderer.hpp | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index bcb0aeb8c5b..892794793da 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -68,6 +68,7 @@ DxEngine::DxEngine() : _invalidateFullRows{ true }, _invalidMap{}, _invalidScroll{}, + _firstFrame{ true }, _presentParams{ 0 }, _presentReady{ false }, _presentScroll{ 0 }, @@ -487,6 +488,9 @@ try // With a new swap chain, mark the entire thing as invalid. RETURN_IF_FAILED(InvalidateAll()); + // This is our first frame on this new target. + _firstFrame = true; + RETURN_IF_FAILED(_PrepareRenderTarget()); } @@ -938,6 +942,9 @@ try // And persist the new size. _displaySizePixels = clientSize; + + // Mark this as the first frame on the new target. We can't use incremental drawing on the first frame. + _firstFrame = true; } _d2dRenderTarget->BeginDraw(); @@ -1075,7 +1082,17 @@ CATCH_RETURN() { HRESULT hr = S_OK; - hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); + // On the first frame, we cannot use partial presentation. + // Just call the old Present method to present the entire frame. + if (_firstFrame) + { + hr = _dxgiSwapChain->Present(1, 0); + _firstFrame = false; + } + else + { + hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); + } if (FAILED(hr)) { diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index d7d06c8afc9..6fd98eb076b 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -140,6 +140,7 @@ namespace Microsoft::Console::Render D2D1_COLOR_F _backgroundColor; D2D1_COLOR_F _selectionBackground; + bool _firstFrame; bool _invalidateFullRows; til::bitmap _invalidMap; til::point _invalidScroll; From 46e526ca5fc724b476a9f48b59b602957fbc1965 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 6 Apr 2020 15:31:08 -0700 Subject: [PATCH 15/29] Add compensation for High DPI. Not perfect but way better. --- src/cascadia/TerminalCore/Terminal.cpp | 3 ++ src/inc/til/point.h | 16 +++++++++++ src/inc/til/rectangle.h | 38 +++++++++++++++++--------- src/renderer/dx/DxRenderer.cpp | 29 ++++++++++---------- 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 880418546a2..cf3770316b8 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -712,6 +712,9 @@ 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; diff --git a/src/inc/til/point.h b/src/inc/til/point.h index 81691d7b641..a44ca714b81 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -33,6 +33,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(y).AssignIfValid(&_y)); } + template + constexpr point(TilMath, T x, T y, std::enable_if_t())>, int> /*sentinel*/ = 0) : + point(, TilMath::template cast(y)) + { + } + constexpr point(ptrdiff_t x, ptrdiff_t y) noexcept : _x(x), _y(y) @@ -163,6 +169,16 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return *this; } + template + 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; diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index ec6ea185587..364b8939ec4 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -175,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 + constexpr rectangle(TilMath, const TOther& other, std::enable_if_t().Left)> && std::is_floating_point_v().Top)> && std::is_floating_point_v().Right)> && std::is_floating_point_v().Bottom)>, int> /*sentinel*/ = 0) : + rectangle(til::point{ TilMath{}, other.Left, other.Top }, til::point{ TilMath{}, other.Right, 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 + constexpr rectangle(TilMath, const TOther& other, std::enable_if_t().left)> && std::is_floating_point_v().top)> && std::is_floating_point_v().right)> && std::is_floating_point_v().bottom)>, int> /*sentinel*/ = 0) : + rectangle(til::point{ TilMath{}, other.left, other.top }, til::point{ TilMath{}, other.right, other.bottom }) + { + } + constexpr bool operator==(const rectangle& other) const noexcept { return _topLeft == other._topLeft && @@ -632,23 +648,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return *this; } - // MUL will scale the entire rectangle up by the size factor - rectangle operator*(const size& size) const + // 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 }; } - rectangle& operator*=(const size& size) - { - *this = *this * size; - return *this; - } - - // DIV will scale the entire rectangle down by the size factor, + // scale_down will scale the entire rectangle down by the size factor, // but rounds the bottom-right corner out. - rectangle operator/(const size& size) const + // This includes moving the origin. + rectangle scale_down(const size& size) const { auto topLeft = _topLeft; auto bottomRight = _bottomRight; @@ -662,10 +674,10 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return til::rectangle{ topLeft, bottomRight }; } - rectangle& operator/=(const size& size) + template + rectangle scale(TilMath, const float scale) const { - *this = *this / size; - return *this; + return til::rectangle{ _topLeft.scale(TilMath{}, scale), _bottomRight.scale(TilMath{}, scale) }; } #pragma endregion diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 892794793da..8c93d052bc9 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -571,7 +571,6 @@ CATCH_RETURN(); ::Microsoft::WRL::ComPtr sc2; RETURN_IF_FAILED(_dxgiSwapChain.As(&sc2)); - RETURN_IF_FAILED(sc2->SetMatrixTransform(&inverseScale)); } return S_OK; @@ -744,7 +743,7 @@ try // Dirty client is in pixels. Use divide specialization against glyph factor to make conversion // to cells. - _InvalidateRectangle(til::rectangle{ *prcDirtyClient } / _glyphCell); + _InvalidateRectangle(til::rectangle{ *prcDirtyClient }.scale_down(_glyphCell)); return S_OK; } @@ -782,13 +781,9 @@ try if (deltaCells != til::point{ 0, 0 }) { - const til::point deltaPixels = deltaCells * _glyphCell; - // Shift the contents of the map and fill in revealed area. _invalidMap.translate(deltaCells, true); - - // TODO: should we just maintain it all in cells? - _invalidScroll += deltaPixels; + _invalidScroll += deltaCells; } return S_OK; @@ -983,21 +978,25 @@ try // Scale all dirty rectangles into pixels std::transform(_presentDirty.begin(), _presentDirty.end(), _presentDirty.begin(), [&](til::rectangle rc) { - return rc * _glyphCell; + return rc.scale_up(_glyphCell).scale(til::math::rounding, _scale); }); - // The scroll rect is the entire screen minus the revealed areas. - // Get the entire screen into a rectangle. - til::rectangle scrollArea{ _displaySizePixels }; + // Invalid scroll is in characters, convert it to pixels. + const auto scrollPixels = (_invalidScroll * _glyphCell).scale(til::math::rounding, _scale); + + // The scroll rect is the entire field of cells, but in pixels. + til::rectangle scrollArea{ _invalidMap.size() * _glyphCell }; + + scrollArea = scrollArea.scale(til::math::ceiling, _scale); // Reduce the size of the rectangle by the scroll. - scrollArea -= til::size{} - _invalidScroll; + scrollArea -= til::size{} - scrollPixels; // Assign the area to the present storage _presentScroll = scrollArea; // Pass the offset. - _presentOffset = _invalidScroll; + _presentOffset = scrollPixels; // Now fill up the parameters structure from the member variables. _presentParams.DirtyRectsCount = gsl::narrow(_presentDirty.size()); @@ -1308,7 +1307,7 @@ try _d2dBrushForeground->SetColor(_selectionBackground); const auto resetColorOnExit = wil::scope_exit([&]() noexcept { _d2dBrushForeground->SetColor(existingColor); }); - const D2D1_RECT_F draw = til::rectangle{ Viewport::FromExclusive(rect).ToInclusive() } * _glyphCell; + const D2D1_RECT_F draw = til::rectangle{ Viewport::FromExclusive(rect).ToInclusive() }.scale_up(_glyphCell); _d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get()); @@ -1339,7 +1338,7 @@ try return S_FALSE; } // Create rectangular block representing where the cursor can fill. - D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } } * _glyphCell; + D2D1_RECT_F rect = til::rectangle{ til::point{ options.coordCursor } }.scale_up(_glyphCell); // If we're double-width, make it one extra glyph wider if (options.fIsDoubleWidth) From f11c7a17e3eb7de46f8b543570464a025f49daae Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 8 Apr 2020 10:58:41 -0700 Subject: [PATCH 16/29] Remove unused constructor in float. Add scale tests to Rectangle (remove MUL/DIV ones). Add scale test to point. --- src/inc/til/point.h | 6 --- src/til/ut_til/PointTests.cpp | 27 +++++++++++ src/til/ut_til/RectangleTests.cpp | 80 +++++++------------------------ 3 files changed, 45 insertions(+), 68 deletions(-) diff --git a/src/inc/til/point.h b/src/inc/til/point.h index a44ca714b81..d8331d96a71 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -33,12 +33,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(y).AssignIfValid(&_y)); } - template - constexpr point(TilMath, T x, T y, std::enable_if_t())>, int> /*sentinel*/ = 0) : - point(, TilMath::template cast(y)) - { - } - constexpr point(ptrdiff_t x, ptrdiff_t y) noexcept : _x(x), _y(y) diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index 8d57e5c32c8..764f4e5e334 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -417,6 +417,33 @@ class PointTests } } + TEST_METHOD(ScaleByFloat) + { + Log::Comment(L"0.) Scale that should be in bounds."); + { + const til::point pt{ 5, 10 }; + const float scale = 1.783f; + + const til::point expected{ static_cast(ceil(5*scale)), static_cast(ceil(10*scale)) }; + + const auto actual = pt.scale(til::math::ceiling, scale); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Scale results in value that is too large."); + { + const til::point pt{ 5, 10 }; + const float scale = std::numeric_limits().max(); + + auto fn = [&]() { + pt.scale(til::math::ceiling, scale); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(Division) { Log::Comment(L"0.) Division of two things that should be in bounds."); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index e9c7e91311f..23d82e9a6a2 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -797,7 +797,7 @@ class RectangleTests } } - TEST_METHOD(MultiplicationSize) + TEST_METHOD(ScaleUpSize) { const til::rectangle start{ 10, 20, 30, 40 }; @@ -805,7 +805,7 @@ class RectangleTests { const til::size scale{ 3, 7 }; const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; - const auto actual = start * scale; + const auto actual = start.scale_up(scale); VERIFY_ARE_EQUAL(expected, actual); } @@ -814,7 +814,7 @@ class RectangleTests const til::size scale{ std::numeric_limits().max(), static_cast(7) }; auto fn = [&]() { - const auto actual = start * scale; + const auto actual = start.scale_up(scale); }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); @@ -825,52 +825,14 @@ class RectangleTests const til::size scale{ static_cast(3), std::numeric_limits().max() }; auto fn = [&]() { - const auto actual = start * scale; + const auto actual = start.scale_up(scale); }; VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); } } - TEST_METHOD(MultiplicationSizeInplace) - { - const til::rectangle start{ 10, 20, 30, 40 }; - - Log::Comment(L"1.) Multiply by size to scale from cells to pixels"); - { - const til::size scale{ 3, 7 }; - const til::rectangle expected{ 10 * 3, 20 * 7, 30 * 3, 40 * 7 }; - auto actual = start; - actual *= scale; - VERIFY_ARE_EQUAL(expected, actual); - } - - Log::Comment(L"2.) Multiply by size with width way too big."); - { - const til::size scale{ std::numeric_limits().max(), static_cast(7) }; - - auto fn = [&]() { - auto actual = start; - actual *= scale; - }; - - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); - } - - Log::Comment(L"3.) Multiply by size with height way too big."); - { - const til::size scale{ static_cast(3), std::numeric_limits().max() }; - - auto fn = [&]() { - auto actual = start; - actual *= scale; - }; - - VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); - } - } - - TEST_METHOD(DivisionSize) + TEST_METHOD(ScaleDownSize) { const til::rectangle start{ 10, 20, 29, 40 }; @@ -887,32 +849,26 @@ class RectangleTests // R: 29 / 3 = 9.667 --> round up ----> 10 // B: 40 / 7 = 5.714 --> round up ----> 6 const til::rectangle expected{ 3, 2, 10, 6 }; - const auto actual = start / scale; + const auto actual = start.scale_down(scale); VERIFY_ARE_EQUAL(expected, actual); } } - TEST_METHOD(DivisionSizeInplace) + TEST_METHOD(ScaleByFloat) { - const til::rectangle start{ 10, 20, 29, 40 }; + const til::rectangle start{ 10, 20, 30, 40 }; - Log::Comment(L"0.) Division by size to scale from pixels to cells"); - { - const til::size scale{ 3, 7 }; + const float scale = 1.45f; - // Division is special. The top and left round down. - // The bottom and right round up. This is to ensure that the cells - // the smaller rectangle represents fully cover all the pixels - // of the larger rectangle. - // L: 10 / 3 = 3.333 --> round down --> 3 - // T: 20 / 7 = 2.857 --> round down --> 2 - // R: 29 / 3 = 9.667 --> round up ----> 10 - // B: 40 / 7 = 5.714 --> round up ----> 6 - const til::rectangle expected{ 3, 2, 10, 6 }; - auto actual = start; - actual /= scale; - VERIFY_ARE_EQUAL(expected, actual); - } + // This is not a test of the various TilMath rounding methods + // so we're only checking one here. + // Expected here is written based on the "ceiling" outcome. + const til::rectangle expected{ 15, 29, 44, 58 }; + + const auto actual = start.scale(til::math::ceiling, scale); + + VERIFY_ARE_EQUAL(actual, expected); + } TEST_METHOD(Top) From 162b516454e93d374ad967d66a5ce5384d11f3d0 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 8 Apr 2020 11:15:09 -0700 Subject: [PATCH 17/29] Tests for rectangle constructors from floats + code formatting pass. --- src/cascadia/TerminalCore/Terminal.cpp | 2 +- src/inc/til/point.h | 7 +- src/inc/til/rectangle.h | 4 +- src/til/ut_til/PointTests.cpp | 2 +- src/til/ut_til/RectangleTests.cpp | 98 +++++++++++++++++++++++++- 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index cf3770316b8..41619830dfd 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -713,7 +713,7 @@ 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(); + auto lock = LockForWriting(); const auto clampedNewTop = std::max(0, viewTop); const auto realTop = ViewStartIndex(); diff --git a/src/inc/til/point.h b/src/inc/til/point.h index d8331d96a71..62148786cfd 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -166,11 +166,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" template point scale(TilMath, const float scale) const { - struct { float x, y; } pt; + 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 ); + return til::point(TilMath(), pt); } point operator/(const point& other) const diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index 364b8939ec4..ba488ea7ea9 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -179,7 +179,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // a math type is required. template constexpr rectangle(TilMath, const TOther& other, std::enable_if_t().Left)> && std::is_floating_point_v().Top)> && std::is_floating_point_v().Right)> && std::is_floating_point_v().Bottom)>, int> /*sentinel*/ = 0) : - rectangle(til::point{ TilMath{}, other.Left, other.Top }, til::point{ TilMath{}, other.Right, other.Bottom }) + rectangle(til::point{ TilMath::template cast(other.Left), TilMath::template cast(other.Top) }, til::point{ TilMath::template cast(other.Right), TilMath::template cast(other.Bottom) }) { } @@ -187,7 +187,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // a math type is required. template constexpr rectangle(TilMath, const TOther& other, std::enable_if_t().left)> && std::is_floating_point_v().top)> && std::is_floating_point_v().right)> && std::is_floating_point_v().bottom)>, int> /*sentinel*/ = 0) : - rectangle(til::point{ TilMath{}, other.left, other.top }, til::point{ TilMath{}, other.right, other.bottom }) + rectangle(til::point{ TilMath::template cast(other.left), TilMath::template cast(other.top) }, til::point{ TilMath::template cast(other.right), TilMath::template cast(other.bottom) }) { } diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index 764f4e5e334..e9c3fb873c7 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -424,7 +424,7 @@ class PointTests const til::point pt{ 5, 10 }; const float scale = 1.783f; - const til::point expected{ static_cast(ceil(5*scale)), static_cast(ceil(10*scale)) }; + const til::point expected{ static_cast(ceil(5 * scale)), static_cast(ceil(10 * scale)) }; const auto actual = pt.scale(til::math::ceiling, scale); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index 23d82e9a6a2..b8bb7859937 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -868,7 +868,6 @@ class RectangleTests const auto actual = start.scale(til::math::ceiling, scale); VERIFY_ARE_EQUAL(actual, expected); - } TEST_METHOD(Top) @@ -1435,4 +1434,101 @@ class RectangleTests } #pragma endregion + + template + struct RectangleTypeWithLowercase + { + T left, top, right, bottom; + }; + template + struct RectangleTypeWithCapitalization + { + T Left, Top, Right, Bottom; + }; + TEST_METHOD(CastFromFloatWithMathTypes) + { + RectangleTypeWithLowercase lowerFloatIntegral{ 1.f, 2.f, 3.f, 4.f }; + RectangleTypeWithLowercase lowerFloat{ 1.6f, 2.4f, 3.2f, 4.8f }; + RectangleTypeWithCapitalization capitalDoubleIntegral{ 3., 4., 5., 6. }; + RectangleTypeWithCapitalization capitalDouble{ 3.6, 4.4, 5.7, 6.3 }; + Log::Comment(L"0.) Ceiling"); + { + { + til::rectangle converted{ til::math::ceiling, lowerFloatIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::ceiling, lowerFloat }; + VERIFY_ARE_EQUAL((til::rectangle{ 2, 3, 4, 5 }), converted); + } + { + til::rectangle converted{ til::math::ceiling, capitalDoubleIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + { + til::rectangle converted{ til::math::ceiling, capitalDouble }; + VERIFY_ARE_EQUAL((til::rectangle{ 4, 5, 6, 7 }), converted); + } + } + + Log::Comment(L"1.) Flooring"); + { + { + til::rectangle converted{ til::math::flooring, lowerFloatIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::flooring, lowerFloat }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::flooring, capitalDoubleIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + { + til::rectangle converted{ til::math::flooring, capitalDouble }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + } + + Log::Comment(L"2.) Rounding"); + { + { + til::rectangle converted{ til::math::rounding, lowerFloatIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::rounding, lowerFloat }; + VERIFY_ARE_EQUAL((til::rectangle{ 2, 2, 3, 5 }), converted); + } + { + til::rectangle converted{ til::math::rounding, capitalDoubleIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + { + til::rectangle converted{ til::math::rounding, capitalDouble }; + VERIFY_ARE_EQUAL((til::rectangle{ 4, 4, 6, 6 }), converted); + } + } + + Log::Comment(L"3.) Truncating"); + { + { + til::rectangle converted{ til::math::truncating, lowerFloatIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::truncating, lowerFloat }; + VERIFY_ARE_EQUAL((til::rectangle{ 1, 2, 3, 4 }), converted); + } + { + til::rectangle converted{ til::math::truncating, capitalDoubleIntegral }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + { + til::rectangle converted{ til::math::truncating, capitalDouble }; + VERIFY_ARE_EQUAL((til::rectangle{ 3, 4, 5, 6 }), converted); + } + } + } }; From 45f81446dafb0f721c703f0f628a66c1505a7e1a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 8 Apr 2020 11:28:22 -0700 Subject: [PATCH 18/29] change to constexpr --- src/til/ut_til/PointTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index e9c3fb873c7..a1293e90cc3 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -434,7 +434,7 @@ class PointTests Log::Comment(L"1.) Scale results in value that is too large."); { const til::point pt{ 5, 10 }; - const float scale = std::numeric_limits().max(); + constexpr float scale = std::numeric_limits().max(); auto fn = [&]() { pt.scale(til::math::ceiling, scale); From 351dc9c6acd42fea4b4ac96e6ea755486bb2a1b1 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Wed, 8 Apr 2020 12:54:06 -0700 Subject: [PATCH 19/29] GetClient for composition now uses size scale factor directly (and I implemented scale on til::size mostly as a copy of til::point's implementation and copied the test.) --- src/inc/til/size.h | 13 +++++++++++++ src/renderer/dx/DxRenderer.cpp | 5 +---- src/til/ut_til/SizeTests.cpp | 27 +++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 67a41023822..b7d71dbd46b 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -126,6 +126,19 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" return size{ width, height }; } + template + 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; diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 8c93d052bc9..c1e0b67edf4 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -837,10 +837,7 @@ CATCH_RETURN(); } case SwapChainMode::ForComposition: { - SIZE size = _sizeTarget; - size.cx = static_cast(size.cx * _scale); - size.cy = static_cast(size.cy * _scale); - return size; + return _sizeTarget.scale(til::math::ceiling, _scale); } default: FAIL_FAST_HR(E_NOTIMPL); diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index 4b708edf373..fb0bb1ecfc7 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -309,6 +309,33 @@ class SizeTests } } + TEST_METHOD(ScaleByFloat) + { + Log::Comment(L"0.) Scale that should be in bounds."); + { + const til::size sz{ 5, 10 }; + const float scale = 1.783f; + + const til::size expected{ static_cast(ceil(5 * scale)), static_cast(ceil(10 * scale)) }; + + const auto actual = sz.scale(til::math::ceiling, scale); + + VERIFY_ARE_EQUAL(expected, actual); + } + + Log::Comment(L"1.) Scale results in value that is too large."); + { + const til::size sz{ 5, 10 }; + constexpr float scale = std::numeric_limits().max(); + + auto fn = [&]() { + sz.scale(til::math::ceiling, scale); + }; + + VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_ABORT; }); + } + } + TEST_METHOD(Division) { Log::Comment(L"0.) Division of two things that should be in bounds."); From d42f60922f2e6eaae2658b319130ad401b4e47f7 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 08:47:09 -0700 Subject: [PATCH 20/29] Adjust the present1/present logic so it will retry if present1 fails to maintain robustness. --- src/renderer/dx/DxRenderer.cpp | 51 ++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index c1e0b67edf4..3e395df3a57 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1078,32 +1078,61 @@ CATCH_RETURN() { HRESULT hr = S_OK; - // On the first frame, we cannot use partial presentation. - // Just call the old Present method to present the entire frame. - if (_firstFrame) + bool recreate = false; + + // On anything but the first frame, try partial presentation. + // We'll do it first because if it fails, we'll try again with full presentation. + if (!_firstFrame) { - hr = _dxgiSwapChain->Present(1, 0); - _firstFrame = false; + hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); + + // These two error codes are indicated for destroy-and-recreate + // If we were told to destroy-and-recreate, we're going to skip straight into doing that + // and not try again with full presentation. + recreate = hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET; + + // Log this as we actually don't expect it to happen, we just will try again + // below for robustness of our drawing. + if (FAILED(hr) && !recreate) + { + LOG_HR(hr); + } } - else + + // If it's the first frame through, we cannot do partial presentation. + // Also if partial presentation failed above and we weren't told to skip straight to + // device recreation. + // In both of these circumstances, do a full presentation. + if (_firstFrame || (FAILED(hr) && !recreate)) { - hr = _dxgiSwapChain->Present1(1, 0, &_presentParams); + hr = _dxgiSwapChain->Present(1, 0); + _firstFrame = false; + + // These two error codes are indicated for destroy-and-recreate + recreate = hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET; } + // Now check for failure cases from either presentation mode. if (FAILED(hr)) { - // These two error codes are indicated for destroy-and-recreate - if (hr == DXGI_ERROR_DEVICE_REMOVED || hr == DXGI_ERROR_DEVICE_RESET) + // If we were told to recreate the device surface, do that. + if (recreate) { // We don't need to end painting here, as the renderer has done it for us. _ReleaseDeviceResources(); FAIL_FAST_IF_FAILED(InvalidateAll()); return E_PENDING; // Indicate a retry to the renderer. } - - FAIL_FAST_HR(hr); + // Otherwise, we don't know what to do with this error. Report it. + else + { + FAIL_FAST_HR(hr); + } } + // Finally copy the front image (being presented now) onto the backing buffer + // (where we are about to draw the next frame) so we can draw only the differences + // next frame. RETURN_IF_FAILED(_CopyFrontToBack()); _presentReady = false; From 3e060e621072785dd719c38a8088e574b869e3d8 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 09:31:40 -0700 Subject: [PATCH 21/29] Literally clear everything, including the gutters, when the complete map is invalid. --- src/renderer/dx/DxRenderer.cpp | 37 ++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 3e395df3a57..0e1cc65c28a 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1169,22 +1169,33 @@ try { D2D1_COLOR_F nothing = { 0 }; - // Runs are counts of cells. - // Use a transform by the size of one cell to convert cells-to-pixels - // as we clear. - _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell)); - for (const auto rect : _invalidMap.runs()) + // If the entire thing is invalid, just use one big clear operation. + // This will also hit the gutters outside the usual paintable area. + // Invalidating everything is supposed to happen with resizes of the + // entire canvas, changes of the font, and other such adjustments. + if (_invalidMap.all()) { - // Use aliased. - // For graphics reasons, it'll look better because it will ensure that - // the edges are cut nice and sharp (not blended by anti-aliasing). - // For performance reasons, it takes a lot less work to not - // do anti-alias blending. - _d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); _d2dRenderTarget->Clear(nothing); - _d2dRenderTarget->PopAxisAlignedClip(); } - _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Identity()); + else + { + // Runs are counts of cells. + // Use a transform by the size of one cell to convert cells-to-pixels + // as we clear. + _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell)); + for (const auto rect : _invalidMap.runs()) + { + // Use aliased. + // For graphics reasons, it'll look better because it will ensure that + // the edges are cut nice and sharp (not blended by anti-aliasing). + // For performance reasons, it takes a lot less work to not + // do anti-alias blending. + _d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED); + _d2dRenderTarget->Clear(nothing); + _d2dRenderTarget->PopAxisAlignedClip(); + } + _d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Identity()); + } return S_OK; } From 4eddc8d50c832c65846bbe3944c897ca22f50c19 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 10:58:55 -0700 Subject: [PATCH 22/29] Fix selection invalidation issues. This was always broken but the nature of union rect invalidation meant it was never a problem before. --- src/renderer/base/renderer.cpp | 64 ++++++++++++++++++++++++++++++---- src/renderer/base/renderer.hpp | 1 + 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 5701dade926..39663e0a9cc 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -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{Viewport::FromInclusive(_srViewportPrevious).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)); @@ -330,13 +346,20 @@ 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)); - }); - _srViewportPrevious = srNewViewport; + if (coordDelta.X != 0 || coordDelta.Y != 0) + { + std::for_each(_rgpEngines.begin(), _rgpEngines.end(), [&](IRenderEngine* const pEngine) { + LOG_IF_FAILED(pEngine->UpdateViewport(srNewViewport)); + LOG_IF_FAILED(pEngine->InvalidateScroll(&coordDelta)); + }); + _srViewportPrevious = srNewViewport; + + _ScrollPreviousSelection(coordDelta); + + return true; + } - return coordDelta.X != 0 || coordDelta.Y != 0; + return false; } // Routine Description: @@ -369,6 +392,8 @@ void Renderer::TriggerScroll(const COORD* const pcoordDelta) LOG_IF_FAILED(pEngine->InvalidateScroll(pcoordDelta)); }); + _ScrollPreviousSelection(*pcoordDelta); + _NotifyPaintFrame(); } @@ -1005,6 +1030,33 @@ std::vector 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: +// - - 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. diff --git a/src/renderer/base/renderer.hpp b/src/renderer/base/renderer.hpp index 3e02cc7aea1..594763edd19 100644 --- a/src/renderer/base/renderer.hpp +++ b/src/renderer/base/renderer.hpp @@ -119,6 +119,7 @@ namespace Microsoft::Console::Render SMALL_RECT _srViewportPrevious; std::vector _GetSelectionRects() const; + void _ScrollPreviousSelection(const til::point delta); std::vector _previousSelection; [[nodiscard]] HRESULT _PaintTitle(IRenderEngine* const pEngine); From f22577d46dfe4e647ae844f53c136247610569b2 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 12:37:59 -0700 Subject: [PATCH 23/29] Fix issue with algorithm around re-using known previous viewport as restriction area because it isn't populated until the first scroll operation. --- src/renderer/base/renderer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 39663e0a9cc..433902286bc 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -308,9 +308,9 @@ void Renderer::TriggerSelection() { // 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{Viewport::FromInclusive(_srViewportPrevious).Dimensions()} }; + const til::rectangle viewport{ til::size{_pData->GetViewport().Dimensions()} }; // Intersect them so we only invalidate things that are still visible. rc &= viewport; From f4b39ec608e06bb9aca9c672d1659fc27f38e65a Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 12:38:47 -0700 Subject: [PATCH 24/29] code format --- src/renderer/base/renderer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 433902286bc..8e7f263c287 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -308,9 +308,9 @@ void Renderer::TriggerSelection() { // 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()} }; + const til::rectangle viewport{ til::size{ _pData->GetViewport().Dimensions() } }; // Intersect them so we only invalidate things that are still visible. rc &= viewport; @@ -351,7 +351,7 @@ bool Renderer::_CheckViewportAndScroll() std::for_each(_rgpEngines.begin(), _rgpEngines.end(), [&](IRenderEngine* const pEngine) { LOG_IF_FAILED(pEngine->UpdateViewport(srNewViewport)); LOG_IF_FAILED(pEngine->InvalidateScroll(&coordDelta)); - }); + }); _srViewportPrevious = srNewViewport; _ScrollPreviousSelection(coordDelta); From 8abf479bc3fe87f719e2ac45c44c2ac2685755bf Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 13:00:25 -0700 Subject: [PATCH 25/29] Use GDI Classic measuring mode to eliminate blended half-pixels at the bottom of characters on every other line when rendering High DPI text. --- src/renderer/dx/CustomTextLayout.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index 6dbaa894a44..d6727a6e0bb 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -760,10 +760,15 @@ CATCH_RETURN(); } // Try to draw it + // NOTE: Using GDI_CLASSIC as the DWRITE_MEASURING_MODE will ensure that + // we're not blurring/blending pixels vertically as the D2D Render Target + // is scaled from Device-Independent-Pixels (DIPs) to the actual monitor + // pixels as dictated by `SetDpi()`. The classic mode appears to take + // the actual display characteristics into account and doesn't get half-pixels. RETURN_IF_FAILED(renderer->DrawGlyphRun(clientDrawingContext, mutableOrigin.x, mutableOrigin.y, - DWRITE_MEASURING_MODE_NATURAL, + DWRITE_MEASURING_MODE_GDI_CLASSIC, &glyphRun, &glyphRunDescription, nullptr)); From d0e6dd103542f49d9eb652ad0864146154dfc0a6 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 14:23:40 -0700 Subject: [PATCH 26/29] separate concerns between updating viewport and scrolling. --- src/renderer/base/renderer.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index 8e7f263c287..812c966dfbd 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -346,13 +346,19 @@ bool Renderer::_CheckViewportAndScroll() coordDelta.X = srOldViewport.Left - srNewViewport.Left; coordDelta.Y = srOldViewport.Top - srNewViewport.Top; + for (auto engine : _rgpEngines) + { + LOG_IF_FAILED(engine->UpdateViewport(srNewViewport)); + } + + _srViewportPrevious = srNewViewport; + if (coordDelta.X != 0 || coordDelta.Y != 0) { - std::for_each(_rgpEngines.begin(), _rgpEngines.end(), [&](IRenderEngine* const pEngine) { - LOG_IF_FAILED(pEngine->UpdateViewport(srNewViewport)); - LOG_IF_FAILED(pEngine->InvalidateScroll(&coordDelta)); - }); - _srViewportPrevious = srNewViewport; + for (auto engine : _rgpEngines) + { + LOG_IF_FAILED(engine->InvalidateScroll(&coordDelta)); + } _ScrollPreviousSelection(coordDelta); From b06720a91497410b95c7bd234335165d0c3bf155 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 15:42:22 -0700 Subject: [PATCH 27/29] Disable incremental for High DPI as it doesn't work with the implicit render target scaling. --- src/renderer/dx/DxRenderer.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 0e1cc65c28a..99a420246f1 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -894,6 +894,13 @@ try _invalidMap.set_all(); } + // If we're doing High DPI, we must invalidate everything for it to draw correctly. + // TODO: GH: 5320 - Remove implicit DPI scaling in D2D target to enable pixel perfect High DPI + if (_scale != 1.0f) + { + _invalidMap.set_all(); + } + if (TraceLoggingProviderEnabled(g_hDxRenderProvider, WINEVENT_LEVEL_VERBOSE, 0)) { const auto invalidatedStr = _invalidMap.to_string(); From 1207e8010007fb734b9facc97a349318cf4cf9d5 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 15:44:59 -0700 Subject: [PATCH 28/29] Put natural rendering back. --- src/renderer/dx/CustomTextLayout.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/renderer/dx/CustomTextLayout.cpp b/src/renderer/dx/CustomTextLayout.cpp index d6727a6e0bb..6dbaa894a44 100644 --- a/src/renderer/dx/CustomTextLayout.cpp +++ b/src/renderer/dx/CustomTextLayout.cpp @@ -760,15 +760,10 @@ CATCH_RETURN(); } // Try to draw it - // NOTE: Using GDI_CLASSIC as the DWRITE_MEASURING_MODE will ensure that - // we're not blurring/blending pixels vertically as the D2D Render Target - // is scaled from Device-Independent-Pixels (DIPs) to the actual monitor - // pixels as dictated by `SetDpi()`. The classic mode appears to take - // the actual display characteristics into account and doesn't get half-pixels. RETURN_IF_FAILED(renderer->DrawGlyphRun(clientDrawingContext, mutableOrigin.x, mutableOrigin.y, - DWRITE_MEASURING_MODE_GDI_CLASSIC, + DWRITE_MEASURING_MODE_NATURAL, &glyphRun, &glyphRunDescription, nullptr)); From 24ecd5b0581755fc91a967755eaa055f9b2d557d Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Fri, 10 Apr 2020 16:20:06 -0700 Subject: [PATCH 29/29] Actually lock in selection code so it doesn't pull the rug out from under the renderer while it's operating. --- src/cascadia/TerminalControl/TermControl.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 51937ef8274..2078a2e1454 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -937,6 +937,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); @@ -978,6 +980,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _lastMouseClickTimestamp = point.Timestamp(); _lastMouseClickPos = cursorPosition; } + _renderer->TriggerSelection(); } else if (point.Properties().IsRightButtonPressed()) @@ -1036,6 +1039,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation if (point.Properties().IsLeftButtonPressed()) { + auto lock = _terminal->LockForWriting(); + const auto cursorPosition = point.Position(); if (_singleClickTouchdownPos)