John, all your alterations to libxmi sound good, given the fact that you're
using it in the GGI framework.  But you'd better fix the segfaulting
problem in mi_spans.c.  I've appended instructions at the end of this
message.  It looks as if you didn't change mi_spans.c too much, actually.

In case you're wondering, the `SpanGroup' idea was Joel McCormack's, when
he was writing the X11 code.  I added `miPaintedSet' as a natural
generalization of his SpanGroup concept, for handling drawing primitives
that can produce pixels of one color.  (Read: multicolored dashes.)  If I
recall correctly, in his X11 code Joel had just two SpanGroups: one for the
fg color, and one for the bg color.  The latter got used only when doing
on-off dashing.

As regards backporting from libXMI, I'm interested in miBlendStage.  I plan
on keeping libxmi's abstract miPixel datatype, but it may be possible for
me to add some of your code as an install-time option.

Too bad you didn't like my reentrant arc drawing functions :-(.  Actually,
the X11 XDrawArcs() is non-reentrant.  I'm not sure that's documented
anywhere, but I thought I had better fix it.  I developed libxmi so that
I'd be able to rasterize the vector primitives in GNU libplot (see
http://www.gnu.org/software/plotutils ).  And libplot is supposed to be
thread-safe, which means that libxmi needed to be reentrant.  Unless
I wanted to add mutex-based locking to libxmi, redoing XDrawArcs(), which was
the only offending function, seemed to be the way to go.

It looks as if you've altered the second stage of the libxmi pipeline
considerably (the miPaintedSet->miCanvas stage, which I wrote from
scratch), but you've kept the first stage, which does span-generation,
almost intact.  In fact it looks as if the `ggi_visual_t vis' you added as
an argument to each internal function could be pruned back.  Right now,
each function in the first stage draws pixels by invoking the macro
MI_PAINT_SPANS, which doesn't take `vis' as an argument.  Are you planning
to change that?

I think it would be great if we could keep the first stage of our
respective pipelines more or less in sync.  If I ever do a libxmi-2.0, this
is what I'm thinking about adding to the first stage.

1. Support for filling compound polygons, i.e., polygons with holes in
   them.  (Equivalently, the scan-conversion of Postscript-style compound
   paths; polygonal ones, anyway.)  This wouldn't be too hard.  Right now,
   Brian Kelleher's code in mi_plycon.c and mi_plygen.c is used for 
   filling polygons.  The upgrade path from it is pretty clear.

2. Sub-pixel placement of polygon vertices.  libxmi (like the original X11
   code) already has support for filling convex polygons with
   floating-point, rather than integer, vertex locations.  (See
   mi_fplycon.c.)  But it's not exposed in the API: it's used only for 
   drawing the small polygons we call `line caps' and `line joins'.  
   A non-convex counterpart would need to be written.

   The hard part would be adding sub-pixel placement to the drawing (rather
   than filling) functions.  I've looked at miDrawLines, and I don't think
   it would be easy.  And I hate even to think about adding it to
   miDrawArcs...

3. Anti-aliased line drawing.  Possibly not so hard, but it would mean
   equipping every pixel in a span with an intensity value.  And the
   usefulness of the miPaintedSet abstraction would become doubtful, if
   a huge number of pixel values are generated by every API function.

4. Low-level support for gradient fill.  This would be much easier to
   add; in fact I've already started work on it.  But again, it doesn't
   fit very well with the miPaintedSet abstraction.

5. Drawing better arcs.  Let's face it, miDrawArcs is a horror.  (And
   if you think it looks bad now, you should see what it looked like before
   I rearranged and commented the code!)  It can't be changed too much
   because that would break pixel-level compatibility with X11.
   However, it's just barely possible that it could be extended to 
   generate wide elliptic arcs whose principal axes aren't aligned with the
   coordinate axes.  As Foley and van Damm point out, the inner and outer
   boundary of any wide ellipse is an eighth-order curve.  I believe the
   reason that miDrawArcs isn't worse than it is (!) is that if the axes
   are aligned, the order is 4 rather than 8.  If I have a _lot_ of time,
   I'll think about doing the extension.

   Actually, Keith Packard and Bob Scheifler, who wrote the original arc
   code, ended up very unhappy with the basic approach.  Keith published an
   article in 1990 (in Software: Practice & Experience) where he said that
   if he had to it over, he'd use a scheme involving a polygonal brush,
   moved along the ellipse.  Unfortunately by that time it was too late.
 
Anyway, it's really good that libxmi is finding its way into GGI.  If I
hadn't extracted the vector code from X11, it would probably have died (no
one had dared to touch it for a decade).

Appended below are instructions on how to fix the mi_spans.c segfaulting
bug.  Also, I have one other comment about libXMI.  It looks as if you're
using the system malloc/realloc/calloc instead of the versions I wrote (in
mi_alloc.c).  Some versions of malloc/realloc/calloc don't handle null
pointers and zero sizes, when passed as arguments, in a reasonable way.
If you have mysterious segfaults, you might want to check into that.

--Robert

----------------------------------------------------------------------

l. 40.  "These give ymax, ymin for a Spans".  Change to "nonempty Spans"
        for clarity.

l. 64.  Add a line

            paintedSet->groups = (SpanGroup **)NULL;

        May not be necessary, but initializing all fields of a newly
        created object is better programming style.

l. 260.  At the very head of XMI_stubs_subtractspans(), add the lines

          if (sub->count == 0)          /* nothing to do */
            return;

         May not be necessary, but it saves a few cycles.

l. 500 or so.  Immediately before the comment line

            /* Yuck.  Gross.  Radix sort into y buckets... */

        add the following lines

  /* Special case : ymin > ymax, so the Spans's in the SpanGroup, no matter
     how numerous, must be empty (and can't contain point or width arrays). */
  if (spanGroup->ymin > spanGroup->ymax)
    {
      spanGroup->count = 0;
      return;
    }

This final addition is necessary: it fixes a segfaulting problem that
occurs when a newly painted region in one pixel color completely covers a
region in another pixel color, and obliterates all trace of that pixel
from the miPaintedSet.

Reply via email to