gnuplot-beta — Developers and beta testers of new gnuplot code

Re: proposal for cvs'ing 'with image'

[Daniel J S. <dan...@ie...>]

Harald Harders wrote:

>On Thu, 12 Aug 2004, Ethan Merritt wrote:
>
>>My issue with map3d_xy has nothing to do with rounding.
>>The problem is that it returns coordinate values as (unsigned int)
>>without checking for negative numbers.
>>See numerous bug reports about vector and arrow problems
>>whenever one end of a vector is outside of the plot area.
>>When I looked at this before I decided that fixing this properly
>>might require doing away with the routine altogether, and
>>having the callers check for out-of-bound endpoints before
>>converting input coordinates to (unsigned int) screen coordinates.
>>    
>>
>
>The problem with negative coordinate values is partly fixed with my
>relative coordinate patch (#991819). May be applying it could do half the
>way to fix that problem.
>  
>

I think this could be a top priority issue.  Once in a while I'll zoom 
and there will be lines going off into seemingly random directions. 
 That kind of thing is unseemly.  I could make the binary data file 
changes Ethan suggested and have a patch ready by monday.  This could 
directly follow.

Dan

Re: proposal for cvs'ing "with image"

[Daniel J S. <dan...@ie...>]

Daniel J Sebald wrote:

>
>
> Ethan Merritt wrote:
>
>> On Thursday 12 August 2004 12:00 am, mi...@ph... wrote:
>>  
>>
>>> I propose the patch gets committed into cvs.
>>>   
>>
>>
>> I also find this comment in post.trm a little alarming:
>>
>> +    /*** FIX ME!!! *****
>> +     *
>> +     * I had to put this grestore command here, otherwise the
>> +     * case where multiple images are plotted on the same graph
>> +     * fails after the first image.  I have no idea what it does.
>> +     * I'm guessing it restores the stack or something and that
>> +     * it must be paired with gsave.  It seems to me there is a
>> +     * gsave in the palette routine that doesn't have a matching
>> +     * grestore.  Could that be the problem?
>> +     */
>> +    fputs("grestore\n", gpoutfile);
>>
>>  
>>
>
> Hey, that's one of those "that's the way it works" sort of things.  
> :-)  I noted what at the time probably seemed like the source of 
> problems. However, a year ago, and even now, if I explained to the 
> list I thought there was a grestore missing in some PostScript code 
> that seemed to work fine the way it was, the reaction would have been 
> "don't believe it". 


Sorry, my bad.  That message is just some bogus note from when I first 
did the code.  There *should* be a grestore there because some of the 
image commands alter the graphics state, probably.  The state was 
probably at the top of the stack so "grestore" without a matched "gsave" 
didn't make a difference.  I put a "gsave" near the top of the routine 
to ensure a match.

Dan

Re: proposal for cvs'ing "with image"

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>On Thursday 12 August 2004 12:00 am, mi...@ph... wrote:
>  
>
>>I propose the patch gets committed into cvs.
>>    
>>
>
>I also find this comment in post.trm a little alarming:
>
>+    /*** FIX ME!!! *****
>+     *
>+     * I had to put this grestore command here, otherwise the
>+     * case where multiple images are plotted on the same graph
>+     * fails after the first image.  I have no idea what it does.
>+     * I'm guessing it restores the stack or something and that
>+     * it must be paired with gsave.  It seems to me there is a
>+     * gsave in the palette routine that doesn't have a matching
>+     * grestore.  Could that be the problem?
>+     */
>+    fputs("grestore\n", gpoutfile);
>
>  
>

Hey, that's one of those "that's the way it works" sort of things.  :-)  
I noted what at the time probably seemed like the source of problems. 
 However, a year ago, and even now, if I explained to the list I thought 
there was a grestore missing in some PostScript code that seemed to work 
fine the way it was, the reaction would have been "don't believe it".

Dan

Re: proposal for cvs'ing "with image"

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>On Thursday 12 August 2004 12:00 am, mi...@ph... wrote:
>  
>
>>I propose the patch gets committed into cvs.
>>    
>>
>
>Question:
>
>The patch contains a new file gplt_x11.h
>Why is it marked
>
> Copyright 2000   Thomas Williams, Colin Kelley
>
>This seems very unlikely to be correct.
>

I did a block copy from some other header file when that was created and 
didn't think about it again.  Hans or someone probably updated all 
Copyright notices in all other files before the 4.0 release.  That would 
be my guess.

That new header is meant to make common some of the codes and parameters 
used on both sides of the gnuplot_x11 pipe.  (A start anyway.)

What should the notice be?  In fact, what should the notice be for this 
new "binary datafile.c" file?

Dan

Re: proposal for cvs'ing 'with image'

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>On Thursday 12 August 2004 02:01 pm, Daniel J Sebald wrote:
>  
>
>>1. Clean up the df_readline() routine for neatness.  If that means a 
>>couple subroutines, that's fine.  But even though that function is 
>>somewhat big, it's flow of logic is pretty straightforward.
>>    
>>
>
>Summarizing my comments of 10-July-2004:
>
>    After I apply the with-image patch, df_readline contains 653 lines of
>    code of which only 303 are shared by the two modes.  
>
>    The largest block of shared code is the section (lines 3362-3465)
>    checking for blank lines, skipped lines, and EOF.  Is this even relevant
>    to binary mode? 
>
>My view of it is that you basically have two separate routines that are
>uneasily co-existing in the same block of code.  I think there should be
>two routines:
>	df_readline      (existing code = 328 lines)
>	df_readbinary  (code path used by pixel/binary routines = ~450 lines)
>By my rough estimate there are at most about 200 lines of code that are
>common to both paths.  I haven't tried to estimate how much of this may
>move easily into a shared subroutine.
>  
>There are only 4 callers of df_readline anyway.  It is the callers 
>business to know whether a binary read or a formatted read is
>appropriate, and call one of the two alternative input routines.
>  
>

Well, breaking up the routine into modules is alright.  But I don't 
agree that a df_readline() and a df_readbinary() is a good idea.  That 
would require a global variable travelling across datafile.c to 
plot2d.c, etc.  Inside of plot2d.c is the following construct

    while ((j = df_readline(v, max_cols)) != DF_EOF) {

How does one easily reconfigure that to incorporate one of two possible 
routines and the global variable to know which is which (df_readline(), 
df_readbinary(), df_general_binary)?  And why is it important for 
plot2d.c to know that it is getting the data from an ASCII or a binary file?

Now, if inside datafile.c you would want something like

int
df_readline(double v[], int max)
{

    if (df_general_binary)
        return df_read_binary_line(v[], max);
    else
        return df_read_ascii_line(v[], max);
}

where df_read_ascii_line() is exactly the same as the current 
df_readline(), that's easy enough.  I would simply copy the existing 
code and separate things so that whatever is currently conditional to 
df_general_binary gets moved to one function and what is left in the 
other.  However, there is some code that would be replicated, but not 
too bad.  Sure, I'm fine with that.  That, in fact would weed out some 
conditionals in exchange for about ten to twenty lines of replicated 
code, so that the total net loss/gain is probably zero.  I opt for 
keeping static variables local to C files, unless well organized.

So, is everyone alright with changing df_readline() as above and calling 
the current df_readline() df_read_ascii_line()?


>  
>
>>>And please check one final time that the patched version of datafile.c
>>>does not inadvertantly back out other changes made since 4.0.
>>>      
>>>
>>How does one check that?
>>    
>>
>
>The changes to datafile.c are so extensive that it is very hard
>to see exactly what is going on.  Can you possibly rearrange
>all the brand new routines so that they are together at the end of the file?
>

Sure.  Easy enough.  Functions down to the bottom of the file... but not 
down below the tables of any of the trm files, right?  And new tables to 
the tops of files... at the bottom of existing tables, or at the top of 
existing tables?

>In fact, ideally I'd like to see a new file containing all the purely new
>routines and also containing the disentangled code from df_readline()
>that constitutes the code path for binary files only.  The original
>df_readline would stay in datafile.c    At that point the remaining
>changes to datafile.c will probably be small enough to understand,
>and much of your new code will be together in one place.
>

What do you want this new file called?  binfile.c?  databin.c?

So, this will be integrated into CVS then?

Dan

Re: proposal for cvs'ing "with image"

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 12:00 am, mi...@ph... wrote:
> I propose the patch gets committed into cvs.

I also find this comment in post.trm a little alarming:

+    /*** FIX ME!!! *****
+     *
+     * I had to put this grestore command here, otherwise the
+     * case where multiple images are plotted on the same graph
+     * fails after the first image.  I have no idea what it does.
+     * I'm guessing it restores the stack or something and that
+     * it must be paired with gsave.  It seems to me there is a
+     * gsave in the palette routine that doesn't have a matching
+     * grestore.  Could that be the problem?
+     */
+    fputs("grestore\n", gpoutfile);




-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: proposal for cvs'ing "with image"

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 12:00 am, mi...@ph... wrote:
> I propose the patch gets committed into cvs.

Question:

The patch contains a new file gplt_x11.h
Why is it marked

 Copyright 2000   Thomas Williams, Colin Kelley

This seems very unlikely to be correct.


-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: proposal for cvs'ing 'with image'

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 02:01 pm, Daniel J Sebald wrote:
> 1. Clean up the df_readline() routine for neatness.  If that means a 
> couple subroutines, that's fine.  But even though that function is 
> somewhat big, it's flow of logic is pretty straightforward.

Summarizing my comments of 10-July-2004:

    After I apply the with-image patch, df_readline contains 653 lines of
    code of which only 303 are shared by the two modes.  

    The largest block of shared code is the section (lines 3362-3465)
    checking for blank lines, skipped lines, and EOF.  Is this even relevant
    to binary mode? 

My view of it is that you basically have two separate routines that are
uneasily co-existing in the same block of code.  I think there should be
two routines:
	df_readline      (existing code = 328 lines)
	df_readbinary  (code path used by pixel/binary routines = ~450 lines)
By my rough estimate there are at most about 200 lines of code that are
common to both paths.  I haven't tried to estimate how much of this may
move easily into a shared subroutine.

There are only 4 callers of df_readline anyway.  It is the callers 
business to know whether a binary read or a formatted read is
appropriate, and call one of the two alternative input routines.


> >And please check one final time that the patched version of datafile.c
> >does not inadvertantly back out other changes made since 4.0.
> 
> How does one check that?

The changes to datafile.c are so extensive that it is very hard
to see exactly what is going on.  Can you possibly rearrange
all the brand new routines so that they are together at the end of the file?

In fact, ideally I'd like to see a new file containing all the purely new
routines and also containing the disentangled code from df_readline()
that constitutes the code path for binary files only.  The original
df_readline would stay in datafile.c    At that point the remaining
changes to datafile.c will probably be small enough to understand,
and much of your new code will be together in one place.

-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: proposal for cvs'ing 'with image'

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>>>I propose clean up of datafile.c, if you really find this necessary, after
>>>the patch is in cvs. The patch is quite big, and its maintenance as a
>>>patch needs more effort than necessary. Thus I propose to commit the patch
>>>ASAP.
>>>      
>>>
>
>OK, I guess.
>But please let's try to get df_readline() cleaned up as the very next thing.
>

What are the goals proposed for cleanup?  I'd offer up a few intial goals:

1. Clean up the df_readline() routine for neatness.  If that means a 
couple subroutines, that's fine.  But even though that function is 
somewhat big, it's flow of logic is pretty straightforward.

2. Integrate the "gnuplot binary" into the more general binary routine 
with a few lines of code.  Then drop the special routine for reading 
"gnuplot binary" and if I recall correctly there are a bunch of routines 
at the end of "datafile.c" that aren't used and could be discarded. 
 (Can always retrieve them from CVS.)

3. Decide a convention of how to properly incorporate info in a datafile 
into the plot style.  There was some discussion about histograms and 
other plot styles that assume something based upon what is in the data 
file, i.e., whether that sort of thing should be done, and if so how 
best to do it.

>And please check one final time that the patched version of datafile.c
>does not inadvertantly back out other changes made since 4.0.
>

How does one check that?  Compare a diff between current CVS and Gnuplot 
4.0 with the patch file?  Any mods to CVS not matching what the patch 
file has for the first few lines of code before and after the change 
will cause a reject.  I've never seen anything in the patched code lost 
from patching, only what is in the patch file gets rejected.  The worst 
I've seen is a hunk positioned out of order so that it doesn't compile 
correctly.  (Those can be nasty to debug if it's an openning or closing 
bracket mispositioned.)

Dan

Re: few remarks

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>On Thursday 12 August 2004 10:35 am, Daniel J Sebald wrote:
>  
>
>>Well, I looked at this.  The value that gnuplot_x11 was putting in for 
>>plot->lwidth is 1, not zero.  So that means the following property of 
>>CapNotLast should *not* apply.
>>    
>>
>
>Yes, but Dave's point was that the call used to look like this:
>	XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>
>so the linewidth was set to 0 no matter what the value of plot->lwidth.
>He also says that x11 apparently interprets this as a license to use
>whatever line style is fastest.  So if it thinks CapNotLast is faster, it
>will use that.
>

Oh, now I see what Dave was saying.  But still, CapButt with 
point->lwidth = 1 removes the last pixel.  That doesn't agree with the 
documentation.

Dan

Re: proposal for cvs'ing 'with image'

[Harald H. <h.h...@tu...>]

On Thu, 12 Aug 2004, Ethan Merritt wrote:

> On Thursday 12 August 2004 06:23 am, Daniel J Sebald wrote:
> >
> > I think there was this issue, and the issue about the way a map3d_xy()
> > routine rounds, especially for PostScript.  Ethan said he will be
> > looking at the routine in question some time in the future.
>
> My issue with map3d_xy has nothing to do with rounding.
> The problem is that it returns coordinate values as (unsigned int)
> without checking for negative numbers.
> See numerous bug reports about vector and arrow problems
> whenever one end of a vector is outside of the plot area.
> When I looked at this before I decided that fixing this properly
> might require doing away with the routine altogether, and
> having the callers check for out-of-bound endpoints before
> converting input coordinates to (unsigned int) screen coordinates.

The problem with negative coordinate values is partly fixed with my
relative coordinate patch (#991819). May be applying it could do half the
way to fix that problem.

By the way: Is there a reason that the group of patches to reach text
offsets (#991819, #993411, #993422) neither is applied to cvs nor anybody
writes which is wrong with them?  Do you think it is unimportant?

Yours
Harald

--=20
Harald Harders                           Langer Kamp 8
Technische Universit=E4t Braunschweig      D-38106 Braunschweig
Institut f=FCr Werkstoffe                  Germany
E-Mail: h.h...@tu...               Tel: +49 (5 31) 3 91-3062
WWW   : http://www.harald-harders.de     Fax: +49 (5 31) 3 91-3058

Re: few remarks

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 10:35 am, Daniel J Sebald wrote:
> Well, I looked at this.  The value that gnuplot_x11 was putting in for 
> plot->lwidth is 1, not zero.  So that means the following property of 
> CapNotLast should *not* apply.

Yes, but Dave's point was that the call used to look like this:
	XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);

so the linewidth was set to 0 no matter what the value of plot->lwidth.
He also says that x11 apparently interprets this as a license to use
whatever line style is fastest.  So if it thinks CapNotLast is faster, it
will use that.

If that is the correct analysis, then it is indeed a property of x11
itself but since it is documented that way I guess we don't get to
call it a bug.


-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: proposal for cvs'ing 'with image'

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 06:23 am, Daniel J Sebald wrote:
> 
> I think there was this issue, and the issue about the way a map3d_xy() 
> routine rounds, especially for PostScript.  Ethan said he will be 
> looking at the routine in question some time in the future.

My issue with map3d_xy has nothing to do with rounding.
The problem is that it returns coordinate values as (unsigned int)
without checking for negative numbers.
See numerous bug reports about vector and arrow problems
whenever one end of a vector is outside of the plot area.
When I looked at this before I decided that fixing this properly
might require doing away with the routine altogether, and 
having the callers check for out-of-bound endpoints before
converting input coordinates to (unsigned int) screen coordinates. 

> >I propose clean up of datafile.c, if you really find this necessary, after
> >the patch is in cvs. The patch is quite big, and its maintenance as a
> >patch needs more effort than necessary. Thus I propose to commit the patch
> >ASAP.

OK, I guess.
But please let's try to get df_readline() cleaned up as the very next thing.

And please check one final time that the patched version of datafile.c
does not inadvertantly back out other changes made since 4.0.

-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: few remarks

[Daniel J S. <dan...@ie...>]

Dave Denholm wrote:

>Ethan Merritt <merritt@u.washington.edu> writes:
>
>  
>
>>On Thursday 12 August 2004 03:44 am, Dave Denholm wrote:
>>    
>>
>>>		if (type != LineSolid || width != 0) {	/* select solid line */
>>>		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>>>		} 
>>>
>>>- try setting the linewidth param to 1 in the XSetLineAttributes call.
>>>      
>>>
>>Thanks for the tip.   The change I actually made was to unconditionally call
>>	
>>            /* Force line type to solid, with round ends */
>>            XSetLineAttributes(dpy, *current_gc, plot->lwidth, LineSolid, CapRound, JoinRound);
>>
>>I was under the impression that passing plot->lwidth was essentially a no-op,
>>since that should have been the current state anyhow.  I made it unconditional
>>in order to force the CapRound attribute.   But if I understand your summary of
>>X oddities, it may actually be the explicit linewidth rather than the CapRound
>>that causes the observed problem to go away.
>>
>>    
>>
>
>
>I don't think plot->lwidth is appropriate for drawing symbols - if
>you've got a very wide (gnuplot) line width, the outline of the points
>will be drawn with a wide line.
>
>But maybe that is what you want.
>

I think it's fine so long as independent control, as it currently 
exists, is there, i.e. that example

plot '-' w points lw 10  pt 1 ps 10
0 0
end

Dan

Re: few remarks

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:

>On Thursday 12 August 2004 03:44 am, Dave Denholm wrote:
>  
>
>>		if (type != LineSolid || width != 0) {	/* select solid line */
>>		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>>		} 
>>
>>- try setting the linewidth param to 1 in the XSetLineAttributes call.
>>    
>>
>
>Thanks for the tip.   The change I actually made was to unconditionally call
>	
>            /* Force line type to solid, with round ends */
>            XSetLineAttributes(dpy, *current_gc, plot->lwidth, LineSolid, CapRound, JoinRound);
>
>I was under the impression that passing plot->lwidth was essentially a no-op,
>since that should have been the current state anyhow.  I made it unconditional
>in order to force the CapRound attribute.   But if I understand your summary of
>X oddities, it may actually be the explicit linewidth rather than the CapRound
>that causes the observed problem to go away.
>
>Daniel, do you care enough to investigate further?
>I'm happy just to leave it as it is, since it seems to work.
>

Well, I looked at this.  The value that gnuplot_x11 was putting in for 
plot->lwidth is 1, not zero.  So that means the following property of 
CapNotLast should *not* apply.

CapNotLast This is equivalent to CapButt except that for a line-width of 
zero the final endpoint is not drawn.


Still, originally it was CapButt, which shouldn't have this "final 
endpoint is not drawn" property, anyway.  I also tried "XDrawLine" 
instead of the "XDrawSegments".  That worked the same way.  But I see 
that if one sets the "ps 2", it appears to fix the problem.  So again, 
I'm confused (very) and I agree with Ethan that there is a bug in the 
X11 software.  CapButt is not being interpretted according to 
documentation, and a the line width of one is treated as though it were 
line width zero.

Anyway, if you want to get an idea of what Ethan's change has done, try

plot '-' w points lw 10  pt 1 ps 10
0 0
end

And try it with "pt 2".  I'm fine with that rounding (although the X11 
routines don't seem to have the radius around the ends of lines just 
right), unless someone wants to argue the WYSIWYG principle that it 
should match the style the other terminals use.  With a line size of 1 
or 2, these effects are hardly noticable.

However, if the rounded line ends are not acceptable, another 
alternative to get around this bug would be the "CapProjecting" option. 
 That has square line ends that extend half line width past the end 
point.  It doesn't appear to have this problem that line width of 1 that 
is occuring with CapButt.  I've a feeling that most will prefer the 
CapProjecting option over CapRound because, as I've noticed, the rounded 
part seems a little askew for some reason.

Dan

Re: few remarks

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 09:34 am, Dave Denholm wrote:
> 
> I don't think plot->lwidth is appropriate for drawing symbols - if
> you've got a very wide (gnuplot) line width, the outline of the points
> will be drawn with a wide line.
> 
> But maybe that is what you want.

I would think the normal expectation if you set a default line width 
is that everything, including point symbols, will use this default width.

If you really want to mix linewidths in the linespoints style,
you can instead use 
	plot '<foo>' with lines lw <n>, '' with points lw 0

-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: few remarks

[Dave D. <dde...@es...>]

Ethan Merritt <merritt@u.washington.edu> writes:

> On Thursday 12 August 2004 03:44 am, Dave Denholm wrote:
>> 		if (type != LineSolid || width != 0) {	/* select solid line */
>> 		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>> 		} 
>> 
>> - try setting the linewidth param to 1 in the XSetLineAttributes call.
>
> Thanks for the tip.   The change I actually made was to unconditionally call
> 	
>             /* Force line type to solid, with round ends */
>             XSetLineAttributes(dpy, *current_gc, plot->lwidth, LineSolid, CapRound, JoinRound);
>
> I was under the impression that passing plot->lwidth was essentially a no-op,
> since that should have been the current state anyhow.  I made it unconditional
> in order to force the CapRound attribute.   But if I understand your summary of
> X oddities, it may actually be the explicit linewidth rather than the CapRound
> that causes the observed problem to go away.
>


I don't think plot->lwidth is appropriate for drawing symbols - if
you've got a very wide (gnuplot) line width, the outline of the points
will be drawn with a wide line.

But maybe that is what you want.



> Daniel, do you care enough to investigate further?
> I'm happy just to leave it as it is, since it seems to work. 
>
> -- 
> Ethan A Merritt       merritt@u.washington.edu
> Biomolecular Structure Center
> Mailstop 357742
> University of Washington, Seattle, WA 98195

-- 
Dave Denholm              <dde...@es...>       http://www.esmertec.com

Re: few remarks

[Ethan M. <merritt@u.washington.edu>]

On Thursday 12 August 2004 03:44 am, Dave Denholm wrote:
> 		if (type != LineSolid || width != 0) {	/* select solid line */
> 		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
> 		} 
> 
> - try setting the linewidth param to 1 in the XSetLineAttributes call.

Thanks for the tip.   The change I actually made was to unconditionally call
	
            /* Force line type to solid, with round ends */
            XSetLineAttributes(dpy, *current_gc, plot->lwidth, LineSolid, CapRound, JoinRound);

I was under the impression that passing plot->lwidth was essentially a no-op,
since that should have been the current state anyhow.  I made it unconditional
in order to force the CapRound attribute.   But if I understand your summary of
X oddities, it may actually be the explicit linewidth rather than the CapRound
that causes the observed problem to go away.

Daniel, do you care enough to investigate further?
I'm happy just to leave it as it is, since it seems to work. 

-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195

Re: few remarks

[Dave D. <dde...@es...>]

Daniel J Sebald <dan...@ie...> writes:

> Dave Denholm wrote:
>
>>Ethan Merritt <merritt@u.washington.edu> writes:
>>
>>
>>>On Monday 09 August 2004 11:04 pm, mi...@ph... wrote:
>>>
>>>>Bug:
>>>>On x11 (well, I haven't look to other terminals), points in the points and
>>>>linepoints styles are "handicapped" -- look by a screen magnificator (xmag,
>>>>kmag) to points obtained by:
>>>>
>>>>set size ratio 1
>>>>plot sin(x)/x w point pt 2
>>>>
>>>That is one strange bug you have found.  The odd thing is that it seems
>>>to be a bug in the X server.   I can run the same gnuplot_x11 executable
>>>on two different machines and get two different results.  My best guess
>>>at the moment is that the key difference is which version of X11 is
>>>running, although it is also possible that it is a difference in the video
>>>driver rather than the version.  The issue is whether the rendering
>>>engine does, or does not, draw the final pixel in a line segment that is
>>>at 45 degrees. Changing the drawing style from CapButt to CapRound
>>>while drawing points fixes this, at least on the machines I have tested.
>>>I can't think of any bad effects from this so I will make the change.
>>>I'm sure someone will speak up if it uncovers some complementary
>>>bug on other X11 display setups.
>>>
>>>
>>
>>
>>Does gnuplot use zero-width lines for these ?  I have a vague
>>recollection that if linewidth is set to 0, the X server can use a
>>device-specific algorithm for drawing the pixels. A linewidth of 1
>>specifies the exact semantics which must be followed.
>>
>>
>>Yeah - looks like :
>>
>>	else if (*buffer == 'P') {
>>	    /* linux sscanf does not like %1d%4d%4d" with Oxxxxyyyy */
>>	    /* sscanf(buffer, "P%1d%4d%4d", &point, &x, &y); */
>>	    point = buffer[1] - '0';
>>	    sscanf(buffer + 2, "%4d%4d", &x, &y);
>>	    if (point == 7) {
>>		/* set point size */
>>		px = (int) (x * xscale * pointsize);
>>		py = (int) (y * yscale * pointsize);
>>	    } else {
>>		if (type != LineSolid || width != 0) {	/* select solid line */
>>		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>>		}
>>
>>
>>- try setting the linewidth param to 1 in the XSetLineAttributes call.
>>
>
> I printed out the numbers and verified that
>
> plot->lwidth
>
> is 1.  I've concluded, like Ethan, that the behavior of this routine
> doesn't match what the documentation says.  (It's as though there is
> some confusion between CapButt and CapNotLast.)  Also, I think
> CapRound, i.e. the endpoints of the lines are rounded when they get
> thicker would look just as good as not rounding when dealing with
> symbols.
>

Sorry, I should have been clearer. I don't mean the gnuplot line
width. I mean the X linewidth set into the GC when drawing the symbol.

X allows a linewidth of 0 to mean a fast line draw, not necessarily
conforming to all the specs. ie it is allowed to be different on
different X servers.

You need to set the X GC linewidth to 1 to get a fully-conforming
line.

dd
-- 
Dave Denholm              <dde...@es...>       http://www.esmertec.com

Re: proposal for cvs'ing 'with image'

[Daniel J S. <dan...@ie...>]

mi...@ph... wrote:

>>>I propose the patch gets committed into cvs.
>>>      
>>>
>>I must admit I haven't quite kept up with its development recently, but
>>from what I remember off-hand, there was quite some discussion about
>>the way it integrated its binary datafile reading with datafile.c. It
>>essentially blows up the largest routine in all of datafile.c by yet
>>another factor of two, but with rather little actual connection
>>between the new and old portions.  I would have liked to have seen that
>>cleaned up before it goes into CVS, but it can be delayed until later.
>>

I think there was this issue, and the issue about the way a map3d_xy() 
routine rounds, especially for PostScript.  Ethan said he will be 
looking at the routine in question some time in the future.  But if you 
want, I can take a look at the rounding issue and see if one is better 
than the other.  My guess is that one case does the rounding directly 
from a CPU register, the other rounds to return a value on the stack.  I 
could repeat some code to avoid the effect of a double version of 
map3d_xy().  But I'd rather not introduce superfluous code that someone 
later could overlook.  (It will still remain obvious in the code what 
the original form of the routine is.)

>I propose clean up of datafile.c, if you really find this necessary, after
>the patch is in cvs. The patch is quite big, and its maintenance as a
>patch needs more effort than necessary. Thus I propose to commit the patch
>ASAP.
>I think that Daniel should get rights for cvs.
>

Well, I don't know if that is a good idea.  I know the guy.  He can run 
CVS on his own system well enough, where any alterations can be undone 
by brute force if necessary.  But as for expertise of a remote site, 
that's dicey.

Anyway, yes the patch is big and consequently almost any alterations to 
CVS (the common files that get most development) cause some hunks to 
fail.  But if only one function within datafile.c gets altered, a patch 
shouldn't go out of date too quickly.  I would propose that I build a 
small patch to clean up that routine in question.  But some discussion 
is required to decide how best to clean up datafile.c, and testing would 
be required.  I think it would be a four to six month time frame for a 
secondary patch.  At that time, if it looks like a little changes are 
required, which I hope isn't the case, maybe then CVS access.

Dan

Re: few remarks

[Daniel J S. <dan...@ie...>]

Dave Denholm wrote:

>Ethan Merritt <merritt@u.washington.edu> writes:
>
>  
>
>>On Monday 09 August 2004 11:04 pm, mi...@ph... wrote:
>>    
>>
>>>Bug:
>>>On x11 (well, I haven't look to other terminals), points in the points and
>>>linepoints styles are "handicapped" -- look by a screen magnificator (xmag,
>>>kmag) to points obtained by:
>>>
>>>set size ratio 1
>>>plot sin(x)/x w point pt 2
>>>      
>>>
>>That is one strange bug you have found.  The odd thing is that it seems
>>to be a bug in the X server.   I can run the same gnuplot_x11 executable
>>on two different machines and get two different results.  My best guess
>>at the moment is that the key difference is which version of X11 is
>>running, although it is also possible that it is a difference in the video
>>driver rather than the version.  The issue is whether the rendering
>>engine does, or does not, draw the final pixel in a line segment that is
>>at 45 degrees. Changing the drawing style from CapButt to CapRound
>>while drawing points fixes this, at least on the machines I have tested.
>>I can't think of any bad effects from this so I will make the change.
>>I'm sure someone will speak up if it uncovers some complementary
>>bug on other X11 display setups.
>>
>>    
>>
>
>
>Does gnuplot use zero-width lines for these ?  I have a vague
>recollection that if linewidth is set to 0, the X server can use a
>device-specific algorithm for drawing the pixels. A linewidth of 1
>specifies the exact semantics which must be followed.
>
>
>Yeah - looks like :
>
>	else if (*buffer == 'P') {
>	    /* linux sscanf does not like %1d%4d%4d" with Oxxxxyyyy */
>	    /* sscanf(buffer, "P%1d%4d%4d", &point, &x, &y); */
>	    point = buffer[1] - '0';
>	    sscanf(buffer + 2, "%4d%4d", &x, &y);
>	    if (point == 7) {
>		/* set point size */
>		px = (int) (x * xscale * pointsize);
>		py = (int) (y * yscale * pointsize);
>	    } else {
>		if (type != LineSolid || width != 0) {	/* select solid line */
>		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
>		}
>
>
>- try setting the linewidth param to 1 in the XSetLineAttributes call.
>

I printed out the numbers and verified that

plot->lwidth

is 1.  I've concluded, like Ethan, that the behavior of this routine 
doesn't match what the documentation says.  (It's as though there is 
some confusion between CapButt and CapNotLast.)  Also, I think CapRound, 
i.e. the endpoints of the lines are rounded when they get thicker would 
look just as good as not rounding when dealing with symbols.

Dan

Re: proposal for cvs'ing 'with image'

[<mi...@ph...>]

>> I propose the patch gets committed into cvs.
>
> I must admit I haven't quite kept up with its development recently, but
> from what I remember off-hand, there was quite some discussion about
> the way it integrated its binary datafile reading with datafile.c. It
> essentially blows up the largest routine in all of datafile.c by yet
> another factor of two, but with rather little actual connection
> between the new and old portions.  I would have liked to have seen that
> cleaned up before it goes into CVS, but it can be delayed until later.

I propose clean up of datafile.c, if you really find this necessary, after
the patch is in cvs. The patch is quite big, and its maintenance as a
patch needs more effort than necessary. Thus I propose to commit the patch
ASAP.
I think that Daniel should get rights for cvs.

---
PM

Re: few remarks

[Dave D. <dde...@es...>]

Ethan Merritt <merritt@u.washington.edu> writes:

> On Monday 09 August 2004 11:04 pm, mi...@ph... wrote:
>> Bug:
>> On x11 (well, I haven't look to other terminals), points in the points and
>> linepoints styles are "handicapped" -- look by a screen magnificator (xmag,
>> kmag) to points obtained by:
>> 
>> set size ratio 1
>> plot sin(x)/x w point pt 2
>
> That is one strange bug you have found.  The odd thing is that it seems
> to be a bug in the X server.   I can run the same gnuplot_x11 executable
> on two different machines and get two different results.  My best guess
> at the moment is that the key difference is which version of X11 is
> running, although it is also possible that it is a difference in the video
> driver rather than the version.  The issue is whether the rendering
> engine does, or does not, draw the final pixel in a line segment that is
> at 45 degrees. Changing the drawing style from CapButt to CapRound
> while drawing points fixes this, at least on the machines I have tested.
> I can't think of any bad effects from this so I will make the change.
> I'm sure someone will speak up if it uncovers some complementary
> bug on other X11 display setups.
>


Does gnuplot use zero-width lines for these ?  I have a vague
recollection that if linewidth is set to 0, the X server can use a
device-specific algorithm for drawing the pixels. A linewidth of 1
specifies the exact semantics which must be followed.


Yeah - looks like :

	else if (*buffer == 'P') {
	    /* linux sscanf does not like %1d%4d%4d" with Oxxxxyyyy */
	    /* sscanf(buffer, "P%1d%4d%4d", &point, &x, &y); */
	    point = buffer[1] - '0';
	    sscanf(buffer + 2, "%4d%4d", &x, &y);
	    if (point == 7) {
		/* set point size */
		px = (int) (x * xscale * pointsize);
		py = (int) (y * yscale * pointsize);
	    } else {
		if (type != LineSolid || width != 0) {	/* select solid line */
		    XSetLineAttributes(dpy, gc, 0, LineSolid, CapButt, JoinBevel);
		}


- try setting the linewidth param to 1 in the XSetLineAttributes call.


dd
-- 
Dave Denholm              <dde...@es...>       http://www.esmertec.com

Re: proposal for cvs'ing "with image"

[Hans-Bernhard B. <br...@ph...>]

On Thu, 12 Aug 2004 mi...@ph... wrote:

> Daniel has made a great work on implementing support for (binary) images
> into gnuplot. The result is a patch implementing two new styles "with
> image" and "with rgbimage", and extending support for reading binary
> files.
> I propose the patch gets committed into cvs.

I must admit I haven't quite kept up with its development recently,
but from what I remember off-hand, there was quite some discussion about
the way it integrated its binary datafile reading with datafile.c.
It essentially blows up the largest routine in all of datafile.c by
yet another factor of two, but with rather little actual connection 
between the new and old portions.  I would have liked to have seen that
cleaned up before it goes into CVS, but it can be delayed until later.

-- 
Hans-Bernhard Broeker (br...@ph...)
Even if all the snow were burnt, ashes would remain.

proposal for cvs'ing "with image"

[<mi...@ph...>]

Daniel has made a great work on implementing support for (binary) images
into gnuplot. The result is a patch implementing two new styles "with
image" and "with rgbimage", and extending support for reading binary
files.
I propose the patch gets committed into cvs.

---
PM