From 839419528a21b26145bf17fe9f27cc42610c8346 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Wed, 14 Oct 2015 21:09:26 -0400 Subject: [PATCH 1/4] Fix #5185: Be more careful about array dims Check the dimensions on arrays passed to C++ to ensure they are what we expect. --- src/_backend_agg.h | 36 -------------- src/_backend_agg_wrapper.cpp | 50 ++++++++++++++++--- src/_path_wrapper.cpp | 19 ++++--- src/py_converters.cpp | 96 ++++++++++++++++++++++++++++++++++++ src/py_converters.h | 4 ++ 5 files changed, 154 insertions(+), 51 deletions(-) diff --git a/src/_backend_agg.h b/src/_backend_agg.h index ee85f00f7ad3..11abfcb98f5e 100644 --- a/src/_backend_agg.h +++ b/src/_backend_agg.h @@ -922,22 +922,6 @@ inline void RendererAgg::_draw_path_collection_generic(GCAgg &gc, typedef agg::conv_curve snapped_curve_t; typedef agg::conv_curve curve_t; - if (offsets.dim(0) != 0 && offsets.dim(1) != 2) { - throw "Offsets array must be Nx2 or empty"; - } - - if (facecolors.dim(0) != 0 && facecolors.dim(1) != 4) { - throw "Facecolors array must be a Nx4 array or empty"; - } - - if (edgecolors.dim(0) != 0 && edgecolors.dim(1) != 4) { - throw "Edgecolors array must by Nx4 or empty"; - } - - if (transforms.dim(0) != 0 && (transforms.dim(1) != 3 || transforms.dim(2) != 3)) { - throw "Transforms array must by Nx3x3 or empty"; - } - size_t Npaths = path_generator.num_paths(); size_t Noffsets = offsets.size(); size_t N = std::max(Npaths, Noffsets); @@ -1266,14 +1250,6 @@ inline void RendererAgg::draw_gouraud_triangle(GCAgg &gc, set_clipbox(gc.cliprect, theRasterizer); bool has_clippath = render_clippath(gc.clippath.path, gc.clippath.trans); - if (points.dim(0) != 3 || points.dim(1) != 2) { - throw "points must be a 3x2 array"; - } - - if (colors.dim(0) != 3 || colors.dim(1) != 4) { - throw "colors must be a 3x4 array"; - } - _draw_gouraud_triangle(points, colors, trans, has_clippath); } @@ -1288,18 +1264,6 @@ inline void RendererAgg::draw_gouraud_triangles(GCAgg &gc, set_clipbox(gc.cliprect, theRasterizer); bool has_clippath = render_clippath(gc.clippath.path, gc.clippath.trans); - if (points.dim(1) != 3 || points.dim(2) != 2) { - throw "points must be a Nx3x2 array"; - } - - if (colors.dim(1) != 3 || colors.dim(2) != 4) { - throw "colors must be a Nx3x4 array"; - } - - if (points.dim(0) != colors.dim(0)) { - throw "points and colors arrays must be the same length"; - } - for (int i = 0; i < points.dim(0); ++i) { typename PointArray::sub_t point = points[i]; typename ColorArray::sub_t color = colors[i]; diff --git a/src/_backend_agg_wrapper.cpp b/src/_backend_agg_wrapper.cpp index 753afe3837d2..752a8fc9484e 100644 --- a/src/_backend_agg_wrapper.cpp +++ b/src/_backend_agg_wrapper.cpp @@ -340,15 +340,15 @@ PyRendererAgg_draw_path_collection(PyRendererAgg *self, PyObject *args, PyObject &convert_trans_affine, &master_transform, &pathobj, - &transforms.converter, + &convert_transforms, &transforms, - &offsets.converter, + &convert_points, &offsets, &convert_trans_affine, &offset_trans, - &facecolors.converter, + &convert_colors, &facecolors, - &edgecolors.converter, + &convert_colors, &edgecolors, &linewidths.converter, &linewidths, @@ -411,14 +411,14 @@ static PyObject *PyRendererAgg_draw_quad_mesh(PyRendererAgg *self, PyObject *arg &mesh_height, &coordinates.converter, &coordinates, - &offsets.converter, + &convert_points, &offsets, &convert_trans_affine, &offset_trans, - &facecolors.converter, + &convert_colors, &facecolors, &antialiased, - &edgecolors.converter, + &convert_colors, &edgecolors)) { return NULL; } @@ -459,6 +459,21 @@ PyRendererAgg_draw_gouraud_triangle(PyRendererAgg *self, PyObject *args, PyObjec return NULL; } + if (points.dim(0) != 3 || points.dim(1) != 2) { + PyErr_Format(PyExc_ValueError, + "points must be a 3x2 array, got %dx%d", + points.dim(0), points.dim(1)); + return NULL; + } + + if (colors.dim(0) != 3 || colors.dim(1) != 4) { + PyErr_Format(PyExc_ValueError, + "colors must be a 3x4 array, got %dx%d", + colors.dim(0), colors.dim(1)); + return NULL; + } + + CALL_CPP("draw_gouraud_triangle", (self->x->draw_gouraud_triangle(gc, points, colors, trans))); Py_RETURN_NONE; @@ -485,6 +500,27 @@ PyRendererAgg_draw_gouraud_triangles(PyRendererAgg *self, PyObject *args, PyObje return NULL; } + if (points.dim(1) != 3 || points.dim(2) != 2) { + PyErr_Format(PyExc_ValueError, + "points must be a Nx3x2 array, got %dx%dx%d", + points.dim(0), points.dim(1), points.dim(2)); + return NULL; + } + + if (colors.dim(1) != 3 || colors.dim(2) != 4) { + PyErr_Format(PyExc_ValueError, + "colors must be a Nx3x4 array, got %dx%dx%d", + colors.dim(0), colors.dim(1), colors.dim(2)); + return NULL; + } + + if (points.dim(0) != colors.dim(0)) { + PyErr_Format(PyExc_ValueError, + "points and colors arrays must be the same length, got %d and %d", + points.dim(0), colors.dim(0)); + return NULL; + } + CALL_CPP("draw_gouraud_triangles", self->x->draw_gouraud_triangles(gc, points, colors, trans)); Py_RETURN_NONE; diff --git a/src/_path_wrapper.cpp b/src/_path_wrapper.cpp index cd80e636dc3d..e824bf290d60 100644 --- a/src/_path_wrapper.cpp +++ b/src/_path_wrapper.cpp @@ -69,7 +69,7 @@ static PyObject *Py_points_in_path(PyObject *self, PyObject *args, PyObject *kwd if (!PyArg_ParseTuple(args, "O&dO&O&:points_in_path", - &points.converter, + &convert_points, &points, &r, &convert_path, @@ -128,7 +128,7 @@ static PyObject *Py_points_on_path(PyObject *self, PyObject *args, PyObject *kwd if (!PyArg_ParseTuple(args, "O&dO&O&:points_on_path", - &points.converter, + &convert_points, &points, &r, &convert_path, @@ -200,7 +200,10 @@ static PyObject *Py_update_path_extents(PyObject *self, PyObject *args, PyObject } if (minpos.dim(0) != 2) { - PyErr_SetString(PyExc_ValueError, "minpos must be of length 2"); + PyErr_Format(PyExc_ValueError, + "minpos must be of length 2, got %d", + minpos.dim(0)); + return NULL; } extent_limits e; @@ -263,9 +266,9 @@ static PyObject *Py_get_path_collection_extents(PyObject *self, PyObject *args, &convert_trans_affine, &master_transform, &pathsobj, - &transforms.converter, + &convert_transforms, &transforms, - &offsets.converter, + &convert_points, &offsets, &convert_trans_affine, &offset_trans)) { @@ -319,9 +322,9 @@ static PyObject *Py_point_in_path_collection(PyObject *self, PyObject *args, PyO &convert_trans_affine, &master_transform, &pathsobj, - &transforms.converter, + &convert_transforms, &transforms, - &offsets.converter, + &convert_points, &offsets, &convert_trans_affine, &offset_trans, @@ -464,7 +467,7 @@ static PyObject *Py_count_bboxes_overlapping_bbox(PyObject *self, PyObject *args "O&O&:count_bboxes_overlapping_bbox", &convert_rect, &bbox, - &bboxes.converter, + &convert_bboxes, &bboxes)) { return NULL; } diff --git a/src/py_converters.cpp b/src/py_converters.cpp index 8635d0271158..23365c71565e 100644 --- a/src/py_converters.cpp +++ b/src/py_converters.cpp @@ -518,4 +518,100 @@ int convert_face(PyObject *color, GCAgg &gc, agg::rgba *rgba) return 1; } + +int convert_points(PyObject *obj, void *pointsp) +{ + numpy::array_view *points = (numpy::array_view *)pointsp; + + if (obj == NULL || obj == Py_None) { + return 1; + } + + points->set(obj); + + if (points->dim(0) == 0) { + return 1; + } + + if (points->dim(1) != 2) { + PyErr_Format(PyExc_ValueError, + "Points must be Nx2 array, got %dx%d", + points->dim(0), points->dim(1)); + return 0; + } + + return 1; +} + +int convert_transforms(PyObject *obj, void *transp) +{ + numpy::array_view *trans = (numpy::array_view *)transp; + + if (obj == NULL || obj == Py_None) { + return 1; + } + + trans->set(obj); + + if (trans->dim(0) == 0) { + return 1; + } + + if (trans->dim(1) != 3 || trans->dim(2) != 3) { + PyErr_Format(PyExc_ValueError, + "Transforms must be Nx3x3 array, got %dx%dx%d", + trans->dim(0), trans->dim(1), trans->dim(2)); + return 0; + } + + return 1; +} + +int convert_bboxes(PyObject *obj, void *bboxp) +{ + numpy::array_view *bbox = (numpy::array_view *)bboxp; + + if (obj == NULL || obj == Py_None) { + return 1; + } + + bbox->set(obj); + + if (bbox->dim(0) == 0) { + return 1; + } + + if (bbox->dim(1) != 2 || bbox->dim(2) != 2) { + PyErr_Format(PyExc_ValueError, + "Bbox array must be Nx2x2 array, got %dx%dx%d", + bbox->dim(0), bbox->dim(1), bbox->dim(2)); + return 0; + } + + return 1; +} + +int convert_colors(PyObject *obj, void *colorsp) +{ + numpy::array_view *colors = (numpy::array_view *)colorsp; + + if (obj == NULL || obj == Py_None) { + return 1; + } + + colors->set(obj); + + if (colors->dim(0) == 0) { + return 1; + } + + if (colors->dim(1) != 4) { + PyErr_Format(PyExc_ValueError, + "Colors array must be Nx4 array, got %dx%d", + colors->dim(0), colors->dim(1)); + return 0; + } + + return 1; +} } diff --git a/src/py_converters.h b/src/py_converters.h index 90cf454e30fb..02d84affe857 100644 --- a/src/py_converters.h +++ b/src/py_converters.h @@ -38,6 +38,10 @@ int convert_snap(PyObject *obj, void *snapp); int convert_offset_position(PyObject *obj, void *offsetp); int convert_sketch_params(PyObject *obj, void *sketchp); int convert_gcagg(PyObject *pygc, void *gcp); +int convert_points(PyObject *pygc, void *pointsp); +int convert_transforms(PyObject *pygc, void *transp); +int convert_bboxes(PyObject *pygc, void *bboxp); +int convert_colors(PyObject *pygc, void *colorsp); int convert_face(PyObject *color, GCAgg &gc, agg::rgba *rgba); } From 37f4dcd8355e2c9fc69d98b5a7ea61ea0e25a363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Thu, 15 Oct 2015 15:28:32 +0300 Subject: [PATCH 2/4] Make array_view.size return 0 for empty arrays --- src/numpy_cpp.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index 0bc2d5c03854..128482c669ec 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -481,7 +481,17 @@ class array_view : public detail::array_view_accessors size_t size() const { - return (size_t)dim(0); + bool empty = (ND == 0); + for (size_t i = 0; i < ND; i++) { + if (m_shape[i] == 0) { + empty = true; + } + } + if (empty) { + return 0; + } else { + return (size_t)dim(0); + } } bool empty() const From d424668829f751c0bbe562e60f7732d61d703dd1 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 19 Oct 2015 09:17:54 -0400 Subject: [PATCH 3/4] Use size() in place of dim(0) --- src/_backend_agg_wrapper.cpp | 6 +++--- src/_path.h | 8 ++++---- src/_path_wrapper.cpp | 8 ++++---- src/py_converters.cpp | 8 ++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/_backend_agg_wrapper.cpp b/src/_backend_agg_wrapper.cpp index 752a8fc9484e..c1564e69ff6e 100644 --- a/src/_backend_agg_wrapper.cpp +++ b/src/_backend_agg_wrapper.cpp @@ -500,21 +500,21 @@ PyRendererAgg_draw_gouraud_triangles(PyRendererAgg *self, PyObject *args, PyObje return NULL; } - if (points.dim(1) != 3 || points.dim(2) != 2) { + if (points.size() != 0 && (points.dim(1) != 3 || points.dim(2) != 2)) { PyErr_Format(PyExc_ValueError, "points must be a Nx3x2 array, got %dx%dx%d", points.dim(0), points.dim(1), points.dim(2)); return NULL; } - if (colors.dim(1) != 3 || colors.dim(2) != 4) { + if (colors.size() != 0 && (colors.dim(1) != 3 || colors.dim(2) != 4)) { PyErr_Format(PyExc_ValueError, "colors must be a Nx3x4 array, got %dx%dx%d", colors.dim(0), colors.dim(1), colors.dim(2)); return NULL; } - if (points.dim(0) != colors.dim(0)) { + if (points.size() != colors.size()) { PyErr_Format(PyExc_ValueError, "points and colors arrays must be the same length, got %d and %d", points.dim(0), colors.dim(0)); diff --git a/src/_path.h b/src/_path.h index 176db3168304..48d61af5be12 100644 --- a/src/_path.h +++ b/src/_path.h @@ -358,7 +358,7 @@ void get_path_collection_extents(agg::trans_affine &master_transform, agg::trans_affine &offset_trans, extent_limits &extent) { - if (offsets.dim(0) != 0 && offsets.dim(1) != 2) { + if (offsets.size() != 0 && offsets.dim(1) != 2) { throw "Offsets array must be Nx2"; } @@ -416,7 +416,7 @@ void point_in_path_collection(double x, return; } - size_t Noffsets = offsets.dim(0); + size_t Noffsets = offsets.size(); size_t N = std::max(Npaths, Noffsets); size_t Ntransforms = std::min(transforms.size(), N); size_t i; @@ -692,11 +692,11 @@ clip_path_to_rect(PathIterator &path, agg::rect_d &rect, bool inside, std::vecto template void affine_transform_2d(VerticesArray &vertices, agg::trans_affine &trans, ResultArray &result) { - if (vertices.dim(0) != 0 && vertices.dim(1) != 2) { + if (vertices.size() != 0 && vertices.dim(1) != 2) { throw "Invalid vertices array."; } - size_t n = vertices.dim(0); + size_t n = vertices.size(); double x; double y; double t0; diff --git a/src/_path_wrapper.cpp b/src/_path_wrapper.cpp index e824bf290d60..18469c5cb46a 100644 --- a/src/_path_wrapper.cpp +++ b/src/_path_wrapper.cpp @@ -79,7 +79,7 @@ static PyObject *Py_points_in_path(PyObject *self, PyObject *args, PyObject *kwd return NULL; } - npy_intp dims[] = { points.dim(0) }; + npy_intp dims[] = { points.size() }; numpy::array_view results(dims); CALL_CPP("points_in_path", (points_in_path(points, r, path, trans, results))); @@ -138,7 +138,7 @@ static PyObject *Py_points_on_path(PyObject *self, PyObject *args, PyObject *kwd return NULL; } - npy_intp dims[] = { points.dim(0) }; + npy_intp dims[] = { points.size() }; numpy::array_view results(dims); CALL_CPP("points_on_path", (points_on_path(points, r, path, trans, results))); @@ -437,7 +437,7 @@ static PyObject *Py_affine_transform(PyObject *self, PyObject *args, PyObject *k try { numpy::array_view vertices(vertices_obj); - npy_intp dims[] = { vertices.dim(0), 2 }; + npy_intp dims[] = { vertices.size(), 2 }; numpy::array_view result(dims); CALL_CPP("affine_transform", (affine_transform_2d(vertices, trans, result))); return result.pyobj(); @@ -445,7 +445,7 @@ static PyObject *Py_affine_transform(PyObject *self, PyObject *args, PyObject *k PyErr_Clear(); try { numpy::array_view vertices(vertices_obj); - npy_intp dims[] = { vertices.dim(0) }; + npy_intp dims[] = { vertices.size() }; numpy::array_view result(dims); CALL_CPP("affine_transform", (affine_transform_1d(vertices, trans, result))); return result.pyobj(); diff --git a/src/py_converters.cpp b/src/py_converters.cpp index 23365c71565e..fee7bf8ad463 100644 --- a/src/py_converters.cpp +++ b/src/py_converters.cpp @@ -529,7 +529,7 @@ int convert_points(PyObject *obj, void *pointsp) points->set(obj); - if (points->dim(0) == 0) { + if (points->size() == 0) { return 1; } @@ -553,7 +553,7 @@ int convert_transforms(PyObject *obj, void *transp) trans->set(obj); - if (trans->dim(0) == 0) { + if (trans->size() == 0) { return 1; } @@ -577,7 +577,7 @@ int convert_bboxes(PyObject *obj, void *bboxp) bbox->set(obj); - if (bbox->dim(0) == 0) { + if (bbox->size() == 0) { return 1; } @@ -601,7 +601,7 @@ int convert_colors(PyObject *obj, void *colorsp) colors->set(obj); - if (colors->dim(0) == 0) { + if (colors->size() == 0) { return 1; } From a291e9817557c09e8a111150c2348fbeceae241d Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 19 Oct 2015 16:47:38 -0400 Subject: [PATCH 4/4] Add comment --- src/numpy_cpp.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index 128482c669ec..cbf8907f7cf0 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -479,6 +479,10 @@ class array_view : public detail::array_view_accessors return m_shape[i]; } + /* + In most cases, code should use size() instead of dim(0), since + size() == 0 when any dimension is 0. + */ size_t size() const { bool empty = (ND == 0);