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

Implement dash pattern scaling in pdf #6590

Closed
wants to merge 3 commits into from

Conversation

jkseppan
Copy link
Member

Factor the dash pattern scaling into a separate function so GraphicsContextPdf.delta can call it. Combine the linewidth and dash comparisons because now linewidth affects dashing.

Fixes #6588

I didn't see if this breaks anything else, it's likely to change the test results for all tests with dashed lines.

Factor the dash pattern scaling into a separate function so
GraphicsContextPdf.delta can call it. Combine the linewidth
and dash comparisons because now linewidth affects dashing.

Fixes matplotlib#6588
@QuLogic QuLogic added this to the 2.0 (style change major release) milestone Jun 15, 2016
@tacaswell
Copy link
Member

This might also address the pdf related issue brought up in #5430

@afvincent
Copy link
Contributor

@tacaswell About your comment on #5430 , are you sure there is an issue with the PDF backend and the offset negative values? Because the produced PDF file looks fine to me, with and without jkseppan's fix (I rapidly copy-pasted his fix and re-tested it on #5430 MWE).

@tacaswell
Copy link
Member

@afvincent Yes, sorry 2nd time being wrong about that 😞

@jkseppan
Copy link
Member Author

I don't think this patch does anything for #5430. It would be a small change, though.

I think the pdf specification implies that the offset should be
nonnegative.
@afvincent
Copy link
Contributor

Wouldn't it be worth including @jkseppan's handling of negative dash offsets into backend_bases.py (for example in the method scale_dashes) instead of just in backend_pdf.py? It might solve the remaining issue in #5430 with the PNG export.

@mdboom
Copy link
Member

mdboom commented Jun 28, 2016

@jkseppan: This looks good and I'd like to get this in for the next release candidate. Would you mind updating the test images so everything passes?

@tacaswell
Copy link
Member

The lw related computation probably needs to be pulled all they way up into Line2D to fix #6592

@jkseppan
Copy link
Member Author

@mdboom: I added new baseline images, but now I'm not so sure this fixes the difference between backends. If you compare the files lib/matplotlib/tests/baseline_images/test_axes/dash_offset.png and lib/matplotlib/tests/baseline_images/test_axes/dash_offset.pdf the pdf file has much longer dashes.

if offset is None or dash is None:
dash = []
offset = 0
if sum(dash) > 0:
Copy link
Member

Choose a reason for hiding this comment

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

This check should probably be moved up into the base class?

@tacaswell
Copy link
Member

Something is not right that this is leaking into the tests under the classic style.

I have an idea of how to fix this and #6592, but am not completely sure it will work.

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jul 12, 2016
When scaling the dash pattern by the linewidth do the scaling at artist
creation / value set time rather than at draw time.

closes matplotlib#6592
closes matplotlib#6588
closes matplotlib#6590
Closes matplotlib#6693
closes matplotlib#5430
@tacaswell tacaswell closed this in 3ef3f5b Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants