Skip to content

OffsetImage: use dpi_cor in get_extent#5112

Merged
efiring merged 3 commits intomatplotlib:masterfrom
Tillsten:patch-1
Sep 19, 2016
Merged

OffsetImage: use dpi_cor in get_extent#5112
efiring merged 3 commits intomatplotlib:masterfrom
Tillsten:patch-1

Conversation

@Tillsten
Copy link
Copy Markdown
Contributor

Before the image was always drawn with 72 dpi in the pdf backend, indepent of the dpi setting..

Before the image was always drawn with 72 dpi.
@tacaswell tacaswell added this to the next point release (1.5.0) milestone Sep 21, 2015
@tacaswell
Copy link
Copy Markdown
Member

looks like a bugfix to me

attn @jkseppan as this names the PDF backend specifically

@jkseppan
Copy link
Copy Markdown
Member

It seems to me that since the pdf backend doesn't define points_to_pixels, dpi_cor would be one and this would have no effect with that backend. This is @leejjoon's code and @NelleV added the FIXME comment, maybe they can comment on the pull request?

@Tillsten
Copy link
Copy Markdown
Contributor Author

@jkseppan I am not 100% sure this the right fix, but it does fix my problem and doesn't brake tests.

Both backend_bases and backend_agg, which afaik renders the image even in the pdf backend, implement points_to_pixels.

To explain what it fixes: Have a look at
http://matplotlib.org/examples/pylab_examples/demo_annotation_box.html
The png, hires-png and pdf version look all different, which should not be the case
if dpi_cor is set to True.

@jkseppan
Copy link
Copy Markdown
Member

I wonder if this is connected to #2831, #3364, #3918, #4375?

attn @mdboom, @WeatherGod

@WeatherGod
Copy link
Copy Markdown
Member

Hmm, quite possibly. I'll try out some of the SCCEs in those issues and
report back.

On Tue, Sep 22, 2015 at 8:08 AM, Jouni K. Seppänen <notifications@github.com

wrote:

I wonder if this is connected to #2831
#2831, #3364
#3364, #3918
#3918, #4375
#4375?

attn @mdboom https://github.com/mdboom, @WeatherGod
https://github.com/WeatherGod


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

@WeatherGod
Copy link
Copy Markdown
Member

none of the examples in those issues got fixed by this.

@tacaswell tacaswell modified the milestones: next bug fix release (2.0.1), next point release (1.5.0) Sep 24, 2015
@efiring
Copy link
Copy Markdown
Member

efiring commented Sep 18, 2016

@tacaswell I don't see any good reason not to merge this and backport it to 2.x, assuming it fixes the blatant error in our gallery as @Tillsten notes above.

@efiring
Copy link
Copy Markdown
Member

efiring commented Sep 19, 2016

The only Travis failure is in the Xcode build:

Error: libpng-1.6.23 already installed
To install this version, first `brew unlink libpng`

Lots of package installations occurred after this, but then the build apparently stalled and timed out.

@efiring efiring merged commit 98472a1 into matplotlib:master Sep 19, 2016
efiring added a commit that referenced this pull request Sep 19, 2016
OffsetImage: use dpi_cor in get_extent
@efiring
Copy link
Copy Markdown
Member

efiring commented Sep 19, 2016

Back-ported to v2.x as 2fe9e32. Thank you, @Tillsten!

@QuLogic QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0 (style change major release) Dec 7, 2016
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.

6 participants