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

[Andrea Canciani]

On Tue, Feb 8, 2011 at 1:39 AM, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
> 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.

Another way to generalize this might be tiled images. But, again, it
would require big changes in pixman (and in software accessing pixman
images), so it can probably be safely ignored right 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.

I might add that if we have fast paths for 0/90/180/270, then any other unit
transform can be obtained by vertical flipping. This is the reason why I said
that most of the code can be shared between these fast paths.

>
>> 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)?

Robert O'Callahan might be able to provide some hints on how to reproduce
this. He pointed out the problem in a post of his blog:
http://weblogs.mozillazine.org/roc/archives/2010/05/cglayer_perform.html

I don't know if this is already a problem in cairo-quartz... it seems to have
worse bottlenecks :(

>
>> 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).

Sorry, I was tricked by the test for an affine matrix:
http://cgit.freedesktop.org/~siamashka/pixman/tree/pixman/pixman-image.c?h=sent/fast-simple-rotate-20110204&id=5dd8ffb018be3a12ea1048f9b1ac452a634b320a#n205
I incorrectly thought it was testing translational components to be 0.

Andrea

>
>> 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
>