AtlasEngine: Fix various bugs found in testing #13906

lhecker · 2022-09-01T23:13:16Z

In testing the following issues were found in AtlasEngine and fixed:

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

Validation Steps Performed

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

DHowett · 2022-09-01T23:18:56Z

src/cascadia/TerminalControl/ControlCore.cpp

@@ -326,7 +326,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

            // Tell the DX Engine to notify us when the swap chain changes.
            // We do this after we initially set the swapchain so as to avoid unnecessary callbacks (and locking problems)
-            _renderEngine->SetCallback(std::bind(&ControlCore::_renderEngineSwapChainChanged, this));
+            _renderEngine->SetCallback([this](auto handle) { _renderEngineSwapChainChanged(handle); });


lhecker · 2022-09-01T23:18:32Z

src/cascadia/TerminalControl/ControlCore.cpp

@@ -326,7 +326,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

            // Tell the DX Engine to notify us when the swap chain changes.
            // We do this after we initially set the swapchain so as to avoid unnecessary callbacks (and locking problems)
-            _renderEngine->SetCallback(std::bind(&ControlCore::_renderEngineSwapChainChanged, this));
+            _renderEngine->SetCallback([this](auto handle) { _renderEngineSwapChainChanged(handle); });


For some reason std::bind wouldn't work and I didn't want to figure it out.

lhecker · 2022-09-01T23:18:55Z

src/cascadia/TerminalControl/ControlCore.cpp

+        const auto path = _settings->PixelShaderPath();
        auto lock = _terminal->LockForWriting();
        // Originally, this action could be used to enable the retro effects
        // even when they're set to `false` in the settings. If the user didn't
        // specify a custom pixel shader, manually enable the legacy retro
        // effect first. This will ensure that a toggle off->on will still work,
        // even if they currently have retro effect off.
-        if (_settings->PixelShaderPath().empty() && !_renderEngine->GetRetroTerminalEffect())
+        if (path.empty())
        {
-            // SetRetroTerminalEffect to true will enable the effect. In this
-            // case, the shader effect will already be disabled (because neither
-            // a pixel shader nor the retro effects were originally requested).
-            // So we _don't_ want to toggle it again below, because that would
-            // toggle it back off.
-            _renderEngine->SetRetroTerminalEffect(true);
+            _renderEngine->SetRetroTerminalEffect(!_renderEngine->GetRetroTerminalEffect());
        }
        else
        {
-            _renderEngine->ToggleShaderEffects();
+            _renderEngine->SetPixelShaderPath(_renderEngine->GetPixelShaderPath().empty() ? std::wstring_view{ path } : std::wstring_view{});


"Toggle terminal visual effects" action not working

oh thank you, this will make #9898 so easy

lhecker · 2022-09-01T23:20:28Z

src/cascadia/TerminalControl/TermControl.cpp


        co_await wil::resume_foreground(Dispatcher());

        if (auto control{ weakThis.get() })
        {
-            const auto chainHandle = reinterpret_cast<HANDLE>(control->_core.SwapChainHandle());
-            _AttachDxgiSwapChainToXaml(chainHandle);
+            _AttachDxgiSwapChainToXaml(handle.get());


Race condition in GetSwapChainHandle() due to it being called outside
of the console lock and with single-threaded Direct2D enabled

lhecker · 2022-09-01T23:20:40Z

src/cascadia/TerminalControl/TermControl.cpp

@@ -830,21 +835,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
        }
        _interactivity.Initialize();

-        _AttachDxgiSwapChainToXaml(reinterpret_cast<HANDLE>(_core.SwapChainHandle()));


Race condition in GetSwapChainHandle() due to it being called outside
of the console lock and with single-threaded Direct2D enabled

this change doesn't impact launching terminal with a bunch of splits at the same time, right? wt sp;sp;sp;sp;sp

(things that may cause device contention)

wtd "sp;sp;sp;sp;sp" worked fine in a debug build. Since I tested this with an added Sleep(3000) I can't conceive any situation myself were this would fail to work. 🤔

lhecker · 2022-09-01T23:20:57Z

src/renderer/atlas/AtlasEngine.cpp

+            D3D_FEATURE_LEVEL_9_3,
+            D3D_FEATURE_LEVEL_9_2,
+            D3D_FEATURE_LEVEL_9_1,


Added support for DirectX 9 hardware

lhecker · 2022-09-01T23:21:18Z

src/renderer/atlas/AtlasEngine.cpp

+        // Sometimes up to 2 buffers are locked, for instance during screen capture or when moving the window.
+        // 3 buffers seems to guarantee a stable framerate at display frequency at all times.
+        desc.BufferCount = 3;


2 swap chain buffers are less performant than 3

lhecker · 2022-09-01T23:21:35Z

src/renderer/atlas/AtlasEngine.cpp

+        // DXGI_SWAP_EFFECT_FLIP_DISCARD is a mode that was created at a time were display drivers
+        // lacked support for Multiplane Overlays (MPO) and were copying buffers was expensive.
+        // This allowed DWM to quickly draw overlays (like gamebars) on top of rendered content.
+        // With faster GPU memory in general and with support for MPO in particular this isn't
+        // really an advantage anymore. Instead DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL allows for a
+        // more "intelligent" composition and display updates to occur like Panel Self Refresh
+        // (PSR) which requires dirty rectangles (Present1 API) to work correctly.
+        desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL;


Flip-Discard and Present() is less energy efficient than
Flip-Sequential and Present1()

lhecker · 2022-09-01T23:21:49Z

src/renderer/atlas/AtlasEngine.r.cpp

+        THROW_IF_FAILED(_r.swapChain->Present1(1, 0, &params));
+    }
+    else
+    {


Flip-Discard and Present() is less energy efficient than
Flip-Sequential and Present1()

(Presumably, this is for that thread with the composition team)

lhecker · 2022-09-01T23:23:49Z

src/renderer/atlas/AtlasEngine.r.cpp

@@ -944,6 +924,8 @@ void AtlasEngine::_d2dDrawDirtyArea()

        // Draw background.
        {
+            _r.d2dRenderTarget->SetPrimitiveBlend(D2D1_PRIMITIVE_BLEND_COPY);


d2dMode failed to work with transparent backgrounds

(D2D defaults to source-over alpha blending so the _d2dFillRectangle to fill the background wouldn't work if the background color is transparent: It would just layer colors on top of the existing content instead of replacing the existing content.)

DHowett

5 comments, but i saw it say "Refresh" so i chose to submit ;P

DHowett · 2022-09-01T23:20:08Z

src/cascadia/TerminalControl/ControlCore.idl

@@ -76,8 +76,6 @@ namespace Microsoft.Terminal.Control
        IControlAppearance UnfocusedAppearance { get; };
        Boolean HasUnfocusedAppearance();

-        UInt64 SwapChainHandle { get; };


cc @zadjii-msft - this will impact xproc work

DHowett · 2022-09-01T23:20:39Z

src/cascadia/TerminalControl/TermControl.cpp

+        winrt::handle handle;
+        const auto processHandle = GetCurrentProcess();
+        const auto sourceHandle = reinterpret_cast<HANDLE>(winrt::unbox_value<uint64_t>(args));
+        THROW_IF_WIN32_BOOL_FALSE(DuplicateHandle(processHandle, sourceHandle, processHandle, handle.put(), 0, FALSE, DUPLICATE_SAME_ACCESS));


no issue doing this in-proc i presume? nice

DHowett · 2022-09-01T23:20:55Z

src/cascadia/TerminalControl/TermControl.cpp

+
+        // Create a copy of the swap chain HANDLE in args, since we don't own that parameter.
+        // By the time we return from the co_await below, it might be deleted already.
+        winrt::handle handle;


winrt has a handle wrapper type? didn't know!

DHowett · 2022-09-01T23:21:13Z

src/cascadia/TerminalControl/TermControl.cpp


        co_await wil::resume_foreground(Dispatcher());

        if (auto control{ weakThis.get() })
        {
-            const auto chainHandle = reinterpret_cast<HANDLE>(control->_core.SwapChainHandle());
-            _AttachDxgiSwapChainToXaml(chainHandle);
+            _AttachDxgiSwapChainToXaml(handle.get());


is it okay that we let the handle de-scope later and get destroyed?

because we just duplicated it... don't we need to keep the duplicate copy "alive" (if we're out of proc, for example?)

I mean yeah we can do that, but it'd be pretty foolish if ISwapChainPanelNative2 didn't duplicate the handle internally. That'd be just not be very robust...
...
.....
Ok I'm concerned now hahaha.

WSLUser · 2022-09-02T01:38:43Z

Added support for DirectX 9 hardware

You are now a hero. Some older versions of virtual software like VMWare Fusion/Workstation only supported up to DirectX9 and required the latest versions on their Pro editions to support DirectX 11 (not even 12). You've just enabled alot more users to use graphics properly on WT.

zadjii-msft

couple questions, but they're mostly for my own intellectual curiosity. I wouldn't treat this as a block

zadjii-msft · 2022-09-02T11:13:07Z

src/cascadia/TerminalControl/TermControl.cpp

+        // By the time we return from the co_await below, it might be deleted already.
+        winrt::handle handle;
+        const auto processHandle = GetCurrentProcess();
+        const auto sourceHandle = reinterpret_cast<HANDLE>(winrt::unbox_value<uint64_t>(args));


So, if the core is out of proc, how will this work? It can raise the event with it's own handle value, sure, but then the TermControl is gonna have to come back around and ask the Core to dupe the handle to the window process anyways.

(see https://github.com/microsoft/terminal/pull/12938/files#diff-c10bb023995e88dac6c1d786129284c454c2df739ea547ce462129dc86dc2697R837)

I think I can merge these together, just gonna ignore args when the control is attached to a content process.

Hmm... Well it's up to us to define whether HANDLE ownership is transferred for that event or not right? If you define it so that the HANDLE ownership is transferred than you can DuplicateHandle on the content-process side of things.

zadjii-msft · 2022-09-02T11:14:52Z

src/cascadia/TerminalControl/TermControl.cpp

        _core.SwapChainChanged({ get_weak(), &TermControl::RenderEngineSwapChainChanged });
-
-        // !! LOAD BEARING !!


don't love the removal of LOAD BEARING warning comments...

They cause problems though... EnablePainting() is not a thread synchronization primitive. Creating the resources on the main thread here and then calling EnablePainting() doesn't ensure that the render thread can actually access the most up to date representation of those classes.

src/renderer/atlas/AtlasEngine.r.cpp

zadjii-msft · 2022-09-02T11:21:06Z

src/renderer/atlas/AtlasEngine.r.cpp

+        THROW_IF_FAILED(_r.swapChain->Present1(1, 0, &params));
+    }
+    else
+    {


(Presumably, this is for that thread with the composition team)

zadjii-msft · 2022-09-02T11:24:33Z

src/renderer/atlas/AtlasEngine.r.cpp

-    if (const auto intersection = _r.previousDirtyRectInPx & dirtyRectInPx)
-    {
-        wil::com_ptr<ID3D11Resource> backBuffer;
-        wil::com_ptr<ID3D11Resource> frontBuffer;
-        THROW_IF_FAILED(_r.swapChain->GetBuffer(0, __uuidof(backBuffer), backBuffer.put_void()));
-        THROW_IF_FAILED(_r.swapChain->GetBuffer(1, __uuidof(frontBuffer), frontBuffer.put_void()));
-
-        D3D11_BOX intersectBox;
-        intersectBox.left = intersection.left;
-        intersectBox.top = intersection.top;
-        intersectBox.front = 0;
-        intersectBox.right = intersection.right;
-        intersectBox.bottom = intersection.bottom;
-        intersectBox.back = 1;
-        _r.deviceContext->CopySubresourceRegion1(backBuffer.get(), 0, intersection.left, intersection.top, 0, frontBuffer.get(), 0, &intersectBox, 0);


we don't need to do this manual copy anymore? Is this cause of the DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL change?

No I think this was never necessary and only fixed a visual glitch I had before I fixed other bugs in the D2D drawing code.

zadjii-msft · 2022-09-02T11:25:06Z

src/renderer/atlas/AtlasEngine.r.cpp

@@ -1072,6 +1056,7 @@ void AtlasEngine::_d2dFillRectangle(u16r rect, u32 color)
        .bottom = static_cast<float>(rect.bottom) * _r.cellSizeDIP.y,
    };
    const auto brush = _brushWithColor(color);
+    // TODO: This does blending, it should do copy


zadjii-msft · 2022-09-02T11:27:41Z

src/cascadia/TerminalControl/ControlCore.cpp

+        const auto path = _settings->PixelShaderPath();
        auto lock = _terminal->LockForWriting();
        // Originally, this action could be used to enable the retro effects
        // even when they're set to `false` in the settings. If the user didn't
        // specify a custom pixel shader, manually enable the legacy retro
        // effect first. This will ensure that a toggle off->on will still work,
        // even if they currently have retro effect off.
-        if (_settings->PixelShaderPath().empty() && !_renderEngine->GetRetroTerminalEffect())
+        if (path.empty())
        {
-            // SetRetroTerminalEffect to true will enable the effect. In this
-            // case, the shader effect will already be disabled (because neither
-            // a pixel shader nor the retro effects were originally requested).
-            // So we _don't_ want to toggle it again below, because that would
-            // toggle it back off.
-            _renderEngine->SetRetroTerminalEffect(true);
+            _renderEngine->SetRetroTerminalEffect(!_renderEngine->GetRetroTerminalEffect());
        }
        else
        {
-            _renderEngine->ToggleShaderEffects();
+            _renderEngine->SetPixelShaderPath(_renderEngine->GetPixelShaderPath().empty() ? std::wstring_view{ path } : std::wstring_view{});


oh thank you, this will make #9898 so easy

zadjii-msft

yea I can deal with the merge

DHowett · 2022-09-02T17:22:35Z

src/renderer/atlas/AtlasEngine.r.cpp

@@ -66,7 +66,7 @@ try
    const til::rect fullRect{ 0, 0, _r.cellCount.x, _r.cellCount.y };

    // A change in the selection or background color (etc.) forces a full redraw.
-    if (WI_IsFlagSet(_r.invalidations, RenderInvalidations::ConstBuffer))
+    if (WI_IsFlagSet(_r.invalidations, RenderInvalidations::ConstBuffer) || _r.customPixelShader)


huh. what happened without this?

It would only present the given dirty area, which for a custom shader might not include all pixels that changed.

ghost · 2022-09-02T22:09:35Z

Hello @DHowett!

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.

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 · 2022-09-13T17:07:17Z

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

Handy links:

ghost · 2022-09-13T17:07:20Z

🎉This issue was addressed in #13906, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

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)

lhecker force-pushed the dev/lhecker/atlas-engine-fixup branch 2 times, most recently from d4b1c4c to 66d9b44 Compare September 1, 2022 23:16

DHowett reviewed Sep 1, 2022

View reviewed changes

AtlasEngine: Fix various issues found in testing

98c80e2

lhecker force-pushed the dev/lhecker/atlas-engine-fixup branch from 66d9b44 to 98c80e2 Compare September 1, 2022 23:22

lhecker commented Sep 1, 2022

View reviewed changes

DHowett reviewed Sep 1, 2022

View reviewed changes

Merge branch 'main' into dev/lhecker/atlas-engine-fixup

de8ab54

zadjii-msft reviewed Sep 2, 2022

View reviewed changes

zadjii-msft added the Area-AtlasEngine label Sep 2, 2022

lhecker mentioned this pull request Sep 2, 2022

Adding a pixel shader to Atlas engine explodes #13899

Closed

zadjii-msft linked an issue Sep 2, 2022 that may be closed by this pull request

Adding a pixel shader to Atlas engine explodes #13899

Closed

Address feedback

700b229

lhecker added Issue-Bug It either shouldn't be doing this or needs an investigation. Severity-Crash Crashes are real bad news. Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels Sep 2, 2022

zadjii-msft approved these changes Sep 2, 2022

View reviewed changes

DHowett reviewed Sep 2, 2022

View reviewed changes

DHowett approved these changes Sep 2, 2022

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Sep 2, 2022

ghost merged commit 813286c into main Sep 2, 2022

ghost deleted the dev/lhecker/atlas-engine-fixup branch September 2, 2022 22:10

This pull request was closed.

		_core.SwapChainChanged({ get_weak(), &TermControl::RenderEngineSwapChainChanged });

		// !! LOAD BEARING !!

AtlasEngine: Fix various bugs found in testing #13906

AtlasEngine: Fix various bugs found in testing #13906

Conversation

lhecker commented Sep 1, 2022 • edited Loading

Validation Steps Performed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WSLUser commented Sep 2, 2022

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Sep 2, 2022

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 commented Sep 13, 2022

ghost commented Sep 13, 2022

lhecker commented Sep 1, 2022 •

edited

Loading

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.