[Pixman] [PATCHv3 0/3] Faster 90/270 degrees rotation

[Siarhei Siamashka]

On Friday 04 February 2011 13:42:42 Andrea Canciani wrote:
> On Fri, Feb 4, 2011 at 11:52 AM, Siarhei Siamashka
> 
> <siarhei.siamashka at gmail.com> wrote:
> > From: Siarhei Siamashka <siarhei.siamashka at nokia.com>
> > 
> > Changes since the previous sumbission:
> > 1. coding style fixes
> > 2. sligthly tweaked flags setting code to make it a bit faster
> > 
> > Siarhei Siamashka (3):
> >  New flags for 90/180/270 rotation
> >  C fast paths for a simple 90/270 degrees rotation
> >  test: affine-test updated to stress 90/180/270 degrees rotation more
> > 
> >  pixman/pixman-fast-path.c |  292
> > +++++++++++++++++++++++++++++++++++++++++++++ pixman/pixman-image.c    
> > |   17 +++
> >  pixman/pixman-private.h   |    3 +
> >  test/affine-test.c        |   34 +++++-
> >  4 files changed, 342 insertions(+), 4 deletions(-)
> 
> While reviewing this code I noticed that there is no fast path for flipping
> (it goes through FAST_PATH_SCALE_TRANSFORM).
> Would it be a good idea to provide that as well?
> In that case, pixman would have fast paths for all unit transforms.

Yes, something can be done for the flipping too, but not necessarily in the
same patchset as 90/270 degrees rotation.

Such support for rotated fast paths is quite limited. Now it is only SRC
operator for just a few color formats. Basically we may have to start over from
the square one and be forced to reimplement all the fast paths with the
existing design (if it is desired to perform some other compositing operations
combined with rotation). That's where JIT may be handy at some point, but let's
skip this topic for now.

But for vertical flipping it is more efficient to wrap image buffer in a new
temporary pixman_image_t, having negative stride. In this case all the
existing fast paths are perfectly usable. It may be a bit inconsistent, but
this is a special case which can be handled quite efficiently. It can be
probably even transparently done inside of pixman itself, hiding the dirty
details from the user, if the transformation matrix is detected to be vertical
flipping. BTW, this reminded me about the older simple repeat code.

> The main use I know for flipping is to pass data between negative and
> positive stride images. I don't know if it is worth the additional effort,
> but this is a known slow path in Quartz.

Is this performance bottleneck reproducible using some cairo traces (even if
exclusively in Quartz)?

> Another thing I noticed is that if we want to get real performance benefits
> we should also be careful about the stack above pixman. For example
> cairo would trigger these fast paths only if the translational part of the
> matrix is 0, because it tries to spread the translation evenly between
> the matrix and the integer offset (unless the matrix is a translation).
> The patch for cairo is trivial, 

Could you explain this problem with a bit more details? Rotated fast paths
also should work fine if the rotation is combined with arbitrary translation
(otherwise they would be not very useful).

> but other libraries/applications might not get the expected benefit from this
> change.

Surely some of the applications and libraries may have hacks and workarounds to
avoid performing rotation with pixman at any costs, because it historically has
been horribly slow doing this. And in any case, optimizing rotation in pixman
should be done first, then all these hacks can be eventually identified and
eliminated.

-- 
Best regards,
Siarhei Siamashka