The attached patch is meant to clean up a logt of the Qt code that launches and establishes a link with gnuplot_qt server as well as the mechanisms for waiting on requested data to come back from gnuplot_qt. There is only a small change to the application code which is to send the name of the server it constructs to standard out so that qt_term can read the name.
Here is a summary of mods:
-
I changed the QProcess to be a conventional one rather than a disconnected one. That way there is still some access to the process so that the constructed server name can be retrieved from the process itself rather than having to duplicate the construction of the name in multiple places. We can then use the Qt waiting mechanisms and get rid of the custom while loop that waits for an external server to appear.
-
Note: In qt_flushOutBuffer() the flush() is necessary because there is no event loop. If there were an event loop, just socket.write() would be enough.
-
I commented out
// Avoid dead-locking when no more data is available
if (qt->socket.bytesToWrite() > 0)
qt->socket.waitForBytesWritten(-1);
because I don't think it is necessary. I left it there just in case it is needed on some systems. I doubt it, though.
qt->socket.disconnectFromServer();
to the end of qt_atexit(). That routine waits for all pending data to be sent before returning and complains if one attempts to wait on disconnect. So, an additional wait-on-disconnect doesn't seem necessary.
-
I altered the way that qt_term waits on gnuplot_qt when it wants font property info. (We are going to need to add a similar routine in order to get the true terminal size back from gnuplot_qt term in to fix the issue Ethan noted.) I make use of the proper timeout mechanism for Qt rather than introducing "micro sleep" and timeouts of 0 or -1. If one reviews that hunk of code they'll realize it is pretty conventional, i.e., keep reading bytes until enough are read to fill sizeof(gp_event_t). It turns out that one read typically get all of the bytes necessary.
-
Even after this change, I don't like the fact that qt_sendFont() discards events. It is usually carriage returns from holding the Enter key down and an event slips in before gnuplot_qt receives GEFontMetricRequest. However, I'm not sure what to do without some kind of queueing mechanism.
-
Similar to the waiting on font info, I changed this while loop:
while (qt->socket.bytesAvailable() >= (int)sizeof(gp_event_t)) {
because, as with a commenter on the gnuplot developers list, that construct doesn't sit well with me. Beyond seeming like it could get stuck in certain situations (maybe not), I don't like that it is whirling away waiting for the number of bytes to be sufficient. We should be using the Qt mechanism to do any type of waiting rather than doing polling.
-
I changed that construct for Windows as well. It looks like almost an exact copy so I suspect it shouldn't break Windows Qt term. But if it breaks something, we'll have to do another rev.
-
The only thing I worry about (and I've left a note in the code) is that when waiting for responses from gnuplot_qt (such as font props) if there was a preceeding plot that has a huge amount of data. If the plot doesn't complete in the timeout, then there could be an error. The proper thing would be to dynamically adjust the wait time. But let's hold off on that scheme. I defined DEFAULT_TIMEOUT at the top of qt_term.cpp and we can lengthen that if need be.
-
There may be a chance that Windows persist works correctly now. I changed the qt_atexit() function slightly to use Qt routines
// This Qt function waits until pending data to write is complete.
qt->socket.disconnectFromServer();
Before that change it seemed like qt_atexit() might be wrapping things up too briskly, not giving enough time for proper interchange and disconnect. Perhaps Windows automatically deletes the gnuplot_qt process without proper disconnect, or more likely the persist command wasn't making its way to gnuplot_qt.
