Fix selection segfault (#136)

* fix segfault when user attempts to move bits

When a user attempts to select and move both bits and lines, the program segfaults. This occurs when the bit objects that are referenced by the selection are being deleted. This solution resolves the issue by first removing the bits from the selection before deleting them.

* remove extra call to clearBits

There is no need for clearBits to be called when the right mouse button is pressed. The only action tied with the right mouse button that requires a call to clearBits is moveLines, which calls clearBits already.

Author: jbeels-ccsw

maskromtool.cpp

@@ -1753,7 +1753,7 @@ void MaskRomTool::moveLine(RomLineItem* line, QPointF newpoint){
      * are not shown.
      *
      * Group selections are not moved with this function, but instead
-     * use moveList().
+     * use moveLines().
      */
     if(bitsVisible){
         removeLine(line,false);  //Remove the line's bits, but not the line itself.
@@ -1770,16 +1770,17 @@ void MaskRomTool::moveLine(RomLineItem* line, QPointF newpoint){
 }
 
 //Moves a group of items by an offset, used while dragging.
-void MaskRomTool::moveList(QList<QGraphicsItem*> list, QPointF offset){
+void MaskRomTool::moveLines(QList<QGraphicsItem*> &list, QPointF offset){
     /* This function's job is to move a list of items by an offset.
      * It's called when you hit the arrow keys or drag with the right
      * mouse button, and it's vital for it to be fast because there's
      * the potential for redrawing a ton of bits on every frame.
      *
      * This implementation drops all bits that are not actively on
-     * moving lines as a necessary performance hack.  The missing bits
-     * are not on moving lines, and they will return as soon as the mouse
-     * button is released.
+     * moving lines as a necessary performance hack. This includes
+     * dropping bits from the selection. The missing bits are not on
+     * moving lines, and they will return as soon as the mouse button
+     * is released.
      *
      * As a workaround when movements are too slow, use the TAB key
      * to hide the bits for a while.
@@ -1957,7 +1958,11 @@ void MaskRomTool::clearBits(bool full){
 
     //This avoids a deep copy, but cannot be partial.
     for(auto i=bits.constBegin(); i!=bits.constEnd(); i++){
+        // The bits must first be removed from the selection
+        scene->selection.removeOne(*i);
+        // Then the scene
         scene->removeItem(*i);
+        // And now they can be deleted
         delete *i;
         bitcount--;
     }

maskromtool.h

@@ -128,7 +128,7 @@ class MaskRomTool : public QMainWindow{
     //Moves a line to a new location.
     void moveLine(RomLineItem* line, QPointF newpoint);
     //Moves a group of items by an offset.
-    void moveList(QList<QGraphicsItem*> list, QPointF offset);
+    void moveLines(QList<QGraphicsItem*> &list, QPointF offset);
     //Inserts a new line, either row or column.
     bool insertLine(RomLineItem* line);
     //Is a point visible?  Handy when dragging.

romscene.cpp

@@ -41,7 +41,7 @@ void RomScene::keyPressEvent(QKeyEvent *event){
         case Qt::Key_Left: dpos.setX(-1); break;
         case Qt::Key_Right: dpos.setX(1); break;
         }
-        maskRomTool->moveList(selection, dpos);
+        maskRomTool->moveLines(selection, dpos);
         break;
     default:
         QGraphicsScene::keyPressEvent(event);
@@ -150,7 +150,8 @@ void RomScene::mouseMoveEvent(QGraphicsSceneMouseEvent *mouseEvent){
     //here instead of on release so we can have preview
     if(mouseEvent->buttons()==Qt::RightButton){
         QPointF dpos = mouseEvent->scenePos() - presspos;
-        maskRomTool->moveList(selection, dpos);
+        
+        maskRomTool->moveLines(selection, dpos);
 
         // update because we already moved it
         presspos = scenepos;
@@ -194,7 +195,6 @@ void RomScene::mousePressEvent(QGraphicsSceneMouseEvent *mouseEvent){
     if(mouseEvent->buttons()==Qt::RightButton){
         maskRomTool->dragging=true;
         maskRomTool->markUndoPoint();
-        maskRomTool->clearBits(false);
     }
 }