[cairo] [cairo-commit] src/cairo-xlib-surface.c

[Andrea Canciani]

On Mon, Jan 10, 2011 at 1:56 AM, Behdad Esfahbod <behdad at behdad.org> wrote:
> On 01/08/11 05:54, Andrea Canciani wrote:
>
>>
>> -     this_x = _cairo_lround (glyphs[i].d.x);
>> -     this_y = _cairo_lround (glyphs[i].d.y);
>> -
> ...
>> -     if (((this_x+4096)|(this_y+4096))&~0x3fffu)
>> +     if (glyphs[i].d.x > INT16_MAX || glyphs[i].d.y > INT16_MAX ||
>> +         glyphs[i].d.x < INT16_MIN || glyphs[i].d.y < INT16_MIN)
>> +     {
>>           break;
>> +     }
>> +
>> +     this_x = _cairo_lround (glyphs[i].d.x);
>> +     this_y = _cairo_lround (glyphs[i].d.y);
>
> Any reason why not round then do the check?

The _cairo_lround might return invalid data if the coordinates
are not representable in 24.8. This should not happen because
gstate culls glyphs "obviously" outside the surface and the
maximum size of the surface is 16 bits, but for some huge glyphs
gstate might be unable to do the culling.

With my limited knowledge of fonts I was unable to see why gstate
should always be able to guarantee that fabs (glyphs[i].d.{x,y}) < 2^24.
This should usually not be a problem, because we are also checking
that the glyph surface fits the maximum request size, but for some
degenerate glyphs (an horizontal line of ~2^24 - 2^16 x 1 pixels)
it would be theoretically possible to cause incorrect computations
(assuming that they fit the request size).

If you can prove that such a situation is not possible, I'd like to
assert it and make the integer comparison instead. I think that
the floating point comparison (current code) should not be much
slower and it should keep on the safe side, so we might still
prefer it.

NB: Glyphs outside the 24.8 will cause a fallback to cairo-image,
which will probably fail to handle them, but that's a different problem.

>
> Looks fine otherwise.

:)

Andrea