Menu

#1602 Qt terminal rotate won't unclick / unpress / unhold on first attempt

None
closed-fixed
nobody
None
2015-06-08
2015-04-23
Dan Sebald
No

Gnuplot development version

It seems that about three or four months ago I started experiencing the rotate function of Qt terminal sticking. Prior to that I recall Qt rotate working fine. It's hard for me to say when exactly it started happening because I was only updating gnuplot sporadically at that time.

Try a simple 3D plot:

splot (x+y) with pm3d

for the Qt terminal. Now click/hold the left button while the cursor is in the view and rotate the plot. The cursor will change to the hand icon. Unclick the mouse button and continue to rotate the plot. The cursor will still be the hand icon and the plot continues to rotate. Click/hold once more and rotate. Unclick the mouse button and rotate the plot. The second time the hand icon changes back to the pointer and the plot stops rotating. Normal behavior seems to continue from then on.

I've also noticed that the cursor stays active until the plot completes drawing for Qt terminal. For example, try

set samples 10000
splot (x+y) with pm3d

and try rotating. It works as expected, but because of the slowness of the plot, when the mouse button is let go the icon remains a hand and the point used as the last mouse position happens right at the last cursor point when the last drawing occurs. In other words, unclicking the mouse button will stop the redraw but not stop the position update. When the last view of the plot is drawn is when the cursor changes from a hand back to a pointer. It's difficult to be precise with that behavior because I'm so used to unclicking being the last position in so many different apps.

On the other hand, for X11 I think that when the mouse is unclicked is when the last cursor position is recorded. What happens in that case is the mouse is unclicked and for a few seconds the cursor changes from the rotate arrows icon to the busy icon and then back to the pointer icon. That is nicer behavior, i.e., the last position coinciding with the button unclick.

Discussion

1 2 > >> (Page 1 of 2)
  • Dan Sebald

    Dan Sebald - 2015-04-29

    I've found another way to make the rotation stick for the Qt terminal. If one clicks the mouse when the cursor is on the splot and then move the mouse, the splot will rotate as expected. Let go of the mouse button while the mouse is moving and there is a good chance the rotate behavior will stick. If the mouse is not moving when letting go of the mouse button, the plot will come out of rotate mode.

     
  • Torquil Macdonald Sørensen

    I'm also experiencing this problem. However, I cannot deterministically reproduce the problem. If I let go of the mouse button while moving, it will not stay in rotate mode every time. For me it seems to be quite random, apart from the fact that it seems to always stay in rotate mode after unclick the first time I rotate a new plot.

    $ gnuplot --version
    gnuplot 5.0 patchlevel 0

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-29

    The attached patch seems to fix the problem in the case that you unclick the mouse while a rotation update is in progress. But it doesn't fix the first time rotate problem.

    Actually "fix" may be the wrong word. The patch prevents a button release event from being lost if the graph widget reports itself as inactive. But I have no idea why it thinks it is inactive in the first place. I won't add this patch to CVS until I understand better what's going on.

     
  • Dan Sebald

    Dan Sebald - 2015-05-29

    I observe the behavior as you describe. I'm a little confused by this too. First, I'll point out that this line:

    m_socket->write((char*) &event, sizeof(gp_event_t));
    

    is no longer protected against m_socket = 0 with the change you made... but that doesn't seem to be an issue here.

    I uncomment the qDebug statements you included and I don't see either of them printed in any circumstance. So apparently isActive is always true. If that is the case, how did this work before the patch?

    I've placed

        qDebug() << type;
    

    at the top of the postTermEvent() routine. I see numbers like

    gnuplot> splot (x+y) with pm3d
    11
    gnuplot> 8
    0
    0
    0
    0
    0
    0
    1
    0
    8
    0
    8
    0
    0
    8
    0
    8
    0
    8
    0
    8
    0
    8
    1
    2
    0
    0

    What I notice is that the GE_buttonrelease event (2) is not coming into this routine at the first unclicking of the mouse button. You'll see there is a first 1 (GE_buttonpress) but no 2 after it. A little movement (8 and 0), then I press the mouse button again and a 1 appears...now let go of the mouse button and the 2 appears.

    It looks like something about the Qt mouse state isn't quite correct initially. The first button release isn't being "eventized".

     
  • Dan Sebald

    Dan Sebald - 2015-05-29

    I think I have a good lead on this. Here is a comment in QtGnuplotScene.cpp about problems with double clicking:

    // FIXME: If the press/release events get out of order or if we see a very
    // fast double-click then the program errors out during event processing.
    // Unfortunately I have not been able to reliably filter out these spurious events
    // by setting a minimum time window.  Maybe an explicit interlock would work?
    qint64 time = 0;
    if (m_watches[button].isValid())
        time = m_watches[button].elapsed();
    if (time > 300) {
        m_eventHandler->postTermEvent(GE_buttonrelease,
            int(event->scenePos().x()), int(event->scenePos().y()), button, time, m_widget);
    }
    m_watches[button].start();
    

    I bet it is sometime around when this timer was added that I began seeing the Qt mouse stick. I suspect that the timer isn't starting until the first mouse button release. Therefore time > 300 is not true on the first button release. That m_watches[button].start() probably needs to be done at initialization (i.e., time of first plot draw).

    I'll think about the issue with the FIXME as well.

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-29

    I am pretty sure that the loss of "unclick" events was introduced by the patch set:

    2014-03-18  Jérôme Lodewyck  <lodewyck@users.sourceforge.net>
    
            * src/qtterminal/QtGnuplotEvent.* src/qtterminal/QtGnuplotWindow.cpp
            src/qtterminal/QtGnuplotWidget.cpp src/qtterminal/QtGnuplotScene.cpp:
            Block events that come from inactive plot widgets.
    

    What I don't know at this point is whether this change made the whole timing loop unnecessary. I haven't managed to reproduce the original problem, but enough time has passed that attributing a change to any particular component is hard.

    2011-12-25  Ethan A Merritt  <merritt@u.washington.edu>
    
            * src/qtterminal/QtGnuplotEvent.cpp src/qtterminal/QtGnuplotScene.cpp:
            If two button release events occur in quick succession then the program
            messes up the event handling and winds up in a non-recoverable state.
            Apply an empirical fix to ignore events in a 300 msec window.
    

    I am leaning toward removing the timing loop, which was only an empirical fix to begin with, but I want more testing first.

     
  • Dan Sebald

    Dan Sebald - 2015-05-29

    That is my thinking as well. The timer debounce (for lack of better word) doesn't seem the way to go. The first step would be to remove that; then if there are problems look for a better fix.

    A couple thoughts on the original problem. I see now that

    void QtGnuplotScene::mouseReleaseEvent(QGraphicsSceneMouseEvent* event)

    is an override for a function in QGraphicsScene.cpp

    http://doc.qt.io/qt-4.8/qgraphicsscene.html#mouseReleaseEvent

    which itself is a response to QWidget mouse events with a bit of extra translation on the event. So indirectly, this is an override of QWidget::mouseReleaseEvent. OK.

    Well, at the bottom of mouseReleaseEvent:

    void     QtGnuplotScene::mouseReleaseEvent(QGraphicsSceneMouseEvent*     event)
    {
    [snip]
        QGraphicsScene::mouseReleaseEvent(event);
    }
    

    is a call to the base function. I wonder if that is the proper thing to do, because the code is then also doing the default QGraphicsScene action which may be undesirable. Read here:

    http://doc.qt.io/qt-4.8/qgraphicsscene.html#mouseReleaseEvent

    Something about mouse grabber and losing an item... eh, sounds like maybe that was the original problem. It seems to me all that really needs to be done at the end of this routine is to accept() the event meaning that it gets pulled out of the queue--I suspect--and is done with. ("ignore()" means to pass it on to the next widget in the hierarchy, I think.)

    So, if you see problems after removing the timer, try also replacing the base call

    QGraphicsScene::mouseReleaseEvent(event);

    with and accept() of the event.

    Another thing I'm wondering about is the fact that there is no mouseDoubleClickEvent implementation in QGraphicsScene.cpp. I see implementations for the other three (press, release, move), but not double click. Shouldn't the double click be handled as well? Even if gnuplot terminal doesn't have a double click functionality, that double click should maybe be treated as a Press-Rlease-Press-Release. Not sure. In all four cases, I don't think we want any of the default mouse behavior to be implemented. Often calling the base class is what one wants to do in OOP, but I don't think in this case.

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-30

    Sigh. Getting rid of the time delay loop doesn't work, at least on my lab machines. Without the time delay check, a quick double-click crashes the display window. Replacing the second mouseReleaseEvent() with accept() does not change this.

     
    • Jun T.

      Jun T. - 2015-05-31

      As I posted to gnuplot-beta mailing list about a half year ago:
      http://thread.gmane.org/gmane.comp.graphics.gnuplot.devel/12431
      I believe the problem is at line 917 of qt_term.cpp:

      qt->out << GECopyClipboard << s;
      

      should be

      qt->out << GECopyClipboard << QString(s);
      

      If qt_term.cpp is fixed, then I think the filtering (time > 300)
      is not necessary anymore. Please try the attached patch.

      Jun (Jun-ichi Takimoto)

       
  • Dan Sebald

    Dan Sebald - 2015-05-30

    Please attach a diff file of what you have so far. I will see what happens here.

    I think the mouseDoubleClickEvent needs to be implemented in some way. Here is what the documentation says:

    "
    The default implementation generates a normal mouse press event.

    Note: The widget will also receive mouse press and mouse release events in addition to the double click event. It is up to the developer to ensure that the application interprets these events correctly.
    "

    I think the best approach is to put qDebug() strings in all four routines and then watch what routines are getting called when a double-click occurs.

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-30

    OK. But as I say the attached patch causes bad behaviour.

    I don't interpret the mouseDoubleClickEvent documentation the same way; I think it means if you want double-click detection then you have to write a private implementation of how to detect double clicks. That's exactly what the timing loop is trying to do but it isn't working well :-)

     
  • Dan Sebald

    Dan Sebald - 2015-05-30

    Double click causes no problems here; all seems to work well. FWIW, I implemented a mouseDoubleClickEvent function and put some qDebug strings in each of the four functions. I'm going to leave out the "move" here because that just confuses matters and there are a so many "move"s with the smallest movement of the mouse.

    Anyway, for a double click, here is what I see:

    press
    release
    double-click
    release

    So it is a bit different than what I would have expected from reading the documentation. Nonetheless, a double-click creates a pattern press-release-release in the current code. Is it OK to have two releases in a row?

    I'm attaching a patch that will convert that double-click to a press so that I see

    press
    release
    double-click
    press
    release

    Let me know if that clears things up for your system. If not, that might point to some kind of socket/pipe mouse issue, as opposed to any problems handling the mouse events.

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-30

    No dice.
    I get

    move 
    move 
    move 
        [double-click here]
    press 
    release 
    ############### WRONG readEvent  540028470 69 
    Error: gnuplot_qt socket not responding 
    Hit return to continuemove 
    

    Yes it is in a sense a socket/pipe issue, but as I understand it that is triggered by a spurious (or at least unexpected) mouse event. The timed lock-out window for new mouse events really does prevent this, even if I don't understand why it's necessary. Next I will try going back to a version that keeps the timed lock-out but tries harder to initialize it for the first time through.

    update - yup. Looks promising.

     

    Last edit: Ethan Merritt 2015-05-30
  • Dan Sebald

    Dan Sebald - 2015-05-30

    OK...

    Let's investigate the mouse double click on your system. I'm attaching a patch that will display the mouse double click speed on your system and after that set it to 400 ms. I'm wondering if by some far-off chance Qt isn't able to get the double click speed from your desktop manager.

     
  • Dan Sebald

    Dan Sebald - 2015-05-30

    Oh, there was a version 2 of the patch I didn't see. Seems to work here...

    Documentation for double click interval is here:

    http://doc.qt.io/qt-4.8/qapplication.html#doubleClickInterval-prop

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-30

    I don't use double-click at all, so I may be thinking about this the wrong way. So far as I know, all clicks are passed through when they happen. There is a KDE panel where I can set a "doubleclick interval" (defaults to 400) but I have never paid any attention to it since I don't use doubleclick for anything. I may well be wrong, but I thought that KDE setting only applied to KDE components; i.e. I wouldn't expect it to affect how a non-KDE application like gnuplot got its mouse clicks delivered.

    Are you thinking that Qt or KDE is grouping successive clicks into a single event and therefore not delivering the individual clicks? I don't think that's happening here, although I haven't tried very hard to trace it.

    Update: If I use your patch but set the interval to 999 (~ 1 second) and add a qDebug() in the timing lock-out code that flags every event that is delivered, then I can see that indeed all mouse clicks are delivered regardless of the doubleclick interval setting.

     
  • Dan Sebald

    Dan Sebald - 2015-05-30

    Not sure what I was thinking. The thought came to mind that perhaps there is a configuration parameter to disable double-click, but all I could find was the double-click interval adjustment.

    I've tried finding out where this "WRONG readEvent" is coming from and what it means. Could it be that some Qt socket interface isn't being configured properly? Or that perhaps on your system there is some other type of readEvent clashing with a mouse press or release and that the Qt code needs to be more careful about testing that the readEvent is something it should accept and act upon? I.e., the code is acting on the WRONG readEvent, and perhaps the timing lockout is keeping the readEvents in an order for which they don't interfere? readEvent sounds like something more at the socket level. Documentation here:

    http://linux.die.net/man/3/qsocketnotifier

    http://doc.qt.io/qt-4.8/qsocketnotifier.html

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-31

    The error comes from qt_term.cpp line 969.
    The comments at the site of the timed lock-out and near the site of the error explain it to the extent that I understand it. If there are two events in quick succession, the pipe synchronization is insufficient to insure that both are processed in order. I think a real mutex or lock would be cleaner than the timed lock-out, but I took the simpler option since it seems to work.

     
  • Dan Sebald

    Dan Sebald - 2015-05-31

    Ah, I see it now, thanks.

    Well, I'm getting a little more of the picture. I may see where the issue lies. Just to confirm the behavior of the socket and QDataStream, I printed out the bytes available pre- and post-read as follows:

        // Extract the message length
        if (m_blockSize == 0)
        {
            if (m_socket->bytesAvailable() < (int)sizeof(qint32))
                return;
    qDebug() << "Will read:";
    qDebug() << m_socket->bytesAvailable();
            in >> m_blockSize;
    qDebug() << m_socket->bytesAvailable();
        }
    

    and I'm seeing numbers as follows:

    gnuplot> splot (x+y) with pm3d
    Will read:
    89
    85
    gnuplot> Will read:
    107206
    107202
    Will read:
    116
    112
    Will read:
    148
    144
    Will read:
    136
    132
    Will read:
    116
    112
    Will read:
    12
    8
    Will read:
    136
    132
    Will read:
    116
    112
    Will read:
    12
    8
    Will read:
    20
    16

    That suggests to me that "in >> m_blockSize" is a read that will reduce the number of bytes available by 4. If that is in fact the case, then the bug could be arising because the socket on your system is slow and not transfering the bytes into the QDataStream fast enough. Also, by adding the timer delay, that might be giving more time for the data stream contents to accumulate.

    In the following code, what happens if the bytesAvailable() is routinely less than the conditional tests are asking for?:

    while(!in.atEnd())
    {
        // Extract the message length
        if (m_blockSize == 0)
        {
            if (m_socket->bytesAvailable() < (int)sizeof(qint32))
                return;
            in >> m_blockSize;
        }
    
        // Break if the message is not entirely received yet
        if (m_socket->bytesAvailable() < m_blockSize)
            return;
    
        // Process the message
        int remaining = m_socket->bytesAvailable() - m_blockSize;
        while (m_socket->bytesAvailable() > remaining)
        {
            int type;
            in >> type;
            if ((type < 1000) || (type > GEDone))
            {
            // FIXME EAM - At this point the program cannot recover and
            // if we try to continue it will eventually become a zombie.
            // Better to just exit explicitly right now.
                qDebug() << "############### WRONG readEvent " << type << m_socket->bytesAvailable();
                exit(0);
                // return;
            }
            receiver->processEvent(QtGnuplotEventType(type), in);
        }
        m_blockSize = 0;
    }
    

    In the case of the m_blockSize, no problem. If there are less than four bytes (the size of m_blockSize), then nothing is pulled from the socket before returning. The next time (or some future time) through the function there will be four bytes available and m_blockSize will be read.

    But what if the program/socket timing is such that after m_blockSize is read there is still less than m_blockSize bytes of data in the stream? Then it is a problem because the routine will return after having read the m_blockSize and pulling 4 bytes from the stream (what I confirmed with the printouts above). But the next time in the routine, although the socket may now have more data, the routine will again read the m_blockSize but it will be a bogus block size. At which point the algorithm is in an incorrect state which eventually leads to

                qDebug() << "############### WRONG readEvent " << type << m_socket->bytesAvailable();
    

    It seems to me what is needed is a static flag indicating that a partial read has been done. That is, the algorithm needs to remember that the m_blockSize was read but not the actual block so that next time the function is called m_blockSize is still valid and not read. Of course, that may lead to some initial synchronization issues if there some fraction of a command in the data stream at the start.

    This is contingent on the following being true:

        // Break if the message is not entirely received yet
        if (m_socket->bytesAvailable() < m_blockSize)
            return;
    

    Whether that is the case for your system and not for mine, I'm not sure. I suppose one could print out the block size along with the bytesAvailable() to see if that condition is arising right before a failure (i.e., WRONG readEvent).

     
  • Ethan Merritt

    Ethan Merritt - 2015-05-31

    Here is a fix from Jun-ichi Takimoto

     
  • Jun T.

    Jun T. - 2015-06-01

    In event_buttonrelease() (mouse.c), ge->par2 is used to detect double-click.
    With qt_clipboard_bug.patch, every mouse-release event is taken as a double-click since par2=0. I think we need to use QTime::start()/elapsed() to get the time elapsed from the previous release event.
    Qt seems to have an event mouseDoubleClickEvent but I don't know whether it can be used here.

     
  • Ethan Merritt

    Ethan Merritt - 2015-06-01

    Ah, sorry. I misunderstood. OK, we can keep the timing code but remove the test on (time > 300). Is that what you intended?

     

    Last edit: Ethan Merritt 2015-06-01
  • Dan Sebald

    Dan Sebald - 2015-06-01

    This works for me (accept for a new related problem I will note at the end); no surprise give that my system seems a little more robust to this issue.

    Could someone explain why this fixes the problem? Is what ends up in the clipboard a Qt string? Or a normal ASCII string?

    The new problem that I'm seeing with the patch of this post:

    https://sourceforge.net/p/gnuplot/bugs/1602/#3ba7

    is that if I double-click with the mouse in motion the rotation will stick. This did not happen with the 300 ms timer in the works.

    And in testing I discovered a related cosmetic bug that existed before the patch in the repository code. If I press down the center button the cursor turns to a filled-in hand which adjust the plot height/depth. If while holding down the center button I click the left mouse button the cursor changes to the non-filled-in hand and then to the cross. However, because I have the center mouse button still down the height/depth action is still active. So the cursor icon isn't exactly correct. If I find a simple solution to that I will submit a separate bug report, but otherwise it isn't that significant a problem to deal with.

    Should we investigate the problem I hypothesized in this post:

    https://sourceforge.net/p/gnuplot/bugs/1602/#2b45

    That seems like a problem no matter if it is actually the source of the problem in this thread.

     
  • Ethan Merritt

    Ethan Merritt - 2015-06-01

    is that if I double-click with the mouse in motion the rotation will stick.

    ?? How can you do that? Do you have two mouse devices?

     
1 2 > >> (Page 1 of 2)

Log in to post a comment.