While I am thinking of this, I should remember to double-check the results
from mplot3d, especially with offset text and other things that do
non-traditional anchoring. Most (all?) mplot3d test have text removed.
Also, the tests may not capture what one might see when interacting with
the 3d axes.

On Fri, Jan 23, 2015 at 2:47 PM, Michael Droettboom <
notifications@github.com> wrote:

Fix#253#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:

Virtually every test with text in it now fails, but from the
spot-checking I've done, they've improved for the better. I'd suggest
updating every image with any text, even if the difference isn't enough for
the test to fail, just to reset the baseline correctly.

The Abel font@sgmentzerhttps://github.com/sgmentzer used in#253
#253 is a good one to
catch this kind of error because the difference between advance width and
control point bounding box is so large and because the kerning is so
extreme. It might make sense to make a test or two based on it -- but then
we have some technical issues to solve about how to get it. (It's under SIL
Open Font License, so we can just incorporate it, but that might be
overkill).

---

You can view, comment on, or merge this pull request online at:

#4031
Commit Summary

• FixAlign text using advance width, not glyph width #253. Use advance width to calculate bounding box.
• Add CHANGELOG

File Changes

• M CHANGELOG
  https://github.com/matplotlib/matplotlib/pull/4031/files#diff-0 (3)
• M lib/matplotlib/backends/backend_pdf.py
  https://github.com/matplotlib/matplotlib/pull/4031/files#diff-1 (2)
• M lib/matplotlib/backends/backend_ps.py
  https://github.com/matplotlib/matplotlib/pull/4031/files#diff-2 (2)
• M src/ft2font.cpp
  https://github.com/matplotlib/matplotlib/pull/4031/files#diff-3 (16)

Patch Links:

• https://github.com/matplotlib/matplotlib/pull/4031.patch
• https://github.com/matplotlib/matplotlib/pull/4031.diff

—
Reply to this email directly or view it on GitHub
#4031.

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.

tcaswell@eowyn:tmp$ git clone matplotlibCloning into 'matplotlib'...remote: Counting objects: 101713, done.remote: Compressing objects: 100% (29291/29291), done.remote: Total 101713 (delta 72112), reused 101687 (delta 72095)Receiving objects: 100% (101713/101713), 294.75 MiB | 9.67 MiB/s, done.Resolving deltas: 100% (72112/72112), done.Checking connectivity... done.

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 historyhttps://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 ofevery 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 repositoryhere on a local copy but that only gives me a total save from 279 to 271 Mb

✔ /tmp/matplotlib [master|✔] du -d 1 -h198M    ./.git4.0K    ./.travis6.9M    ./doc2.1M    ./examples3.9M    ./extern 67M    ./lib 36K    ./LICENSE8.0K    ./release712K    ./src 24K    ./tools168K    ./unit279M.✔ /tmp/matplotlib [master|✔] du -d 1 -h190M    ./.git4.0K    ./.travis6.9M    ./doc2.1M    ./examples3.9M    ./extern 67M    ./lib 36K    ./LICENSE8.0K    ./release712K    ./src 24K    ./tools168K    ./unit271M.✔ /tmp/matplotlib [master|✔]

For some reason I only had to download ~195 MB Strange

✘-1 /tmp/test git clone https://github.com/matplotlib/matplotlib.gitCloning into 'matplotlib'...remote: Counting objects: 96987, done.remote: Compressing objects: 100% (115/115), done.remote: Total 96987 (delta 79), reused 32 (delta 15)Receiving objects: 100% (96987/96987), 195.26 MiB | 1.75 MiB/s, done.Resolving deltas: 100% (70659/70659), done.Checking connectivity... done.

Oh, I know what the problem is, the mpl fork on my gh has a gh-pages branch, your number ismore accurate.

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.

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?

Thanks to@tacaswell's urging, it turns out the kerning wasn't as broken as I thought. I have a new approach that Ihope 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.

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.

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 seemaster is passing on Travis. I assume that means all of our imagesare currently good, then?

I fixed one last night after I merged this

On Thu, Feb 19, 2015, 08:19 Michael Droettboomnotifications@github.com
wrote:

I see master is passing on Travis. I assume that means all of our images
are currently good, then?

—
Reply to this email directly or view it on GitHub
#4031 (comment)
.