|
From: <br...@ph...> - 2006-06-23 22:56:51
|
Daniel J Sebald wrote:
> Daniel J Sebald wrote:
>> To make bug 1004754 work properly and remove the grid line from outside the plot, I commented out the following lines of code from gen_tics() in axis.c:
>
> Taking another look at this routine, let's "decode" this code. First--well let's enumerate
>
> 1) The following code
>
> for (tic = start; tic <= end; tic += step) {
> if (anyticput == 2) /* See below... */
> break;
>
> would work better as
>
> for (tic = start; tic <= end, anyticput != 2; tic += step) {
No, it wouldn't. The comma operator is quite wrong for a conditional
expression. It would have to be replaced by &&, which would leave you
with code exactly equivalent to the original one.
> so that the for loop exits right away once it is determined that the
> tics should not be plotted.
That's exactly what the 'break' already does.
> 2) I think this method of determining when to not plot the tics,
> i.e., setting anyticput = 2 is silly.
It's not. It's complicated because it has to be. We learned that the
hard way, years ago. The code looks the way it does because it was the
simplest approach we found that actually worked reliably, even in
extreme cases.
> That could be easily done
> BEFORE even executing the for loop.
No, it can't. Numerical maths on an optimizing compiler is trickier
than you know. Things that look they should be mathematically
equivalent, rather often aren't.
> First, a more appropriate method
> of computing the tic should probably be to increment an integer index
> and compute from that, as pointed out in last email. It would be
> easy to compute the number of tics there are supposed to be before
> this foor loop.
It would be easy to compute, sure. Unfortunately, it can just as easily
be way off. Especially where floating point maths close to its limit of
precision is involved:
> => fabs((start + step) - start) < (step * 0.01)
> => fabs(step) < (step * 0.01)
That last line does *not* follow from the one above. Not where
floating-point arithmetic reigns.
|