Conversation
|
While I am thinking of this, I should remember to double-check the results On Fri, Jan 23, 2015 at 2:47 PM, Michael Droettboom <
|
|
Note: There are some problems discovered with this approach. Working on a better solution now. |
|
All of the baseline images with text (all 7.7MB of them) have been updated so this passes. I'm not crazy about the increase in size in the repo, but I don't have any better ideas... |
|
@mdboom The only thing that I can think of is to put the test images in a git submodule similar to what IPython does with it's javascript dependencies. Not great since it will involve another setup step. |
|
That won't really help us with historical problem. The repo is currently almost 300MB, another 8 doesn't really matter. What we need is a way to hijack git and only pull the large objects when needed or get people to start doing shallow clones. |
|
Yes moving the images to a submodule will of cause only stop the growth of the repository. There are ways of removing files entirely from the history https://help.github.com/articles/remove-sensitive-data/ but I don't really thing that it is a good idea and I would be very scared about doing such a thing to the matplotlib repository. |
|
That would change sha1s of every commit back to the first image we added which is a complete non-starter. This is a feature of git, you can't tamper with the development history with out destroying all of it. |
|
Yes I agree which was what I tried to say above. I don't think there is going to be a good way of only hijacking git to not pull large objects for the same reason. I think the only thing that can be done is to advice on using shallow clones and try to figure out a solution to stop making it worse. BTW I tried doing something similar to what Fernando did to the IPython repository here on a local copy but that only gives me a total save from 279 to 271 Mb ✔ /tmp/matplotlib [master|✔] du -d 1 -h
198M ./.git
4.0K ./.travis
6.9M ./doc
2.1M ./examples
3.9M ./extern
67M ./lib
36K ./LICENSE
8.0K ./release
712K ./src
24K ./tools
168K ./unit
279M .
✔ /tmp/matplotlib [master|✔] du -d 1 -h
190M ./.git
4.0K ./.travis
6.9M ./doc
2.1M ./examples
3.9M ./extern
67M ./lib
36K ./LICENSE
8.0K ./release
712K ./src
24K ./tools
168K ./unit
271M .
✔ /tmp/matplotlib [master|✔] |
|
For some reason I only had to download ~195 MB Strange |
|
Oh, I know what the problem is, the mpl fork on my gh has a gh-pages branch, your number is |
|
Ok -- well let's not worry about repo size for this PR, then, but address it in some way in the future. I think this PR is ready to merge, then. |
There was a problem hiding this comment.
why did the sign of the kern adjustment change? This no longer matches the sign in the pdf backend...
There was a problem hiding this comment.
I'm looking into this further... I may have been mistaken about this change... (But in any event the sign does match because the analogous change was made in backend_pdf.py)
There was a problem hiding this comment.
This also may be an issue of me reading badly as this looked fine before the pep8 fixes
|
For reference to see just the code changes: mdboom/matplotlib@matplotlib:master...b52589b |
|
@mdboom Can you also document which FreeType and font version you used for this so that we can (eventually) pin that on travis? |
14a8da6 to
d5284f5
Compare
|
Thanks to @tacaswell's urging, it turns out the kerning wasn't as broken as I thought. I have a new approach that I hope is now correct, and I've rewritten the history so the updated test images are only in here once. I'll leave a few inline comments to @tacaswell's earlier comments. |
d5284f5 to
3afdaf0
Compare
|
These images were generated with freetype 2.5.3-21 on Fedora 21. The fonts used are all the ones that ship with matplotlib. |
|
I have manually checked for any negative effects in mplot3d. I didn't see any regressions. |
API : Font advance width This resets _all_ of the images with text.
|
Merged this before we accumulate too many more test images which will need to be reset (I think we already have one or two). |
|
I see |
|
I fixed one last night after I merged this On Thu, Feb 19, 2015, 08:19 Michael Droettboom notifications@github.com
|
Document what version of freetype was used to generate baseline images in PR matplotlib#4031 / commit 005cfde and which version is (currently) being run on travis.ci
Document what version of freetype was used to generate baseline images in PR matplotlib#4031 / commit 005cfde and which version is (currently) being run on travis.ci
Fix #253.
This changes the text bounding box calculation to use advance width of the character, rather than the bounding box of the control points.
It also fixes a long-standing bug in kerning in that (a) the conversion from 26.6 fixed point to floating-point was incorrect, and (b) the sign was backwards.
This PR needs a couple of things to push it over the finish line: