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
Conversation
@@ -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): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
@@ -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() |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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!
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. |
Much tidier, thank you @piannucci. If you have a moment, would you mind taking a look @mdehoon? |
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 . |
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$') |
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. |
OK, I understand the bug now. |
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 Thanks, On Dec 13, 2013, at 7:02 AM, mdehoon notifications@github.com wrote:
|
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. |
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. |
There are a couple of issues we should discuss:
On Dec 17, 2013, at 9:34 AM, mdehoon notifications@github.com wrote:
|
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! |
MacOSX backend supports 2x DPI images and MathTeX.
This is a bit of a hack to fix #1958. Test with
The behavior on figimage is possibly not correct?