[cairo] The short road left to 1.6 - EXTEND_PAD

[Bertram Felgenhauer]

Antoine Azar wrote:
>> Looks good to me, code-wise. However, the indentation is messed up
>> badly, and there are a few other style nits.
>
> I've had trouble with Visual Studio, hopefully everything should be fine 
> now.

Looks good now.

>> There's a conflict with commit 1d89bac5a7a5693911d8a74701bd1c0292160478
>> -- fix cairo's  a1-image-sample  test.
>
> I integrated these changes, let me know if there's still any trouble.

You applied the changes in too many places. (Which is partly my fault,
I wasn't clear about that in my previous mail.)

See http://lists.cairographics.org/archives/cairo/2008-January/012789.html
for an explanation for the different treatment of gradients.

I'll attach an updated patch.

>> > -/* Note n trailing semicolon on the above macro; if it's there, then
>> > - * the typical usage of YV12_SETUP(pict); will have an extra trailing ;
>> > - * that some compilers will interpret as a statement -- and then any 
>> further
>> > - * variable declarations will cause an error.
>> > - */
>>
>> Is there any good reason to remove that comment?
>
> I might be missing something but I don't see the usefulness of having a 
> trailing semicolon. Is the comment only there to let developers know they 
> should NOT append a semicolon to the define? if so that should probably be 
> in the coding guidelines rather than in the code. Anyhow I put it back for 
> now.

As far as I am concerned, the comment is not needed (but I'm not an
official pixman maintainer). But removing it has nothing to do with
refactoring fbFetchTransformed(), so it should be a separate patch.

Question to pixman maintainers: Is there a strong preference between
    if (foo)
    {
        bar;
    }
and
    if (foo) {
        bar;
    }
? The patch mixes both styles freely (which is ok from my perspective)

Oh and I've run the cairo performance suite now (once before and after
the patch, so the numbers should be taken with a grain of salt)

| Speedups
| ========
| image-rgb    paint_similar_rgb_source-256    0.41 1.61% ->   0.29 0.60%:  1.38x speedup
| ▍
| image-rgb      paint_similar_rgb_over-256    0.40 1.60% ->   0.29 0.86%:  1.38x speedup
| ▍
| image-rgb        paint_image_rgb_over-256    0.40 1.54% ->   0.29 0.17%:  1.37x speedup
| ▍
| image-rgb      paint_image_rgb_source-256    0.41 1.32% ->   0.29 0.46%:  1.37x speedup
| ▍
| image-rgb   paint_similar_rgba_source-256    0.53 1.17% ->   0.42 0.57%:  1.26x speedup
| ▎
| image-rgb     paint_image_rgba_source-256    0.53 1.05% ->   0.42 1.90%:  1.25x speedup
| ▎
| image-rgba      pattern_create_radial-16     4.94 0.06% ->   4.50 0.00%:  1.10x speedup
| ▏
| image-rgb        text_linear_rgb_over-64     3.08 0.14% ->   2.93 0.38%:  1.05x speedup
| Slowdowns
| =========
| image-rgba  paint_similar_rgba_source-256    0.26 0.68% ->   0.31 0.26%:  1.20x slowdown
| ▎
| image-rgba     paint_image_rgb_source-256    0.85 1.05% ->   0.98 0.16%:  1.17x slowdown
| ▏
| image-rgb       pattern_create_radial-16     4.94 0.53% ->   5.46 0.07%:  1.11x slowdown
| ▏
| image-rgba   paint_similar_rgb_source-256    0.86 1.13% ->   0.96 0.57%:  1.11x slowdown
| ▏
| image-rgb    paint_radial_rgba_source-256   10.68 0.67% ->  11.35 0.03%:  1.07x slowdown
| ▏
| image-rgb     paint_radial_rgb_source-512   43.62 0.41% ->  46.55 0.11%:  1.07x slowdown
| ▏
| image-rgb       paint_radial_rgb_over-256   11.05 0.60% ->  11.81 0.04%:  1.07x slowdown
| ▏
| image-rgb       paint_radial_rgb_over-512   44.67 0.61% ->  47.36 0.06%:  1.07x slowdown
| ▏
| image-rgb    paint_radial_rgba_source-512   44.14 0.55% ->  46.55 0.20%:  1.06x slowdown
| ▏
| image-rgb               subimage_copy-128    0.01 1.54% ->   0.01 0.22%:  1.06x slowdown
| 
| image-rgb     paint_radial_rgb_source-256   10.78 0.16% ->  11.35 0.00%:  1.06x slowdown

Looks reasonable, I think.

Bertram
-------------- next part --------------