From d4b45d085cd3daab997022af546cf8c2f7e87f19 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Mon, 23 Sep 2024 23:29:39 -0400 Subject: [PATCH 1/4] Deprecate passing floats to RendererAgg.draw_text_image These were silently truncated to int anyway, so we should make the types explicit. --- .../deprecations/28967-ES.rst | 5 ++++ src/_backend_agg_wrapper.cpp | 30 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 doc/api/next_api_changes/deprecations/28967-ES.rst diff --git a/doc/api/next_api_changes/deprecations/28967-ES.rst b/doc/api/next_api_changes/deprecations/28967-ES.rst new file mode 100644 index 000000000000..7a7a5bd4e6c9 --- /dev/null +++ b/doc/api/next_api_changes/deprecations/28967-ES.rst @@ -0,0 +1,5 @@ +Passing floating-point values to ``RendererAgg.draw_text_image`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Any floating-point values passed to the *x* and *y* parameters were truncated to integers +silently. This behaviour is now deprecated, and only `int` values should be used. diff --git a/src/_backend_agg_wrapper.cpp b/src/_backend_agg_wrapper.cpp index 233c2289972f..269e2aaa9ee5 100644 --- a/src/_backend_agg_wrapper.cpp +++ b/src/_backend_agg_wrapper.cpp @@ -58,11 +58,37 @@ PyRendererAgg_draw_path(RendererAgg *self, static void PyRendererAgg_draw_text_image(RendererAgg *self, py::array_t image_obj, - double x, - double y, + std::variant vx, + std::variant vy, double angle, GCAgg &gc) { + int x, y; + + if (auto value = std::get_if(&vx)) { + auto api = py::module_::import("matplotlib._api"); + auto warn = api.attr("warn_deprecated"); + warn("since"_a="3.10", "name"_a="x", "obj_type"_a="parameter as float", + "alternative"_a="int(x)"); + x = static_cast(*value); + } else if (auto value = std::get_if(&vx)) { + x = *value; + } else { + throw std::runtime_error("Should not happen"); + } + + if (auto value = std::get_if(&vy)) { + auto api = py::module_::import("matplotlib._api"); + auto warn = api.attr("warn_deprecated"); + warn("since"_a="3.10", "name"_a="y", "obj_type"_a="parameter as float", + "alternative"_a="int(y)"); + y = static_cast(*value); + } else if (auto value = std::get_if(&vy)) { + y = *value; + } else { + throw std::runtime_error("Should not happen"); + } + // TODO: This really shouldn't be mutable, but Agg's renderer buffers aren't const. auto image = image_obj.mutable_unchecked<2>(); From ad4a625e1a2f23ce9f9b84e834319b420908ab3f Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 24 Sep 2024 00:39:32 -0400 Subject: [PATCH 2/4] Make some casts in Agg usage explicit The Agg API uses `double`s here, but the array sizes are `pybind11::ssize_t`, which are technically bigger (in some ways), and cause a warning on MSVC. We don't accept a backing surface larger than ~2^24, so there's no worry about these overflowing. --- src/_backend_agg.h | 25 ++++++++++++++++--------- src/_image_resample.h | 3 ++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/_backend_agg.h b/src/_backend_agg.h index b14eb3f9e565..8010508ae920 100644 --- a/src/_backend_agg.h +++ b/src/_backend_agg.h @@ -350,7 +350,8 @@ RendererAgg::_draw_path(path_t &path, bool has_clippath, const facepair_t &face, agg::trans_affine hatch_trans; hatch_trans *= agg::trans_affine_scaling(1.0, -1.0); hatch_trans *= agg::trans_affine_translation(0.0, 1.0); - hatch_trans *= agg::trans_affine_scaling(hatch_size, hatch_size); + hatch_trans *= agg::trans_affine_scaling(static_cast(hatch_size), + static_cast(hatch_size)); hatch_path_trans_t hatch_path_trans(hatch_path, hatch_trans); hatch_path_curve_t hatch_path_curve(hatch_path_trans); hatch_path_stroke_t hatch_path_stroke(hatch_path_curve); @@ -739,16 +740,19 @@ inline void RendererAgg::draw_text_image(GCAgg &gc, ImageArray &image, int x, in set_clipbox(gc.cliprect, theRasterizer); + auto image_height = static_cast(image.shape(0)), + image_width = static_cast(image.shape(1)); + agg::trans_affine mtx; - mtx *= agg::trans_affine_translation(0, -image.shape(0)); + mtx *= agg::trans_affine_translation(0, -image_height); mtx *= agg::trans_affine_rotation(-angle * (agg::pi / 180.0)); mtx *= agg::trans_affine_translation(x, y); agg::path_storage rect; rect.move_to(0, 0); - rect.line_to(image.shape(1), 0); - rect.line_to(image.shape(1), image.shape(0)); - rect.line_to(0, image.shape(0)); + rect.line_to(image_width, 0); + rect.line_to(image_width, image_height); + rect.line_to(0, image_height); rect.line_to(0, 0); agg::conv_transform rect2(rect, mtx); @@ -842,12 +846,15 @@ inline void RendererAgg::draw_image(GCAgg &gc, agg::trans_affine mtx; agg::path_storage rect; - mtx *= agg::trans_affine_translation((int)x, (int)(height - (y + image.shape(0)))); + auto image_height = static_cast(image.shape(0)), + image_width = static_cast(image.shape(1)); + + mtx *= agg::trans_affine_translation((int)x, (int)(height - (y + image_height))); rect.move_to(0, 0); - rect.line_to(image.shape(1), 0); - rect.line_to(image.shape(1), image.shape(0)); - rect.line_to(0, image.shape(0)); + rect.line_to(image_width, 0); + rect.line_to(image_width, image_height); + rect.line_to(0, image_height); rect.line_to(0, 0); agg::conv_transform rect2(rect, mtx); diff --git a/src/_image_resample.h b/src/_image_resample.h index dbc6171f3ec2..282bf8ef82f6 100644 --- a/src/_image_resample.h +++ b/src/_image_resample.h @@ -566,7 +566,8 @@ class span_conv_alpha { if (m_alpha != 1.0) { do { - span->a *= m_alpha; + span->a = static_cast( + static_cast(span->a) * m_alpha); ++span; } while (--len); } From 422b6cf544a0f54123d6850727796c82f74ca145 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 24 Sep 2024 01:29:47 -0400 Subject: [PATCH 3/4] Remove unused parameter in TriContourGenerator::find_interior_lines Its only use was removed in 193e5a9525e2a2314260dadc63dc3c89d03966ab. --- src/tri/_tri.cpp | 9 ++++----- src/tri/_tri.h | 6 ++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/tri/_tri.cpp b/src/tri/_tri.cpp index 908136081971..10d958c5ea11 100644 --- a/src/tri/_tri.cpp +++ b/src/tri/_tri.cpp @@ -751,7 +751,7 @@ py::tuple TriContourGenerator::create_contour(const double& level) Contour contour; find_boundary_lines(contour, level); - find_interior_lines(contour, level, false, false); + find_interior_lines(contour, level, false); return contour_line_to_segs_and_kinds(contour); } @@ -766,8 +766,8 @@ py::tuple TriContourGenerator::create_filled_contour(const double& lower_level, Contour contour; find_boundary_lines_filled(contour, lower_level, upper_level); - find_interior_lines(contour, lower_level, false, true); - find_interior_lines(contour, upper_level, true, true); + find_interior_lines(contour, lower_level, false); + find_interior_lines(contour, upper_level, true); return contour_to_segs_and_kinds(contour); } @@ -880,8 +880,7 @@ void TriContourGenerator::find_boundary_lines_filled(Contour& contour, void TriContourGenerator::find_interior_lines(Contour& contour, const double& level, - bool on_upper, - bool filled) + bool on_upper) { const Triangulation& triang = _triangulation; int ntri = triang.get_ntri(); diff --git a/src/tri/_tri.h b/src/tri/_tri.h index cfdfd827124c..2319650b367b 100644 --- a/src/tri/_tri.h +++ b/src/tri/_tri.h @@ -407,12 +407,10 @@ class TriContourGenerator final * intersect any boundary. * contour: Contour to add new lines to. * level: Contour level. - * on_upper: Whether on upper or lower contour level. - * filled: Whether contours are filled or not. */ + * on_upper: Whether on upper or lower contour level. */ void find_interior_lines(Contour& contour, const double& level, - bool on_upper, - bool filled); + bool on_upper); /* Follow contour line around boundary of the Triangulation from the * specified TriEdge to its end which can be on either the lower or upper From 4d40c1bfb18acfc4450dda3369129cca4030bcff Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 24 Sep 2024 02:14:34 -0400 Subject: [PATCH 4/4] Deprecate passing floats to FT2Image These were silently truncated to int anyway, so we should make the types explicit. --- .../deprecations/28967-ES.rst | 7 +++ lib/matplotlib/_mathtext.py | 6 +-- lib/matplotlib/ft2font.pyi | 6 +-- src/ft2font_wrapper.cpp | 50 +++++++++++++++++-- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/doc/api/next_api_changes/deprecations/28967-ES.rst b/doc/api/next_api_changes/deprecations/28967-ES.rst index 7a7a5bd4e6c9..8bb238def943 100644 --- a/doc/api/next_api_changes/deprecations/28967-ES.rst +++ b/doc/api/next_api_changes/deprecations/28967-ES.rst @@ -3,3 +3,10 @@ Passing floating-point values to ``RendererAgg.draw_text_image`` Any floating-point values passed to the *x* and *y* parameters were truncated to integers silently. This behaviour is now deprecated, and only `int` values should be used. + +Passing floating-point values to ``FT2Image`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Any floating-point values passed to the `.FT2Image` constructor, or the *x0*, *y0*, *x1*, +and *y1* parameters of `.FT2Image.draw_rect_filled` were truncated to integers silently. +This behaviour is now deprecated, and only `int` values should be used. diff --git a/lib/matplotlib/_mathtext.py b/lib/matplotlib/_mathtext.py index c9bb90608a85..6a1d9add9e8a 100644 --- a/lib/matplotlib/_mathtext.py +++ b/lib/matplotlib/_mathtext.py @@ -153,7 +153,7 @@ def to_raster(self, *, antialiased: bool) -> RasterParse: w = xmax - xmin h = ymax - ymin - self.box.depth d = ymax - ymin - self.box.height - image = FT2Image(np.ceil(w), np.ceil(h + max(d, 0))) + image = FT2Image(int(np.ceil(w)), int(np.ceil(h + max(d, 0)))) # Ideally, we could just use self.glyphs and self.rects here, shifting # their coordinates by (-xmin, -ymin), but this yields slightly @@ -163,7 +163,7 @@ def to_raster(self, *, antialiased: bool) -> RasterParse: for ox, oy, info in shifted.glyphs: info.font.draw_glyph_to_bitmap( - image, ox, oy - info.metrics.iceberg, info.glyph, + image, int(ox), int(oy - info.metrics.iceberg), info.glyph, antialiased=antialiased) for x1, y1, x2, y2 in shifted.rects: height = max(int(y2 - y1) - 1, 0) @@ -172,7 +172,7 @@ def to_raster(self, *, antialiased: bool) -> RasterParse: y = int(center - (height + 1) / 2) else: y = int(y1) - image.draw_rect_filled(int(x1), y, np.ceil(x2), y + height) + image.draw_rect_filled(int(x1), y, int(np.ceil(x2)), y + height) return RasterParse(0, 0, w, h + d, d, image) diff --git a/lib/matplotlib/ft2font.pyi b/lib/matplotlib/ft2font.pyi index 88c65d139939..1638bac692d3 100644 --- a/lib/matplotlib/ft2font.pyi +++ b/lib/matplotlib/ft2font.pyi @@ -198,7 +198,7 @@ class FT2Font(Buffer): def _get_fontmap(self, string: str) -> dict[str, FT2Font]: ... def clear(self) -> None: ... def draw_glyph_to_bitmap( - self, image: FT2Image, x: float, y: float, glyph: Glyph, antialiased: bool = ... + self, image: FT2Image, x: int, y: int, glyph: Glyph, antialiased: bool = ... ) -> None: ... def draw_glyphs_to_bitmap(self, antialiased: bool = ...) -> None: ... def get_bitmap_offset(self) -> tuple[int, int]: ... @@ -281,8 +281,8 @@ class FT2Font(Buffer): @final class FT2Image(Buffer): - def __init__(self, width: float, height: float) -> None: ... - def draw_rect_filled(self, x0: float, y0: float, x1: float, y1: float) -> None: ... + def __init__(self, width: int, height: int) -> None: ... + def draw_rect_filled(self, x0: int, y0: int, x1: int, y1: int) -> None: ... if sys.version_info[:2] >= (3, 12): def __buffer__(self, flags: int) -> memoryview: ... diff --git a/src/ft2font_wrapper.cpp b/src/ft2font_wrapper.cpp index 59400d4a2de5..9f9fab999707 100644 --- a/src/ft2font_wrapper.cpp +++ b/src/ft2font_wrapper.cpp @@ -13,6 +13,30 @@ namespace py = pybind11; using namespace pybind11::literals; +template +using double_or_ = std::variant; + +template +static T +_double_to_(const char *name, double_or_ &var) +{ + if (auto value = std::get_if(&var)) { + auto api = py::module_::import("matplotlib._api"); + auto warn = api.attr("warn_deprecated"); + warn("since"_a="3.10", "name"_a=name, "obj_type"_a="parameter as float", + "alternative"_a="int({})"_s.format(name)); + return static_cast(*value); + } else if (auto value = std::get_if(&var)) { + return *value; + } else { + // pybind11 will have only allowed types that match the variant, so this `else` + // can't happen. We only have this case because older macOS doesn't support + // `std::get` and using the conditional `std::get_if` means an `else` to silence + // compiler warnings about "unhandled" cases. + throw std::runtime_error("Should not happen"); + } +} + /********************************************************************** * Enumerations * */ @@ -227,8 +251,15 @@ const char *PyFT2Image_draw_rect_filled__doc__ = R"""( )"""; static void -PyFT2Image_draw_rect_filled(FT2Image *self, double x0, double y0, double x1, double y1) +PyFT2Image_draw_rect_filled(FT2Image *self, + double_or_ vx0, double_or_ vy0, + double_or_ vx1, double_or_ vy1) { + auto x0 = _double_to_("x0", vx0); + auto y0 = _double_to_("y0", vy0); + auto x1 = _double_to_("x1", vx1); + auto y1 = _double_to_("y1", vy1); + self->draw_rect_filled(x0, y0, x1, y1); } @@ -920,7 +951,7 @@ const char *PyFT2Font_draw_glyph_to_bitmap__doc__ = R"""( ---------- image : FT2Image The image buffer on which to draw the glyph. - x, y : float + x, y : int The pixel location at which to draw the glyph. glyph : Glyph The glyph to draw. @@ -933,9 +964,13 @@ const char *PyFT2Font_draw_glyph_to_bitmap__doc__ = R"""( )"""; static void -PyFT2Font_draw_glyph_to_bitmap(PyFT2Font *self, FT2Image &image, double xd, double yd, +PyFT2Font_draw_glyph_to_bitmap(PyFT2Font *self, FT2Image &image, + double_or_ vxd, double_or_ vyd, PyGlyph *glyph, bool antialiased = true) { + auto xd = _double_to_("x", vxd); + auto yd = _double_to_("y", vyd); + self->x->draw_glyph_to_bitmap(image, xd, yd, glyph->glyphInd, antialiased); } @@ -1625,7 +1660,14 @@ PYBIND11_MODULE(ft2font, m, py::mod_gil_not_used()) py::class_(m, "FT2Image", py::is_final(), py::buffer_protocol(), PyFT2Image__doc__) - .def(py::init(), "width"_a, "height"_a, PyFT2Image_init__doc__) + .def(py::init( + [](double_or_ width, double_or_ height) { + return new FT2Image( + _double_to_("width", width), + _double_to_("height", height) + ); + }), + "width"_a, "height"_a, PyFT2Image_init__doc__) .def("draw_rect_filled", &PyFT2Image_draw_rect_filled, "x0"_a, "y0"_a, "x1"_a, "y1"_a, PyFT2Image_draw_rect_filled__doc__)