gnuplot-beta

Bug in 5.4.rc2

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

I just hit a bug in 5.4.rc2. I'm guessing this isn't a new bug, but it'd
be nice to fix it before the release. To reproduce:

  set size ratio -1
  plot '-'   using 1:2:3 notitle with circles lw 2
  0 0 2.5
  e

It looks like when "with circles" data is used for autoscaling, the
edges of the circles are used for the x-axis autoscaling, but only the
centers for the y-axis autoscaling. So in the plot above, most of the
one circle gets cut off.

I can debug and come up with a patch, but some of you can do that MUCH
faster than me.

If yall are busy, let me know, and I can produce a patch.

Thanks!

Re: Bug in 5.4.rc2

[Ethan A M. <me...@uw...>]

On Monday, 22 June 2020 23:11:41 PDT Dima Kogan wrote:
> I just hit a bug in 5.4.rc2. I'm guessing this isn't a new bug, but it'd
> be nice to fix it before the release. To reproduce:
> 
>   set size ratio -1
>   plot '-'   using 1:2:3 notitle with circles lw 2
>   0 0 2.5
>   e
> 
> It looks like when "with circles" data is used for autoscaling, the
> edges of the circles are used for the x-axis autoscaling, but only the
> centers for the y-axis autoscaling. So in the plot above, most of the
> one circle gets cut off.

Autoscaling on y for "with circles" is not a well-defined operation.
The whole point of the style is that the circle radius depends only on x.
It will accommodate any yrange by adjusting the extent on y taking
into account the aspect ratio of the plot.

	Ethan  


> 
> I can debug and come up with a patch, but some of you can do that MUCH
> faster than me.
> 
> If yall are busy, let me know, and I can produce a patch.
> 
> Thanks!

Re: Bug in 5.4.rc2

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

I finished a patch just as you wrote this. The patch is attached. I
tried several operations, and it appears to do what I want, and not
break anything. The code has a lot of confusion about the meaning of
xlow/xhigh/ylow/yhigh in arguments to store2d_point() and again, in the
*cp structure this function fills. The previous code was passing the x,y
extents to store2d_point(), but if you do that, you run out of
arguments, and can't pass the y extents even if you wanted to. This
patch computes the extents inside store2d_point(), so there are
arguments left over. This patch also documents the meanings of
xlow/xhigh/ylow/yhigh.



Ethan A Merritt <me...@uw...> writes:

> On Monday, 22 June 2020 23:11:41 PDT Dima Kogan wrote:
>
> Autoscaling on y for "with circles" is not a well-defined operation.
> The whole point of the style is that the circle radius depends only on
> x. It will accommodate any yrange by adjusting the extent on y taking
> into account the aspect ratio of the plot.

Maybe I'm abusing the intent of the style here, but in my intended
usage, the meaning is well-defined, and the existing behavior highly
surprising. I want a circle in the x and y coordinates of the plot. If
the aspect ratio is square, it will look like a circle. If the aspect
ratio isn't square, it will look like an ellipse. Is "with circles" not
the proper style? Do I want "with ellipses"? I do see that what you're
saying is spelled-out in the "with circles" documentation; I guess I
thought the meaning of "circle" was clear-enough such that I didn't need
to read that page :)

And regardless of the answer to that question, the user expectation is
that the autoscaling would cause everything in the plot to be visible.
So given that, we shouldn't be cutting off any of the plotted circle.

Thanks

Re: Bug in 5.4.rc2

[Ethan A M. <me...@uw...>]

On Tuesday, 23 June 2020 11:24:24 PDT Dima Kogan wrote:
> I finished a patch just as you wrote this. The patch is attached. I
> tried several operations, and it appears to do what I want, and not
> break anything. The code has a lot of confusion about the meaning of
> xlow/xhigh/ylow/yhigh in arguments to store2d_point() and again, in the
> *cp structure this function fills. The previous code was passing the x,y
> extents to store2d_point(), but if you do that, you run out of
> arguments, and can't pass the y extents even if you wanted to. This
> patch computes the extents inside store2d_point(), so there are
> arguments left over. 

That won't work.
The eventual extent of a cicle along the y axis will not be known
until after the scaling on both x and y is complete.  For an auto-scaled
plot that will not be true until all the plot components, including functions,
have been processed.  There is not enough information yet at the time
store2d_point() is called.

"with circles" is not the only plot style for which autoscaling must
be delayed till later.  That is the reason for the extra routines
box_range_fiddling()  impulse_range_fiddling() histogram_range_fiddling()
and so on.   However for those plot styles the extra routine can
be called following the individual plot component "with <whatever>".
To handle circles you'd have to wait until _all_ plot components had
contributed to the autoscaling.  That could be done, but it would introduce
a new stage of processing that has not been needed before this.
And even so it would be imprecise, because if you must increase
the y range to fit the circles that will itself increase the y-radius of
the circles so you would need to iterate or estimate the asymptote.

So not impossible, but more difficult than autoscaling other plot styles.

	Ethan

Re: Bug in 5.4.rc2

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

Dima Kogan <gn...@di...> writes:

> I finished a patch just as you wrote this.

So I just played with the patched code a bit more, and it's plotting the
circles as circles even if the aspect ratio is not square. So it's
clearly insufficient. Hmm

Re: Bug in 5.4.rc2

[Ethan A M. <me...@uw...>]

On Tuesday, 23 June 2020 11:33:13 PDT Dima Kogan wrote:
> Dima Kogan <gn...@di...> writes:
> 
> > I finished a patch just as you wrote this.
> 
> So I just played with the patched code a bit more, and it's plotting the
> circles as circles even if the aspect ratio is not square. So it's
> clearly insufficient. Hmm

Indeed. That is the purpose of the style.  It _always_ draws circles,
regardless of aspect ratio and regardless of y scaling.

Sounds like your plot wants to use  "with ellipses".

	Ethan

Re: Bug in 5.4.rc2

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

Ethan A Merritt <me...@uw...> writes:

> Sounds like your plot wants to use "with ellipses".

OK, I'm using ellipses now. It does what I want. I just hit a corner
case, however that did a surprising thing. The corner case is this:

  plot '-' notitle with ellipses
  0 0 0 0

I'm plotting an ellipse at the origin with axis lengths 0. The
previously stated purpose was to draw an ellipse with the described
geometry against the graph coordinates, instead of drawing an abstract
symbol, like "with circles" does. Thus here I would expect nothing
drawn, or maybe just a single dot drawn. Instead it draws an ellipse of
some arbitrary size, independent of the zoom level: it's an abstract
symbol.

Proposal: ellipses with axis lengths >= 0 are not abstract symbols, and
are drawn as the data dictates. axis lengths < 0 may be rendered as
symbols.

Reasonable?

Re: Bug in 5.4.rc2

[Ethan A M. <me...@uw...>]

On Friday, 26 June 2020 02:21:58 PDT Dima Kogan wrote:
> Ethan A Merritt <me...@uw...> writes:
> 
> > Sounds like your plot wants to use "with ellipses".
> 
> OK, I'm using ellipses now. It does what I want. I just hit a corner
> case, however that did a surprising thing. The corner case is this:
> 
>   plot '-' notitle with ellipses
>   0 0 0 0
> 
> I'm plotting an ellipse at the origin with axis lengths 0. The
> previously stated purpose was to draw an ellipse with the described
> geometry against the graph coordinates, instead of drawing an abstract
> symbol, like "with circles" does.

I do not recognize that description.  Abstract symbol?
Sounds like a bug.  There is no special code in plot_circles() to
do something other than draw an arc of a circle.
What exactly did you provide as a plot command, and what did
the symbol look like?

> Thus here I would expect nothing
> drawn, or maybe just a single dot drawn. Instead it draws an ellipse of
> some arbitrary size, independent of the zoom level: it's an abstract
> symbol.

I can see why you would expect the size to go to zero or a dot if
the axis length values reach 0.  That would be reasonable, but currently
it interprets that as a request to use the default properties set by
      set style ellipse ...
which also seem reasonable.  Did it do something else?

> Proposal: ellipses with axis lengths >= 0 are not abstract symbols, and
> are drawn as the data dictates. axis lengths < 0 may be rendered as
> symbols.
> 
> Reasonable?

Currently the code uses the absolute value of the axis lengths,
so negative values have no special meaning.
What kind of symbols are you proposing,
and when would you use this rather than plotting `with points`?

	Ethan

	Ethan

Re: Bug in 5.4.rc2

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

Ethan A Merritt <me...@uw...> writes:

> On Friday, 26 June 2020 02:21:58 PDT Dima Kogan wrote:
>> Ethan A Merritt <me...@uw...> writes:
>>
>> > Sounds like your plot wants to use "with ellipses".
>>
>> OK, I'm using ellipses now. It does what I want. I just hit a corner
>> case, however that did a surprising thing. The corner case is this:
>>
>>   plot '-' notitle with ellipses
>>   0 0 0 0
>>
>> <snip>
>>
>> Thus here I would expect nothing
>> drawn, or maybe just a single dot drawn. Instead it draws an ellipse of
>> some arbitrary size, independent of the zoom level: it's an abstract
>> symbol.
>
> I can see why you would expect the size to go to zero or a dot if
> the axis length values reach 0.  That would be reasonable, but currently
> it interprets that as a request to use the default properties set by
>       set style ellipse ...
> which also seem reasonable.  Did it do something else?
>
>> Proposal: ellipses with axis lengths >= 0 are not abstract symbols, and
>> are drawn as the data dictates. axis lengths < 0 may be rendered as
>> symbols.
>>
>> Reasonable?
>
> Currently the code uses the absolute value of the axis lengths,
> so negative values have no special meaning.

Right. But that's an implementation detail. The data could potentially
contain size-0 ellipses, and they should be drawn as size-0 ellipses. If
we really want to have a mode where the default properties are used
instead, they should be accessed with size<0. But I don't actually think
that's useful: if the user is specifying an ellipse size, we should use
it.

Want a patch?

Re: Bug in 5.4.rc2

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

The simplest patch is this:

diff --git a/src/plot2d.c b/src/plot2d.c
index c81fc2344..a8e598359 100644
--- a/src/plot2d.c
+++ b/src/plot2d.c
@@ -1097,7 +1097,7 @@ get_data(struct curve_points *current_plot)
 	    coordval major_axis = (j >= 3) ? fabs(v[2]) : 0.0;
 	    coordval minor_axis = (j >= 4) ? fabs(v[3]) : (j >= 3) ? fabs(v[2]) : 0.0;
 	    coordval orientation = (j >= 5) ? v[4] : 0.0;
-	    coordval flag = (major_axis > 0 && minor_axis > 0) ? 0.0 : DEFAULT_RADIUS;
+	    coordval flag = (j >= 3) ? 0.0 : DEFAULT_RADIUS;

 	    if (j == 2)	/* FIXME: why not also for j == 3 or 4? */
 		orientation = default_ellipse.o.ellipse.orientation;

I.e. we use the default ellipse style only if no axis sizes are given at
all. If anything is given, we use it. Thoughts?

Re: Bug in 5.4.rc2

[Ethan A M. <me...@uw...>]

On Friday, 26 June 2020 12:42:04 PDT Dima Kogan wrote:
> The simplest patch is this:
> 
> diff --git a/src/plot2d.c b/src/plot2d.c
> index c81fc2344..a8e598359 100644
> --- a/src/plot2d.c
> +++ b/src/plot2d.c
> @@ -1097,7 +1097,7 @@ get_data(struct curve_points *current_plot)
>  	    coordval major_axis = (j >= 3) ? fabs(v[2]) : 0.0;
>  	    coordval minor_axis = (j >= 4) ? fabs(v[3]) : (j >= 3) ? fabs(v[2]) : 0.0;
>  	    coordval orientation = (j >= 5) ? v[4] : 0.0;
> -	    coordval flag = (major_axis > 0 && minor_axis > 0) ? 0.0 : DEFAULT_RADIUS;
> +	    coordval flag = (j >= 3) ? 0.0 : DEFAULT_RADIUS;
> 
>  	    if (j == 2)	/* FIXME: why not also for j == 3 or 4? */
>  		orientation = default_ellipse.o.ellipse.orientation;
> 
> I.e. we use the default ellipse style only if no axis sizes are given at
> all. If anything is given, we use it. Thoughts?

I think I like the other option better:
	zero means zero
	negative means use default from "set style ellipse"

But I also think you can achieve what you want without changing the code
at all, by setting the default ellipse to have 0 size.
Right?

I'm working on something else at the moment.
I'll get back to this afterwards.

	Ethan