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

Rewrite AtlasEngine to allow arbitrary overhangs #14959

Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Mar 6, 2023

This is practically a from scratch rewrite of AtlasEngine.

The initial approach used a very classic monospace text renderer, where
the viewport is subdivided into cells and each cell is assigned one
glyph texture, just like how real terminals used to work.
While we knew that it would have problems with overly large glyphs,
like those found in less often used languages, we didn't expect the
absolutely massive number of fonts that this approach would break.
For one, the assumption that monospace fonts are actually mostly
monospace has turned out to be a complete lie and we can't force users
to use better designed fonts. But more importantly, we can't just
design an entire Unicode fallback font collection from scratch where
every major glyph is monospace either. This is especially problematic
for vertical overhangs which are extremely difficult to handle in a
way that outperforms the much simpler alternative approach:
Just implementing a bog-standard, modern, quad-based text renderer.

Such an approach is both, less code and runs faster due to a less
complex CPU-side. The text shaping engine (in our case DirectWrite)
has to resolve text into glyph indices anyways, so using them directly
for text rendering allows reduces the effort of turning it back into
text ranges and hashing those. It's memory overhead is also reduced,
because we can now break up long ligatures into their individual glyphs.
Especially on AMD APUs I found this approach to run much faster.

A list of issues I think are either obsolete (and could be closed)
or resolved with this PR in combination with #14255:

Closes #6864
Closes #6974
Closes #8993
Closes #9940
Closes #10128
Closes #12537
Closes #13064
Closes #13527
Closes #13662
Closes #13700
Closes #13989
Closes #14022
Closes #14057
Closes #14094
Closes #14098
Closes #14117
Closes #14533
Closes #14877
Closes #9381

PR Checklist

  • Enabling software rendering enables D2D mode ✅
  • Both D2D and D3D:
    • Background appears correctly ✅✅
    • Text appears correctly
      • Cascadia Code Regular ✅✅
      • Cascadia Code Bold ✅✅
      • Cascadia Code Italic ✅✅
      • Cascadia Code block chars leave (almost) no gaps ✅✅
      • Terminus TTF at 13.5pt leaves no gaps between block chars ✅✅
      • "`u{e0b2}`u{e0b0}" in Fira Code Nerd Font forms a square ✅✅
    • Cursor appears correctly
      • Legacy small/medium/large ✅✅
      • Vertical bar ✅✅
      • Underscore ✅✅
      • Empty box ✅✅
      • Full box ✅✅
      • Double underscore ✅✅
    • Changing the cursor color works ✅✅
    • Selection appears correctly ✅✅
    • Scrolling in various manners always renders correctly ✅✅
    • Changing the text antialising mode works ✅✅
    • Semi-transparent backgrounds work ✅✅
    • Scroll-zooming the font size works ✅✅
    • Double-size characters work ✅✅
    • Resizing while text is printing works ✅✅
    • DWM +Heatmap_ShowDirtyRegions shows that only the cursor
      region is dirty when it's blinking ✅✅
  • D2D
    • Margins are filled with background color ❌
      They're filled with the neighboring's cell background color for
      convenience, as D2D doesn't support D3D11_TEXTURE_ADDRESS_BORDER
  • D3D
    • Margins are filled with background color ✅
    • Soft fonts work ✅
    • Custom shaders enable continous redraw if time constant is used ✅
    • Retro shader appears correctly ✅
    • Resizing while a custom shader is running works ✅

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-AtlasEngine Area-Fonts Related to the font Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Priority-2 A description (P2) Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Mar 6, 2023
@zadjii-msft
Copy link
Member

minor

@DHowett
Copy link
Member

DHowett commented Mar 9, 2023

Custom shaders enable continous redraw ❔

Only when they use the time component, right? ;)

@Dan-Albrecht
Copy link
Member

Noticed while testing this branch it brings back rendering of shaders over remote desktop which was lost in current Preview builds. Not sure which is the bug, but I want this way.

@lhecker
Copy link
Member Author

lhecker commented Mar 13, 2023

Noticed while testing this branch it brings back rendering of shaders over remote desktop which was lost in current Preview builds. Not sure which is the bug, but I want this way.

I got a chance to ask someone much more knowledgeable about this today and they suggested to only stop using Direct3D when we don't detect a phyiscal GPU. So I think that's what we'll do now.

But at least in my understanding the current preview builds should already work that way. I'm surprised they don't. If your host has a phyiscal GPU you should always get shader support... 🤔
Maybe the current approach of getting the IDXGIAdapter off of the device retroactively is causing issues?

wil::com_ptr<IDXGIAdapter1> dxgiAdapter;
THROW_IF_FAILED(_r.device.query<IDXGIObject>()->GetParent(__uuidof(dxgiAdapter), dxgiAdapter.put_void()));
THROW_IF_FAILED(dxgiAdapter->GetParent(__uuidof(_r.dxgiFactory), _r.dxgiFactory.put_void()));
DXGI_ADAPTER_DESC1 desc;
THROW_IF_FAILED(dxgiAdapter->GetDesc1(&desc));
_r.d2dMode = debugForceD2DMode || WI_IsAnyFlagSet(desc.Flags, DXGI_ADAPTER_FLAG_REMOTE | DXGI_ADAPTER_FLAG_SOFTWARE);

The new approach doesn't do this, so maybe that's what fixed it.

@Dan-Albrecht
Copy link
Member

Dan-Albrecht commented Mar 16, 2023

The new approach doesn't do this, so maybe that's what fixed it.

It's the DXGI_ADAPTER_FLAG_SOFTWARE check that appears to be doing it. The remote machine is a VM.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

@PankajBhojwani PankajBhojwani left a comment

Choose a reason for hiding this comment

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

My main request would be to add method descriptions to all the new functions in AtlasEngine.cpp, AtlastEngine.r.cpp, Backend.cpp, BackendD2D.cpp and BackendD3D.cpp. Not worth blocking over but it would be nice to have those before this merges. Otherwise I'm down to see what breaks in post

@DHowett

This comment was marked as off-topic.

@zadjii-msft

This comment was marked as off-topic.

@lhecker lhecker changed the title A minor AtlasEngine refactoring Rewrite AtlasEngine to allow arbitrary overhangs Apr 25, 2023
@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 25, 2023
@lhecker

This comment was marked as resolved.

@lhecker
Copy link
Member Author

lhecker commented Apr 26, 2023

@j4james I've implemented "reversed" text colors for colored cursors now, as per your suggestion:
image

(Thank you so much once again!)

It doesn't include an algorithm to ensure proper contrast at all times, but I hope to get to that soon. I'll probably go with ΔEOK, because it is computationally less expensive than ΔE2000 while providing results of similar quality. And it can be easily implemented with SIMD via DirectXMath.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Area-Fonts Related to the font Area-Performance Performance-related issue Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
6 participants