Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AtlasEngine: Implement remaining grid lines #13587

Merged
5 commits merged into from
Jul 28, 2022
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 25, 2022

This commit implements the remaining 5 of 8 grid lines:
left/top/right/bottom (COMMON_LVB) borders and double underline

AtlasEngine::_resolveFontMetrics was partially refactored to use floats
instead of doubles, because that's what the remaining code uses as well.
It also helps the new, slightly more complex double underline calculation.

Validation Steps Performed

  • Print characters with the COMMON_LVB_GRID_HORIZONTAL, GRID_LVERTICAL,
    GRID_RVERTICAL and UNDERSCORE attributes via WriteConsoleOutputW
  • All 4 grid lines are visible ✅
  • Grid lines correctly scale according to the lineWidth
  • Print a double underline with printf "\033[21mtest\033[0m"
  • A double underline is fully visible ✅

Comment on lines 626 to 629
// A double-underline is 3 lines tall due to a 1 line gap between the two lines.
// This logic should be kept in sync with AtlasEngine::_updateConstantBuffer,
// which calculates the offset/position of the lower line.
const auto cellHeightViaUnderlines = underlinePosInPx + 3 * lineThicknessInPx;
Copy link
Member Author

@lhecker lhecker Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Consolas for instance the cellHeightViaUnderlines is usally larger than the cellHeightViaDescent. Without this code change here a double-underline would appear clipped off.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered just using the same algorithm that we have in the other renderers? Because people can be a picky about line spacing, and I suspect some users will not appreciate having their line height stretched to accommodate a double underline that they may never even use.

And eventually I'm assuming we'll have options for adjust line spacing (e.g. #3498), so we really need to be thinking in terms of fitting the content within the height that the user as requested (at least as best we can) rather than adjusting the height to fit the content.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I have. I choose to not use that, because I noticed that for most common fonts the descender height is approximately the same as the underline offset + 3 line widths down to just +/- 1px for fonts I have installed.

Since the difference was so small I built the PR this way. It allows me align double underlines with regular underlines in a consistent manner:

Monofur 24pt 150%
image

Consolas 24pt 100%
image

MS Gothic 10pt 100%
image

I liked that result a lot! And it only costs us +1px line height at most (for fonts I tested). Of course it's possible that it can increase the line height in weird ways for certain fonts, but so far I'm feeling somewhat confident that we should try this first.
Do you think that's acceptable? I'm open to adopting the algorithm you used in GdiEngine, but I feel like this approach has its advantages.

#3498 is a difficult one IMO. If we were to treat it like CSS line-height, then glyphs would simply overlap each other and not shrink the glyphs themselves or their underlines. I'd strongly prefer such a behavior if anything over fitting text contents into the given height. (I know how to do that, but it's "a bit" involved.)
A simpler solution is of course to simply clip the cell size to the given size and downscale glyphs as needed and is what I intend as a stop-gap solution as well. However I'm not really concerned about underlines being clipped away - if the user specifies a line height that's too small to support descenders (= underline offset + 3 lines widths +/- 1px), I'd simply clip the underline off. In practice that probably even works for most users. 🤔

Copy link
Collaborator

@j4james j4james Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know when I was testing I definitely had fonts where the underline position was right up against the bottom of the cell, which is why I had the fallback option of rendering the second line above the first. So even with a single pixel line width you'd need to stretch the height by at least two pixels on those cases, and wider line widths would assumedly be even more.

That said, I don't feel strongly about this either way. I only brought it up because I know some people are quite fanatical about line heights. Also, if we're hoping to replace the GDI engine in conhost with Atlas one day, I suspect we might need to retain the current cell height calculations for backwards compatibility reasons. I may be wrong about that though.

As for #3498, I could understand having the glyphs overlapping each other if the line height was too small for them to fit, but I'd definitely expect an overline to be clamped to the top of the cell, and the underline to be clamped to the bottom. To me it makes more sense to have an underline overlapping its own text rather than the text of the row below it.

But again I'm not hugely concerned about any of this. I just wanted to raise the issue in case you hadn't considered the other approach. I do like the fact that you're always getting a proper double underline with this method.

Copy link
Member Author

@lhecker lhecker Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, checking Word for reference, it seems that putting the second line above the underline might be generally preferable (Consolas 24pt):
image

It's halfway between baseline and underline. Interesting! But I'm not sure if I'm ready for such a more complex approach (it gets trickier at small font sizes). I'd rather adopt the Gdi/DxEngine code and iterate on this later on.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting. It also looks to me like they've taken the underline and sort of split it in half to produce the double underline, which makes it much more compact. But that only really works when the base underline width is more than a pixel to begin with.

Copy link
Member Author

@lhecker lhecker Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I did a little something... 😎 It's just 6 lines of code, but the result is surprisingly close to what Word does. It also adapts to small descender heights if needed.

Consolas
image

Courier New
image

Cascadia Code
image

MS Gothic (here you can see how the lower line gets squished back into the cell)
image

All images are at 24pt and 150% display scale.

@german-one
Copy link
Contributor

Curious: Are box drawing characters and block characters still conjunct? I recall #455.

@lhecker
Copy link
Member Author

lhecker commented Jul 25, 2022

@german-one I'm not sure what you mean... Are you asking whether the characters still properly overlap so that there are no pixel gaps between them? #13549 would fix that situation for the AtlasEngine. But this PR is only about lines unrelated to box drawings.

@german-one
Copy link
Contributor

Are you asking whether the characters still properly overlap so that there are no pixel gaps between them?

Yes that's what I've been asking. It's just because I interpreted ...

With Consolas for instance the cellHeightViaUnderlines is usally larger than the cellHeightViaDescent.

... like the height of the lines is going to get increased to fit the double underline. However, I probably missunderstood what it means.

@lhecker
Copy link
Member Author

lhecker commented Jul 25, 2022

However, I probably missunderstood what it means.

Actually you understood that correctly. 🙂 But we have that issue already anyways. #13549 will fix it categorically (by resizing those glyphs to fit the cell exactly).

@lhecker lhecker force-pushed the dev/lhecker/atlas-engine-lines branch from 539618e to dec1c11 Compare July 26, 2022 15:29
@lhecker lhecker added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-AtlasEngine labels Jul 26, 2022
@DHowett
Copy link
Member

DHowett commented Jul 26, 2022

Since the double underline may be taller than the descender height,
AtlasEngine::_resolveFontMetrics was slightly rewritten as well.

(Make sure you update the description to match the reality of what you implemented! I'm not sure if you have in this case, but reading this made me think of the implementation that made cells taller.)

src/renderer/atlas/shader_ps.hlsl Outdated Show resolved Hide resolved
@@ -394,14 +394,17 @@ namespace Microsoft::Console::Render
struct FontMetrics
{
wil::com_ptr<IDWriteFontCollection> fontCollection;
wil::unique_process_heap_string fontName;
std::wstring fontName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whhhyy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wil::unique_process_heap_string is not copyable and I didn't want to be overly complex. I mean it's 4x larger without benefits (wstring only stores 7 characters inline after all and even something trivial like "Consolas" is longer than that), but that doesn't really matter much here I think.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this bit of shader magic.

Comment on lines 674 to 681
const auto fontWeightU16 = gsl::narrow<u16>(requestedWeight);
const auto underlinePosU16 = gsl::narrow<u16>(underlinePos);
const auto underlineWidthU16 = gsl::narrow<u16>(underlineWidth);
const auto strikethroughPosU16 = gsl::narrow<u16>(strikethroughPos);
const auto strikethroughWidthU16 = gsl::narrow<u16>(strikethroughWidth);
const auto doubleUnderlinePosTopU16 = gsl::narrow<u16>(doubleUnderlinePosTop);
const auto doubleUnderlinePosBottomU16 = gsl::narrow<u16>(doubleUnderlinePosBottom);
const auto thinLineWidthU16 = gsl::narrow<u16>(thinLineWidth);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all throwing -- OK?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically I only want to throw if the result is out of bounds for an uint16_t. This code should work because floats can represent 16-bit integers exactly (i.e. converting back and forth yields the same result). I could convert them to int32_t first and then do the narrowing, e.g. with lround. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever you think strikes the best clarity vs work balance. This is fine with me :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this perhaps being unnecessarily cautious? I means is there any scenario here where a simple narrow_cast would actually produce a worse outcome than a throw?

Copy link
Member Author

@lhecker lhecker Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I tend to prefer gsl::narrow over gsl::narrow_cast unless I need the latter for performance or exception safety (or I just want narrowing). I don't think the cost of the narrowing check is particularly high in most cases, so I'm fine with it.

In this case I'm a bit afraid that these values could be negative or larger than UINT16_MAX (since they come from the font file which might specify anything). I'll probably modify this PR to use gsl::narrow<u16>(lroundf(...)) which is even more excessively cautious, but it avoids any potential rounding errors, while catching any weird behavior (e.g. font size of >9000 leads to cell size >65536) and is fast enough (this code is only called once and each lroundf is <10ns).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was getting at is say you have some kind of overflow, and your underline position is now so large that it's off screen, is that a big deal? Is it likely to crash? If not, and the alternative is that the font just doesn't work at all, it doesn't seem like the narrow_cast is really an improvement. But it's not important. It was just a passing thought.

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 28, 2022
@ghost
Copy link

ghost commented Jul 28, 2022

Hello @lhecker!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 2470805 into main Jul 28, 2022
@ghost ghost deleted the dev/lhecker/atlas-engine-lines branch July 28, 2022 13:34
DHowett pushed a commit that referenced this pull request Sep 9, 2022
AtlasEngine: Implement LRU invalidation for glyph tiles (#13458)

So far AtlasEngine would only grow the backing texture atlas once it gets full,
without the ability to reuse tiles once it gets full. This commit adds LRU
capabilities to the glyph-to-tile hashmap, allowing us to reuse the least
recently used tiles for new ones once the atlas texture is full.
This commit uses a quadratic growth factor with power-of-2 textures,
resulting in a backing atlas of 1x to 2x the size of the window.
While AtlasEngine is still incapable of shrinking the texture, it'll now at
least not grow to 128MB or result in weird glitches under most circumstances.

* Print `utf8_sequence_0-0x2ffff_assigned_printable_unseparated.txt`
  from https://github.com/bits/UTF-8-Unicode-Test-Documents
* Scroll back up to the top
* PowerShell input line is still there rendering as ASCII. ✅

AtlasEngine: Improve glyph generation performance (#13477)

so that we stop running out of GPU memory for complex Unicode.
This however can result in our glyph generation being a performance issue in
edge cases, to the point that the application may feel outright unuseable.

CJK glyphs for instance can easily exceed the maximum atlas texture size
(twice the window size), but take a significant amount of CPU and GPU time to
rasterize and draw, which results in "jelly scrolling" down to ~1 FPS.
This PR improves the situation of the latter half by directly drawing
glyphs into the texture atlas without an intermediate scratchpad texture.

This reduces GPU usage by 96% on my system (33% -> 2%) which improves general
render performance by ~100% (15 -> 30 FPS). CPU usage remains the same however,
but that's not really something we can do anything about at this time.
The atlas texture is already our primary means to reduce the CPU cost after all.

* Disable V-Sync for OpenConsole in NVIDIA Control Panel
* Enable `debugGlyphGenerationPerformance`
* Print the entire CJK block U+4E00..U+9FFF
* Measure the above GPU usage and FPS improvements ✅
  (Alternatively: Just scroll around and judge the "jellyness".)

AtlasEngine: Fix bugs introduced in 66f4f9d and d74b66a (#13496)

We only process glyphs within the dirtyRect, but glyphs outside of the dirtyRect
are still in use and shouldn't be discarded. This is critical if someone uses
a tool like tmux to split the terminal horizontally. If they then print a lot
of Unicode text on just one side, we have to ensure that the (for example)
plain ASCII glyphs on the other half of the viewport are still retained.

The cursor was drawn without a clip rect, causing the entire atlas
texture to be filled with black. This just so happened to work fine
in Windows Terminal but relied on a race condition.

Closes #13490

* Disappearing glyphs
  * Start `tmux` in `wsl`
  * Split horizontally with `Ctrl+B`, `"`
  * `cat` a huge Unicode text file on the bottom
  * Ensure ASCII glyphs in the top half don't disappear ✅
* Black viewport after font changes
  * Start `OpenConsole` with `AtlasEngine`
  * Open Properties dialog and click "Ok"
  * Viewport content doesn't disappear ✅

AtlasEngine: Improve robustness against TextBuffer bugs (#13530)

The current TextBuffer implementation will happily overwrite the
leading/trailing half of a wide glyph with a narrow one without
padding the other half with whitespace. This could crash AtlasEngine
which aggressively guarded against such inconsistencies.

Closes #13522

* Run .bat file linked in #13522
  (Override wide glyph with a single space.)
* `AtlasEngine` doesn't crash ✅

AtlasEngine: Handle IntenseIsBold (#13577)

This change adds support for the `IntenseIsBold` rendering setting.
Windows Terminal for instance defaults to `false` here, causing
intense colors to only be bright but not bold.

* Set "Intense text style" to "Bright colors"
* Enable AtlasEngine
* Print ``echo "`e[1mtest`e[0m"``
* "test" appears as bright without being bold ✅

AtlasEngine: Fix LRU state after scrolling (#13607)

66f4f9d had another bug: Just like how we scroll our viewport by `memmove`ing
the `_r.cells` array, we also have to `memmove` the new `_r.cellGlyphMapping`.
Without this fix drawing lots of glyphs while only scrolling slightly
(= not invalidating the entire viewport), would erroneously call
`makeNewest` on glyphs now outside of the viewport. This would cause
actually visible glyphs to be recycled and overwritten by new ones.

* Switch to Cascadia Code
* Print some text that fills the glyph atlas
* Scroll down by a few rows
* Write a long "==========" ligature (this quickly fills up
  any remaining space in the atlas and exacerbates the issue)
* Unrelated rows don't get corrupted ✅

AtlasEngine: Remove support for Windows 7 (#13608)

We recently figured that we can drop support for Windows 7. Coincidentally
AtlasEngine never actually supported Windows 7 properly, because it called
`ResizeBuffers` with `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT`
no matter whether the swap chain was created with it enabled.

The new minimally supported version is Windows 8.1.

AtlasEngine: Implement remaining grid lines (#13587)

This commit implements the remaining 5 of 8 grid lines:
left/top/right/bottom (COMMON_LVB) borders and double underline

`AtlasEngine::_resolveFontMetrics` was partially refactored to use `float`s
instead of `double`s, because that's what the remaining code uses as well.
It also helps the new, slightly more complex double underline calculation.

* Print characters with the `COMMON_LVB_GRID_HORIZONTAL`, `GRID_LVERTICAL`,
  `GRID_RVERTICAL` and `UNDERSCORE` attributes via `WriteConsoleOutputW`
* All 4 grid lines are visible ✅
* Grid lines correctly scale according to the `lineWidth` ✅
* Print a double underline with `printf "\033[21mtest\033[0m"`
* A double underline is fully visible ✅

AtlasEngine: Scale glyphs to better fit the cell size (#13549)

This commit contains 3 improvements for glyph rendering:
* Scale block element and box drawing characters to fit the cell size
  "perfectly" without leaving pixel gaps between cells.
* Use `IDWriteTextLayout::GetOverhangMetrics` to determine whether glyphs
  are outside the given layout box and if they are, offset their position
  to fit them back in. If that still fails to fit, we downscale them.
* Always scale up glyphs that are more than 2 cells wide
  This ensures that long ligatures that mimic box drawing characters like
  "===" under Cascadia Code are upscaled just like regular box drawings.
  Unfortunately this results in ligature-heavy text (like Myanmar) to get an
  "uneven" appearance because some ligatures can suddenly appear too large.
  It's difficult to come up with a good heuristic here.

Closes #12512

* Print UTF-8-demo.txt
* Block characters don't leave gaps ✅
* Print a lorem-ipsum in Myanmar
* Glyphs aren't cut off anymore ✅
* Print a long "===" ligature under Cascadia Code
* The ligature is as wide as the number of cells used ✅

AtlasEngine: Recognize Powerline glyphs (#13650)

This commit makes AtlasEngine recognize Powerline glyphs as box drawing ones.
The extra pixel offsets when determining the `scale` caused weird artifacts
and thus were removed. It seems like this causes no noticeable regressions.

Closes #13029

* Run all values of `wchar_t` through `isInInversionList`
  and ensure it produces the expected value ✅
* Powerline glyphs are correctly scaled with Cascadia Code PL ✅

AtlasEngine: Fix debugGlyphGenerationPerformance (#13757)

`debugGlyphGenerationPerformance` used to only test the performance of
text segmentation/parsing, so I renamed it to `debugTextParsingPerformance`.
The new `debugGlyphGenerationPerformance` actually clears the glyph atlas now.

Additionally this fixes a bug with `debugGeneralPerformance`:
If a `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT` is requested,
it needs to be used. Since `debugGeneralPerformance` is for testing without
V-Sync, we need to ensure that the waitable object is properly disabled.

AtlasEngine: Fix the fix for LRU state after scrolling (#13784)

The absolute disgrace of a fix called 65b71ff failed to account for `std::move`
being unsafe to use for overlapping ranges. While `std::move` works for trivial
types (it happens to delegate to `memmove`), we need to dynamically switch
between that and `std::move_backward` to be correct.

Without this fix the LRU refresh is incorrect and might lead to crashes.

I'm working on a new, pure D2D renderer inside AtlasEngine, which uses
the iterators contained in `_r.cellGlyphMapping` to draw text.
I noticed the bug, because scrolling up caused the text to be garbled
and with this fix applied it works as expected.

AtlasEngine: Round cell sizes to nearest instead of up (#13833)

After some deliberation I noticed that rounding the glyph advance up to yield
the cell width is at least just as wrong as rounding it. This is because
we draw glyphs centered, meaning that (at least in theory) anti-aliased
pixels might clip outside of the layout box on _both_ sides of the glyph
adding not 1 but 2 extra pixels to the glyph size. Instead of just `ceilf`
we would have had to use `ceilf(advanceWidth / 2) * 2` to account for that.

This commit simplifies our issue by just going with what other applications do:
Round all sizes (cell width and height) to the nearest pixel size.

Closes #13812

* Set a breakpoint on `scaling Required == true` in `AtlasEngine::_drawGlyph`
* Test an assortment of Cascadia Mono, Consolas, MS Gothic, Lucida Console
  at various font sizes (6, 7, 8, 10, 12, 24, ...)
* Ensure breakpoint isn't hit ✅
  This tells us that no glyph resizing was necessary

AtlasEngine: Improve RDP performance (#13816)

Direct2D is able to detect remote connections and will switch to sending
draw commands across RDP instead of rendering the data on the server.
This reduces the amount of data that needs to be transmitted as well
as the CPU load of the server, if it has no GPU installed.
This commit changes `AtlasEngine` to render with just Direct2D if a software or
remote device was chosen by `D3D11CreateDevice`. Selecting the DXGI adapter the
window is on explicitly in the future would allow us to be more precise here.

This new rendering mode doesn't implement some of the more fancy features just
yet, like inverted cursors or coloring a single wide glyph in multiple colors.
It reuses most existing facilities and uses the existing tile hash map to cache
DirectWrite text layouts to improve performance. Unfortunately this does incur
a fairly high memory overhead of approximately 25MB for a 120x30 viewport.

Additional drive-by changes include:
* Treat the given font size exactly as its given without rounding
  Apparently we don't really need to round the font size to whole pixels
* Stop updating the const buffer on every frame
* Support window resizing if `debugGeneralPerformance` is enabled

Closes #13079

* Tested fairly exhaustively over RDP ✅

AtlasEngine: Implement support for custom shaders (#13885)

This commit implements support for custom shaders in AtlasEngine
(`experimental.retroTerminalEffect` and `experimental.pixelShaderPath`).
Setting these properties invalidates the device because that made it
the easiest to implement this less often used feature.
The retro shader was slightly rewritten so that it compiles without warnings.

Additionally we noticed that AtlasEngine works well with D3D 10.0 hardware,
so support for that was added bringing feature parity with DxRenderer.

Closes #13853

* Default settings (Independent Flip) ✅
* ClearType (Independent Flip) ✅
* Retro Terminal Effect (Composed Flip) ✅
* Use wallpaper as background image (Composed Flip) ✅
  * Running `color 40` draws everything red ✅
  * With Retro Terminal Effect ✅

AtlasEngine: Add support for SetSoftwareRendering (#13886)

This commit implements support for `experimental.rendering.software`.
There's not much to it. It's just another 2 if conditions.

* `"experimental.rendering.software": false` renders with D3D ✅
* `"experimental.rendering.software": true` triggers the new code path ✅

atlas: only enable continuous redraw if the shader needs it (#13903)

We do this by detecting whether the shader is using variable 0 in
constant buffer 0 (typically "time", but it can go by many names.)

Closes #13901

AtlasEngine: Fix various bugs found in testing (#13906)

In testing the following issues were found in AtlasEngine and fixed:
1. "Toggle terminal visual effects" action not working
2. `d2dMode` failed to work with transparent backgrounds
3. `GetSwapChainHandle()` is thread-unsafe due to it being called outside
  of the console lock and with single-threaded Direct2D enabled
4. 2 swap chain buffers are less performant than 3
5. Flip-Discard and `Present()` is less energy efficient than
  Flip-Sequential and `Present1()`
6. `d2dMode` used to copy the front to back buffer for partial rendering,
  but always redraw the entire dirty region anyways
7. Added support for DirectX 9 hardware
8. If custom shaders are used not all pixels would be presented

Closes #13906

1. Toggling visual effects runs retro shader ✅
   With a custom shader set, it toggles the shader ✅
   Toggling `experimental.rendering.software` toggles the shader ✅
2. `"backgroundImage": "desktopWallpaper"` works with D2D ✅ and D3D ✅
3. Adding a `Sleep(3000)` in `_AttachDxgiSwapChainToXaml` doesn't break
   Windows 10 ✅ nor Windows 11 ✅
4. Screen animations run at 144 FPS ✅ even while moving the window ✅
5. No weird artefacts during cursor movement or scrolling ✅
6. No weird artefacts during cursor movement or scrolling ✅
7. Forcing DirectX 9.3 in `dxcpl` runs fine ✅

AtlasEngine: Fix a correctness bug (#13956)

`ATLAS_POD_OPS` doesn't check for `has_unique_object_representations` and so a
bug exists where `CachedCursorOptions` comparisons invoke undefined behavior.
@ghost
Copy link

ghost commented Sep 13, 2022

🎉Windows Terminal Preview v1.16.252 has been released which incorporates this pull request.:tada:

Handy links:

DHowett pushed a commit that referenced this pull request Sep 21, 2022
AtlasEngine: Implement LRU invalidation for glyph tiles (#13458)

So far AtlasEngine would only grow the backing texture atlas once it gets full,
without the ability to reuse tiles once it gets full. This commit adds LRU
capabilities to the glyph-to-tile hashmap, allowing us to reuse the least
recently used tiles for new ones once the atlas texture is full.
This commit uses a quadratic growth factor with power-of-2 textures,
resulting in a backing atlas of 1x to 2x the size of the window.
While AtlasEngine is still incapable of shrinking the texture, it'll now at
least not grow to 128MB or result in weird glitches under most circumstances.

* Print `utf8_sequence_0-0x2ffff_assigned_printable_unseparated.txt`
  from https://github.com/bits/UTF-8-Unicode-Test-Documents
* Scroll back up to the top
* PowerShell input line is still there rendering as ASCII. ✅

AtlasEngine: Improve glyph generation performance (#13477)

so that we stop running out of GPU memory for complex Unicode.
This however can result in our glyph generation being a performance issue in
edge cases, to the point that the application may feel outright unuseable.

CJK glyphs for instance can easily exceed the maximum atlas texture size
(twice the window size), but take a significant amount of CPU and GPU time to
rasterize and draw, which results in "jelly scrolling" down to ~1 FPS.
This PR improves the situation of the latter half by directly drawing
glyphs into the texture atlas without an intermediate scratchpad texture.

This reduces GPU usage by 96% on my system (33% -> 2%) which improves general
render performance by ~100% (15 -> 30 FPS). CPU usage remains the same however,
but that's not really something we can do anything about at this time.
The atlas texture is already our primary means to reduce the CPU cost after all.

* Disable V-Sync for OpenConsole in NVIDIA Control Panel
* Enable `debugGlyphGenerationPerformance`
* Print the entire CJK block U+4E00..U+9FFF
* Measure the above GPU usage and FPS improvements ✅
  (Alternatively: Just scroll around and judge the "jellyness".)

AtlasEngine: Fix bugs introduced in 66f4f9d and d74b66a (#13496)

We only process glyphs within the dirtyRect, but glyphs outside of the dirtyRect
are still in use and shouldn't be discarded. This is critical if someone uses
a tool like tmux to split the terminal horizontally. If they then print a lot
of Unicode text on just one side, we have to ensure that the (for example)
plain ASCII glyphs on the other half of the viewport are still retained.

The cursor was drawn without a clip rect, causing the entire atlas
texture to be filled with black. This just so happened to work fine
in Windows Terminal but relied on a race condition.

Closes #13490

* Disappearing glyphs
  * Start `tmux` in `wsl`
  * Split horizontally with `Ctrl+B`, `"`
  * `cat` a huge Unicode text file on the bottom
  * Ensure ASCII glyphs in the top half don't disappear ✅
* Black viewport after font changes
  * Start `OpenConsole` with `AtlasEngine`
  * Open Properties dialog and click "Ok"
  * Viewport content doesn't disappear ✅

AtlasEngine: Improve robustness against TextBuffer bugs (#13530)

The current TextBuffer implementation will happily overwrite the
leading/trailing half of a wide glyph with a narrow one without
padding the other half with whitespace. This could crash AtlasEngine
which aggressively guarded against such inconsistencies.

Closes #13522

* Run .bat file linked in #13522
  (Override wide glyph with a single space.)
* `AtlasEngine` doesn't crash ✅

AtlasEngine: Handle IntenseIsBold (#13577)

This change adds support for the `IntenseIsBold` rendering setting.
Windows Terminal for instance defaults to `false` here, causing
intense colors to only be bright but not bold.

* Set "Intense text style" to "Bright colors"
* Enable AtlasEngine
* Print ``echo "`e[1mtest`e[0m"``
* "test" appears as bright without being bold ✅

AtlasEngine: Fix LRU state after scrolling (#13607)

66f4f9d had another bug: Just like how we scroll our viewport by `memmove`ing
the `_r.cells` array, we also have to `memmove` the new `_r.cellGlyphMapping`.
Without this fix drawing lots of glyphs while only scrolling slightly
(= not invalidating the entire viewport), would erroneously call
`makeNewest` on glyphs now outside of the viewport. This would cause
actually visible glyphs to be recycled and overwritten by new ones.

* Switch to Cascadia Code
* Print some text that fills the glyph atlas
* Scroll down by a few rows
* Write a long "==========" ligature (this quickly fills up
  any remaining space in the atlas and exacerbates the issue)
* Unrelated rows don't get corrupted ✅

AtlasEngine: Remove support for Windows 7 (#13608)

We recently figured that we can drop support for Windows 7. Coincidentally
AtlasEngine never actually supported Windows 7 properly, because it called
`ResizeBuffers` with `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT`
no matter whether the swap chain was created with it enabled.

The new minimally supported version is Windows 8.1.

AtlasEngine: Implement remaining grid lines (#13587)

This commit implements the remaining 5 of 8 grid lines:
left/top/right/bottom (COMMON_LVB) borders and double underline

`AtlasEngine::_resolveFontMetrics` was partially refactored to use `float`s
instead of `double`s, because that's what the remaining code uses as well.
It also helps the new, slightly more complex double underline calculation.

* Print characters with the `COMMON_LVB_GRID_HORIZONTAL`, `GRID_LVERTICAL`,
  `GRID_RVERTICAL` and `UNDERSCORE` attributes via `WriteConsoleOutputW`
* All 4 grid lines are visible ✅
* Grid lines correctly scale according to the `lineWidth` ✅
* Print a double underline with `printf "\033[21mtest\033[0m"`
* A double underline is fully visible ✅

AtlasEngine: Scale glyphs to better fit the cell size (#13549)

This commit contains 3 improvements for glyph rendering:
* Scale block element and box drawing characters to fit the cell size
  "perfectly" without leaving pixel gaps between cells.
* Use `IDWriteTextLayout::GetOverhangMetrics` to determine whether glyphs
  are outside the given layout box and if they are, offset their position
  to fit them back in. If that still fails to fit, we downscale them.
* Always scale up glyphs that are more than 2 cells wide
  This ensures that long ligatures that mimic box drawing characters like
  "===" under Cascadia Code are upscaled just like regular box drawings.
  Unfortunately this results in ligature-heavy text (like Myanmar) to get an
  "uneven" appearance because some ligatures can suddenly appear too large.
  It's difficult to come up with a good heuristic here.

Closes #12512

* Print UTF-8-demo.txt
* Block characters don't leave gaps ✅
* Print a lorem-ipsum in Myanmar
* Glyphs aren't cut off anymore ✅
* Print a long "===" ligature under Cascadia Code
* The ligature is as wide as the number of cells used ✅

AtlasEngine: Recognize Powerline glyphs (#13650)

This commit makes AtlasEngine recognize Powerline glyphs as box drawing ones.
The extra pixel offsets when determining the `scale` caused weird artifacts
and thus were removed. It seems like this causes no noticeable regressions.

Closes #13029

* Run all values of `wchar_t` through `isInInversionList`
  and ensure it produces the expected value ✅
* Powerline glyphs are correctly scaled with Cascadia Code PL ✅

AtlasEngine: Fix debugGlyphGenerationPerformance (#13757)

`debugGlyphGenerationPerformance` used to only test the performance of
text segmentation/parsing, so I renamed it to `debugTextParsingPerformance`.
The new `debugGlyphGenerationPerformance` actually clears the glyph atlas now.

Additionally this fixes a bug with `debugGeneralPerformance`:
If a `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT` is requested,
it needs to be used. Since `debugGeneralPerformance` is for testing without
V-Sync, we need to ensure that the waitable object is properly disabled.

AtlasEngine: Fix the fix for LRU state after scrolling (#13784)

The absolute disgrace of a fix called 65b71ff failed to account for `std::move`
being unsafe to use for overlapping ranges. While `std::move` works for trivial
types (it happens to delegate to `memmove`), we need to dynamically switch
between that and `std::move_backward` to be correct.

Without this fix the LRU refresh is incorrect and might lead to crashes.

I'm working on a new, pure D2D renderer inside AtlasEngine, which uses
the iterators contained in `_r.cellGlyphMapping` to draw text.
I noticed the bug, because scrolling up caused the text to be garbled
and with this fix applied it works as expected.

AtlasEngine: Round cell sizes to nearest instead of up (#13833)

After some deliberation I noticed that rounding the glyph advance up to yield
the cell width is at least just as wrong as rounding it. This is because
we draw glyphs centered, meaning that (at least in theory) anti-aliased
pixels might clip outside of the layout box on _both_ sides of the glyph
adding not 1 but 2 extra pixels to the glyph size. Instead of just `ceilf`
we would have had to use `ceilf(advanceWidth / 2) * 2` to account for that.

This commit simplifies our issue by just going with what other applications do:
Round all sizes (cell width and height) to the nearest pixel size.

Closes #13812

* Set a breakpoint on `scaling Required == true` in `AtlasEngine::_drawGlyph`
* Test an assortment of Cascadia Mono, Consolas, MS Gothic, Lucida Console
  at various font sizes (6, 7, 8, 10, 12, 24, ...)
* Ensure breakpoint isn't hit ✅
  This tells us that no glyph resizing was necessary

AtlasEngine: Improve RDP performance (#13816)

Direct2D is able to detect remote connections and will switch to sending
draw commands across RDP instead of rendering the data on the server.
This reduces the amount of data that needs to be transmitted as well
as the CPU load of the server, if it has no GPU installed.
This commit changes `AtlasEngine` to render with just Direct2D if a software or
remote device was chosen by `D3D11CreateDevice`. Selecting the DXGI adapter the
window is on explicitly in the future would allow us to be more precise here.

This new rendering mode doesn't implement some of the more fancy features just
yet, like inverted cursors or coloring a single wide glyph in multiple colors.
It reuses most existing facilities and uses the existing tile hash map to cache
DirectWrite text layouts to improve performance. Unfortunately this does incur
a fairly high memory overhead of approximately 25MB for a 120x30 viewport.

Additional drive-by changes include:
* Treat the given font size exactly as its given without rounding
  Apparently we don't really need to round the font size to whole pixels
* Stop updating the const buffer on every frame
* Support window resizing if `debugGeneralPerformance` is enabled

Closes #13079

* Tested fairly exhaustively over RDP ✅

AtlasEngine: Implement support for custom shaders (#13885)

This commit implements support for custom shaders in AtlasEngine
(`experimental.retroTerminalEffect` and `experimental.pixelShaderPath`).
Setting these properties invalidates the device because that made it
the easiest to implement this less often used feature.
The retro shader was slightly rewritten so that it compiles without warnings.

Additionally we noticed that AtlasEngine works well with D3D 10.0 hardware,
so support for that was added bringing feature parity with DxRenderer.

Closes #13853

* Default settings (Independent Flip) ✅
* ClearType (Independent Flip) ✅
* Retro Terminal Effect (Composed Flip) ✅
* Use wallpaper as background image (Composed Flip) ✅
  * Running `color 40` draws everything red ✅
  * With Retro Terminal Effect ✅

AtlasEngine: Add support for SetSoftwareRendering (#13886)

This commit implements support for `experimental.rendering.software`.
There's not much to it. It's just another 2 if conditions.

* `"experimental.rendering.software": false` renders with D3D ✅
* `"experimental.rendering.software": true` triggers the new code path ✅

atlas: only enable continuous redraw if the shader needs it (#13903)

We do this by detecting whether the shader is using variable 0 in
constant buffer 0 (typically "time", but it can go by many names.)

Closes #13901

AtlasEngine: Fix various bugs found in testing (#13906)

In testing the following issues were found in AtlasEngine and fixed:
1. "Toggle terminal visual effects" action not working
2. `d2dMode` failed to work with transparent backgrounds
3. `GetSwapChainHandle()` is thread-unsafe due to it being called outside
  of the console lock and with single-threaded Direct2D enabled
4. 2 swap chain buffers are less performant than 3
5. Flip-Discard and `Present()` is less energy efficient than
  Flip-Sequential and `Present1()`
6. `d2dMode` used to copy the front to back buffer for partial rendering,
  but always redraw the entire dirty region anyways
7. Added support for DirectX 9 hardware
8. If custom shaders are used not all pixels would be presented

Closes #13906

1. Toggling visual effects runs retro shader ✅
   With a custom shader set, it toggles the shader ✅
   Toggling `experimental.rendering.software` toggles the shader ✅
2. `"backgroundImage": "desktopWallpaper"` works with D2D ✅ and D3D ✅
3. Adding a `Sleep(3000)` in `_AttachDxgiSwapChainToXaml` doesn't break
   Windows 10 ✅ nor Windows 11 ✅
4. Screen animations run at 144 FPS ✅ even while moving the window ✅
5. No weird artefacts during cursor movement or scrolling ✅
6. No weird artefacts during cursor movement or scrolling ✅
7. Forcing DirectX 9.3 in `dxcpl` runs fine ✅

AtlasEngine: Fix a correctness bug (#13956)

`ATLAS_POD_OPS` doesn't check for `has_unique_object_representations` and so a
bug exists where `CachedCursorOptions` comparisons invoke undefined behavior.

Relax shader strictness in RELEASE mode (#13998)

Disables strictness and warnings as errors for custom pixel shaders in
RELEASE. Windows terminal is not telling the user why the shader won't
compile which makes it very frustrating for the shader hacker.

After trying the recent preview none of my shaders loaded anymore in
Windows Terminal Preview which made me very sad. I had no idea what was
wrong with them. After cloning the git repo, building it, fighting an
issue that prevent DEBUG SDK from being used I finally was able to
identify some issues that were blocking my shaders.

> error X3556: integer modulus may be much slower, try using uints if possible.
> error X4000: use of potentially uninitialized variable (rayCylinder)

While the first one is a good warning I don't think it is an error and
the tools I use didn't flag it so was hard to know.

The second one I was staring at the code and was unable to identify what
exactly was causing the issues, I fumbled with the code a few times and
just felt the fun drain away.

IMHO: I want it to be fun to develop shaders for windows terminal.
Fighting invisible errors are not fun. I am not after building
production shaders for Windows Terminal, I want some cool effects. So
while I am as a .NET developer always runs with Warning as errors I
don't think it's the right option here. Especially since Windows
Terminal doesn't tell what is the problem.

However, I understand if the shaders you ship with Windows Terminal
should be free of errors and silly mistakes, so I kept the stricter
setting in DEBUG mode.

Loaded Windows Terminal in RELEASE and DEBUG mode and validated that
RELEASE mode had reduced strictness but DEBUG retained the previous more
restrictive mode.

(cherry picked from commit b4b6636)
Service-Card-Id: 85660397
Service-Version: 1.16
(cherry picked from commit b899d49)

AtlasEngine: Properly detect shader model 4 support (#13994)

Direct3D 10.0 and 10.1 only have optional support for shader model 4.
This commit fixes our assumption that it's always present by checking
`ComputeShaders_Plus_RawAndStructuredBuffers_Via_Shader_4_x` first.

Closes #13985

* Set feature level to 10.1 via `dxcpl`
* `CheckFeatureSupport` is called and doesn't throw ✅

(cherry picked from commit e2b2d9b)
Service-Card-Id: 85653388
Service-Version: 1.16
(cherry picked from commit 5e9147e)

AtlasEngine: Fix a crash when drawing double width rows (#13966)

The `TileHashMap` refresh via `makeNewest()` in `StartPaint()` depends
on us filling the entire `cellGlyphMapping` row with valid data.
This commit makes sure to initialize the `cellGlyphMapping` buffer.
Additionally it clears the rest of the row with whitespace
until proper `LineRendition` support is added.

Closes #13962

* vttest's "Test of double-sized characters" stops crashing ✅
* No weird leftover characters ✅

(cherry picked from commit 16aa79d)
Service-Card-Id: 85653281
Service-Version: 1.16
(cherry picked from commit c2c5f41)

AtlasEngine: Fix bugs around bitmap font rendering (#14014)

This commit fixes several issues:
* Some fonts set a line-gap even though they behave as if they
  don't want any line-gaps. Since Terminals don't really have
  any gaps anyways, it'll now not be taken into account anymore.
* Center alignment breaks bitmap glyphs which expect left-alignment.
* Automatic "opsz" axis makes Terminus TTF's italic glyphs look quite
  weird. I disabled this feature as we might not need it anyways.

A complete fix depends on #14013
Closes #14006

* Use Terminus TTF at 13.5pt
* Print UTF-8-demo.txt
* No gaps between block characters ✅

(cherry picked from commit bea13bd)
Service-Card-Id: 85767355
Service-Version: 1.16
(cherry picked from commit 9310db5)

AtlasEngine: Fix cursor invalidation (#14038)

There's a different behavior regarding cursors between conhost and Windows
Terminal. In case of the latter we don't necessarily call `PaintCursor`
during cursor movement, because the cursor blinker never stops "blinking".

Closes #14028

* Enter text until after the line wraps
* Hold backspace until the line unwraps
* No leftover cursor on the second line ✅

(cherry picked from commit 08096b2)
Service-Card-Id: 85767353
Service-Version: 1.16
(cherry picked from commit 9d0346c)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants