On Wed, Oct 15, 2008 at 02:52:32AM +0100, Peter Clifton wrote:
> Nice patch series. I'll push those into the main repository if you're
> asserting that they are tested, and don't cause any regressions in
> hatching output. I had a few issues though - which you could fix, or I
> could fix as I applied them:
>
>
> BTW.. there is no great need to add things like:
>
> + g_return_if_fail(toplevel != NULL);
> + g_return_if_fail(fp != NULL);
>
> To the various functions called internally within libgeda.
>
> I'm not so against it that I'd take it out, but it would be nice to
> define somewhere, just where we're adding these tests, and which
> code-paths assume the input is sensible.
>
> This function is called from inside libgeda only IIRC, so we should take
> its input as not needing testing. APIs called from outside libgeda
> "might" need a little more validation.
>
>
> Seems there is an unrelated change here. Was this the old behaviour?
>
> + /* Avoid printing line widths too small */
> + if (fill_width <= 1) fill_width = 2;
>
> If not, please don't change it - certainly not in an unrelated patch.
>
>
> Postscript / PDF will special case "0" as meaning "minimum representable
> line width, and that's why gEDA allows "0" specified as a line-width.
But not if it ever goes through cairo: the cairo_stroke code path goes
through cairo_gstate_stroke and one of the first thing it does is:
if (gstate->stroke_style.line_width <= 0.0)
return CAIRO_STATUS_SUCCESS;
so if you want to ultimately produce screen/pdf/postscript through
cairo (advantage: if printed output does not match what is on screen,
you can blame cairo for the bug), you'll have to revisit it.
Regards,
Gabriel
_______________________________________________
geda-dev mailing list
[email protected]
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
