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

Update various low resolution graphics for retina displays #2234

Closed
asmeurer opened this issue Aug 1, 2012 · 61 comments
Closed

Update various low resolution graphics for retina displays #2234

asmeurer opened this issue Aug 1, 2012 · 61 comments
Milestone

Comments

@asmeurer
Copy link
Contributor

asmeurer commented Aug 1, 2012

I have a retina MacBook Pro, and I've noticed that several of the images in IPython need to be updated to higher resolution so that they don't look bad. What I've noticed so far:

  • The icons in the notebook, including the reload icon on the dashboard and all the toolbar icons in the notebook itself
  • The IPython icon in the header of the qtconsole
  • The IPython icon itself from the qtconsole (Note that Mac OS X supports extremely high resolution application icons)
  • The IPython icon in the quit screen in the qtconsole
  • The default resolution for inline plots in both the qtconsole and the notebook is too low (this one is probably the most important).
  • The favicon for the notebook.

Note that you'll need to use Safari to test the notebook, as Chrome doesn't fully support retina graphics yet (Chrome Canary supports retina graphics, but not retina favicons).

This is only based on my little knowledge. Likely there are images and such for features that I don't know about that also need to be updated.

On non-retina Macs, you can test this by using Quartz Debug to increase the scaling to 2x.

Sorry if this seems low priority, but you really have to use a retina device to see just how bad non-retina things look.

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 1, 2012

Another thing to note: Safari apparently smooths out low resolution graphics (i.e., the toolbar buttons) and Chrome does not.

@minrk
Copy link
Member

minrk commented Aug 2, 2012

The default resolution of plots is also the easiest to deal with, since it's just matplotlib rcParams, and 2x values are completely inappropriate for everyone else. So I don't think that one is going to change for at least the years it takes for pixel-doubled displays to become relatively common.

I expect a Pull Request with 2x assets :)

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 2, 2012

Well I imagine that retina displays will be much less than relatively common before the low res plots annoy enough people to get you guys to change it :)

I know almost nothing about the code, so I could be wrong here, but I think for the notebook and qtconsole you may need to update something from upstream, if it exists (or in the case of qt, perhaps just wait for an upstream update the supports retina rendering). But again, this is just from a cursory glance at the code, so I could be wrong.

The favicon obviously you have complete control over.

@minrk
Copy link
Member

minrk commented Aug 2, 2012

Well I imagine that retina displays will be much less than relatively common before the low res plots annoy enough people to get you guys to change it

But that's the issue - people with exotic environments need to use non-default config, and that config is trivial right now. Until that becomes standard, it makes no sense to change the defaults.

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 2, 2012

I wonder if it would be possible to change it dynamically. Is it possible to determine if a monitor is using 2x assets with javascript?

Is the issue that rendering plots with double resolution would be computationally expensive?

@minrk
Copy link
Member

minrk commented Aug 2, 2012

Mostly just a big waste of data. If you want bigger figures, set the figure size to be bigger. One line of config, or one call at runtime. It makes no sense to change defaults to serve a tiny minority to the detriment of everyone else.

@Carreau
Copy link
Member

Carreau commented Aug 2, 2012

I have a retina MacBook Pro, and I've noticed that several of the images in IPython need to be updated to higher resolution so that they don't look bad. What I've noticed so far:

The icons in the notebook, including the reload icon on the dashboard and all the toolbar icons in the notebook itself

Except for IPython notebook , we depend on jQuery for all that.

(maybe they'll switch to a custom font for icon like github does...)

The IPython icon in the header of the qtconsole
The IPython icon itself from the qtconsole (Note that Mac OS X supports extremely high resolution application icons)
The IPython icon in the quit screen in the qtconsole

I can't make much higher resolution than a SVG..we'll have to wait for Qt Update.
I think the only .icns we have (for notebook file icons in 0.14 dev) are 1024x1024

The default resolution for inline plots in both the qtconsole and the notebook is too low (this one is probably the most important).
The favicon for the notebook.

Note that you'll need to use Safari to test the notebook, as Chrome doesn't fully support retina graphics yet (Chrome Canary supports retina graphics, but not retina favicons).

You are not up to date with your stable version of chrome, [it's still hidden in the page](http://chrome.blogspot.fr/2012/07/new-senses-for-web.html?utm_source=feedburner&utm_medium=feed&utm_campaign=Feed:+blogspot/Egta+(Google+Chrome+Blog) ...

Sorry if this seems low priority, but you really have to use a retina device to see just how bad non-retina things look.

If you want to make a PR that fixes some of that...

Have fun with your hi-res screen.

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 2, 2012

(maybe they'll switch to a custom font for icon like github does...)

Quite a few of the icons are already in unicode.

You are not up to date with your stable version of chrome, it's still hidden in the page ...

Ah. I stopped using the stable version and started using Canary every since I got my new Mac, because I couldn't stand the low res graphics (or the alternative, which is using Safari). And anyway, you still need Canary for the high-res pdf viewer.

I think the only .icns we have (for notebook file icons in 0.14 dev) are 1024x1024

The largest notebook icon that I see (in docs/resources/) appears to only be 512 x 512.

Have fun with your hi-res screen.

Yes, it's pretty awesome, except you notice every low-res graphic on everything (hence my opening this issue).

So I guess the only thing that can actually be done here is the favicon.

@sergeyk
Copy link

sergeyk commented Aug 8, 2012

I have been unable to figure out how to fix this through config.
The dpi setting of the figure seems to have no effect on the inline-rendered plot.
Compare 72 dpi
with 600 dpi

From web search, I found someone saying that the inline PNG display is hard-coded at 72 dpi to match the qtconsole settings here, and I've found old ipython code that confirms this.

However, I can't find current code that would make this true.
Setting the backend rendering to SVG definitely works, but I'd like to be able to plot millions of points, so that's not a scalable solution.

Please let me know how I can help improve this area.
Looking at low-res graphics on a retina is strangely annoying!

P.S. If anyone has any knowledge of why qtconsole is limited to 72dpi, I'd love to hear it! It looks like crap...

@minrk
Copy link
Member

minrk commented Aug 8, 2012

DPI is just matplotlib config:

matplotlib.rcParams['savefig.dpi'] = 144

The inline backend sets some of these values at load time (including DPI), since the matplotlib defaults don't work well in the notebook/qtconsole. You can change these overrides to whatever you want in your config files, e.g.:

c.InlineBackend.rc = {
    'savefig.dpi' : 144, # 2x
    'figure.facecolor' : 'white',
}

or change the matplotlib rc directly at any time.

P.S. If anyone has any knowledge of why qtconsole is limited to 72dpi, I'd love to hear it! It looks like crap...

I don't know that it is limited. This DPI number is totally specific to matplotlib figures, and if I recall it was chosen so that switching between SVG/PNG doesn't change the size of figures in the QtConsole (the notebook didn't even exist when this code was written!)

If Qt supports 2x rendering, then by all means, proceed with pull requests.

@sergeyk
Copy link

sergeyk commented Aug 8, 2012

Sorry for being thick here, but is there a way to automatically scale down the size of the displayed figure?
With greater DPI it displays proportionally bigger.

@minrk
Copy link
Member

minrk commented Aug 9, 2012

PNGs do have a DPI field in their header, but I think browsers simply ignore it (almost everything does). It's also possible that matplotlib doesn't even write this header, given how rarely viewers actually pay any attention.

To display a 2x PNG, you can send it as HTML instead of raw PNG data.

Here's a notebook that hopefully demonstrates how to display 2x figs with IPython. Can you let me know if that looks right?

the source

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 9, 2012

Looks good to me.

@minrk
Copy link
Member

minrk commented Aug 9, 2012

Thanks, the 2x figures actually look sharper? I don't want to deal with
logging in/out to test with HiDPI mode.

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 9, 2012

Yes. There's a noticeable difference between the first plot and the later ones. They appear the same size (except for the one that is supposed to be bigger).

@asmeurer
Copy link
Contributor Author

asmeurer commented Aug 9, 2012

Actually, the first one is a little smaller, but that is the case even on my regular lowdpi external display.

minrk added a commit to minrk/ipython that referenced this issue Aug 9, 2012
This lets custom display functions (e.g. HTML) be used without any extra changes (see ipython#2234).

possible downsides:

* The previous code guarantees that only one format is published. If multiple figure formatters are registered, display will send them all.
* If people for some reason disable the type-printers, then they will display the automatic figure display.

Neither of these cases can come up unless people are messing with the formatters, and I think the first is actually an improvement.
@minrk
Copy link
Member

minrk commented Aug 9, 2012

#2271 should make the post_execute workaround unnecessary.

@minrk
Copy link
Member

minrk commented Aug 9, 2012

That's simply because the actual size matplotlib puts out is not, in fact, strictly dpi * figsize. But it's close enough. If the matplotlib math can be more precisely reproduced (or just inspect the PNG header), then it should match.

If there's better CSS for 2x graphics than the most primitive thing I just tossed together, then this can obviously be improved.

But the important illustration here is that you can, yourself, enable 2x graphics with no changes to IPython. A more concise version of the code here can just go in a script in your profile startup dir.

And ultimately, we can support InlineBackend.figure_format = 'png2x'.

fperez added a commit that referenced this issue Aug 10, 2012
use display instead of send_figure in inline backend hooks

This lets custom display functions (e.g. HTML) be used without any extra changes (see #2234).

possible downsides:

* The previous code guarantees that only one format is published. If multiple figure formatters are registered, display will send them all.
* If people for some reason disable the type-printers, then they will display the automatic figure display.

**Backwards-incompatible change**

Note that `IPython.zmq.pylab.backend_inline.send_figure` has been removed, as `display()` can do the same job and we avoid an unnecessary special-case code path.
Carreau pushed a commit to Carreau/ipython that referenced this issue Sep 5, 2012
This lets custom display functions (e.g. HTML) be used without any extra changes (see ipython#2234).

possible downsides:

* The previous code guarantees that only one format is published. If multiple figure formatters are registered, display will send them all.
* If people for some reason disable the type-printers, then they will display the automatic figure display.

Neither of these cases can come up unless people are messing with the formatters, and I think the first is actually an improvement.
@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

I'm willing to add png2x. I need it too. I will make a PR soon.

@minrk
Copy link
Member

minrk commented Apr 11, 2013

The right way to do this is to add support for image shape information in metadata, as a part of IPEP 13. I've been using this extension to get 2x images in the meantime, but it's definitely the wrong way to do it for real.

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

I was going to do exactly what you did in the extension. Guess it's a bad idea then.

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

Could you point me to the javascript that handle the image display. I kinda see that it puts in width and height when it's displayed.
Screen Shot 2013-04-10 at 9 17 51 PM

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

The fix might be as easy as having this display respect the dpi.

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

I can also confirm that savefig indeed put DPI in the header. It seems that is img tag just doesn't respect it. Is there any reason the width and height are fixed to that number?

@Carreau
Copy link
Member

Carreau commented Apr 11, 2013

I think it's an artifact of being able to resize images from the corner.

Le jeudi 11 avril 2013, Piti Ongmongkolkul a écrit :

I can also confirm that savefig indeed put DPI in the header. It seems
that is img tag just doesn't respect it. Is there any reason the width and
height are fixed to that number?


Reply to this email directly or view it on GitHubhttps://github.com//issues/2234#issuecomment-16216545
.

@piti118
Copy link
Contributor

piti118 commented Apr 11, 2013

Yes it definitely is that dblclick_to_reset_size (around line 432 in outputarea.js. But

  1. I couln't find where in the javascript that write the original style.
  2. Although it's easy enough to get screen density pixel ratio.
        var dpr = 1;
        console.debug(['aha', img, dpr])
        if(window.devicePixelRatio !== undefined) dpr = window.devicePixelRatio;

But I couldn't find the way to get image dpi.If I could do this then problem is solved.

@piti118
Copy link
Contributor

piti118 commented Apr 13, 2013

However this will make the default image looks tiny(but sharp) too

@piti118
Copy link
Contributor

piti118 commented Apr 13, 2013

Hmm I'm thinking about this I think the right way is actually the other way around. Instead of having the ipython kernel send the metadata, it should be the browser sending metadata along with the code saying that hey I want hires image.

browser --> (code, hires=True) --> kernel --> (hires image) --> browser

then browser knows that itself is a hires one and then display with appropriate scaling
http://developer.apple.com/library/safari/#documentation/NetworkingInternet/Conceptual/SafariImageDeliveryBestPractices/Introduction/Introduction.html

@Carreau
Copy link
Member

Carreau commented Apr 13, 2013

No, because when sharing the notebook or converting it, or having multiple
client .... You would have inconsistencies.

Le samedi 13 avril 2013, Piti Ongmongkolkul a écrit :

Hmm I'm thinking about this I think the right way is actually the other
way around. Instead of having the ipython kernel send the metadata, it
should be the browser sending metadata along with the code saying that hey
I want hires image.

browser --> (code, hires=True) --> kernel --> (hires image) --> browser

then browser knows itself that i'm a hires one and then display with
appropriate scaling

http://developer.apple.com/library/safari/#documentation/NetworkingInternet/Conceptual/SafariImageDeliveryBestPractices/Introduction/Introduction.html


Reply to this email directly or view it on GitHubhttps://github.com//issues/2234#issuecomment-16333703
.

@minrk
Copy link
Member

minrk commented Apr 13, 2013

it is a totally generic use case to specify the size of an image when you display it - 2x graphics is just a simple subset of that. We need to support that use case, so we might as well implement the 2x figures in the same way.

@piti118
Copy link
Contributor

piti118 commented Apr 14, 2013

OK. I'm trying to implement it with metadata but it looks like this will require some change to core API. So I want to consult you first:

  1. Current display doesn't pass metadata to publish. This is easy fix. I add it as an additional keyword argument metadata.
  2. A more serious issue is that formatters doesn't provide a mechanism to compute metadata from the provided data. There are couple ways to implement/workaround this. This is necessary for passing png size metadata.
    1. Workaround: get the size in pixel from Figure object and add metadata before calling display. Ex in backend_inline.show()

       fig = figure_manager.canvas.figure
       widthpx, heightpx = fig.get_size_in_pixel() # not sure if there is one
       display(figure_manager.canvas.figure, metadata={'width':widthpx, 'height':heightpx})
    2. Allow formatter to return metadata. This is a more proper way to do it I think but will require API change. Currently, formatters.for_type(self, typ, func) only accept fund that compute the raw data but no mechanism to compute metadata.

      We could have func return a dict of {'raw':'data', 'metadata':{magic}}, but this will require a lot of changes.

      Another choice would be to change the signature of formatters.for_type(self, typ, func, metafunc=None) where metafunc can compute metadata for given output from func output and object. Then formatters.format can return the dictionary that include the key metadata which will be pass along to publish.

What do you think?

@minrk
Copy link
Member

minrk commented Apr 14, 2013

As I proposed in the IPEP I linked above a few times, I think that formatters should be able to either return raw data (as now) or a tuple of length 2 of the form (raw_data, metadata). This should be the only necessary change, and it is fully backwards compatible. All formatters need to do is relay this information up to the DisplayFormatter and DisplayPublisher as they already do with raw data, just including metadata now.

@piti118
Copy link
Contributor

piti118 commented Apr 14, 2013

what if the raw data is tuple of length 2?

@piti118
Copy link
Contributor

piti118 commented Apr 14, 2013

nvm raw data has to be a dict. I'll have a PR soon.

@piti118
Copy link
Contributor

piti118 commented Apr 14, 2013

Actually that was a valid question: what if raw data returned from formatter is a tuple of length 2?

@minrk
Copy link
Member

minrk commented Apr 14, 2013

There are no display formats for which that is valid.

@minrk
Copy link
Member

minrk commented Apr 14, 2013

To perfectly eliminate any ambiguity, we could always define a particular namedtuple, but the display spec, as it is defined today, there are exactly two display data types: text and bytes.

-MinRK

On Apr 14, 2013, at 2:14, Min RK benjaminrk@gmail.com wrote:

There are no display formats for which that is valid.

@minrk
Copy link
Member

minrk commented Apr 16, 2013

@piti118 are you still working on this? It was next on my todo list, so I can probably get it done fairly quickly. Or I can just do the 'add metadata support' to the display formatters step from my IPEP, which is the more sensitive and pervasive part, and then leave the task of using that logic to get 2x matplotlib figures to you.

@piti118
Copy link
Contributor

piti118 commented Apr 16, 2013

I think it's better to leave the core part to you (I do have partial implementation which I could get it done when i'm free.). I like the plan your propose I can do DPR part.

@minrk
Copy link
Member

minrk commented Apr 16, 2013

Sounds good - I'll have a PR for the metadata stuff this afternoon, and you can base the matplotlib work on that. I know it can be uncomfortable to work on the bits deep inside IPython.

@minrk
Copy link
Member

minrk commented Jul 4, 2013

closed by #3381.

@minrk minrk closed this as completed Jul 4, 2013
@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

Images still look bad in the qtconsole. Take for example

from sympy import init_session
init_session()
sqrt(x)

in the development version of SymPy (make sure you have LaTeX installed). I increased the resolution, but that just made the image bigger :(

@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

It's the same in the notebook too, actually, but at least the browser antialiases it.

Probably this is SymPy's fault, because we probably don't specify a bound for the picture, so it just shows it at the normal resolution. How do you specify a bound for the image, so that we can just provide a high resolution version and have it scaled down? (sorry, I didn't actually write the SymPy code, so maybe @thisch should look here too)

@minrk
Copy link
Member

minrk commented Jul 4, 2013

Ah, I was only thinking of the notebook. I don't think we have any plan to do retina enhancements to the qtconsole, most of which limitations I think are really those of Qt itself.

As for images coming from sympy, the main thing is that we have exposed the ability to specify the display size of images. If sympy wants 2x display, it will have to use this - it is not going to be automatic.

@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

What is that API? All I see are changes to Image.

@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

And regarding retina qtconsole, could we at least have an open issue for it?

@minrk
Copy link
Member

minrk commented Jul 4, 2013

Where is the sympy code for image-based output? Does it use _repr_png_? If so, reprs can now return a tuple of the raw data and the metadata, so if you want display size to be other than the natural size, you would do:

def _repr_png_(self):
    metadata = dict(width=display_width, height=display_height)
    return pngdata, metadata

And regarding retina qtconsole, could we at least have an open issue for it?

Sure, #3537.

@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

https://github.com/sympy/sympy/blob/master/sympy/interactive/printing.py#L130 (comments on any part of that file welcome, actually). The resolution is set at https://github.com/sympy/sympy/blob/master/sympy/interactive/printing.py#L45. I'd like to make that twice as large and scale it down, so that it is always its current size but high resolution on a retina screen.

@asmeurer
Copy link
Contributor Author

asmeurer commented Jul 4, 2013

Since this is LaTeX, we could also probably display a vector graphic, assuming it is supported everywhere on the pipeline. I think the issue is that LaTeX does well with formats like pdf and dvi and browsers do well with formats like svg. Maybe @thisch has an idea.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
This lets custom display functions (e.g. HTML) be used without any extra changes (see ipython#2234).

possible downsides:

* The previous code guarantees that only one format is published. If multiple figure formatters are registered, display will send them all.
* If people for some reason disable the type-printers, then they will display the automatic figure display.

Neither of these cases can come up unless people are messing with the formatters, and I think the first is actually an improvement.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this issue Nov 3, 2014
use display instead of send_figure in inline backend hooks

This lets custom display functions (e.g. HTML) be used without any extra changes (see ipython#2234).

possible downsides:

* The previous code guarantees that only one format is published. If multiple figure formatters are registered, display will send them all.
* If people for some reason disable the type-printers, then they will display the automatic figure display.

**Backwards-incompatible change**

Note that `IPython.zmq.pylab.backend_inline.send_figure` has been removed, as `display()` can do the same job and we avoid an unnecessary special-case code path.
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

No branches or pull requests

5 participants