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: Improve appearance of curly underlines #17501

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 1, 2024

We'd previously subtract one underline-height from the curly line
offset, even though we already had subtracted its complete height.

Additionally, the pixel shader received some fine tuning:

  • Shrink the stroke width so that the anti-aliasing can be seen
    all the way up to the horizontal edges of the bounding box.
  • Add a phase shift to break apart the symmetry of the curve.

Closes #17482

Co-authored-by: Carlos Zamora [email protected]

@lhecker
Copy link
Member Author

lhecker commented Jul 1, 2024

All of these are with the Hack font:

Before, 10pt, 100% scale, 800% zoom:
image

After, 10pt, 100% scale, 800% zoom:
image

Before, 10pt, 150% scale, 800% zoom:
image

After, 10pt, 150% scale, 800% zoom:
image

Before, 12pt, 300% scale, 400% zoom:
image

After, 12pt, 300% scale, 400% zoom:
image

Regarding the last image: The line is actually supposed to be 1px wide (according to the font's underline width). This probably best demonstrates how the previous pixel shader drew the line too "fat".

color = all(cell < backgroundCellCount) ? background[cell] : backgroundColor;
weights = float4(1, 1, 1, 1);
break;
}
case SHADING_TYPE_TEXT_GRAYSCALE:
{
// These are independent of the glyph texture and could be moved to the vertex shader or CPU side of things.
const float4 foreground = premultiplyColor(data.color);
Copy link
Member

Choose a reason for hiding this comment

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

??? did we learn something new about const here?

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.

did we make a similar mistake with BackendD2D?

@DHowett DHowett added this pull request to the merge queue Jul 2, 2024
Merged via the queue into main with commit ad3797a Jul 2, 2024
20 checks passed
@DHowett DHowett deleted the dev/lhecker/17482-curly branch July 2, 2024 16:29
github-merge-queue bot pushed a commit that referenced this pull request Jul 2, 2024
DHowett pushed a commit that referenced this pull request Aug 13, 2024
We'd previously subtract one underline-height from the curly line
offset, even though we already had subtracted its complete height.

Additionally, the pixel shader received some fine tuning:
* Shrink the stroke width so that the anti-aliasing can be seen
  all the way up to the horizontal edges of the bounding box.
* Add a phase shift to break apart the symmetry of the curve.

Closes #17482

Co-authored-by: Carlos Zamora <[email protected]>
(cherry picked from commit ad3797a)
Service-Card-Id: 92942049
Service-Version: 1.21
DHowett pushed a commit that referenced this pull request Aug 13, 2024
See #17501.

(cherry picked from commit 7f2249c)
Service-Card-Id: 92942980
Service-Version: 1.21
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AtlasEngine Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
Status: To Cherry Pick
Development

Successfully merging this pull request may close these issues.

Configuration for undercurl vertical offset
3 participants