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

Make colorbar draw edge with facecolor around the faces. #1178

Closed
wants to merge 2 commits into from

Conversation

jenshnielsen
Copy link
Member

This was changed when #901 and was merged, to allow different edgecolors,
due to the way get_edgecolors behave. This restores the
original behavior of colorbar in both mpl and the axisgrid1 toolkit by setting the edgecolor to 'face'

Without the edge color set to facecolor the colorbars have white stripes in pdf. See the attached files for an example. The rendering in pdf might be viewer dependent. The png screenshots illustrates the rendering in evince on linux

Pdf before fix
http://ubuntuone.com/7YrairHdLJAaskzIC215KI
Pdf after fix
http://ubuntuone.com/1qNZvSFlXzZNRDqMAafxBJ

Screenshot before
http://ubuntuone.com/3RRIM3P6j3rqgLnXpTI2c1
Screenshot after
http://ubuntuone.com/5Dd7x4gwRXAWyPswZRfOdk

…nged when matplotlib#901 and was

merged, to allow different edgecolors, due to the way get_edgecolors behave. This restors the
original behavior of colorbar in both mpl and the axisgrid1 toolkit by setting the edgecolor to 'face'
@travisbot
Copy link

This pull request fails (merged d631dfd into cf7618c).

@dmcdougall
Copy link
Member

I've always wondered what caused those. Thanks for looking into this.

@efiring
Copy link
Member

efiring commented Aug 31, 2012

Caution: you have identified a real problem and a possible solution, but it might not be the best one. Coloring the edges tends to have bad effects when alpha is not 1; and even when alpha is 1, it leads to a subtle position shift. Are you sure that prior to the #901 merge, the edgecolor was being set to "face"?

@jenshnielsen
Copy link
Member Author

Yes I am. If you look at the draw_quad_mesh code in backend_bases.py before the merge of #901 you can see that only two options exist. Either the edge is draw in black if showedges is true otherwise edgecolor is set equal to facecolor. We might consider changing the default in axes.py back to none and only change it to face in the colorbars.

@travisbot
Copy link

This pull request fails (merged 811f917 into cf7618c).

@efiring
Copy link
Member

efiring commented Sep 1, 2012

This problem of boundary effects keeps cropping up, and your proposed solution will help in some circumstances and hurt in others. Fundamentally, I think that not drawing edges is the correct procedure, and so I think the problem is in the implementation of the drawing. This may be partly in mpl, and partly in external renderers such as pdf viewers. Wherever we can find places in mpl where we are handling this wrong, such as by making patches overlap by one pixel, or have a gap of one pixel, we should fix it.

@efiring
Copy link
Member

efiring commented Sep 1, 2012

This is a problem in the renderer (the viewer), in the sense that when displaying the pdf, the stripes move around as the figure is resized; so the stripes are not embedded in the pdf. It seems to be a truncation error; it looks like aliasing.

@jenshnielsen
Copy link
Member Author

This solution also has problems with colorbar extensions rendering outside the black edges. I can confirm that rebuilding libpoppler with the fix indicated in #1188 fixes the problem here. If anyone are interested I build a new version of libpoppler here with the patch for ubuntu 12.04. https://launchpad.net/~jenshnielsen/+archive/ppa Other viewers that seem to suffer from the same problems are svg rendering in firefox and chrome as well as pdfs in mozillas pdf.js.

jenshnielsen added a commit to jenshnielsen/matplotlib that referenced this pull request Sep 23, 2012
the colorbar. Resolve problems with white lines in colorbars.
Not enabled by default.  See matplotlib#1178 and matplotlib#1188
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.

None yet

4 participants