Render row-by-row instead of invalidating entire screen #5185

miniksa · 2020-03-30T22:31:23Z

Summary of the Pull Request

Adjusts DirectX renderer to use til::bitmap to track invalidation
regions. Uses special modification to invalidate a row-at-a-time to
ensure ligatures and NxM glyphs continue to work.

References

Likely helps #1064

PR Checklist

Closes Restore differential drawing to DirectWrite renderer #778
I work here.
Manual testing performed. See Performance traces in Restore differential drawing to DirectWrite renderer #778.
Automated tests for til changes.
Am core contributor. And discussed with @DHowett-MSFT.

Detailed Description of the Pull Request / Additional comments

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

Validation Steps Performed

Ran cmatrix, cmatrix -u0, and cacafire after changes were made.
Made a bunch of ligatures with Cascadia Code in the Terminal
before/after the changes and confirmed they still ligate.
Ran dir in Powershell and fixed the scrolling issues
Clicked all over the place and dragged to make sure selection works.
Checked retro terminal effect manually with Powershell.

…bad.

src/inc/til/bitmap.h

src/cascadia/TerminalCore/Terminal.cpp

src/inc/til/rectangle.h

miniksa · 2020-03-30T22:33:49Z

src/renderer/dx/DxRenderer.cpp

-        break;
-    case SwapChainMode::ForComposition:
-        D2D1_COLOR_F nothing = { 0 };
+    D2D1_COLOR_F nothing = { 0 };


Take note, there's no difference between hwnd and composition here anymore. On purpose. We need to fill a specific rectangle for both. This new code works to do that for both.

src/renderer/dx/DxRenderer.cpp

src/inc/til/rectangle.h

src/cascadia/TerminalCore/Terminal.cpp

src/inc/til.h

src/inc/til/rectangle.h

src/renderer/dx/precomp.h

src/renderer/dx/DxRenderer.cpp

DHowett-MSFT · 2020-04-01T00:53:11Z

I merged this into a selfhost build for testing, and it's definitely not right 😄

(it also crashes occasionally on resize)

miniksa · 2020-04-01T17:31:45Z

I merged this into a selfhost build for testing, and it's definitely not right 😄

OK. I mean, it is a draft PR. 😜 I'll look into it.

miniksa · 2020-04-01T18:37:56Z

Got the selection issue. Will follow on the scroll one after lunch.

oising · 2020-04-01T19:31:04Z

CHAFA IS WAITING

#410

…s to pixels.

…X renderer now to know what the signal means and figure it out.

miniksa · 2020-04-01T21:19:33Z

CHAFA IS WAITING

#410

Hm, good call. I'll see what that looks like with these changes.

miniksa · 2020-04-01T21:24:38Z

This is busted once we reach the bottom of the buffer. Gotta look into that.

miniksa · 2020-04-01T23:22:22Z

CHAFA IS WAITING
#410

Hm, good call. I'll see what that looks like with these changes.

miniksa · 2020-04-01T23:47:37Z

This is busted once we reach the bottom of the buffer. Gotta look into that.

It's likely that the terminal variant of the control just isn't calling TriggerScroll(delta) after the circular buffer is circling.
The conhost does this on

terminal/src/host/output.cpp

Lines 296 to 323 in a13ccfd

    
           bool StreamScrollRegion(SCREEN_INFORMATION& screenInfo) 
        
           { 
        
               // Rotate the circular buffer around and wipe out the previous final line. 
        
               const bool inVtMode = WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING); 
        
               bool fSuccess = screenInfo.GetTextBuffer().IncrementCircularBuffer(inVtMode); 
        
               if (fSuccess) 
        
               { 
        
                   // Trigger a graphical update if we're active. 
        
                   if (screenInfo.IsActiveScreenBuffer()) 
        
                   { 
        
                       COORD coordDelta = { 0 }; 
        
                       coordDelta.Y = -1; 
        
                       IAccessibilityNotifier* pNotifier = ServiceLocator::LocateAccessibilityNotifier(); 
        
                       if (pNotifier) 
        
                       { 
        
                           // Notify accessibility that a scroll has occurred. 
        
                           pNotifier->NotifyConsoleUpdateScrollEvent(coordDelta.X, coordDelta.Y); 
        
                       } 
        
                       if (ServiceLocator::LocateGlobals().pRender != nullptr) 
        
                       { 
        
                           ServiceLocator::LocateGlobals().pRender->TriggerScroll(&coordDelta); 
        
                       } 
        
                   } 
        
               } 
        
               return fSuccess; 
        
           }

in StreamScrollRegion.

Just need to identify that same place in the new terminal control and set that scroll trigger, most likely.

DHowett-MSFT · 2020-04-02T00:14:23Z

My last annoying question: what happens if you enable retro mode?

… render target scaling.

miniksa · 2020-04-10T22:42:49Z

Disabled for High DPI. #5320 filed as follow on so this PR doesn't get bigger.

…nder the renderer while it's operating.

zadjii-msft

Playing with this branch locally seems fine to me. I'm infinitely sad that it's disabled for high-dpi, but as long as we're still going to do it, let's do it

zadjii-msft · 2020-04-13T17:57:30Z

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


oh this makes me sad

DHowett-MSFT

GO GO GO

DHowett-MSFT · 2020-04-13T19:58:22Z

@msftbot merge this in 10 minutes

ghost · 2020-04-13T19:58:26Z

Hello @DHowett-MSFT!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Mon, 13 Apr 2020 20:08:24 GMT, which is in 10 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

miniksa · 2020-04-15T19:54:03Z

For 1.0, I'm disabling most of this code. Bitmap will remain and we will revisit post-1.0 when we have enough time for DPI and the other fallout.

Improve wide glyph support in UIA (GH-4946) Add enhanced key support for ConPty (GH-5021) Set DxRenderer non-text alias mode (GH-5149) Reduce CursorChanged Events for Accessibility (GH-5196) Add more object ID tracing for Accessibility (GH-5215) Add SS3 cursor key encoding to ConPty (GH-5383) UIA: Prevent crash from invalid UTR endpoint comparison (GH-5399) Make CodepointWidthDetector::GetWidth faster (CC-3727) add til::math, use it for float conversions to point, size (GH-5150) Add support for renderer backoff, don't FAIL_FAST on 3x failures, add UI (GH-5353) Fix a deadlock and a bounding rects issue in UIA (GH-5385) Don't duplicate spaces from potentially-wrapped EOL-deferred lines (GH-5398) Reimplement the VT tab stop functionality (CC-5173) Clamp parameter values to a maximum of 32767. (CC-5200) Prevent the cursor type being reset when changing the visibility (CC-5251) Make RIS switch back to the main buffer (CC-5248) Add support for the DSR-OS operating status report (CC-5300) Update the virtual bottom location if the cursor moves below it (CC-5317) ci: run spell check in CI, fix remaining issues (CC-4799) (CC-5352) Set Cascadia Code as default font (GH-5121) Show a double width cursor for double width characters (GH-5319) Delegate all character input to the character event handler (CC-4192) Update til::bitmap to use dynamic_bitset<> + libpopcnt (GH-5092) Merged PR 4465022: [Git2Git] Merged PR 4464559: Console: Ingest OSS changes up to e055079 Correct scrolling invalidation region for tmux in pty w/ bitmap (GH-5122) Render row-by-row instead of invalidating entire screen (GH-5185) Make conechokey use ReadConsoleInputW by default (GH-5148) Manually pass mouse wheel messages to TermControls (GH-5131) This fixes C-M-space for WSL but not for Win32, but I'm not sure there's a problem in Win32 quite yet. (GH-5208) Fix copying wrapped lines by implementing better scrolling (GH-5181) Emit lines wrapped due to spaces at the end correctly (GH-5294) Remove unneeded whitespace (CC-5162)

## Summary of the Pull Request - Adjusts scaling practices in `DxEngine` (and related scaling practices in `TerminalControl`) for pixel-perfect row baselines and spacing at High DPI such that differential row-by-row rendering can be applied at High DPI. ## References - #5185 ## PR Checklist * [x] Closes #5320, closes #3515, closes #1064 * [x] I work here. * [x] Manually tested. * [x] No doc. * [x] Am core contributor. Also discussed with some of them already via Teams. ## Detailed Description of the Pull Request / Additional comments **WAS:** - We were using implicit DPI scaling on the `ID2D1RenderTarget` and running all of our processing in DIPs (Device-Independent Pixels). That's all well and good for getting things bootstrapped quickly, but it leaves the actual scaling of the draw commands up to the discretion of the rendering target. - When we don't get to explicitly choose exactly how many pixels tall/wide and our X/Y placement perfectly, the nature of floating point multiplication and division required to do the presentation can cause us to drift off slightly out of our control depending on what the final display resolution actually is. - Differential drawing cannot work unless we can know the exact integer pixels that need to be copied/moved/preserved/replaced between frames to give to the `IDXGISwapChain1::Present1` method. If things spill into fractional pixels or the sizes of rows/columns vary as they are rounded up and down implicitly, then we cannot do the differential rendering. **NOW:** - When deciding on a font, the `DxEngine` will take the scale factor into account and adjust the proposed height of the requested font. Then the remainder of the existing code that adjusts the baseline and integer-ifies each character cell will run naturally from there. That code already works correctly to align the height at normal DPI and scale out the font heights and advances to take an exact integer of pixels. - `TermControl` has to use the scale now, in some places, and stop scaling in other places. This has to do with how the target's nature used to be implicit and is now explicit. For instance, determining where the cursor click hits must be scaled now. And determining the pixel size of the display canvas must no longer be scaled. - `DxEngine` will no longer attempt to scale the invalid regions per my attempts in #5185 because the cell size is scaled. So it should work the same as at 96 DPI. - The block is removed from the `DxEngine` that was causing a full invalidate on every frame at High DPI. - A TODO was removed from `TermControl` that was invalidating everything when the DPI changed because the underlying renderer will already do that. ## Validation Steps Performed * [x] Check at 150% DPI. Print text, scroll text down and up, do selection. * [x] Check at 100% DPI. Print text, scroll text down and up, do selection. * [x] Span two different DPI monitors and drag between them. * [x] Giant pile of tests in #5345 (comment) Co-authored-by: Dustin Howett <duhowett@microsoft.com> Co-authored-by: Mike Griese <migrie@microsoft.com>

Takes the lock inside two routines in `TermControl` that were changing the selection endpoint while a rendering frame was still drawing, resulting in several variants of graphical glitches from double-struck selection boxes to duplicated line text. ## References - Introduced with #5185 ## PR Checklist * [x] Closes #5471 * [x] Already signed life away to company. * [x] Manual tests passed since it's visual. * [x] No extra doc besides the comments. * [x] Am core contributor: Roar. The renderer base and specific renderer engine do a lot of work to remember the previous selection and compensate for scrolling regions and deltas between frames. However, all that work doesn't quite match up when the endpoints are changed out from under it. Unfortunately, `TermControl` doesn't have a robust history of locking correctly in step with the renderer nor does the renderer's `IRenderData` currently provide any way of 'snapping' state at the beginning of a frame so it could work without a full lock. So the solution for now is for the methods that scroll the display in `TermControl` to take the lock that is shared with the renderer's frame painter so they can't change out of sync. ## Validation Steps Performed - Opened terminal with Powershell core. Did ls a bunch of times. Clicked to make selection and held mouse button while wheeling around. - Opened terminal with Powershell core.; Did ls a bunch of times. Clicked to make selection and dragged mouse outside the window to make auto scroll happen. - Opened terminal with Powershell core. Did ls a bunch of times. Clicked to make selection and released. Wheeled around like a crazy person to make sure I didn't regress that.

ghost · 2020-05-05T22:27:52Z

🎉Windows Terminal Release Candidate v0.11.1251.0 (1.0rc1) has been released which incorporates this pull request.:tada:

Handy links:

## Summary of the Pull Request We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that. ## References #5185 - applies logic from this PR ## PR Checklist * [X] Closes #5756 ## Validation Steps Performed Followed bug repro steps.

## Summary of the Pull Request We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that. ## References #5185 - applies logic from this PR ## PR Checklist * [X] Closes #5756 ## Validation Steps Performed Followed bug repro steps. (cherry picked from commit 75752ae)

## Summary of the Pull Request We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that. ## References microsoft#5185 - applies logic from this PR ## PR Checklist * [X] Closes microsoft#5756 ## Validation Steps Performed Followed bug repro steps.

miniksa added 5 commits March 23, 2020 09:55

Start moving dx renderer onto bitmap.

7e8f039

it works!

b46b5d0

try to get present1 working and get mad because std::vector<bool> is …

78c1bc1

…bad.

Merge branch 'master' into dev/miniksa/dx_bitmap

39d67e3

Change invalidation to full rows to restore ligature drawing.

777524a

miniksa added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-Performance Performance-related issue Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Mar 30, 2020

miniksa self-assigned this Mar 30, 2020

miniksa commented Mar 30, 2020

View reviewed changes

DHowett-MSFT suggested changes Mar 30, 2020

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 30, 2020

miniksa added 3 commits March 30, 2020 16:20

Add tests for til and round out the operators.

a15af9b

code format

ac01c0b

code format and audit mode.

c478983

miniksa added 2 commits April 1, 2020 10:39

Dustin's suggestion about helper for invalidating rectangles .

f39b352

Fix selection issues.

0d863c2

miniksa added 2 commits April 1, 2020 12:40

Restore scrolling by appropriately scaling dirty rectangles from cell…

ea8e379

…s to pixels.

Don't redraw everything. We're competent enough at scrolling in the D…

1d72e6b

…X renderer now to know what the signal means and figure it out.

Disable incremental for High DPI as it doesn't work with the implicit…

b06720a

… render target scaling.

miniksa added 2 commits April 10, 2020 15:44

Put natural rendering back.

1207e80

Actually lock in selection code so it doesn't pull the rug out from u…

24ecd5b

…nder the renderer while it's operating.

skyline75489 mentioned this pull request Apr 12, 2020

docker stats flickers #1680

Closed

zadjii-msft mentioned this pull request Apr 13, 2020

[v0.10] When two characters are ligatured, the first one becomes invisible #4974

Open

zadjii-msft approved these changes Apr 13, 2020

View reviewed changes

DHowett-MSFT approved these changes Apr 13, 2020

View reviewed changes

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 13, 2020

ghost merged commit 79684bf into master Apr 13, 2020

ghost deleted the dev/miniksa/dx_bitmap_rows branch April 13, 2020 20:09

miniksa mentioned this pull request Apr 13, 2020

Adjusts High DPI scaling to enable differential rendering #5345

Merged

9 tasks

zadjii-msft mentioned this pull request Apr 15, 2020

Rendering slows down under load #1311

Closed

This was referenced Apr 15, 2020

Disable incremental rendering. #5365

Closed

Terminal Core: Whenever we scroll the buffer, we RepaintAll #1182

Closed

zadjii-msft mentioned this pull request Apr 17, 2020

mc is slower and shows artifacts #1665

Closed

This was referenced Apr 22, 2020

Font rendering issues in Terminal #610

Closed

Lock when changing selection endpoint on wheel/auto-scroll #5551

Merged

ghost mentioned this pull request May 5, 2020

Restore differential drawing to DirectWrite renderer #778

Closed

carlos-zamora mentioned this pull request May 7, 2020

Trigger scroll on scrolled selection #5798

Merged

1 task

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render row-by-row instead of invalidating entire screen #5185

Render row-by-row instead of invalidating entire screen #5185

miniksa commented Mar 30, 2020 •

edited by DHowett-MSFT

Loading

miniksa Mar 30, 2020

DHowett-MSFT commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

oising commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

DHowett-MSFT commented Apr 2, 2020

miniksa commented Apr 10, 2020

zadjii-msft left a comment

zadjii-msft Apr 13, 2020

DHowett-MSFT left a comment

DHowett-MSFT commented Apr 13, 2020

ghost commented Apr 13, 2020

miniksa commented Apr 15, 2020

ghost commented May 5, 2020

Render row-by-row instead of invalidating entire screen #5185

Render row-by-row instead of invalidating entire screen #5185

Conversation

miniksa commented Mar 30, 2020 • edited by DHowett-MSFT Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

miniksa Mar 30, 2020

Choose a reason for hiding this comment

DHowett-MSFT commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

oising commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

miniksa commented Apr 1, 2020

DHowett-MSFT commented Apr 2, 2020

miniksa commented Apr 10, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Apr 13, 2020

Choose a reason for hiding this comment

DHowett-MSFT left a comment

Choose a reason for hiding this comment

DHowett-MSFT commented Apr 13, 2020

ghost commented Apr 13, 2020

miniksa commented Apr 15, 2020

ghost commented May 5, 2020

miniksa commented Mar 30, 2020 •

edited by DHowett-MSFT

Loading