lanczos3: Inline uv -> source pixel coord calculation and drop inv_src_size
#30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rc_size` By inlining this function it becomes clear that we're once again unnecessarily converting between uniform and pixel space. We only need the size of the source image to convert from `uv` to pixel coordinates, as the kernel after that is just integer additions in whole pixels. No need to first convert the kernel `x`/`y` index to `uv` space wrt the inverse of the source image size, only to immediately convert it back to "whole" pixels (rounding issues left aside!) again. Finally, remove the unnecessary `0.5` offset that comes out of this, as the `uv` coordinate is already centered on the target image and adding yet another centering for the destination image only puts the kernel more to the bottom-right of the desired pixel, giving it the wrong values. It is already indecisive what should happen when e.g. a 2x downsampling is happening, where the midpoint of a target pixel ends up at exactly the corner of four pixels: this should sample an even amount of pixels, while we now "sample" an uneven 7x7 kernel (but disregard the last row/column because the weight becomes `0`...).
MarijnS95
force-pushed
the
lanczos3-inline-pixel-coord-calculation
branch
from
2b3f4c4
to
99fc217
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recreating #29 because GitHub deleted the wrong branch, and even after restoring it the PR cannot be opened because "the branch has been deleted". Pfff.
By inlining this function it becomes clear that we're once again unnecessarily converting between uniform and pixel space. We only need the size of the source image to convert from
uvto pixel coordinates, as the kernel after that is just integer additions in whole pixels. No need to first convert the kernelx/yindex touvspace wrt the inverse of the source image size, only to immediately convert it back to "whole" pixels (rounding issues left aside!) again.Finally, remove the unnecessary
0.5offset that comes out of this, as theuvcoordinate is already centered on the target image and adding yet another centering for the destination image only puts the kernel more to the bottom-right of the desired pixel, giving it the wrong values. It is already indecisive what should happen when e.g. a 2x downsampling is happening, where the midpoint of a target pixel ends up at exactly the corner of four pixels: this should sample an even amount of pixels, while we now "sample" an uneven 7x7 kernel (but disregard the last row/column because the weight becomes0...).