|
From: Ralf J. <jue...@cs...> - 2009-01-04 04:45:03
|
On Sat, 3 Jan 2009, Ethan A Merritt wrote: > [from a conversation started off list] > [summary] The question is whether the code in get_data() > plot2d.c lines 327ff can be replaced by a table-lookup, > or deleted altogether. This code sets limits min_cols and max_cols > for the number of columns specified in the "using" part of a plot > command. Ralf's proposed table would also hold properties like > PLOT_STYLE_HAS_FILL that are currently single bits set in the > line style definitions in gp_types.h The number of data columns all the plotting style accepts and what optional columns they may take (for variable line color or variable point size) is scattered across the documentation. There currently does not seem to be a place in the code that makes this information explicit. I think it would be useful to have this, however, for the reasons stated below. >> While my main motivation for this patch his making plot style >> properties explicit to applications that generate gnuplot >> scripts, it certainly is useful in that it improves clarity >> of the code. And I would not be surprised if having such a >> descriptor table would enable simplifications at other places >> as well. The reason I would rather see it builtin and used >> than external is that this guarantees the table is current. While working on this patch I noticed that xyerrorlines takes up to six data columns plus optionally two more (ps variable, lc variable), which exceeds MAXDATACOLS in datafile.h. Is this a known problem? Ralf |
|
From: Ethan A M. <merritt@u.washington.edu> - 2009-01-04 04:53:42
|
On Saturday 03 January 2009, Ralf Juengling wrote:
> While working on this patch I noticed that xyerrorlines takes
> up to six data columns plus optionally two more (ps variable, lc
> variable), which exceeds MAXDATACOLS in datafile.h. Is this a
> known problem?
Huh? xyerrorlines has no points, so how can it have 'ps variable'?
But yes, there are plot styles that would go over the 7 column limit
if all the properties were variable. This is particularly true
for "linespoints", and several people have complained about it.
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 large datasets this would
cause a significant increase in memory use, whether or not the
plot actually used the extra slots.
Maybe that's OK. Memory is cheap these days.
But if we are going to do that, we need some more coherent system of
where to store what kind of value. The definitions that are in the
code as deceptive, in that they are not always true.
/* These fields of 'struct coordinate' used for storing the color of 3D data
* points (if requested by NEED_PALETTE(this_plot), for instance).
*/
#define CRD_COLOR ylow
#define CRD_R yhigh
#define CRD_G xlow
#define CRD_B xhigh
#define CRD_A ylow
/* The field of 'struct coordinate' used for storing the point size in plot
* style POINTSTYLE with variable point size
*/
#define CRD_PTSIZE xlow
--
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742
|
|
From: Ethan A M. <merritt@u.washington.edu> - 2009-01-04 04:59:09
|
On Saturday 03 January 2009, Ethan A Merritt wrote:
> On Saturday 03 January 2009, Ralf Juengling wrote:
>
> > While working on this patch I noticed that xyerrorlines takes
> > up to six data columns plus optionally two more (ps variable, lc
> > variable), which exceeds MAXDATACOLS in datafile.h. Is this a
> > known problem?
>
> Huh? xyerrorlines has no points, so how can it have 'ps variable'?
>
> But yes, there are plot styles that would go over the 7 column limit
> if all the properties were variable. This is particularly true
> for "linespoints", and several people have complained about it.
>
> 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.
That would both simplify the color-handling code and free up
another data slot for storing values read to support other uses
of the "variable" attribute.
Ethan
> For large datasets this would
> cause a significant increase in memory use, whether or not the
> plot actually used the extra slots.
> Maybe that's OK. Memory is cheap these days.
> But if we are going to do that, we need some more coherent system of
> where to store what kind of value. The definitions that are in the
> code as deceptive, in that they are not always true.
>
> /* These fields of 'struct coordinate' used for storing the color of 3D data
> * points (if requested by NEED_PALETTE(this_plot), for instance).
> */
> #define CRD_COLOR ylow
> #define CRD_R yhigh
> #define CRD_G xlow
> #define CRD_B xhigh
> #define CRD_A ylow
> /* The field of 'struct coordinate' used for storing the point size in plot
> * style POINTSTYLE with variable point size
> */
> #define CRD_PTSIZE xlow
>
>
>
>
--
Ethan A Merritt
Biomolecular Structure Center
University of Washington, Seattle 98195-7742
|
|
From: Ethan A M. <merritt@u.washington.edu> - 2009-01-04 06:07:44
|
[from a conversation started off list] [summary] The question is whether the code in get_data() plot2d.c lines 327ff can be replaced by a table-lookup, or deleted altogether. This code sets limits min_cols and max_cols for the number of columns specified in the "using" part of a plot command. Ralf's proposed table would also hold properties like PLOT_STYLE_HAS_FILL that are currently single bits set in the line style definitions in gp_types.h On Saturday 03 January 2009, Ralf Juengling wrote: > > On Sat, 3 Jan 2009, Ethan A Merritt wrote: > > > Your patch may be heading towards something useful, I'll wait > > and see how it ends up. As I understand it, the idea is not so > > much to fix or improve gnuplot per se, but to make a table for > > use by an external program. I'm not sure it is worth doing this > > inside gnuplot; for the same amount of work you could just make > > the table part of the external program. That would have the > > advantage of working with older versions of gnuplot that don't > > have your table built in. > > While my main motivation for this patch his making plot style > properties explicit to applications that generate gnuplot > scripts, it certainly is useful in that it improves clarity > of the code. And I would not be surprised if having such a > descriptor table would enable simplifications at other places > as well. The reason I would rather see it builtin and used > than external is that this guarantees the table is current. > > Can you think of other properties that would be useful to > have in the table? I think it would be useful to know that > a plot style expects non-numeric input (e.g., labels). As you move into that kind of question, I start to think that the whole idea of a fixed set of properties for a given plot type breaks down. For instance, any plot command that includes using xticlabels(N) will require reading from column N and will be expecting non-numeric data. But this is outside of the count of columns that the code in get_data() is making, and the expectation of non-numeric data is tacked on to whatever expectations the plot style already had. Similarly, each use of the propery "variable" adds a column of expected input, independent of however many columns were already expected. I am not sure that preparing a table in advance actually serves any useful purpose, at least internal to gnuplot. One really has to total up the number of columns referred to in this specific plot command. The specific plot style is largely irrelevant. It was this kind of argument that made me inclined to delete all the code tracking min_cols and max_cols. Other than issuing an error message if the command fails to provide at least min_cols of using specs, they aren't much use for anything. It may not even generate an error message if you exceed max_cols, as you have already pointed out with regard to filledcurves. > What > aboutthe has_grid_topology property in struct surface_points? > Is that not plot-style specific property? In truth, I know essentially nothing about the 3dgrid code. Let's move this discussion to the mailing list so that other people can usefully comment. -- Ethan A Merritt |
|
From: Hans-Bernhard B. <HBB...@t-...> - 2009-01-04 17:46:05
|
Ethan A Merritt wrote: > [summary] The question is whether the code in get_data() > plot2d.c lines 327ff can be replaced by a table-lookup, > or deleted altogether. This code sets limits min_cols and max_cols > for the number of columns specified in the "using" part of a plot > command. That's not all that code does. And a table would fail to represent those other things. In short, I don't this hurts anywhere near bad enough to change it. > Ralf's proposed table would also hold properties like > PLOT_STYLE_HAS_FILL that are currently single bits set in the > line style definitions in gp_types.h I agree that that part would make sense. > As you move into that kind of question, I start to think that > the whole idea of a fixed set of properties for a given plot > type breaks down. Indeed. The original design has been thoroughly swamped under by new features. We're doing so much stuff outside (or in conflict with) the concept of plot styles that it's hard to see what it was originally meant to be: a complete description of how a given dataset would be displayed. And not a lot of that is described too well in the documentation, either... > It was this kind of argument that made me inclined to delete > all the code tracking min_cols and max_cols. Other than issuing > an error message if the command fails to provide at least min_cols > of using specs, they aren't much use for anything. Well, given that that's exactly what they're being computed for, why should they do more? > It may not even generate an error message if you exceed max_cols, as > you have already pointed out with regard to filledcurves. Now that would be bug. >> What >> aboutthe has_grid_topology property in struct surface_points? >> Is that not plot-style specific property? No. It's a feature of the data, not the plot style. Which is why it's in the data structure holding the dataset: struct surface_points. |
|
From: Ralf J. <jue...@cs...> - 2009-01-05 06:27:23
|
On Sun, 4 Jan 2009, Hans-Bernhard Bröker wrote: >> Ralf's proposed table would also hold properties like >> PLOT_STYLE_HAS_FILL that are currently single bits set in the >> line style definitions in gp_types.h > > I agree that that part would make sense. > >> As you move into that kind of question, I start to think that >> the whole idea of a fixed set of properties for a given plot >> type breaks down. > > Indeed. The original design has been thoroughly swamped under by new > features. We're doing so much stuff outside (or in conflict with) the > concept of plot styles that it's hard to see what it was originally meant to > be: a complete description of how a given dataset would be displayed. > And not a lot of that is described too well in the documentation, either... Hm, aren't you mixing different things here? A feature like 'using xticslabel(1)' is not specific to plot-styles, it works for all of them. These newer features may have affected the parts of the data reading machinery that currently includes plot style properties hard-coded (plot2d.c:get_data()), but that does not make them plot style properties. Here are the properties that I consider worth knowing; as a direct user of gnuplot, I sometimes want to job my memory and look them up; as an author of some other application that generates gnuplot script, I want to query them: * plot style keyword * number of basic data colums in 2d (excluding lc variable, ps variable) * if applicable, number of basic data colums in 3d * does expect non-numeric data * does accept line style specifiers * does accept point style specifiers * does accept fill style specifiers The two questions now are: Is a help command delivering such a summary a desirable feature? Is it a good idea to fix this in code as a table? Ralf |
|
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
|