From 285a01a79bfe4c436f7d93e499fd4495d9b065d8 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 22 Jan 2021 18:53:12 -0500 Subject: [PATCH 1/6] MNT: make sure we forward path configuration when splitting --- lib/matplotlib/backends/backend_agg.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py index b937c64fce95..bec3c955ece9 100644 --- a/lib/matplotlib/backends/backend_agg.py +++ b/lib/matplotlib/backends/backend_agg.py @@ -148,6 +148,7 @@ def draw_path(self, gc, path, transform, rgbFace=None): c = c[ii0:ii1] c[0] = Path.MOVETO # move to end of last chunk p = Path(v, c) + p.simplify_threshold = path.simplify_threshold try: self._renderer.draw_path(gc, p, transform, rgbFace) except OverflowError as err: From 2116856327500b6ddb9a24bfbff52900aed588a1 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 22 Jan 2021 18:53:44 -0500 Subject: [PATCH 2/6] ENH: make error messages from exceeding agg cell block count clearer The error messages now provide better instruction on how to fix the problem. There are some paths we can not split (one with hatching, fills, or that claim they can not be simplified) so tell user that. If we could have otherwise split up the path if the chunksize is set to less than 100, which disables the feature, ask user to make it bigger or make the path simplification more aggressive. If the chunksize is bigger than the data, ask user to make it smaller or make the path simplification more aggressive. If the chunksize is smaller than the data, but still too big ask user to make it smaller or make the path simplification more aggressive. closes #19325 Co-authored-by: Antony Lee --- lib/matplotlib/backends/backend_agg.py | 57 ++++++++++++++++-- lib/matplotlib/tests/test_agg.py | 81 +++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py index bec3c955ece9..992695a9f188 100644 --- a/lib/matplotlib/backends/backend_agg.py +++ b/lib/matplotlib/backends/backend_agg.py @@ -152,15 +152,62 @@ def draw_path(self, gc, path, transform, rgbFace=None): try: self._renderer.draw_path(gc, p, transform, rgbFace) except OverflowError as err: - raise OverflowError( - "Exceeded cell block limit (set 'agg.path.chunksize' " - "rcparam)") from err + msg = ("Exceeded cell block limit in Agg.\n\n" + "Please reduce " + "the value of rcParams['agg.path.chunksize'] " + f"(currently {nmax}) or increase the " + "path simplification threshold" + "(rcParams['path.simplify_threshold'] = " + f"{mpl.rcParams['path.simplify_threshold']:.2f} " + "by default and path.simplify_threshold " + f"= {path.simplify_threshold:.2f} " + "on the input)." + ) + raise OverflowError(msg) from err else: try: self._renderer.draw_path(gc, path, transform, rgbFace) except OverflowError as err: - raise OverflowError("Exceeded cell block limit (set " - "'agg.path.chunksize' rcparam)") from err + cant_chunk = '' + if rgbFace is not None: + cant_chunk += "- can not split filled path\n" + if gc.get_hatch() is not None: + cant_chunk += "- can not split hatched path\n" + if not path.should_simplify: + cant_chunk += "- path.should_simplify is False\n" + if len(cant_chunk): + msg = ("Exceeded cell block limit in Agg, however " + "for the following reasons:\n\n" + f"{cant_chunk}\n" + "we can not automatically split up this path " + "to draw.\n\n" + "Please manually simplify your path.") + + else: + inc_threhold = ( + "or increase the " + "path simplification threshold" + "(rcParams['path.simplify_threshold'] = " + f"{mpl.rcParams['path.simplify_threshold']} " + "by default and path.simplify_threshold " + f"= {path.simplify_threshold} " + "on the input)." + ) + if nmax > 100: + msg = ("Exceeded cell block limit in Agg. Please " + "reduce the value of " + "rcParams['agg.path.chunksize'] " + f"(currently {nmax}) " + + inc_threhold + ) + else: + msg = ("Exceeded cell block limit in Agg. Please set " + "the value of rcParams['agg.path.chunksize'], " + f"(currently {nmax}) to be greater than 100 " + + inc_threhold + ) + + raise OverflowError(msg) from err def draw_mathtext(self, gc, x, y, s, prop, angle): """Draw mathtext using :mod:`matplotlib.mathtext`.""" diff --git a/lib/matplotlib/tests/test_agg.py b/lib/matplotlib/tests/test_agg.py index 14573f5941f6..6a0b625ef386 100644 --- a/lib/matplotlib/tests/test_agg.py +++ b/lib/matplotlib/tests/test_agg.py @@ -8,9 +8,12 @@ from matplotlib import ( collections, path, pyplot as plt, transforms as mtransforms, rcParams) -from matplotlib.image import imread +from matplotlib.backends.backend_agg import RendererAgg from matplotlib.figure import Figure +from matplotlib.image import imread +from matplotlib.path import Path from matplotlib.testing.decorators import image_comparison +from matplotlib.transforms import IdentityTransform def test_repeated_save_with_alpha(): @@ -251,3 +254,79 @@ def test_draw_path_collection_error_handling(): ax.scatter([1], [1]).set_paths(path.Path([(0, 1), (2, 3)])) with pytest.raises(TypeError): fig.canvas.draw() + + +@pytest.fixture +def chunk_limit_setup(): + N = 100_000 + dpi = 500 + w = 5*dpi + h = 6*dpi + + # just fit in the width + x = np.linspace(0, w, N) + # and go top-to-bottom + y = np.ones(N) * h + y[::2] = 0 + + idt = IdentityTransform() + # make a renderer + ra = RendererAgg(w, h, dpi) + # setup the minimal gc to draw a line + gc = ra.new_gc() + gc.set_linewidth(1) + gc.set_foreground('r') + # make a Path + p = Path(np.vstack((x, y)).T) + # effectively disable path simplification (but leaving it "on") + p.simplify_threshold = 0 + + return ra, gc, p, idt + + +def test_chunksize_hatch_fail(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + + gc.set_hatch('/') + + with pytest.raises(OverflowError, match='hatched path'): + ra.draw_path(gc, p, idt) + + +def test_chunksize_rgbFace_fail(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + + with pytest.raises(OverflowError, match='filled path'): + ra.draw_path(gc, p, idt, (1, 0, 0)) + + +def test_chunksize_no_simplify_fail(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + p.should_simplify = False + with pytest.raises(OverflowError, match="should_simplify is False"): + ra.draw_path(gc, p, idt) + + +def test_chunksize_zero(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + # set to zero to disable, currently defaults to 0, but lets be sure + rcParams['agg.path.chunksize'] = 0 + with pytest.raises(OverflowError, match='Please set'): + ra.draw_path(gc, p, idt) + + +def test_chunksize_too_big_to_chunk(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + # set big enough that we do not try to chunk + rcParams['agg.path.chunksize'] = 1_000_000 + with pytest.raises(OverflowError, match='Please reduce'): + ra.draw_path(gc, p, idt) + + +def test_chunksize_toobig_chunks(chunk_limit_setup): + ra, gc, p, idt = chunk_limit_setup + # small enough we will try to chunk, but big enough we will fail + # to render + rcParams['agg.path.chunksize'] = 90_000 + with pytest.raises(OverflowError, match='Please reduce'): + ra.draw_path(gc, p, idt) From c4b1dddbf47353b20c8d9553465f15b212362759 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Fri, 22 Jan 2021 19:05:21 -0500 Subject: [PATCH 3/6] MNT: increase the maximum number of cells in the Agg rasterizer This allows longer, more textured, paths to be drawn with the default settings at the risk of very long run times. --- src/_backend_agg.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_backend_agg.cpp b/src/_backend_agg.cpp index 0a9e7ab7e160..79575697a08b 100644 --- a/src/_backend_agg.cpp +++ b/src/_backend_agg.cpp @@ -45,7 +45,7 @@ RendererAgg::RendererAgg(unsigned int width, unsigned int height, double dpi) rendererBase(), rendererAA(), rendererBin(), - theRasterizer(8192), + theRasterizer(32768), lastclippath(NULL), _fill_color(agg::rgba(1, 1, 1, 0)) { From 0f0ea22b6e91c7ec3f2aae2dcfbcababf60979dc Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 26 May 2021 20:06:15 -0400 Subject: [PATCH 4/6] TST: tune test parameters - Make sure values are be big enough / small enough - make long path test more pathological (Random data will sometimes go in the same direction) --- lib/matplotlib/tests/test_agg.py | 8 ++++---- lib/matplotlib/tests/test_simplification.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/tests/test_agg.py b/lib/matplotlib/tests/test_agg.py index 6a0b625ef386..0e4abf86fe02 100644 --- a/lib/matplotlib/tests/test_agg.py +++ b/lib/matplotlib/tests/test_agg.py @@ -75,10 +75,10 @@ def test_marker_with_nan(): def test_long_path(): buff = io.BytesIO() - - fig, ax = plt.subplots() - np.random.seed(0) - points = np.random.rand(70000) + fig = Figure() + ax = fig.subplots() + points = np.ones(100_000) + points[::2] *= -1 ax.plot(points) fig.savefig(buff, format='png') diff --git a/lib/matplotlib/tests/test_simplification.py b/lib/matplotlib/tests/test_simplification.py index 952e890ce660..0749d0f3a115 100644 --- a/lib/matplotlib/tests/test_simplification.py +++ b/lib/matplotlib/tests/test_simplification.py @@ -305,8 +305,8 @@ def test_start_with_moveto(): def test_throw_rendering_complexity_exceeded(): plt.rcParams['path.simplify'] = False - xx = np.arange(200000) - yy = np.random.rand(200000) + xx = np.arange(2_000_000) + yy = np.random.rand(2_000_000) yy[1000] = np.nan fig, ax = plt.subplots() From 09fa321c2f0013a4dcaafa31a1bca19cd7307814 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Wed, 22 Sep 2021 15:18:38 -0400 Subject: [PATCH 5/6] MNT: simplify the error reporting in agg chunk overflows --- lib/matplotlib/backends/backend_agg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py index 992695a9f188..490bb772f2c4 100644 --- a/lib/matplotlib/backends/backend_agg.py +++ b/lib/matplotlib/backends/backend_agg.py @@ -163,7 +163,7 @@ def draw_path(self, gc, path, transform, rgbFace=None): f"= {path.simplify_threshold:.2f} " "on the input)." ) - raise OverflowError(msg) from err + raise OverflowError(msg) from None else: try: self._renderer.draw_path(gc, path, transform, rgbFace) @@ -207,7 +207,7 @@ def draw_path(self, gc, path, transform, rgbFace=None): + inc_threhold ) - raise OverflowError(msg) from err + raise OverflowError(msg) from None def draw_mathtext(self, gc, x, y, s, prop, angle): """Draw mathtext using :mod:`matplotlib.mathtext`.""" From 23f5da41945444bfdd584a06398327cc74b3907a Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Sat, 25 Sep 2021 00:37:39 -0400 Subject: [PATCH 6/6] Fix a few minor typos --- lib/matplotlib/backends/backend_agg.py | 61 +++++++++++++------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/lib/matplotlib/backends/backend_agg.py b/lib/matplotlib/backends/backend_agg.py index 490bb772f2c4..4a62f5a921cd 100644 --- a/lib/matplotlib/backends/backend_agg.py +++ b/lib/matplotlib/backends/backend_agg.py @@ -152,17 +152,16 @@ def draw_path(self, gc, path, transform, rgbFace=None): try: self._renderer.draw_path(gc, p, transform, rgbFace) except OverflowError as err: - msg = ("Exceeded cell block limit in Agg.\n\n" - "Please reduce " - "the value of rcParams['agg.path.chunksize'] " - f"(currently {nmax}) or increase the " - "path simplification threshold" - "(rcParams['path.simplify_threshold'] = " - f"{mpl.rcParams['path.simplify_threshold']:.2f} " - "by default and path.simplify_threshold " - f"= {path.simplify_threshold:.2f} " - "on the input)." - ) + msg = ( + "Exceeded cell block limit in Agg.\n\n" + "Please reduce the value of " + f"rcParams['agg.path.chunksize'] (currently {nmax}) " + "or increase the path simplification threshold" + "(rcParams['path.simplify_threshold'] = " + f"{mpl.rcParams['path.simplify_threshold']:.2f} by " + "default and path.simplify_threshold = " + f"{path.simplify_threshold:.2f} on the input)." + ) raise OverflowError(msg) from None else: try: @@ -176,17 +175,17 @@ def draw_path(self, gc, path, transform, rgbFace=None): if not path.should_simplify: cant_chunk += "- path.should_simplify is False\n" if len(cant_chunk): - msg = ("Exceeded cell block limit in Agg, however " - "for the following reasons:\n\n" - f"{cant_chunk}\n" - "we can not automatically split up this path " - "to draw.\n\n" - "Please manually simplify your path.") + msg = ( + "Exceeded cell block limit in Agg, however for the " + "following reasons:\n\n" + f"{cant_chunk}\n" + "we can not automatically split up this path to draw." + "\n\nPlease manually simplify your path." + ) else: - inc_threhold = ( - "or increase the " - "path simplification threshold" + inc_threshold = ( + "or increase the path simplification threshold" "(rcParams['path.simplify_threshold'] = " f"{mpl.rcParams['path.simplify_threshold']} " "by default and path.simplify_threshold " @@ -194,18 +193,18 @@ def draw_path(self, gc, path, transform, rgbFace=None): "on the input)." ) if nmax > 100: - msg = ("Exceeded cell block limit in Agg. Please " - "reduce the value of " - "rcParams['agg.path.chunksize'] " - f"(currently {nmax}) " - + inc_threhold - ) + msg = ( + "Exceeded cell block limit in Agg. Please reduce " + "the value of rcParams['agg.path.chunksize'] " + f"(currently {nmax}) {inc_threshold}" + ) else: - msg = ("Exceeded cell block limit in Agg. Please set " - "the value of rcParams['agg.path.chunksize'], " - f"(currently {nmax}) to be greater than 100 " - + inc_threhold - ) + msg = ( + "Exceeded cell block limit in Agg. Please set " + "the value of rcParams['agg.path.chunksize'], " + f"(currently {nmax}) to be greater than 100 " + + inc_threshold + ) raise OverflowError(msg) from None