Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Neatest way to avoid re-entrant selectionChanged signal
Forum Updated to NodeBB v4.3 + New Features

Neatest way to avoid re-entrant selectionChanged signal

Scheduled Pinned Locked Moved Solved General and Desktop
5 Posts 2 Posters 1.9k Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • J Online
    J Online
    JonB
    wrote on 19 Feb 2019, 10:18 last edited by JonB
    #1

    Code I have inherited has a multi-selection list widget which has to handle receiving QListWidget::selectionChanged() (https://doc.qt.io/qt-5/qlistview.html#selectionChanged) signal, where if clicked on a certain item will cause other items to be deselected (via QListWidgetItem::setSelected(bool select) (https://doc.qt.io/qt-5/qlistwidgetitem.html#setSelected)). This will cause re-entrant call of slot, which must be dealt with. The way current code does that is IMHO horrible, so I'd like to ask how others would handle it.

    My code is Python/PyQt. However, don't be frightened, the issue would be just the same in C++, and I'm sure you can figure how the Python corresponds to C++. I am happy to accept proposed solutions in C++ (provided they can be turned into Python). With that in mind, here is what the code reads at present, with an "English" explanation of what's going on below.

    The list has a special first item, allItem, which when clicked on indicates that "all items are wanted":

    All Items
    Item #1
    Item #2
    ...
    

    It does not select all the other items; instead it deselects all other items, leaving the "allItem" as the sole selection, which code will act on appropriately.

    Slot definition:

    @staticmethod
    def allItemSelection(lw: QListWidget, selected: QItemSelection, allItem: QListWidgetItem, connectedLambda):
        if selected.contains(lw.indexFromItem(allItem)):
            # If 'all' item just selected, deselect everything else
            lw.selectionModel().selectionChanged.disconnect(connectedLambda)
            lw.clearSelection()
            allItem.setSelected(True)
            lw.selectionModel().selectionChanged.connect(connectedLambda)
        else:
            # else deselect 'all' item
            allItem.setSelected(False)
    

    Caller's signal connection:

    connectedLambda = lambda selection: self.allItemSelection(self.listWidget, selection, self.allItem, connectedLambda)
    self.listWidget.selectionModel().selectionChanged.connect(connectedLambda)
    

    The essential of the slot definition is that if the changed selection contains the allItem we clear all existing selected items and re-select just the allItem (otoh if any other item is selected the allItem is automatically deselected). In itself, fair enough, the problem is that the lw.clearSelection(), and more specifically the following allItem.setSelected(True), will cause the slot to be re-entered, because (unfortunately for me) calling setSelected() from code will raise the signal anew.

    The existing code works by:

    • The slot is itself a lambda (because of the parameters needing to be passed to it).
    • The caller connects the lambda, passing it the lambda itself as one of the arguments (the slot is used shared by other lists with the same behaviour), so that...
    • The slot prevents re-entrancy by QObject::disconnect(lambdaArgument) before setSelected() and re-attach via QObject::connect(lambdaArgument) afterwards.

    To me this is horrible! I don't like storing the lambda in a variable and passing it around, and I don't really like the whole idea of disconnecting the slot and reconnecting it anyway as the way to deal with this.

    So... is there another/neater way to accomplish what is needed here?

    1 Reply Last reply
    0
    • V Offline
      V Offline
      VRonin
      wrote on 19 Feb 2019, 13:05 last edited by VRonin
      #2

      I think you are kinda of overcomplicating the problem... selectionChanged already has all the information it needs not to continuously recurse. see the example below:

      #include <QApplication>
      #include <QListWidget>
      #include <QDebug>
      #include <QItemSelection>
      #include <QItemSelectionModel>
      
      int main(int argc, char ** argv)
      {
          QApplication app(argc,argv);
          QListWidget mainWid;
          mainWid.addItem("All");
          mainWid.setSelectionMode(QAbstractItemView::MultiSelection);
          for(int i=1;i<6;++i)
              mainWid.addItem("Item "+ QString::number(i));
          const QModelIndex allIdx = mainWid.model()->index(0,0);
          QObject::connect(mainWid.selectionModel(),&QItemSelectionModel::selectionChanged,&mainWid,[&mainWid,&allIdx](const QItemSelection &selected){
              if(selected.isEmpty())
                  return;
              if(selected.contains(allIdx)){
      // if the changed selection contains the allItem we clear all existing selected items and re-select just the allItem
                  mainWid.selectionModel()->select(allIdx,QItemSelectionModel::ClearAndSelect);
                  return;
              }
              if(mainWid.selectionModel()->isSelected(allIdx))
      //otoh if any other item is selected the allItem is automatically deselected
                  mainWid.selectionModel()->select(allIdx,QItemSelectionModel::Deselect);
          });
          mainWid.show();
          return app.exec();
      }
      

      P.S.
      Reentrancy has nothing to do with the topic

      "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
      ~Napoleon Bonaparte

      On a crusade to banish setIndexWidget() from the holy land of Qt

      J 1 Reply Last reply 19 Feb 2019, 13:14
      4
      • V VRonin
        19 Feb 2019, 13:05

        I think you are kinda of overcomplicating the problem... selectionChanged already has all the information it needs not to continuously recurse. see the example below:

        #include <QApplication>
        #include <QListWidget>
        #include <QDebug>
        #include <QItemSelection>
        #include <QItemSelectionModel>
        
        int main(int argc, char ** argv)
        {
            QApplication app(argc,argv);
            QListWidget mainWid;
            mainWid.addItem("All");
            mainWid.setSelectionMode(QAbstractItemView::MultiSelection);
            for(int i=1;i<6;++i)
                mainWid.addItem("Item "+ QString::number(i));
            const QModelIndex allIdx = mainWid.model()->index(0,0);
            QObject::connect(mainWid.selectionModel(),&QItemSelectionModel::selectionChanged,&mainWid,[&mainWid,&allIdx](const QItemSelection &selected){
                if(selected.isEmpty())
                    return;
                if(selected.contains(allIdx)){
        // if the changed selection contains the allItem we clear all existing selected items and re-select just the allItem
                    mainWid.selectionModel()->select(allIdx,QItemSelectionModel::ClearAndSelect);
                    return;
                }
                if(mainWid.selectionModel()->isSelected(allIdx))
        //otoh if any other item is selected the allItem is automatically deselected
                    mainWid.selectionModel()->select(allIdx,QItemSelectionModel::Deselect);
            });
            mainWid.show();
            return app.exec();
        }
        

        P.S.
        Reentrancy has nothing to do with the topic

        J Online
        J Online
        JonB
        wrote on 19 Feb 2019, 13:14 last edited by JonB
        #3

        @VRonin

        P.S.
        Reentrancy has nothing to do with the topic

        You may have picked a definition of "re-entrancy" which is specifically to do with threads, that's not what I meant! I am talking about: a signal calls my slot, and my slot is in danger of doing something which will cause a new signal calling the same slot to be re-raised. That may then cause unending recursion. I don't really mind what terminology we use!

        I will now look at your code and try implementing. I didn't know about QItemSelectionModel::ClearAndSelect. But doesn't doing that cause a new "selection changed" signal to be raised from my slot, which will call my slot again? I'll have to see, I guess, using the debugger....

        V 1 Reply Last reply 19 Feb 2019, 13:19
        0
        • J JonB
          19 Feb 2019, 13:14

          @VRonin

          P.S.
          Reentrancy has nothing to do with the topic

          You may have picked a definition of "re-entrancy" which is specifically to do with threads, that's not what I meant! I am talking about: a signal calls my slot, and my slot is in danger of doing something which will cause a new signal calling the same slot to be re-raised. That may then cause unending recursion. I don't really mind what terminology we use!

          I will now look at your code and try implementing. I didn't know about QItemSelectionModel::ClearAndSelect. But doesn't doing that cause a new "selection changed" signal to be raised from my slot, which will call my slot again? I'll have to see, I guess, using the debugger....

          V Offline
          V Offline
          VRonin
          wrote on 19 Feb 2019, 13:19 last edited by VRonin
          #4

          @JonB said in Neatest way to avoid re-entrant selectionChanged signal:

          But doesn't doing that cause a new "selection changed" signal to be raised from my slot, which will call my slot again?

          You have 5 cases:

          • Nothing is selected, you select "All":
            • The new selection coming from ClearAndSelect is identical to the current one so no selectionChanged signal is emitted
          • Nothing is selected, you select an item:
          • An item is selected and you select another item:
            • All the conditions in the ifs are false and nothing happens in the lambda
          • An item is selected and you select "All":
            • ClearAndSelect causes selectionChanged to fire again but since you are not selecting any new index ("All" was already selected) selected will be empty and the first if takes care of stopping
          • "All" is selected and you select another item:
            • Deselect will cause selectionChanged to fire again but, as above, selected will be empty and the first if takes care of stopping

          "La mort n'est rien, mais vivre vaincu et sans gloire, c'est mourir tous les jours"
          ~Napoleon Bonaparte

          On a crusade to banish setIndexWidget() from the holy land of Qt

          J 1 Reply Last reply 19 Feb 2019, 13:53
          5
          • V VRonin
            19 Feb 2019, 13:19

            @JonB said in Neatest way to avoid re-entrant selectionChanged signal:

            But doesn't doing that cause a new "selection changed" signal to be raised from my slot, which will call my slot again?

            You have 5 cases:

            • Nothing is selected, you select "All":
              • The new selection coming from ClearAndSelect is identical to the current one so no selectionChanged signal is emitted
            • Nothing is selected, you select an item:
            • An item is selected and you select another item:
              • All the conditions in the ifs are false and nothing happens in the lambda
            • An item is selected and you select "All":
              • ClearAndSelect causes selectionChanged to fire again but since you are not selecting any new index ("All" was already selected) selected will be empty and the first if takes care of stopping
            • "All" is selected and you select another item:
              • Deselect will cause selectionChanged to fire again but, as above, selected will be empty and the first if takes care of stopping
            J Online
            J Online
            JonB
            wrote on 19 Feb 2019, 13:53 last edited by
            #5

            @VRonin
            Only 5? ;-)

            Yep, thanks. Now changed over. Each click now still causes the slot to be called, and from within the slot the new selection calls it to be called recursively a second time. But as you say, the logic means that the recursive call will do nothing further. This still "concerns" me, in case someone in future changes what the code does without realising the recursive implications and hangs/crashes the app, but that's programming for you!

            At any rate, it seems much nicer to me than passing lambdas around and disconnecting signals, I trust you feel the same, so thanks.

            1 Reply Last reply
            2

            1/5

            19 Feb 2019, 10:18

            • Login

            • Login or register to search.
            1 out of 5
            • First post
              1/5
              Last post
            0
            • Categories
            • Recent
            • Tags
            • Popular
            • Users
            • Groups
            • Search
            • Get Qt Extensions
            • Unsolved