X server 1.6_beta1 pending pixman
keithp at keithp.com
Tue Nov 25 09:37:45 PST 2008
On Tue, 2008-11-25 at 17:58 +0100, Soeren Sandmann wrote:
> Keith Packard <keithp at keithp.com> writes:
> > So, we'll see if we can't get a bit of pixman review and perhaps a
> > pixman release done tomorrow so that the X server beta can head out.
> Here are some comments on the matrix code. I didn't review all the
> numericals, but nothing jumped out at me either.
I just copied the code from the X server where it hasn't demonstrated
any problems. I think most of the paths are actually tested in the
projective transform RandR work too.
> - I'd like to have the interface const correct, for example in
> pixman_transform_multily (struct pixman_transform_t *dst,
> struct pixman_transform_t *l,
> struct pixman_transform_t *r);
Yeah, I briefly considered doing that; I'll go fix it.
> - For the rotation interfaces, maybe expand the names s and c to sin
> and cos? I first thought c meant center and was then mystified what
> s could mean.
> - The interfaces that take forward/reverse matrices should probably
> accept NULL's.
> - Pixman's version numbering scheme is similar to cairo's: The git
> master version has an odd micro number, released versions have even
> micro numbers.
What version would you like?
> - The name pixman_f_transform bothers me, but I don't have a better
> suggestion since pixman_transformf would be worse.
I'll leave this alone then.
> - Is there any particular reason for the fixed point epsilon of 2 (as
> opposed to 1)?
Just giving more space to allow for rounding errors.
> - There is a comment about floating point interfaces, but the file
> contains both fixed and floating point interfaces.
I'll fix the comment.
Thanks for the review!
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 189 bytes
Desc: This is a digitally signed message part
More information about the xorg