Siarhei Siamashka <siarhei.siamas...@gmail.com> writes: > 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.
There is really a lot to be said for this 31.16 format. Intermediate results from matrix and vector multiplications fit in 64 bits, and image access coordinates up to +/- 1 billion pixels can be supported. Yet it is still a fixed-point format so there won't be any weird effects where positions are rounded differently depending on where they are on the screen. If 64 bit coordinates become a performance problem, a new flag, "FAST_PATH_16_16_IS_ENOUGH" or something, could be added so that fast paths could use 32 bit coordinates. (We used to have something like this before pixman started to just return without compositing when it detects overflow). It's very tempting to switch pixman over to using this format internally. The one thing I'm not sure of is whether this bug: https://bugs.freedesktop.org/show_bug.cgi?id=15355 "Under a non-affine transform, or, in fact, any transform where the 'w' component of the (u,v,w) homogeneous source coordinate is not 1, the representation of u,v,w as 16.16 fixed point numbers will over/under flow on a regular basis even given fairly mundane transformations (like a simple keystone correction). I've fixed the intel driver to do these computations in floating point; I'm not sure we want to try to use fixed point here." is adequately taken care off by using this format. It's worth pointing out that even if there are useful non-affine transformations that still overflow, a homogeneous transformation matrix doesn't change if multiplied or divided by an arbitrary constant, so if worse comes to worst, we could drop precision by rounding the overflowing result down to 47 bits and then dividing all the other entries by a similar amount, in effect faking a floating point type. Keith, comments on this would definitely be appreciated. For the RandR use cases, would 31.16 be good enough? Søren _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman