[Pixman] [PATCH 03/14] More general BILINEAR=>NEAREST reduction
Oded Gabbay
oded.gabbay at gmail.com
Fri Apr 29 12:38:35 UTC 2016
On Tue, Apr 12, 2016 at 7:03 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> Only minor comments on the large nearest patch at the bottom
>
>
> On 04/11/2016 07:36 PM, Søren Sandmann Pedersen wrote:
>>
>> Generalize and simplify the code that reduces BILINEAR to NEAREST so
>> that the reduction happens for all affine transformations where
>> t00...t12 are integers and (t00 + t01) and (t10 + t11) are both
>> odd. This is a sufficient condition for the resulting transformed
>> coordinates to be exactly at the center of a pixel so that BILINEAR
>> becomes identical to NEAREST.
>>
>> V2: Address some comments by Bill Spitzak
>>
>> Signed-off-by: Søren Sandmann <soren.sandmann at gmail.com>
>> ---
>> pixman/pixman-image.c | 66
>> +++++++++++++++++++++++++++++----------------------
>> 1 file changed, 38 insertions(+), 28 deletions(-)
>>
>> diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
>> index 1ff1a49..681864e 100644
>> --- a/pixman/pixman-image.c
>> +++ b/pixman/pixman-image.c
>> @@ -335,37 +335,47 @@ compute_image_info (pixman_image_t *image)
>> {
>> flags |= FAST_PATH_NEAREST_FILTER;
>> }
>> - else if (
>> - /* affine and integer translation components in matrix ... */
>> - ((flags & FAST_PATH_AFFINE_TRANSFORM) &&
>> - !pixman_fixed_frac (image->common.transform->matrix[0][2] |
>> - image->common.transform->matrix[1][2]))
>> &&
>> - (
>> - /* ... combined with a simple rotation */
>> - (flags & (FAST_PATH_ROTATE_90_TRANSFORM |
>> - FAST_PATH_ROTATE_180_TRANSFORM |
>> - FAST_PATH_ROTATE_270_TRANSFORM)) ||
>> - /* ... or combined with a simple non-rotated translation
>> */
>> - (image->common.transform->matrix[0][0] == pixman_fixed_1
>> &&
>> - image->common.transform->matrix[1][1] == pixman_fixed_1
>> &&
>> - image->common.transform->matrix[0][1] == 0 &&
>> - image->common.transform->matrix[1][0] == 0)
>> - )
>> - )
>> + else if (flags & FAST_PATH_AFFINE_TRANSFORM)
>> {
>> - /* FIXME: there are some affine-test failures, showing that
>> - * handling of BILINEAR and NEAREST filter is not quite
>> - * equivalent when getting close to 32K for the translation
>> - * components of the matrix. That's likely some bug, but for
>> - * now just skip BILINEAR->NEAREST optimization in this case.
>> + /* Suppose the transform is
>> + *
>> + * [ t00, t01, t02 ]
>> + * [ t10, t11, t12 ]
>> + * [ 0, 0, 1 ]
>> + *
>> + * and the destination coordinates are (n + 0.5, m + 0.5).
>> Then
>> + * the transformed x coordinate is:
>> + *
>> + * tx = t00 * (n + 0.5) + t01 * (m + 0.5) + t02
>> + * = t00 * n + t01 * m + t02 + (t00 + t01) * 0.5
>> + *
>> + * which implies that if t00, t01 and t02 are all integers
>> + * and (t00 + t01) is odd, then tx will be an integer plus
>> 0.5,
>> + * which means a BILINEAR filter will reduce to NEAREST. The
>> same
>> + * applies in the y direction
>> */
>> - pixman_fixed_t magic_limit = pixman_int_to_fixed (30000);
>> - if (image->common.transform->matrix[0][2] <= magic_limit &&
>> - image->common.transform->matrix[1][2] <= magic_limit &&
>> - image->common.transform->matrix[0][2] >= -magic_limit &&
>> - image->common.transform->matrix[1][2] >= -magic_limit)
>> + pixman_fixed_t (*t)[3] = image->common.transform->matrix;
>> +
>> + if ((pixman_fixed_frac (
>> + t[0][0] | t[0][1] | t[0][2] |
>> + t[1][0] | t[1][1] | t[1][2]) == 0) &&
>> + (pixman_fixed_to_int (
>> + (t[0][0] + t[0][1]) & (t[1][0] + t[1][1])) % 2) == 1)
>> {
>> - flags |= FAST_PATH_NEAREST_FILTER;
>> + /* FIXME: there are some affine-test failures, showing
>> that
>> + * handling of BILINEAR and NEAREST filter is not quite
>> + * equivalent when getting close to 32K for the
>> translation
>> + * components of the matrix. That's likely some bug, but
>> for
>> + * now just skip BILINEAR->NEAREST optimization in this
>> case.
>> + */
>
>
> May be a good idea to point out that the bug is (probably) in BILINEAR, not
> in NEAREST. Also you think this may have been fixed so this could be
> removed, but that would be a different patch.
>
>> + pixman_fixed_t magic_limit = pixman_int_to_fixed (30000);
>> + if (image->common.transform->matrix[0][2] <= magic_limit
>> &&
>> + image->common.transform->matrix[1][2] <= magic_limit
>> &&
>> + image->common.transform->matrix[0][2] >= -magic_limit
>> &&
>> + image->common.transform->matrix[1][2] >= -magic_limit)
>
>
> I would change these to use 't'.
Søren,
Is it OK if I will change those to 't' and push the patch with that change ?
Oded
>
>> + {
>> + flags |= FAST_PATH_NEAREST_FILTER;
>> + }
>> }
>> }
>> break;
>>
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
With that change, this patch is:
Acked-by: Oded Gabbay <oded.gabbay at gmail.com>
More information about the Pixman
mailing list