Re: [Pixman] [PATCH] test: Add cover-test v5
On Wed, 16 Sep 2015 12:47:34 +0300 Oded Gabbay wrote: > On Fri, Sep 11, 2015 at 12:06 PM, Pekka Paalanen wrote: > > On Thu, 10 Sep 2015 16:11:07 +0100 > > "Ben Avison" wrote: > > > >> On Thu, 10 Sep 2015 13:16:22 +0100, Pekka Paalanen > >> wrote: > >> > Changes in v5: > >> > Skip if fenced memory is not supported. > >> > >> OK, I see how you've done that - fence_get_page_size() returning 0. > >> > >> I have no further comments. Is it appropriate for me to say I'll give a > >> Reviewed-by since I wrote the original version of the patch? > > > > Hi Ben, > > > > I think we can do it by adding this bunch of tags to the commit message: > > > > Signed-off-by: Ben Avison > > Reviewed-by: Pekka Paalanen > > [Pekka: changes in v4 and v5] > > Signed-off-by: Pekka Paalanen > > Reviewed-by: Ben Avison > > > > You will naturally be the patch author. This should convey exactly what > > we did. > > > > Oded wants to have a final look too, so I'll see about pushing this on > > Monday. > > Acked-by: Oded Gabbay Pushed: 812c9c9..4c71f59 master -> master Thanks, pq pgpDvHVwGIQwz.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test v5
On Fri, Sep 11, 2015 at 12:06 PM, Pekka Paalanen wrote: > On Thu, 10 Sep 2015 16:11:07 +0100 > "Ben Avison" wrote: > >> On Thu, 10 Sep 2015 13:16:22 +0100, Pekka Paalanen >> wrote: >> > Changes in v5: >> > Skip if fenced memory is not supported. >> >> OK, I see how you've done that - fence_get_page_size() returning 0. >> >> I have no further comments. Is it appropriate for me to say I'll give a >> Reviewed-by since I wrote the original version of the patch? > > Hi Ben, > > I think we can do it by adding this bunch of tags to the commit message: > > Signed-off-by: Ben Avison > Reviewed-by: Pekka Paalanen > [Pekka: changes in v4 and v5] > Signed-off-by: Pekka Paalanen > Reviewed-by: Ben Avison > > You will naturally be the patch author. This should convey exactly what > we did. > > Oded wants to have a final look too, so I'll see about pushing this on > Monday. > > > Thanks, > pq > > ___ > Pixman mailing list > Pixman@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/pixman > Hi, Acked-by: Oded Gabbay Oded ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test v5
On Thu, 10 Sep 2015 16:11:07 +0100 "Ben Avison" wrote: > On Thu, 10 Sep 2015 13:16:22 +0100, Pekka Paalanen > wrote: > > Changes in v5: > > Skip if fenced memory is not supported. > > OK, I see how you've done that - fence_get_page_size() returning 0. > > I have no further comments. Is it appropriate for me to say I'll give a > Reviewed-by since I wrote the original version of the patch? Hi Ben, I think we can do it by adding this bunch of tags to the commit message: Signed-off-by: Ben Avison Reviewed-by: Pekka Paalanen [Pekka: changes in v4 and v5] Signed-off-by: Pekka Paalanen Reviewed-by: Ben Avison You will naturally be the patch author. This should convey exactly what we did. Oded wants to have a final look too, so I'll see about pushing this on Monday. Thanks, pq pgpJs3xAV0QWD.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test v5
On Thu, 10 Sep 2015 13:16:22 +0100, Pekka Paalanen wrote: Changes in v5: Skip if fenced memory is not supported. OK, I see how you've done that - fence_get_page_size() returning 0. I have no further comments. Is it appropriate for me to say I'll give a Reviewed-by since I wrote the original version of the patch? Ben ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 03 Sep 2015 13:59:07 +0100 "Ben Avison" wrote: > On Thu, 03 Sep 2015 11:13:25 +0100, Pekka Paalanen > wrote: > > D'oh, of course, that's why Oded didn't see a segfault. But he did say > > the CRC does not match on PPC64, neither LE nor BE. > > Just to be clear, is that still an issue after I fixed the DST_HEIGHT > SRC_HEIGHT bug? He says he tested v3, so that should be fixed. I suppose the culprit lies with page size. Thanks, pq pgpKK60wbuOMB.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 03 Sep 2015 17:14:05 +0100, Siarhei Siamashka wrote: On Thu, 03 Sep 2015 13:59:07 +0100 "Ben Avison" wrote: Previously, the flag definition was overly cautious, resulting in some cases which could have been handled by cover paths instead being sent to repeat paths. I first encountered this myself with lowlevel-blt-bench, which routinely used repeat paths for its bilinear tests Oh, this was actually intended. The bilinear scaling benchmark in the lowlevel-blt-bench program tries to stress the use case, where we are fetching pixels from the outside of the source image a little bit. The reason is that this is quite typical for bilinear scaling and that's how it is designed to work. If this whole ordeal with the new COVER flag interpretation was just a way to make it run fast in lowlevel-blt-bench, then this new interpretation may not make much real practical sense. That was just where it started... I should maybe explain that I encountered it when I was writing bilinear fetchers for ARMv6, which (at the time (*)) only handled the COVER case. This meant that lowlevel-blt-bench was useless to me as it stood, because my COVER fetchers were never selected, and it was important to me because I rely on it to be able to compare different implementations (for example to select the best prefetch distance). I was worried there might be objections to changing lowlevel-blt-bench, and I also knew that I followed different code paths for different scale factors (which are fixed in lowlevel-blt-bench), many of which were so extreme that the L1/L2/M/HT/VT/RT/R tests of lowlevel-blt-bench no longer made sense - so that's how I came to write affine-bench. But while writing affine-bench, it became apparent that it's really complicated from a high level caller's point of view to construct a transformation matrix that's guaranteed to hit a COVER path - and it requires knowledge of details of the internal implementation of the guts of Pixman, even after you deal with the obsolete 8*pixman_fixed_e adjustment. This seemed wrong to me; I can imagine applications might well want to use Pixman to scale between pixel centres rather than pixel edges, and to expect such an application to know whether to exclude the left pixel, or the right pixel, or both, or neither, and the same in the vertical direction (for which the rules were often different) in order to get the best performance is surely unreasonable. Ben (*) Actually, I've been revisiting this recently, and for various reasons I've added repeat support to these fetchers. But now I'm getting a long way ahead of myself... ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 03 Sep 2015 13:59:07 +0100 "Ben Avison" wrote: > On Thu, 03 Sep 2015 11:13:25 +0100, Pekka Paalanen > wrote: > > Unless, you go and change the implementation meaning of Pixman's cover > > flags which, reading your other reply, is what you are doing. I'm > > finally starting to see it. > > Glad the penny has dropped! The issue has been muddied somewhat by the > 8*epsilon border. Perhaps I should have reposted the series as soon as it > became clear what the history was in that respect. Yes, sure. I'm also waiting for an updated patch set. Because dealing with the 8*epsilon border separately is a rather major change, affecting the way how the set is split into patches and the wording of commit messages. We need a new patch set before starting a new review round. Sorry for not communicating this explicitly enough. > (from your other post on this thread) > > This is *not* for protecting against out-of-bounds access, but this is > > allowing for the extreme cases where a cover path is still > > *theoretically* possible. (Ignoring what the COVER flags actually mean > > in Pixman implementation.) > > Yes. Pixman as a whole should be able to handle any coordinates without > bounds violations. The COVER_CLIP flags just determine the threshold > coordinates at which it chooses to use a cover path rather than the > appropriate repeat path (PAD, NONE, NORMAL or REFLECT); while you would > get the correct mathematical result even if always use the repeat path, > the cover path is very likely to be faster because it will be implemented > in a way that assumes it doesn't need to do bounds checking (because the > COVER_CLIP calculation has effectively done the bounds check for it). For BILINEAR scaling, the NONE and PAD repeats generally do not need bounds checking either. So they should be as fast as COVER, except for having a bit higher setup overhead. And in the special edge case where bilinear weight coefficients are zero for the pixels fetched from the outside of the source image (your new interpretation of the COVER flag), we can also safely reduce NORMAL and REFLECT repeats to NONE or PAD. > Previously, the flag definition was overly cautious, resulting in some > cases which could have been handled by cover paths instead being sent to > repeat paths. I first encountered this myself with lowlevel-blt-bench, > which routinely used repeat paths for its bilinear tests Oh, this was actually intended. The bilinear scaling benchmark in the lowlevel-blt-bench program tries to stress the use case, where we are fetching pixels from the outside of the source image a little bit. The reason is that this is quite typical for bilinear scaling and that's how it is designed to work. If this whole ordeal with the new COVER flag interpretation was just a way to make it run fast in lowlevel-blt-bench, then this new interpretation may not make much real practical sense. And why even stop here? If we can prove that the scaling factor, used by lowlevel-blt-bench, in fact behaves exactly as identity transform for certain composite operations, then it probably can be optimized too! Well, maybe we need some improvements for the scaling benchmarks in lowlevel-blt-bench. We can just measure performance numbers for NONE, PAD, NORMAL, REFLECT and COVER cases and report them all. The scaling factor needs to be adjusted, so that we hit these particular code paths. When we get the performance numbers for different repeat cases, then we can easily compare them. Like I said, NONE and PAD performance should be normally roughly the same as COVER. Not all the fast paths and iterators can handle NONE and PAD repeats efficiently yet (SSSE3 is a good example), but we can and should eventually get there. > but cover paths for its nearest tests. However, we have to be careful > when adjusting the flag calculations, as the cases where bounds violations > occur may only be 1*epsilon wide, and so would have a high chance of not > being detected by the previous test suite in a reasonable length of time > - hence the need for a new test program. For scaling, previously we had 'scaling-test', 'affine-test' and 'scaling-crash-test' programs, which are relying on the public pixman API. And 'scaling-helpers-test' program as a unit test for the internal 'bilinear_pad_repeat_get_scanline_bounds()' function, which is also very important. In the case of NEAREST scaling, potential reads outside of the source image can be usually easily spotted, because this pixel value contributes to the destination picture. This is not completely reliable, but still reasonably efficient in practice. But in the case of BILINEAR scaling, the new 'cover-test' is indeed very useful and represents a nice addition to the test suite. Thanks for doing this work. -- Best regards, Siarhei Siamashka ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 03 Sep 2015 11:13:25 +0100, Pekka Paalanen wrote: Unless, you go and change the implementation meaning of Pixman's cover flags which, reading your other reply, is what you are doing. I'm finally starting to see it. Glad the penny has dropped! The issue has been muddied somewhat by the 8*epsilon border. Perhaps I should have reposted the series as soon as it became clear what the history was in that respect. (from your other post on this thread) This is *not* for protecting against out-of-bounds access, but this is allowing for the extreme cases where a cover path is still *theoretically* possible. (Ignoring what the COVER flags actually mean in Pixman implementation.) Yes. Pixman as a whole should be able to handle any coordinates without bounds violations. The COVER_CLIP flags just determine the threshold coordinates at which it chooses to use a cover path rather than the appropriate repeat path (PAD, NONE, NORMAL or REFLECT); while you would get the correct mathematical result even if always use the repeat path, the cover path is very likely to be faster because it will be implemented in a way that assumes it doesn't need to do bounds checking (because the COVER_CLIP calculation has effectively done the bounds check for it). Previously, the flag definition was overly cautious, resulting in some cases which could have been handled by cover paths instead being sent to repeat paths. I first encountered this myself with lowlevel-blt-bench, which routinely used repeat paths for its bilinear tests but cover paths for its nearest tests. However, we have to be careful when adjusting the flag calculations, as the cases where bounds violations occur may only be 1*epsilon wide, and so would have a high chance of not being detected by the previous test suite in a reasonable length of time - hence the need for a new test program. When EXACT is set, cover-test only performs operations which are theoretically achievable with a cover path (and in fact, they *are* all achieved with a cover path if you apply all my patches). When it isn't set, some operations will stray into territory that definitely needs a repeat path. The size of the the fuzz factors that are applied to the coordinates was set large enough, using knowledge of the 8*epsilon factor in the old definition of the COVER_CLIP flags, that we should exercise both cover and repeat paths right up to the coordinate limits at which either old or new definition will allow them to be used. Of course, I've now changed cover-test so it doesn't do randmemset straight into fenced images anyway (due to needing to handle systems with different memory page sizes) so this is no longer an issue. D'oh, of course, that's why Oded didn't see a segfault. But he did say the CRC does not match on PPC64, neither LE nor BE. Just to be clear, is that still an issue after I fixed the DST_HEIGHT SRC_HEIGHT bug? Ben ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Wed, 02 Sep 2015 20:34:56 +0100 "Ben Avison" wrote: > On Wed, 02 Sep 2015 14:03:01 +0100, Pekka Paalanen > wrote: > > I am still a tiny bit struggling with why 31 and not 32, but it does > > add up. > > Maybe the reasoning in the comments in random_scale_factor() make more > sense? There, it only talks about the numbers as integers, avoiding the > additional confusion caused by MAX_INC being 0.32 fixed-point and the > return value being 16.16 fixed-point. > > If you still want to think of it as real numbers, then: > > * random_scale_factor() generates a scale factor between >2^(-LOG2_MAX_FACTOR-0.5) and 2^(LOG2_MAX_FACTOR+0.5) > * INV_SQRT_2_0POINT32_FIXED represents 2^-0.5 > * to get from 2^-0.5 to 2^(LOG2_MAX_FACTOR+0.5) we must multiply by >2^(LOG2_MAX_FACTOR+1) > * but to convert from 0.32 to 16.16 fixed point, we multiply by 2^(-32+16) > * so the total factor is 2^(LOG2_MAX_FACTOR+1-32+16) > * but this is < 1, so is actually a shift right by >-LOG2_MAX_FACTOR-1+32-16, which can trivially be rearranged to match >the macro definition. Yes. My point is, you spell out the -16 in the code, but not the 32 - 1. Anyway, it doesn't matter, it is correct. > >> +static void > >> +check_transform (pixman_image_t *dst_img, > >> + pixman_image_t *src_img, > >> + pixman_transform_t *transform, > >> + pixman_bool_t bilinear) > >> +{ > [...] > >> +if (bilinear) > >> +{ > >> +assert (v1.vector[0] >= pixman_fixed_1 / 2); > >> +assert (v1.vector[1] >= pixman_fixed_1 / 2); > >> +assert (v2.vector[0] <= pixman_int_to_fixed (src_img->bits.width) > >> - > >> +pixman_fixed_1 / 2); > >> +assert (v2.vector[1] <= pixman_int_to_fixed > >> (src_img->bits.height) - > >> +pixman_fixed_1 / 2); > > > > Since we had that discussion about bilinear filtering sampling pixels at > > n=0,1 instead of n=-1,0, should the last two asserts not have < instead > > of <=? > > No. calc_translate() specifically ensures that (depending upon low_align) > either the lower or upper coordinate exactly corresponds to the lowest or > highest value that contains no non-zero-weighted contribution from any > pixel beyond the source image (and the image sizes are chosen so that the > opposite coordinate should always fall within the source image too, at > the largest possible increment). These are in fact the very cases that > are most likely to trigger a fast path or fetcher to perform an out-of > bounds access, so they were deliberately included. Ah, yes, now I understand. This is *not* for protecting against out-of-bounds access, but this is allowing for the extreme cases where a cover path is still *theoretically* possible. (Ignoring what the COVER flags actually mean in Pixman implementation.) > > I mean, wouldn't sampling for x=width-0.5 cause reads on pixels > > n=width-1,width, where the latter would be out of bounds (even if > > weight zero)? > > My recent 7-patch series deals with precisely this case: > http://lists.freedesktop.org/archives/pixman/2015-August/003878.html Right! Now I think I'm getting the hang of this. :-) > Before this patch series, the conditions under which the > FAST_PATH_SAMPLES_COVER_CLIP_BILINEAR flag is set mean that this case > never gets passed to a "cover" fast path, even though such fast paths can > actually handle them by at least these three methods: > > * check the weight and don't load the upper sample if it's zero (it turns >out this can be achieved for free with ARM fetchers) > * invert the increments so that when x=width-0.5 it reads samples >n=width-2,width-1 and applies a weight of 0 to the former (I used this >for pixman-fast-path.c) > * detect the case in the C binding function and handle it separately (I >used this for the existing armv7/mips/sse2 fast paths) Now I understand that what you want to do with the mentioned patch series is to change the meaning of COVER_CLIP_BILINEAR flag, not only to remove the useless fuzz margins. Thanks, pq pgpc9Tx2IeJhu.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Wed, 02 Sep 2015 20:15:26 +0100 "Ben Avison" wrote: > On Wed, 02 Sep 2015 12:56:40 +0100, Pekka Paalanen > wrote: > > Right. I looked at fast_bilinear_cover_iter_init() and > > fast_fetch_bilinear_cover() -> fetch_horizontal(), and yes, that really > > is how it is implemented. The leftmost pixel is chosen essentially by > > n = pixman_fixed_to_int(x - pixman_fixed_1 / 2). > > > > So, pixman_fixed_1 / 2 -> 0, not -1. > > > > The thing that confuses me is that with nearest, x=0 will sample pixel > > n=-1. However with bilinear, x=0.5 will sample pixels n=0 and n=1, not > > n=-1 and n=0. > > When x=0.5, the numerical output of bilinear scaling only depends upon > source pixel n=0. The stuff about whether you consider pixel n=+1 or n=-1 > to be involved but with a weighting factor of 0 is incidental to the > numerical output. With nearest scaling, you don't have the option of > sitting on the fence so you have to round one way or the other. Yes, I have no problem with the weight zero. My point is with the memory accesses. Bilinear fetchers always fetch two pixels regardless of the coordinate, right? At least they must be allowed to do that for efficient inner loop implementations, or so I've understood the current design in Pixman. Therefore when we are tightening the cover boundaries and relaxing the conditions when cover is applicable, we must take that memory access into account, even if the weight is zero. We must know whether all paths sample on the left or the right the zero-weight pixel, when sampling the exact center of a pixel. Indeed, nearest does n = pixman_fixed_to_int(x - pixman_pixed_e) and bilinear does n0 = pixman_fixed_to_int(x - pixman_fixed_1 / 2) n1 = n0 + 1 Just like you explained, the epsilon offset applies only to nearest, and that is what I find confusing. That's all. It is like nearest "rounds" down, while bilinear "rounds" up by sampling n0 + 1 instead of n0 - 1 when x is exactly half. Anyway, that should be all settled now. We just have to remember the "always sample two pixels" when we tighten the cover boundaries for bilinear, since we must avoid accessing memory outside of the row. Unless, you go and change the implementation meaning of Pixman's cover flags which, reading your other reply, is what you are doing. I'm finally starting to see it. > Of course, I've now changed cover-test so it doesn't do randmemset > straight into fenced images anyway (due to needing to handle systems with > different memory page sizes) so this is no longer an issue. D'oh, of course, that's why Oded didn't see a segfault. But he did say the CRC does not match on PPC64, neither LE nor BE. Thanks, pq pgpyy7N6ZjHlb.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Wed, 02 Sep 2015 14:03:01 +0100, Pekka Paalanen wrote: I am still a tiny bit struggling with why 31 and not 32, but it does add up. Maybe the reasoning in the comments in random_scale_factor() make more sense? There, it only talks about the numbers as integers, avoiding the additional confusion caused by MAX_INC being 0.32 fixed-point and the return value being 16.16 fixed-point. If you still want to think of it as real numbers, then: * random_scale_factor() generates a scale factor between 2^(-LOG2_MAX_FACTOR-0.5) and 2^(LOG2_MAX_FACTOR+0.5) * INV_SQRT_2_0POINT32_FIXED represents 2^-0.5 * to get from 2^-0.5 to 2^(LOG2_MAX_FACTOR+0.5) we must multiply by 2^(LOG2_MAX_FACTOR+1) * but to convert from 0.32 to 16.16 fixed point, we multiply by 2^(-32+16) * so the total factor is 2^(LOG2_MAX_FACTOR+1-32+16) * but this is < 1, so is actually a shift right by -LOG2_MAX_FACTOR-1+32-16, which can trivially be rearranged to match the macro definition. +static pixman_image_t * +create_src_image (pixman_format_code_t fmt) +{ [...] +prng_randmemset (tmp_img->bits.bits, + tmp_img->bits.rowstride * DST_HEIGHT * sizeof (uint32_t), 0); DST_HEIGHT? Should that not be SRC_HEIGHT? I think you're right, yes - looks like careless copy-and-paste from the code to initialise the destination image. That means part of the image was uninitialised, yet it never showed up as a difference on subsequent runs. The checksums will need updating too. I'll repost shortly. +static void +check_transform (pixman_image_t *dst_img, + pixman_image_t *src_img, + pixman_transform_t *transform, + pixman_bool_t bilinear) +{ [...] +if (bilinear) +{ +assert (v1.vector[0] >= pixman_fixed_1 / 2); +assert (v1.vector[1] >= pixman_fixed_1 / 2); +assert (v2.vector[0] <= pixman_int_to_fixed (src_img->bits.width) - +pixman_fixed_1 / 2); +assert (v2.vector[1] <= pixman_int_to_fixed (src_img->bits.height) - +pixman_fixed_1 / 2); Since we had that discussion about bilinear filtering sampling pixels at n=0,1 instead of n=-1,0, should the last two asserts not have < instead of <=? No. calc_translate() specifically ensures that (depending upon low_align) either the lower or upper coordinate exactly corresponds to the lowest or highest value that contains no non-zero-weighted contribution from any pixel beyond the source image (and the image sizes are chosen so that the opposite coordinate should always fall within the source image too, at the largest possible increment). These are in fact the very cases that are most likely to trigger a fast path or fetcher to perform an out-of bounds access, so they were deliberately included. I mean, wouldn't sampling for x=width-0.5 cause reads on pixels n=width-1,width, where the latter would be out of bounds (even if weight zero)? My recent 7-patch series deals with precisely this case: http://lists.freedesktop.org/archives/pixman/2015-August/003878.html Before this patch series, the conditions under which the FAST_PATH_SAMPLES_COVER_CLIP_BILINEAR flag is set mean that this case never gets passed to a "cover" fast path, even though such fast paths can actually handle them by at least these three methods: * check the weight and don't load the upper sample if it's zero (it turns out this can be achieved for free with ARM fetchers) * invert the increments so that when x=width-0.5 it reads samples n=width-2,width-1 and applies a weight of 0 to the former (I used this for pixman-fast-path.c) * detect the case in the C binding function and handle it separately (I used this for the existing armv7/mips/sse2 fast paths) The fuzz factor added into the non-EXACT case means that even with the old rules for the cover clip flag, we should end up exercising both cover fast paths and repeat fast paths at least some of the time, right up to the limits of their applicability. Ben ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Wed, 02 Sep 2015 12:56:40 +0100, Pekka Paalanen wrote: Right. I looked at fast_bilinear_cover_iter_init() and fast_fetch_bilinear_cover() -> fetch_horizontal(), and yes, that really is how it is implemented. The leftmost pixel is chosen essentially by n = pixman_fixed_to_int(x - pixman_fixed_1 / 2). So, pixman_fixed_1 / 2 -> 0, not -1. The thing that confuses me is that with nearest, x=0 will sample pixel n=-1. However with bilinear, x=0.5 will sample pixels n=0 and n=1, not n=-1 and n=0. When x=0.5, the numerical output of bilinear scaling only depends upon source pixel n=0. The stuff about whether you consider pixel n=+1 or n=-1 to be involved but with a weighting factor of 0 is incidental to the numerical output. With nearest scaling, you don't have the option of sitting on the fence so you have to round one way or the other. If you look at the following ASCII art diagrams, the vertical axis is the contribution from source pixel 0 (+) and source pixel 1 (x). Where these coincide, I have used a *. Each column represents an increment of 0.125 (you'll have to read the labels on the horizontal axis vertically). Upper diagram is nearest, lower diagram is bilinear. * - - + + + + + + 1 0 0 0 1 1 2 2 3 . . . . . . . . . 0 5 0 5 0 5 0 5 0 + x + + x x + * x + x + x * * See how the bilinear diagram is already symmetrical; no rounding is needed of coordinates on the horizontal axis. Of course, coordinates actually have 13 more bits of granularity than this, but the same patterns hold. > - Looks like it processes the whole stride, not width * bytespp, > which means this is going to explode on fenced images in the > padding, right? Yes, looks like it is. Good spot. That probably does belong in a separate patch; are you volunteering? I should, shouldn't I? :-) I'll do it. Of course, I've now changed cover-test so it doesn't do randmemset straight into fenced images anyway (due to needing to handle systems with different memory page sizes) so this is no longer an issue. Ben ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
> For BILINEAR, x=0.5 is precisely at pixel 0, so that must be used with a > weight of 1. > The other pixel could be either -1 or 1 since it will have a weight of 0 > in either case. > But since we round down, -1 it is. > Hmm, this is wrong. We do actually read pixel 0 and 1 in this case as you say, though as mentioned it doesn't really matter since the weight of the other pixel is always 0 with a BILINEAR filter. Sorry to create even more confusion. Søren ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Wed, Sep 2, 2015 at 7:56 AM, Pekka Paalanen wrote: > Right. I looked at fast_bilinear_cover_iter_init() and > fast_fetch_bilinear_cover() -> fetch_horizontal(), and yes, that really > is how it is implemented. The leftmost pixel is chosen essentially by > n = pixman_fixed_to_int(x - pixman_fixed_1 / 2). > > So, pixman_fixed_1 / 2 -> 0, not -1. > > The thing that confuses me is that with nearest, x=0 will sample pixel > n=-1. However with bilinear, x=0.5 will sample pixels n=0 and n=1, not > n=-1 and n=0. > > Pixman's rounding rules can be confusing, but they do have an internal logic: The important thing to understand is that pixel 0 has coordinate 0.5, pixel -1 has coordinate -0.5, pixel 1 has coordinate 1.5. In general, pixel n has coordinate n + 0.5. Given that, and given that we always break ties by rounding down, we find that, for NEAREST: a coordinate of 0.5 is exactly at pixel 0, so we should use that. A coordinate of 0 is precisely midways between pixels -1 and 0, so since we round down, pixel -1 is the one to use. For BILINEAR, x=0.5 is precisely at pixel 0, so that must be used with a weight of 1. The other pixel could be either -1 or 1 since it will have a weight of 0 in either case. But since we round down, -1 it is. There are some more details about rounding in the file pixman/rounding.txt. Søren ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 27 Aug 2015 16:17:55 +0100 Ben Avison wrote: > This test aims to verify both numerical correctness and the honouring of > array bounds for scaled plots (both nearest-neighbour and bilinear) at or > close to the boundary conditions for applicability of "cover" type fast paths > and iter fetch routines. > > It has a secondary purpose: by setting the env var EXACT (to any value) it > will only test plots that are exactly on the boundary condition. This makes > it possible to ensure that "cover" routines are being used to the maximum, > although this requires the use of a debugger or code instrumentation to > verify. > --- > test/Makefile.sources |1 + > test/cover-test.c | 424 > + > 2 files changed, 425 insertions(+), 0 deletions(-) > create mode 100644 test/cover-test.c Hi Ben this is an incremental review compared to the previous version. I also refer to things I wrote as a reply to your previous email (the reply to my review comments). > > diff --git a/test/Makefile.sources b/test/Makefile.sources > index 5b9ae84..aece143 100644 > --- a/test/Makefile.sources > +++ b/test/Makefile.sources > @@ -27,6 +27,7 @@ TESTPROGRAMS =\ > glyph-test\ > solid-test\ > stress-test \ > + cover-test\ > blitters-test \ > affine-test \ > scaling-test \ > diff --git a/test/cover-test.c b/test/cover-test.c > new file mode 100644 > index 000..dbdf0f8 > --- /dev/null > +++ b/test/cover-test.c > @@ -0,0 +1,424 @@ > +/* > + * Copyright © 2015 RISC OS Open Ltd > + * > + * Permission to use, copy, modify, distribute, and sell this software and > its > + * documentation for any purpose is hereby granted without fee, provided that > + * the above copyright notice appear in all copies and that both that > + * copyright notice and this permission notice appear in supporting > + * documentation, and that the name of the copyright holders not be used in > + * advertising or publicity pertaining to distribution of the software > without > + * specific, written prior permission. The copyright holders make no > + * representations about the suitability of this software for any purpose. > It > + * is provided "as is" without express or implied warranty. > + * > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN > + * AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING > + * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS > + * SOFTWARE. > + * > + * Author: Ben Avison (bavi...@riscosopen.org) > + * > + */ > + > +/* > + * This test aims to verify both numerical correctness and the honouring of > + * array bounds for scaled plots (both nearest-neighbour and bilinear) at or > + * close to the boundary conditions for applicability of "cover" type fast > paths > + * and iter fetch routines. > + * > + * It has a secondary purpose: by setting the env var EXACT (to any value) it > + * will only test plots that are exactly on the boundary condition. This > makes > + * it possible to ensure that "cover" routines are being used to the maximum, > + * although this requires the use of a debugger or code instrumentation to > + * verify. > + */ Very good. > + > +#include "utils.h" > +#include > +#include > + > +/* Approximate limits for random scale factor generation - these ensure we > can > + * get at least 8x reduction and 8x enlargement. > + */ > +#define LOG2_MAX_FACTOR (3) > + > +/* 1/sqrt(2) (or sqrt(0.5), or 2^-0.5) as a 0.32 fixed-point number */ > +#define INV_SQRT_2_0POINT32_FIXED (0xB504F334u) > + > +/* The largest increment that can be generated by random_scale_factor(). > + * This occurs when the "mantissa" part is 0x and the "exponent" > + * part is -LOG2_MAX_FACTOR. > + */ > +#define MAX_INC ((pixman_fixed_t)(INV_SQRT_2_0POINT32_FIXED >> (31 - 16 - > LOG2_MAX_FACTOR))) Much better. I am still a tiny bit struggling with why 31 and not 32, but it does add up. 2^-0.5 is presented in 0.32, then an imaginary excercise: - shift right by 32 bits to convert to 32.0 (ones) - shift left by 1 bit to produce 2^0.5 - shift left by 16 bits to convert to 16.16 - shift left by LOG2_MAX_FACTOR, which produces 2^3.5 So we indeed get 2^3.5 presented in 16.16 fixed point. All good here. > + > +/* Minimum source width (in pixels) based on a typical page size of 4K and > + * maximum colour depth of 32bpp. > + */ > +#define MIN_SRC_WIDTH (4096 / 4) > + > +/* Derive the destination width so that at max increment we fit within > source */ > +#def
Re: [Pixman] [PATCH] test: Add cover-test
Hi Ben, here I'm only replying to the questions that are not yet completely clear to me. On Wed, 26 Aug 2015 21:26:57 +0100 "Ben Avison" wrote: > On Thu, 04 Jun 2015 14:00:30 +0100, Pekka Paalanen > wrote: > > > On Tue, 26 May 2015 23:58:30 +0100 > > Ben Avison wrote: > > > >> +static pixman_fixed_t > >> +calc_translate (intdst_size, > >> +intsrc_size, > >> +pixman_fixed_t scale, > >> +pixman_bool_t low_align, > >> +pixman_bool_t bilinear) > >> +{ > >> +pixman_fixed_t ref_src, ref_dst, scaled_dst; > >> + > >> +if (low_align) > >> +{ > >> +ref_src = bilinear ? pixman_fixed_1 / 2 : pixman_fixed_e; > > > > Why the bilinear case is not pixman_fixed_1 / 2 + pixman_fixed_e? > > At this point we're determining the translations required so that the > first destination pixel, when transformed into source space, has exactly > the lowest value it can have without straying into the repeat zones. > > With bilinear scaling, a coordinate of pixman_fixed_1 / 2 has pixel value > determined only by source pixel 0. A coordinate of pixman_fixed_1 * 3 / 2 > has pixel value determined only by source pixel 1. Any value in between > has to be assumed to contain elements from both pixels 0 and 1. > With nearest scaling, any coordinate between pixman_fixed_e and > pixman_fixed_1 inclusive is determined by source pixel 0. Yes, this is > slightly counter-intuitive, but it's the way Pixman has always done it. > See for example in bits_image_fetch_nearest_affine() in > pixman-fast-path.c where we have the lines > > x0 = pixman_fixed_to_int (x - pixman_fixed_e); > y0 = pixman_fixed_to_int (y - pixman_fixed_e); Ok, I can take that as a given. > There is no equivalent offset by pixman_fixed_e for bilinear scaling, for > example in fast_bilinear_cover_iter_init() also in pixman-fast-path.c: > > info->x = v.vector[0] - pixman_fixed_1 / 2; > info->y = v.vector[1] - pixman_fixed_1 / 2; > > All the other implementations follow suit (they have to, or they wouldn't > pass the "make check" suite.) Right. I looked at fast_bilinear_cover_iter_init() and fast_fetch_bilinear_cover() -> fetch_horizontal(), and yes, that really is how it is implemented. The leftmost pixel is chosen essentially by n = pixman_fixed_to_int(x - pixman_fixed_1 / 2). So, pixman_fixed_1 / 2 -> 0, not -1. The thing that confuses me is that with nearest, x=0 will sample pixel n=-1. However with bilinear, x=0.5 will sample pixels n=0 and n=1, not n=-1 and n=0. That alone is surprising, but I am further confused by the talk about the sampling case n=-1 & n=0: http://lists.freedesktop.org/archives/pixman/2015-August/003911.html where you refer to the very end of: http://lists.freedesktop.org/archives/pixman/2014-October/003457.html It's exactly the question of whether to sample the next pixel to the left or to the right of the sampling point. Eh, maybe I'm trying too hard. I do believe your code is correct, but in that case I have a follow-up question on check_transform() in my following patch review email. > > - Looks like it processes the whole stride, not width * bytespp, which > > means this is going to explode on fenced images in the padding, > > right? > > Yes, looks like it is. Good spot. That probably does belong in a separate > patch; are you volunteering? I should, shouldn't I? :-) I'll do it. > >> * We need to test alignment with both the extreme minimum and extreme > >>maximum source coordinates permitted. Given that the scale factors > >>can't be assumed to be nice numbers, I think we need to test them > >>independently, using the translation offsets in the transformation > >>matrix to ensure they align as desired. > > > > To do? > > No - that's covered by supporting both left- and right-aligning (and top- > and bottom-) in calc_translate(). Translation is done entirely using the > transformation matrix because the src_x, src_y, mask_x and mask_y > parameters to pixman_image_composite() only allow for integer positions > in source/mask image coordinates and we need 16.16 fixed-point precision. Ok, I think I misunderstood. I was thinking of the range of 16.16 fp numbers in general, but that's going a bit off-topic here, too. Thanks, pq pgp4FEcncW4o7.pgp Description: OpenPGP digital signature ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Thu, 04 Jun 2015 14:00:30 +0100, Pekka Paalanen wrote: On Tue, 26 May 2015 23:58:30 +0100 Ben Avison wrote: +#define SQRT_0POINT5_0POINT32_FIXED (0xB504F334u) Ok, I checked. Mathematically it is equivalent, but maybe a slightly more obvious name would be INV_SQRT_2_...? Fair enough, renamed (will repost when I've resolved everything else below). +#define MAX_INC (SQRT_0POINT5_0POINT32_FIXED >> (31 - 16 - LOG2_MAX_FACTOR)) This evaluates to 11.314 decimal I think. It does match 2^3.5 with an error in the order of 3e-6, is what Octave tells me. log2(abs(error)) = -18.321, so if I'm thinking right, it means around 18 fractional bits being correct, which means pixman_fixed_t should be accurate to the last bit? I suppose it should be obvious from the integer math, but I just checked with Octave. It's basically just the largest random number that can be generated by random_scale_factor(), in other words if you start out with the largest number and shift it right by the smallest number of places. I hope that makes it clearer where it comes from - I'll update the comment. MAX_INC is a pixman_fixed_t value, right? Could use a comment for that, or making it a static const variable. Yes. I've a feeling some compilers might force a load and runtime calculation of expressions if you use static const, so I'll stick a cast in the definition (being careful because pixman_fixed_t is signed but we need to use an unsigned shift to generate the number). I think it would be valuable to explain these high level goals in a comment at the top, even when they are in the commit message. I've duplicated the commit message in a comment. +static pixman_fixed_t +random_scale_factor(void) [...] Out of curiosity and because it's hard to reason about these things, I did an experiment, and generated 10 million samples with this function. The resulting distribution in log2 space is here: https://people.collabora.com/~pq/pixman/random_scale_factor.png I think it is close to uniform enough, right? And covers the expected range. Nice diagram. For these purposes, I'd say it was close enough to uniform; no particular value is more than 2x more or less likely than any other. Without the shift step in the random number generator, what you'd have instead would be a ramp with one end being soemthing like 128x more likely than the other. You'd also get effects such as the probability of a plot which was an enlargement in both X and Y being only 1 in 64, even though that's quite a likely outcome in real life; this way it's 1 in 4. +static pixman_fixed_t +calc_translate (intdst_size, +intsrc_size, +pixman_fixed_t scale, +pixman_bool_t low_align, +pixman_bool_t bilinear) +{ +pixman_fixed_t ref_src, ref_dst, scaled_dst; + +if (low_align) +{ +ref_src = bilinear ? pixman_fixed_1 / 2 : pixman_fixed_e; Why the bilinear case is not pixman_fixed_1 / 2 + pixman_fixed_e? At this point we're determining the translations required so that the first destination pixel, when transformed into source space, has exactly the lowest value it can have without straying into the repeat zones. With bilinear scaling, a coordinate of pixman_fixed_1 / 2 has pixel value determined only by source pixel 0. A coordinate of pixman_fixed_1 * 3 / 2 has pixel value determined only by source pixel 1. Any value in between has to be assumed to contain elements from both pixels 0 and 1. With nearest scaling, any coordinate between pixman_fixed_e and pixman_fixed_1 inclusive is determined by source pixel 0. Yes, this is slightly counter-intuitive, but it's the way Pixman has always done it. See for example in bits_image_fetch_nearest_affine() in pixman-fast-path.c where we have the lines x0 = pixman_fixed_to_int (x - pixman_fixed_e); y0 = pixman_fixed_to_int (y - pixman_fixed_e); There is no equivalent offset by pixman_fixed_e for bilinear scaling, for example in fast_bilinear_cover_iter_init() also in pixman-fast-path.c: info->x = v.vector[0] - pixman_fixed_1 / 2; info->y = v.vector[1] - pixman_fixed_1 / 2; All the other implementations follow suit (they have to, or they wouldn't pass the "make check" suite.) +scaled_dst = ((uint64_t) ref_dst * scale + pixman_fixed_1 / 2) / pixman_fixed_1; ref_dst is 16.16 fp, scale is 16.16 fp, so the product is 32.32 fp, but adding pixman_fixed_1/2 to that actually means +0.5/65536, not +0.5. And the final division then just scales it back to .16 fp. Basically it's adding 0.5 * pixman_fixed_e... for rounding to nearest representable value? Yes, the transform coefficients, source and destination coordinates are all 16.16 fixed point, and round-to-nearest is how Pixman handles this in all sorts of places, for example in pixman_transform_point_31_16_3d(). It's worth noting that since we increment the destination coordinate by 0x1 for each output pixel, the change
Re: [Pixman] [PATCH] test: Add cover-test
On Tue, 26 May 2015 23:58:30 +0100 Ben Avison wrote: > This test aims to verify both numerical correctness and the honouring of > array bounds for scaled plots (both nearest-neighbour and bilinear) at or > close to the boundary conditions for applicability of "cover" type fast paths > and iter fetch routines. > > It has a secondary purpose: by setting the env var EXACT (to any value) it > will only test plots that are exactly on the boundary condition. This makes > it possible to ensure that "cover" routines are being used to the maximum, > although this requires the use of a debugger or code instrumentation to > verify. > --- > Note that this must be pushed after Pekka's fence-image patches. > > test/Makefile.sources |1 + > test/cover-test.c | 376 > + > 2 files changed, 377 insertions(+), 0 deletions(-) > create mode 100644 test/cover-test.c Hi Ben, this was quite a task to review. :-) Most of the comments are just analysis of the code, to make sure I got it right. Requests for changes should be obviously phrased. > diff --git a/test/Makefile.sources b/test/Makefile.sources > index 14a3710..5b901db 100644 > --- a/test/Makefile.sources > +++ b/test/Makefile.sources > @@ -26,6 +26,7 @@ TESTPROGRAMS =\ > glyph-test\ > solid-test\ > stress-test \ > + cover-test\ > blitters-test \ > affine-test \ > scaling-test \ > diff --git a/test/cover-test.c b/test/cover-test.c > new file mode 100644 > index 000..fb173b2 > --- /dev/null > +++ b/test/cover-test.c > @@ -0,0 +1,376 @@ > +/* > + * Copyright © 2015 RISC OS Open Ltd > + * > + * Permission to use, copy, modify, distribute, and sell this software and > its > + * documentation for any purpose is hereby granted without fee, provided that > + * the above copyright notice appear in all copies and that both that > + * copyright notice and this permission notice appear in supporting > + * documentation, and that the name of the copyright holders not be used in > + * advertising or publicity pertaining to distribution of the software > without > + * specific, written prior permission. The copyright holders make no > + * representations about the suitability of this software for any purpose. > It > + * is provided "as is" without express or implied warranty. > + * > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN > + * AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING > + * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS > + * SOFTWARE. > + * > + * Author: Ben Avison (bavi...@riscosopen.org) > + * > + */ > + > +#include "utils.h" > +#include > +#include > + > +/* Approximate limits for random scale factor generation - these ensure we > can > + * get at least 8x reduction and 8x enlargement. > + */ > +#define LOG2_MAX_FACTOR (3) > + > +#define SQRT_0POINT5_0POINT32_FIXED (0xB504F334u) Ok, I checked. Mathematically it is equivalent, but maybe a slightly more obvious name would be INV_SQRT_2_...? > + > +#define MAX_INC (SQRT_0POINT5_0POINT32_FIXED >> (31 - 16 - LOG2_MAX_FACTOR)) This evaluates to 11.314 decimal I think. It does match 2^3.5 with an error in the order of 3e-6, is what Octave tells me. log2(abs(error)) = -18.321, so if I'm thinking right, it means around 18 fractional bits being correct, which means pixman_fixed_t should be accurate to the last bit? I suppose it should be obvious from the integer math, but I just checked with Octave. MAX_INC is a pixman_fixed_t value, right? Could use a comment for that, or making it a static const variable. What MAX_INC actually is, is sqrt(0.5) * 2 * 2^3 = 2^-0.5 * 2^4 = 2^(4 - 0.5) = 2^3.5. Just like it is in random_scale_factor(). It took a good while to decode this. Could use more commentary on where this comes from. :-) > + > +/* Minimum source width (in pixels) based on a typical page size of 4K and > + * maximum colour depth of 32bpp. > + */ > +#define MIN_SRC_WIDTH (4096 / 4) This could actually be anything you want, fence_image_create_bits() will round it up as necessary, but yeah, this is the minimum width for 4 bytespp images. The a8 mask image will be extended to be 4096 pixels wide, too. The size will depend on the page size. > + > +/* Derive the destination width so that at max increment we fit within > source */ > +#define DST_WIDTH (MIN_SRC_WIDTH * pixman_fixed_1 / MAX_INC) This assumes fence_image_create_bits() uses exactly MIN_SRC_WIDTH, rather than taking it only as a minimum. Would be "more cor
Re: [Pixman] [PATCH] test: Add cover-test
On Tue, 26 May 2015 18:09:21 -0700 Matt Turner wrote: > On Tue, May 26, 2015 at 3:58 PM, Ben Avison wrote: > > This test aims to verify both numerical correctness and the honouring of > > array bounds for scaled plots (both nearest-neighbour and bilinear) at or > > close to the boundary conditions for applicability of "cover" type fast > > paths > > and iter fetch routines. > > > > It has a secondary purpose: by setting the env var EXACT (to any value) it > > will only test plots that are exactly on the boundary condition. This makes > > it possible to ensure that "cover" routines are being used to the maximum, > > although this requires the use of a debugger or code instrumentation to > > verify. > > --- > > Note that this must be pushed after Pekka's fence-image patches. > > > > test/Makefile.sources |1 + > > test/cover-test.c | 376 > > + > > 2 files changed, 377 insertions(+), 0 deletions(-) > > create mode 100644 test/cover-test.c > > > > diff --git a/test/Makefile.sources b/test/Makefile.sources > > index 14a3710..5b901db 100644 > > --- a/test/Makefile.sources > > +++ b/test/Makefile.sources > > @@ -26,6 +26,7 @@ TESTPROGRAMS = \ > > glyph-test\ > > solid-test\ > > stress-test \ > > + cover-test\ > > Remember to add cover-test to .gitignore. No need, there is a test/*-test already. Thanks, pq ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH] test: Add cover-test
On Tue, May 26, 2015 at 3:58 PM, Ben Avison wrote: > This test aims to verify both numerical correctness and the honouring of > array bounds for scaled plots (both nearest-neighbour and bilinear) at or > close to the boundary conditions for applicability of "cover" type fast paths > and iter fetch routines. > > It has a secondary purpose: by setting the env var EXACT (to any value) it > will only test plots that are exactly on the boundary condition. This makes > it possible to ensure that "cover" routines are being used to the maximum, > although this requires the use of a debugger or code instrumentation to > verify. > --- > Note that this must be pushed after Pekka's fence-image patches. > > test/Makefile.sources |1 + > test/cover-test.c | 376 > + > 2 files changed, 377 insertions(+), 0 deletions(-) > create mode 100644 test/cover-test.c > > diff --git a/test/Makefile.sources b/test/Makefile.sources > index 14a3710..5b901db 100644 > --- a/test/Makefile.sources > +++ b/test/Makefile.sources > @@ -26,6 +26,7 @@ TESTPROGRAMS = \ > glyph-test\ > solid-test\ > stress-test \ > + cover-test\ Remember to add cover-test to .gitignore. ___ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman