Repository navigation
FIX: do not skip isolated points in vector outputs - #8297
Conversation
tacaswell
added
the
Release critical
|
|
||
|
|
||
| @image_comparison(baseline_images=['nan_path'], style='default', | ||
| remove_text=True,extensions=['pdf', 'svg', 'eps', 'png']) |
| unsigned vertex(double *x, double *y) | ||
| { | ||
| unsigned code; | ||
| bool emit_moveto = false; |
There was a problem hiding this comment.
Our C/C++ code uses spaces as well, not tabs.
There was a problem hiding this comment.
gah! emacs has betrayed me!
| // seen at least one command ? | ||
| // if so, shove it in the queue if in clip box | ||
| if (m_moveto && m_has_init && | ||
| m_lastX >= m_cliprect.x1 && |
There was a problem hiding this comment.
I prefer reversing the arguments here so that it looks similar to Python's chained form.
There was a problem hiding this comment.
This matches the block further down, would rather stay consistent with that.
| m_lastX <= m_cliprect.x2 && | ||
| m_lastY >= m_cliprect.y1 && | ||
| m_lastY <= m_cliprect.y2) { | ||
| // push the last moveto onto the queue |
There was a problem hiding this comment.
moveto -> move to (or maybe the reverse for the other places would be clearer.)
There was a problem hiding this comment.
Actually, this comment is kind of redundant with the one above the if.
| m_initY = m_lastY = *y; | ||
| m_has_init = true; | ||
| m_moveto = true; | ||
| // if the last command was move to exit the loop to emit the code |
There was a problem hiding this comment.
I mean, this is pretty clearly what it does already, so it's kind of a redundant comment. Would be better as a why instead of a what.
|
Have not bisected, but looking at the blame I suspect this was broken by #5911 so broken from 1.5.2 on. |
bb0d1a9 to
27d4596
Compare
Fixed bug in path interaction code where multiple moves in a row (which correspond to isolated points surrounded by nans after they have been filtered from the path) would be discarded which resulted in points not being plotted. closes matplotlib#7293
27d4596 to
5f54543
Compare
| m_lastX <= m_cliprect.x2 && | ||
| m_lastY >= m_cliprect.y1 && | ||
| m_lastY <= m_cliprect.y2) { | ||
| // push the last moveto onto the queue |
There was a problem hiding this comment.
I think this comment is still redundant.
There was a problem hiding this comment.
I wrote this as comments to my self while understanding this. I do not see a problem with leaving them.
|
Thanks @tacaswell ! |
|
This PR was about 2 LoC / hr 😆 (of changes to the source, not the surrounding tests). |
Fixed bug in path interaction code where multiple moves in a
row (which correspond to isolated points surrounded by nans after they
have been filtered from the path) would be discarded which resulted in
points not being plotted.
closes #7293