-
-
Notifications
You must be signed in to change notification settings - Fork 320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add better support for Arabic language to Text Layer #2232
Conversation
Actually I don't know anything about those languages. @Fahad-Alsaidi could you test it? |
Known issue: TAB is not rendered properly when HarfBuzz is enabled. |
0dee87a
to
2089a98
Compare
This pull request introduces 1 alert and fixes 1 when merging 2089a98 into 223026f - view on LGTM.com new alerts:
fixed alerts:
|
2089a98
to
c42938b
Compare
This pull request introduces 1 alert and fixes 1 when merging c42938b into 223026f - view on LGTM.com new alerts:
fixed alerts:
|
c42938b
to
693c61f
Compare
The last forced push was to amend the following comments, due to @ice0 review:
|
This pull request introduces 1 alert and fixes 1 when merging 693c61f into f32ae7f - view on LGTM.com new alerts:
fixed alerts:
|
@FirasH could you test it? I don't know Arabic, Hebrew, etc... So I tried to make according to the manuals, but I had to adapt to Synfig needs. |
Testing now! |
I cannot start the AppVeyor image on Windows, need to build it. |
Thank you, @FirasH ! I'll fix it. Luckily, this error happened after I tried to fix the case when I include an English word in an Arabic phrase like I show a few comments above. I tested without that fix, and the text shapes like you showed me how it should do! :) What about multiple lines? If a single phrase extends to next line it shapes differently if the second line would be another phrase starting with the same word? Er... So confusing. Let me show I want to say. Please consider each character-sequence as a valid word, and if a character-sequence repeats, it means they are the same word: Case 1: one single phrase in two lines (5 words in the first line and the last 3 in the second line)
Case 2: two phrases, one per line
My question is: does the In English, Portuguese (my mother tongue/native language), French, Spanish, German, I know that "it doesn't matter": the first character needs to be changed to "upper case", but it's another character/byte that user need to type directly, it's not automatic. In Japanese, the position in phrase doesn't matter at all. Explaining why I'm asking it: if it doesn't matter for Arabic and other languages, I can split the text into lines first and optimize the code a bit. Otherwise I could only do it at each frame rendering (in current text renderer). |
The shape of
In Arabic there is no upper cases, in your example |
Great work I test it, and it works fine except Firas comments |
693c61f
to
17d43e4
Compare
I believe I fixed it :) @FirasH @Fahad-Alsaidi |
17d43e4
to
ace87eb
Compare
This pull request fixes 1 alert when merging ace87eb into fb80f27 - view on LGTM.com fixed alerts:
|
well, there is no difference see I notice one bug in many fonts in my system, the last letter is converted to rectangular see: here is a test file with fonts |
Hey! I think I found out the problem @Fahad-Alsaidi : synfig doesn't recognize the font file ;) |
- reduce variable scope - remove useless comments - remove unused variable - remove unneeded typecasts
Now Hebrew, Arabic and other languages that need text shaping is properly supported, I hope!
And FriBiDi required only when HarfBuzz is used
a356cfa
to
fd29152
Compare
This pull request fixes 1 alert when merging fd29152 into fcaa6fb - view on LGTM.com fixed alerts:
|
Merged. Thank you! |
Woohooo! 🎉 🎉 🎉 |
@rodolforg Now eagerly waiting a PR with optimized (cobra-enabled) text layer. ^__^ |
I would like to thank @rodolforg & @morevnaproject & @ice0 for supporting RTL in synfig. You did a great job, thanks 🌹🌹 |
@morevnaproject
@Fahad-Alsaidi |
I've got an idea how we can handle situations with missing font. |
Arabic, Hebrew and other languages that needs text shaping is now finally supported, thanks to FriBiDi and HarfBuzz libraries.
These libraries are already backend for Gtk/Glib, so there is not any change for packaging.
Fix #507
Fix #1621