|
From: Daniel J S. <dan...@ie...> - 2003-11-07 05:09:22
|
I've updated the image/binary patch (694789) to include Ethan's
drivers for PDF and PNG/JPEG. (I'm intending that to be the
last update.)
-------
I'm now going to offer some comments about ways I feel Gnuplot
can be improved in terms of code and enhanced in terms of
features. (Call it a harangue if you like, but I'm not intending this
to be critical.) No one need respond because the suggestions
are more long term. Or should I say post 4.0? Maybe just keep
this post around for future ideas.
1) Convergence of 2D and 3D plots: I'm not sure how Gnuplot
developed in terms of 2D and 3D plots. Perhaps they were
coded separately and then combined. Perhaps there was a
bifurcation along the way for some reason. In any case it seems
there is some duplication of code for plotting in 2D and in 3D.
For example, there is "plot_lines()" in "graphics.c" and "plot3d_lines()"
in "graph3d.c". But are these really that different? (Perhaps someone
knows better than I do if in fact they are drastically different.) It
seems to me that "map3d_xy" is the most important detail. But
couldn't this just be "map_xy()" and if the current mode is PLOT_MODE
as opposed to SPLOT_MODE, say, the map_xy() routine does a simple
unity mapping?
Another example is the image drawing routine I put in
the patch, which is designed to work for 2D or 3D plots. If a
Boolean variable indicates that a 3D mapping should be done
then the code will call "map3d_xy()". [I pause to make a point that
this mapping has to be at the point right where the data is utilized.
The reason is that the data stored in 3d plots is before mapping
because the viewing angle can be changed and the splot redrawn.
The only other way is to map all data and store in a temporary chunk
of memory and pass that in. But I don't think anyone would think
that is a good idea.] This image plotting routine is in "graphics.c".
Also in "graphics.c" I see some notes left by Hans such as
> /* FIXME HBB 20020225: this is shared with graph3d.c, so it shouldn't
> * be in this module */
so I'm guessing he's been faced with trying to adhere to the practice
of code reuse as well.
The point, I believe, is that when it boils down to the actual process
of plotting something, it is always in the 2D viewing plane that this
is done. Whether this means first mapping from the 3D space or not
is a minor detail, I think.
So a bit contrary to Hans remark in the code, I would contend that
graphics.c is the appropriate spot for all graphic object drawing
routines. Then the dimension-specific details should go into
"graph3d.c" and a new file "graph2d.c". Furthermore, consider that
there are two different structures for 2D plots and 3D plots,
"curve_points" and "surface_points". An argument could be made
for having just one structure. But even if that shouldn't be the case,
it is the
int p_count; /* count of points in points */
struct coordinate GPHUGE *points;
which are important to the actual graphics drawing routines. So
my suggestion would be to make all graphics drawing routines
operate on a pointer to the points and number of points or combine them in
their own structure and pass that into the routine. In fact, if one were to
think about this, I bet if these were packed together and put at the
start of both the "curve_points" and "surface_points" structures the
pointers to these plots could be passed in and there would be no need to
distinguish between the two types of pointers because the important data
would have the same offset in the structures. The graphics routines would
have a "map_xy()" right before they are used. (The conditional done
inside the "map_xy()" function to conserve code space.) So I guess that
means the mode would also need to be packed with those two. Anyway,
I hope you get my point.
2) Not too far removed from #1, with the image stuff I've added a
a short bit of code to allow a colorbox in 2D plots. It became clear
to Petr and me that the "3d" associated with colorboxes should be
dropped. So, it seems that a nice consistent method of adding a colorbox
to a plot, either 2D or 3D, is in order. Right now they are separate.
However, they should essentially use the same code when laying out
the plot. Petr had some ideas for enhancing the positioning of the color
box and it is logical to not have to change that in multiple spots. Also,
the slick routine should place the colorbox at the side of the plot. Petr
and I have agreed that having the colorbox embedded in the splot 3D
space, so that it moves and changes shape when panning the view, is
a bit lacking. We contemplated fixing that, but I think it is better
for the
group to think that one through.
3) Add the capability of RGB to polygons: Currently polygons use palette
lookup for generating the fill color. However, I don't see why there can't
be RGB components just like for images. I personally can't think right
now what the applications might be, but it doesn't seem too far fetched that
such a thing could find a use. (I don't know, contouring for landscapes,
color image warping, etc.) One might argue that would require altering
so many of the terminal drivers so the term->polygon has one extra
variable. But with xemacs, something like "xemacs *.trm" at the command
line will open all files and a replace function could get the job done in an
hour or two. Sure it would take some more work to add functionality to
the drivers, but various people could choose their favorite and make the
alterations.
4) Memory usage for storing data points: This one probably has the
least chance of being addressed. The "struct coordinate GPHUGE"
is a bit wasteful of memory if only a few columns of it store data that
is actually used. It would be nice to have a scheme not so wasteful,
say using a "void" pointer and store just the number of columns actually
used. However, this is some very "advanced" programming, especially
when there is a group of developers. Many would make the "worst
case scenario" argument for not doing this. So, ey, just an idea and
I wonder how many feel this is important.
5) There are some routines in "show" and "set" that list the options
for plot styles, etc. The strings for these always have to be updated
and some times may become out of date. Would a little routine be
worthwhile to simple go to the table and pull out the style strings and
print them? The routine would have to know when there is no more
room on an 80 character line to show the next string and first enter a
carriage return.
6) How about a simple "help search <text>" routine that searches through
all the gnuplot.doc strings and dumps out the headings of all subtopics that
contain <text>?
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2003-11-10 20:58:13
|
[Folks... tell me if this email is too big to be submitting
to the mailing list. Are people able to see the PNG
images in their browsers?]
Ethan Merritt wrote:
>On Monday 10 November 2003 09:30, you wrote:
>
>
>>Ethan A Merritt wrote:
>>
>>
>>>I'll try to test it on an alpha next week (other endian, 64-bit).
>>>
>>>
>>That will be interesting. Let me know how it goes.
>>
>>
>
>First report: not so good.
>
Well, not too bad either. I'm encouraged the images
look reasonable.
>There are additional warnings during compilation for datafile.c and gplt_x11.c
>
>cc -DHAVE_CONFIG_H -I. -I. -I.. -I../term -I../term -DBINDIR=\"/usr/local/bin\" -DX11_DRIVER_DIR=\"/usr/local/libexec/gnuplot/3.8j\" -DCONTACT=\"bug...@da...\" -DHELPFILE=\"/usr/local/share/gnuplot/3.8j/gnuplot.gih\" -I/usr/local/include -g -c datafile.c
>cc: Warning: datafile.c, line 3156: In this statement, the expression "*pchar=*++pchar" modifies "pchar", and fetches its value in a computation that is not used to produce the modified value without an intervening sequence point. This behavior is undefined.
> *pchar = *++pchar;
>------------------------^
>
I will change that in the code. But if you want, try
changing, in your code, the three lines in that loop to
char temp = *pchar;
*pchar = *(pchar+1);
*++pchar = temp;
>cc -DHAVE_CONFIG_H -I. -I. -I.. -I../term -I../term -DBINDIR=\"/usr/local/bin\" -DX11_DRIVER_DIR=\"/usr/local/libexec/gnuplot/3.8j\" -DCONTACT=\"bug...@da...\" -DHELPFILE=\"/usr/local/share/gnuplot/3.8j/gnuplot.gih\" -I/usr/local/include -g -c gplt_x11.c
>cc: Warning: gplt_x11.c, line 2435: In this statement, the referenced type of the pointer value "&R_rshift" is "short", which is not compatible with "unsigned short".
> R_msb_mask = BitMaskDetails(vis->red_mask, &R_rshift, &R_lshift);
>-----------------------------------------^
>cc: Warning: gplt_x11.c, line 2435: In this statement, the referenced type of the pointer value "&R_lshift" is "short", which is not compatible with "unsigned short".
> R_msb_mask = BitMaskDetails(vis->red_mask, &R_rshift, &R_lshift);
>-----------------------------------------^
>cc: Warning: gplt_x11.c, line 2436: In this statement, the referenced type of the pointer value "&G_rshift" is "short", which is not compatible with "unsigned short".
> G_msb_mask = BitMaskDetails(vis->green_mask, &G_rshift, &G_lshift);
>-----------------------------------------^
>cc: Warning: gplt_x11.c, line 2436: In this statement, the referenced type of the pointer value "&G_lshift" is "short", which is not compatible with "unsigned short".
> G_msb_mask = BitMaskDetails(vis->green_mask, &G_rshift, &G_lshift);
>-----------------------------------------^
>cc: Warning: gplt_x11.c, line 2437: In this statement, the referenced type of the pointer value "&B_rshift" is "short", which is not compatible with "unsigned short".
> B_msb_mask = BitMaskDetails(vis->blue_mask, &B_rshift, &B_lshift);
>-----------------------------------------^
>cc: Warning: gplt_x11.c, line 2437: In this statement, the referenced type of the pointer value "&B_lshift" is "short", which is not compatible with "unsigned short".
> B_msb_mask = BitMaskDetails(vis->blue_mask, &B_rshift, &B_lshift);
>-----------------------------------------^
>
The above warnings I can fix by just declaring chars and
uchars correctly. I can get to it this evening. But the
graphics card details you printed below look reasonable.
>Attached are some screen shots from the X11 output.
>Tux starts out looking a bit ill, and gets worse from there.
>
>Terminal output during the first demo (tux_1) is
> vis->visualid: 0x23 vis->class: 4 vis->bits_per_rgb: 8
> vis->red_mask: ff0000 vis->green_mask: ff00 vis->blue_mask: ff
> ImageByteOrder: 0
>
There certainly is a problem with the graphics mapping.
(The sort of thing seen before.) It looks like the usual
masks, but there seems to be a lot of variation just exactly
what bits within that mask do what. And the mangled
image is not good. (Not seen before.) I can only assume
that has to do with the decoding of data. These may all
have something to do with the signed/unsigned warnings
above.
The blue channel seems to come out correctly. No
surprise since that is the one with the mask associate
with the rightmost bits.
>The PNG output looks OK, so I think this part of it is due to problems in
>gplt_x11.c
>
Yes, definitely.
>However, the program then dies on a floating exception from the
>1st binary data demo, right after printing:
>
Not so good. But these usually aren't so hard to
track down. Gnuplot crashes for PNG as well I assume.
That particular example is a rather innocuous one. I wonder
what happened. Could you please try a few things in
image.dem to isolate the problem just a bit more? Try
inputting the lines one at a time. Try skipping that example.
Etc.
> The following binary data sizes are machine dependent:
>
> name (size in bytes)
>
> "char" "schar" "c" (1)
> "uchar" (1)
> "short" (2)
> "ushort" (2)
> "int" "sint" "i" "d" (4)
> "uint" "u" (4)
> "long" "ld" (8)
> "ulong" "lu" (8)
> "float" "f" (4)
> "double" "lf" (8)
>
> The following binary data sizes attempt to be machine independent:
>
> name (size in bytes)
>
> "int8" "byte" (1)
> "uint8" "ubyte" (1)
> "int16" "word" (2)
> "uint16" "uword" (2)
> "int32" (4)
> "uint32" (4)
> "float32" (4)
> "float64" (8)
>
Well hey! This is very nice. This is probably the first case
where the doubles, longs and ulongs are 8 bytes. Also, notice
that the "machine independent" code did what it is supposed
to do. However, it looks like an "int64" and "uint64" are needed.
>I am not sure yet whether this is due to the error message from
>datafile.c, or whether it is something more fundamental.
>
At first glance, it doesn't look like that command would
be the problem. That is part of the PDP endian, which
I suspect isn't being reached in this case.
Let me think these over.
Thanks,
Dan
>
> ------------------------------------------------------------------------
>
>
> ------------------------------------------------------------------------
>
>
> ------------------------------------------------------------------------
>
>
> ------------------------------------------------------------------------
>
--
Dan Sebald
email: daniel . sebald @ ie ee . o rg
URL: ht tp://acer-access.c om/~dsebald @ acer-access.c om/
|
|
From: Daniel J S. <dan...@ie...> - 2003-11-15 04:13:18
|
Ethan Merritt wrote:
>On Friday 14 November 2003 13:31, Ethan Merritt wrote:
>
>
>>On Friday 14 November 2003 13:36, you wrote:
>>
>>
>>>That crash you are describing with the
>>>swap bytes, it may be that that is not a problem
>>>with the code, but perhaps the fact that the
>>>internal floating point code for DEC cc can't
>>>handle bogus floating point numbers.
>>>
>>>
>>But yes, there are various compiler options
>>to treat floating point errors differently.
>>I'll try again with a different set of floating
>>point settings.
>>
>>
>
>OK. l've rebuilt everything using IEEE floating
>point rather than the alpha native floating point.
>The crashes go away, and the postscript output
>is rather more informative (attached).
>
Glad that is accounted for. Interesting problem I didn't anticipate.
>Note in particular that the test plot labeled
>"Closeup of pixels having grid points ...."
>is missing the lower two boxes. That must mean
>something.... if we only recognized what.
>
The data is still not correctly ASCII85 encoded. That example
with the close up works because with so little data it happens that
the bad coding ends up being valid ASCII85 characters still. Of
course, the image is incorrect.
I think I see what the issue is. It is a bug. The ASCII85 encoding
uses 4 bytes as the input to its mapping. That is 32 bits. It is a
series of shift operations in which a 'long' is filled with bits by shifting
left. When the long is 4 bytes, successive codings will simply shift
the old value out past the length of the long. But when a long is
8 bytes, the old value is shifted into the more significant bytes of the
variable which, consequently, makes a mess of the coding. I will update
this on SourceForge, but if you want a quick fix, search in `post.trm'
for the first use of PS_encode85() and put "tuple4 = 0;" after it. I.e.,
PS_encode85(tuple4, tuple5);
tuple4 = 0;
This will clear the tuple4 value so that garbage isn't shifted upward.
>If you really require some particular floating
>point behavior then you need to figure out some
>way of testing for this and at least issue a warning
>when ./configure is run.
>
I'm going to cede to suggestions from you and the list on this one.
My experience with the innards of floating point numbers in
compilers is minimal. I just assumed that any binary floating
point numbers read in will translate to some valid number and the
user would recognize that plotted results are invalid. I hadn't
anticipated the possibility of a program crash.
That leaves the shredded X11 images problem unresolved. I'll
keep thinking.
Thanks,
Dan
|