improve snapping algorithm #726
Conversation
|
@mdboom: Can you check this? |
|
Looks right. Interestingly, it doesn't seem to cause any additional test failures -- but as we drift further and further from the baseline images, we'll probably want to update them at some point. |
|
Do we want this for v1.1.1? |
|
Given that 1.1.1 is imminent, I would think no. This feels like a pretty pervasive change to me -- I would hate to later discover things like images aligning correctly to the edges of the axes breaks in some corner cases because of this change. |
|
The commit f4f2a1a introduces one pixel offset between lines and markers for integer coordinates, e.g., (0, 0). I believe that this is related to how maker paths are snapped. snap_t marker_path_snapped(marker_path_transformed,
gc.snap_mode,
marker_path.total_vertices(),
gc.linewidth);If we consider a marker path (0, 0.5) - (0, 1.5) and linewidth of 1, I believe that the path actually snapped and becomes (-0.5, 0.5) - (0.5, 0.5) as round(-0.5) returns -1, while it is supposed to (or used to) become (0.5, 0.5) - (0.5, 1.5). I think one option is just to translate the marker path by (0.5, 0.5) before the snapping (cbc5fed). But @mdboom may have different (and better) idea. |
|
Closing -- this was merged as part of #695. |
|
Ok -- I see. This PR was incorporated in #695, but I missed the fact that it had problems. I'm looking into this now and am reopening #695 to do the work over there. EDIT: @leejjoon: you can see a solution to the problems presented here on top of #695 in #734. Can you test with all of your use cases? It seems to a) have pixel markers align with the ticks correctly, b) have the ticks align with the axes rectangle correctly, c) have the clipbox align with the axes rectangle, d) not distort the shape of the markers and e) makes evenly spaced data look reasonably even. |
I think the current snapping gives somewhat wrong result.
For example, consider m_snap_value=0.5 and (x, y) = (0.45, 0.55). This is supposed to be snapped to (0.5, 0.5) but the above code gives (0.5, 1.5). And I think the right thing to do is subtract m_snap_value before rounding.