Reduce the use of XObjects in pdf backend [backport to 1.4.x]

[jkseppan]

Fixes #3345, fixes #2910. Fixes the pdf part of #2247, and it seems to me that the svg part is already fixed on master.

[jkseppan]

The failing tests are:

mpl_toolkits.tests.test_mplot3d.test_surface3d.test
mpl_toolkits.tests.test_mplot3d.test_mixedsubplots.test

In both cases, the pdf output fails to match the expected output by omitting some contour lines that are not there in the Agg output but are in the old pdf output. I'd say that's an improvement, but it's not yet clear to me why this change would bring it about.

Here's the Agg output for test_surface3d:

And the resulting pdf:

Compare to the expected pdf:

And test_mixedsubplots, Agg:

Pdf:

Expected pdf:

[tacaswell]

👍 for fixing a bug we didn't know we had.

I would advocate not merging this until we understand why this fixed a bug we didn't know we had.

[mdboom]

Thanks for this -- I think this is a really helpful optimization of an optimization.

I think the bug is related to the edge color not getting cleared for every XObject (since every patch in the 3d surface was an XObject before this patch). So I think it means we still technically have the bug if we trigger the case where XObjects are being used. I'll poke around and see if I can trigger that case and fix it.

[mdboom]

jkseppan#3 has a fix for the underlying bug.

[jkseppan]

Thanks @mdboom! I merged your addition into this pull request.

There's still the question of why draw_path_collection was being called with collections of one path each. That could have consequences for other backends as well.

[jkseppan]

The Travis tests are passing, so that didn't uncover any more surprises.

But now I realize this could probably go on the 1.4.x branch. Should I rebase the pull request on top of that?

[tacaswell]

Don't worry about it, I think it is easier to cherry-pick it to maintenance branch.

[jkseppan]

I looked at the code again, and it seems my earlier worry about draw_path_collection being called with one-path collections was mistaken. In the example that spurred this pull request (#3345) the method is called just once, with a collection of 5000 paths. Turning each path into a PDF XObject makes no sense in this case, because there is no reuse of each path and the definition and single use of an XObject are more expensive than just inlining the path definition. This is not an indication of a bug in the calling code, but it would make sense to investigate whether the svg and ps backends are doing something similar that could be optimized.

With @mdboom's fix for the alternative code path, I think this could be merged.

[jkseppan]

Yes, there are similar effects in svg and ps, but not as dramatic. It seems that the relative cost of XObjects in pdf is higher (and it's exacerbated by the fact that compression works well on the page stream but not so well on the short XObject streams). I added similar changes to the svg and ps backends.

[mdboom]

Nice! Ready to merge?

[jkseppan]

I think it's ready to merge, but I don't know if anyone wants to do more review. The Travis build is passing, which of course doesn't say anything about the ps backend but I think the risk there is small, since the change is simply using another known-working code path in some cases.

[tacaswell]

gah! this fell through the cracks and did not make it into 1.4.1 and is too big to tack onto the rc 😞 .

[jkseppan]

Oh; I've been busy with other things in the mean time. I agree this is too big to add in a release candidate, so how about we merge this into master and backport to 1.4.x after 1.4.1 is out, so it can get included in 1.4.2?

[tacaswell]

I just created a 1.4.2 milestone for exactly this reason.

I am going to hold of merging it as I plan to tag 1.4.1 tomorrow and it is harder to forget to merge that to forget to backport.

[tacaswell]

cherry-picked as efa3434