From: Dima K. <gn...@di...> - 2012-09-28 08:35:20
|
> On Thu, 27 Sep 2012 12:00:10 -0700 > Ethan Merritt <merritt@u.washington.edu> wrote: > > On Thursday, September 27, 2012 11:41:50 am Ethan A Merritt wrote: > > On Thursday, September 27, 2012 01:40:18 am Dima Kogan wrote: > > > > On Tue, 25 Sep 2012 21:31:55 -0700 > > > > "sfeam (Ethan Merritt)" <eam...@gm...> wrote: > > > > > > > > On Saturday, 22 September 2012, Dima Kogan > > > > <gn...@di...> wrote: > > > > > > > > > > I tackled the long-standing issue of the x11 terminal not > > > > > respecting the requested plot aspect ratio > > > > <snip> > > > > > > > > Hi Dima. > > > > > > > > I've applied your patch to CVS in the development branch after > > > > review and testing. <snip> > > > > > > Hi Ethan. Thanks for applying the patch. Main difference I > > > noticed so far is that the point sizes appear smaller in the x11 > > > terminal than they were before this patch. > > > > I have found another problem. I suspect it may be an integer > > overflow, but if so I can't figure out where. Here's a simple > > recipe to show the problem: > > gnuplot> set term x11 > > gnuplot> set sample 15 > > gnuplot> plot x with points pt 5 > > > > 1) Adjust the X-window so that the plot is very tall. > > 2) Gradually make the plot window narrower and narrower. > > At some point the top border of the plot stops being correctly > > drawn near the top of the window; instead it is drawn near the > > bottom. > > > > gnuplot> show var GPVAL_TERM > > GPVAL_TERM_XMIN = 473 > > GPVAL_TERM_XMAX = 3837 > > GPVAL_TERM_YMIN = 280 > > GPVAL_TERM_YMAX = 10072 > > GPVAL_TERM_XSIZE = 4096 > > GPVAL_TERM_YSIZE = 10245 > > > > The problem seems to trigger when YMAX crosses exactly 10000, > > but I don't see any relevant use of 9999 or 10000 as a constant in > > the code. Perhaps it's a formatted field overflow from 4 digits to > > 5 in the y coordinate? > > I think that must be it. The coordinates sent from x11.trm to > gnuplot_x11 use format statements like PRINT2("V%04d%04d\n", x, y); > > It looks like either the field width has to be increased everywhere > to 5 digits, or else the new rescaling code needs to rethought > (perhaps by adjusting both xsize and ysize rather than only adjusting > ysize?) Increasing the field width has the drawback that more bytes > will be sent over the channel regardless of the window size, which > can slow down the communication. > > What to do? > > Ethan Hi Ethan. Good you found the issue. Do you know why field widths are specified at all? Why don't we PRINT2("V%d %d\n") and then sscanf("%d %d", ...) on the other side? No efficiency is gained by specifying the widths, it only builds in limitations and makes the code brittle, as we have just observed. I'm attaching a patch that removes hardcoded field sizes from ALL %d and %u fields. Some extra care had to be taken in places where a numerical field is followed by a string (%s), since the '4' was hardcoded in those places too. The bug you described is now gone, and the code is more robust. I noticed another, very related issue. x11.trm has /* badly outrange labels can overflow into text field */ if (x < 10000 && y < 10000) { PRINT3("T%d %d %s\n", x, y, str); } Here we're hardcoding the 10000 again. This has the effect of not printing the legend label when the window is skinny (as you were making it). This test needs to be adjusted or removed entirely; not sure which. CVS says this test was added by lhecking in 1998. This predates all of sourceforge. Newsgroup logs go back that far, but I didn't see any mention of this issue. Thoughts? Finally, in writing the patch, I stumbled on what looks like a bug in what looks like dead code. In gplt_x11.c, look at the else if (*buffer == X11_GR_FILLED_POLYGON) { /* filled polygon */ ... } block. Look at where the ptr is updated (the ptr += ... lines). I suspect the 2nd update of this pointer (the one not in an if() or a while()) was meant to happen only if the first one didn't happen. Otherwise we're skipping over data we haven't looked at yet. The attached patch could be wrong here, depending on what is actually meant. In any case, I can't find anything in the inboard driver that sends X11_GR_FILLED_POLYGON commands. None of the demos trigger this block either. I suspect the whole block is dead, and this bug is thus never hit. If this whole block is truly dead, it should be removed. If this actually IS called somehow, I'd like to know from where to make sure the attached patch is good. dima |