Introduce UnicodeAttribute for better Unicode support by skyline75489 · Pull Request #5407 · microsoft/terminal

skyline75489 · 2020-04-18T09:38:19Z

Summary of the Pull Request

This is the successor of #3578 which aims to improve Unicode support in the perspective of M:N cell rendering and ZWJ/ZWNJ.

References

#1472 #3546 #3578

PR Checklist

Closes The text layout code is expecting the quantity of glyphs is equal to the cells allocated for each line #3546
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Requires documentation to be updated
I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

I'm trying to add the possibility of Unicode support while leaving good old cmd.exe along. So I added a UnicodeAttribute class, which is specifically crafted for the all kinds of Unicode thingy.

The code is in its very early stage and the tests are left broken. Just wanna share the idea.

I managed to get many features including Combing/ZWJ/ZWNJ to work. Emoji with VSS-16 also works . But without VSS-16 those emoji will still be half-sized.

Hangul composition requires more interaction with clusters inside CustomTextLayout which I believe is out of the scope of this PR.

I'm not really sure what I did in the PR is the correct direction overall. Cluster category and adhesive logic (which @reli-msft mentioned) may also leads to the same result.

Validation Steps Performed

WIP

skyline75489 · 2020-04-18T09:44:26Z

This is what I got by far using the reproducing code in #3546 :

There's still something missing (the umbrella emoji is small) . Considering how many lines of code this PR actually adds, I feel it's quite worth the effort. I'll take a look and check that if it also fixes the other Unicode related issues.

By the way jialli is my alias, if you're wondering.

skyline75489 · 2020-04-18T09:46:16Z

src/renderer/base/renderer.cpp

+
+                    std::wstring* currentText = new std::wstring(it->Chars());
+
+                    while (moveForward)


This is of course a very naive implementation. But I think it's a good start.

skyline75489 · 2020-04-18T09:48:47Z

src/renderer/base/renderer.cpp

+                    bool inJoiner = nextCell->UnicodeAttr().IsJoiner();
+                    bool moveForward = nextCell && (nextCell->UnicodeAttr().IsBackwordAdhesive());
+
+                    std::wstring* currentText = new std::wstring(it->Chars());


This is just purely wrong. I couldn't find a way to concat two std::string_view. Neither can I access the underlying text buffer. I had to allocate a new string to get the content I need. I'd love to hear some opinions.

Perhaps use of the newly added to deps: https://github.com/fmtlib/fmt might help?

skyline75489 · 2020-04-18T09:50:51Z

src/buffer/out/UnicodeAttribute.hpp

+
+    void SetGlyph(std::wstring_view glyph) noexcept
+    {
+        const auto codepoint = CodepointWidthDetector::ExtractCodepoint(glyph);


I basically just pulled these from Unicode.org Website

github-actions · 2020-04-18T10:20:51Z

New misspellings found, please review:

FITZPATRICK
jialli
Unspacing

To accept these changes, run the following commands

remove_obsolete_words=$(mktemp)
echo '#!/usr/bin/perl -ni
my $re=join "|", qw('"
aaaaa
aaaaaa
aaaaaaaaaaaaaaaaaaa
ABCDEFGHIJKLMNOPQRSTQ
ABCDEFGHIJPQRST
ABCDEFGHIJPQRSTQ
APPLOGIC
backgrounder
BBBB
BBBBBBBBBBBBBBBBBBBBBBBBBB
CCCC
codeofconduct
controlkeystates
customframe
cvd
dataprotection
dataprotectionprovider
DDDD
directx
EEEE
FAILOUT
ffff
findlocalename
getboundingrectangles
getglyphs
getlocalename
getlocalenamelength
getstring
getstringlength
ggggg
gggggg
ggggggg
hackathon
hidpi
idwritefontcollection
idwritelocalizedstrings
idwritetextanalyzer
implementingtextandtextrange
inputdev
itextprovider
itextrangeprovider
iuiautomationtextrange
LLLLLLLL
nchittest
nf
nn
pgp
pointerpoint
polytextoutw
QQQQ
RRRRRRR
Spc
sysinfo
tounicodeex
uiauto
uiautomationclient
unicodechar
usr
varenum
vsconfig
winauto
windowsinsiderpreview
windowsterminal
wpt
XChars
xxxx
YChars
"');
next if /^($re)(?:$| .*)/;
print;' > $remove_obsolete_words
chmod +x $remove_obsolete_words
for file in .github/actions/spell-check/whitelist/alphabet.txt .github/actions/spell-check/whitelist/web.txt .github/actions/spell-check/whitelist/whitelist.txt; do $remove_obsolete_words $file; done
rm $remove_obsolete_words
(
echo "
applogic
Backgrounder
DIRECTX
FITZPATRICK
Hackathon
jialli
NCHITTEST
PGP
Unspacing
"
) | sort -u -f | perl -ne 'next unless /./; print' > new_whitelist.txt && mv new_whitelist.txt '.github/actions/spell-check/whitelist/bed3c47c4036ef62be34c34e37dcd50514a96c0b.txt'

✏️ Contributor please read this

If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
If you need to use a specific token in one place and it shouldn't generally be used, you can
add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

github-actions · 2020-04-18T14:21:14Z

New misspellings found, please review:

FITZPATRICK
jialli
JONGSEONG
JUNGSEONG
KIYEOK
SSANGNIEUN
Unspacing

To accept these changes, run the following commands

remove_obsolete_words=$(mktemp)
echo '#!/usr/bin/perl -ni
my $re=join "|", qw('"
aaaaa
aaaaaa
aaaaaaaaaaaaaaaaaaa
ABCDEFGHIJKLMNOPQRSTQ
ABCDEFGHIJPQRST
ABCDEFGHIJPQRSTQ
APPLOGIC
backgrounder
BBBB
BBBBBBBBBBBBBBBBBBBBBBBBBB
CCCC
codeofconduct
controlkeystates
customframe
cvd
dataprotection
dataprotectionprovider
DDDD
directx
EEEE
FAILOUT
ffff
findlocalename
getboundingrectangles
getglyphs
getlocalename
getlocalenamelength
getstring
getstringlength
ggggg
gggggg
ggggggg
hackathon
hidpi
idwritefontcollection
idwritelocalizedstrings
idwritetextanalyzer
implementingtextandtextrange
inputdev
itextprovider
itextrangeprovider
iuiautomationtextrange
LLLLLLLL
nchittest
nf
nn
pgp
pointerpoint
polytextoutw
QQQQ
RRRRRRR
Spc
sysinfo
tounicodeex
uiauto
uiautomationclient
unicodechar
usr
varenum
vsconfig
winauto
windowsinsiderpreview
windowsterminal
wpt
XChars
xxxx
YChars
"');
next if /^($re)(?:$| .*)/;
print;' > $remove_obsolete_words
chmod +x $remove_obsolete_words
for file in .github/actions/spell-check/whitelist/alphabet.txt .github/actions/spell-check/whitelist/web.txt .github/actions/spell-check/whitelist/whitelist.txt; do $remove_obsolete_words $file; done
rm $remove_obsolete_words
(
echo "
applogic
Backgrounder
DIRECTX
FITZPATRICK
Hackathon
jialli
JONGSEONG
JUNGSEONG
KIYEOK
NCHITTEST
PGP
SSANGNIEUN
Unspacing
"
) | sort -u -f | perl -ne 'next unless /./; print' > new_whitelist.txt && mv new_whitelist.txt '.github/actions/spell-check/whitelist/eb2000bb79caf4608098ec9a728535c75ba85ebc.txt'

✏️ Contributor please read this

If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
If you need to use a specific token in one place and it shouldn't generally be used, you can
add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

zadjii-msft · 2020-04-21T13:55:17Z

Hey for the record, we're all pretty heads down with getting the 1.0 release ready at the moment - it might be a bit before we can get to reviewing this. Thanks for your patience!

skyline75489 · 2021-01-07T08:14:43Z

This is a stale PR. Closing it for now.

skyline75489 added 3 commits April 17, 2020 21:58

Introduce Unicode Attribute

35a95b7

Actually use it for rendering

aca7f4d

Looking good enough

bed3c47

skyline75489 commented Apr 18, 2020

View reviewed changes

skyline75489 added 3 commits April 18, 2020 20:53

VSS 16

672120d

Revert submodule change

ea37acb

Hangul detection

eb2000b

skyline75489 changed the title ~~Introduce Unicode Attribute for better Unicode support~~ Introduce UnicodeAttribute for better Unicode support Apr 18, 2020

skyline75489 mentioned this pull request May 14, 2020

CodepointWidthDetector: reclassify U+25FB, U+25FC as Narrow #5914

Merged

skyline75489 mentioned this pull request Jul 17, 2020

Dig directly into text buffer with render base for perf #6924

Closed

6 tasks

microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:27

ghost added 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. labels Oct 22, 2020

skyline75489 closed this Jan 7, 2021

skyline75489 deleted the feature/unicode-attribute branch January 25, 2021 02:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce UnicodeAttribute for better Unicode support#5407

Introduce UnicodeAttribute for better Unicode support#5407
skyline75489 wants to merge 6 commits intomicrosoft:mainfrom
skyline75489:feature/unicode-attribute

skyline75489 commented Apr 18, 2020 •

edited

Loading

Uh oh!

skyline75489 commented Apr 18, 2020

Uh oh!

skyline75489 Apr 18, 2020

Uh oh!

skyline75489 Apr 18, 2020

Uh oh!

WSLUser Apr 21, 2020 •

edited

Loading

Uh oh!

skyline75489 Apr 18, 2020

Uh oh!

github-actions bot commented Apr 18, 2020

Uh oh!

github-actions bot commented Apr 18, 2020

Uh oh!

zadjii-msft commented Apr 21, 2020

Uh oh!

skyline75489 commented Jan 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		std::wstring* currentText = new std::wstring(it->Chars());

		while (moveForward)

Conversation

skyline75489 commented Apr 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

skyline75489 commented Apr 18, 2020

Uh oh!

skyline75489 Apr 18, 2020

Choose a reason for hiding this comment

Uh oh!

skyline75489 Apr 18, 2020

Choose a reason for hiding this comment

Uh oh!

WSLUser Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skyline75489 Apr 18, 2020

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 18, 2020

New misspellings found, please review:

⚠️ Reviewers

Uh oh!

github-actions bot commented Apr 18, 2020

New misspellings found, please review:

⚠️ Reviewers

Uh oh!

zadjii-msft commented Apr 21, 2020

Uh oh!

skyline75489 commented Jan 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skyline75489 commented Apr 18, 2020 •

edited

Loading

WSLUser Apr 21, 2020 •

edited

Loading