Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize sampling for empty images #2342

Merged
merged 2 commits into from
Oct 12, 2024
Merged

Optimize sampling for empty images #2342

merged 2 commits into from
Oct 12, 2024

Conversation

HeroicKatora
Copy link
Member

@HeroicKatora HeroicKatora commented Oct 10, 2024

This is a tip of an iceberg of better sampling, but the most critical case. Up-sampling in general may in the implementation allocate a larger temporary buffer than its input. Of course this makes little semantic sense here: after all, the actual information can not increase by this.

If one dimension increases while the other decreases the unfortunate consequence is that callers may somewhat reasonably expect a small buffer but internally will get a very large buffer. The approach of swapping sampling orders accordingly (first down, then up) might address the memory issue but is lossy.

So instead let's fix the most pressing issue: if no information was present in the input, nothing can be lost and we can pretend to perform everything by a very small intermediate image.

closes: #2340

@fintelia
Copy link
Contributor

I think it would be cleaner to add a check at the start of resize to immediately return ImageBuffer::new(nwidth, nheight) if any new or old image dimensions are empty. vertical_sample is only called from resize and blur. In the blur case, height==new_height so there isn't much difference from this change. And in resize, it seems nicer to handle all combinations of input and output sizes being empty at once.

This is a tip of an iceberg of better sampling, but the most critical
case. Up-sampling in general may in the implementation allocate a larger
temporary buffer than its input. Of course this makes little semantic
sense here: after all, the actual information can not increase by this.

If one dimension increases while the other decreases the unfortunate
consequence is that callers may somewhat reasonably expect a small
buffer but internally will get a very large buffer. The approach of
swapping sampling orders accordingly (first down, then up) might address
the memory issue but is lossy.

So instead let's fix the most pressing issue: if no information was
present in the input, nothing can be lost and we can pretend to perform
everything by a very small intermediate image.
src/imageops/sample.rs Outdated Show resolved Hide resolved
src/imageops/sample.rs Show resolved Hide resolved
@HeroicKatora HeroicKatora merged commit 3b10606 into main Oct 12, 2024
32 checks passed
@HeroicKatora HeroicKatora deleted the issue-2340 branch October 12, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory Allocation Failed in image::DynamicImage::resize_exact
4 participants