From 27d119900dd30846548f21d141f2af3740dbeb47 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 19 May 2016 08:15:33 -0400 Subject: [PATCH 1/4] Revert 2c1bc6b for performance --- lib/matplotlib/tests/test_path.py | 1 - src/_path.h | 16 ++++++++-------- src/_path_wrapper.cpp | 4 ++-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/matplotlib/tests/test_path.py b/lib/matplotlib/tests/test_path.py index 22035b3dc501..6543137c0e1f 100644 --- a/lib/matplotlib/tests/test_path.py +++ b/lib/matplotlib/tests/test_path.py @@ -39,7 +39,6 @@ def test_contains_points_negative_radius(): expected = [True, False, False] result = path.contains_points(points, radius=-0.5) - assert result.dtype == np.bool assert np.all(result == expected) diff --git a/src/_path.h b/src/_path.h index 6b1b3d1f7053..8ba732509366 100644 --- a/src/_path.h +++ b/src/_path.h @@ -79,7 +79,7 @@ struct XY template void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &inside_flag) { - bool yflag1; + uint8_t yflag1; double vtx0, vty0, vtx1, vty1; double tx, ty; double sx, sy; @@ -89,13 +89,13 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins size_t n = points.size(); - std::vector yflag0(n); - std::vector subpath_flag(n); + std::vector yflag0(n); + std::vector subpath_flag(n); path.rewind(0); for (i = 0; i < n; ++i) { - inside_flag[i] = false; + inside_flag[i] = 0; } unsigned code = 0; @@ -118,7 +118,7 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins // get test bit for above/below X axis yflag0[i] = (vty0 >= ty); - subpath_flag[i] = false; + subpath_flag[i] = 0; } } @@ -164,7 +164,7 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins // Haigh-Hutchinson's different polygon inclusion // tests. if (((vty1 - ty) * (vtx0 - vtx1) >= (vtx1 - tx) * (vty0 - vty1)) == yflag1) { - subpath_flag[i] = subpath_flag[i] ^ true; + subpath_flag[i] ^= 1; } } @@ -196,8 +196,8 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins subpath_flag[i] = subpath_flag[i] ^ true; } } - inside_flag[i] = inside_flag[i] || subpath_flag[i]; - if (inside_flag[i] == false) { + inside_flag[i] |= subpath_flag[i]; + if (inside_flag[i] == 0) { all_done = false; } } diff --git a/src/_path_wrapper.cpp b/src/_path_wrapper.cpp index 4a4fc2db1eb3..45c1e288e86c 100644 --- a/src/_path_wrapper.cpp +++ b/src/_path_wrapper.cpp @@ -93,7 +93,7 @@ static PyObject *Py_points_in_path(PyObject *self, PyObject *args, PyObject *kwd } npy_intp dims[] = { (npy_intp)points.size() }; - numpy::array_view results(dims); + numpy::array_view results(dims); CALL_CPP("points_in_path", (points_in_path(points, r, path, trans, results))); @@ -152,7 +152,7 @@ static PyObject *Py_points_on_path(PyObject *self, PyObject *args, PyObject *kwd } npy_intp dims[] = { (npy_intp)points.size() }; - numpy::array_view results(dims); + numpy::array_view results(dims); CALL_CPP("points_on_path", (points_on_path(points, r, path, trans, results))); From 2c5d2927688870f1f3cdfdcb06f85dd3e24a82ba Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 19 May 2016 08:16:44 -0400 Subject: [PATCH 2/4] Use ( , ) instead of [] for indexing Fix #6444 --- src/_path.h | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/_path.h b/src/_path.h index 8ba732509366..b8cee3629410 100644 --- a/src/_path.h +++ b/src/_path.h @@ -18,6 +18,7 @@ #include "path_converters.h" #include "_backend_agg_basic_types.h" +#include "numpy_cpp.h" struct XY { @@ -112,7 +113,7 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins sy = vty0 = vty1 = y; for (i = 0; i < n; ++i) { - ty = points[i][1]; + ty = points(i, 1); if (std::isfinite(ty)) { // get test bit for above/below X axis @@ -135,8 +136,8 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins } for (i = 0; i < n; ++i) { - tx = points[i][0]; - ty = points[i][1]; + tx = points(i, 0); + ty = points(i, 1); if (!(std::isfinite(tx) && std::isfinite(ty))) { continue; @@ -183,8 +184,8 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins all_done = true; for (i = 0; i < n; ++i) { - tx = points[i][0]; - ty = points[i][1]; + tx = points(i, 0); + ty = points(i, 1); if (!(std::isfinite(tx) && std::isfinite(ty))) { continue; @@ -242,11 +243,10 @@ template inline bool point_in_path( double x, double y, const double r, PathIterator &path, agg::trans_affine &trans) { - std::vector point; - std::vector > points; - point.push_back(x); - point.push_back(y); - points.push_back(point); + npy_intp shape[] = {1, 2}; + numpy::array_view points(shape); + points(0, 0) = x; + points(0, 1) = y; int result[1]; result[0] = 0; @@ -285,11 +285,10 @@ template inline bool point_on_path( double x, double y, const double r, PathIterator &path, agg::trans_affine &trans) { - std::vector point; - std::vector > points; - point.push_back(x); - point.push_back(y); - points.push_back(point); + npy_intp shape[] = {1, 2}; + numpy::array_view points(shape); + points(0, 0) = x; + points(0, 1) = y; int result[1]; result[0] = 0; From 7976970830a10e5572e114fb1da2d4458ebd01c9 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 19 May 2016 08:18:40 -0400 Subject: [PATCH 3/4] Rename operator[] to subarray... Since we now know the performance is so bad, this should discourage its use and flag it as something special. Also, use () in more places --- src/_backend_agg.h | 26 +++++++++++++------------- src/_image.h | 28 ++++++++++++---------------- src/_path.h | 31 +++++++++++++++---------------- src/_png.cpp | 2 +- src/numpy_cpp.h | 4 ++-- 5 files changed, 43 insertions(+), 48 deletions(-) diff --git a/src/_backend_agg.h b/src/_backend_agg.h index 4816cd826f60..2467084eb5cc 100644 --- a/src/_backend_agg.h +++ b/src/_backend_agg.h @@ -961,13 +961,13 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc, typename PathGenerator::path_iterator path = path_generator(i); if (Ntransforms) { - typename TransformArray::sub_t subtrans = transforms[i % Ntransforms]; - trans = agg::trans_affine(subtrans(0, 0), - subtrans(1, 0), - subtrans(0, 1), - subtrans(1, 1), - subtrans(0, 2), - subtrans(1, 2)); + int it = i % Ntransforms; + trans = agg::trans_affine(transforms(it, 0, 0), + transforms(it, 1, 0), + transforms(it, 0, 1), + transforms(it, 1, 1), + transforms(it, 0, 2), + transforms(it, 1, 2)); trans *= master_transform; } else { trans = master_transform; @@ -989,13 +989,13 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc, trans *= agg::trans_affine_translation(0.0, (double)height); if (Nfacecolors) { - typename ColorArray::sub_t facecolor = facecolors[i % Nfacecolors]; - face.second = agg::rgba(facecolor(0), facecolor(1), facecolor(2), facecolor(3)); + int ic = i % Nfacecolors; + face.second = agg::rgba(facecolors(ic, 0), facecolors(ic, 1), facecolors(ic, 2), facecolors(ic, 3)); } if (Nedgecolors) { - typename ColorArray::sub_t edgecolor = edgecolors[i % Nedgecolors]; - gc.color = agg::rgba(edgecolor(0), edgecolor(1), edgecolor(2), edgecolor(3)); + int ic = i % Nedgecolors; + gc.color = agg::rgba(edgecolors(ic, 0), edgecolors(ic, 1), edgecolors(ic, 2), edgecolors(ic, 3)); if (Nlinewidths) { gc.linewidth = linewidths(i % Nlinewidths); @@ -1274,8 +1274,8 @@ inline void RendererAgg::draw_gouraud_triangles(GCAgg &gc, bool has_clippath = render_clippath(gc.clippath.path, gc.clippath.trans); for (int i = 0; i < points.dim(0); ++i) { - typename PointArray::sub_t point = points[i]; - typename ColorArray::sub_t color = colors[i]; + typename PointArray::sub_t point = points.subarray(i); + typename ColorArray::sub_t color = colors.subarray(i); _draw_gouraud_triangle(point, color, trans, has_clippath); } diff --git a/src/_image.h b/src/_image.h index 0385f45fd9aa..8a9bb8626ad4 100644 --- a/src/_image.h +++ b/src/_image.h @@ -126,14 +126,12 @@ Image *from_color_array(ArrayType &array, bool isoutput) for (size_t rownum = 0; rownum < (size_t)array.dim(0); rownum++) { for (size_t colnum = 0; colnum < (size_t)array.dim(1); colnum++) { - typename ArrayType::sub_t::sub_t color = array[rownum][colnum]; - - r = color(0); - g = color(1); - b = color(2); + r = array(rownum, colnum, 0); + g = array(rownum, colnum, 1); + b = array(rownum, colnum, 2); if (rgba) { - alpha = color(3); + alpha = array(rownum, colnum, 3); } *buffer++ = int(255 * r); // red @@ -164,13 +162,12 @@ Image *frombyte(ArrayType &array, bool isoutput) for (size_t rownum = 0; rownum < (size_t)array.dim(0); rownum++) { for (size_t colnum = 0; colnum < (size_t)array.dim(1); colnum++) { - typename ArrayType::sub_t::sub_t color = array[rownum][colnum]; - r = color(0); - g = color(1); - b = color(2); + r = array(rownum, colnum, 0); + g = array(rownum, colnum, 1); + b = array(rownum, colnum, 2); if (rgba) { - alpha = color(3); + alpha = array(rownum, colnum, 3); } *buffer++ = r; // red @@ -295,13 +292,12 @@ Image *pcolor(CoordinateArray &x, a10 = (1.0 - alpha) * beta; a11 = 1.0 - a00 - a01 - a10; - typename ColorArray::sub_t::sub_t start00 = d[rowstart[i]][colstart[j]]; - typename ColorArray::sub_t::sub_t start01 = d[rowstart[i]][colstart[j] + 1]; - typename ColorArray::sub_t::sub_t start10 = d[rowstart[i] + 1][colstart[j]]; - typename ColorArray::sub_t::sub_t start11 = d[rowstart[i] + 1][colstart[j] + 1]; for (size_t k = 0; k < 4; ++k) { position[k] = - start00(k) * a00 + start01(k) * a01 + start10(k) * a10 + start11(k) * a11; + d(rowstart[i], colstart[j], k) * a00 + + d(rowstart[i], colstart[j] + 1, k) * a01 + + d(rowstart[i] + 1, colstart[j], k) * a10 + + d(rowstart[i] + 1, colstart[j] + 1, k) * a11; } position += 4; } diff --git a/src/_path.h b/src/_path.h index b8cee3629410..71da86b0f42c 100644 --- a/src/_path.h +++ b/src/_path.h @@ -384,13 +384,13 @@ void get_path_collection_extents(agg::trans_affine &master_transform, for (i = 0; i < N; ++i) { typename PathGenerator::path_iterator path(paths(i % Npaths)); if (Ntransforms) { - typename TransformArray::sub_t subtrans = transforms[i % Ntransforms]; - trans = agg::trans_affine(subtrans(0, 0), - subtrans(1, 0), - subtrans(0, 1), - subtrans(1, 1), - subtrans(0, 2), - subtrans(1, 2)); + size_t ti = i % Ntransforms; + trans = agg::trans_affine(transforms(ti, 0, 0), + transforms(ti, 1, 0), + transforms(ti, 0, 1), + transforms(ti, 1, 1), + transforms(ti, 0, 2), + transforms(ti, 1, 2)); } else { trans = master_transform; } @@ -436,13 +436,13 @@ void point_in_path_collection(double x, typename PathGenerator::path_iterator path = paths(i % Npaths); if (Ntransforms) { - typename TransformArray::sub_t subtrans = transforms[i % Ntransforms]; - trans = agg::trans_affine(subtrans(0, 0), - subtrans(1, 0), - subtrans(0, 1), - subtrans(1, 1), - subtrans(0, 2), - subtrans(1, 2)); + size_t ti = i % Ntransforms; + trans = agg::trans_affine(transforms(ti, 0, 0), + transforms(ti, 1, 0), + transforms(ti, 0, 1), + transforms(ti, 1, 1), + transforms(ti, 0, 2), + transforms(ti, 1, 2)); trans *= master_transform; } else { trans = master_transform; @@ -770,8 +770,7 @@ int count_bboxes_overlapping_bbox(agg::rect_d &a, BBoxArray &bboxes) size_t num_bboxes = bboxes.size(); for (size_t i = 0; i < num_bboxes; ++i) { - typename BBoxArray::sub_t bbox_b = bboxes[i]; - b = agg::rect_d(bbox_b(0, 0), bbox_b(0, 1), bbox_b(1, 0), bbox_b(1, 1)); + b = agg::rect_d(bboxes(i, 0, 0), bboxes(i, 0, 1), bboxes(i, 1, 0), bboxes(i, 1, 1)); if (b.x2 < b.x1) { std::swap(b.x1, b.x2); diff --git a/src/_png.cpp b/src/_png.cpp index 05ac9b28960c..974b47d5e56a 100644 --- a/src/_png.cpp +++ b/src/_png.cpp @@ -91,7 +91,7 @@ static PyObject *Py_write_png(PyObject *self, PyObject *args, PyObject *kwds) int channels = buffer.dim(2); std::vector row_pointers(height); for (png_uint_32 row = 0; row < (png_uint_32)height; ++row) { - row_pointers[row] = (png_bytep)buffer[row].data(); + row_pointers[row] = (png_bytep)&buffer(row, 0, 0); } FILE *fp = NULL; diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index cbf8907f7cf0..887ffe48aee7 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -284,7 +284,7 @@ class array_view_accessors self->m_strides[1] * j); } - sub_t operator[](npy_intp i) const + sub_t subarray(npy_intp i) const { const AVC *self = static_cast(this); @@ -318,7 +318,7 @@ class array_view_accessors self->m_strides[1] * j + self->m_strides[2] * k); } - sub_t operator[](npy_intp i) const + sub_t subarray(npy_intp i) const { const AVC *self = static_cast(this); From aef86ada619fe81e3b2d665ef3c286b509a9a5be Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Thu, 19 May 2016 08:19:19 -0400 Subject: [PATCH 4/4] Don't use contouring on the path unless necessary --- src/_path.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/_path.h b/src/_path.h index 71da86b0f42c..bb730be71ff2 100644 --- a/src/_path.h +++ b/src/_path.h @@ -233,10 +233,13 @@ inline void points_in_path(PointArray &points, transformed_path_t trans_path(path, trans); no_nans_t no_nans_path(trans_path, true, path.has_curves()); curve_t curved_path(no_nans_path); - contour_t contoured_path(curved_path); - contoured_path.width(r); - - point_in_path_impl(points, contoured_path, result); + if (r != 0.0) { + contour_t contoured_path(curved_path); + contoured_path.width(r); + point_in_path_impl(points, contoured_path, result); + } else { + point_in_path_impl(points, curved_path, result); + } } template