Comment 231 for bug 217908

Thanks for your comments.

<email address hidden> wrote:
>> @@ -685,11 +688,33 @@
>> + //
>> + // Update 6/24/09: The underlying X server/driver bugs are now
>> + // fixed, and cairo uses the fast XRender code-path as of 1.9.2
>> + // (commit commit a1d0a06b6275cac3974be84919993e187394fe43) --
>
> The word commit is repeated.
Sorry, fixed.

>> + // but only if running on a 1.7 X server.
>> + // So we enable EXTEND_PAD provided that we're running a recent
>> + // enough cairo version (obviously, this is only relevant if
>> + // --enable-system-cairo is used) AND running on a recent
>> + // enough X server. This should finally bring linux up to par
>> + // with other systems.
>> PRBool isDownscale =
>> deviceToImage.xx >= 1.0 && deviceToImage.yy >= 1.0 &&
>> deviceToImage.xy == 0.0 && deviceToImage.yx == 0.0;
>> if (!isDownscale) {
>> - pattern->SetFilter(0);
>> + PRBool fastExtendPad = false;
>> + if (currentTarget->GetType() == gfxASurface::SurfaceTypeXlib &&
>> + cairo_version() >= CAIRO_VERSION_ENCODE(1,9,2)) {
>> + gfxXlibSurface *xlibSurface = static_cast<gfxXlibSurface *>((gfxASurface *)currentTarget);
>
> Why cast to (gfxASurface *) first?
It's not really a cast. currentTarget is of type nsRefPtr<gfxASurface>, which is converted into a (gfxASurface *) in order to be able to use the static cast. There might be a better way, but I couldn't find any nsRefPtr cast operators in the mozilla source.

>
>> + Display *dpy = xlibSurface->XDisplay();
>> + if (VendorRelease (dpy) < 60700000 && VendorRelease (dpy) >= 10699000)
>> + fastExtendPad = true;
>
> I don't understand this VendorRelease check. I would expect something more
> like:
> VendorRelease(dpy) >= 10700000
> Please add a comment explaining why you don't use the more intuitive check.
We need to use the exact same check that cairo is using. The reason for the VendorRelease check is explained at length in src/cairo-xlib-display.c, basically there used to be a different numbering scheme in older X servers. We use 10699000 instead of 10700000 in order to be able to test the code with pre-release servers.

>> + }
>> + if (fastExtendPad) {
>> + pattern->SetExtend(gfxPattern::EXTEND_PAD);
>> + } else {
>> + pattern->SetFilter(0);
>> + }
>