[kgraphviewer-devel] [KGraphViewer/libkgraphviz] 0bd155e: Fix item selection in graphics view

Kevin Funk krf at electrostorm.net
Wed Jan 12 17:35:20 CET 2011


commit 0bd155e3f58f598e6115884c6f987655ae5ffec9
branch libkgraphviz
Author: Kevin Funk <krf at electrostorm.net>
Date:   Wed Jan 12 16:12:09 2011 +0100

    Fix item selection in graphics view
    
    Drop GraphElement::m_selected as it is not needed, QAbstractGraphicsItem
    covers that. It is not necessary for the graph model to know about selected
    items, only for the view.
    Also remove redundant logic about selecting multiple elements in the
    view, QGraphicsView is already capable of this.
    
    This commit fixes item (un)selection and multiple selection logic

diff --git a/src/kgraphviz/canvasedge.cpp b/src/kgraphviz/canvasedge.cpp
index 67109c4..11bc539 100644
--- a/src/kgraphviz/canvasedge.cpp
+++ b/src/kgraphviz/canvasedge.cpp
@@ -410,7 +410,7 @@ void CanvasEdge::paint(QPainter* p, const QStyleOptionGraphicsItem* option,
       }
     }
   }
-  if (edge()->isSelected())
+  if (isSelected())
   {
 //     kDebug() << "draw square";
 //     p->drawRect(m_boundingRect);
diff --git a/src/kgraphviz/canvaselement.cpp b/src/kgraphviz/canvaselement.cpp
index 2f94482..3cc8870 100644
--- a/src/kgraphviz/canvaselement.cpp
+++ b/src/kgraphviz/canvaselement.cpp
@@ -562,7 +562,7 @@ QWidget *widget)
       p->restore();
     }
   }
-  if (element()->isSelected())
+  if (isSelected())
   {
 //     kDebug() << "element is selected: draw selection marks";
     p->save();
@@ -587,38 +587,17 @@ void CanvasElement::mousePressEvent(QGraphicsSceneMouseEvent* event)
 {
   Q_D(CanvasElement);
   kDebug() << d->m_element->id() << boundingRect();
-  if (d->m_view->isReadOnly())
-  {
+
+  if (event->button() != Qt::LeftButton)
     return;
-  }
+
   if (d->m_view->editingMode() == DotGraphView::AddNewEdge)
   {
     d->m_view->createNewEdgeDraftFrom(this);
-    return;
   }
   else if (d->m_view->editingMode() == DotGraphView::DrawNewEdge)
   {
     d->m_view->finishNewEdgeTo(this);
-    return;
-  }
-
-  if (event->button() == Qt::LeftButton)
-  {
-    d->m_element->setSelected(!d->m_element->isSelected());
-    if (d->m_element->isSelected())
-    {
-      kDebug() << "Element selected:" << d->m_element->id() << d->m_element;
-    }
-    update();
-  }
-  else if (event->button() == Qt::RightButton)
-  {
-    // opens the selected edge contextual menu and if necessary select the edge
-    if (!d->m_element->isSelected())
-    {
-      d->m_element->setSelected(true);
-      update();
-    }
   }
 }
 
diff --git a/src/kgraphviz/dotgraphview.cpp b/src/kgraphviz/dotgraphview.cpp
index 645f159..a3cae62 100644
--- a/src/kgraphviz/dotgraphview.cpp
+++ b/src/kgraphviz/dotgraphview.cpp
@@ -84,11 +84,9 @@ using namespace KGraphViz;
 
 DotGraphViewPrivate::DotGraphViewPrivate(KActionCollection* actions, DotGraphView* parent) :
   q_ptr( parent ),
-  m_labelViews(),
   m_popup(0),
   m_zoom(1),
   m_isMoving(false),
-  m_exporter(),
   m_zoomPosition(DEFAULT_ZOOMPOS),
   m_lastAutoPosition(DotGraphView::TopLeft),
   m_graph(0),
@@ -104,8 +102,6 @@ DotGraphViewPrivate::DotGraphViewPrivate(KActionCollection* actions, DotGraphVie
   m_readOnly(true),
   m_leavedTimer(std::numeric_limits<int>::max()),
   m_highlighting(false),
-  m_loadThread(),
-  m_layoutThread(),
   m_backgroundColor(QColor("white"))
 {
 }
@@ -475,6 +471,18 @@ void DotGraphViewPrivate::exportToImage()
   }
 }
 
+void DotGraphViewPrivate::selectionChanged()
+{
+  Q_Q(DotGraphView);
+  kDebug();
+  QList<QString> selection;
+  foreach(QGraphicsItem* item, q->scene()->selectedItems()) {
+    CanvasElement* ce = dynamic_cast<CanvasElement*>(item);
+    if (ce)
+      selection << ce->element()->id();
+  }
+  emit q->selectionIs(selection, QPoint());
+}
 
 //
 // DotGraphView
@@ -617,6 +625,8 @@ void DotGraphViewPrivate::setupCanvas()
   m_birdEyeView->setScene(newCanvas);
   
   q->setScene(newCanvas);
+  q->connect(newCanvas, SIGNAL(selectionChanged()), SLOT(selectionChanged()));
+  
   q->centerOn(m_textItem);
 
   m_cvZoom = 0;
@@ -819,9 +829,11 @@ bool DotGraphView::displayGraph()
   centerOn(scene()->sceneRect().center());
 
   // hide text item again
-  scene()->removeItem(d->m_textItem);
-  delete d->m_textItem;
-  d->m_textItem = 0;
+  if (d->m_textItem) {
+    scene()->removeItem(d->m_textItem);
+    delete d->m_textItem;
+    d->m_textItem = 0;
+  }
 
   viewport()->setUpdatesEnabled(true);
   QSet<QGraphicsSimpleTextItem*>::iterator labelViewsIt, labelViewsIt_end;
@@ -1018,8 +1030,6 @@ void DotGraphView::mousePressEvent(QMouseEvent* e)
     return;
   }
   kDebug() << e << d->m_editingMode;
-  QGraphicsView::mousePressEvent(e);
-
   if (d->m_editingMode == AddNewElement)
   {
     double scaleX = 1.0, scaleY = 1.0;
@@ -1065,9 +1075,9 @@ void DotGraphView::mousePressEvent(QMouseEvent* e)
   else if (d->m_editingMode == SelectingElements)
   {
   }
-  else
-  {
-    if (d->m_editingMode != None && itemAt(e->pos()) == 0) // click outside any item: unselect all
+  else {
+    CanvasElement* topMostItem = dynamic_cast<CanvasElement*>(itemAt(e->pos()));
+    if (!topMostItem) // click outside any item: QGraphicsView unselects all
     {
       if (d->m_editingMode == DrawNewEdge) // was drawing an edge; cancel it
       {
@@ -1085,33 +1095,14 @@ void DotGraphView::mousePressEvent(QMouseEvent* e)
       {
         d->m_editingMode = None;
       }
-      foreach(GraphEdge* e, d->m_graph->edges())
-      {
-        if (e->isSelected()) {
-          e->setSelected(false);
-          e->canvasElement()->update();
-        }
-      }
-      foreach(GraphNode* n, d->m_graph->nodes())
-      {
-        if (n->isSelected()) {
-          n->setSelected(false);
-          n->canvasElement()->update();
-        }
-      }
-      foreach(GraphSubgraph* s, d->m_graph->subgraphs())
-      {
-        if (s->isSelected()) {
-          s->setSelected(false);
-          s->canvasElement()->update();
-        }
-      }
-      emit selectionIs(QList<QString>(),QPoint());
     }
-    d->m_pressPos = e->globalPos();
-    d->m_pressScrollBarsPos = QPoint(horizontalScrollBar()->value(), verticalScrollBar()->value());
   }
+
+  d->m_pressPos = e->globalPos();
+  d->m_pressScrollBarsPos = QPoint(horizontalScrollBar()->value(), verticalScrollBar()->value());
   d->m_isMoving = true;
+
+  QGraphicsView::mousePressEvent(e);
 }
 
 void DotGraphView::mouseMoveEvent(QMouseEvent* e)
@@ -1161,10 +1152,8 @@ void DotGraphView::mouseReleaseEvent(QMouseEvent* e)
     kDebug() << "Stopping selection" << scene();
     QList<QGraphicsItem *> items = scene()->selectedItems();
     QList<QString> selection;
-    foreach (QGraphicsItem * item, items)
-    {
+    foreach (QGraphicsItem * item, items) {
       CanvasElement* element = dynamic_cast<CanvasElement*>(item);
-      element->element()->setSelected(true);
       if (element != 0)
       {
         selection.push_back(element->element()->id());
@@ -1173,11 +1162,6 @@ void DotGraphView::mouseReleaseEvent(QMouseEvent* e)
     d->m_editingMode = None;
     unsetCursor();
     setDragMode(NoDrag);
-    if (!selection.isEmpty())
-    {
-      update();
-      emit selectionIs(selection, mapToGlobal( e->pos() ));
-    }
   }
   else
   {
@@ -1444,7 +1428,7 @@ void DotGraphView::slotUpdate()
 void DotGraphView::prepareAddNewElement(QMap<QString,QString> attribs)
 {
   Q_D(DotGraphView);
-  kDebug() ;
+  kDebug();
   d->m_editingMode = AddNewElement;
   d->m_newElementAttributes = attribs;
   unsetCursor();
@@ -1458,7 +1442,7 @@ void DotGraphView::prepareAddNewEdge(QMap<QString,QString> attribs)
   bool anySelected = false;
   foreach (GraphEdge* edge, d->m_graph->edges())
   {
-    if (edge->isSelected())
+    if (edge->canvasElement()->isSelected())
     {
       anySelected = true;
       QMap<QString,QString>::const_iterator it = attribs.constBegin();
@@ -1574,70 +1558,13 @@ void DotGraphView::finishNewEdgeTo(CanvasElement* node)
 //   emit newEdgeAdded(gedge->fromNode()->id(),gedge->toNode()->id());
 // }
 
-void DotGraphView::slotElementSelected(CanvasElement* element, Qt::KeyboardModifiers modifiers)
-{
-  Q_D(DotGraphView);
-  kDebug() << "Element:" << element->element()->id() << element->element();
-  QList<QString> selection;
-  selection.push_back(element->element()->id());
-  if (!modifiers.testFlag(Qt::ControlModifier))
-  {
-    foreach(GraphEdge* e, d->m_graph->edges())
-    {
-      if (e->canvasElement() != element)
-      {
-        if (e->isSelected()) {
-          e->setSelected(false);
-          e->canvasElement()->update();
-        }
-      }
-    }
-    foreach(GraphNode* e, d->m_graph->nodes())
-    {
-      if (e->canvasElement() != element)
-      {
-        if (e->isSelected()) {
-          e->setSelected(false);
-          e->canvasElement()->update();
-        }
-      }
-    }
-    foreach(GraphSubgraph* s, d->m_graph->subgraphs())
-    {
-      s->setElementSelected(element->element(), true, true);
-    }
-  }
-  else
-  {
-    foreach(GraphEdge* e, d->m_graph->edges())
-    {
-      if (e->isSelected())
-      {
-        selection.push_back(e->id());
-      }
-    }
-    foreach(GraphNode* n, d->m_graph->nodes())
-    {
-      if (n->isSelected())
-      {
-        selection.push_back(n->id());
-      }
-    }
-    foreach(GraphSubgraph* s, d->m_graph->subgraphs())
-    {
-      s->retrieveSelectedElementsIds(selection);
-    }
-  }
-  emit selectionIs(selection, QPoint());
-}
-
 void DotGraphView::removeSelectedEdges()
 {
   Q_D(DotGraphView);
   kDebug();
   foreach(GraphEdge* e, d->m_graph->edges())
   {
-    if (e->isSelected())
+    if (e->canvasElement()->isSelected())
     {
       kDebug() << "emiting removeEdge " << e->id();
       d->m_graph->removeEdge(e->id());
@@ -1652,7 +1579,7 @@ void DotGraphView::removeSelectedNodes()
   kDebug();
   foreach(GraphNode* e, d->m_graph->nodes())
   {
-    if (e->isSelected())
+    if (e->canvasElement()->isSelected())
     {
       kDebug() << "emiting removeElement " << e->id();
       d->m_graph->removeElement(e->id());
@@ -1667,7 +1594,7 @@ void DotGraphView::removeSelectedSubgraphs()
   kDebug();
   foreach(GraphSubgraph* e, d->m_graph->subgraphs())
   {
-    if (e->isSelected())
+    if (e->canvasElement()->isSelected())
     {
       kDebug() << "emiting removeElement " << e->id();
       d->m_graph->removeElement(e->id());
@@ -1773,21 +1700,22 @@ void DotGraphView::slotSelectNode(const QString& nodeName)
 {
   kDebug() << nodeName;
   GraphNode* node = dynamic_cast<GraphNode*>(graph()->elementNamed(nodeName));
-  if (node == 0) return;
-  node->setSelected(true);
-  if (node->canvasElement()!=0)
-  {
-    node->canvasElement()->modelChanged();
-    slotElementSelected(node->canvasElement(),Qt::NoModifier);
+  if (!node)
+    return;
+
+  CanvasElement* canvasElement = node->canvasElement();
+  if (canvasElement !=0) {
+    canvasElement->setSelected(true);
   }
 }
 
 void DotGraphView::centerOnNode(const QString& nodeId)
 {
   GraphNode* node = dynamic_cast<GraphNode*>(graph()->elementNamed(nodeId));
-  if (node == 0) return;
-  if (node->canvasElement()!=0)
-  {
+  if (!node)
+    return;
+
+  if (node->canvasElement()) {
     centerOn(node->canvasElement());
   }
 }
diff --git a/src/kgraphviz/dotgraphview.h b/src/kgraphviz/dotgraphview.h
index 91a3c26..27ddce7 100644
--- a/src/kgraphviz/dotgraphview.h
+++ b/src/kgraphviz/dotgraphview.h
@@ -125,6 +125,7 @@ public Q_SLOTS:
   void zoomRectMovedTo(QPointF newZoomPos);
   void zoomRectMoveFinished();
   bool reload();
+  bool displayGraph();
   void dirty(const QString& dotFileName);
   void pageSetup();
   void initEmpty();
@@ -143,8 +144,6 @@ public Q_SLOTS:
   void slotBevBottomRight();
   void slotBevAutomatic();
   void slotUpdate();
-  bool displayGraph();
-  void slotElementSelected(CanvasElement*, Qt::KeyboardModifiers);
   void slotSelectionChanged();
   void slotContextMenuEvent(const QString&, const QPoint&);
   void slotElementHoverEnter(CanvasElement*);
@@ -199,6 +198,8 @@ private Q_SLOTS:
 
 private:
   Q_DECLARE_PRIVATE(DotGraphView);
+
+  Q_PRIVATE_SLOT(d_func(), void selectionChanged());
 };
 
 }
diff --git a/src/kgraphviz/dotgraphview_p.h b/src/kgraphviz/dotgraphview_p.h
index b0afaf0..20cd91b 100644
--- a/src/kgraphviz/dotgraphview_p.h
+++ b/src/kgraphviz/dotgraphview_p.h
@@ -54,6 +54,8 @@ public:
   KActionCollection* actionCollection() {return m_actions;}
   int displaySubgraph(GraphSubgraph* gsubgraph, int zValue, CanvasElement* parent = 0);
 
+  void selectionChanged(); // private slot
+
   DotGraphView * const q_ptr;
   Q_DECLARE_PUBLIC(DotGraphView);
 
diff --git a/src/kgraphviz/graphelement.cpp b/src/kgraphviz/graphelement.cpp
index 5b056b5..493c300 100644
--- a/src/kgraphviz/graphelement.cpp
+++ b/src/kgraphviz/graphelement.cpp
@@ -33,7 +33,6 @@ namespace KGraphViz
 {
 
 GraphElementPrivate::GraphElementPrivate() :
-  m_selected(false),
   m_z(1.0),
   m_canvasElement(0)
 {
@@ -44,7 +43,6 @@ GraphElementPrivate::~GraphElementPrivate()
 }
 
 GraphElementPrivate::GraphElementPrivate(const GraphElementPrivate& other) :
-  m_selected(other.m_selected),
   m_z(other.m_z),
   m_renderOperations(other.m_renderOperations),
   m_canvasElement(other.m_canvasElement)
@@ -95,18 +93,6 @@ void GraphElement::setCanvasElement(CanvasElement* canvasElement)
   d->m_canvasElement = QSharedPointer<CanvasElement>(canvasElement);
 }
 
-bool GraphElement::isSelected() const
-{
-  Q_D(const GraphElement);
-  return d->m_selected;
-}
-
-void GraphElement::setSelected(bool selected)
-{
-  Q_D(GraphElement);
-  d->m_selected = selected;
-}
-
 double GraphElement::z() const
 {
   Q_D(const GraphElement);
@@ -231,7 +217,7 @@ void GraphElement::exportToGraphviz(void* element) const
       }
       else if (originalAttributes().isEmpty() || originalAttributes().contains(it.key()))
       {
-        //         kDebug() << it.key() << it.value();
+        kDebug() << it.key() << it.value();
         
         agsafeset(element, it.key().toUtf8().data(), it.value().toUtf8().data(), QString().toUtf8().data());
       }
diff --git a/src/kgraphviz/graphelement.h b/src/kgraphviz/graphelement.h
index 4ed3f58..b5a9dd9 100644
--- a/src/kgraphviz/graphelement.h
+++ b/src/kgraphviz/graphelement.h
@@ -100,9 +100,6 @@ public:
   double z() const;
   void setZ(double z);
 
-  void setSelected(bool selected);
-  bool isSelected() const;
-
   void exportToGraphviz(void* element)  const;
 
 Q_SIGNALS:
diff --git a/src/kgraphviz/graphelement_p.h b/src/kgraphviz/graphelement_p.h
index e9df701..9b5c4d6 100644
--- a/src/kgraphviz/graphelement_p.h
+++ b/src/kgraphviz/graphelement_p.h
@@ -37,7 +37,6 @@ public:
   
   ~GraphElementPrivate();
 
-  bool m_selected;
   double m_z;
 
   DotRenderOpVec m_renderOperations;
diff --git a/src/kgraphviz/graphsubgraph.cpp b/src/kgraphviz/graphsubgraph.cpp
index 8ace9bc..757af4a 100644
--- a/src/kgraphviz/graphsubgraph.cpp
+++ b/src/kgraphviz/graphsubgraph.cpp
@@ -225,16 +225,16 @@ bool GraphSubgraph::setElementSelected(
   bool res = false;
   if (element == this)
   {
-    if (isSelected() != selectValue)
+    if (canvasElement()->isSelected() != selectValue)
     {
-      setSelected(selectValue);
+      canvasElement()->setSelected(selectValue);
       canvasElement()->update();
     }
     res = true;
   }
-  else if (isSelected() && unselectOthers)
+  else if (canvasElement()->isSelected() && unselectOthers)
   {
-    setSelected(false);
+    canvasElement()->setSelected(false);
     canvasElement()->update();
   }
   foreach (GraphElement* el, content())
@@ -247,17 +247,17 @@ bool GraphSubgraph::setElementSelected(
     else if (element == el)
     {
       res = true;
-      if (el->isSelected() != selectValue)
+      if (el->canvasElement()->isSelected() != selectValue)
       {
-        el->setSelected(selectValue);
+        el->canvasElement()->setSelected(selectValue);
         el->canvasElement()->update();
       }
     }
     else 
     {
-      if (unselectOthers && el->isSelected())
+      if (unselectOthers && el->canvasElement()->isSelected())
       {
-        el->setSelected(false);
+        el->canvasElement()->setSelected(false);
         el->canvasElement()->update();
       }
     }
@@ -267,7 +267,7 @@ bool GraphSubgraph::setElementSelected(
 
 void GraphSubgraph::retrieveSelectedElementsIds(QList<QString> selection)
 {
-  if (isSelected())
+  if (canvasElement()->isSelected())
   {
     selection.push_back(id());
   }
@@ -277,7 +277,7 @@ void GraphSubgraph::retrieveSelectedElementsIds(QList<QString> selection)
     {
       dynamic_cast<GraphSubgraph*>(el)->retrieveSelectedElementsIds(selection);
     }
-    else  if (el->isSelected())
+    else if (el->canvasElement()->isSelected())
     {
       selection.push_back(el->id());
     }


More information about the kgraphviewer-devel mailing list