Fwd: [cairo] [PATCH] PDF: alpha-gradients; reused patterns; cairo_paint_with_alpha

[Kristian Høgsberg]

Last email, sorry for the spam.  Stupid gmail.

---------- Forwarded message ----------
From: Miklós Erdélyi <erdelyim at gmail.com>
Date: Jul 27, 2006 7:18 PM
Subject: Re: [cairo] [PATCH] PDF: alpha-gradients; reused patterns;
cairo_paint_with_alpha
To: Kristian Høgsberg <krh at bitplanet.net>


On 7/27/06, Kristian Høgsberg <krh at bitplanet.net> wrote:
> Well, that's a good point... on one hand I think that the case where
> somebody repeatedly creates a cairo surface for the same bitmap data,
> is not something we need to optimize for, but on the other hand the
> code is there and handles this case.  However, CRC32 isn't suitable
> for this use.  It's a checksum algorithm designed for detecting
> transmission errors, that is, small bit deviations from the original
> message, not for reliably representing the contents of an image
> surface as a 32 bit checksum.  So if we want to do this, we need
> either 1) a more reliable hash that can be used to represent the image
> data, or 2) keep all emitted images in memory and compare against
> these to see if we already have the image in the PDF document (note,
> we can keep images in a hash table in this case and use CRC32 as the
> hash function). For 1), we need to pull in something like SHA-1, but
> I'm not sure it's worth it at that point, and 2) doesn't scale.
You are right. This usage scenario isn't frequent so the code doesn't
have to be optimized for this. But I think a clever cache could be
used for keeping the image data: storing the hash of all the emitted
images and a reference count, and keeping in memory a fixed
number/mbytes of images with the greatest reference count; this could
scale well.

> The fallbacks are supposed to only cover the part of the page that
> needs the fallback.  Right now they don't but the infrastructure is in
> place to optimize this.
This would be cool.

> As you mention, it
> is possible to do better in some cases, for example, if the background
> color is known to be constant across the gradient, you can just
> pre-composite the gradient color stops over the background color and
> do an opaque gradient.  You can do lots of other tricks here, but my
> take is that it's not worth pursuing very far - the more you want to
> optimize, the more complicated it gets, and the smaller the gains.
I completely agree. This will make the code simpler.

> Btw, do try to follow the coding style of the rest of cairo.
> CODING_STYLE documents most of the conventions used, but when in
> doubt, copy the style of similar constructs elsewhere in cairo.
I tried to do that. Though this looks ugly [I haven't found an example
for this in the core code]:
    if (n_stops == 2) {
        /* no need for stiched function */
        functions.colorgradient = emit_linear_colorgradient (surface,
&stops[0], &stops[1]);
    if (emit_alpha) {
        functions.alphagradient = _cairo_pdf_surface_new_object (surface);
        functions.alphagradient = emit_linear_alphagradient (surface,
&stops[0], &stops[1]);
    }
    } else {
  ^^
These two brackets above each other in the same column...

Please point out those inconsistencies with cairo's coding style to me
which didn't catch my attention.
Besides of this, is there any other obstacle to use the separated
patches in upstream cairo? Let me know and I'll bump them away :)
I will post the two patches to the list also so others can look at it.

Cheers,
miklos