From 7d73e2a426338c09b836eac218c2be7c27059258 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Fri, 27 Sep 2024 21:04:25 -0400 Subject: [PATCH 1/3] Agg: Use 32-bit scan line classes When rendering objects, Agg rasterizes them into scan line objects (an x/y point, horizontal length, and colour), and the renderer class writes those to the pixels in the final buffer. Though we have determined that Agg buffers cannot be larger than 2**16, the scan line classes that we use contain 16-bit _signed_ integers internally, cutting off positive values at half the maximum. Since the renderer uses 32-bit integers, this can cause odd behaviour where any horizontal span that _starts_ before 2**15 (such as a horizontal spine) is rendered correctly even if it cross that point, but those that start after (such as a vertical spine or any portion of an angled line) end up clipped. For example, see how the spines and lines break in #28893. A similar problem occurs for resampled images, which also uses Agg scanlines internally. See the breakage in #26368 for an example. The example in that issue also contains horizontal spines that are wider than 2**15, which also exhibit strange behaviour. By moving to 32-bit scan lines, positions and lengths of the lines will no longer be clipped, and this fixes rendering on very large figures. Fixes #23826 Fixes #26368 Fixes #28893 --- src/_backend_agg.h | 6 +++--- src/_image_resample.h | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/_backend_agg.h b/src/_backend_agg.h index 5549978cfb80..a0147f9832c3 100644 --- a/src/_backend_agg.h +++ b/src/_backend_agg.h @@ -117,10 +117,10 @@ class RendererAgg typedef agg::renderer_scanline_bin_solid renderer_bin; typedef agg::rasterizer_scanline_aa rasterizer; - typedef agg::scanline_p8 scanline_p8; - typedef agg::scanline_bin scanline_bin; + typedef agg::scanline32_p8 scanline_p8; + typedef agg::scanline32_bin scanline_bin; typedef agg::amask_no_clip_gray8 alpha_mask_type; - typedef agg::scanline_u8_am scanline_am; + typedef agg::scanline32_u8_am scanline_am; typedef agg::renderer_base renderer_base_alpha_mask_type; typedef agg::renderer_scanline_aa_solid renderer_alpha_mask_type; diff --git a/src/_image_resample.h b/src/_image_resample.h index ddf1a4050325..dbc6171f3ec2 100644 --- a/src/_image_resample.h +++ b/src/_image_resample.h @@ -712,6 +712,7 @@ void resample( using renderer_t = agg::renderer_base; using rasterizer_t = agg::rasterizer_scanline_aa; + using scanline_t = agg::scanline32_u8; using reflect_t = agg::wrap_mode_reflect; using image_accessor_t = agg::image_accessor_wrap; @@ -739,7 +740,7 @@ void resample( span_alloc_t span_alloc; rasterizer_t rasterizer; - agg::scanline_u8 scanline; + scanline_t scanline; span_conv_alpha_t conv_alpha(params.alpha); From 474bb4f156c0df6766ad66490693ab785bdc13de Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 1 Oct 2024 20:25:25 -0400 Subject: [PATCH 2/3] Agg: Fix overflow when splitting large lines For very large Figures with a single Axes, the horizontal spines may be long enough to need splitting into separate lines. For large enough coordinates, the `x1+x2` calculation may overflow, flipping the sign bit, causing the coordinates to become negative. This causes an infinite loop as some internal condition is never met. By dividing `x1`/`x2` by 2 first, we avoid the overflow, and can calculate the split point correctly. --- extern/agg24-svn/include/agg_rasterizer_cells_aa.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extern/agg24-svn/include/agg_rasterizer_cells_aa.h b/extern/agg24-svn/include/agg_rasterizer_cells_aa.h index d1cc705405dc..44a55417b492 100644 --- a/extern/agg24-svn/include/agg_rasterizer_cells_aa.h +++ b/extern/agg24-svn/include/agg_rasterizer_cells_aa.h @@ -325,8 +325,10 @@ namespace agg if(dx >= dx_limit || dx <= -dx_limit) { - int cx = (x1 + x2) >> 1; - int cy = (y1 + y2) >> 1; + // These are overflow safe versions of (x1 + x2) >> 1; divide each by 2 + // first, then add 1 if both were odd. + int cx = (x1 >> 1) + (x2 >> 1) + ((x1 & 1) & (x2 & 1)); + int cy = (y1 >> 1) + (y2 >> 1) + ((y1 & 1) & (y2 & 1)); line(x1, y1, cx, cy); line(cx, cy, x2, y2); return; From 9259989e017e601e36d06f53ef1cda3547be82c5 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 2 Oct 2024 03:38:35 -0400 Subject: [PATCH 3/3] Agg: Increase maximum size to 2**23 With 32-bit scan lines, we are able to take advantage of the full 32-bit range of coordinates. However, as noted in `extern/agg24-svn/include/agg_rasterizer_scanline_aa.h`, Agg uses 24.8-bit fixed point coordinates. With one bit taken for the sign, the maximum coordinate is now 2**23. --- doc/users/next_whats_new/increased_figure_limits.rst | 9 +++++++++ lib/matplotlib/tests/test_agg.py | 2 +- src/_backend_agg.cpp | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 doc/users/next_whats_new/increased_figure_limits.rst diff --git a/doc/users/next_whats_new/increased_figure_limits.rst b/doc/users/next_whats_new/increased_figure_limits.rst new file mode 100644 index 000000000000..499701cbca38 --- /dev/null +++ b/doc/users/next_whats_new/increased_figure_limits.rst @@ -0,0 +1,9 @@ +Increased Figure limits with Agg renderer +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Figures using the Agg renderer are now limited to 2**23 pixels in each +direction, instead of 2**16. Additionally, bugs that caused artists to not +render past 2**15 pixels horizontally have been fixed. + +Note that if you are using a GUI backend, it may have its own smaller limits +(which may themselves depend on screen size.) diff --git a/lib/matplotlib/tests/test_agg.py b/lib/matplotlib/tests/test_agg.py index 80c1f165382c..59387793605a 100644 --- a/lib/matplotlib/tests/test_agg.py +++ b/lib/matplotlib/tests/test_agg.py @@ -199,7 +199,7 @@ def process_image(self, padded_src, dpi): def test_too_large_image(): - fig = plt.figure(figsize=(300, 1000)) + fig = plt.figure(figsize=(300, 2**25)) buff = io.BytesIO() with pytest.raises(ValueError): fig.savefig(buff) diff --git a/src/_backend_agg.cpp b/src/_backend_agg.cpp index 3460b429ec12..eed27323ba9e 100644 --- a/src/_backend_agg.cpp +++ b/src/_backend_agg.cpp @@ -33,10 +33,10 @@ RendererAgg::RendererAgg(unsigned int width, unsigned int height, double dpi) throw std::range_error("dpi must be positive"); } - if (width >= 1 << 16 || height >= 1 << 16) { + if (width >= 1 << 23 || height >= 1 << 23) { throw std::range_error( "Image size of " + std::to_string(width) + "x" + std::to_string(height) + - " pixels is too large. It must be less than 2^16 in each direction."); + " pixels is too large. It must be less than 2^23 in each direction."); } unsigned stride(width * 4);