Agg: Remove 16-bit limits #28904
Agg: Remove 16-bit limits #28904
Conversation
|
Even if we double the book keeping cost, I would expect it to be small in absolute terms? Our output is small (on the scale of the data our users feed us, on the scale of a small of the footprint of a typical modern program and available memory on a typical machine). |
|
... and given AGG is basically unchanged since 2006 and typical RAM sizes back then were about 512MB, I believe we can afford a worst-case factor of 2 increase now after 20 years. |
|
I ran tests with |
src/_image_resample.h
Outdated
| @@ -699,8 +702,8 @@ static void get_filter(const resample_params_t ¶ms, | |||
| } | |||
|
|
|||
|
|
|||
| template<typename color_type> | |||
| void resample( | |||
| template<typename color_type, bool is_large = false> | |||
There was a problem hiding this comment.
Would this be a better name?
| template<typename color_type, bool is_large = false> | |
| template<typename color_type, bool use_32bit = false> |
There was a problem hiding this comment.
Since there's not really been much change to memory usage, I'm going to drop the template change and just always use the larger scan lines. That'll save us some code space, and template complications.
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 matplotlib#28893. A similar problem occurs for resampled images, which also uses Agg scanlines internally. See the breakage in matplotlib#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 matplotlib#23826 Fixes matplotlib#26368 Fixes matplotlib#28893
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.
ff86c19 to
ace1cc2
Compare
|
After some additional checks, I was able to determine that the image dimension limit is also outdated, and we can expand it to match Agg's coordinates at 2**23 pixels. Also after some searching, I believe I've essentially re-created #3451 which was closed due to the CXX removal. With these changes, I am able to run: and produce an 8388607x50-pixel (i.e., 2**23-1) image with lines that are not broken in any visible way. Shortening the example from #26368: which produces a 1Mx50-pixel image of a gradient. Unfortunately, the resampling step takes about 16s per 100000 pixels, or, nearly 3 minutes for 1M pixels and 5-7GiB of RAM. So I did not test out the full 8M pixel image. I also read issues that mentioned the 32768-pixel limit as being something about |
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.
ace1cc2 to
9259989
Compare
|
Does this need a rebase? |
|
It could to fix the CI, but from other PRs, it seems that macOS/homebrew broke again for other reasons. |
PR summary
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.
Now, this does mean that internal bookkeeping will use double the memory. I have not yet benchmarked the memory usage to see what effect that would have. But I have implemented this two ways to show how it might work:
_image_resample.h, the larger scan lines are used if the output image is >=2**15 in either dimension._backend_agg.h, the larger scan lines are always used. While this is unfortunately the more common case, it's also the harder one to conditionalize as the type is in the class, and the template is used everywhere.That being said, having the scan lines be conditional means that double the code is emitted instead. For the Agg backend, it increases by 4096 bytes (after stripping debug info), whereas for the image extension, it increased by 65536 bytes vs no increase for just using the larger one.
However, given the breakage in #23826, it may be necessary to always use the 32-bit scan lines.
Fixes #23826
Fixes #26368
Fixes #28893
PR checklist