Support rendering of underline style and color #16097

tusharsnx · 2023-10-03T16:18:36Z

Add support for underline style and color in the renderer

Important

The PR adds underline style and color feature to AtlasEngine (WT) and GDIRenderer (Conhost) only.

After the underline style and color feature addition to Conpty, this PR takes it further and add support for rendering them to the screen!

Out of five underline styles, we already supported rendering for 3 of those types (Singly, Doubly, Dotted) in some form in our (Atlas) renderer. The PR adds the remaining types, namely, Dashed and Curly underlines support to the renderer.

All renderer engines now receive both gridline and underline color, and the latter is used for drawing the underlines. When no underline color is set, we use the foreground color.
Curly underline is rendered using sin() within the pixel shader.
To draw underlines for DECDWL and DECDHL, we send the line rendition scale within QuadInstance's texcoord attribute.
In GDI renderer, dashed and dotted underline is drawn using HPEN with a desired style. Curly line is a cubic Bezier that draws one wave per cell.

PR Checklist

✅ Set the underline color to underlines only, without affecting the gridline color.
❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced by AtlasEngine)
✅ Port underline coloring and style to GDI renderer (Conhost).
✅ Wide/Tall CurlyUnderline variant for DECDWL/DECDHL.

Closes #7228

DHowett · 2023-10-03T17:22:48Z

Port to DX renderer

I would be OK if you chose not to do this since we're deprecating DX... but maybe porting it to the AtlasEngine BackendD2D would be good! 😄

@lhecker can probably explain why/what.

lhecker · 2023-10-03T18:55:45Z

It's amazing how you make changes to these complex parts of our code base. I'll have to review it at some future time though because I'm currently working on another complex issue.

I would be OK if you chose not to do this since we're deprecating DX... but maybe porting it to the AtlasEngine BackendD2D would be good! 😄

Yeah the DX engine will be replaced in the near term. BackendD2D doesn't necessarily need this IMO, because only very few people use it (basically only people using RDP on headless servers without GPU). But if you're able to implement it there, then that's even better of course.

If you want to, you can simplify the BackendD3D implementation quite a bit by using sin/cos directly in the shader to compute a wavy line and then calculate the gaussian distance of the current pixel to the line and use that as the alpha value. I can help you write the necessary HLSL if you'd like.

I'm not 100% sure how it would look like. The solutions others have come up with on https://www.shadertoy.com/ have various flaws. There's https://math.stackexchange.com/questions/514820/distance-between-point-and-sine-wave which results in something like https://www.shadertoy.com/view/Md3yRH

lhecker · 2023-10-03T22:53:58Z

If you add this to the end of SHADING_TYPE_TEXT_BACKGROUND:

float centerY = 300.0f;
float displayScale = 1.5f; // I use a 150% display scale monitor
float scaleX = 1.5f / displayScale;
float scaleY = displayScale;
float width = displayScale * 0.25f;
float4 col = float4(0.8, 0.5, 0.2, 1);

float s = sin(data.position.x * scaleX);
float d = abs(centerY - data.position.y + s * scaleY);
float a = 1 - saturate(d - width);
color = alphaBlendPremultiplied(color, col * a);

you get a sine wave.

Even though the distance to the sine curve (d) is only a delta of the y axis, the anti-aliasing seems to be sufficient. It's not perfect and the Direct2D sprite that you generate looks a lot better, but... it's another option we can consider using. (Feel free to choose whatever approach you like best.)

FYI (and you might know this already) you can set this:

#define ATLAS_DEBUG_CONTINUOUS_REDRAW 1

and use OpenConsole with UseDx set to 2. AtlasEngine will hot-reload the shader whenever you change it in your editor.

tusharsnx · 2023-10-09T07:01:06Z

Even though the distance to the sine curve (d) is only a delta of the y axis, the anti-aliasing seems to be sufficient. It's not perfect and the Direct2D sprite that you generate looks a lot better, but... it's another option we can consider using. (Feel free to choose whatever approach you like best.)

The snippet you shared is arguably better than the current approach in this PR. To my eyes, they have exactly the same anti-aliasing effect as seen on the curly wave generated using D2D, and I'm amazed that this is even possible 😲

Note
The white curve is generated by D2D and the colored one from inside the shader code.

I think going with your approach will simplify things a lot. There is an added benefit that, wide and tall variants can be generated just by tweaking the scaleX and scaleY constants, which is super easy. WDYT?

Ohh, and thanks for the tip regarding ATLAS_DEBUG_CONTINUOUS_REDRAW, and useDX. Didn't know that before 👍

src/types/UiaTextRangeBase.cpp

lhecker · 2023-10-09T10:29:51Z

The snippet you shared is arguably better than the current approach in this PR. To my eyes, they have exactly the same anti-aliasing effect as seen on the curly wave generated using D2D, and I'm amazed that this is even possible 😲

I didn't expect that. I think at lower line widths my HLSL code might not work that well though, because it only has vertical anti-aliasing (and no horizontal). But I do think it works sufficiently well, so if you don't mind, I think we should go with the HLSL approach, because it simplifies the code.

We should probably scale the wave based on the font size instead of display scale so that zooming the font size works as expected, so maybe we can do something like:

float scaleX = 3.0f / underlineWidth;
float scaleY = 1.5f / underlineWidth;
float width = scaleY * 0.25f;

Since this scaleY code is then very specific to BackendD3D, we should probably also calculate the curlyUnderline height in BackendD3D::_updateFontDependents instead. There, you can store the height in a member variable. (I'm planning to slim down the FontSettings struct and use it for all renderers, including the old GDI renderer for conhost, so this would help me a bit.) BTW I think we also need to add 2px to our height estimate to account for the anti-aliasing at the top and bottom.

Let me know if you have any questions whatsoever. Don't worry about the details - I can clean up the code at a later time, if needed. 🙂

lhecker · 2023-10-09T10:39:11Z

While editing my comment I realized that getting the centerY might be problematic. I think you'll have to add a new member to cbuffer ConstBuffer and PSConstBuffer respectively and update BackendD3D::_recreateConstBuffer, so that we can get the center-offset-y value in the shader.

tusharsnx · 2023-10-09T13:24:54Z

Since this scaleY code is then very specific to BackendD3D, we should probably also calculate the curlyUnderline height in BackendD3D::_updateFontDependents instead. There, you can store the height in a member variable.

I think we can club some of the font settings specific to BackendD3D into their own struct BackendD3D::FontRenderSettings?

class BackendD3D
{
    private:
        struct FontRenderSettings
        {
            ShadingType textShadingType = ShadingType::Default,
            til::CoordType ligatureOverhangTriggerLeft = 0,
            til::CoordType ligatureOverhangTriggerRight = 0,
            f32 curlyUnderlineWaviness = 0.28f,
            FontDecorationPosition curlyUnderline;
            float cleartypeEnhancedContrast = 0,
            float grayscaleEnhancedContrast = 0,
            
            // Possibly this too
            bool fontChangedResetGlyphAtlas = false
        };
}

It's okay if we move things in a separate PR, but should I at least create BackendD3D:FontRenderSettings and put curly underline related settings into it?

lhecker · 2023-10-09T16:24:35Z

I feel somewhat impartial about that. I personally don't mind the many members at the moment, but I think that grouping them by their use would be useful. So, if you'd like to group them, please feel free to do so. 🙂

src/renderer/atlas/BackendD3D.cpp

src/renderer/atlas/shader_ps.hlsl

src/renderer/atlas/BackendD3D.h

DHowett · 2023-10-12T20:49:14Z

I just grabbed your build output and played with it for a bit. I love it. It's so cool!

…erline color

DHowett · 2023-10-24T19:16:08Z

Here's GDI with all the known styles. This is so cool

src/renderer/gdi/paint.cpp

tusharsnx · 2023-10-28T17:43:27Z

Ready for review! 😀

lhecker

LGTM. Just a couple nits.

src/renderer/atlas/BackendD3D.cpp

src/renderer/atlas/shader_ps.hlsl

src/renderer/atlas/BackendD3D.cpp

src/renderer/atlas/shader_ps.hlsl

src/renderer/atlas/BackendD3D.cpp

DHowett

Absolutely brilliant. Thank you. I'm so proud to bring this into conhost for Windows, too 😁

@lhecker, you had one outstanding comment about changing up some math, but you signed off so I'm gonna call it fine? Explain further if there's changes to make!

DHowett · 2023-11-10T00:38:57Z

src/renderer/atlas/BackendD3D.cpp

+    const auto maxCurlyLinePeakHeight = maxCurlyLinePeakHeightEm * font.fontSize;
+    auto curlyLinePeakHeight = std::min(cellBottomGap, maxCurlyLinePeakHeight);
+
+    // When it's too small to be curly, make it straight.


Hi guys,

Something isn't right here: I can't see curly underlines until I change font size to at least 35 in conhost or 20 in WT, which is huge even on a full HD screen:

Conhost (20) | WT (12.8107)

It's definitely not too small to be curly yet:
wezterm (8.0)

Technically it's not too small if the wave height is at least 2 pixels:

This seems to be the case for me too when using a 1440p monitor.

Curly line is only drawn when there's enough space for it ¹ - space between (singly) underline and Cell bottom. If we don't do this, the curly line will always be rendered, but it would be clipped, which wouldn't look nice (or correct) 😅

For example, when using Courier New (font):

When WT draws Curlyline:

When it doesn't:

I'd love to help you guys 😀, but can you file an issue for this? We can discuss in detail what might be causing this issue at those font sizes. Also, make sure to add font name (and other details that might be helpful) for repro.

Footnotes

To get a rough idea of how much space is available between cell bottom and underlines is to select the cell where the underline is drawn. Cell area is highlighted by the selection, and you can see how much space is available between cell bottom and underline baseline. This works on Conhost too (even better since you can scroll to increase fontsize, and the selection is preserved). ↩

can you file an issue for this?

Sure, #16288

src/types/UiaTextRangeBase.cpp

Add support for underline style and color in the renderer > [!IMPORTANT] > The PR adds underline style and color feature to AtlasEngine (WT) and GDIRenderer (Conhost) only. After the underline style and color feature addition to Conpty, this PR takes it further and add support for rendering them to the screen! Out of five underline styles, we already supported rendering for 3 of those types (Singly, Doubly, Dotted) in some form in our (Atlas) renderer. The PR adds the remaining types, namely, Dashed and Curly underlines support to the renderer. - All renderer engines now receive both gridline and underline color, and the latter is used for drawing the underlines. **When no underline color is set, we use the foreground color.** - Curly underline is rendered using `sin()` within the pixel shader. - To draw underlines for DECDWL and DECDHL, we send the line rendition scale within `QuadInstance`'s texcoord attribute. - In GDI renderer, dashed and dotted underline is drawn using `HPEN` with a desired style. Curly line is a cubic Bezier that draws one wave per cell. ## PR Checklist - ✅ Set the underline color to underlines only, without affecting the gridline color. - ❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced by **AtlasEngine**) - ✅ Port underline coloring and style to GDI renderer (Conhost). - ✅ Wide/Tall `CurlyUnderline` variant for `DECDWL`/`DECDHL`. Closes #7228 (cherry picked from commit e268c1c) Service-Card-Id: 91349180 Service-Version: 1.19

initial commit for underline style and color

d2bea4c

tusharsnx marked this pull request as draft October 3, 2023 16:18

lostl1ght mentioned this pull request Oct 5, 2023

Colored undercurl and underline on WSL2 / updated ConPTY wez/wezterm#4400

Open

add new underline styles in UIA

d341402

tusharsnx commented Oct 9, 2023

View reviewed changes

src/types/UiaTextRangeBase.cpp Show resolved Hide resolved

src/types/UiaTextRangeBase.cpp Show resolved Hide resolved

draw curly line from the shader

a042401

tusharsnx commented Oct 12, 2023

View reviewed changes

src/renderer/atlas/BackendD3D.cpp Outdated Show resolved Hide resolved

src/renderer/atlas/shader_ps.hlsl Outdated Show resolved Hide resolved

src/renderer/atlas/BackendD3D.h Outdated Show resolved Hide resolved

revert D2D antialias mode

e16055c

tusharsnx added 7 commits October 14, 2023 22:12

fix gridlines being drawn with underline color

1df5894

add dotted underline to BackendD2D and Dx renderers

0bdeb1a

fix ScrollTest's MockScrollRenderEngine wasn't updated to receive und…

c179041

…erline color

make sine wave start with a crest

8684de8

fix strikethrough being drawn with underline color

dc986c1

add support for underline styles in gdi renderer

bea07a5

add spellings

4f13a3c

fix signed/unsigned mismatch

1f3e535

tusharsnx added 2 commits October 27, 2023 21:05

GDIRenderer: use cellBottomGap as the peak height for curly line

63ae6ce

initialize _curlyLineDrawPeakHeight

b5574d0

tusharsnx commented Oct 27, 2023

View reviewed changes

src/renderer/gdi/paint.cpp Outdated Show resolved Hide resolved

tusharsnx added 2 commits October 28, 2023 21:26

send line rendition scale using texcoord

b8d3caa

underlines are mutually exclusive

a2c4a36

tusharsnx marked this pull request as ready for review October 28, 2023 17:42

revert underline thickness scaling

6e4c65a

lhecker reviewed Nov 1, 2023

View reviewed changes

tusharsnx added 4 commits November 2, 2023 19:43

move curlyline peak height constants into the function

b0be8b6

fix curlyline equation that was making a negative peak at the start

1ba1782

BackendD2D: use else if

4f7ba10

shrink QuadInstance::ShadingType and add renditionScale member

c7a8a55

lhecker approved these changes Nov 3, 2023

View reviewed changes

replace SelectObject with wil::SelectObject

3923fde

lhecker mentioned this pull request Nov 7, 2023

Remove DxEngine #16278

Merged

DHowett approved these changes Nov 10, 2023

View reviewed changes

DHowett merged commit e268c1c into microsoft:main Nov 10, 2023
14 checks passed

alabuzhev mentioned this pull request Nov 10, 2023

Curly underline is rendered as single #16288

Closed

tusharsnx deleted the renderer-underline-style-color branch November 24, 2023 12:08

ajmandourah mentioned this pull request Dec 26, 2023

Implementing undercurl support in windows terminal neovim/neovim#26744

Closed

ajmandourah mentioned this pull request Feb 25, 2024

feat(tui): colorized underline in Windows Terminal neovim/neovim#27629

Closed

tusharsnx mentioned this pull request Feb 28, 2024

Enhance text editing experience on WindowsTerminal #15555

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support rendering of underline style and color #16097

Support rendering of underline style and color #16097

tusharsnx commented Oct 3, 2023 •

edited

Loading

DHowett commented Oct 3, 2023 •

edited

Loading

lhecker commented Oct 3, 2023

lhecker commented Oct 3, 2023 •

edited

Loading

tusharsnx commented Oct 9, 2023 •

edited

Loading

lhecker commented Oct 9, 2023 •

edited

Loading

lhecker commented Oct 9, 2023

tusharsnx commented Oct 9, 2023 •

edited

Loading

lhecker commented Oct 9, 2023

DHowett commented Oct 12, 2023

DHowett commented Oct 24, 2023

tusharsnx commented Oct 28, 2023

lhecker left a comment

DHowett left a comment

DHowett Nov 10, 2023

alabuzhev Nov 10, 2023 •

edited

Loading

dstanberry Nov 10, 2023

tusharsnx Nov 10, 2023 •

edited

Loading

alabuzhev Nov 10, 2023

Support rendering of underline style and color #16097

Support rendering of underline style and color #16097

Conversation

tusharsnx commented Oct 3, 2023 • edited Loading

PR Checklist

DHowett commented Oct 3, 2023 • edited Loading

lhecker commented Oct 3, 2023

lhecker commented Oct 3, 2023 • edited Loading

tusharsnx commented Oct 9, 2023 • edited Loading

lhecker commented Oct 9, 2023 • edited Loading

lhecker commented Oct 9, 2023

tusharsnx commented Oct 9, 2023 • edited Loading

lhecker commented Oct 9, 2023

DHowett commented Oct 12, 2023

DHowett commented Oct 24, 2023

tusharsnx commented Oct 28, 2023

lhecker left a comment

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

DHowett Nov 10, 2023

Choose a reason for hiding this comment

alabuzhev Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

dstanberry Nov 10, 2023

Choose a reason for hiding this comment

tusharsnx Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Footnotes

alabuzhev Nov 10, 2023

Choose a reason for hiding this comment

tusharsnx commented Oct 3, 2023 •

edited

Loading

DHowett commented Oct 3, 2023 •

edited

Loading

lhecker commented Oct 3, 2023 •

edited

Loading

tusharsnx commented Oct 9, 2023 •

edited

Loading

lhecker commented Oct 9, 2023 •

edited

Loading

tusharsnx commented Oct 9, 2023 •

edited

Loading

alabuzhev Nov 10, 2023 •

edited

Loading

tusharsnx Nov 10, 2023 •

edited

Loading