Skip to content

Conversation

Kudze
Copy link
Contributor

@Kudze Kudze commented May 6, 2025

Description

Fixes #10583

In Action

Added unit test case that verifies that the flag emojis are split correctly.
Name of the test case correctly splits strings including flag emojis into graphmes.

Copy link

codesandbox bot commented May 6, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@asturur
Copy link
Member

asturur commented May 8, 2025

We don't really support splitting internally outside the whole char function.
I would rather use a real grapheme splitter.
This code won't work for any emoji outside the basic one anyway. Would it?

Copy link
Contributor

github-actions bot commented May 8, 2025

Build Stats

file / KB (diff) bundled minified
fabric 914.410 (+0.543) 301.450 (+0.216)

@Kudze
Copy link
Contributor Author

Kudze commented May 8, 2025

We don't really support splitting internally outside the whole char function. I would rather use a real grapheme splitter. This code won't work for any emoji outside the basic one anyway. Would it?

I guess yeah its better what you recommended in issue. I edited the PR with following cnhages: added https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Segmenter as main method of splitting string into graphemes if its availabe, current code as fallback.

Imo Its just weird that this library by default does not support all emojis.

@asturur
Copy link
Member

asturur commented May 8, 2025

I know is weird, but emoji is a living standard that updates once at year at minimum, there are unicode tables that grows in size to support those.
Now for the people making using of emoji or languages that do not live in the main unicode plane ( or how is that called, i do not remember ) it make senze to have that baggage of extra code, for all the others no.
Subclassing and adding a real grapheme splitter is few lines of code.
In this case browser support came in our rescue offering this 2 - 3 lines method that solves flags, emoji and mani langauges and offers proper word splitting that can be leveraged ( in another time ) for better word splitting.

If you look at a grapheme splitter that supports up to unicode 15
https://github.com/flmnt/graphemer/blob/master/src/Graphemer.ts
you can see it is not a simple script.

Unicode 16 adds a bunch of new chars
image

and is not supported yet by that library.
Is an entire world i decide i don't have the strength to support in custom javascript.

@asturur
Copy link
Member

asturur commented May 8, 2025

This looks good to me, thanks for following up.
I ll merge tomorrow after giving it a little test.

@asturur asturur changed the title fix: flag emojis in text feat(Text): Add support for Intl.segmenter May 8, 2025
@asturur
Copy link
Member

asturur commented May 9, 2025

Well apparently i didn't fix prettier.
You should be able to run npm run prettier:write in your command line to fix it.

@asturur
Copy link
Member

asturur commented May 9, 2025

Ok so for this test here:

水行末 雲来末 風来末、やぶら小路の藪柑子、海砂利水魚の、シューリンガンのグーリンダイ、食う寝る処に住む処、五劫の擦り切れ。パイポパイポ パイポのシューリンガン、水行末 雲来末 風来末。寿限無、寿限無、海砂利水魚の。やぶら小路の藪柑子、シューリンガンのグーリンダイ、パイポパイポ パイポのシューリンガン。

やぶら小路の藪柑子。水行末 雲来末 風来末、シューリンガンのグーリンダイ。グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ。水行末 雲来末 風来末。長久命の長助、寿限無、寿限無、パイポパイポ パイポのシューリンガン、食う寝る処に住む処、寿限無、寿限無。五劫の擦り切れ。海砂利水魚の。やぶら小路の藪柑子、食う寝る処に住む処、長久命の長助、海砂利水魚の、グーリンダイのポンポコピーのポンポコナーの、パイポパイポ パイポのシューリンガン。五劫の擦り切れ。

寿限無、寿限無、グーリンダイのポンポコピーのポンポコナーの、グーリンダイのポンポコピーのポンポコナーの。寿限無、寿限無、やぶら小路の藪柑子。やぶら小路の藪柑子、食う寝る処に住む処。水行末 雲来末 風来末、長久命の長助。五劫の擦り切れ。海砂利水魚の。海砂利水魚の。シューリンガンのグーリンダイ、シューリンガンのグーリンダイ、長久命の長助、水行末 雲来末 風来末。

グーリンダイのポンポコピーのポンポコナーの。長久命の長助、海砂利水魚の、寿限無、寿限無、食う寝る処に住む処、シューリンガンのグーリンダイ。海砂利水魚の、水行末 雲来末 風来末。水行末 雲来末 風来末、やぶら小路の藪柑子、五劫の擦り切れ。グーリンダイのポンポコピーのポンポコナーの、パイポパイポ パイポのシューリンガン。パイポパイポ パイポのシューリンガン。やぶら小路の藪柑子、長久命の長助、シューリンガンのグーリンダイ、寿限無、寿限無、五劫の擦り切れ。食う寝る処に住む処。

食う寝る処に住む処、海砂利水魚の、五劫の擦り切れ。寿限無、寿限無、グーリンダイのポンポコピーのポンポコナーの。寿限無、寿限無。水行末 雲来末 風来末、海砂利水魚の、やぶら小路の藪柑子、シューリンガンのグーリンダイ、パイポパイポ パイポのシューリンガン。五劫の擦り切れ。

シューリンガンのグーリンダイ。パイポパイポ パイポのシューリンガン、海砂利水魚の。食う寝る処に住む処。水行末 雲来末 風来末、パイポパイポ パイポのシューリンガン、水行末 雲来末 風来末。長久命の長助。寿限無、寿限無。やぶら小路の藪柑子。グーリンダイのポンポコピーのポンポコナーの、五劫の擦り切れ、やぶら小路の藪柑子、五劫の擦り切れ、食う寝る処に住む処、長久命の長助。寿限無、寿限無。グーリンダイのポンポコピーのポンポコナーの。海砂利水魚の。

食う寝る処に住む処。グーリンダイのポンポコピーのポンポコナーの。寿限無、寿限無。パイポパイポ パイポのシューリンガン、やぶら小路の藪柑子。長久命の長助。寿限無、寿限無。パイポパイポ パイポのシューリンガン。五劫の擦り切れ、やぶら小路の藪柑子、シューリンガンのグーリンダイ、長久命の長助。食う寝る処に住む処、海砂利水魚の。海砂利水魚の。グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ。水行末 雲来末 風来末。五劫の擦り切れ、水行末 雲来末 風来末。

海砂利水魚の、長久命の長助、シューリンガンのグーリンダイ、長久命の長助。食う寝る処に住む処、グーリンダイのポンポコピーのポンポコナーの。やぶら小路の藪柑子、パイポパイポ パイポのシューリンガン、水行末 雲来末 風来末。パイポパイポ パイポのシューリンガン。寿限無、寿限無、シューリンガンのグーリンダイ、五劫の擦り切れ、やぶら小路の藪柑子、寿限無、寿限無。グーリンダイのポンポコピーのポンポコナーの。

海砂利水魚の。グーリンダイのポンポコピーのポンポコナーの。パイポパイポ パイポのシューリンガン、食う寝る処に住む処、海砂利水魚の。食う寝る処に住む処、シューリンガンのグーリンダイ。寿限無、寿限無。五劫の擦り切れ。水行末 雲来末 風来末、やぶら小路の藪柑子、やぶら小路の藪柑子、寿限無、寿限無。グーリンダイのポンポコピーのポンポコナーの。

長久命の長助。食う寝る処に住む処。やぶら小路の藪柑子、グーリンダイのポンポコピーのポンポコナーの、寿限無、寿限無、海砂利水魚の、五劫の擦り切れ。やぶら小路の藪柑子、シューリンガンのグーリンダイ。食う寝る処に住む処。グーリンダイのポンポコピーのポンポコナーの。寿限無、寿限無。シューリンガンのグーリンダイ、長久命の長助、五劫の擦り切れ、パイポパイポ パイポのシューリンガン、海砂利水魚の、水行末 雲来末 風来末。パイポパイポ パイポのシューリンガン。水行末 雲来末 風来末。

グーリンダイのポンポコピーのポンポコナーの、長久命の長助。シューリンガンのグーリンダイ。海砂利水魚の、食う寝る処に住む処。食う寝る処に住む処。グーリンダイのポンポコピーのポンポコナーの。海砂利水魚の。水行末 雲来末 風来末。

シューリンガンのグーリンダイ。水行末 雲来末 風来末、五劫の擦り切れ。寿限無、寿限無、やぶら小路の藪柑子。寿限無、寿限無。長久命の長助、パイポパイポ パイポのシューリンガン。長久命の長助。パイポパイポ パイポのシューリンガン、水行末 雲来末 風来末、海砂利水魚の。グーリンダイのポンポコピーのポンポコナーの、食う寝る処に住む処。

水行末 雲来末 風来末。水行末 雲来末 風来末、食う寝る処に住む処。海砂利水魚の。食う寝る処に住む処。寿限無、寿限無、シューリンガンのグーリンダイ。寿限無、寿限無、グーリンダイのポンポコピーのポンポコナーの。長久命の長助。やぶら小路の藪柑子、五劫の擦り切れ。やぶら小路の藪柑子、パイポパイポ パイポのシューリンガン、海砂利水魚の、長久命の長助。グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ。パイポパイポ パイポのシューリンガン。五劫の擦り切れ。

五劫の擦り切れ、グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ、パイポパイポ パイポのシューリンガン、寿限無、寿限無。海砂利水魚の、長久命の長助、長久命の長助。

海砂利水魚の。食う寝る処に住む処。グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ。水行末 雲来末 風来末。パイポパイポ パイポのシューリンガン、五劫の擦り切れ、寿限無、寿限無、長久命の長助。五劫の擦り切れ。海砂利水魚の。長久命の長助、パイポパイポ パイポのシューリンガン。水行末 雲来末 風来末、グーリンダイのポンポコピーのポンポコナーの。シューリンガンのグーリンダイ、食う寝る処に住む処。やぶら小路の藪柑子、やぶら小路の藪柑子。

パイポパイポ パイポのシューリンガン。グーリンダイのポンポコピーのポンポコナーの、寿限無、寿限無。五劫の擦り切れ。水行末 雲来末 風来末。食う寝る処に住む処、シューリンガンのグーリンダイ。パイポパイポ パイポのシューリンガン。水行末 雲来末 風来末、長久命の長助。やぶら小路の藪柑子、食う寝る処に住む処、海砂利水魚の、シューリンガンのグーリンダイ、やぶら小路の藪柑子、長久命の長助。五劫の擦り切れ。グーリンダイのポンポコピーのポンポコナーの。海砂利水魚の。

The normal js simple loop takes 1ms to split, with the grapheme splitter it takes 8ms, that is surprising to me ( those are the values on my machine a 5y old m1 pro )

If we factor out the segmenter creation like this:

const segmenter =
  'Intl' in getFabricWindow() &&
  'Segmenter' in Intl &&
  new Intl.Segmenter(undefined, {
    granularity: 'grapheme',
  });

/**
 * Divide a string in the user perceived single units
 * @param {String} textstring String to escape
 * @return {Array} array containing the graphemes
 */
export const graphemeSplit = (textstring: string): string[] => {
  if (segmenter) {
    const segments = segmenter.segment(textstring);
    return Array.from(segments).map(({ segment }) => segment);
  }
  //Fallback
  return graphemeSplitImpl(textstring);
};

it goes back to 1ms or less ( depending on the garbage collector, my there is no perceived degradation of performance ).
Now this code snippet i quickly wrote isn't good

We will need to think something around the env or config object and see what is best.

We are nearly there.

@Kudze
Copy link
Contributor Author

Kudze commented May 9, 2025

Hmm weird i tried to test in chrome on my machine it was not making this big difference. (linux chrome ryzen).

It encoded 1k of these string in 250-300 ms.

But yeah would make sense to initialize it once.

@asturur
Copy link
Member

asturur commented May 11, 2025

For now let's merge this, i will try to make it fit in the cache or config class in some way. This is a good improvement, thank you.

@asturur asturur merged commit d106b7a into fabricjs:master May 11, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Text component does not render correctly flag emojis correctly in case text is rendered by each character seperately.
2 participants