From 877d9c596abd37170bc85a3f427cd6d48aecc7cb Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Mon, 8 Jan 2024 16:51:35 +0100 Subject: [PATCH] VSE: Fix various "off by half a pixel" issues in image transform Code inside `IMB_transform` (which is pretty much only used inside VSE to do translation/rotation/scale of image or movie strips) was not correctly doing mapping between pixel and texel spaces. This is similar to e.g. GPU rasterization rules and has to do with whether some coordinate refers to pixel/texel "corner" or "center" etc. It's a long topic, but short summary would be: - Coordinates refer to pixel/texel corner, - Do sampling at pixel centers, - Bilinear filter should use floor(x-0.5) and floor(x-0.5)+1 texels. Also, there was a sign error introduced in Subsampling 3x3 filter, in commit b3fd169259ac. After making the PR, I found out that this seems to fix #90785, #112923 and possibly some others. Long explanation with lots of images is in the PR. Pull Request: https://projects.blender.org/blender/blender/pulls/116628 --- source/blender/imbuf/intern/transform.cc | 31 ++++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/source/blender/imbuf/intern/transform.cc b/source/blender/imbuf/intern/transform.cc index 833d8ef52f7..95775a2ad76 100644 --- a/source/blender/imbuf/intern/transform.cc +++ b/source/blender/imbuf/intern/transform.cc @@ -81,16 +81,12 @@ struct TransformUserData { void init_add_x(const float4x4 &transform_matrix) { - const double width = src->x; - add_x = double2(transform_matrix.x_axis()) * width + double2(transform_matrix.location()); - add_x = (add_x - start_uv) * (1.0 / width); + add_x = double2(transform_matrix.x_axis()); } void init_add_y(const float4x4 &transform_matrix) { - const double height = src->y; - add_y = double2(transform_matrix.y_axis()) * height + double2(transform_matrix.location()); - add_y = (add_y - start_uv) * (1.0 / height); + add_y = double2(transform_matrix.y_axis()); } void init_subsampling(const int num_subsamples) @@ -102,7 +98,7 @@ struct TransformUserData { for (int y : IndexRange(0, num_subsamples)) { for (int x : IndexRange(0, num_subsamples)) { - double2 delta_uv = -offset_x - offset_y; + double2 delta_uv = offset_x + offset_y; delta_uv += x * subsample_add_x; delta_uv += y * subsample_add_y; subsampling.delta_uvs.append(delta_uv); @@ -313,6 +309,13 @@ class Sampler { u = wrap_uv(u, source->x); v = wrap_uv(v, source->y); } + /* BLI_bilinear_interpolation functions use `floor(uv)` and `floor(uv)+1` + * texels. For proper mapping between pixel and texel spaces, need to + * subtract 0.5. */ + if constexpr (Filter == IMB_FILTER_BILINEAR) { + u -= 0.5f; + v -= 0.5f; + } if constexpr (Filter == IMB_FILTER_BILINEAR && std::is_same_v && NumChannels == 4) { @@ -505,9 +508,10 @@ class ScanlineProcessor { private: void process_one_sample_per_pixel(const TransformUserData *user_data, int scanline) { - double2 uv = user_data->start_uv + - user_data->destination_region.x_range.first() * user_data->add_x + - user_data->add_y * scanline; + /* Note: sample at pixel center for proper filtering. */ + double pixel_x = user_data->destination_region.x_range.first() + 0.5; + double pixel_y = scanline + 0.5; + double2 uv = user_data->start_uv + user_data->add_x * pixel_x + user_data->add_y * pixel_y; output.init_pixel_pointer(user_data->dst, int2(user_data->destination_region.x_range.first(), scanline)); @@ -526,9 +530,10 @@ class ScanlineProcessor { void process_with_subsampling(const TransformUserData *user_data, int scanline) { - double2 uv = user_data->start_uv + - user_data->destination_region.x_range.first() * user_data->add_x + - user_data->add_y * scanline; + /* Note: sample at pixel center for proper filtering. */ + double pixel_x = user_data->destination_region.x_range.first() + 0.5; + double pixel_y = scanline + 0.5; + double2 uv = user_data->start_uv + user_data->add_x * pixel_x + user_data->add_y * pixel_y; output.init_pixel_pointer(user_data->dst, int2(user_data->destination_region.x_range.first(), scanline));