-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix single pixel markers #695
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
Conversation
Thanks Mike. This is really fast! I'll give it a try once I figure out how to check out from github and how not to mess up with my current mpl installation. |
@@ -263,9 +274,11 @@ def _set_circle(self, reduction = 1.0): | |||
|
|||
def _set_pixel(self): | |||
self._path = Path.unit_rectangle() | |||
self._transform = Affine2D().translate(-0.5, 0.5) | |||
self._transform = Affine2D().translate(-0.5, -0.5) \ | |||
.scale(0.5, 0.5).translate(0.5, 0.5) | |||
self._snap_threshold = False |
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.
Left this in by accident?
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.
You're quick. I just saw this myself and amended.
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.
Are you reverting this change? I think this is a key to get a single pixel marker.
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.
No -- there was previously a "print" statement here -- that has been removed, but this change will not be.
As I tested just now, the Agg backend works perfectly. Any fix for postscript output? It's still 2x2 squares. Also, in this branch the subplot configuration tool has some problem. When making adjustment and the screen being redrawn, the indicating bar changes from a blue rectangle to a blue triangle. I wish I can attach a screenshot here. |
My bad. pixels in postscript are indeed smaller than before, just not single pixel as the ticker line. |
For the postscript (or any vector backend) there's no way to enforce strictly one pixel. All we can do is have it be one "dot" in whatever the "dots per inch" is set to. By that measurement, I believe it's correct, but let me know if you see otherwise. |
My only concern here is that the center position of the original marker and the new one is shifted by (0.5, 0.5) pixel. |
I did some test. plot([0.012], [0.4], "k,")
xlim(0,1); ylim(0,1) As one of y-ticks is at 0.4, I expected the maker will be aligned with the tickline, but it does not. If I adjust the transformation to Affine2D().translate(-0.5, -0.5).scale(0.5, 0.5) which I tend to think is a right transformation. Also, note that the pixel size in PS backend is a bit larger than the tick line width. I guess this is expected as we are stroking a square of 0.5x0.5 pixel with 1 pixel thick line, i.e., resulting in a 1.5x1.5 pixel square. Maybe we can reduce the square size a bit more? |
Snapping is turned off for single pixel markers. By definition for Agg, the center of a pixel is (0.5, 0.5), so the only way to draw a single pixel is by defining that as the center. We should probably fix the ticks to match this rather than the other way around, though we'll have to see what consequences that has. |
Where are we on this? Does the snapping fix have anything to do with this? |
I've been working with this, but it's kind of a rat's nest down there. I'll see what @leejjoon's snapping fixes do to this. |
@mdboom, should the translation to a pixel center of (0.5, 0.5) be done in the backckend level? I'm not sure why we need that shift for ps or pdf backend. |
… the right thing in the Agg backend. For raster backends, linejoin and cap styles is now correctly passed to and used in the backend.
I think I have a solution here now. Can you test? |
It looks perfect in PS. In the png version, there is no gap between the pixel marker and the tick. Also, it doesn't show up as a single pixel. Then again, I guess that could be due to compression? |
Hmm... it should show up as a single pixel -- PNG uses lossless compression -- that's puzzling. Can you put a picture up somewhere (or e-mail to me?) |
Hmm... I'm stumped. Anything set in your matplotlibrc? |
Same result after getting rid of my modified matplotlibrc file. I should also note that in GTKAgg, the pixel point looks to be located exactly one pixel north of the tick mark (when doing show()). It didn't look like that for TKAgg. |
Dumb question: Are you sure you're running the latest version? You seem to be describing the behavior before today's latest round of changes. |
I am running your branch off based off of v1.1.x, which is what this PR is set for. I will note that I had to do some round-about way of getting it because (even though I have your repo for a remote) I couldn't just fetch your branch. I had to create my own branch (off of v1.1.x) and pull your branch. But the log looks right to me. |
@@ -263,9 +274,9 @@ def _set_circle(self, reduction = 1.0): | |||
|
|||
def _set_pixel(self): | |||
self._path = Path.unit_rectangle() | |||
self._transform = Affine2D().translate(-0.5, 0.5) | |||
self._transform = Affine2D().translate(-0.49999, -0.50001) |
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.
If anything deserves a comment, this does. This just doesn't give me any sort of "warm fuzzies"....
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.
Just added a comment in a new commit.
Ok -- one of my changes didn't make it in. Please try again now -- sorry for the wild goose chase. |
Woot! Success! It looks perfect in show() (GTKAgg and TkAgg), png, ps, pdf and svg. |
Great! Merging... |
Looks like merging this into master needs some work. This is holding up the merging of the doc fixes as well. |
I'll take a look |
Seems to be resolved now. |
did you push your changes up? I don't see any change |
I see it now... |
So do I, and I now have that all taken care of. Thanks! |
The pixel shape is now optimized so it does the right thing in the Agg backend. For raster backends, linejoin and cap styles is now correctly passed to and used in the backend.