Intermittent error plotting images with the x11 terminal

[Dima K. <gn...@di...>]

Hi all.

I'm seeing an intermittent error that happens when plotting an image
with the x11 terminal. To reproduce:

- Fresh checkout of gnuplot
- $ ./prepare && ./configure && make -j2
- $ export GNUPLOT_DRIVER_DIR=$PWD/src
- $ src/gnuplot /tmp/tst.gp

The data file is attached (it's mostly the same as the one in an
unrelated email a few days ago). Normally, that command will make a plot
and exit immediately; the plot window should quickly flicker. There is a
bug, however, that causes this plot command to fail occasionally. If I
run that last command repeatedly, I sometimes get

 GNUPLOT (gplt_x11):  Couldn't read image parameters correctly.

When this error pops up, the resulting plot is generally corrupt. The
error happens much more readily if the plot window comes up under the
mouse cursor. I'm using a more-or-less stock Debian/unstable box, and I
see this error often, even when I'm using gnuplot to get work done, not
just trying to coax out the bug.

I dug into this, and I see what's happening. There's quite a bit of
state in gplt_x11.c and it doesn't all stay in sync all the time. The
image is transferred into gplt_x11.c from gnuplot itself using lots of
'i' commands. The bug occurs when an X EnterNotify event is received,
and causes display() to be called in the middle of the image transfer.
The display() then processes all the 'i' commands that have come through
so far; not all of them arrived yet, so it doesn't actually draw
anything. There is state in processing the 'i' commands, however. This
state is normally reset when the last 'i' command is processed to
complete the image. Here, we never completed the image, so the state is
never reset, and gnuplot is confused the next time display() is called
when all the data is finally available. The static variables in question
are in exec_cmd() in this block:

     else if (*buffer == X11_GR_IMAGE) {	/* image */

         static unsigned char *iptr;
         static TBOOLEAN transferring = 0;
         static unsigned short *image;
         static int M, N;
         static int pixel_1_1_x, pixel_1_1_y, pixel_M_N_x, pixel_M_N_y;
         static int visual_1_1_x, visual_1_1_y, visual_M_N_x, visual_M_N_y;
         static int color_mode;
         static unsigned int i_remaining;


There are multiple ways to address this, so I'm not making fixes yet. We
could disallow calling display() with incomplete data, or we could reset
the state at the start of the display() call. As a separate comment,
making this less stateful and simplifying the image transfer would be a
great thing to do in either case, I think.

Is any particular approach favored here?

dima