gnuplot-beta

Short vectors are not plotted

[Dima K. <gn...@di...>]

Hi all, a non-version-control question for yall!

I'm trying to plot a vector field (attached). The vectors start at an
evenly-spaced 20x20 grid. On my machine when I run the script, plotting
into an x11 terminal, I don't see all 400 vectors, I see maybe 1/4 of
them; the rest don't show up at all. It turns out what happens is this:

  do_plot()
  {
    ...
    plot_vectors();
    ...
  }

  plot_vectors()
  {
    ...
    for(vectors)
    {
      double x1_data_coords, x2_data_coords, ...;
      ...
      int x1_terminal_coords = map_x(x1_data_coords);
      int x2_terminal_coords = map_x(x2_data_coords);
      ...
      (terminal->arrow) (x1_terminal_coords, y1_terminal_coords, ...);
    }
  }

  arrow(int x1, int y1, ...)
  {
     if( x1 == x2 && y1 == y2 )
       return;

     ...
  }


I.e. we convert the vector start/end to integer terminal coords; short
vectors become 0-vectors in this representation, so the
terminal-specific arrow() function plots nothing.

This is a bug: checking for a 0-vector should happen before we convert
to integer pixel coordinates.

I'm attaching a patch series to fix this. The fundamental difference is
a change to the arrow() terminal callback to take FLOATING-POINT
terminal coordinates, so the vector direction, length can still be
accurately computed. This feels incomplete, since I suspect there're
other terminal functions that can benefit from such a change, but this
works.

Note that these patches are made with 'git format-patch', so once the
conversion is complete, they should be applied with 'git am'. This
imports the patch series together with the commit metadata (author,
message, etc).

Re: Short vectors are not plotted

[sfeam <sf...@us...>]

On Sunday, 29 October 2017 13:26:39 Dima Kogan wrote:
> Hi all, a non-version-control question for yall!
> 
> I'm trying to plot a vector field (attached). The vectors start at an
> evenly-spaced 20x20 grid. On my machine when I run the script, plotting
> into an x11 terminal, I don't see all 400 vectors, I see maybe 1/4 of
> them; the rest don't show up at all. It turns out what happens is this:
> 
>   do_plot()
>   {
>     ...
>     plot_vectors();
>     ...
>   }
> 
>   plot_vectors()
>   {
>     ...
>     for(vectors)
>     {
>       double x1_data_coords, x2_data_coords, ...;
>       ...
>       int x1_terminal_coords = map_x(x1_data_coords);
>       int x2_terminal_coords = map_x(x2_data_coords);
>       ...
>       (terminal->arrow) (x1_terminal_coords, y1_terminal_coords, ...);
>     }
>   }
> 
>   arrow(int x1, int y1, ...)
>   {
>      if( x1 == x2 && y1 == y2 )
>        return;
> 
>      ...
>   }
> 
> 
> I.e. we convert the vector start/end to integer terminal coords; short
> vectors become 0-vectors in this representation, so the
> terminal-specific arrow() function plots nothing.
> 
> This is a bug: checking for a 0-vector should happen before we convert
> to integer pixel coordinates.

I am not following the logic.

If it is a 0-length vector then isn't it correct to not draw it?
Even if you keep the floating point representation for a bit longer,
at the time you convert to integer terminal coordinates it will
again become 0-length and thus not drawn.  What am I missing?

Is the idea that you want to draw an arrowhead even if the
shaft length is zero?
 
	Ethan


> I'm attaching a patch series to fix this. The fundamental difference is
> a change to the arrow() terminal callback to take FLOATING-POINT
> terminal coordinates, so the vector direction, length can still be
> accurately computed. This feels incomplete, since I suspect there're
> other terminal functions that can benefit from such a change, but this
> works.
> 
> Note that these patches are made with 'git format-patch', so once the
> conversion is complete, they should be applied with 'git am'. This
> imports the patch series together with the commit metadata (author,
> message, etc).
> 
> 
> 

Re: Short vectors are not plotted

[Dima K. <gn...@di...>]

sfeam <sf...@us...> writes:

>> I.e. we convert the vector start/end to integer terminal coords; short
>> vectors become 0-vectors in this representation, so the
>> terminal-specific arrow() function plots nothing.
>>
>> This is a bug: checking for a 0-vector should happen before we convert
>> to integer pixel coordinates.
>
> I am not following the logic.
>
> If it is a 0-length vector then isn't it correct to not draw it?
> Even if you keep the floating point representation for a bit longer,
> at the time you convert to integer terminal coordinates it will
> again become 0-length and thus not drawn.  What am I missing?
>
> Is the idea that you want to draw an arrowhead even if the
> shaft length is zero?

Right. It's not actually a length-0 vector, but its length is < 1 pixel
long. We should still draw an arrowhead, and the arrowhead should point
in the correct direction. Once we convert to integers, then even if we
wanted to draw something, we can't do it, since the direction has been
lost.

Here's a (hopefully convincing) thought: let's say you're looking at a
plot of a vector field. Then you zoom out repeatedly. I would expect the
plotted vector lengths to get shorted as you zoom out. The arrowheads
maybe would get smaller too, depending on how they're plotted. But the
transitions should be smooth. What currently happens is that as you zoom
out, some subset of the vectors disappears. As you zoom out more, some
OTHER subset disappears. This subset could be entirely different, and
some vectors actually come back. Eventually they all disappear, but as
they do so, sometimes they're plotted in an intermediate state where the
arrowhead IS plotted, but its direction is completely wrong.

Try with the example attached in the last email; use the mouse wheel to
zoom out.

Re: Short vectors are not plotted

[sfeam <sf...@us...>]

On Sunday, 29 October 2017 16:02:11 Dima Kogan wrote:
> sfeam <sf...@us...> writes:
> 
> >> I.e. we convert the vector start/end to integer terminal coords; short
> >> vectors become 0-vectors in this representation, so the
> >> terminal-specific arrow() function plots nothing.
> >>
> >> This is a bug: checking for a 0-vector should happen before we convert
> >> to integer pixel coordinates.
> >
> > I am not following the logic.
> >
> > If it is a 0-length vector then isn't it correct to not draw it?
> > Even if you keep the floating point representation for a bit longer,
> > at the time you convert to integer terminal coordinates it will
> > again become 0-length and thus not drawn.  What am I missing?
> >
> > Is the idea that you want to draw an arrowhead even if the
> > shaft length is zero?
> 
> Right. It's not actually a length-0 vector, but its length is < 1 pixel
> long. We should still draw an arrowhead, and the arrowhead should point
> in the correct direction. Once we convert to integers, then even if we
> wanted to draw something, we can't do it, since the direction has been
> lost.

I don't at all like the idea of mixing (double) and (int) coordinates in the
terminal API.  Let's try to find a different solution.

First off, I think your patch only handles a subset of arrows.
It modifies the handling of "plot ... with vectors" but not the code that
handles "splot ... with vectors" or "set arrow ...".

That last one is interesting because rather than the in-line coordinate
transform and clipping code used by plot_vectors() and plot3d_vectors()
it goes through a much simpler routine draw_clip_arrow(). 
And it looks to me at first glance that the bulk of the former two code
blocks can be replaced by a call to draw_clip_arrow() also.
That at least brings all the arrow-drawing onto a common path.

Next I note that most of the callers of draw_clip_arrow are already
working with (double) coordinates. Converting the remaining ones
should be relatively simple.

At that point I think you are mostly home free. Now all you need to
do is add a flag or a new set of enums to the 5th parameter of 
do_arrow() that signals "draw only the arrowhead".   In places where
you want to guarantee that the arrowhead is drawn even if the vector
length approaches zero, you can call term->arrow() twice.
The first call is either exactly like now or perhaps we would clear
all the arrowhead flags.
The second call we dummy up a unit length vector in the desired
direction  but set the "arrowhead only" flag.

And that should do it except for the oddball terminals that have
private arrow-drawing code.  Leave that to a second round of cleanup
after the generic code path is working.

	Ethan


> 
> Here's a (hopefully convincing) thought: let's say you're looking at a
> plot of a vector field. Then you zoom out repeatedly. I would expect the
> plotted vector lengths to get shorted as you zoom out. The arrowheads
> maybe would get smaller too, depending on how they're plotted. But the
> transitions should be smooth. What currently happens is that as you zoom
> out, some subset of the vectors disappears. As you zoom out more, some
> OTHER subset disappears. This subset could be entirely different, and
> some vectors actually come back. Eventually they all disappear, but as
> they do so, sometimes they're plotted in an intermediate state where the
> arrowhead IS plotted, but its direction is completely wrong.
> 
> Try with the example attached in the last email; use the mouse wheel to
> zoom out.
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> gnuplot-beta mailing list
> gnu...@li...
> Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta

Re: Short vectors are not plotted

[Dima K. <gn...@di...>]

Ethan Merritt <eam...@gm...> writes:

> Here's a start. I'll probably commit these changes to CVS when the
> current turmoil settles. After applying this patch there is only one
> call site for term->arrow() in the code base, in draw_clip_arrow().
> Could be I'm missing some complication, but it looks to me that any
> changes needed for your heads-only implementation can be limited to
> this one routine.

Here's a new patch series. These assume your use_draw_clip_arrow.patch
is applied. Again, please apply with 'git am', or let me know what is
unsatisfactory, so that I can address it.

The updates are similar to the last patch series in that some functions
took integers previously, but now they take floating-point values. In a
number of cases, this patch series makes available both flavors: integer
and floating-point. Is this what you had in mind?

And two more related points:

1. This patch uncovered an inconsistency in much of our code: when
converting a floating-point terminal coordinate to an integer one,
sometimes we round (either with (int)(x+0.5) or by calling
axis_map_toint()) and sometimes we floor ( (int)(x) ). Looks like it's
more or less random which one we pick. We should always round, I
suspect. This patch series doesn't attempt to resolve this, but a future
set of patches should.

2. I discovered the issue with the vector fields manifests in two
different ways. For very short vectors, they simply disappear, as we
discussed. For vectors that are short, but not 0-pixels-short, the
arrowheads are still plotted, but the angular resolution becomes very
poor. The patches here resolve both of these.


Thanks.

Re: Short vectors are not plotted

[Ethan A M. <EAM...@gm...>]

On Saturday, 04 November 2017 21:43:09 Dima Kogan wrote:
> Ethan Merritt <eam...@gm...> writes:
> 
> > Here's a start. I'll probably commit these changes to CVS when the
> > current turmoil settles. After applying this patch there is only one
> > call site for term->arrow() in the code base, in draw_clip_arrow().
> > Could be I'm missing some complication, but it looks to me that any
> > changes needed for your heads-only implementation can be limited to
> > this one routine.
> 
> Here's a new patch series. These assume your use_draw_clip_arrow.patch
> is applied. Again, please apply with 'git am', or let me know what is
> unsatisfactory, so that I can address it.
> 
> The updates are similar to the last patch series in that some functions
> took integers previously, but now they take floating-point values. In a
> number of cases, this patch series makes available both flavors: integer
> and floating-point. Is this what you had in mind?
> 
> And two more related points:
> 
> 1. This patch uncovered an inconsistency in much of our code: when
> converting a floating-point terminal coordinate to an integer one,
> sometimes we round (either with (int)(x+0.5) or by calling
> axis_map_toint()) and sometimes we floor ( (int)(x) ). Looks like it's
> more or less random which one we pick. We should always round, I
> suspect. This patch series doesn't attempt to resolve this, but a future
> set of patches should.

Your patch set definitely runs aground on that point.
I tried to compare results before/after the patches but there are so many
single-digit coordinate changes throughout that it's impossible.
Yes it may be worth it to review the double->termcoord conversion 
everywhere in the program, but let's disentangle that from changes to
handling short vectors.

Here's how I tested:
    ./gnuplot-before -e 'set term post color' all.dem < /bin/yes > all_before.ps
    ./gnuplot-after -e 'set term post color' all.dem < /bin/yes > all_after.ps

    diff -ur all_before.ps all_after.ps > all.diff
    diffstat all.diff

    diffstat all.diff
     all_after.ps |234431 +++++++++++++++++++++++++++++++-----------------------------
     1 file changed, 121820 insertions(+), 112611 deletions(-)

Ugh. Too many differences to evaluate in detail.  So I tried to back out all
the changes to existing coordinate routines and instead provide totally
separate routines to be called only along the new clip_arrow code path.
This reduced the background single-pixel changes a bit, but I must have
missed some places because are still a very large number of single-pixel
differences.

     diff -ur all_before.ps all_version2.ps > all_v2.diff
     diffstat all_v2.diff
     all_version2.ps |59388 +++++++++++++++++++++++++++++++++--------------------------
     1 file changed, 34040 insertions(+), 25348 deletions(-)

That let me look for problem areas, and I found some.
There are many cases where the coordinates passed to the terminal
driver have overflowed.  For example the "Let's smile with parametric filled curves" plot
in the filledcurves demo ends like this:

    gsave [] 0 setdash
    3980 1811 M
    0 -221 V
    -218 37 V
    -653259491 771754262 M
    3980 1590 L
    stroke
    grestore

The coordinates in that last move (M) command are nonsense.
This is also evident in the *.ps output from the Gantt chart demo.
Note that this demo seems to display correctly on the Qt termimal.
That might mean there's a postscript-specific bug or it might
be different terminal scale factors, or different terminal clipping
policies, or something else.

However, my quick hack to separate the generic and arrow-specific
effects of your patches may have introduced its own bugs.  
So I think I will hand it back to you to debug my debugging :-)

	Ethan


> 2. I discovered the issue with the vector fields manifests in two
> different ways. For very short vectors, they simply disappear, as we
> discussed. For vectors that are short, but not 0-pixels-short, the
> arrowheads are still plotted, but the angular resolution becomes very
> poor. The patches here resolve both of these.
> 
> 
> Thanks.
> 
> 

Re: Short vectors are not plotted

[Dima K. <gn...@di...>]

Ethan A Merritt <EAM...@gm...> writes:

> On Saturday, 04 November 2017 21:43:09 Dima Kogan wrote:
>
>> 1. This patch uncovered an inconsistency in much of our code: when
>> converting a floating-point terminal coordinate to an integer one,
>> sometimes we round (either with (int)(x+0.5) or by calling
>> axis_map_toint()) and sometimes we floor ( (int)(x) ). Looks like it's
>> more or less random which one we pick. We should always round, I
>> suspect. This patch series doesn't attempt to resolve this, but a future
>> set of patches should.
>
> Your patch set definitely runs aground on that point.
> I tried to compare results before/after the patches but there are so many
> single-digit coordinate changes throughout that it's impossible.
> Yes it may be worth it to review the double->termcoord conversion
> everywhere in the program, but let's disentangle that from changes to
> handling short vectors.
>
> Here's how I tested:
>     ./gnuplot-before -e 'set term post color' all.dem < /bin/yes > all_before.ps
>     ./gnuplot-after -e 'set term post color' all.dem < /bin/yes > all_after.ps
>
>     diff -ur all_before.ps all_after.ps > all.diff
>     diffstat all.diff
>
>     diffstat all.diff
>      all_after.ps |234431 +++++++++++++++++++++++++++++++-----------------------------
>      1 file changed, 121820 insertions(+), 112611 deletions(-)

That is a great way to test this! Definitely keeping that in mind for
the future.

I have new patches (attached). This is a similar series from before,
except with finer granularity. The bulk of the main patch lives in patch
0003. This is the same as before except:

- The meat of the changes in draw_clip_arrow() has been left out
- The previous round/truncate behavior was carefully preserved

Thus your all.dem test results after applying patch 0003 should match
those before applying any of these patches.

Then patch 0004 is a short one that removes the compatibility off-by-one
changes and patch 0005 actually updates draw_clip_arrow() to handle the
short vectors.

Clearly patches 0004 and 0005 WILL produce rendering differences.

To be clear, these patches still don't try to clean up the
round/truncate differences, but at least make it obvious where the
questionable spots are regarding this patch series: in all the things
that patch 0004 touches.



> That let me look for problem areas, and I found some. There are many
> cases where the coordinates passed to the terminal driver have
> overflowed. For example the "Let's smile with parametric filled
> curves" plot in the filledcurves demo ends like this:
>
>     gsave [] 0 setdash
>     3980 1811 M
>     0 -221 V
>     -218 37 V
>     -653259491 771754262 M
>     3980 1590 L
>     stroke
>     grestore
>
> The coordinates in that last move (M) command are nonsense.

I don't see this. Could it be because I applied the postscript-terminal
patch you sent out?

I can think of one piece of the new code that could be producing an
overflow, though. In draw_clip_arrow() I have this:

    // Direction vector in (dex,dey). I need to convert this to integers
    // with a scale that's large-enough to give me good angular resolution,
    // but small-enough to not overflow the data type. Let's aim for the
    // vectors to be on the order of 1e6 ~ 2^20
    dex -= dsx;
    dey -= dsy;

    double delta_largest = fmax( fabs(dex), fabs(dey) );
    double scale_want    = (double)(1U << 20);
    double scale = scale_want / delta_largest;

    dex *= scale_want;
    dey *= scale_want;

Maybe 2^20 is too large for some terminals?

Re: Short vectors are not plotted

[sfeam <sf...@us...>]

On Saturday, 11 November 2017 17:50:18 you wrote:
> Ethan A Merritt <EAM...@gm...> writes:
> 
> > On Saturday, 04 November 2017 21:43:09 Dima Kogan wrote:
> >
> >> 1. This patch uncovered an inconsistency in much of our code: when
> >> converting a floating-point terminal coordinate to an integer one,
> >> sometimes we round (either with (int)(x+0.5) or by calling
> >> axis_map_toint()) and sometimes we floor ( (int)(x) ). Looks like it's
> >> more or less random which one we pick. We should always round, I
> >> suspect. This patch series doesn't attempt to resolve this, but a future
> >> set of patches should.
> >
> > Your patch set definitely runs aground on that point.
> > I tried to compare results before/after the patches but there are so many
> > single-digit coordinate changes throughout that it's impossible.
> > Yes it may be worth it to review the double->termcoord conversion
> > everywhere in the program, but let's disentangle that from changes to
> > handling short vectors.
> >
> > Here's how I tested:
> >     ./gnuplot-before -e 'set term post color' all.dem < /bin/yes > all_before.ps
> >     ./gnuplot-after -e 'set term post color' all.dem < /bin/yes > all_after.ps
> >
> >     diff -ur all_before.ps all_after.ps > all.diff
> >     diffstat all.diff
> >
> >     diffstat all.diff
> >      all_after.ps |234431 +++++++++++++++++++++++++++++++-----------------------------
> >      1 file changed, 121820 insertions(+), 112611 deletions(-)
> 
> That is a great way to test this! Definitely keeping that in mind for
> the future.
> 
> I have new patches (attached). This is a similar series from before,
> except with finer granularity. The bulk of the main patch lives in patch
> 0003. This is the same as before except:
> 
> - The meat of the changes in draw_clip_arrow() has been left out
> - The previous round/truncate behavior was carefully preserved
> 
> Thus your all.dem test results after applying patch 0003 should match
> those before applying any of these patches.
> 
> Then patch 0004 is a short one that removes the compatibility off-by-one
> changes and patch 0005 actually updates draw_clip_arrow() to handle the
> short vectors.

Very good.
That allows me to test before/after applying 0005 so that I can evaluate
the intended change by itself.  Here's what I see.

1) Something has gone wrong with the arrowhead direction.
    Try running arrowstyle.dem.   All the arrows at the page bottom have
    inverted arrowheads.

2) The patch has lost the distinction between "fixed" arrowhead size and
     the default variable arrowhead size.  Many of the arrows in the
     arrowstyle demo look strange because their heads are too big.

3) Arrowstyle 6 in the demo is supposed to show open unfilled heads.
    After patch 0005 the arrow shaft extends through the head so it no
    longer appears unfilled.

4) Not a result of your patch but I realized while testing - The code in
    post.trm that was messing up was intended to enforce solid lines 
    when drawing an arrowhead (dotted arrowheads look terrible).
    Since version 5 introduced dash patterns to all terminals, all terminals
    now suffer from this.  Any fix to replace the one in post.trm will have
    to catch all terminals.   But maybe it's not worth it.

I attach a revised version of your patch 0005 that addresses 1 + 2.

It adds a parameter to draw_clip_arrow that passes the fixed/variable
arrowhead size state.  I changed most the call sites to match.
This change is in patch 0006

0005-EAM is to be applied instead of your previous 0005
0006-EAM is to be applied on top of it
(these are not git patch-format.  I'm still getting up to speed with that).


I consider this an imperfect solution to be used for debugging.
A cleaner solution would be to pass a pointer
to the structure containing the arrowhead style.  E.g. instead of
      draw_clip_arrow(x1, y1, x2, y2, ap.head, ap.head_fixedsize);
it would be
      draw_clip_arrow(x1, y1, x2, y2, &ap);

I didn't change the call sites in util3d.c (draw3d_line_unconditional)
you'll have to dummy those up for testing.  I think that routine will
need additional changes eventually.

> Clearly patches 0004 and 0005 WILL produce rendering differences.
> To be clear, these patches still don't try to clean up the
> round/truncate differences, but at least make it obvious where the
> questionable spots are regarding this patch series: in all the things
> that patch 0004 touches.

I understand.  I will disregard the 1-pixel change side effects for now.

[snip fixed problem with post.trm]
> I don't see this. Could it be because I applied the postscript-terminal
> patch you sent out?
Yes.

> I can think of one piece of the new code that could be producing an
> overflow, though. In draw_clip_arrow() I have this:
> 
>     // Direction vector in (dex,dey). I need to convert this to integers
>     // with a scale that's large-enough to give me good angular resolution,
>     // but small-enough to not overflow the data type. Let's aim for the
>     // vectors to be on the order of 1e6 ~ 2^20
>     dex -= dsx;
>     dey -= dsy;
> 
>     double delta_largest = fmax( fabs(dex), fabs(dey) );
>     double scale_want    = (double)(1U << 20);
>     double scale = scale_want / delta_largest;
> 
>     dex *= scale_want;
>     dey *= scale_want;
> 
> Maybe 2^20 is too large for some terminals?

Yes.  See revised code in my version of 0005 that uses a
terminal-specific scale.

I have no clue what is going on with bug (3).  Maybe you can spot the error.
Otherwise I think this is converging rapidly on a nice improvement.

As noted above I am suspicious that the 3D code may trip other problems
but I have not looked at it very hard.

	Ethan

Re: Short vectors are not plotted

[sfeam <sf...@us...>]

On Saturday, 11 November 2017 21:58:48 sfeam via gnuplot-beta wrote:
> Very good.
> That allows me to test before/after applying 0005 so that I can evaluate
> the intended change by itself.  Here's what I see.
> 
> 1) Something has gone wrong with the arrowhead direction.
>     Try running arrowstyle.dem.   All the arrows at the page bottom have
>     inverted arrowheads.
> 
> 2) The patch has lost the distinction between "fixed" arrowhead size and
>      the default variable arrowhead size.  Many of the arrows in the
>      arrowstyle demo look strange because their heads are too big.
> 
> 3) Arrowstyle 6 in the demo is supposed to show open unfilled heads.
>     After patch 0005 the arrow shaft extends through the head so it no
>     longer appears unfilled.

[snip]

> I have no clue what is going on with bug (3).  Maybe you can spot the error.
> Otherwise I think this is converging rapidly on a nice improvement.


Found it. 
The old code adjusted the shaft length so that it stopped short of the head.
The adjustment depends on the head style.
The new code always calls (*t->arrow)(sx, sy, ex, ey, NOHEAD)
and then draws the head separately. 
This means that the shaft is never corrected for the head length because
the head style is not known at that point. 

	Ethan

Re: Short vectors are not plotted

[Dima K. <gn...@di...>]

sfeam <sf...@us...> writes:

> That allows me to test before/after applying 0005 so that I can evaluate
> the intended change by itself.  Here's what I see.
>
> 1) Something has gone wrong with the arrowhead direction.
>     Try running arrowstyle.dem.   All the arrows at the page bottom have
>     inverted arrowheads.
>
> 2) The patch has lost the distinction between "fixed" arrowhead size and
>      the default variable arrowhead size.  Many of the arrows in the
>      arrowstyle demo look strange because their heads are too big.
>
> I attach a revised version of your patch 0005 that addresses 1 + 2.
>
> It adds a parameter to draw_clip_arrow that passes the fixed/variable
> arrowhead size state.  I changed most the call sites to match.
> This change is in patch 0006

Hold on. I don't understand this. What is "fixed"? The docs say ONLY

  By default the size of the arrow head is reduced for very short
  arrows. This can be disabled using the `fixed` keyword after the
  `size` command.

I don't know what that means. A bit of testing with the demo you
mentioned revealed a few bugs with my draw_clip_arrow(). The best one I
have is attached. Note that it doesn't have "fixed" path since I can't
tell what it's supposed to do. If nothing else, the way you implemented
that path suffers from the truncation issue.

With that function, the arrowstyle.dem demo looks "correct", except the
line-through-should-be-empty arrowhead problem you mentioned. Some
arrowheads are shorter than I'd expect, but adding "fixed" to the
arrowstyle in the demo makes it consistent. Is this the "fixed" you're
talking about?

Re: Short vectors are not plotted

[sfeam <sf...@us...>]

On Sunday, 12 November 2017 13:24:28 Dima Kogan wrote:
> sfeam <sf...@us...> writes:
> 
> > That allows me to test before/after applying 0005 so that I can evaluate
> > the intended change by itself.  Here's what I see.
> >
> > 1) Something has gone wrong with the arrowhead direction.
> >     Try running arrowstyle.dem.   All the arrows at the page bottom have
> >     inverted arrowheads.
> >
> > 2) The patch has lost the distinction between "fixed" arrowhead size and
> >      the default variable arrowhead size.  Many of the arrows in the
> >      arrowstyle demo look strange because their heads are too big.
> >
> > I attach a revised version of your patch 0005 that addresses 1 + 2.
> >
> > It adds a parameter to draw_clip_arrow that passes the fixed/variable
> > arrowhead size state.  I changed most the call sites to match.
> > This change is in patch 0006
> 
> Hold on. I don't understand this. What is "fixed"? The docs say ONLY
> 
>   By default the size of the arrow head is reduced for very short
>   arrows. This can be disabled using the `fixed` keyword after the
>   `size` command.
> 
> I don't know what that means. 

"fixed" means "fixed arrowhead size".  The arrow head is always the same
size regardless of the vector length.  Without this keyword the arrowhead
size for short vectors is scaled down in proportion with the length.

> A bit of testing with the demo you
> mentioned revealed a few bugs with my draw_clip_arrow(). The best one I
> have is attached. Note that it doesn't have "fixed" path since I can't
> tell what it's supposed to do. If nothing else, the way you implemented
> that path suffers from the truncation issue.

That's not "suffering".  That's the intended behaviour :-)
By default the arrowhead scales with the vector length, so as the length
goes to zero the size of the head does  also.
I don't think truncation is relevant.

> With that function, the arrowstyle.dem demo looks "correct", except the
> line-through-should-be-empty arrowhead problem you mentioned. Some
> arrowheads are shorter than I'd expect, but adding "fixed" to the
> arrowstyle in the demo makes it consistent. Is this the "fixed" you're
> talking about?

Yes, but the new code has to handle both the default case and the
"fixed size" case.   Adding "fixed" to the demo hides the problem.

With regard to the code you attached, I think this part can never happen:

    bool drawbody = true;
    if( head < 0 )
    {
        drawbody = false;
        head = -head;
    }

Unless I overlooked a code path, draw_clip_arrow() is never called with (head < 0).
That convention "negative means only draw the arrowhead" is used only by the
term->arrow() entry, and draw_clip_arrow  itself is the only remaining caller of
term->arrow.   So I think the code I sent previously in 
0005-EAM-draw_clip_arrow-with-double-precision.patch
was correct.