#633 Add keyboard shortcut for hiding all lines in wxt

[Jon Gjengset]

When plotting many aspects of a dataset in a single graph, it is often convenient to be able to hide all lines and then only selectively enable those that are of interest. The attached patch makes 'c' a keyboard shortcut for doing this in wxterminal.

The patch currently adds a case to wxtPanel::OnKeyDownChar for handling this. In the long term, a better solution might be to dispatch a command like "hide line 1" to the main gnuplot process, which then triggers a replot, but then there first has to be a way of targeting a command to a specific line.

Patch has been tested against the latest CVS version on Arch Linux x64.

[Jon Gjengset]

I've added an implementation for qtterminal now (patch attached).

It seems to work correctly, although there seems to be occasional delay between pressing the key and the visibilities changing. I don't know enough about the event dispatching here to figure out what might be causing it. Any thoughts?

[Jon Gjengset]

Managed to remove the delay by adding qt_flushOutBuffer() to qt_modify_plots().

The hotkeys do not seem to work if gnuplot is invoked with -p however. This is not because of my code though I think, as other hotkeys like "g" also stop working when run through gnuplot -p. The hotkeys work fine in wxt with -p.

[Jon Gjengset]

I've been trying to write an integration with the x11 terminal, but am having some issues. Whenever any of the hotkeys are pressed, the x11 window will completely freeze, and I just can't figure out what is causing it.

The attached patch contains only the changes to enable support in x11 (+mouse.c, term_api.h, etc). Any chance you could take a look and see if you immediately spot where the error is?

[Ethan Merritt]

I don't know exactly why it hangs, but I can tell you that there is a fundamental problem with the code. The code you added to gplt_x11.c is inserting the toggle into the display list, where it will be executed every time the plot window is refreshed. It is not being executed just once, asynchronously, which is what you wanted it to do.

I'm kind of busy these next few days. I'll have another look next weekend.

[Jon Gjengset]

Ah, yes, that'd do it. Found the problem.

[Jon Gjengset]

Okay. The attached patch now has working implementations for wxt, qt and x11.

[Ethan Merritt]

It's looking good, but there are some problems lurking, particularly with multiplots. Some of them may simply be revealing existing bugs. Here's what happens when I try the qt terminal and 'load "layout.dem"' followed by hitting any of i/V/v

Program received signal SIGABRT, Aborted.
0xffffe424 in __kernel_vsyscall ()
(gdb) where
#0  0xffffe424 in __kernel_vsyscall ()
#1  0xb64bca81 in raise () from /lib/i686/libc.so.6
#2  0xb64be3d2 in abort () from /lib/i686/libc.so.6
#3  0xb69e9b64 in qt_message_output(QtMsgType, char const*) ()
   from /usr/lib/libQtCore.so.4
#4  0xb69e9d83 in ?? () from /usr/lib/libQtCore.so.4
#5  0xb69e9ea8 in qFatal(char const*, ...) () from /usr/lib/libQtCore.so.4
#6  0xb69e9eed in qt_assert_x(char const*, char const*, char const*, int) ()
   from /usr/lib/libQtCore.so.4
#7  0x0805d8b4 in QList<QGraphicsItemGroup*>::operator[] (this=0x912903c, i=20)
    at /usr/lib/qt4/include/QtCore/qlist.h:447
#8  0x0805d29b in QtGnuplotScene::processEvent (this=0x9129028, type=GEModPlots, in=...)
    at qtterminal/QtGnuplotScene.cpp:452
#9  0x080561df in QtGnuplotWidget::processEvent (this=0x9128e90, type=0, in=...)
    at qtterminal/QtGnuplotWidget.cpp:150
#10 0x08052d37 in QtGnuplotWindow::processEvent (this=0x91968f0, type=0, in=...)
    at qtterminal/QtGnuplotWindow.cpp:141
#11 0x08054974 in QtGnuplotApplication::processEvent (this=0xbfc86a50, type=0, in=...)
    at qtterminal/QtGnuplotApplication.cpp:119
#12 0x0805f056 in QtGnuplotEventHandler::readEvent (this=0x9196aa8)
    at qtterminal/QtGnuplotEvent.cpp:127
#13 0x080607e3 in QtGnuplotEventHandler::qt_metacall (this=0x9196aa8, _c=
---Type <return> to continue, or q <return> to quit---q
QMetaObject::InvQuit

This works out to be an illegal access from the following line in QtGnuplotScene.cpp

        else if (type == GEModPlots)
        {
                int i = m_key_boxes.count();
                unsigned int ops_i;
                in >> ops_i;
                enum QtGnuplotModPlots ops = (enum QtGnuplotModPlots) ops_i;
                while (i-- > 0) {
                        bool isVisible = m_plot_group[i]->isVisible();
                                         ^^^^^^^^^^^^^^^^

(gdb) up
#8  0x0805d29b in QtGnuplotScene::processEvent (this=0x9129028, type=GEModPlots, in=...)
    at qtterminal/QtGnuplotScene.cpp:452
452                             bool isVisible = m_plot_group[i]->isVisible();
(gdb) print i
$2 = 20

However, if I try hard enough I can trigger a similar failure in the CVS version, so I suspect some piece of incorrect bookkeeping that is unfortunately triggered more easily by the new code than the existing code.

The other thing is more of a "feature request" than a bug. Ideally if gnuplot is run in "-persist" mode, the display thread will try to handle mouse/hotkey events to the extent possible even after the original command line process has exited.

[Jon Gjengset]

Hmm, yes, I can trigger the same bug here. When I run:

$ gnuplot
gnuplot> set terminal qt
gnuplot> load "layout.dem"

however, the qt window doesn't accept any of v/V/i. I have to continue pressing enter until the last of the plots has been displayed until it recognizes them. At that point I see the SIGABRT.

After looking at the code, I'm not entirely sure what might be causing this. My first though was that i should be initialized to m_key_boxes.count() - 1, rather than just m_key_boxes.count(), but this does not seem to make a difference. The loop is pretty much a copy of what happens in QtGnuplotScene::mouseReleaseEvent, so I find it strange that the bug is not happening there as well.

Another thing I noticed while tinkering with this is that 'g' does some weird things to the graphs plotted by layout.dem. They change completely. Also, I'm not entirely sure why 'g' still works on all the graphs, but v/V/i only works on the last one...

[Jon Gjengset]

My guess is that m_key_boxes contains every key box (that is, for every plot), whereas m_plot_group contains only the visible plots, but since it loops over m_key_boxes.count()-0 it will naturally overflow m_plot_group. I'll do some more investigation...

[Ethan Merritt]

I suspect what's happening is that m_key_boxes.count() is the total number of key entries in all graphs in the multiplot. m_plot_group.count() may instead be the number of components in the current (i.e. last) plot in the multiplot. I have not yet dived into the code to confirm this guess. Both Qt and X11 seem to fail in distinguishing the elements of distinct plots in a multiplot, though the symptoms in X11 are different. Again I don't think the problem lies in your code except in that it provides an easy way to trigger the error.

I can prevent the segfault by adding code like this (needed in several places)

        /* Check for click in one of the keysample boxes */
        int i = m_key_boxes.count();
            /* DEBUG */ if (i > m_plot_group.count())
            /* DEBUG */     i = m_plot_group.count();

But that just suppresses the crash; it doesn't make the code correct.

[Ethan Merritt]

Great minds think alike :-)
(overlapped posts)

[Jon Gjengset]

Hehe, within minutes of each other. Not bad.

In QtGnuplotScene::resetItems(), what is the rationale for clearing m_plot_group, but not m_key_boxes? I've tried adding code for clearing m_key_boxes there as well, but it doesn't seem to make a difference...

Any particular reason why all the key boxes should be kept, but the plot group shoudl only the current plots?

[Ethan Merritt]

m_plot_group[] is indexed by the m_currentPlotNumber, which runs from 0-N within a single "plot" command. This is what you would need after plotting in order to send down a request from the command line "toggle plot 4". A multiplot by definition contains multiple plot commands.

m_key_boxes[] holds one member for each distinct plot over the whole graph. For a multiplot this can be as high as the sum of all elements in all plots. This should allow you to toggle an individual plot element within a single one of the multiplot plots.

Perhaps Qt needs a new variable m_totalPlotNumber that is incremented in parallel to m_currentPlotNumber but doesn't get reset to 0 with each new plot in a multiplot. If m_plot_group[] were index by this new variable then the problem would go away. A cleaner solution might be to drop the idea that the indexing runs in parallel. Instead we could store a pointer to the plot group in the keybox structure. So instead of

  if (m_key_boxes[i].contains(m_lastMousePos))
      m_plot_group[i]->setVisible(false);

we'd have something like

  if (m_key_boxes[i].contains(m_lastMousePos))
      m_key_boxes[i]->plot_group->setVisible(false);

That seems cleaner to me if there isn't some other snag.

I assume the problem in X11 can be similarly fixed, but it's written in vanilla C so it will take more manual bookkeeping.

[Jon Gjengset]

The latter seems cleaner to me as well, but I'm not sure I understand why m_key_boxes holds a member for each distinct plot. Surely it doesn't make sense logically to have more keys than you have plots, so why does the program maintain these in such different ways? Why not make m_key_boxes contain only the keys for the current plot instead, as only a single plot (in the current implementations at least) is visible at any given time?

[Jon Gjengset]

The reverse might also be useful - that is, being able to get the key_box for a particular plot. Then the MODPLOT code would just have to iterate over the plot_group and indirectly access the corresponding key.

[Ethan Merritt]

I have multiplot + toggle working in qt and x11 as well as wxt. As we noted, this was an existing problem that was easily triggered by the new code.

The new code is now in CVS for 4.7. Thanks for the patch.

It would be nice to extend this to
- svg and canvas terminals
- command line toggling of individual plots
- pop plot to top of stack (maybe only possible in qt)

[Ethan Merritt]

No one has reported problems with the code now in CVS. Perhaps they didn't test, or perhaps it actually works :)

I don't think this tracker should be closed yet, because there are still issues like extension to more terminal types, possibly tweaking which hotkeys are involved, and adding a command line interface.