Fix clip_path_to_rect, add convenience method on Path object for it #1778

Merged
merged 3 commits into from Feb 25, 2013

2 participants

@mdboom
Matplotlib Developers member

Aternative to #1777. Properly closes the paths returned by clip_to_rect and adds a new clip_to_bbox method to Path that returns a Path object. Adds a test that tests clipping compound paths and multiple subsegments.

@mdboom mdboom Aternative to #1777. Properly closes the paths returned by clip_to_re…
…ct and adds a new clip_to_bbox method to Path that returns a Path object. Adds a test that tests clipping compound paths and multiple subsegments.
d56aaa1
@pelson pelson and 1 other commented on an outdated diff Feb 23, 2013
lib/matplotlib/path.py
lengths = [len(x) for x in args]
total_length = sum(lengths)
vertices = np.vstack([x.vertices for x in args])
vertices.reshape((total_length, 2))
- codes = cls.LINETO * np.ones(total_length)
+ codes = np.ones(total_length, dtype=cls.code_type)
@pelson
Matplotlib Developers member
pelson added a note Feb 23, 2013

Would this be better as np.repeat(cls.LINETO, total_length)? (Though I'm not sure you can specify the dtype that way)

@pelson
Matplotlib Developers member
pelson added a note Feb 23, 2013

Otherwise, it may as well be np.empty?

@mdboom
Matplotlib Developers member
mdboom added a note Feb 24, 2013

Indeed. I think np.empty is the best here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pelson pelson commented on the diff Feb 23, 2013
src/_path.cpp
@@ -1075,7 +1078,10 @@ class _path_module : public Py::ExtensionModule<_path_module>
((double *)pyarray->data)[2*i] = (*p)[i].x;
((double *)pyarray->data)[2*i+1] = (*p)[i].y;
}
- if (PyList_SetItem(py_results, p - results.begin(), (PyObject *)pyarray) != -1)
+ ((double *)pyarray->data)[2*size] = (*p)[0].x;
@pelson
Matplotlib Developers member
pelson added a note Feb 23, 2013

I'm not sure if you always want to do this or not. You could potentially be clipping a line, for which you certainly wouldn't want this terminal vertex. I suppose it is reasonable to say that the algorithm is a Polygon clipping algorithm, and it doesn't make sense to clip a line with it...

@mdboom
Matplotlib Developers member
mdboom added a note Feb 24, 2013

If you were clipping a line, you'd need a different algorithm anyway, since this would cause the line to trace around the edge of the clipping region, rather than just inserting a MOVETO. This algorithm assumes closed polys in, closed polys out. We have an algorithm for clipping lines to boxes in PathClipper, which doesn't currently have a direct way to be used from Python. I think eventually -- and here we really in the realm of new feature and not bug fix -- we should probably generalize this to find the polygons and lines within the path and shunt off to the appropriate algorithm for each, and then recombine directly into a path (since in that case each entry could be either a polygon or a line, we'd have to do the appropriate thing for each).

In the short term, though, we should update the docstring to document the fact that this assumes polygons.

EDIT: Sorry I hadn't read your comment carefully enough -- indeed I think we're saying the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pelson
Matplotlib Developers member

Looks good to me. Just a couple of comments, but I'd be happy enough to merge this early next week.
FWIW, you've pretty much implemented the Sutherland-Hodgman algorithm here, except it has been reduced to a path-bbox clip - I'm pretty sure it wouldn't be too much work to do a path-path clip as I have done in https://github.com/SciTools/cartopy/pull/230/files#L1R39. I don't propose we do that here (or in v1.2.x) but it is something we could bear in mind if anybody else finds this functionality useful.

Good stuff @mdboom!

@pelson pelson merged commit 41fcfd2 into matplotlib:v1.2.x Feb 25, 2013

1 check failed

Details default The Travis build could not complete due to an error
@mdboom mdboom deleted the mdboom:clip_path_to_rect branch Aug 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment