|
From: Ethan A M. <merritt@u.washington.edu> - 2009-01-05 00:07:44
|
On Sunday 04 January 2009, you wrote:
>
> On Sat, 3 Jan 2009, Ethan A Merritt wrote:
>
> >> Of course it is possible to increase the number of slots in
> >>
> >> typedef struct coordinate {
> >> enum coord_type type; /* see above */
> >> coordval x, y, z;
> >> coordval ylow, yhigh; /* ignored in 3d */
> >> coordval xlow, xhigh; /* also ignored in 3d */
> >> } coordinate;
> >>
> >> from 7 to some larger number.
> >
> > For instance, I would welcome a patch that added a field
> > coordval color;
> > as already commented in the code.
> > The current handling of color information is a terrible hodgpodge,
> > with different plot style storing it in various different slots,
> > and the actual plot code then having to figure out where to pull
> > it from.
>
> I am interested in working out a patch but don't have a good
> unterstanding of the code and so not a good idea yet of the
> magnitude of the task. Would most necessary changes take
> place in datafile.c and graphics.c or are other places affected
> as well?
INPUT SIDE
==========
The key routines on the input side are
1) plot2d.c: store2d_point()
store2d_point(
struct curve_points *current_plot,
int i, /* point number */
double x, double y,
double xlow, double xhigh,
double ylow, double yhigh,
double width) /* BOXES widths: -1 -> autocalc, 0 -> use xlow/xhigh */
It stores 7 data values for each data point. Confusingly, other than x and y
these are often not used for what the name implies. Thus "ylow" or "yhigh"
is often the variable color, and "width" is an ugly overloading of either
z or some magic flag depending on what the plot style is.
My first thought is to increase the number of parameters and make them match
up more consistently with the actual field contents:
store2d_point( ...,
x,y,xlow,xhigh,ylow,yhigh /* as before, but this time we really mean it */
z /* the 7th existing slot, now named properly */
color /* the new slot */
variable_prop1 /* somewhat problematic */
variable_prop2 /* e.g. pointsize variable */
)
The first 7 parameters, if relevant to the current plot style, would be
stored directly in the corresponding slot in the coord structure.
Color would be either stored directly or calculated from other values and
stored in the new 8th slot.
If we are willing to add a 9th or even a 10th slot, then additional variable
properties like point size or point type could be stored in a dedicated slot;
otherwise they could continue as they are now to be aliased on top of
xlow, xhigh, ylow and yhigh (which are used by only a few plot types).
9 slots would be enough, I think, for every plot style except maybe
xyerrorlines.
2) the macros in axis.h
STORE_WITH_LOG_AND_UPDATE_RANGE()
COLOR_STORE_WITH_LOG_AND_UPDATE_RANGE()
These routines are called for each data line read, from a switch statement that
tries to figure out what kind of plot this is and therefore what data goes in which
of the seven slots. That switch statement could benefit from a total re-work
based on your proposed table, but that change can be done separately from any
work to clean up what data is stored in which slot.
Every call site for these routines/macros would have to be modified in
accordance with whatever change you make. That's a lot of places, so best to
do it only once. That is, let's think hard about what changes to make and
then modify all the callers in one go.
OUTPUT SIDE
===========
The routines that actual plot the data using the previously stored data
are mostly in graphics.c
static void plot_lines __PROTO((struct curve_points * plot));
static void plot_points __PROTO((struct curve_points * plot));
static void plot_dots __PROTO((struct curve_points * plot));
static void plot_bars __PROTO((struct curve_points * plot));
static void plot_boxes __PROTO((struct curve_points * plot, int xaxis_y));
...etc..
and in graph3d.c
static void plot3d_impulses __PROTO((struct surface_points * plot));
static void plot3d_lines __PROTO((struct surface_points * plot));
static void plot3d_points __PROTO((struct surface_points * plot, /* FIXME PM3D: */ int p_type));
static void plot3d_vectors __PROTO((struct surface_points * plot));
...etc...
There are a few routines in other source files that refer to the data values
directly, but many of these may not be affected by changes in color handling.
Let's look at the simplest case:
static void
plot_dots(struct curve_points *plot)
{
int i;
int x, y;
struct termentry *t = term;
for (i = 0; i < plot->p_count; i++) {
if (plot->points[i].type == INRANGE) {
x = map_x(plot->points[i].x);
y = map_y(plot->points[i].y);
/* rgb variable - color read from data column */
check_for_variable_color(plot, &plot->points[i]);
/* point type -1 is a dot */
(*t->point) (x, y, -1);
}
}
}
You can see that it pulls the x coordinate from point[i].x
and the y coordinate from point[i].y. No problem there.
Plots with error bars would pull their ranges from
point[i].ylow point[i].high, etc, also no problem.
But figuring out where to pull the color from is not so clean.
I tried to abstract the retrieval of color info by creating the helper
routine check_for_variable_color(), but it is not used everywhere.
It basically pulls and possibly applies the value stored in "yhigh".
NB: This is *not* what it says in the comment in gp_types.h
which claims that color info is in ylow (aliased as CRD_COLOR)
The plot styles which really do have a yhigh value obviously cannot
use this abstraction. Some of them instead use the CRD_COLOR alias,
others I don't even remember.
My thought is that all of these, the check_for_variable_color guys
and the CRD_COLOR guys and any remaining stragglers, should all be
changed to use the new field point[i].color, and the abstraction
routine should be extended as necessary to handle all plot styles.
--
Ethan A Merritt
|