A dirty solution to #891 #960
Conversation
Since embedded bitmap font works incorrectly, we should avoid using them, until a final patch is available and tested. I've added `FT_LOAD_NO_BITMAP` to ALL(3) places in `_imagingft.c`, which did (not much) actually fixed the issue. A notice has also been added to `_imagingft.c`.
|
Any comment on this pull request? I do know it's dirty, but I think it's the best solution at this time :| |
|
I haven't had time to review -- the SSL bug has taken up too much of my time this week. |
|
Can you add a test case for this using a freely distributable font? |
|
I'll try find one :) |
Strangely, the bitmap version of DejaVu Sans is always vertical one pixer longer.
StringIO does not exists on py3, which leads to failure of building.
|
Just a note that on my PC, test suite failed on my test(passed on travis), because of different font size between outline one and bitmap one (first one is exactly one pixel smaller than the second one), don't know why. |
| draw_bitmap.text((0, 0), text, fill=(0, 0, 0), font=font_bitmap) | ||
| draw_outline.text((0, 0), text, fill=(0, 0, 0), font=font_outline) | ||
| self.assert_image_similar(im_bitmap, im_outline, 0.01) | ||
|
|
There was a problem hiding this comment.
Please add this at the bottom so the script is runnable as a standalone:
if __name__ == '__main__':
unittest.main()
There was a problem hiding this comment.
Are we still need it? I thought that this legacy code, because tests are run via nosetests Tests/test_imagefont_bitmap.py
There was a problem hiding this comment.
@homm Travis CI runs the tests using nosetests, but at least I find it useful when debugging just a single test locally (or creating/adding new tests). Sure, there's a nose command to run a single test, but it's much easier and transparent to be able to do python Tests/test_etc.py, and it's only an extra couple of lines.
There was a problem hiding this comment.
On the contrary, it is much easier to run single tests with nose.
# Run test class:
$ nosetests -v Tests/test_image_transpose.py:TestImageTranspose
# Run individual test:
$ nosetests -v Tests/test_image_transpose.py:TestImageTranspose.test_rotate_270There was a problem hiding this comment.
OK. When I said single test I meant a single file, and python Tests/test_etc.py is the standard way to run a Python script, and a useful basic, extra option to having to remember a nose command and its syntax, especially for new contributors who may never have seen nosetests before.
|
When will my pull request merged? I have to use custom hacked version of Pillow here and there in my own project :| |
|
@jackyyf You probably need @wiredfool or @hugovk to review |
|
@jackyyf I'd like @wiredfool to check this. In the meantime, you can pip install from your own branch, something like this: |
|
Sorry, I missed that you had added the font and the testcases. I'll take a look. |
|
Looking @ this on my Mavericks machine, the tests are failing due to different font metrics between the bitmap and truetype font. On master, the test fails with Looking on Ubuntu, the metrics and renderings pass the test as committed. On linux, size_outline = size_bitmap = (212, 28), on the mac: size_outline = (212, 27) size_bitmap = (212, 28). So if we take that into account, and up the permissible error to something like 20, we should be able to easily distinguish between failures and successes. I've put that together on my wiredfool:pr960 branch. Once the tests pass, I'll merge that. |
|
#1072 merged. Thanks! |
Whoa! |
|
A Coveralls glitch, unless Pillow has been replaced by two .js files without me noticing: https://coveralls.io/builds/6232236 |
Currently we have no good solution to handle embedded bitmap fonts (#891), so we should avoid using them, until a final patch is available.