Stop padding text columns for fullwidth characters#4781
Conversation
… more text than the column value we were also given.
|
@msftbot merge this in 1 minutes |
|
Hello @DHowett-MSFT! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
|
Hello @miniksa! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 54 seconds. No worries though, I will be back when the time is right! 😉 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 (
|
Summary of the Pull Request
Adjusts column padding code in
CustomTextLayoutto only pad out for surrogate pairs, not anything that reports two columns.References
PR Checklist
Detailed Description of the Pull Request / Additional comments
For surrogate pairs like high Unicode emoji, we receive two wchar_ts but only one column count (which is usually 2 because emoji are usually inscribed in the full-width squares.) To compensate for this, I added in a little padding function at the top of the
CustomTextLayoutconstruction that adds a column of 0 aligned with the second half of a surrogate pair so the text-to-glyph mapping lines up correctly.Unfortunately, I made a mistake while either responding to PR feedback in #4747 or in the first place and I made it pad out extra 0 columns based on the FIRST column count, not based on whether or not there is a trailing surrogate pair. The correct thing to do is to pad it out based on the LENGTH of text associated with the given column count. This means that full width characters which can be represented in one wchar_t, like those coming from the IME in most cases (U+5C41 for example) will have a column count of 2. This is perfectly correct for mapping text-to-glyphs and doesn't need a 0 added after it. A house emoji (U+1F3E0) comes in as two wchar_ts (0xD83C 0xDFE0) with the column count of 2. To ensure that the arrays are aligned, the 2 matches up with the 0xD83C but the 0xDFE0 needs a 0 on it so it will be skipped over. (Don't worry, because it's a surrogate, it's naturally consumed correctly by the glyph mapper.)
The effect was that every OTHER character inserted by the IME was scaled to 0 size (as an advance of 0 was expected for 0 columns).
The fix restores it so those characters don't have an associated count and aren't scaled.
Validation Steps Performed