[cairo] [Pixman] Supersampling - 2nd attempt
sandmann at daimi.au.dk
Mon Aug 23 08:39:26 PDT 2010
Krzysztof Kosiński <tweenk.pl at gmail.com> writes:
> This is the second attempt at bitmap supersampling in Pixman.
Some general comments:
I generally like where this is going. It's certainly a large quality
improvement, and the code is well-written. And of course, it fixes
what is probably the biggest image quality issue in pixman.
As I mentioned elsewhere, I don't think there should be any
automatically computed supersampling. While it might be nice to make
image scaling look good without any changes to cairo, I think we are
going to want more fine grained control over the API. Also, simply
changing pixman behavior will cause the X server behavior to change,
which seems undesirable. So by default pixman should still just point
A question is, what should the API look like? It is probably useful to
make sure we can user other algorithms than regular supersampling in
the future. Suggestions welcome.
Some more specific comments on the code:
- Is it possible to share the supersampling code between the affine
and projective cases? Maybe the super sampling could be done in an
inline function that takes other inline functions as arguments.
- The NO_SUPERSAMPLING flag needs to be added all over the place. None
of the existing fast paths can deal with super sampling. Also, maybe
it should be renamed to POINT_SAMPLING instead; that would get rid
of the negative in the name.
- Can we get away with using a simpler algorithm? I can see why you do
the computation of all the remainders and I appreciate the attention
However, in most cases, the rate_x/rate_y is going to be small
compared to the fixed point unit of 65536, so it might be possible
to just compute a "small step" and a "big step":
normal_step = ux/rate_x
initial_step = (ux - (rate - 1) * big_step) / 2
and then just use those instead of the remainder computations.
> + uxstart = (ux * (rate_x-1) + rate_x) / (2*rate_x);
This seems to compute a uxstart of
ux / 2 - ux / (2 * rate_x)
which later gets subtracted from the transformed x coordinate to get
the first subsample coordinate. So far, so good. ux/(2 * rate_x) is
the small initial step inside the transformed pixel, and we do need to
subtract ux/2 to get to the corner of the pixel. But then it seems
that _before_ actually sampling for the first time, you end up adding
> + spx += uxfrac;
which offsets the position by uxfrac.
Also, just to make sure, the "+ rate_x" in the first expression is a
rounding correction, and that is the reason it shouldn't converted to
fixed point? (Adding a fixed point and an integer triggers a red flag
for me, but it might be correct).
Finally, I suspect getting rid of the four per-pixel divisions with
samples is going to be a noticable performance win. It should be
possible to just compute (1/samples) up front and then multiply with
More information about the cairo