ajohnson at redneon.com
Mon Apr 2 04:48:47 PDT 2012
As there has been no update to the supersampling patch posted by
Krzysztof Kosiński in 2010  I have rebased the patch and incorporated
Søren's review comments . The patches are at:
My responses to the review comments are below.
On 24/08/10 01:09, Soeren Sandmann wrote:
> 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.
As a result of an irc discussion with Søren the suggested api is:
void pixman_image_set_sampling (pixman_image_t *image,
The default sampling is PIXMAN_SAMPLE_POINT. When sampling is
PIXMAN_SAMPLE_BOX the supersampling rate is automatically computed.
> 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.
I could not figure out a good way to do this.
> - 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.
I've renamed it to POINT_SAMPLING and added it where ever it appeared to
> - 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
> to correctness.
> 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.
I've removed 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.
I've moved the small step addition to the end of the loop.
> 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).
I don't know. I left it as is.
> 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
> cairo mailing list
> cairo at cairographics.org
More information about the Pixman