Skip to content

Conversation

@ShamsaHamed
Copy link
Contributor

This pull request adds support for languages that require complex text layout.

We are using the Raqm library, that wraps FriBidi (for bidirectional
text support) and HarfBuzz (for text shaping), and does proper BiDi and script
itemization:
https://github.com/HOST-Oman/libraqm

This should fix #1089.

@wiredfool
Copy link
Member

Thanks for the contribution. This is something that have been requested several times.

From a quick look at this I've got several concerns:

  • This changes the test platform from precise to trusty. While that may be a reasonable change, it's something that should be considered separately.
  • This ties all of the font rendering to three new dependencies, at least one of which is listed as an early prototype and is not available as a binary on any of our supported platforms other than through a PPA. Specifically, that means that this is going to break font rendering (and testing) on windows and older linuxes unless there is a build process for them.
  • We've got to have automated tests for this that are well specified. I don't believe that any of the core developers could look at the output from this and tell if it's correct or incorrect. So the level of testing needs to be somewhat more complete and fool proof than our usual tests.
  • Documentation on the changes to the api needs to be added.

@wiredfool
Copy link
Member

Also, please don't rebase PRs when we're reviewing them.

@khaledhosny
Copy link

Raqm depends on HarfBuzz, which is not packaged for Precise. For the other point, would providing fallback code for when Raqm is missing (that does essentially what the old code is doing) an acceptable compromise?

@wiredfool
Copy link
Member

I think I'd want fallback code to be comfortable merging this. I'd also want to update the scripts in the depends directory for those platforms where there aren't packages, and the winbuild directory for testing on windows. I couldn't tell if libraqm builds on windows or not, but it does appear that at least harfbuzz is available.

@ShamsaHamed ShamsaHamed force-pushed the CTL branch 2 times, most recently from 877e0f9 to 07d96e0 Compare January 26, 2016 04:57
@ShamsaHamed
Copy link
Contributor Author

I am working on fallback code in case that Raqm is missing, so it will use the old code.
I will also work on the tests and documentation fixing.

@radarhere radarhere changed the title Add complex text support. Add complex text support Jan 29, 2016
@ShamsaHamed
Copy link
Contributor Author

This provide the documentation, testing and fallback code.

@hugovk
Copy link
Member

hugovk commented Feb 3, 2016

Travis CI didn't build. I checked .travis.yml on https://lint.travis-ci.org/ and it said:

your repository must be feature flagged for the dist setting to be used

@ShamsaHamed
Copy link
Contributor Author

The failure is due to merge conflict, so I will rebase this after you finish reviewing the code.

@wiredfool
Copy link
Member

Go ahead and fix the merge conflicts so we can get good test runs on this.

@ShamsaHamed ShamsaHamed force-pushed the CTL branch 2 times, most recently from 6468cf7 to 9dd39e9 Compare February 4, 2016 04:50
This pull request adds support for languages that require complex text layout.

We are using the Raqm library, that wraps FriBidi (for bidirectional
text support) and HarfBuzz (for text shaping), and does proper BiDi and script
itemization:
https://github.com/HOST-Oman/libraqm

This should fix python-pillow#1089.
@ShamsaHamed ShamsaHamed force-pushed the CTL branch 3 times, most recently from 719890a to 0c9f04f Compare February 16, 2016 10:35
@ShamsaHamed ShamsaHamed force-pushed the CTL branch 2 times, most recently from 4e584ca to 9d806ee Compare February 17, 2016 10:56
@ShamsaHamed ShamsaHamed force-pushed the CTL branch 2 times, most recently from c419212 to b15f362 Compare February 22, 2016 06:12
@ShamsaHamed
Copy link
Contributor Author

I have a problem in testing file I have added. I run it locally and it is ok, but I can't find why it fails during travis-ci buliding. Even I can't reproduce the issue locally.
So anyone can debug the code and see if the issue is reproduced?
I have python 2.7 and 3.4.

@andreymal
Copy link

Any progress with this?

@ShamsaHamed can you upgrade this to Pillow 3.4.2? Then I will try to find and fix problems with the tests, if possible (this feature is needed for me :)

@wiredfool
Copy link
Member

Closed, merged with #2576

Thanks to everyone who worked on this, I think it's a valuable addition to the library.

@wiredfool wiredfool closed this Jul 1, 2017
@ShamsaHamed
Copy link
Contributor Author

Thanks to you for your efforts and sorry for my absence to complete the work. I have a problem with my device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug in rendering Indic fonts

6 participants