Restore differential drawing to DirectWrite renderer #778
Assignees
Labels
Area-Performance
Performance-related issue
Area-Rendering
Text rendering, emoji, complex glyph & font-fallback issues
Issue-Task
It's a feature request, but it doesn't really need a major design.
Priority-0
Bugs that we consider release-blocking/recall-class (P0)
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Comments
miniksa
added
Area-Rendering
|
I think line invalidation is an honorable first pass and would be welcome. |
miniksa
added
Issue-Task
This was referenced Jun 24, 2019
Closed
4 tasks
|
I've tried enable differential drawing on db79758 . To my surprise, it does not help much of the performance as I originally anticipated. First with #2959 and co the raw rendering performance is faster than before. In a way it is "fast enough" to deliquate the effect of differential drawing. Second, AFAIK most terminal operations requires entire screen redrawing, at which differential drawing makes little difference. Update: I'm blinded by the PC I've be using myself. People with 4K display or higher would still need this. |
cinnamon-msft
added
Priority-0
ghost
pushed a commit
that referenced
this issue
Feb 20, 2020
…es (#4664) ## Summary of the Pull Request Adds an ETW provider for tracing out operations inside the DirectX Renderer. ## References This supports #2191 and #778 and other rendering issues. ## PR Checklist * [x] I work here ## Detailed Description of the Pull Request / Additional comments This declares and defines the provider with the a GUID appropriate for the namespace and adds an initial invalidation rectangle method for figuring out what is being drawn mostly to understand what could be differential. ## Validation Steps Performed Implemented provider. Opened real-time ETW tracing tool Ran the Terminal and watched invalidation events appear live
ghost
pushed a commit
that referenced
this issue
Feb 21, 2020
…PTY rendering on trailing half of fullwidth glyphs (#4668) ## Summary of the Pull Request - Height adjustment of a glyph is now restricted to itself in the DX renderer instead of applying to the entire run - ConPTY compensates for drawing the right half of a fullwidth character. The entire render base has this behavior restored now as well. ## PR Checklist * [x] Closes #2191 * [x] I work here * [x] Tests added/passed * [x] No doc * [x] Am core contributor. ## Detailed Description of the Pull Request / Additional comments Two issues: 1. On the DirectX renderer side, when confronted with shrinking a glyph, the correction code would apply the shrunken size to the entire run, not just the potentially individual glyph that needed to be reduced in size. Unfortunately while adjusting the horizontal X width can be done for each glyph in a run, the vertical Y height has to be adjusted for an entire run. So the solution here was to split the individual glyph needing shrinking out of the run into its own run so it can be shrunk. 2. On the ConPTY side, there was a long standing TODO that was never completed to deal with a request to draw only the right half of a two-column character. This meant that when encountering a request for the right half only, we would transmit the entire full character to be drawn, left and right halves, struck over the right half position. Now we correct the cursor back a position (if space) and draw it out so the right half is struck over where we believe the right half should be (and the left half is updated as well as a consequence, which should be OK.) The reason this happens right now is because despite VIM only updating two cells in the buffer, the differential drawing calculation in the ConPTY is very simplistic and intersects only rectangles. This means from the top left most character drawn down to the row/col cursor count indicator in vim's modeline are redrawn with each character typed. This catches the line below the edited line in the typing and refreshes it. But incorrectly. We need to address making ConPTY smarter about what it draws incrementally as it's clearly way too chatty. But I plan to do that with some of the structures I will be creating to solve #778. ## Validation Steps Performed - Ran the scenario listed in #2191 in vim in the Terminal - Added unit tests similar to examples given around glyph/text mapping in runs from Microsoft community page
This was referenced Mar 5, 2020
Closed
|
|
|
I think that's justification for using the row/line invalidator for DX. It's better than painting the whole screen and works for ligatures. Not as fast as individual cells, but scoping it down further can be a later problem. |
5 tasks
ghost
pushed a commit
that referenced
this issue
Apr 13, 2020
## Summary of the Pull Request Adjusts DirectX renderer to use `til::bitmap` to track invalidation regions. Uses special modification to invalidate a row-at-a-time to ensure ligatures and NxM glyphs continue to work. ## References Likely helps #1064 ## PR Checklist * [x] Closes #778 * [x] I work here. * [x] Manual testing performed. See Performance traces in #778. * [x] Automated tests for `til` changes. * [x] Am core contributor. And discussed with @DHowett-MSFT. ## Detailed Description of the Pull Request / Additional comments - Applies `til::bitmap` as the new invalidation scheme inside the DirectX renderer and updates all entrypoints for collecting invalidation data to coalesce into this structure. - Semi-permanently routes all invalidations through a helper method `_InvalidateRectangle` that will expand any invalidation to cover the entire line. This ensures that ligatures and NxM glyphs will continue to render appropriately while still allowing us to dramatically reduce the number of lines drawn overall. In the future, we may come up with a tighter solution than line-by-line invalidation and can modify this helper method appropriately at that later date to further scope the invalid region. - Ensures that the `experimental.retroTerminalEffects` feature continues to invalidate the entire display on start of frame as the shader is applied at the end of the frame composition and will stack on itself in an amusing fashion when we only redraw part of the display. - Moves many member variables inside the DirectX renderer into the new `til::size`, `til::point`, and `til::rectangle` methods to facilitate easier management and mathematical operations. Consequently adds `try/catch` blocks around many of the already-existing `noexcept` methods to deal with mathematical or casting failures now detected by using the support classes. - Corrects `TerminalCore` redraw triggers to appropriately communicate scrolling circumstances to the renderer so it can optimize the draw regions appropriately. - Fixes an issue in the base `Renderer` that was causing overlapping scroll regions due to behavior of `Viewport::TrimToViewport` modifying the local. This fix is "good enough" for now and should go away when `Viewport` is fully migrated to `til::rectangle`. - Adds multiplication and division operators to `til::rectangle` and supporting tests. These operates will help scale back and forth between a cell-based rectangle and a pixel-based rectangle. They take special care to ensure that a pixel rectangle being divided downward back to cells will expand (with the ceiling division methods) to cover a full cell when even one pixel inside the cell is touched (as is how a redraw would have to occur). - Blocks off trace logging of invalid regions if no one is listening to optimize performance. - Restores full usage of `IDXGISwapChain1::Present1` to accurately and fully communicate dirty and scroll regions to the underlying DirectX framework. This additional information allows the framework to optimize drawing between frames by eliminating data transfer of regions that aren't modified and shuffling frames in place. See [Remarks](https://docs.microsoft.com/en-us/windows/win32/api/dxgi1_2/nf-dxgi1_2-idxgiswapchain1-present1#remarks) for more details. - Updates `til::bitmap` set methods to use more optimized versions of the setters on the `dynamic_bitset<>` that can bulk fill bits as the existing algorithm was noticeably slow after applying the "expand-to-row" helper to the DirectX renderer invalidation. - All `til` import hierarchy is now handled in the parent `til.h` file and not in the child files to prevent circular imports from happening. We don't expect the import of any individual library file, only the base one. So this should be OK for now. ## Validation Steps Performed - Ran `cmatrix`, `cmatrix -u0`, and `cacafire` after changes were made. - Made a bunch of ligatures with `Cascadia Code` in the Terminal before/after the changes and confirmed they still ligate. - Ran `dir` in Powershell and fixed the scrolling issues - Clicked all over the place and dragged to make sure selection works. - Checked retro terminal effect manually with Powershell.
ghost
added
the
Resolution-Fix-Committed
|
I have to back this out for 1.0. I can't land #5345. There are too many loose ends. |
miniksa
added
Needs-Triage
miniksa
removed
the
Needs-Triage
|
Closing this one out again pending our decision on differential drawing. We think we can stick the landing for v1. |
DHowett-MSFT
added
the
Resolution-Fix-Committed
|
🎉This issue was addressed in #5185, which has now been successfully released as Handy links: |
|
using the terminal is so much faster now. its just crazy fast. a git pull used to scroll so slowly and now its just boom done |
7 tasks
miniksa
added a commit
that referenced
this issue
May 11, 2020
## Summary of the Pull Request Adds user settings to adjust rendering behavior to mitigate blurry text on some devices. ## References - #778 introduced this, almost certainly. ## PR Checklist * [x] Closes #5759, mostly * [x] I work here. * [ ] We need community verification that this will help. * [x] Updated schema and schema doc. * [x] Am core contributor. Discussed in Monday sync meeting and w/ @DHowett-MSFT. ## Detailed Description of the Pull Request / Additional comments When we switched from full-screen repaints to incremental rendering, it seems like we exposed a situation where some display drivers and hardware combinations do not handle scroll and/or dirty regions (from `IDXGISwapChain::Present1`) without blurring the data from the previous frame. As we're really close to ship, I'm offering two options to let people in this situation escape it on their own. We hope in the future to figure out what's actually going on here and mitigate it further in software, but until then, these escape hatches are available. 1. `experimental.rendering.forceFullRepaint` - This one restores the pre-778 behavior to the Terminal. On every single frame paint, we'll invalidate the entire screen and repaint it. 2. `experimental.rendering.software` - This one uses the software WARP renderer instead of using the hardware and display driver directly. The theory is that this will sidestep any driver bugs or hardware variations. One, the other, or both of these may be field-applied by users who are experiencing this behavior. Reverting #778 completely would also resolve this, but it would give back our largest performance win in the whole Terminal project. We don't believe that's acceptable when seemingly a majority of the users are experiencing the performance benefit with no detriment to graphical display. ## Validation Steps Performed - [x] Flipped them on and verified with the debugger that they are being applied to the rendering pipeline - [ ] Gave a private copy to community members in #5759 and had them try whether one, the other, or both resolved their issue.
DHowett-MSFT
pushed a commit
that referenced
this issue
May 11, 2020
## Summary of the Pull Request Adds user settings to adjust rendering behavior to mitigate blurry text on some devices. ## References - #778 introduced this, almost certainly. ## PR Checklist * [x] Closes #5759, mostly * [x] I work here. * [ ] We need community verification that this will help. * [x] Updated schema and schema doc. * [x] Am core contributor. Discussed in Monday sync meeting and w/ @DHowett-MSFT. ## Detailed Description of the Pull Request / Additional comments When we switched from full-screen repaints to incremental rendering, it seems like we exposed a situation where some display drivers and hardware combinations do not handle scroll and/or dirty regions (from `IDXGISwapChain::Present1`) without blurring the data from the previous frame. As we're really close to ship, I'm offering two options to let people in this situation escape it on their own. We hope in the future to figure out what's actually going on here and mitigate it further in software, but until then, these escape hatches are available. 1. `experimental.rendering.forceFullRepaint` - This one restores the pre-778 behavior to the Terminal. On every single frame paint, we'll invalidate the entire screen and repaint it. 2. `experimental.rendering.software` - This one uses the software WARP renderer instead of using the hardware and display driver directly. The theory is that this will sidestep any driver bugs or hardware variations. One, the other, or both of these may be field-applied by users who are experiencing this behavior. Reverting #778 completely would also resolve this, but it would give back our largest performance win in the whole Terminal project. We don't believe that's acceptable when seemingly a majority of the users are experiencing the performance benefit with no detriment to graphical display. ## Validation Steps Performed - [x] Flipped them on and verified with the debugger that they are being applied to the rendering pipeline - [ ] Gave a private copy to community members in #5759 and had them try whether one, the other, or both resolved their issue. (cherry picked from commit d01317c)
jelster
pushed a commit
to jelster/terminal
that referenced
this issue
May 28, 2020
## Summary of the Pull Request Adds user settings to adjust rendering behavior to mitigate blurry text on some devices. ## References - microsoft#778 introduced this, almost certainly. ## PR Checklist * [x] Closes microsoft#5759, mostly * [x] I work here. * [ ] We need community verification that this will help. * [x] Updated schema and schema doc. * [x] Am core contributor. Discussed in Monday sync meeting and w/ @DHowett-MSFT. ## Detailed Description of the Pull Request / Additional comments When we switched from full-screen repaints to incremental rendering, it seems like we exposed a situation where some display drivers and hardware combinations do not handle scroll and/or dirty regions (from `IDXGISwapChain::Present1`) without blurring the data from the previous frame. As we're really close to ship, I'm offering two options to let people in this situation escape it on their own. We hope in the future to figure out what's actually going on here and mitigate it further in software, but until then, these escape hatches are available. 1. `experimental.rendering.forceFullRepaint` - This one restores the pre-778 behavior to the Terminal. On every single frame paint, we'll invalidate the entire screen and repaint it. 2. `experimental.rendering.software` - This one uses the software WARP renderer instead of using the hardware and display driver directly. The theory is that this will sidestep any driver bugs or hardware variations. One, the other, or both of these may be field-applied by users who are experiencing this behavior. Reverting microsoft#778 completely would also resolve this, but it would give back our largest performance win in the whole Terminal project. We don't believe that's acceptable when seemingly a majority of the users are experiencing the performance benefit with no detriment to graphical display. ## Validation Steps Performed - [x] Flipped them on and verified with the debugger that they are being applied to the rendering pipeline - [ ] Gave a private copy to community members in microsoft#5759 and had them try whether one, the other, or both resolved their issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For some reason, I turned this off in the haste to get things "working" well enough.
But it's definitely a performance hog. We should get differential drawing back inside the DirectWrite renderer.
Most of the provisions are already there with the invalid rectangle, we're just not using them.
I'm not sure if I'll be able to do cell-by-cell differential drawing like the GDI renderer because the more advanced glyphs/runs we support now (like ligatures) might necessitate that we instead just mark "invalid lines" and redraw the whole line. That would be mildly better than the whole screen.
This also would re-enable the use of the
Present1call to the DX infrastructure which is more efficient at scrolling and dirty regions.The text was updated successfully, but these errors were encountered: