[cairo] Cairo/pixman behavior with large translations

[Siarhei Siamashka]

Hello,

There is an old (low priority?) Mozilla bug which just got
a recent "ping":
    https://bugzilla.mozilla.org/show_bug.cgi?id=654570

Basically, when you go to http://www.rifters.com/real/Blindsight.htm
with the Firefox browser and scroll the page down, the background of
the page turns black (after scrolling around ~32K pixels). This is
reproducible with "gfx.xrender.enabled" set to "false" in about:config
Firefox settings, so that the client side software rendering via
cairo/pixman is used. Both cairo and pixman are the today's master
branches from git.

The problematic image has width=1800, height=7 and NORMAL repeat set.

The "src_x", "src_y" arguments and the transformation matrix as they
come to the pixman_image_composite32() function while scrolling the
page down are the following:

src_x=0, src_y=0
src_x=0, src_y=977
src_x=0, src_y=1197
src_x=0, src_y=1712
src_x=0, src_y=1913
...
src_x=0, src_y=32328
src_x=0, src_y=32540
src_x=0, src_y=16384, {{65536, 0, 0}, {0, 65536, 1073741824}, {0, 0, 65536}}
src_x=0, src_y=16439, {{65536, 0, 0}, {0, 65536, 1077280768}, {0, 0, 65536}}
src_x=0, src_y=16452, {{65536, 0, 0}, {0, 65536, 1078132736}, {0, 0, 65536}}
...
src_x=0, src_y=32649, {{65536, 0, 0}, {0, 65536, 2139684864}, {0, 0, 65536}}
src_x=0, src_y=32747, {{65536, 0, 0}, {0, 65536, 2146107392}, {0, 0, 65536}}
src_x=0, src_y=32832, {{65536, 0, 0}, {0, 65536, -2143289344}, {0, 0, 65536}}
src_x=0, src_y=32877, {{65536, 0, 0}, {0, 65536, -2140340224}, {0, 0, 65536}}
...
src_x=0, src_y=65309, {{65536, 0, 0}, {0, 65536, -14942208}, {0, 0, 65536}}
src_x=0, src_y=65464, {{65536, 0, 0}, {0, 65536, -4784128}, {0, 0, 65536}}
src_x=0, src_y=65574, {{65536, 0, 0}, {0, 65536, 2490368}, {0, 0, 65536}}
src_x=0, src_y=65662, {{65536, 0, 0}, {0, 65536, 8192000}, {0, 0, 65536}}
...

The first thing to fail is pixman. Around 32K pixels, cairo starts
attempting to evenly spread this large translate between "src_y"
and "matrix[1][2]" in _cairo_matrix_to_pixman_matrix_offset() function:
    http://cgit.freedesktop.org/cairo/tree/src/cairo-matrix.c?id=1.12.8#n1094
However pixman can't digest this correctly and bails out in a check
performed by analyze_extent() function:
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman.c?id=pixman-0.28.0#n487

Around 64K pixels down, the next thing to fail is cairo. Spreading the
large translation does not work correctly anymore, and we get an
overflow in "matrix[1][2]" (-2143289344 in the log above). After this
point no fixes in pixman can help anymore.

The most funny thing is that this operation itself is very simple.
Because we have NORMAL repeat set on this image, there is even no need
to play with the transformation matrix. We can just tweak "src_y" to
bring it within image bounds and be done with it (negative src_y might
need some special handling though):

    src_y = src_y % height;

This fix is better to be applied in cairo, because xlib backend has a
hard 16-bit limit for the offsets (src-x, src-y, mask-x, mask-y, dst-x,
dst-y) enforced by XRENDER spec. An interesting additional exercise is
to investigate whether similar tricks can work for the other repeat
types and also in the case if the transformation is not a simple
translation (for example scaling, rotation, etc.).


Now back to pixman problems. It would be nice if it could correctly
handle composite operations with any input that is valid for XRENDER.
One example, the split between "src_y" and "matrix[1][2]" done by
cairo in this particular use case:
    src_x=0, src_y=16384, {{65536,0,0},{0,65536,1073741824},{0,0,65536}}

The main culprits are functions pixman_transform_point_3d() and
pixman_transform_point() here:
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman-matrix.c?id=pixman-0.28.0#n49
They perform multiplication of a matrix (16.16 fixed point) with a
vector (16.16 fixed point), to get a vector with 16.16 fixed point
values. This code can be upgraded to perform multiplication of the same
16.16 fixed point matrix, but use something like 31.16 fixed point
for input vectors and get results in the 48.16 fixed point output
vectors. The caller then should be able to deal with the 48.16
results depending on the type of repeat set for the image. One
example is here:
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman-inlines.h?id=pixman-0.28.0#n417
In this code, the "src_x" and "src_y" arguments are originally coming
from pixman_image_composite32() function and are already supposed to be
larger than 16 bits after the following commit:
    http://cgit.freedesktop.org/pixman/commit/?id=e841c556d59ca0aa6d86eaf6dbf061ae0f4287de
If they are getting converted to fixed point, we already get something
like 32.16 fixed point values which are asking for a larger vector
argument for pixman_transform_point_3d() function (though we might
want not to allow the use of full 32-bit range for "src_x" and "src_y"
in order to keep some headroom and safeguard against overflows). In any
case, immediately after pixman_transform_point_3d() we are
tweaking v.vector[0] and v.vector[1] according to the repeat type:
    http://cgit.freedesktop.org/pixman/tree/pixman/pixman-inlines.h?id=pixman-0.28.0#n432
Updating this code ("repeat" and "pad_repeat_get_scanline_bounds"
functions) to deal with 48.16 fixed point values from the vector can't
be too hard.

Of course, a lot of tweaks and updates will need to be done all over
the pixman code. And the test suite needs new tests for all these
corner cases. We even almost have some test here:
    http://cgit.freedesktop.org/pixman/tree/test/scaling-crash-test.c?id=pixman-0.28.0#n7
Except that "or, if the transformation is such that we can't composite
anything at all, that nothing has changed in the destination" part
needs to be removed because it is not going to be a desirable behavior
anymore.

However this all is not something that I'm planning to do myself at
least within the next few days or weeks. It was just a notification
from the Mozilla bugtracker and I thought that it may be worth bringing
it up here.

-- 
Best regards,
Siarhei Siamashka