Do what the comment actually says and only insert 0s if we were given more text than the column value we were also given. (#4781)

## Summary of the Pull Request
Adjusts column padding code in `CustomTextLayout` to only pad out for surrogate pairs, not anything that reports two columns.

## References
- See also #4747

## PR Checklist
* [x] Closes #4780
* [x] I work here.
* [x] Manual tests.
* [x] No doc, this fixes code to match comment. Oops.
* [x] Am core contributor. Also discussed with @leonMSFT. 

## 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 `CustomTextLayout` construction 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
- Opened it up
- Put in the house emoji like #4747 (U+1f3e0)
- Put in some characters with simplified Chinese IME (fixed now)
- Put in the utf83.txt sample text used in #4747
This commit is contained in:
Michael Niksa 2020-03-02 16:29:38 -08:00 committed by GitHub
parent a3eb427d8b
commit 142a9e1f9d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -52,7 +52,7 @@ CustomTextLayout::CustomTextLayout(gsl::not_null<IDWriteFactory1*> const factory
// If there is more than one text character here, push 0s for the rest of the columns
// of the text run.
_textClusterColumns.resize(_textClusterColumns.size() + base::ClampSub(cols, 1u), gsl::narrow_cast<UINT16>(0u));
_textClusterColumns.resize(_textClusterColumns.size() + base::ClampSub(text.size(), 1u), gsl::narrow_cast<UINT16>(0u));
_text += text;
}