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

Add basic mimpmap downscaling support #145

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jermy
Copy link
Contributor

@jermy jermy commented Dec 24, 2024

This is small patch to fix quality issues when scaling images down. It'll create a number of mipmap/half-scaled versions of a given source as required and use the next largest version as the source for bilinear or bicubic scaling.

This uses the same downsampling functions as Skia, but similarly doesn't run SIMD versions of these. As they are integer-based, these should be reasonably quick.

This addresses issue #13, but probably wants some work and tests before merging.

@jermy jermy force-pushed the add_basic_mipmap_scaling branch 2 times, most recently from 2de9b5b to 4cffa27 Compare December 24, 2024 13:02
@RazrFalcon
Copy link
Collaborator

Thanks a lot 🎉

src/mipmap.rs Outdated Show resolved Hide resolved
src/pipeline/blitter.rs Outdated Show resolved Hide resolved
This is small patch to fix quality issues when scaling images down.
It'll create a number of mipmap/half-scaled versions of a given source
as required and use the next largest version as the source for bilinear
or bicubic scaling.

This uses the same downsampling functions as Skia, but similarly doesn't
run SIMD versions of these. As they are integer-based, these should be
reasonably quick.
@jermy jermy force-pushed the add_basic_mipmap_scaling branch from 4cffa27 to a79233a Compare December 27, 2024 14:11
@jermy jermy requested a review from waywardmonkeys December 27, 2024 14:14
Copy link
Collaborator

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking pretty good!

Any thoughts on adding a couple of tests?

@jermy
Copy link
Contributor Author

jermy commented Jan 6, 2025

@waywardmonkeys any idea of what sort of tests would be useful in this case? I'm imagining just integration tests, but if there are sensible unit tests I could add those? (I think compute_required_levels is the only non-complicated function).

Integration tests would involve some source and expected PNGs so might be a little tricky to keep the size down, but I'm guessing would include:

  • That a resized image looks like the same as it currently does if FilterQuality::Nearest is used
  • That a "better" image is produced if FilterQuality::Bicubic or FilterQuality::Bilinear are used
  • That a 1x1 image scaled down doesn't cause a crash
  • That all of the downsample functions are used - I think a source image of 21x19 scaled down to less than 1x1 (ie. 2% size) would be the smallest source image to invoke all of these in one pass

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.

3 participants