Menu
▾
▴
gnuplot-beta
|
From: <mi...@ph...> - 2004-08-16 08:50:38
|
There is a bug in the color palette treatment in the X11 terminal: when using multiple X11 terminals, window redraw (requested e.g. by a window manager) will change its palette. Try this script: set pm3d map set term x11 10 set title '10 gray levels' set palette gray set palette maxcolors 10 splot x*x set term x11 2 set title '2 colors' set palette color set palette maxcolors 2 splot x Now, maximize or resize window #10 by mouse => it will change from gray map with 10 gray levels to color map with 2 colors. Is it possible to fix it? --- PM |
|
From: Ethan M. <merritt@u.washington.edu> - 2004-08-16 19:31:49
|
On Monday 16 August 2004 01:50 am, Petr Mikulik <mi...@ph...> wrote: > There is a bug in the color palette treatment in the X11 terminal: when > using multiple X11 terminals, window redraw (requested e.g. by a window > manager) will change its palette. > > Is it possible to fix it? I am not very familiar with this, and I may have it wrong. But It looks to me that the PM3D code would have to be re-written. As it is, gnuplot_x11 only maintains two GC (= "Graphics Context") structures. One of these is used for PM3D operations, and the other is used for everything else. There is not currently any notion of keeping a separate GC for each plot window. So if you redefine the PM3D GC to use a different palette, it affects all PM3D operations in all windows. I imagine it would be possible to create a new GC for each window, but I am not sure what pitfalls that would expose. I believe that Johannes Zellner wrote this code, so maybe he has some insight into the issue. Otherwise, maintaining per-window information is related to code that Daniel Sebald added recently, so he might be interested in experimenting with this. -- Ethan A Merritt merritt@u.washington.edu Biomolecular Structure Center Mailstop 357742 University of Washington, Seattle, WA 98195 |
|
From: Dave D. <dde...@es...> - 2004-08-16 19:56:18
|
Ethan Merritt <merritt@u.washington.edu> writes:
> On Monday 16 August 2004 01:50 am, Petr Mikulik <mi...@ph...> wrote:
>> There is a bug in the color palette treatment in the X11 terminal: when
>> using multiple X11 terminals, window redraw (requested e.g. by a window
>> manager) will change its palette.
>>
>> Is it possible to fix it?
>
> I am not very familiar with this, and I may have it wrong.
> But It looks to me that the PM3D code would have to be re-written.
>
> As it is, gnuplot_x11 only maintains two GC (= "Graphics Context")
> structures. One of these is used for PM3D operations, and the other
> is used for everything else. There is not currently any notion of
> keeping a separate GC for each plot window. So if you redefine
> the PM3D GC to use a different palette, it affects all PM3D operations
> in all windows.
>
> I imagine it would be possible to create a new GC for each window,
> but I am not sure what pitfalls that would expose.
>
I had a quick look, and decided that the problem was that colors[] is
global, so that any change will affect all subsequent
repaints. Including the colors[] in the (gnuplot_x11) plot structure
may help.
I don't think the single GC is an issue - the values in the GC are
reset as required.
Eg in
/*-----------------------------------------------------------------------------
* display - display a stored plot
*---------------------------------------------------------------------------*/
void
display(plot)
plot_struct *plot;
{
we have
...
/* loop over accumulated commands from inboard driver */
for (n = 0; n < plot->ncommands; n++) {
...
/* X11_linetype(type) - set line type */
else if (*buffer == 'L') {
sscanf(buffer, "L%4d", <);
lt = (lt % 8) + 2;
/* default width is 0 {which X treats as 1} */
width = widths[lt] ? user_width * widths[lt] : user_width;
if (dashes[lt][0]) {
type = LineOnOffDash;
XSetDashes(dpy, gc, 0, dashes[lt], strlen(dashes[lt]));
} else {
type = LineSolid;
}
XSetForeground(dpy, gc, colors[lt + 3]);
XSetLineAttributes(dpy, gc, width, type, CapButt, JoinBevel);
}
which is updating the Gc foreground colour, but from the global
colors[] array, which may have changed since the plot was created.
(I also decided that line widths and dashes could suffer from the same
problem, but couldn't immediately reproduce any problem
)
dd
--
Dave Denholm <dde...@es...> http://www.esmertec.com
|
|
From: <mi...@ph...> - 2004-08-25 06:55:06
|
I have found yet another bug of X11 terminal which may (or may not?) have a common base with the bug in $Subj. Please try this: set palette maxcolors 4 set pm3d set term x11 1 splot x set term x11 2 splot x => only the 1st x11 window shows the palette correctly, the following ignore 'set palette' options. It seems like x11 window structure is either not copying the palette information, or ignores it. --- Petr Mikulik |
|
From: Daniel J S. <dan...@ie...> - 2004-08-25 09:37:59
|
I believe that may be tied into the same problem area. I think I described something in that recursive code where removing one conditional statement would allow the palette to be stored and then reinstalled upon refreshing the graph. But the problem was that with that alteration the palette wasn't orignally coming out with the proper number of colors. The behavior seemed similar to this example. Dan mi...@ph... wrote: >I have found yet another bug of X11 terminal which may (or may not?) have >a common base with the bug in $Subj. Please try this: >set palette maxcolors 4 >set pm3d >set term x11 1 >splot x >set term x11 2 >splot x > >=> only the 1st x11 window shows the palette correctly, the following >ignore 'set palette' options. It seems like x11 window structure is either >not copying the palette information, or ignores it. >--- >Petr Mikulik > > > > > >------------------------------------------------------- >SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media >100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 >Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift. >http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285 >_______________________________________________ >gnuplot-beta mailing list >gnu...@li... >https://lists.sourceforge.net/lists/listinfo/gnuplot-beta > > > > -- Dan Sebald email: daniel DOT sebald AT ieee DOT org URL: http://acer-access DOT com/~dsebald AT acer-access DOT com/ |
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 20:00:16
|
Ethan Merritt wrote: >On Monday 16 August 2004 01:50 am, Petr Mikulik <mi...@ph...> wrote: > > >>There is a bug in the color palette treatment in the X11 terminal: when >>using multiple X11 terminals, window redraw (requested e.g. by a window >>manager) will change its palette. >> >>Is it possible to fix it? >> >> > >I am not very familiar with this, and I may have it wrong. >But It looks to me that the PM3D code would have to be re-written. > >As it is, gnuplot_x11 only maintains two GC (= "Graphics Context") >structures. One of these is used for PM3D operations, and the other >is used for everything else. There is not currently any notion of >keeping a separate GC for each plot window. So if you redefine >the PM3D GC to use a different palette, it affects all PM3D operations >in all windows. > >I imagine it would be possible to create a new GC for each window, >but I am not sure what pitfalls that would expose. > >I believe that Johannes Zellner wrote this code, so maybe he has >some insight into the issue. > >Otherwise, maintaining per-window information is related to code >that Daniel Sebald added recently, so he might be interested in >experimenting with this. > I didn't look at the code, but as soon as I tried Petr's example it struck me that what Ethan's describing is the case. I'll have a look this evening to assess how much it would involve. But it seems logical that since an X11 plot keeps all information about the plotting primitives so that it can be redrawn, it should also keep track of the palette. If things are organized as I suspect, that palette should be a separate entity in the plot structure, not go into the list of graphics primitives. That's easy enough to add. Then, when reactivated, just install the palette. What this means, I think, is that when you issue a palette command to a certain X11 plot, the graphics primitives can still be used to draw under that new palette. The X11 code could certainly be redrawn upon getting that palette command. Do you want that? I.e., for X11 do you want to have to have to do "set palette; replot"? Or should the "set palette" change the X11 plot right away? Note, that for non-GUI terminals you would always have to do a "set palette; replot" to affect a change. Dan |
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 20:07:34
|
Daniel J Sebald wrote: > What this means, I think, is that when you issue a palette command to > a certain X11 plot, the graphics primitives can still be used to draw > under that new palette. The X11 code could certainly be redrawn upon > getting that palette command. Do you want that? I.e., for X11 do you > want to have to have to do "set palette; replot"? Or should the "set > palette" change the X11 plot right away? Note, that for non-GUI > terminals you would always have to do a "set palette; replot" to > affect a change. Hold on. There may be a problem with that scheme. We're not sure the primitives in the plot memory match the same size of the new palette installed. "set palette; replot" must be done to affect a change in the plot appearance. Yes? No? Dan |
|
From: <mi...@ph...> - 2004-08-17 06:50:48
|
> "set palette; replot" must be done to affect a change in > the plot appearance. Yes, it should work this way. -- PM |
|
From: Ethan M. <merritt@u.washington.edu> - 2004-08-17 22:46:18
|
On Monday 16 August 2004 11:47 pm, mi...@ph... wrote: > > "set palette; replot" must be done to affect a change in > > the plot appearance. > > Yes, it should work this way. I don't understand what you guys are saying. You cannot do a "replot" on a previous plot, only on the current plot. -- Ethan A Merritt merritt@u.washington.edu Biomolecular Structure Center Mailstop 357742 University of Washington, Seattle, WA 98195 |
|
From: Daniel J S. <dan...@ie...> - 2004-08-17 18:39:04
|
Ethan Merritt wrote: >On Monday 16 August 2004 11:47 pm, mi...@ph... wrote: > > >>>"set palette; replot" must be done to affect a change in >>>the plot appearance. >>> >>> >>Yes, it should work this way. >> >> > >I don't understand what you guys are saying. >You cannot do a "replot" on a previous plot, only on the current plot. > Right, I think it is already set up that way. Dan |
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 20:24:30
|
Dave,
If you want to look into this one, let me know. (Otherwise I can do it
one of these evenings.) I can picture in my mind how to do it. A
palette pointer needs to be placed in the plot array. There is going to
be some alloc() and realloc() for that pointer and the palette when it
is changed, or in things like prepare_plot(). The one careful detail is
immediately upon creating the memory for the plot structure itself as
part of the linked-list of plots, be sure to set the pointer for the
palette to 0 (!!! much emphasis). That could also be done in
prepare_plot() but I advise putting it at the spot where memory is
created, because doing it in prepare_plot() leaves open the door for
someone to inadvertently call prepare_plot() and mash a valid palette
pointer, creating a memory leak.
Dan
Dave Denholm wrote:
>Ethan Merritt <merritt@u.washington.edu> writes:
>
>
>
>>On Monday 16 August 2004 01:50 am, Petr Mikulik <mi...@ph...> wrote:
>>
>>
>>>There is a bug in the color palette treatment in the X11 terminal: when
>>>using multiple X11 terminals, window redraw (requested e.g. by a window
>>>manager) will change its palette.
>>>
>>>Is it possible to fix it?
>>>
>>>
>>I am not very familiar with this, and I may have it wrong.
>>But It looks to me that the PM3D code would have to be re-written.
>>
>>As it is, gnuplot_x11 only maintains two GC (= "Graphics Context")
>>structures. One of these is used for PM3D operations, and the other
>>is used for everything else. There is not currently any notion of
>>keeping a separate GC for each plot window. So if you redefine
>>the PM3D GC to use a different palette, it affects all PM3D operations
>>in all windows.
>>
>>I imagine it would be possible to create a new GC for each window,
>>but I am not sure what pitfalls that would expose.
>>
>>
>>
>
>
>I had a quick look, and decided that the problem was that colors[] is
>global, so that any change will affect all subsequent
>repaints. Including the colors[] in the (gnuplot_x11) plot structure
>may help.
>
>I don't think the single GC is an issue - the values in the GC are
>reset as required.
>
>
>Eg in
>
>/*-----------------------------------------------------------------------------
> * display - display a stored plot
> *---------------------------------------------------------------------------*/
>
>void
>display(plot)
>plot_struct *plot;
>{
>
>we have
>
>...
>
> /* loop over accumulated commands from inboard driver */
> for (n = 0; n < plot->ncommands; n++) {
>
> ...
>
> /* X11_linetype(type) - set line type */
> else if (*buffer == 'L') {
> sscanf(buffer, "L%4d", <);
> lt = (lt % 8) + 2;
> /* default width is 0 {which X treats as 1} */
> width = widths[lt] ? user_width * widths[lt] : user_width;
> if (dashes[lt][0]) {
> type = LineOnOffDash;
> XSetDashes(dpy, gc, 0, dashes[lt], strlen(dashes[lt]));
> } else {
> type = LineSolid;
> }
> XSetForeground(dpy, gc, colors[lt + 3]);
> XSetLineAttributes(dpy, gc, width, type, CapButt, JoinBevel);
> }
>
>
>which is updating the Gc foreground colour, but from the global
>colors[] array, which may have changed since the plot was created.
>
>(I also decided that line widths and dashes could suffer from the same
> problem, but couldn't immediately reproduce any problem
>)
>
>
>
>dd
>
>
--
Dan Sebald
email: daniel DOT sebald AT ieee DOT org
URL: http://acer-access DOT com/~dsebald AT acer-access DOT com/
|
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 20:28:50
|
And when the plot structure memory is freed, must first test whether the palette pointer is valid and if so free that as well. Dan Daniel J Sebald wrote: > Dave, > > If you want to look into this one, let me know. (Otherwise I can do > it one of these evenings.) I can picture in my mind how to do it. A > palette pointer needs to be placed in the plot array. There is going > to be some alloc() and realloc() for that pointer and the palette when > it is changed, or in things like prepare_plot(). The one careful > detail is immediately upon creating the memory for the plot structure > itself as part of the linked-list of plots, be sure to set the pointer > for the palette to 0 (!!! much emphasis). That could also be done in > prepare_plot() but I advise putting it at the spot where memory is > created, because doing it in prepare_plot() leaves open the door for > someone to inadvertently call prepare_plot() and mash a valid palette > pointer, creating a memory leak. > > Dan |
|
From: Dave D. <dde...@es...> - 2004-08-16 20:42:35
|
Daniel J Sebald <dan...@ie...> writes: > Dave, > > If you want to look into this one, let me know. (Otherwise I can do > it one of these evenings.) I can picture in my mind how to do it. A Sorry, I'm really out of touch with what it's *supposed* to do :-( (Come to think of it - I may not even have the latest sources, in which case what I just wrote may have been rubbish) dd -- Dave Denholm <dde...@es...> http://www.esmertec.com |
|
From: Ethan M. <merritt@u.washington.edu> - 2004-08-16 21:08:39
|
On Monday 16 August 2004 01:42 pm, Dave Denholm wrote:
>
> (Come to think of it - I may not even have the latest sources, in
> which case what I just wrote may have been rubbish)
The particular code bit you posted applies to line colors, not
to pm3d rectangles. But there is a similar bit of code in
PaletteSetColor(plot_struct * plot, double gray) that applies
to pm3d:
index = gray * (plot->cmap->allocated - 1);
if (index >= plot->cmap->allocated)
index = plot->cmap->allocated -1;
XSetForeground(dpy, gc_pm3d, plot->cmap->pixels[index]);
I don't really have a grasp of the big picture.
I *think* the problem is that plot->cmap itself is still only
a list of color indices, and since the time it was first set up the
true colormap stored in the gc has changed. So the above
code updates the index, but it is an index into an obsolete table.
That's why I thought we might have to store a copy of the gc
in use at the time the original color mapping was set up.
Maybe.
--
Ethan A Merritt merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195
|
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 21:49:18
|
Ethan Merritt wrote: >On Monday 16 August 2004 01:42 pm, Dave Denholm wrote: > > >>(Come to think of it - I may not even have the latest sources, in >> which case what I just wrote may have been rubbish) >> <snip> >That's why I thought we might have to store a copy of the gc >in use at the time the original color mapping was set up. > > I'll give that a try this evening. I know exactly where the memory pointer initialization and alloc's should go. Dan |
|
From: Daniel J S. <dan...@ie...> - 2004-08-16 22:07:48
|
Looking through some code, I see the following line
df_column = (df_column_struct *) gp_realloc(df_column,
(df_max_cols += (df_max_cols < 20 ? 20 :
df_max_cols)) * sizeof(df_column_struct),
"datafile column");
Does anyone else find this suspicious? Particularly the part where
df_max_cols is changed *before* the actual allocation takes place.
Here is a far fetched scenario that might cause a problem. Say someone
runs gnuplot and has a bunch of other programs running. The person
tries a plot and it fails to allocate memory because there is no room.
Gnuplot gives an error in the gp_realloc() function. However,
df_max_cols has already been updated. The person then thinks to exit
some program to free memory, runs his or her plot command and it
crashes. The reason being, as I'm thinking, df_max_cols was adjusted
previously and the limit test thinks there is enough room for
df_max_cols when in fact that memory couldn't be allocated last time the
plot command was attempted.
There may be other similar instances of this and I'd say hold off
addressing it for a while.
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2004-08-17 23:10:08
Attachments:
junk
|
Ethan Merritt wrote:
>On Monday 16 August 2004 01:42 pm, Dave Denholm wrote:
>
>
>>(Come to think of it - I may not even have the latest sources, in
>> which case what I just wrote may have been rubbish)
>>
>>
>
>The particular code bit you posted applies to line colors, not
>to pm3d rectangles. But there is a similar bit of code in
>PaletteSetColor(plot_struct * plot, double gray) that applies
>to pm3d:
>
> index = gray * (plot->cmap->allocated - 1);
> if (index >= plot->cmap->allocated)
> index = plot->cmap->allocated -1;
> XSetForeground(dpy, gc_pm3d, plot->cmap->pixels[index]);
>
>I don't really have a grasp of the big picture.
>I *think* the problem is that plot->cmap itself is still only
>a list of color indices, and since the time it was first set up the
>true colormap stored in the gc has changed. So the above
>code updates the index, but it is an index into an obsolete table.
>That's why I thought we might have to store a copy of the gc
>in use at the time the original color mapping was set up.
>
I believe this is correct. I looked at the array of GC values, it
doesn't look to have anything to do with the color map.
In
process_configure_notify_event(XEvent *event)
is where the mouse event comes for resizing the window. Now, I'm pretty
sure that a problem here is that in this routine is
plot = Find_Plot_In_Linked_List_By_Window(event->xconfigure.window);
near the top and if there is a change in the window size and a few other
things, eventually there is a
display(plot);
So, nothing in that code looks as though a window color scheme was
activated or reinstalled between those two commands. My thinking is
that the fix should be as simple as that attached diff file. But there
is that "small" detail that Ethan describes.
The complete color map needs to be stored as part of the structure.
That is, plot->cmap needs to point at something that is dynamically
allocated independent of the current color map. I'll work on that fix,
but if this doesn't sound correct, let me know. Also, maybe think if
what I've added in this patch is adequate to fix the problem when I have
the plot->cmap problem fixed. That is, are there any other details
about the GC that also must be updated?
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2004-08-17 22:58:05
Attachments:
partialfix
|
Is joze out there?
> The complete color map needs to be stored as part of the structure.
> That is, plot->cmap needs to point at something that is dynamically
> allocated independent of the current color map. I'll work on that
> fix, but if this doesn't sound correct, let me know. Also, maybe
> think if what I've added in this patch is adequate to fix the problem
> when I have the plot->cmap problem fixed. That is, are there any
> other details about the GC that also must be updated?
I started on this one and soon realized that this code is already
present. (The default "cmap" is used similar to zero when doing dynamic
memory allocation.) So, I tracked down this problem to a line of code.
The small change in the attached diff will solve the problem Petr
found, but it also creates another.
Here is the situation. That update of plot->cmap to point to the proper
colormap is done inside PaletteMake. Here is the block of code:
if (plot->cmap->allocated < min_colors && !recursion) {
ReleaseColormap(plot);
/* create a private colormap. */
fprintf(stderr, "switching to private colormap\n");
plot->cmap = (cmap_t *) malloc(sizeof(cmap_t));
assert(plot->cmap);
CmapClear(plot->cmap);
plot->cmap->colormap = XCreateColormap(dpy, root, vis, AllocNone);
assert(plot->cmap->colormap);
pr_color(plot->cmap); /* set default colors for lines */
RecolorWindow(plot);
recursion = 1;
PaletteMake(plot, (t_sm_palette *) 0);
} else {
This allocates memory for the map and then sets plot->cmap to point to
it. If all things are hunky dory then, whenever plot->cmap is refered
to in the future plot->cmap will point to the proper colormap.
However, that test at the start:
plot->cmap->allocated < min_colors
I think that is the problem. The first time in, the min_colors will be
2 or 10 in one of the examples Petr gives. However, the default cmap,
what plot->cmap points to by default has more than 10 allocated values.
Hence, that test never passes and the code which dynamically allocates
the cmap never gets called.
So, remove that test as I did in the diff file. That fixes the problem
of the palette being restored to the proper palette. Unfortunately,
it's not creating the proper palette in the first place. (I has far
more than the 2 or 10 colors requested.)
I could look at it further, but the recursive nature of this routine
causes me to initially punt to joze.
Dan
|
|
From: Daniel J S. <dan...@ie...> - 2004-08-18 07:35:19
|
Some observations about demos:
borders.dem isn't very useful. Too fast to see what is on the page.
Plus requires CNTRL-C to exit.
The following demos are not in all.dem:
borders.dem
candlesticks.dem
charset.dem
enhancedtext.dem
multiplt.dem (such a nice demo)
starmap.dem (another nice demo)
vector.dem (another nice demo)
mousevariables.dem, fontfile.dem and fontfile_latex.dem aren't in
all.dem, but on purpose I think. Not finding the font files will cause
gnuplot to error out. Maybe they could be put at the very end of
all.dem so that if they do crash, nothing will be missed. (Include them
might be good because "all.dem" is a way of testing that nothing is
broke in gnuplot.)
|
|
From: <mi...@ph...> - 2004-08-19 06:12:09
|
> The following demos are not in all.dem: > borders.dem > candlesticks.dem > charset.dem > enhancedtext.dem > multiplt.dem (such a nice demo) > starmap.dem (another nice demo) > vector.dem (another nice demo) > > mousevariables.dem, fontfile.dem and fontfile_latex.dem aren't in > all.dem, but on purpose I think. Not finding the font files will cause > gnuplot to error out. Maybe they could be put at the very end of > all.dem so that if they do crash, nothing will be missed. (Include > them might be good because "all.dem" is a way of testing that nothing > is broke in gnuplot.) There could be files like all_tex.dem all_enhanced.dem which would load those demos which are not accessible from all.dem. --- PM |
|
From: Hans-Bernhard B. <br...@ph...> - 2004-08-19 08:35:54
|
mi...@ph... wrote:
> There could be files like
> all_tex.dem
> all_enhanced.dem
>
> which would load those demos which are not accessible from all.dem.
Nah, that'd be a placebo solution at best. What we really need is some
kind of feature availability test function exported to the user, so
all.dem could do
if (supported("enhanced")) load 'enhancedtext.dem'
(Syntax details to be discussed)
|
|
From: Petr M. <mi...@ph...> - 2004-09-17 13:20:40
|
> The complete color map needs to be stored as part of the structure. > That is, plot->cmap needs to point at something that is dynamically > allocated independent of the current color map. I'll work on that fix, > but if this doesn't sound correct, let me know. Also, maybe think if > what I've added in this patch is adequate to fix the problem when I have > the plot->cmap problem fixed. That is, are there any other details > about the GC that also must be updated? I have just tried the patch, but it was rejected. Don't you have an up-to-date version? Petr |
|
From: Daniel J S. <dan...@ie...> - 2004-09-17 15:05:28
|
Petr Mikulik wrote:
>>The complete color map needs to be stored as part of the structure.
>> That is, plot->cmap needs to point at something that is dynamically
>>allocated independent of the current color map. I'll work on that fix,
>>but if this doesn't sound correct, let me know. Also, maybe think if
>>what I've added in this patch is adequate to fix the problem when I have
>>the plot->cmap problem fixed. That is, are there any other details
>>about the GC that also must be updated?
>>
>>
>
>I have just tried the patch, but it was rejected. Don't you have an
>up-to-date version?
>
Why would that patch be rejected? I don't think anyone has altered that
bit of code in CVS... Anyway, that patch was just a lame alteration to
illustrate where the problem lies. Basically, search for this line of
code in gplt_x11.c
if (plot->cmap->allocated < min_colors && !recursion) {
and replace it with the following
if (!recursion) {
That fixes the problem of keeping track of the color map, but it also
results in an incorrect color map. It looks like a tricky bit of code
because it is recursive. I'm not exactly sure why it is recursive, as
opposed to calling a subroutine twice. In fact, I'm looking at some code
there and wondering what in fact it does. Here is the code in question:
if (plot->cmap->allocated < min_colors && !recursion) {
ReleaseColormap(plot);
/* create a private colormap. */
fprintf(stderr, "switching to private colormap\n");
plot->cmap = (cmap_t *) malloc(sizeof(cmap_t));
assert(plot->cmap);
CmapClear(plot->cmap);
plot->cmap->colormap = XCreateColormap(dpy, root, vis, AllocNone);
assert(plot->cmap->colormap);
pr_color(plot->cmap); /* set default colors for lines */
RecolorWindow(plot);
recursion = 1;
PaletteMake(plot, (t_sm_palette *) 0);
} else {
/* this is just for calculating the number of unique colors */
int i;
unsigned long previous = plot->cmap->allocated ? plot->cmap->pixels[0] : 0;
int unique_colors = 1;
for (i = 0; i < plot->cmap->allocated; i++) {
if (plot->cmap->pixels[i] != previous) {
previous = plot->cmap->pixels[i];
unique_colors++;
}
}
}
Notice that the second portion of the if/else statement alters two local
variables, previous and unique_colors, and that is all. That's a useless
bit of code, as I see it. I wish there were a short note explaining why
this is recursive, i.e., on the second time through, what portion of the
code is important?
Dan
|
|
From: Petr M. <mi...@ph...> - 2004-09-20 09:31:23
|
> >I have just tried the patch, but it was rejected. Don't you have an
> >up-to-date version?
>
> Why would that patch be rejected? I don't think anyone has altered that
> bit of code in CVS...
In the cvs current version, there is no that piece of code to be patched.
Haven't you use some old gplt_x11.c?
> if (plot->cmap->allocated < min_colors && !recursion) {
> and replace it with the following
> if (!recursion) {
But that's completely different from the patch you have sent:
XFreePixmap(dpy, plot->pixmap);
plot->pixmap = None;
}
+ if (plot != current_plot)
+ RecolorWindow(plot);
display(plot);
+ if (plot != current_plot)
+ RecolorWindow(current_plot);
}
}
Thus, what's the patch?
> That fixes the problem of keeping track of the color map, but it also
> results in an incorrect color map. It looks like a tricky bit of code
> because it is recursive. I'm not exactly sure why it is recursive, as
> opposed to calling a subroutine twice. In fact, I'm looking at some code
> there and wondering what in fact it does. Here is the code in question:
>
> Notice that the second portion of the if/else statement alters two local
> variables, previous and unique_colors, and that is all. That's a useless
> bit of code, as I see it. I wish there were a short note explaining why
> this is recursive, i.e., on the second time through, what portion of the
> code is important?
I remember from the old times when Johannes was coding "Make Palette"
function for X11 (term->makepalette(NULL)), that number of available colors
is not known under some visual modes. Then, you have to try to allocate
color palette several times until you find the limit. Can this explain the
recursion?
However, I wonder why this happens -- once the palette is constructed
(according to t_sm_palette) and copied into plot->cmap, it should be used in
every replot of that window.
Petr
|
|
From: Daniel J S. <dan...@ie...> - 2004-09-20 22:34:08
|
Petr Mikulik wrote:
>But that's completely different from the patch you have sent:
> XFreePixmap(dpy, plot->pixmap);
> plot->pixmap = None;
> }
>+ if (plot != current_plot)
>+ RecolorWindow(plot);
> display(plot);
>+ if (plot != current_plot)
>+ RecolorWindow(current_plot);
> }
> }
>
>Thus, what's the patch?
>
Oh, that one at first escaped me. But looking back in the emails, I
think that was the first attempt to make the link list contain
information about palettes. But soon after that I realized that such
code already exists. So the above patch you are referring to was
jettisoned.
>>That fixes the problem of keeping track of the color map, but it also
>>results in an incorrect color map. It looks like a tricky bit of code
>>because it is recursive. I'm not exactly sure why it is recursive, as
>>opposed to calling a subroutine twice. In fact, I'm looking at some code
>>there and wondering what in fact it does. Here is the code in question:
>>
>>Notice that the second portion of the if/else statement alters two local
>>variables, previous and unique_colors, and that is all. That's a useless
>>bit of code, as I see it. I wish there were a short note explaining why
>>this is recursive, i.e., on the second time through, what portion of the
>>code is important?
>>
>>
>
>I remember from the old times when Johannes was coding "Make Palette"
>function for X11 (term->makepalette(NULL)), that number of available colors
>is not known under some visual modes. Then, you have to try to allocate
>color palette several times until you find the limit. Can this explain the
>recursion?
>
>However, I wonder why this happens -- once the palette is constructed
>(according to t_sm_palette) and copied into plot->cmap, it should be used in
>every replot of that window.
>
If I recall, I think I tracked down the flaw in program flow. Let me
look for that... here's a portion of what I wrote before:
=====
I think that is the problem. The first time in, the min_colors will be
2 or 10 in one of the examples Petr gives. However, the default cmap,
what plot->cmap points to by default has more than 10 allocated values.
Hence, that test never passes and the code which dynamically allocates
the cmap never gets called.
=====
Regardless of whether that is correct or not, I think I'm getting a
better view of the big picture here. The real flaw may not necessarily
be in PaletteMake(), although I think PaletteMake() needs a good going
over to weed out cruft. Well, I shouldn't say that, because I think
this problem could be fixed in multiple ways.
The problem may be the following. When a plot window is first created,
its "plot->cmap" is set equal to the default "cmap". I propose that
rather than just pointing to the default colormap, the colomap should be
*duplicated* in memory. The reason is that PaletteMake() doesn't
necessarily create a new version of color map unless those strange
conditions (which I don't fully understand) are met, i.e.,
if (plot->cmap->allocated < min_colors && !recursion) {
ReleaseColormap(plot);
/* create a private colormap. */
fprintf(stderr, "switching to private colormap\n");
plot->cmap = (cmap_t *) malloc(sizeof(cmap_t));
assert(plot->cmap);
CmapClear(plot->cmap);
plot->cmap->colormap = XCreateColormap(dpy, root, vis, AllocNone);
assert(plot->cmap->colormap);
pr_color(plot->cmap); /* set default colors for lines */
RecolorWindow(plot);
recursion = 1;
PaletteMake(plot, (t_sm_palette *) tpal);
} else {
The above is the only place in PaletteMake() that plot->cmap is
malloced. That means that if that condition above is not met, which is
almost certain to be the case, then the rest of PaletteMake() must act
upon the "plot->cmap".
So, say two plots are created and by default they are both set to
"plot->cmap = cmap". Then whenever PaletteMake() is run on the colormap
for one of the plots, it is effectively changing the color map for the
other. So, that is why I'm saying that somewhere along the way upon the
creation of a plots color map, it must *duplicate* the default palette,
i.e., malloc an equal amount of memory as "cmap" and then use memcpy().
That is, if one wants color maps to be preserved in plots. I assume
that when the palette is changed at gnuplot's command line, that change
is supposed to apply to the default palette as well as the current
plot's palette.
Dan
|
