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.

Sign up for GitHub

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

Jump to bottom

Chinese characters are not rendering correctly if I type above them. #2191

Closed
klesh opened this issue Aug 1, 2019 · 3 comments · Fixed by #4668
Closed

Chinese characters are not rendering correctly if I type above them. #2191

klesh opened this issue Aug 1, 2019 · 3 comments · Fixed by #4668
Assignees
@miniksa
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.0

Comments

@klesh
Copy link

klesh commented Aug 1, 2019 •
edited
Loading

Environment

Windows build number:  Microsoft Windows [Version 10.0.18362.239]
Windows Terminal version (if applicable):   0.2.1831.0

Steps to reproduce

  1. Fire up WSL
  2. Run vim
  3. Enter some Chinese characters, they seems to be fine.
  4. Press Esc, press o to insert a new line above
  5. Start typeing some English letter...
  6. And the Chinese characters become smaller than normal size...
  7. If I type English letter beneath, It works OK



Expected behavior

Chinese characters should stay the same size no matter I type something above or beneath it

Actual behavior

Chinese characters turn into smaller size if I type English above the line.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 1, 2019
@zadjii-msft zadjii-msft added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Aug 1, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 1, 2019
@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Aug 1, 2019
@miniksa miniksa self-assigned this Aug 1, 2019
@miniksa miniksa removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 1, 2019
@miniksa
Copy link
Member

miniksa commented Aug 1, 2019

WELP. I'll get to this.

@etern etern mentioned this issue Nov 11, 2019
Closed
@skyline75489 skyline75489 mentioned this issue Nov 13, 2019
Closed
@skyline75489
Copy link
Collaborator

This is also happening in OpenConsole:

图片

But strange enough. The stock cmd.exe on Windows 10 is good:

图片

@etern etern mentioned this issue Nov 27, 2019
Closed
@mkitzan mkitzan mentioned this issue Dec 3, 2019
Closed
@zadjii-msft
Copy link
Member

Another repro gif courtesy of @zhixiangjoy in #3812

@zadjii-msft zadjii-msft added the Priority-3 A description (P3) label Jan 22, 2020
@cinnamon-msft cinnamon-msft added v1-Scrubbed Priority-1 A description (P1) and removed Priority-3 A description (P3) labels Jan 28, 2020
This was referenced Feb 20, 2020
Merged
Merged
@ghost ghost added the In-PR This issue has a related PR label Feb 20, 2020
ghost pushed a commit that referenced this issue Feb 20, 2020
@miniksa
Add DxEngine drawing ETW tracing for debugging and diagnostics purpos…
215df32 
…es (#4664)

## Summary of the Pull Request
Adds an ETW provider for tracing out operations inside the DirectX Renderer.

## References
This supports #2191 and #778 and other rendering issues.

## PR Checklist
* [x] I work here

## Detailed Description of the Pull Request / Additional comments
This declares and defines the provider with the a GUID appropriate for the namespace and adds an initial invalidation rectangle method for figuring out what is being drawn mostly to understand what could be differential.

## Validation Steps Performed
Implemented provider.
Opened real-time ETW tracing tool
Ran the Terminal and watched invalidation events appear live
@ghost ghost closed this as completed in #4668 Feb 21, 2020
ghost pushed a commit that referenced this issue Feb 21, 2020
@miniksa
Restrict DX run height adjustment to only relevant glyph AND Correct …
4420950 
…PTY rendering on trailing half of fullwidth glyphs (#4668)

## Summary of the Pull Request
- Height adjustment of a glyph is now restricted to itself in the DX
  renderer instead of applying to the entire run
- ConPTY compensates for drawing the right half of a fullwidth
  character. The entire render base has this behavior restored now as
  well.

## PR Checklist
* [x] Closes #2191
* [x] I work here
* [x] Tests added/passed
* [x] No doc
* [x] Am core contributor.

## Detailed Description of the Pull Request / Additional comments
Two issues:
1. On the DirectX renderer side, when confronted with shrinking a glyph,
   the correction code would apply the shrunken size to the entire run, not
   just the potentially individual glyph that needed to be reduced in size.
   Unfortunately while adjusting the horizontal X width can be done for
   each glyph in a run, the vertical Y height has to be adjusted for an
   entire run. So the solution here was to split the individual glyph
   needing shrinking out of the run into its own run so it can be shrunk.
2. On the ConPTY side, there was a long standing TODO that was never
   completed to deal with a request to draw only the right half of a
   two-column character. This meant that when encountering a request for
   the right half only, we would transmit the entire full character to be
   drawn, left and right halves, struck over the right half position. Now
   we correct the cursor back a position (if space) and draw it out so the
   right half is struck over where we believe the right half should be (and
   the left half is updated as well as a consequence, which should be OK.)

The reason this happens right now is because despite VIM only updating
two cells in the buffer, the differential drawing calculation in the
ConPTY is very simplistic and intersects only rectangles. This means
from the top left most character drawn down to the row/col cursor count
indicator in vim's modeline are redrawn with each character typed. This
catches the line below the edited line in the typing and refreshes it.
But incorrectly.

We need to address making ConPTY smarter about what it draws
incrementally as it's clearly way too chatty. But I plan to do that with
some of the structures I will be creating to solve #778.

## Validation Steps Performed
- Ran the scenario listed in #2191 in vim in the Terminal
- Added unit tests similar to examples given around glyph/text mapping
  in runs from Microsoft community page
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 21, 2020
This was referenced Nov 29, 2020
Merged
Closed
ghost pushed a commit that referenced this issue Dec 4, 2020
@j4james
Fix rendering of DBCS characters when partially off screen (#8438)
3ccd831 
When the renderer is called on to render part of a line starting halfway
through a DBCS character (as can occur in conhost when the viewport is
offset horizontally), it could result in the character not being
displayed, and/or with following the characters drawn in the wrong
place. This PR is an attempt to fix those problems. 

The original code for handling the trailing half of fullwidth glyphs was
introduced in PR #4668 to fix issue #2191.

When the content being rendered starts with the trailing half of a DBCS
character, the renderer tries to move the `screenPoint` back a position,
so it can instead render the full character, but instructing the render
engine to trim off the left half of it.

If the X position was already in column 0, though, it would instead move
forward one position, intending to skip that character. At best this
would mean the half character wouldn't be rendered, but since the
iterator wasn't incremented correctly, it actually just ended up
rendering the character in the wrong place.

The fix for this was simply to drop the check for the X position being
in column 0, and allow it go negative. The rendering engine would then
just start rendering the character partially off screen, and only the
second half of it would be displayed, which is exactly what is needed.

The second problem was that the code incrementing the iterator was using
the `columnCount` variable rather than the `it->Columns()` value for the
current position. When dealing with the trailing half of a DBCS
character, the `columnCount` is 2, but the `Columns()` value is 1. If
you use the former rather than the later, then the renderer may skip the
following character.

## Validation Steps Performed
I've developed a more easily reproducible version of the test case
described in #8390, and confirmed that the problem no longer occurs when
this PR is applied.

I've also manually confirmed that the problem described in #2191 that
was fixed by PR #4668 is still working correctly now.

Closes #8390
@zadjii-msft zadjii-msft mentioned this issue Jul 6, 2021
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@miniksa miniksa

Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.0
5 participants
@klesh @skyline75489 @miniksa @zadjii-msft @cinnamon-msft