[Pixman] [PATCH/RFC 1/2] New FAST_PATH_SIMPLE_ROTATE_TRANSFORM flag

[Siarhei Siamashka]

On Monday 02 August 2010 17:08:58 Soeren Sandmann wrote:
> Siarhei Siamashka <siarhei.siamashka at gmail.com> writes:
> > +static pixman_bool_t
> > +has_suitable_filter_for_simple_rotate (pixman_image_t *image)
> > +{
> > +    if (image->common.filter == PIXMAN_FILTER_NEAREST)
> > +	return TRUE;
> > +
> > +    if (image->common.filter == PIXMAN_FILTER_BILINEAR &&
> > +	pixman_fixed_frac (image->common.transform->matrix[0][2]) == 0 &&
> > +	pixman_fixed_frac (image->common.transform->matrix[1][2]) == 0)
> > +    {
> > +	return TRUE;
> > +    }
> > +
> > +    return FALSE;
> > +}
> 
> This seems to basically compute whether the filter is effectively
> NEAREST. If so, would it be possible to just set the
> HAS_NEAREST_FILTER flag in that case, and then rely on that instead?
> 
> Doing it this way would make it possible to hit the NEAREST scaling
> fast paths in more cases too.
> 
> The transformation check could be generalized too I think. Suppose the
> transform is t00..t22, and the input coordinates are (n + 0.5, m + 0.5).
> Then the transformed x coordinate would be:
> 
>         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 the bilinear
> filter will reduce to NEAREST. Something similar applies in
> the Y direction. (And the transform would have to be affine).

Yes, using PIXMAN_FILTER_NEAREST flag for the simple rotation fast paths is 
reasonable. I'll do this in the next patch revision.

Also like you explained above, setting PIXMAN_FILTER_NEAREST flag for some of
the cases where bilinear filter effectively works in the the same way as the
nearest filter is possible and can make sense. But in practice, it is going
to improve performance just for exactly those worst looking examples of
bilinear downscaling, which are the primary motivating factor for developing a
better filter ;)

In the patch being discussed I only wanted to address the case of simple 
90/180/270 rotation with a bilinear filter. Just like the standard non-rotated
fast paths are still taken even if bilinear filter is used, and no transform is
set.

I think that this more intelligent setting of PIXMAN_FILTER_NEAREST flag for
some of the uses of bilinear filter could be postponed a bit. It is not
directly related to the rotation fast paths. And after all, it is a user's
fault for setting bilinear filter without real need. I'm just afraid that this
new functionality may need more complex tests to be sure that it is always
correct.

Btw, taking it even further and handling even more exotic cases with dedicated 
fast paths, the PIXMAN_FILTER_NEAREST flag could be split to handle vertical
and horizontal directions separately.

> > +	const pixman_transform_t *t = image->common.transform;
> 
> It would make sense to move this shorthand to the top of the function
> and use it in other places too.

OK

> > +	if (t->matrix[0][1] == 0 && t->matrix[1][0] == 0)
> > 
> >  	{
> > 
> > +	    if (t->matrix[0][0] == -pixman_fixed_1 &&
> > +		t->matrix[1][1] == -pixman_fixed_1 &&
> > +		has_suitable_filter_for_simple_rotate (image))
> > +	    {
> > +		/* 180 degrees rotation */
> > +		flags |= FAST_PATH_SIMPLE_ROTATE_TRANSFORM;
> > +	    }
> > 
> >  	    flags |= FAST_PATH_SCALE_TRANSFORM;
> >  	
> >  	}
> 
> Isn't this case already covered by the existing NEAREST scalers?

Yes, but it is handled there in a somewhat perverted way :) A special 180
degrees rotated fast path may be easily SIMD optimized by using byte reversal
or shuffle instructions.

Another question is how to manage the flags bits space more efficiently. For 
example, allocating its own bit for 90, 180 and 270 degrees rotation could be 
also possible. Or alternatively combining 90/270 in one flag and having 180 
degrees handled separately (or even dropping it altogether, it is not so 
important). Which one do you think would be the best?

-- 
Best regards,
Siarhei Siamashka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20100826/d21a026e/attachment.pgp>