[cairo] [PATCH] Improve handling of degenerate font matrices

[Andrea Canciani]

On Sun, Feb 6, 2011 at 11:27 PM, Behdad Esfahbod <behdad at behdad.org> wrote:
> On 02/05/11 05:20, Andrea Canciani wrote:
>> A first step towards the handling of degenerate matrices can be the handling of
>> degenerate font scale matrices.
>
> So amazing to see you tackle this problem.  Removes a lot from my "on day I'll
> do..." list :).
>
>
>> This should be relatively easy, because cairo already supports 0-size fonts and
>> because this drawing operation always degenerates in either a noop or a clear
>> operation (depending on the mask-boundedness of the operator).
>
> Right.  We do need to check the get_path() and extents() paths however.

Yes, we'd better both add automated testing and do a code review of them.

>
>
>> 0002 makes it possible to create scaled fonts with 1 rank matrices. This
>> needs to be tested (as in "tests for it should be added"). It currently adds
>> a failure in invalid-matrix, but this is expected, because the "invalid" matrix
>> is now considered ok.
>
> Looks good.  Maybe add a cairo_matrix_is_degenerate() instead of computing
> determinants directly...

Sure, I can do that.

>
> I took a quick look to see if we can completely remove scale_inverse.  It may
> be possible, but I check the current uses and they are mostly harmless, so its
> not a big deal.

I had a commit performing that. It should be quite trivial as soon as
I manage to
get rid of it in cairo-user-font.c

>
>
>> 0003 is the "real" thing.
>
> Doing any kind of real work other than gstate-tracking in the gstate layer is
> wrong.  Imagine replaying a recording surface against a surface with a
> degenerate matrix, etc.  The check definitely belongs to the scaled-font
> layer.  Should be as easy to add it there.
>
> I agree that some of the stuff already in that function do not belong there
> either.

Exactly. Some weeks ago I talked about moving the optimizations (and any
kind of real work) out of gstate and Chris agreed.

I'm already working on a branch in which gstate only prepares the parameters
for the cairo_surface_* call (i.e. it only transforms the patterns
with the ctm),
cairo-surface calls optimization functions (so we can easily disable most
optimizations if we want to catch bugs in them) and then the backend-specific
function (or the fallback).
Unfortunately I also wanted to add some additional cleanup and optimizations
to this branch... maybe I'll defer the changes and I'll just cleanup the commit
which moves the code around and post it to the ml.

>
>
>> I don't completely understand what
>> _cairo_matrix_compute_basis_scale_factors is supposed to do.
>> It is only used in font code and its documentation seems to be font-oriented
>> and incorrect. Was it supposed to be an SVD?
>
>
> The Return value docs are wrong for sure, but the rest is correct.  No, it's
> not SVD.  It's mostly used to compute det(M) / |M*(1,0)'|, which is the line
> height of the font in user space when applied on the font matrix.

Ok, the geometrical insight was very useful (it also explains why 0,0 should
be a correct answer when det(M)==0). I'll try to fix the doc and maybe inline
a couple of comments expaining the geometrical meaning of the
computations.

Andrea