search

Traverse-Research / ispc-downsampler

Image downsampler using a Lanczos filter implemented in ISPC
Other
11 stars 1 forks source link

lanczos3: Scale input kernel by source size, not target size #26

Closed MarijnS95 closed 1 year ago

MarijnS95 commented 1 year ago

When the 7x7 kernel runs over pixels in the source image, its offset is multiplied by the inverse of the target image size to turn these integer coordinates into float coordinates (only to turn them back into integer on the next line :facepalm:). This means that the floating-point offset of the kernel relative to the center of the source pixel we are sampling is now relative to the dimensions of the target image instead of the source image, leading us to skip source pixels as the target is smaller than the source and thus the offset is larger than one step in source-pixels.

Take for example the Dalai Lama test picture from http://www.ericbrasseur.org/gamma.html?i=1 with alternating green and pink lines. When downsampling it 4 times using our examples/test.rs only pink (or green) lines are sampled consecutively, highlighting that we are skipping the pixel in-between entirely:

square_test_result

After this PR, all pixels are properly sampled:

square_test_result

This leaves the image grey with some artifacts, highlighting the linearization issues pointed out in #25.

Note that I am still completely unsure if the same fix should be applied to when we calculate the "center pixel" which is also converted to be relative to the target image:

https://github.com/Traverse-Research/ispc-downsampler/blob/dff71fbec2b21ca3787439dd8550cf877d142c70/src/ispc/kernels/lanczos3.ispc#L55-L59

An alternative fix is to revisit this code and convert it to compute in integer space instead.

MarijnS95 commented 1 year ago

Note that I am still completely unsure if the same fix should be applied to when we calculate the "center pixel" which is also converted to be relative to the target image:

Nope, but it could be drastically simplified. Understanding what this calculation does lead me to #28.

MarijnS95 commented 1 year ago

Note that this variable is once again removed in #29, as it turns out to be unnecessary (who'd have thought) after using integer space math as suggested above.