x11 plotting into an existing window behaves badly if the window doesn't (yet) exist

Dima Kogan <[email protected]> Wed, 10 Aug 2022 13:51:15 -0700
Newsgroups gmane.comp.graphics.gnuplot.devel
Message-ID <[email protected]>
Hi. I'm using the x11 terminal to plot into an existing window:

  set terminal x11 window XID

Overall it works really well. I'm seeing an issue when the requested
window doesn't exist: gnuplot simply ignores the request, and pops up a
new X window, as usual. This is never what the user intends, I think.

A less made-up scenario is what I actually encountered when trying to
use this feature: I made a new window, and asked gnuplot to plot into
it. This didn't work because my new target window hasn't been created
YET.

Trying to debug this, I added some more diagnostics to gplt_x11.c
(attached; these should be good-enough to merge). I see that the
failing sequence is:

- in pr_window() gplt_x11.c calls XGetWindowAttributes() on the
  requested window. This is the first time gnuplot does anything with
  the requested window

- If the window already exists, this succeeds, and we continue, happily.
  If not, the error handler is invoked instead. Here's the diagnostic
  output produced with DEBUG:

    gplt_x11.c:1655 psp->external_container = 0x2400005
    gplt_x11.c:6679 (pr_window) 
    gplt_x11.c:6683 was asked to plot externally...
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_GetWindowAttributes minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:7269   -> Added an element to FIFO queue.
    gplt_x11.c:6692 making plot window
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_CreateWindow minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeWindowAttributes minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:7269   -> Added an element to FIFO queue.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeProperty minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeProperty minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeProperty minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeWindowAttributes minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:4354 Received X error:  error_code=BadWindow request_code=X_ChangeWindowAttributes minor_code=0
    gplt_x11.c:7230 Add plot to remove FIFO queue called.
    gplt_x11.c:6795 term_number is -1
    gplt_x11.c:4778 Cursors reset
    gplt_x11.c:7321 Processed element in remove FIFO queue.
    gplt_x11.c:1209 Delete plot -1
    gplt_x11.c:1233 Destroy window 0x36000fe
    gplt_x11.c:7321 Processed element in remove FIFO queue.
    gplt_x11.c:7063 psp->external_container = None
    ...

  The line numbers are off, since I had some extra diagnostic code. We
  do see that X errors are received for every request that touches the
  not-yet-existing window, the error handler does some complex thing I
  don't yet understand, and eventually we psp->external_container =
  None, i.e. we give up on plotting to the requested window, and create
  a new one, as usual.


Would it not make more sense for gplt_x11 to retry
XGetWindowAttributes() call until the requested window exists? I'm not
familiar with Xlib error handling, and this doesn't look completely
trivial, so I wanted to ask here first. Anybody here have experience
with Xlib, and suggestions on the "right" way to implement that?

Thanks

_______________________________________________
gnuplot-beta mailing list
[email protected]
Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta
0001-ifdef-DEBUG-gplt_x11.c-reports-any-X-errors-it-recei.patch (text/x-diff, 9 KB)
From 9c53a805218183155fd08d96e1ce6c00df3575e6 Mon Sep 17 00:00:00 2001
From: Dima Kogan <[email protected]>
Date: Wed, 10 Aug 2022 13:28:00 -0700
Subject: [PATCH 1/3] #ifdef DEBUG: gplt_x11.c reports any X errors it receives

---
 src/gplt_x11.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/src/gplt_x11.c b/src/gplt_x11.c
index f84109a8b..3c8a5c1f8 100644
--- a/src/gplt_x11.c
+++ b/src/gplt_x11.c
@@ -105,6 +105,7 @@
 #include <X11/Xutil.h>
 #include <X11/Xatom.h>
 #include <X11/keysym.h>
+#include <X11/Xproto.h>
 #ifdef USE_X11_MULTIBYTE
 # include <X11/Xlocale.h>
 #endif
@@ -4051,6 +4052,174 @@ ErrorHandler(Display * display, XErrorEvent * error_event)
 {
     /* Don't remove directly.  Main program might be using the memory. */
     (void) display;		/* avoid -Wunused warnings */
+
+
+#ifdef DEBUG
+
+#define STRINGIFY_ENUM(n) case n: return #n;
+
+    const char* lookup_error_code_string(int x)
+    {
+#define ERROR_LIST(_)                           \
+  _(Success)                                    \
+  _(BadRequest)                                 \
+  _(BadValue)                                   \
+  _(BadWindow)                                  \
+  _(BadPixmap)                                  \
+  _(BadAtom)                                    \
+  _(BadCursor)                                  \
+  _(BadFont)                                    \
+  _(BadMatch)                                   \
+  _(BadDrawable)                                \
+  _(BadAccess)                                  \
+  _(BadAlloc)                                   \
+  _(BadColor)                                   \
+  _(BadGC)                                      \
+  _(BadIDChoice)                                \
+  _(BadName)                                    \
+  _(BadLength)                                  \
+  _(BadImplementation)
+        switch(x) { ERROR_LIST(STRINGIFY_ENUM) default: return NULL; }
+#undef ERROR_LIST
+    }
+    const char* lookup_request_code_string(int x)
+    {
+#define REQUEST_LIST(_)                         \
+  _(X_CreateWindow)                             \
+  _(X_ChangeWindowAttributes)                   \
+  _(X_GetWindowAttributes)                      \
+  _(X_DestroyWindow)                            \
+  _(X_DestroySubwindows)                        \
+  _(X_ChangeSaveSet)                            \
+  _(X_ReparentWindow)                           \
+  _(X_MapWindow)                                \
+  _(X_MapSubwindows)                            \
+  _(X_UnmapWindow)                              \
+  _(X_UnmapSubwindows)                          \
+  _(X_ConfigureWindow)                          \
+  _(X_CirculateWindow)                          \
+  _(X_GetGeometry)                              \
+  _(X_QueryTree)                                \
+  _(X_InternAtom)                               \
+  _(X_GetAtomName)                              \
+  _(X_ChangeProperty)                           \
+  _(X_DeleteProperty)                           \
+  _(X_GetProperty)                              \
+  _(X_ListProperties)                           \
+  _(X_SetSelectionOwner)                        \
+  _(X_GetSelectionOwner)                        \
+  _(X_ConvertSelection)                         \
+  _(X_SendEvent)                                \
+  _(X_GrabPointer)                              \
+  _(X_UngrabPointer)                            \
+  _(X_GrabButton)                               \
+  _(X_UngrabButton)                             \
+  _(X_ChangeActivePointerGrab)                  \
+  _(X_GrabKeyboard)                             \
+  _(X_UngrabKeyboard)                           \
+  _(X_GrabKey)                                  \
+  _(X_UngrabKey)                                \
+  _(X_AllowEvents)                              \
+  _(X_GrabServer)                               \
+  _(X_UngrabServer)                             \
+  _(X_QueryPointer)                             \
+  _(X_GetMotionEvents)                          \
+  _(X_TranslateCoords)                          \
+  _(X_WarpPointer)                              \
+  _(X_SetInputFocus)                            \
+  _(X_GetInputFocus)                            \
+  _(X_QueryKeymap)                              \
+  _(X_OpenFont)                                 \
+  _(X_CloseFont)                                \
+  _(X_QueryFont)                                \
+  _(X_QueryTextExtents)                         \
+  _(X_ListFonts)                                \
+  _(X_ListFontsWithInfo)                        \
+  _(X_SetFontPath)                              \
+  _(X_GetFontPath)                              \
+  _(X_CreatePixmap)                             \
+  _(X_FreePixmap)                               \
+  _(X_CreateGC)                                 \
+  _(X_ChangeGC)                                 \
+  _(X_CopyGC)                                   \
+  _(X_SetDashes)                                \
+  _(X_SetClipRectangles)                        \
+  _(X_FreeGC)                                   \
+  _(X_ClearArea)                                \
+  _(X_CopyArea)                                 \
+  _(X_CopyPlane)                                \
+  _(X_PolyPoint)                                \
+  _(X_PolyLine)                                 \
+  _(X_PolySegment)                              \
+  _(X_PolyRectangle)                            \
+  _(X_PolyArc)                                  \
+  _(X_FillPoly)                                 \
+  _(X_PolyFillRectangle)                        \
+  _(X_PolyFillArc)                              \
+  _(X_PutImage)                                 \
+  _(X_GetImage)                                 \
+  _(X_PolyText8)                                \
+  _(X_PolyText16)                               \
+  _(X_ImageText8)                               \
+  _(X_ImageText16)                              \
+  _(X_CreateColormap)                           \
+  _(X_FreeColormap)                             \
+  _(X_CopyColormapAndFree)                      \
+  _(X_InstallColormap)                          \
+  _(X_UninstallColormap)                        \
+  _(X_ListInstalledColormaps)                   \
+  _(X_AllocColor)                               \
+  _(X_AllocNamedColor)                          \
+  _(X_AllocColorCells)                          \
+  _(X_AllocColorPlanes)                         \
+  _(X_FreeColors)                               \
+  _(X_StoreColors)                              \
+  _(X_StoreNamedColor)                          \
+  _(X_QueryColors)                              \
+  _(X_LookupColor)                              \
+  _(X_CreateCursor)                             \
+  _(X_CreateGlyphCursor)                        \
+  _(X_FreeCursor)                               \
+  _(X_RecolorCursor)                            \
+  _(X_QueryBestSize)                            \
+  _(X_QueryExtension)                           \
+  _(X_ListExtensions)                           \
+  _(X_ChangeKeyboardMapping)                    \
+  _(X_GetKeyboardMapping)                       \
+  _(X_ChangeKeyboardControl)                    \
+  _(X_GetKeyboardControl)                       \
+  _(X_Bell)                                     \
+  _(X_ChangePointerControl)                     \
+  _(X_GetPointerControl)                        \
+  _(X_SetScreenSaver)                           \
+  _(X_GetScreenSaver)                           \
+  _(X_ChangeHosts)                              \
+  _(X_ListHosts)                                \
+  _(X_SetAccessControl)                         \
+  _(X_SetCloseDownMode)                         \
+  _(X_KillClient)                               \
+  _(X_RotateProperties)                         \
+  _(X_ForceScreenSaver)                         \
+  _(X_SetPointerMapping)                        \
+  _(X_GetPointerMapping)                        \
+  _(X_SetModifierMapping)                       \
+  _(X_GetModifierMapping)                       \
+  _(X_NoOperation)
+        switch(x) { REQUEST_LIST(STRINGIFY_ENUM) default: return NULL; }
+#undef REQUEST_LIST
+    }
+#undef STRINGIFY_ENUM
+
+    const char* error_code_string   = lookup_error_code_string(  error_event->error_code);
+    const char* request_code_string = lookup_request_code_string(error_event->request_code);
+    FPRINTF((stderr, "Received X error: "));
+    if(error_code_string)   fprintf(stderr, " error_code=%s",             error_code_string);
+    else                    fprintf(stderr, " error_code=%d (unknown)",   error_event->error_code);
+    if(request_code_string) fprintf(stderr, " request_code=%s",           request_code_string);
+    else                    fprintf(stderr, " request_code=%d (unknown)", error_event->request_code);
+    fprintf(stderr, " minor_code=%d\n", error_event->minor_code);
+#endif // DEBUG
+
     Add_Plot_To_Remove_FIFO_Queue((Window) error_event->resourceid);
     gp_exec_event(GE_reset, 0, 0, 0, 0, 0);
     return 0;
-- 
2.34.1
0002-gplt_x11.c-added-missing-n-in-a-DEBUG-print.patch (text/x-diff, 823 B)
From 931f6039fed6cf1dbc83297f8f73cec4b51e00b4 Mon Sep 17 00:00:00 2001
From: Dima Kogan <[email protected]>
Date: Wed, 10 Aug 2022 13:29:45 -0700
Subject: [PATCH 2/3] gplt_x11.c: added missing \n in a DEBUG print

---
 src/gplt_x11.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gplt_x11.c b/src/gplt_x11.c
index 3c8a5c1f8..7239e4da1 100644
--- a/src/gplt_x11.c
+++ b/src/gplt_x11.c
@@ -6647,7 +6647,7 @@ pr_window(plot_struct *plot)
 	sprintf(numstr, "%s%d%c", ICON_TEXT, plot->plot_number, '\0');
     else
 	sprintf(numstr, "%s%c", ICON_TEXT, '\0');
-    FPRINTF((stderr, "term_number is %d", plot->plot_number));
+    FPRINTF((stderr, "term_number is %d\n", plot->plot_number));
     XSetIconName(dpy, plot->window, numstr);
 #undef TEMP_NUM_LEN
     if (!plot->titlestring) {
-- 
2.34.1
0003-gplt_x11.c-added-DEBUG-printing-every-time-we-set-th.patch (text/x-diff, 1.1 KB)
From 7ab35fe19330d31afeb79b19cb109d2a820cb231 Mon Sep 17 00:00:00 2001
From: Dima Kogan <[email protected]>
Date: Wed, 10 Aug 2022 13:54:36 -0700
Subject: [PATCH 3/3] gplt_x11.c: added DEBUG printing every time we set the
 external window id

---
 src/gplt_x11.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/gplt_x11.c b/src/gplt_x11.c
index 7239e4da1..2f64ca0dc 100644
--- a/src/gplt_x11.c
+++ b/src/gplt_x11.c
@@ -1514,6 +1514,7 @@ record()
 		    else {
 			current_plot = Add_Plot_To_Linked_List(-1); /* Use invalid plot number. */
 			if (current_plot) {
+			    FPRINTF((stderr, "psp->external_container = 0x%x\n", window_id));
 			    current_plot->external_container = window_id;
 			    prepare_plot(current_plot);
 			}
@@ -6914,7 +6915,10 @@ Add_Plot_To_Linked_List(int plot_number)
 #if EXTERNAL_X11_WINDOW
 	    /* Number and container methods are mutually exclusive. */
 	    if (plot_number >= 0)
+	    {
+		FPRINTF((stderr, "psp->external_container = None\n"));
 		psp->external_container = None;
+	    }
 #endif
 	    /* Add link to beginning of the list. */
 	    psp->prev_plot = NULL;
-- 
2.34.1
0004-gplt_x11.c-DEBUG-printing-writes-out-the-received-gn.patch (text/x-diff, 799 B)
From 1f814557829e9a11547e09398bf36cb31d574336 Mon Sep 17 00:00:00 2001
From: Dima Kogan <[email protected]>
Date: Wed, 10 Aug 2022 14:07:18 -0700
Subject: [PATCH 4/4] gplt_x11.c: DEBUG printing writes out the received
 gnuplot commands

---
 src/gplt_x11.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/gplt_x11.c b/src/gplt_x11.c
index 2f64ca0dc..d843b54ee 100644
--- a/src/gplt_x11.c
+++ b/src/gplt_x11.c
@@ -1231,6 +1231,13 @@ read_input()
 
     if (!buffered_input_available) {
 	total_chars = read(fd, rdbuf, rdbuf_size);
+
+#ifdef DEBUG
+	FPRINTF((stderr, "Received data from gnuplot: ========\n"));
+	fwrite(rdbuf, 1, total_chars, stderr);
+	FPRINTF((stderr, "\n========\n"));
+#endif
+
 	buffered_input_available = 1;
 	partial_read = 0;
 	rdbuf_offset = 0;
-- 
2.34.1