Re: [RFC] introduce new routine draw_clip_arrow()

[Hans-Bernhard B. <br...@ph...>]

Ethan Merritt wrote:

> Current state of the code in cvs, as recently modified:
> 
> - full clipping is done in do_arrow(), which is a generic
>   routine called by most terminals. Because this is a
>   terminal entry point, the not-yet-clipped coordinates have
>   already been forced to (unsigned int).  Casting them back
>   to (int) works, but is an ugly hack.

IOW, that was exactly the wrong way of doing it ;-(

> This would replace the clipping code in do_arrow() and
> possibly code in some individual terminal drivers.
> 
> The call site could specify a bounding box to clip
> against if the default is not appropriate:
> 
> BoundingBox *clip_save = clip_area;
> clip_area = &(BoundingBox *){ 0, xmax, 0, ymax };
> draw_clip_arrow(...)
> clip_area = clip_save;

Objection, your honour --- we should absolutely not add more global 
variables.  Please consider making the clip area to be used a parameter 
of draw_clip_arrow.

> draw_clip_arrow( int sx, int sy, int ex, int ey, int head)

I'm not at all sure that doing this in ints is a good plan.  These
should probably be coordvals (or even a complete struct arrow_def, by
joining draw_clip_arrow() with get_arrow()).

> {
>     /* Don't draw head if the arrow itself is clipped */
>     if (head == BOTH_HEADS && clip_point(sx,sy))
>         head = END_HEAD;
>     if (head == BOTH_HEADS && clip_point(ex,ey))
>         head = BACKHEAD;
>     if (head == BACKHEAD && clip_point(sx,sy))
>         head = NOHEAD;
>     if (head == END_HEAD && clip_point(ex,ey))
>         head = NOHEAD;

This sequence hints at a design flaw.  We should have individual flag
bits for each head, not a single combined enum name for both.  Put in 
another way, BOTH_HEADS should be killed.