From 7e8f0398c3c851c96189b1ef72f1c82feda7510e Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 23 Mar 2020 09:55:01 -0700 Subject: [PATCH 01/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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/44] 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) From 0d5cfc7e85a8ea9eb795c968576ffc08bb33ff6e Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 15:03:29 -0700 Subject: [PATCH 30/44] Adjust DxRenderer and TerminalControl to change the area of concern over DPI from the D2DRenderTarget into the font choice itself. --- src/cascadia/TerminalControl/TermControl.cpp | 25 +++++++------------- src/renderer/dx/DxRenderer.cpp | 15 ++++-------- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 2078a2e1454..1865bbc29c0 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -518,8 +518,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return false; } - const auto windowWidth = SwapChainPanel().ActualWidth(); // Width() and Height() are NaN? - const auto windowHeight = SwapChainPanel().ActualHeight(); + const auto windowWidth = SwapChainPanel().ActualWidth() * SwapChainPanel().CompositionScaleX(); // Width() and Height() are NaN? + const auto windowHeight = SwapChainPanel().ActualHeight() * SwapChainPanel().CompositionScaleY(); if (windowWidth == 0 || windowHeight == 0) { @@ -1636,9 +1636,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto scale = sender.CompositionScaleX(); const auto dpi = (int)(scale * USER_DEFAULT_SCREEN_DPI); - // TODO: MSFT: 21169071 - Shouldn't this all happen through _renderer and trigger the invalidate automatically on DPI change? THROW_IF_FAILED(_renderEngine->UpdateDpi(dpi)); - _renderer->TriggerRedrawAll(); } } @@ -1979,23 +1977,14 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation THROW_IF_FAILED(dxEngine->UpdateDpi(dpi)); THROW_IF_FAILED(dxEngine->UpdateFont(desiredFont, actualFont)); - const float scale = dxEngine->GetScaling(); const auto fontSize = actualFont.GetSize(); - - // Manually multiply by the scaling factor. The DX engine doesn't - // actually store the scaled font size in the fontInfo.GetSize() - // property when the DX engine is in Composition mode (which it is for - // the Terminal). At runtime, this is fine, as we'll transform - // everything by our scaling, so it'll work out. However, right now we - // need to get the exact pixel count. - const float fFontWidth = gsl::narrow_cast(fontSize.X * scale); - const float fFontHeight = gsl::narrow_cast(fontSize.Y * scale); + const auto scale = dxEngine->GetScaling(); // UWP XAML scrollbars aren't guaranteed to be the same size as the // ComCtl scrollbars, but it's certainly close enough. const auto scrollbarSize = GetSystemMetricsForDpi(SM_CXVSCROLL, dpi); - double width = cols * fFontWidth; + double width = cols * fontSize.X; // Reserve additional space if scrollbar is intended to be visible if (settings.ScrollState() == ScrollbarState::Visible) @@ -2003,7 +1992,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation width += scrollbarSize; } - double height = rows * fFontHeight; + double height = rows * fontSize.Y; auto thickness = _ParseThicknessFromPadding(settings.Padding()); // GH#2061 - make sure to account for the size the padding _will be_ scaled to width += scale * (thickness.Left + thickness.Right); @@ -2197,6 +2186,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - the corresponding viewport terminal position for the given Point parameter const COORD TermControl::_GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition) { + // compensate for DPI scenarios. + cursorPosition.X *= SwapChainPanel().CompositionScaleX(); + cursorPosition.Y *= SwapChainPanel().CompositionScaleY(); + // Exclude padding from cursor position calculation COORD terminalPosition = { static_cast(cursorPosition.X - SwapChainPanel().Margin().Left), diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 99a420246f1..f88f187f0e6 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -562,9 +562,6 @@ CATCH_RETURN(); // If in composition mode, apply scaling factor matrix if (_chainMode == SwapChainMode::ForComposition) { - const auto fdpi = static_cast(_dpi); - _d2dRenderTarget->SetDpi(fdpi, fdpi); - DXGI_MATRIX_3X2_F inverseScale = { 0 }; inverseScale._11 = 1.0f / _scale; inverseScale._22 = inverseScale._11; @@ -837,7 +834,7 @@ CATCH_RETURN(); } case SwapChainMode::ForComposition: { - return _sizeTarget.scale(til::math::ceiling, _scale); + return _sizeTarget; } default: FAIL_FAST_HR(E_NOTIMPL); @@ -1971,13 +1968,9 @@ CATCH_RETURN(); // The advance is the number of pixels left-to-right (X dimension) for the given font. // We're finding a proportional factor here with the design units in "ems", not an actual pixel measurement. - // For HWND swap chains, we play trickery with the font size. For others, we use inherent scaling. - // For composition swap chains, we scale by the DPI later during drawing and presentation. - if (_chainMode == SwapChainMode::ForHwnd) - { - heightDesired *= (static_cast(dpi) / static_cast(USER_DEFAULT_SCREEN_DPI)); - } - + // Now we play trickery with the font size. Scale by the DPI to get the height we expect. + heightDesired *= (static_cast(dpi) / static_cast(USER_DEFAULT_SCREEN_DPI)); + const float widthAdvance = static_cast(advanceInDesignUnits) / fontMetrics.designUnitsPerEm; // Use the real pixel height desired by the "em" factor for the width to get the number of pixels From e476e5c573a3714b52676b42b67bfa9307486e9f Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 15:14:58 -0700 Subject: [PATCH 31/44] Remove block on incremental rendering for High DPI. Remove scale compensations in calculating invalid areas as the font has already adjusted to compensate. --- src/renderer/dx/DxRenderer.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index f88f187f0e6..341b34ea214 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -891,13 +891,6 @@ 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(); @@ -979,17 +972,15 @@ try // Scale all dirty rectangles into pixels std::transform(_presentDirty.begin(), _presentDirty.end(), _presentDirty.begin(), [&](til::rectangle rc) { - return rc.scale_up(_glyphCell).scale(til::math::rounding, _scale); + return rc.scale_up(_glyphCell); }); // Invalid scroll is in characters, convert it to pixels. - const auto scrollPixels = (_invalidScroll * _glyphCell).scale(til::math::rounding, _scale); + const auto scrollPixels = (_invalidScroll * _glyphCell); // 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{} - scrollPixels; From 05bd203eef7c80e950853db1dd997bcef3688b5e Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 15:57:00 -0700 Subject: [PATCH 32/44] rearrange items in terminalcontrol to not show delta. use scaling on resize. --- src/cascadia/TerminalControl/TermControl.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index cba61fbb524..311958aba26 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1674,7 +1674,9 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto lock = _terminal->LockForWriting(); - const auto foundationSize = e.NewSize(); + auto foundationSize = e.NewSize(); + foundationSize.Width *= SwapChainPanel().CompositionScaleX(); + foundationSize.Height *= SwapChainPanel().CompositionScaleY(); _DoResize(foundationSize.Width, foundationSize.Height); } @@ -2092,8 +2094,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation THROW_IF_FAILED(dxEngine->UpdateDpi(dpi)); THROW_IF_FAILED(dxEngine->UpdateFont(desiredFont, actualFont)); - const auto fontSize = actualFont.GetSize(); const auto scale = dxEngine->GetScaling(); + const auto fontSize = actualFont.GetSize(); // UWP XAML scrollbars aren't guaranteed to be the same size as the // ComCtl scrollbars, but it's certainly close enough. From f4ddf1f8c078c51d426f0575170c5e6ee382b30d Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 16:05:08 -0700 Subject: [PATCH 33/44] code format --- src/renderer/dx/DxRenderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 341b34ea214..ebc118ca087 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1961,7 +1961,7 @@ CATCH_RETURN(); // Now we play trickery with the font size. Scale by the DPI to get the height we expect. heightDesired *= (static_cast(dpi) / static_cast(USER_DEFAULT_SCREEN_DPI)); - + const float widthAdvance = static_cast(advanceInDesignUnits) / fontMetrics.designUnitsPerEm; // Use the real pixel height desired by the "em" factor for the width to get the number of pixels From d330a00a4d7a60d09f0c46a2bff2f197b8c07629 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 16:37:20 -0700 Subject: [PATCH 34/44] remember previous scale transform and resize/recreate if different. --- src/renderer/dx/DxRenderer.cpp | 5 ++++- src/renderer/dx/DxRenderer.hpp | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index ebc118ca087..2b7f02c0cb2 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -88,6 +88,7 @@ DxEngine::DxEngine() : _sizeTarget{}, _dpi{ USER_DEFAULT_SCREEN_DPI }, _scale{ 1.0f }, + _prevScale{ 1.0f }, _chainMode{ SwapChainMode::ForComposition }, _customRenderer{ ::Microsoft::WRL::Make() } { @@ -569,6 +570,8 @@ CATCH_RETURN(); ::Microsoft::WRL::ComPtr sc2; RETURN_IF_FAILED(_dxgiSwapChain.As(&sc2)); RETURN_IF_FAILED(sc2->SetMatrixTransform(&inverseScale)); + + _prevScale = _scale; } return S_OK; } @@ -910,7 +913,7 @@ try { RETURN_IF_FAILED(_CreateDeviceResources(true)); } - else if (_displaySizePixels != clientSize) + else if (_displaySizePixels != clientSize || _prevScale != _scale) { // 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. diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index 6fd98eb076b..d5b4cf11cdd 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -124,6 +124,7 @@ namespace Microsoft::Console::Render til::size _sizeTarget; int _dpi; float _scale; + float _prevScale; std::function _pfn; From 767f491a6c07fbdfb48913116737ee854723627c Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 13 Apr 2020 16:44:10 -0700 Subject: [PATCH 35/44] Run dpi change through the font trigger in render base. --- src/cascadia/TerminalControl/TermControl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 311958aba26..fd11f7ed1fe 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1689,7 +1689,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto scale = sender.CompositionScaleX(); const auto dpi = (int)(scale * USER_DEFAULT_SCREEN_DPI); - THROW_IF_FAILED(_renderEngine->UpdateDpi(dpi)); + _renderer->TriggerFontChange(dpi, _desiredFont, _actualFont); } } From 9a5d257b8bbcdb3a02f0561454733f17c802d559 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Wed, 15 Apr 2020 15:46:39 -0700 Subject: [PATCH 36/44] wpf: Save the scale for HWND as well (avoid flicker); fix HTML --- src/cascadia/PublicTerminalCore/HwndTerminal.cpp | 2 +- src/renderer/dx/DxRenderer.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp index 0e599b786b1..1047b1327f5 100644 --- a/src/cascadia/PublicTerminalCore/HwndTerminal.cpp +++ b/src/cascadia/PublicTerminalCore/HwndTerminal.cpp @@ -586,7 +586,7 @@ HRESULT HwndTerminal::_CopyTextToSystemClipboard(const TextBuffer::TextAndColor& if (fAlsoCopyFormatting) { const auto& fontData = _actualFont; - int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / this->_currentDpi; + int const iFontHeightPoints = fontData.GetUnscaledSize().Y; // this renderer uses points already const COLORREF bgColor = _terminal->GetBackgroundColor(_terminal->GetDefaultBrushColors()); std::string HTMLToPlaceOnClip = TextBuffer::GenHTML(rows, iFontHeightPoints, fontData.GetFaceName(), bgColor, "Hwnd Console Host"); diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 2b7f02c0cb2..0b21fcc747f 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -570,9 +570,9 @@ CATCH_RETURN(); ::Microsoft::WRL::ComPtr sc2; RETURN_IF_FAILED(_dxgiSwapChain.As(&sc2)); RETURN_IF_FAILED(sc2->SetMatrixTransform(&inverseScale)); - - _prevScale = _scale; } + + _prevScale = _scale; return S_OK; } CATCH_RETURN(); From ca7ccf32b6e702349227f3264e29a61ab03d74e9 Mon Sep 17 00:00:00 2001 From: Dustin Howett Date: Wed, 15 Apr 2020 15:49:23 -0700 Subject: [PATCH 37/44] uia: fix dip/dpx calculations now that controls are in DPX --- src/types/TermControlUiaTextRange.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/types/TermControlUiaTextRange.cpp b/src/types/TermControlUiaTextRange.cpp index 0f13ca0ef66..e2bfa950407 100644 --- a/src/types/TermControlUiaTextRange.cpp +++ b/src/types/TermControlUiaTextRange.cpp @@ -98,9 +98,10 @@ void TermControlUiaTextRange::_TranslatePointToScreen(LPPOINT clientPoint) const const gsl::not_null provider = static_cast(_pProvider); const auto includeOffsets = [](long clientPos, double termControlPos, double padding, double scaleFactor) { - auto result = base::ClampedNumeric(clientPos); - result += padding; + auto result = base::ClampedNumeric(padding); + // only the padding is in DIPs now result *= scaleFactor; + result += clientPos; result += termControlPos; return result; }; @@ -131,10 +132,11 @@ void TermControlUiaTextRange::_TranslatePointFromScreen(LPPOINT screenPoint) con const gsl::not_null provider = static_cast(_pProvider); const auto includeOffsets = [](long screenPos, double termControlPos, double padding, double scaleFactor) { - auto result = base::ClampedNumeric(screenPos); - result -= termControlPos; + auto result = base::ClampedNumeric(padding); + // only the padding is in DIPs now result /= scaleFactor; - result -= padding; + result -= screenPos; + result -= termControlPos; return result; }; From cbe389fdea22fb0f8885895b0af70bfc7b066ef0 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 20 Apr 2020 17:01:25 -0500 Subject: [PATCH 38/44] Various Differential Drawing fixes for #5345 (#5427) * This seems to work, but can it avoid resizing the buffer twice on a DPI change? * Add a doc comment that should have been in the previous commit * Remove some dead code I didn't end up needing * convert TSFInputControl::_RedrawCanvas to use til wherever possible This is just here to help me debug * I think this works fine on high-dpi for latin, cyrillic, emoji, chinese * This _actaully_ works and displays the composition correctly. Needs comments * Clean up this code significantly * This does in fact work * Skip one of the two resizes during a DPI scale change. * This definitely fixes the gutters, but lets try to be more surgical about it * Add some comments * This fixes the 'vim open in an inactive tab' bug I was seeing, but I might be able to do it without reverting the double resize * More good good til helpers * rewrite TermControl::_GetTerminalPosition, this might all be wrong though. * Do pointer event scaling in DIPs * Largely cleanup, but also add a TODO as it's almost 5pm here * Remove unused code, comments about receiving a useless ScaleChanged event * Update the font size in response to a settings reload * Fix blanking vim on a settings update * good bot * Immediately tell the VT Engine about the new viewport size when we resize conpty * fix the bitmap::all() function so we can actually use it (cherry picked from commit ba1a8a34292180cae10c135743ecd94b1da6f286) * maybe I should let it finish building before I commit (cherry picked from commit 98cdbeca349acb55c5b5aef1d96206a8fa7b62e4) * Hey what's this doing here * Account for IME wrapping --- .../TerminalControl/TSFInputControl.cpp | 77 +++++--- src/cascadia/TerminalControl/TermControl.cpp | 171 +++++++++++++----- src/cascadia/TerminalControl/TermControl.h | 1 + src/host/screenInfo.cpp | 14 ++ src/inc/til/bitmap.h | 26 +-- src/inc/til/point.h | 15 +- src/inc/til/rectangle.h | 12 ++ src/inc/til/size.h | 26 ++- src/renderer/dx/DxRenderer.cpp | 13 +- 9 files changed, 250 insertions(+), 105 deletions(-) diff --git a/src/cascadia/TerminalControl/TSFInputControl.cpp b/src/cascadia/TerminalControl/TSFInputControl.cpp index 477a565733a..b025eee4a4e 100644 --- a/src/cascadia/TerminalControl/TSFInputControl.cpp +++ b/src/cascadia/TerminalControl/TSFInputControl.cpp @@ -175,52 +175,69 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto fontArgs = winrt::make_self(); _CurrentFontInfoHandlers(*this, *fontArgs); - const auto fontWidth = fontArgs->FontSize().Width; - const auto fontHeight = fontArgs->FontSize().Height; + const til::size fontSize{ til::math::flooring, fontArgs->FontSize() }; - // Convert text buffer cursor position to client coordinate position within the window - COORD clientCursorPos; - clientCursorPos.X = ::base::ClampMul(_currentTerminalCursorPos.x(), ::base::ClampedNumeric(fontWidth)); - clientCursorPos.Y = ::base::ClampMul(_currentTerminalCursorPos.y(), ::base::ClampedNumeric(fontHeight)); + // Convert text buffer cursor position to client coordinate position + // within the window. This point is in _pixels_ + const til::point clientCursorPos{ _currentTerminalCursorPos * fontSize }; - // position textblock to cursor position - Canvas().SetLeft(TextBlock(), clientCursorPos.X); - Canvas().SetTop(TextBlock(), clientCursorPos.Y); + // Get scale factor for view + const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); + + const til::point clientCursorInDips{ clientCursorPos / scaleFactor }; + + // Position our TextBlock at the cursor position + Canvas().SetLeft(TextBlock(), clientCursorInDips.x()); + Canvas().SetTop(TextBlock(), clientCursorInDips.y()); + + // calculate FontSize in pixels from Points + const double fontSizePx = (fontSize.height() * 72) / USER_DEFAULT_SCREEN_DPI; - // calculate FontSize in pixels from DPIs - const double fontSizePx = (fontHeight * 72) / USER_DEFAULT_SCREEN_DPI; - TextBlock().FontSize(fontSizePx); + // Make sure to unscale the font size to correct for DPI! XAML needs + // things in DIPs, and the fontSize is in pixels. + TextBlock().FontSize(fontSizePx / scaleFactor); TextBlock().FontFamily(Media::FontFamily(fontArgs->FontFace())); - const auto widthToTerminalEnd = _currentCanvasWidth - ::base::ClampedNumeric(clientCursorPos.X); + const auto widthToTerminalEnd = _currentCanvasWidth - clientCursorInDips.x(); // Make sure that we're setting the MaxWidth to a positive number - a // negative number here will crash us in mysterious ways with a useless // stack trace const auto newMaxWidth = std::max(0.0, widthToTerminalEnd); TextBlock().MaxWidth(newMaxWidth); - // Get window in screen coordinates, this is the entire window including tabs - const auto windowBounds = CoreWindow::GetForCurrentThread().Bounds(); + // Get window in screen coordinates, this is the entire window including + // tabs. THIS IS IN DIPs + const auto windowBounds{ CoreWindow::GetForCurrentThread().Bounds() }; + const til::point windowOrigin{ til::math::flooring, windowBounds }; - // Convert from client coordinate to screen coordinate by adding window position - COORD screenCursorPos; - screenCursorPos.X = ::base::ClampAdd(clientCursorPos.X, ::base::ClampedNumeric(windowBounds.X)); - screenCursorPos.Y = ::base::ClampAdd(clientCursorPos.Y, ::base::ClampedNumeric(windowBounds.Y)); + // Get the offset (margin + tabs, etc..) of the control within the window + const til::point controlOrigin{ til::math::flooring, + this->TransformToVisual(nullptr).TransformPoint(Point(0, 0)) }; - // get any offset (margin + tabs, etc..) of the control within the window - const auto offsetPoint = this->TransformToVisual(nullptr).TransformPoint(winrt::Windows::Foundation::Point(0, 0)); + // The controlAbsoluteOrigin is the origin of the control relative to + // the origin of the displays. THIS IS IN DIPs + const til::point controlAbsoluteOrigin{ windowOrigin + controlOrigin }; - // add the margin offsets if any - screenCursorPos.X = ::base::ClampAdd(screenCursorPos.X, ::base::ClampedNumeric(offsetPoint.X)); - screenCursorPos.Y = ::base::ClampAdd(screenCursorPos.Y, ::base::ClampedNumeric(offsetPoint.Y)); + // Convert the control origin to pixels + const til::point scaledFrameOrigin = controlAbsoluteOrigin * scaleFactor; - // Get scale factor for view - const double scaleFactor = DisplayInformation::GetForCurrentView().RawPixelsPerViewPixel(); - const auto yOffset = ::base::ClampedNumeric(_currentTextBlockHeight) - fontHeight; - const auto textBottom = ::base::ClampedNumeric(screenCursorPos.Y) + yOffset; + // Get the location of the cursor in the display, in pixels. + til::point screenCursorPos{ scaledFrameOrigin + clientCursorPos }; + + // GH #5007 - make sure to account for wrapping the IME composition at + // the right side of the viewport. + const ptrdiff_t textBlockHeight = ::base::ClampMul(_currentTextBlockHeight, scaleFactor); + + // Get the bounds of the composition text, in pixels. + const til::rectangle textBounds{ til::point{ screenCursorPos.x(), screenCursorPos.y() }, + til::size{ 0, textBlockHeight } }; + + _currentTextBounds = textBounds; - _currentTextBounds = ScaleRect(Rect(screenCursorPos.X, textBottom, 0, fontHeight), scaleFactor); - _currentControlBounds = ScaleRect(Rect(screenCursorPos.X, screenCursorPos.Y, 0, fontHeight), scaleFactor); + _currentControlBounds = Rect(screenCursorPos.x(), + screenCursorPos.y(), + 0, + fontSize.height()); } // Method Description: diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index c0a83adff63..1a80e89e11e 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -225,17 +225,12 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _terminal->UpdateSettings(_settings); // Refresh our font with the renderer + const auto actualFontOldSize = _actualFont.GetSize(); _UpdateFont(); - - const auto width = SwapChainPanel().ActualWidth(); - const auto height = SwapChainPanel().ActualHeight(); - if (width != 0 && height != 0) + const auto actualFontNewSize = _actualFont.GetSize(); + if (actualFontNewSize != actualFontOldSize) { - // If the font size changed, or the _swapchainPanel's size changed - // for any reason, we'll need to make sure to also resize the - // buffer. _DoResize will invalidate everything for us. - auto lock = _terminal->LockForWriting(); - _DoResize(width, height); + _RefreshSize(); } } } @@ -514,8 +509,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation return false; } - const auto windowWidth = SwapChainPanel().ActualWidth() * SwapChainPanel().CompositionScaleX(); // Width() and Height() are NaN? - const auto windowHeight = SwapChainPanel().ActualHeight() * SwapChainPanel().CompositionScaleY(); + const auto actualWidth = SwapChainPanel().ActualWidth(); + const auto actualHeight = SwapChainPanel().ActualHeight(); + + const auto windowWidth = actualWidth * SwapChainPanel().CompositionScaleX(); // Width() and Height() are NaN? + const auto windowHeight = actualHeight * SwapChainPanel().CompositionScaleY(); if (windowWidth == 0 || windowHeight == 0) { @@ -1056,8 +1054,10 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Figure out if the user's moved a quarter of a cell's smaller axis away from the clickdown point auto& touchdownPoint{ *_singleClickTouchdownPos }; auto distance{ std::sqrtf(std::powf(cursorPosition.X - touchdownPoint.X, 2) + std::powf(cursorPosition.Y - touchdownPoint.Y, 2)) }; - const auto fontSize{ _actualFont.GetSize() }; - if (distance >= (std::min(fontSize.X, fontSize.Y) / 4.f)) + const til::size fontSize{ _actualFont.GetSize() }; + + const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); + if (distance >= (std::min(fontSizeInDips.width(), fontSizeInDips.height()) / 4.f)) { _terminal->SetSelectionAnchor(_GetTerminalPosition(touchdownPoint)); // stop tracking the touchdown point @@ -1097,19 +1097,22 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation winrt::Windows::Foundation::Point newTouchPoint{ contactRect.X, contactRect.Y }; const auto anchor = _touchAnchor.value(); - // Get the difference between the point we've dragged to and the start of the touch. - const float fontHeight = float(_actualFont.GetSize().Y); + // Our _actualFont's size is in pixels, convert to DIPs, which the + // rest of the Points here are in. + const til::size fontSize{ _actualFont.GetSize() }; + const auto fontSizeInDips = fontSize.scale(til::math::rounding, 1.0f / _renderEngine->GetScaling()); + // Get the difference between the point we've dragged to and the start of the touch. const float dy = newTouchPoint.Y - anchor.Y; // Start viewport scroll after we've moved more than a half row of text - if (std::abs(dy) > (fontHeight / 2.0f)) + if (std::abs(dy) > (fontSizeInDips.height() / 2.0f)) { // Multiply by -1, because moving the touch point down will // create a positive delta, but we want the viewport to move up, // so we'll need a negative scroll amount (and the inverse for // panning down) - const float numRows = -1.0f * (dy / fontHeight); + const float numRows = -1.0f * (dy / fontSizeInDips.height()); const auto currentOffset = ::base::ClampedNumeric(ScrollBar().Value()); const auto newValue = numRows + currentOffset; @@ -1648,12 +1651,11 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // Refresh our font with the renderer _UpdateFont(); - auto lock = _terminal->LockForWriting(); // Resize the terminal's BUFFER to match the new font size. This does // NOT change the size of the window, because that can lead to more // problems (like what happens when you change the font size while the // window is maximized?) - _DoResize(SwapChainPanel().ActualWidth(), SwapChainPanel().ActualHeight()); + _RefreshSize(); } CATCH_LOG(); } @@ -1673,22 +1675,84 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation auto lock = _terminal->LockForWriting(); - auto foundationSize = e.NewSize(); - foundationSize.Width *= SwapChainPanel().CompositionScaleX(); - foundationSize.Height *= SwapChainPanel().CompositionScaleY(); + const auto newSize = e.NewSize(); + const auto currentScaleX = SwapChainPanel().CompositionScaleX(); + const auto currentEngineScale = _renderEngine->GetScaling(); + auto foundationSize = newSize; + + // A strange thing can happen here. If you have two tabs open, and drag + // across a DPI boundary, then switch to the other tab, that tab will + // receive two events: First, a SizeChanged, then a ScaleChanged. In the + // SizeChanged event handler, the SwapChainPanel's CompositionScale will + // _already_ be the new scaling, but the engine won't have that value + // yet. If we scale by the CompositionScale here, we'll end up in a + // weird torn state. I'm not totally sure why. + // + // Fortunately we will be getting that following ScaleChanged event, and + // we'll end up resizing again, so we don't terribly need to worry about + // this. + foundationSize.Width *= currentEngineScale; + foundationSize.Height *= currentEngineScale; _DoResize(foundationSize.Width, foundationSize.Height); } + // Method Description: + // - Triggered when the swapchain changes DPI. When this happens, we're + // going to receive 3 events: + // - 1. First, a CompositionScaleChanged _for the original scale_. I don't + // know why this event happens first. **It also doesn't always happen.** + // However, when it does happen, it doesn't give us any useful + // information. + // - 2. Then, a SizeChanged. During that SizeChanged, either: + // - the CompositionScale will still be the original DPI. This happens + // when the control is visible as the DPI changes. + // - The CompositionScale will be the new DPI. This happens when the + // control wasn't focused as the window's DPI changed, so it only got + // these messages after XAML updated it's scaling. + // - 3. Finally, a CompositionScaleChanged with the _new_ DPI. + // - 4. We'll usually get another SizeChanged some time after this last + // ScaleChanged. This usually seems to happen after something triggers + // the UI to re-layout, like hovering over the scrollbar. This event + // doesn't reliably happen immediately after a scale change, so we can't + // depend on it (despite the fact that both the scale and size state is + // definitely correct in it) + // - In the 3rd event, we're going to update our font size for te new DPI. + // At that point, we know how big the font should be for the new DPI, and + // how big the SwapChainPanel will be. If these sizes are different, we'll + // need to resize the buffer to fit in the new window. + // Arguments: + // - sender: The SwapChainPanel who's DPI changed. This is our _swapchainPanel. + // - args: This param is unused in the CompositionScaleChanged event. void TermControl::_SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& /*args*/) { if (_renderEngine) { - const auto scale = sender.CompositionScaleX(); - const auto dpi = (int)(scale * USER_DEFAULT_SCREEN_DPI); + const auto scaleX = sender.CompositionScaleX(); + const auto scaleY = sender.CompositionScaleY(); + const auto dpi = (float)(scaleX * USER_DEFAULT_SCREEN_DPI); + const auto currentEngineScale = _renderEngine->GetScaling(); + + // If we're getting a notification to change to the DPI we already + // have, then we're probably just beginning the DPI change. Since + // we'll get _another_ event with the real DPI, do nothing here for + // now. We'll also skip the next resize in _SwapChainSizeChanged. + const bool dpiWasUnchanged = currentEngineScale == scaleX; + if (dpiWasUnchanged) + { + return; + } - _renderer->TriggerFontChange(dpi, _desiredFont, _actualFont); + const auto actualFontOldSize = _actualFont.GetSize(); + + _renderer->TriggerFontChange(::base::saturated_cast(dpi), _desiredFont, _actualFont); + + const auto actualFontNewSize = _actualFont.GetSize(); + if (actualFontNewSize != actualFontOldSize) + { + _RefreshSize(); + } } } @@ -1727,6 +1791,31 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation _selectionNeedsToBeCopied = true; } + // Method Description: + // - Perform a resize for the current size of the swapchainpanel. If the + // font size changed, we'll need to resize the buffer to fit the existing + // swapchain size. This helper will call _DoResize with the current size + // of the swapchain, accounting for scaling due to DPI. + // - Note that a DPI change will also trigger a font size change, and will call into here. + // Arguments: + // - + // Return Value: + // - + void TermControl::_RefreshSize() + { + const auto currentScaleX = SwapChainPanel().CompositionScaleX(); + const auto currentScaleY = SwapChainPanel().CompositionScaleY(); + const auto actualWidth = SwapChainPanel().ActualWidth(); + const auto actualHeight = SwapChainPanel().ActualHeight(); + + const auto widthInPixels = actualWidth * currentScaleX; + const auto heightInPixels = actualHeight * currentScaleY; + + // Grab the lock, because we might be changing the buffer size here. + auto lock = _terminal->LockForWriting(); + _DoResize(widthInPixels, heightInPixels); + } + // Method Description: // - Process a resize event that was initiated by the user. This can either // be due to the user resizing the window (causing the swapchain to @@ -1762,11 +1851,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels); // If this function succeeds with S_FALSE, then the terminal didn't - // actually change size. No need to notify the connection of this - // no-op. - // TODO: MSFT:20642295 Resizing the buffer will corrupt it - // I believe we'll need support for CSI 2J, and additionally I think - // we're resetting the viewport to the top + // actually change size. No need to notify the connection of this no-op. const HRESULT hr = _terminal->UserResize({ vp.Width(), vp.Height() }); if (SUCCEEDED(hr) && hr != S_FALSE) { @@ -2302,25 +2387,21 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - the corresponding viewport terminal position for the given Point parameter const COORD TermControl::_GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition) { - // compensate for DPI scenarios. - cursorPosition.X *= SwapChainPanel().CompositionScaleX(); - cursorPosition.Y *= SwapChainPanel().CompositionScaleY(); + // cursorPosition is DIPs, relative to SwapChainPanel origin + const til::point cursorPosInDIPs{ til::math::rounding, cursorPosition }; + const til::size marginsInDips{ til::math::rounding, SwapChainPanel().Margin().Left, SwapChainPanel().Margin().Top }; - // Exclude padding from cursor position calculation - COORD terminalPosition = { - static_cast(cursorPosition.X - SwapChainPanel().Margin().Left), - static_cast(cursorPosition.Y - SwapChainPanel().Margin().Top) - }; + // This point is the location of the cursor within the actual grid of characters, in DIPs + const til::point relativeToMarginInDIPs = cursorPosInDIPs - marginsInDips; - const auto fontSize = _actualFont.GetSize(); - FAIL_FAST_IF(fontSize.X == 0); - FAIL_FAST_IF(fontSize.Y == 0); + // Convert it to pixels + const til::point relativeToMarginInPixels{ relativeToMarginInDIPs * SwapChainPanel().CompositionScaleX() }; - // Normalize to terminal coordinates by using font size - terminalPosition.X /= fontSize.X; - terminalPosition.Y /= fontSize.Y; + // Get the size of the font, which is in pixels + const til::size fontSize{ _actualFont.GetSize() }; - return terminalPosition; + // Convert the location in pixels to characters within the current viewport. + return til::point{ relativeToMarginInPixels / fontSize }; } // Method Description: diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index d4f62168ef3..81a8fc85ce6 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -198,6 +198,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation void _SwapChainSizeChanged(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::SizeChangedEventArgs const& e); void _SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& args); void _DoResize(const double newWidth, const double newHeight); + void _RefreshSize(); void _TerminalTitleChanged(const std::wstring_view& wstr); winrt::fire_and_forget _TerminalScrollPositionChanged(const int viewTop, const int viewHeight, const int bufferSize); winrt::fire_and_forget _TerminalCursorPositionChanged(); diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index 75e16af1ade..ecd49d0c5a7 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -1229,6 +1229,20 @@ void SCREEN_INFORMATION::_InternalSetViewportSize(const COORD* const pcoordSize, _viewport = newViewport; UpdateBottom(); Tracing::s_TraceWindowViewport(_viewport); + + // In Conpty mode, call TriggerScroll here without params. By not providing + // params, the renderer will make sure to update the VtEngine with the + // updated viewport size. If we don't do this, the engine can get into a + // torn state on this frame. + // + // Without this statement, the engine won't be told about the new view size + // till the start of the next frame. If any other text gets output before + // that frame starts, there's a very real chance that it'll cause errors as + // the engine tries to invalidate those regions. + if (gci.IsInVtIoMode() && ServiceLocator::LocateGlobals().pRender) + { + ServiceLocator::LocateGlobals().pRender->TriggerScroll(); + } } // Routine Description: diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index acafbaf1560..ea3c024c8c2 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -135,7 +135,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz{}, _rc{}, _bits{}, - _dirty{}, _runs{} { } @@ -149,7 +148,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _sz(sz), _rc(sz), _bits(_sz.area()), - _dirty(fill ? sz : til::rectangle{}), _runs{} { if (fill) @@ -162,7 +160,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { return _sz == other._sz && _rc == other._rc && - _dirty == other._dirty && // dirty is before bits because it's a rough estimate of bits and a faster comparison. _bits == other._bits; // _runs excluded because it's a cache of generated state. } @@ -187,15 +184,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // If we don't have cached runs, rebuild. if (!_runs.has_value()) { - // If there's only one square dirty, quick save it off and be done. - if (one()) - { - _runs.emplace({ _dirty }); - } - else - { - _runs.emplace(begin(), end()); - } + _runs.emplace(begin(), end()); } // Return a reference to the runs. @@ -274,8 +263,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" _runs.reset(); // reset cached runs on any non-const method _bits.set(_rc.index_of(pt)); - - _dirty |= til::rectangle{ pt }; } void set(const til::rectangle rc) @@ -287,22 +274,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { _bits.set(_rc.index_of(til::point{ rc.left(), row }), rc.width(), true); } - - _dirty |= rc; } void set_all() noexcept { _runs.reset(); // reset cached runs on any non-const method _bits.set(); - _dirty = _rc; } void reset_all() noexcept { _runs.reset(); // reset cached runs on any non-const method _bits.reset(); - _dirty = {}; } // True if we resized. False if it was the same size as before. @@ -360,7 +343,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" bool one() const { - return _dirty.size() == til::size{ 1, 1 }; + return _bits.count() == 1; } constexpr bool any() const noexcept @@ -370,12 +353,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" constexpr bool none() const noexcept { - return _dirty.empty(); + return _bits.none(); } constexpr bool all() const noexcept { - return _dirty == _rc; + return _bits.all(); } constexpr til::size size() const noexcept @@ -399,7 +382,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } private: - til::rectangle _dirty; til::size _sz; til::rectangle _rc; dynamic_bitset<> _bits; diff --git a/src/inc/til/point.h b/src/inc/til/point.h index fa9a869a6e9..6d7f40a36d6 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -53,8 +53,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" { } + // This template will convert to point from floating-point args; + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" + template + constexpr point(TilMath, const TOther& x, const TOther& y, std::enable_if_t, int> /*sentinel*/ = 0) : + point(TilMath::template cast(x), TilMath::template cast(y)) + { + } + // This template will convert to size from anything that has a X and a Y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" template constexpr point(TilMath, const TOther& other, std::enable_if_t().X)> && std::is_floating_point_v().Y)>, int> /*sentinel*/ = 0) : point(TilMath::template cast(other.X), TilMath::template cast(other.Y)) @@ -62,7 +72,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a x and a y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::point" template constexpr point(TilMath, const TOther& other, std::enable_if_t().x)> && std::is_floating_point_v().y)>, int> /*sentinel*/ = 0) : point(TilMath::template cast(other.x), TilMath::template cast(other.y)) diff --git a/src/inc/til/rectangle.h b/src/inc/til/rectangle.h index ba488ea7ea9..83bb0b3f11a 100644 --- a/src/inc/til/rectangle.h +++ b/src/inc/til/rectangle.h @@ -863,6 +863,18 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } #endif +#ifdef WINRT_Windows_Foundation_H + operator winrt::Windows::Foundation::Rect() const + { + winrt::Windows::Foundation::Rect ret; + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(left()).AssignIfValid(&ret.X)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(top()).AssignIfValid(&ret.Y)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(width()).AssignIfValid(&ret.Width)); + THROW_HR_IF(E_ABORT, !base::MakeCheckedNum(height()).AssignIfValid(&ret.Height)); + return ret; + } +#endif + std::wstring to_string() const { return wil::str_printf(L"(L:%td, T:%td, R:%td, B:%td) [W:%td, H:%td]", left(), top(), right(), bottom(), width(), height()); diff --git a/src/inc/til/size.h b/src/inc/til/size.h index b7d71dbd46b..67230b88202 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -25,6 +25,14 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" size(static_cast(width), static_cast(height)) { } + constexpr size(ptrdiff_t width, int height) noexcept : + size(width, static_cast(height)) + { + } + constexpr size(int width, ptrdiff_t height) noexcept : + size(static_cast(width), height) + { + } #endif size(size_t width, size_t height) @@ -54,7 +62,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a X and a Y field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().X)> && std::is_floating_point_v().Y)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.X), TilMath::template cast(other.Y)) @@ -62,7 +71,8 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a cx and a cy field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().cx)> && std::is_floating_point_v().cy)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.cx), TilMath::template cast(other.cy)) @@ -70,13 +80,23 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } // This template will convert to size from anything that has a Width and a Height field that are floating-point; - // a math type is required. + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" template constexpr size(TilMath, const TOther& other, std::enable_if_t().Width)> && std::is_floating_point_v().Height)>, int> /*sentinel*/ = 0) : size(TilMath::template cast(other.Width), TilMath::template cast(other.Height)) { } + // This template will convert to size from floating-point args; + // a math type is required. If you _don't_ provide one, you're going to + // get a compile-time error about "cannot convert from initializer-list to til::size" + template + constexpr size(TilMath, const TOther& width, const TOther& height, std::enable_if_t, int> /*sentinel*/ = 0) : + size(TilMath::template cast(width), TilMath::template cast(height)) + { + } + constexpr bool operator==(const size& other) const noexcept { return _width == other._width && diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 0b21fcc747f..654c71846c0 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -800,6 +800,16 @@ CATCH_RETURN(); try { _invalidMap.set_all(); + + // Since everything is invalidated here, mark this as a "first frame", so + // that we won't use incremental drawing on it. The caller of this intended + // for _everything_ to get redrawn, so setting _firstFrame will force us to + // redraw the entire frame. This will make sure that things like the gutters + // get cleared correctly. + // + // Invalidating everything is supposed to happen with resizes of the + // entire canvas, changes of the font, and other such adjustments. + _firstFrame = true; return S_OK; } CATCH_RETURN(); @@ -1168,9 +1178,6 @@ try D2D1_COLOR_F nothing = { 0 }; // 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()) { _d2dRenderTarget->Clear(nothing); From acb9f4cb19b70e1c7d0a9e223c525f21d89361f7 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 15:36:40 -0700 Subject: [PATCH 39/44] Remove tests against internal rectangle in BitmapTests as it is no longer needed/used. --- src/til/ut_til/BitmapTests.cpp | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/til/ut_til/BitmapTests.cpp b/src/til/ut_til/BitmapTests.cpp index cb560d240aa..9be1b87e792 100644 --- a/src/til/ut_til/BitmapTests.cpp +++ b/src/til/ut_til/BitmapTests.cpp @@ -22,25 +22,6 @@ class BitmapTests void _checkBits(const std::vector& bitsOn, const til::bitmap& map) { - Log::Comment(L"Check dirty rectangles."); - - // Union up all the dirty rectangles into one big one. - if (bitsOn.empty()) - { - VERIFY_ARE_EQUAL(til::rectangle{}, map._dirty); - } - else - { - auto dirtyExpected = bitsOn.front(); - for (auto it = bitsOn.cbegin() + 1; it < bitsOn.cend(); ++it) - { - dirtyExpected |= *it; - } - - // Check if it matches. - VERIFY_ARE_EQUAL(dirtyExpected, map._dirty); - } - Log::Comment(L"Check all bits in map."); // For every point in the map... for (const auto pt : map._rc) @@ -71,7 +52,6 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(0u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -86,7 +66,6 @@ class BitmapTests VERIFY_ARE_EQUAL(expectedSize, bitmap._sz); VERIFY_ARE_EQUAL(expectedRect, bitmap._rc); VERIFY_ARE_EQUAL(50u, bitmap._bits.size()); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); // The find will go from begin to end in the bits looking for a "true". // It should miss so the result should be "cend" and turn out true here. @@ -112,12 +91,10 @@ class BitmapTests if (!fill) { VERIFY_IS_TRUE(bitmap._bits.none()); - VERIFY_ARE_EQUAL(til::rectangle{}, bitmap._dirty); } else { VERIFY_IS_TRUE(bitmap._bits.all()); - VERIFY_ARE_EQUAL(expectedRect, bitmap._dirty); } } From e11d6a7a1e9e2f3d445202c6a09893eea7a25c36 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 16:02:58 -0700 Subject: [PATCH 40/44] Add some til tests for the til things that Mike introduced. --- src/til/precomp.h | 18 ++++++++++++++++++ src/til/ut_til/PointTests.cpp | 7 +++++++ src/til/ut_til/RectangleTests.cpp | 17 +++++++++++++++++ src/til/ut_til/SizeTests.cpp | 27 +++++++++++++++++++++++++++ 4 files changed, 69 insertions(+) diff --git a/src/til/precomp.h b/src/til/precomp.h index 2375e15ec09..e14bc792cf0 100644 --- a/src/til/precomp.h +++ b/src/til/precomp.h @@ -35,6 +35,24 @@ Module Name: // (before TIL so its support lights up) #include +// Include some things structs from WinRT without including WinRT +// because I just want to make sure it fills structs correctly. +// Adapted from C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\winrt\windows.foundation.h +#define WINRT_Windows_Foundation_H +namespace winrt { + namespace Windows { + namespace Foundation { + struct Rect + { + FLOAT X; + FLOAT Y; + FLOAT Width; + FLOAT Height; + }; + } /* Foundation */ + } /* Windows */ +} /* winrt */ + // Include TIL after Wex to get test comparators. #include "til.h" diff --git a/src/til/ut_til/PointTests.cpp b/src/til/ut_til/PointTests.cpp index 994f2df4bdd..496fbc65d3e 100644 --- a/src/til/ut_til/PointTests.cpp +++ b/src/til/ut_til/PointTests.cpp @@ -27,6 +27,13 @@ class PointTests VERIFY_ARE_EQUAL(10, pt._y); } + TEST_METHOD(RawFloatingConstruct) + { + const til::point pt{ til::math::rounding, 3.2f, 7.6f }; + VERIFY_ARE_EQUAL(3, pt._x); + VERIFY_ARE_EQUAL(8, pt._y); + } + TEST_METHOD(UnsignedConstruct) { Log::Comment(L"0.) Normal unsigned construct."); diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index b8bb7859937..bcff267db59 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -1349,6 +1349,23 @@ class RectangleTests // All ptrdiff_ts fit into a float, so there's no exception tests. } + + TEST_METHOD(CastToWindowsFoundationRect) + { + Log::Comment(L"0.) Typical situation."); + { + const til::rectangle rc{ 5, 10, 15, 20 }; + winrt::Windows::Foundation::Rect val = rc; + VERIFY_ARE_EQUAL(5.f, val.X); + VERIFY_ARE_EQUAL(10.f, val.Y); + VERIFY_ARE_EQUAL(10.f, val.Width); + VERIFY_ARE_EQUAL(10.f, val.Height); + } + + // All ptrdiff_ts fit into a float, so there's no exception tests. + // The only other exceptions come from things that don't fit into width() or height() + // and those have explicit tests elsewhere in this file. + } #pragma region iterator TEST_METHOD(Begin) diff --git a/src/til/ut_til/SizeTests.cpp b/src/til/ut_til/SizeTests.cpp index fb0bb1ecfc7..ce75c5835cf 100644 --- a/src/til/ut_til/SizeTests.cpp +++ b/src/til/ut_til/SizeTests.cpp @@ -27,6 +27,13 @@ class SizeTests VERIFY_ARE_EQUAL(10, sz._height); } + TEST_METHOD(RawFloatingConstruct) + { + const til::size sz{ til::math::rounding, 3.2f, 7.8f }; + VERIFY_ARE_EQUAL(3, sz._width); + VERIFY_ARE_EQUAL(8, sz._height); + } + TEST_METHOD(UnsignedConstruct) { Log::Comment(L"0.) Normal unsigned construct."); @@ -74,6 +81,26 @@ class SizeTests VERIFY_ARE_EQUAL(height, sz._height); } + TEST_METHOD(MixedRawTypeConstruct) + { + const ptrdiff_t a = -5; + const int b = -10; + + Log::Comment(L"Case 1: ptrdiff_t/int"); + { + const til::size sz{ a, b }; + VERIFY_ARE_EQUAL(a, sz._width); + VERIFY_ARE_EQUAL(b, sz._height); + } + + Log::Comment(L"Case 2: int/ptrdiff_t"); + { + const til::size sz{ b, a }; + VERIFY_ARE_EQUAL(b, sz._width); + VERIFY_ARE_EQUAL(a, sz._height); + } + } + TEST_METHOD(CoordConstruct) { COORD coord{ -5, 10 }; From 3d4b978952e2f98f2ceed4c2efd040b14b49cc67 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 16:04:04 -0700 Subject: [PATCH 41/44] Typo. --- src/cascadia/TerminalControl/TermControl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 1a80e89e11e..93dea4b4959 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1717,7 +1717,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // doesn't reliably happen immediately after a scale change, so we can't // depend on it (despite the fact that both the scale and size state is // definitely correct in it) - // - In the 3rd event, we're going to update our font size for te new DPI. + // - In the 3rd event, we're going to update our font size for the new DPI. // At that point, we know how big the font should be for the new DPI, and // how big the SwapChainPanel will be. If these sizes are different, we'll // need to resize the buffer to fit in the new window. From ec614ab88b694deced8e5d49fab0e869487ffba8 Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 16:04:43 -0700 Subject: [PATCH 42/44] Code format. --- src/til/ut_til/RectangleTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/til/ut_til/RectangleTests.cpp b/src/til/ut_til/RectangleTests.cpp index bcff267db59..c79d2c8cd4f 100644 --- a/src/til/ut_til/RectangleTests.cpp +++ b/src/til/ut_til/RectangleTests.cpp @@ -1349,7 +1349,7 @@ class RectangleTests // All ptrdiff_ts fit into a float, so there's no exception tests. } - + TEST_METHOD(CastToWindowsFoundationRect) { Log::Comment(L"0.) Typical situation."); From 4094d9b1630c654df40ca964fea39dea6413135b Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 16:08:58 -0700 Subject: [PATCH 43/44] Audit mode noexcept/constexpr possible. --- src/inc/til/bitmap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inc/til/bitmap.h b/src/inc/til/bitmap.h index ea3c024c8c2..3cb88830a54 100644 --- a/src/inc/til/bitmap.h +++ b/src/inc/til/bitmap.h @@ -341,7 +341,7 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } } - bool one() const + constexpr bool one() const noexcept { return _bits.count() == 1; } From 4337cf551498d34cccc2412ef3e5715365be1c3e Mon Sep 17 00:00:00 2001 From: Michael Niksa Date: Mon, 20 Apr 2020 16:20:59 -0700 Subject: [PATCH 44/44] oops, missed these two foundation structs. --- src/til/precomp.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/til/precomp.h b/src/til/precomp.h index e14bc792cf0..2d2d07599d8 100644 --- a/src/til/precomp.h +++ b/src/til/precomp.h @@ -49,10 +49,24 @@ namespace winrt { FLOAT Width; FLOAT Height; }; + + struct Point + { + FLOAT X; + FLOAT Y; + }; + + struct Size + { + FLOAT Width; + FLOAT Height; + }; } /* Foundation */ } /* Windows */ } /* winrt */ + + // Include TIL after Wex to get test comparators. #include "til.h"