[cairo] Working on degenerate matrix problem, some questions
Daniel Kraft
d at domob.eu
Thu Mar 27 03:30:47 PDT 2008
Chris Wilson wrote:
> On Mon, 2008-03-24 at 17:22 +0000, Daniel Kraft wrote:
> > BTW, how should I communicate patches like this
> > to you? Is it ok like this, should I append the diff directly to the
> > mail text, use something other than the mailing list...?
>
> [I'll expect Carl will be able to answer more completely. Indeed since
> this is a FAQ, I'd expected to be able to find some instructions on the
> wiki...]
> For any non-trivial series of patches, it's preferable to publish a git
> branch with your changes. Carl will set up a freedesktop.org account for
> you, from which you'll be able to host a repository, if you ask politely
> (and follow the instructions at
> http://freedesktop.org/wiki/AccountRequests). Given such a repo, for us
> to keep current with your work is as simple as:
> $ git remote add kraft git://somewhere.org/~dkraft/cairo # once
> $ git fetch kraft
Ah, thanks for the hint, I'll look at this now; and yes, you actually
convinced me this will be much better to handle than working with patches...
> > * At some points I am currently ignoring a cairo_status_t return value;
> > that is, in functions already present with a void return type. How can
> > I set the error flag if an error occurs there without having to return
> > the error code? What should I do there? (For instance, the gstate
> > device-to-user transformations, IIRC)
>
> Here you should add a cairo_status_t return and propagate the error code
> back to the caller, and there you will be able to set the error flag on
> the correct object. The functions are currently void as they only ever
> return SUCCESS so the code was simplified by removing the redundant
> conditionals.
Ok, I'll look at this.
> [snip]
> I'm still digesting the bulk of the patch, in particular whether it is
> actually a step in the correct direction. ;-) It does seem to avoid the
> initial problem of flagging an error for cairo_scale(0,0), but you've
> neither updated the test suite (test/invalid-matrix.c, checking that the
> error is not set immediately, but only on the invalid use) nor added new
> tests to check drawing with cairo_scale(0,0).
Actually, at the moment I didn't do much into the direction of
supporting degenerate matrices; I just started this cairo_transform_t
stuff and wanted to get some feedback about the general "looking" of my
code. I believe it is far from being any complete ;) BTW thanks for
the name of the testcase for me, I'll look at it!
> However, I did spot this:
> > @@ -1140,14 +1091,20 @@ _cairo_gstate_stroke_extents (cairo_gstate_t
> *gstate,
> >
> > _cairo_traps_init (&traps);
> >
> > - status = _cairo_path_fixed_stroke_to_traps (path,
> > - &gstate->stroke_style,
> > - &gstate->ctm,
> > - &gstate->ctm_inverse,
> > - gstate->tolerance,
> > - &traps);
> > - if (status == CAIRO_STATUS_SUCCESS) {
> > - _cairo_gstate_traps_extents_to_user_rectangle(gstate,
> &traps, x1, y1, x2, y2);
> > + status = cairo_transform_get_inverse (&gstate->transform,
> &matrix_inverse);
> > + if (status)
> > + {
> > + /* XXX: Transform */
> > + status = _cairo_path_fixed_stroke_to_traps (path,
> > + &gstate->stroke_style,
> > + &gstate->transform.matrix,
> > + &matrix_inverse,
> > + gstate->tolerance,
> > + &traps);
> > + if (status == CAIRO_STATUS_SUCCESS) {
> > + _cairo_gstate_traps_extents_to_user_rectangle(gstate,
> &traps,
> > + x1, y1,
> x2, y2);
> > + }
> > }
> >
> > _cairo_traps_fini (&traps);
>
> which looks wrong, as the bbox determination seems only to occur on the
> error path from cairo_transform_get_inverse().
You mean, if (status) should become if (status == CAIRO_STATUS_SUCCESS)?
If so, that's probably right and what I meant. (I've just got the
habit of using such if (variable) constructs, mainly from checking
pointers I think; and I did not think about this probably, but I suppose
if any !status should be right)
> [Hmm, that should be
> _cairo_transform_get_inverse(), unless you really are proposing that
> cairo exports the cairo_transform_t as public API.]
I see, so I should probably change it, right?
Daniel
--
Done: Bar-Sam-Val-Wiz, Dwa-Elf-Hum-Orc, Cha-Law, Fem-Mal
Underway: Ran-Gno-Neu-Fem
To go: Arc-Cav-Hea-Kni-Mon-Pri-Rog-Tou
More information about the cairo
mailing list