Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MixedModeRenderer non-72-dpi fixes & Pgf mixed rendering #1975

Merged
merged 2 commits into from May 27, 2013
Merged

MixedModeRenderer non-72-dpi fixes & Pgf mixed rendering #1975

merged 2 commits into from May 27, 2013

Conversation

pwuertz
Copy link
Contributor

@pwuertz pwuertz commented May 4, 2013

I would like to add mixed mode rendering + test to backend_pgf for handling plots that are too complex for vectorized output (https://github.com/pwuertz/matplotlib-backend-pgf/issues/5).

A small fix was necessary since MixedModeRenderer assumes that the figure's dpi is always 72. I think I also fixed a bad indent at stop_rasterizing that was probably unnoticeable due to the fixed dpi assumption. According to a comment in the code @leejjoon might know if this is correct.

@@ -48,6 +48,7 @@ def __init__(self, figure, width, height, dpi, vector_renderer,
# the figure dpi before and after the rasterization. Although
# this looks ugly, I couldn't find a better solution. -JJL
self.figure=figure
self._figdpi = figure.get_dpi()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just spotted this one too.

@pelson
Copy link
Member

pelson commented May 7, 2013

👍 - looks good to me.

@pwuertz
Copy link
Contributor Author

pwuertz commented May 7, 2013

Ok, this looks VERY related to the PR you mentioned. You think this will be beneficial or does it interfere with the other patch?

@pelson
Copy link
Member

pelson commented May 8, 2013

Ok, this looks VERY related to the PR you mentioned.

This PR fixes a bug which #1185 does not. Namely: The figure dpi is reverted to 72 after using the MixedModeRasterizer. After that, there is an awful lot of overlap. I suggest that we focus on #1185 (with a keen eye from yourself) and once that is in, we look at implementing this fix.

How does that sound?

@leejjoon
Copy link
Contributor

leejjoon commented May 9, 2013

Just looking through the code, I don't think it will interfere with #1185 (or other backends).
@pwuertz, did you happen to check if the pdf output from the pgf backend is consistent with that from the pdf backend when rasterization is used?

The hard-coded dpi of 72 was because all the vector backend uses dpi of 72 internally. But, this seems not to be the case of pgf backend.

While the PR looks good to me, I will try some test sometime this weekends.

@pwuertz
Copy link
Contributor Author

pwuertz commented May 22, 2013

@leejjoon The test I included looks the same for backend_pdf and backend_pgf.

@mdboom
Copy link
Member

mdboom commented May 24, 2013

#2044 (which is a rebased #1185) is almost ready -- there's an unreproducible test failure there at the moment. But once that's done, maybe we could update this PR to be based on it? I'd like to get this in for 1.3.0 if at all possible.

@mdboom
Copy link
Member

mdboom commented May 24, 2013

Ok -- #2044 is in, so I think all that's left for you @pwuertz is to submit everything this does that it doesn't do. Thanks!

@pwuertz
Copy link
Contributor Author

pwuertz commented May 25, 2013

Rebased on new master, but a test for py2.6 failed, don't know if that is related to my PR. I can check that tomorrow.

pwuertz added a commit that referenced this pull request May 27, 2013
MixedModeRenderer non-72-dpi fixes & Pgf mixed rendering
@pwuertz pwuertz merged commit df75b0b into matplotlib:master May 27, 2013
@pwuertz pwuertz deleted the pgf_mixedrenderer branch May 27, 2013 12:54
@pwuertz
Copy link
Contributor Author

pwuertz commented May 27, 2013

Allright, the 2.6 test fail is not related to this PR, merging...

@pwuertz
Copy link
Contributor Author

pwuertz commented May 27, 2013

py2.6 now passes, but py3.3 throws an ImageComparisonFailure for matplotlib.tests.test_image.test_rasterize_dpi.test although the images seem to be identical. The diff image is completely black.

pwuertz added a commit that referenced this pull request May 28, 2013
MixedModeRenderer non-72-dpi fixes & Pgf mixed rendering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants