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

Add clip_pixels to Image and automatically clip RGB visualisations #761

Merged
merged 3 commits into from Jan 10, 2017

Conversation

Projects
None yet
4 participants
@grigorisg9gr
Member

grigorisg9gr commented Nov 23, 2016

Till now if the image is out of range, then the visualisation is weird. Such a case might appear in filtering or even with value scaling, e.g.

import menpo.io as mio
im = mio.import_builtin_asset.lenna_png()
im.pixels *= 1.2
im.view()

This proposes one optional variable in the .view() methods to allow the clipping inside the image viewer (the original values are untouched).

@nontas

This comment has been minimized.

Member

nontas commented Nov 23, 2016

Actually this issue is also discussed in #704.

The issue only applies in the case of RGB images. For example, first define this function:

%matplotlib inline
import menpo.io as mio
from menpo.feature import fast_dsift
import matplotlib.pyplot as plt

im = mio.import_builtin_asset.lenna_png()

def scale_pixel_range(im, channels=None):
    # Visualize original
    r = im.pixels_range()
    print("Original pixels: {:.2f}, {:.2f}".format(r[0], r[1]))
    plt.subplot(121)
    im.view(channels=channels)
    
    # Scale pixels
    im_scaled = im.copy()
    im_scaled.pixels *= 1.6
    im_scaled.pixels -= 2 * im_scaled.pixels.min()
    
    # Visualize scaled
    r = im_scaled.pixels_range()
    print("Scaled pixels: {:.2f}, {:.2f}".format(r[0], r[1]))
    plt.subplot(122)
    im_scaled.view(channels=channels)

Let's run it on the RGB image:

scale_pixel_range(im)

screenshot from 2016-11-23 19-13-48

Now on a greyscale image:

scale_pixel_range(im.as_greyscale())

screenshot from 2016-11-23 19-15-05

And final on a Dense DIFT image:

scale_pixel_range(fast_dsift(im), channels=0)

screenshot from 2016-11-23 19-16-35

So, given that it only affects the RGB images, I'm not sure what's the best approach.

Maybe we should by default normalize the pixels inside menpo.visualize.base.ImageViewer in case we detect an RGB image that needs to be visualized as RGB (and not each channel separately). And then this could raise a warning that the pixels were actually rescaled. This channel check is already performed by the _parse_channels() method of ImageViewer. I don't really think that there should be a check_value_range argument, since it only applies for the RGB case.

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Nov 25, 2016

Isn't this just Matplotlib behaving badly? Can't we see if there are options in matplotlib to make it behave better?

@grigorisg9gr

This comment has been minimized.

Member

grigorisg9gr commented Nov 26, 2016

Well, i had spent some time with trying to configure matplotlib as well, but the documentation is mostly for normalising the values (scaling them) rather than clipping them.

There are the set_over and set_under methods, e.g. http://matplotlib.org/examples/pylab_examples/image_masked.html , http://stackoverflow.com/a/11386204/1716869 , but I didn't manage it to work well with rgb image and jet colourmap (or None as is the default).

@nontas

This comment has been minimized.

Member

nontas commented Nov 29, 2016

I suggest we do the following. Add a clip_pixels() method in our Image class, similar to the rescale_pixels() one. Then, in case an image has 3 channels, we check if the pixels are in range [0, 1] and if they are not then we clip them and raise a warning.

What do you think?

@jabooth

This comment has been minimized.

Member

jabooth commented Nov 29, 2016

@nontas I think there is general utility in clipping pixels beyond just for visualisation, so I like adding .clip_pixels() to the API of Image. However we then use the new API or matplotlib directly to solve the clipping in the visualisation, I don't have strong feelings on (whatever is simplest/fastest/most reliable).

@nontas

This comment has been minimized.

Member

nontas commented Nov 29, 2016

I forgot to mention that I had a look again and this issue cannot be solved with matplotlib. Specifically, the documentation of matplotlib.imshow mentions that this

norm = Normalize(vmin=0., vmax=1., clip=True)
plt.imshow(image, norm=norm, vmin=0, vmax=1)

should work, but it doesn't!

@jabooth jabooth referenced this pull request Nov 29, 2016

Merged

3D Visualization Upgrade #762

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Nov 30, 2016

Fair enough! I can agree to clipping - be careful with types though. I would prefer to explicitly add clipping to image and then we just have a bool here for clipping that when set to true just clips it into RGB/0..1 range

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Dec 8, 2016

Did we agree to change this to clip_pixels?

@nontas

This comment has been minimized.

Member

nontas commented Dec 9, 2016

@grigorisg9gr do you agree?

@grigorisg9gr grigorisg9gr force-pushed the grigorisg9gr:visualisation_clipping branch from 768847c to 42cce28 Dec 9, 2016

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Jan 9, 2017

@grigorisg9gr Sorry I missed that you updated this - does this fix your visualization issues?

@grigorisg9gr

This comment has been minimized.

Member

grigorisg9gr commented Jan 9, 2017

Yes it does. At least the issue reported here.

@patricksnape patricksnape changed the title from Add the clip option in image visualisations. to Add clip_pixels to Image and automatically clip RGB visualisations Jan 10, 2017

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Jan 10, 2017

@jabooth Want to merge this before or after the dev tag?

@jabooth

This comment has been minimized.

Member

jabooth commented Jan 10, 2017

@patricksnape this can come in now it seems, looks fine to me?

@patricksnape go ahead and merge if you agree.

@patricksnape patricksnape merged commit e168879 into menpo:master Jan 10, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
macOS MenpoBot Jenkins build passed
Details

@patricksnape patricksnape deleted the grigorisg9gr:visualisation_clipping branch Jan 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment