|
From: Daniel J S. <dan...@ie...> - 2006-06-23 08:35:01
|
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) {
so that the for loop exits right away once it is determined that the tics should not be plotted. That would be good, because "step" could be very small and could go through the loop many, many times.
2) I think this method of determining when to not plot the tics, i.e., setting anyticput = 2 is silly. That could be easily done BEFORE even executing the for loop. 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. Second, look at this test:
if (anyticput) {
if (NearlyEqual(tic, start, step)) {
/* step is too small.. */
anyticput = 2; /* Don't try again. */
tic = end; /* Put end tic. */
"tic" starts out as "start". The test isn't perform the first time through. Second time through: tic = start + step, the closest that "tic" will ever be to "start". And NearlyEqual(x,y,tic) checks
fabs((tic)-(start)) < ((step) * SIGNIF))
=> fabs((start + step) - start) < (step * 0.01)
=> fabs(step) < (step * 0.01)
Which I believe is never true. Now, the only other intent here I can see is that perhaps it was meant to limit the number of tics to 100 per plot. But that can be done before the for loop and tested there. Bottom line, I think this is useless code.
This line of code looks similar:
} else if (NearlyEqual(tic, end, step)) {
but I think what this does is attempt to catch the rounding effects that might produce a "double tic" right at the end. This goes back to the fact that an integer index should be used, and in all likelihood this artifact can be avoided.
3) OK, let's say the intention of the first test in (2) was meant to limit the number of tics per plot to 100. I'm going to raise the question Why? I really don't care, but Hans made a valid point the other day about assuming some resolution of the plot and how any tiny overrun of the tic value should not be ignored. Presumably some plotting device could zoom in close enough that it could be important. Isn't the same sort of resolution restriction assumed by limiting the number of tics?
Dan
|