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

MacOSX backend supports 2x DPI images and MathTeX. #2665

Merged
merged 1 commit into from Dec 18, 2013

Conversation

piannucci
Copy link
Contributor

This is a bit of a hack to fix #1958. Test with

import numpy as np, pylab as pl
pl.imshow(np.arange(1000000).reshape(1000,1000) % 997)
pl.text(0,100,'$foo$')

The behavior on figimage is possibly not correct?

@@ -39,7 +39,7 @@ class RendererMac(RendererBase):

texd = maxdict(50) # a cache of tex image rasters

def __init__(self, dpi, width, height):
def __init__(self, dpi, width, height, canvas):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RendererMac needs a reference to the canvas because the manager hasn't been created yet, and the manager's NSWindow is our ticket to reading the scale parameter of the backing store.

Copy link
Member

Choose a reason for hiding this comment

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

My knee jerk reaction is to not add arguments to the __init__ signature, but add this information as an attribute at some later point (and make sure where ever you use it can deal with self.canvas is None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind making the change, but since RendererMac is only instantiated in one place (FigureCanvasMac.init), it seems like extra complexity. The renderer's canvas would get set immediately after constructing it.

@mdboom
Copy link
Member

mdboom commented Dec 10, 2013

@mdehoon

@@ -107,28 +108,40 @@ def draw_gouraud_triangle(self, gc, points, colors, transform):
points = transform.transform(points)
gc.draw_gouraud_triangle(points, colors)

def get_image_magnification(self):
return self.canvas.manager.get_backing_scale_factor()
Copy link
Member

Choose a reason for hiding this comment

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

At least with the other backends, canvas objects can be embedded in other programs without having to go through our manager classes. You can not assume that canvas has a the attribute manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we can do this with FigureCanvas instead of FigureManager. Thanks!

@piannucci
Copy link
Contributor Author

Okay, pull request rewritten to be a little more subtle. Note that calling renderer.get_image_magnification() outside of the OS X drawing callbacks will just error because the graphics context doesn't exist at that point. This is kind of lousy but it appears to work fine.

@pelson
Copy link
Member

pelson commented Dec 12, 2013

Much tidier, thank you @piannucci. If you have a moment, would you mind taking a look @mdehoon?

@mdehoon
Copy link
Contributor

mdehoon commented Dec 12, 2013

I'd be happy to look at it, but first I'll have to understand which bug this pull request is trying to solve. I have not been able to reproduce the bug reported in issue #1958 .

@piannucci
Copy link
Contributor Author

My test case is

import numpy as np, pylab as pl
pl.imshow(np.arange(1000000).reshape(1000,1000) % 997)
pl.text(0,100,'$foo$')

@mdehoon
Copy link
Contributor

mdehoon commented Dec 13, 2013

Thanks for your reply. Then what is the bug? Because when I zoom in, the pixel size of the matplotlib graphics is exactly the same as the pixel size elsewhere on the screen.

@piannucci
Copy link
Contributor Author

The MathTeX is low-res, as is the AxesImage. These two screenshots are from before and after applying my patch. I get the same results off of trunk and 1.3.1.

Before:
non_retina

After:
retina

@mdehoon
Copy link
Contributor

mdehoon commented Dec 13, 2013

OK, I understand the bug now.
But after applying this patch to matplotlib-master, I get exactly the same resolution as before the patch.. It looks the same as your screen shot before applying the patch.

@piannucci
Copy link
Contributor Author

Can you please double-check that you are using the macosx backend? What does pl.gcf().canvas.renderer.get_image_magnification() say? What about the following?

import AppKit
AppKit.NSScreen.mainScreen().backingScaleFactor()

Thanks,
-Peter

On Dec 13, 2013, at 7:02 AM, mdehoon notifications@github.com wrote:

OK, I understand the bug now.
But after applying this patch to matplotlib-master, I get exactly the same resolution as before the patch.. It looks the same as your screen shot before applying the patch.


Reply to this email directly or view it on GitHub.

@mdehoon
Copy link
Contributor

mdehoon commented Dec 13, 2013

I tried on a different computer, and there indeed I got the higher resolution after applying the patch. After the weekend I can try my first computer again, and check why I didn't get the higher resolution there.

@mdehoon
Copy link
Contributor

mdehoon commented Dec 17, 2013

I realized the first computer I tried didn't have a retina display, so that was a false alarm. My apologies. On the second computer (with a retine display), the patch works fine. So I am in favor of accepting this patch.

@piannucci
Copy link
Contributor Author

There are a couple of issues we should discuss:

  • The behavior of FigImage
  • The rasterization of event plots (IIRC, can't check right now)

On Dec 17, 2013, at 9:34 AM, mdehoon notifications@github.com wrote:

I realized the first computer I tried didn't have a retina display, so that was a false alarm. My apologies. On the second computer (with a retine display), the patch works fine. So I am in favor of accepting this patch.


Reply to this email directly or view it on GitHub.

@pelson
Copy link
Member

pelson commented Dec 18, 2013

Merging. This is clearly a good piece of work which is making a step in the right direction. If there are any follow on issues relating to this change, are you able to lend a hand @piannucci?

Cheers!

pelson added a commit that referenced this pull request Dec 18, 2013
MacOSX backend supports 2x DPI images and MathTeX.
@pelson pelson merged commit 4f5bc02 into matplotlib:master Dec 18, 2013
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.

Macosx: Retina displays are not supported
5 participants