Fixed an issue where combined emoji was not displayed #20

Jocs · 2024-04-26T17:19:01Z

fixed an issue where combined emoji was not displayed. please refer to The combination emoji(👩‍❤️‍👩) is not displayed correctly #19

close #19

Jocs · 2024-04-26T17:21:15Z

src/layout-text.ts

@@ -1759,6 +1759,11 @@ export class Paragraph {
        const word = this.string.slice(wordStart, wordStart + wordLen);
        let wordGlyphs = wordCacheGet(font, word);

+        // Why the buffer is empty?


@chearon On the first text shaping, the buffer is released, but I don't know where it was released, so regenerating a buffer, but it should not be a good solution?

I'm not sure how the buffer could be empty here. It should only get destroyed after layout:

dropflow/src/layout-flow.ts

Lines 1497 to 1504 in 75b1b57

this.paragraph.destroy();

this.paragraph = createParagraph(this);

this.paragraph.shape();

}

}

postprocess() {

super.postprocess();

Could it be that harfbuzz clears the buffer in some cases?

Let's fix this place like this？ and then find out the reason why the buffer was released in the future, and then modify it later.

chearon

Btw, for these kinds of problems, you can add a x-dropflow-log attribute to the element surrounding the text:

<html x-dropflow-log style="background-color: #067; margin: 1em; color: #afe">
hhj👩‍❤️‍👩jz 
</html>

and you'll get debug info:

Log from Safari

[Log] Preprocess 648 (layout-text.js, line 1649)
==============
Full text: " hhj👩‍❤️‍👩jz "
  Item 0..4:
  emoji=false level=0 script=Latin size=16 variant=normal
  cascade=latin-400-normal, emoji-400-normal
    Shaping " hhj" with font https://cdn.jsdelivr.net/fontsource/fonts/noto-sans@latest/latin-400-normal.ttf
    Shaper returned: 3 75 75 77 
    ==> Glyphs OK: 3 75 75 77 
  Item 4..12:
  emoji=true level=0 script=Latin size=16 variant=normal
  cascade=latin-400-normal, emoji-400-normal
    Shaping "👩‍❤️‍👩" with font https://cdn.jsdelivr.net/fontsource/fonts/noto-sans@latest/latin-400-normal.ttf
    Shaper returned: 0 3 0 3 3 0 
    ==> Must reshape "👩‍"
    ==> Must reshape "❤️‍"
    ==> Must reshape "👩"
    Shaping "👩" with font https://cdn.jsdelivr.net/fontsource/fonts/noto-color-emoji@latest/emoji-400-normal.ttf
    Shaper returned: 2218 
    ==> Glyphs OK: 2218 
    Shaping "❤️‍" with font https://cdn.jsdelivr.net/fontsource/fonts/noto-color-emoji@latest/emoji-400-normal.ttf
    Shaper returned: 2797 1 
    ==> Glyphs OK: 2797 1 
    Shaping "👩‍" with font https://cdn.jsdelivr.net/fontsource/fonts/noto-color-emoji@latest/emoji-400-normal.ttf
    Shaper returned: 2218 1 
    ==> Glyphs OK: 2218 1 
  Item 12..15:
  emoji=false level=0 script=Latin size=16 variant=normal
  cascade=latin-400-normal, emoji-400-normal
    Shaping "jz " with font https://cdn.jsdelivr.net/fontsource/fonts/noto-sans@latest/latin-400-normal.ttf
    Shaper returned: 77 93 3 
    ==> Glyphs OK: 77 93 3 
[Log] Paragraph 648 (layout mode normal): (layout-text.js, line 2045)
[Log]   @0 F:latin-400-normal (layout-text.js, line 650)
[Log]      T:" hhj" (layout-text.js, line 651)
[Log]      G:3 75 75 77  (layout-text.js, line 652)
[Log]   @4 F:emoji-400-normal (layout-text.js, line 650)
[Log]      T:"👩‍" (layout-text.js, line 651)
[Log]      G:2218 1  (layout-text.js, line 652)
[Log]   @7 F:emoji-400-normal (layout-text.js, line 650)
[Log]      T:"❤️‍" (layout-text.js, line 651)
[Log]      G:2797 1  (layout-text.js, line 652)
[Log]   @10 F:emoji-400-normal (layout-text.js, line 650)
[Log]       T:"👩" (layout-text.js, line 651)
[Log]       G:2218  (layout-text.js, line 652)
[Log]   @12 F:latin-400-normal (layout-text.js, line 650)
[Log]       T:"jz " (layout-text.js, line 651)
[Log]       G:77 93 3  (layout-text.js, line 652)
[Log] Line 0 (W:95.32 A:17.10 D:4.69 B:0.00): â€œ hhjâ€� â€œ👩‍â€� â€œ❤️‍â€� â€œ👩â€� â€œjz â€�  (layout-text.js, line 2056)

I believe emoji segments are supposed to be shaped as a whole, which is why I added attrs.isEmoji in the first place. So the shaping change you made is close. I might need to think a little bit more about it.

chearon · 2024-04-28T19:31:08Z

src/layout-text.ts

          while (!hbClusterState.done) {
            nextGlyph(hbClusterState);

+            // If the current font does not support emojis, should fallback to the next match font.
+            if (attrs.isEmoji && hbClusterState.needsReshape) {


Need to at least check isLastMatch here or it will infinite loop. One of the tests does that currently.

I add a isLastMatch check, finished with a tofu glyph when this is the last font match.

It will render like this with tofu glyph on macOS.

chearon · 2024-04-28T19:36:10Z

src/layout-text.ts

@@ -1759,6 +1759,11 @@ export class Paragraph {
        const word = this.string.slice(wordStart, wordStart + wordLen);
        let wordGlyphs = wordCacheGet(font, word);

+        // Why the buffer is empty?


I'm not sure how the buffer could be empty here. It should only get destroyed after layout:

dropflow/src/layout-flow.ts

Lines 1497 to 1504 in 75b1b57

this.paragraph.destroy();

this.paragraph = createParagraph(this);

this.paragraph.shape();

}

}

postprocess() {

super.postprocess();

chearon · 2024-05-03T11:29:45Z

@Jocs I dug deeper and realized that the problem is with the grapheme iterator used for reshaping boundaries. The grapheme iterator is based on Unicode 8 and I'd like to update it all to the latest, the rules in Unicode 15, which doesn't break ZWJs, skin tone modifiers, etc.

If you want to just add the ZWJ check there for now, I'd merge that. Or the whole upgrade to Unicode 15. If not I'll do it when I'm back from vacation. Thanks!

Jocs · 2024-05-06T02:47:17Z

If you want to just add the ZWJ check there for now, I'd merge that. Or the whole upgrade to Unicode 15. If not I'll do it when I'm back from vacation. Thanks!

Can we merge it first? When you come back from vacation, you can modify it on top of this.

Fixed an issue where combo emoji was not displayed

75feb3e

Jocs commented Apr 26, 2024

View reviewed changes

Add some comments

3c7a3c5

Jocs changed the title ~~Fixed an issue where combo emoji was not displayed~~ Fixed an issue where combined emoji was not displayed Apr 27, 2024

chearon force-pushed the master branch from 234bebf to 75b1b57 Compare April 28, 2024 19:24

chearon reviewed Apr 28, 2024

View reviewed changes

Jocs added 2 commits April 29, 2024 12:59

refactor: finished with tofu when is last font match

0548103

remove parse-css.js

86c0970

Jocs requested a review from chearon April 29, 2024 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed an issue where combined emoji was not displayed #20

Fixed an issue where combined emoji was not displayed #20

Jocs commented Apr 26, 2024 •

edited

Jocs Apr 26, 2024

chearon Apr 28, 2024

Jocs Apr 29, 2024

Jocs Apr 29, 2024

chearon left a comment •

edited

chearon Apr 28, 2024

Jocs Apr 29, 2024

chearon Apr 28, 2024

chearon commented May 3, 2024

Jocs commented May 6, 2024

	this.paragraph.destroy();
	this.paragraph = createParagraph(this);
	this.paragraph.shape();
	}
	}

	postprocess() {
	super.postprocess();

Fixed an issue where combined emoji was not displayed #20

Are you sure you want to change the base?

Fixed an issue where combined emoji was not displayed #20

Conversation

Jocs commented Apr 26, 2024 • edited

Jocs Apr 26, 2024

Choose a reason for hiding this comment

chearon Apr 28, 2024

Choose a reason for hiding this comment

Jocs Apr 29, 2024

Choose a reason for hiding this comment

Jocs Apr 29, 2024

Choose a reason for hiding this comment

chearon left a comment • edited

Choose a reason for hiding this comment

chearon Apr 28, 2024

Choose a reason for hiding this comment

Jocs Apr 29, 2024

Choose a reason for hiding this comment

chearon Apr 28, 2024

Choose a reason for hiding this comment

chearon commented May 3, 2024

Jocs commented May 6, 2024

Jocs commented Apr 26, 2024 •

edited

chearon left a comment •

edited