From 990458d187c109aca0fc2359484b16f040329c17 Mon Sep 17 00:00:00 2001 From: Aleksei Kanash Date: Thu, 30 Aug 2012 18:30:49 +0300 Subject: [PATCH 1/5] Removed memory leaks at deletion of UBGraphicsGroupContainerItem. Corrected adding items to group. Corrected group creation. Corrected adding items to scene - items shouldn't be added to itemsToRemove list because that list needs for cleaning undo stack. Improved mechanism of removing "itemsToRemove". Removed wrong deletion of mButtons list from UBGraphicsItemDelegate destructor because that buttons has Frame as parent. Removed possible memory leak in UBBoardController::clearUndoStack. Some items was able not deleted. tmpScene in cffReader not deletes now because ithas a proxy as parent. --- src/adaptors/UBCFFSubsetAdaptor.cpp | 2 -- src/board/UBBoardController.cpp | 3 +- src/domain/UBGraphicsGroupContainerItem.cpp | 33 ++++++++++----------- src/domain/UBGraphicsGroupContainerItem.h | 3 +- src/domain/UBGraphicsItemDelegate.cpp | 3 +- src/domain/UBGraphicsScene.cpp | 2 +- src/frameworks/UBCoreGraphicsScene.cpp | 27 ++++++++++------- 7 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/adaptors/UBCFFSubsetAdaptor.cpp b/src/adaptors/UBCFFSubsetAdaptor.cpp index 7eb667b5..097540a7 100644 --- a/src/adaptors/UBCFFSubsetAdaptor.cpp +++ b/src/adaptors/UBCFFSubsetAdaptor.cpp @@ -1189,8 +1189,6 @@ bool UBCFFSubsetAdaptor::UBCFFSubsetReader::persistScenes() UBGraphicsScene *tmpScene = UBSvgSubsetAdaptor::loadScene(mProxy, i); tmpScene->setModified(true); UBThumbnailAdaptor::persistScene(mProxy, tmpScene, i); - delete tmpScene; - mCurrentScene->setModified(false); } diff --git a/src/board/UBBoardController.cpp b/src/board/UBBoardController.cpp index bcfc1ddd..d2397285 100644 --- a/src/board/UBBoardController.cpp +++ b/src/board/UBBoardController.cpp @@ -1515,7 +1515,8 @@ void UBBoardController::ClearUndoStack() } if(!scene) { - mActiveScene->deleteItem(item); + if (!mActiveScene->deleteItem(item)) + delete item; } } diff --git a/src/domain/UBGraphicsGroupContainerItem.cpp b/src/domain/UBGraphicsGroupContainerItem.cpp index bed3b219..ff4a8bc4 100644 --- a/src/domain/UBGraphicsGroupContainerItem.cpp +++ b/src/domain/UBGraphicsGroupContainerItem.cpp @@ -28,18 +28,12 @@ UBGraphicsGroupContainerItem::UBGraphicsGroupContainerItem(QGraphicsItem *parent setUuid(QUuid::createUuid()); setData(UBGraphicsItemData::itemLayerType, QVariant(itemLayerType::ObjectItem)); //Necessary to set if we want z value to be assigned correctly - - } UBGraphicsGroupContainerItem::~UBGraphicsGroupContainerItem() { - foreach (QGraphicsItem *item, childItems()) - { - removeFromGroup(item); - if (item && item->scene()) - item->scene()->removeItem(item); - } + if (mDelegate) + delete mDelegate; } void UBGraphicsGroupContainerItem::addToGroup(QGraphicsItem *item) @@ -83,6 +77,8 @@ void UBGraphicsGroupContainerItem::addToGroup(QGraphicsItem *item) QTransform newItemTransform(itemTransform); item->setPos(mapFromItem(item, 0, 0)); + + item->scene()->removeItem(item); item->setParentItem(this); // removing position from translation component of the new transform @@ -113,10 +109,12 @@ void UBGraphicsGroupContainerItem::removeFromGroup(QGraphicsItem *item) { if (!item) { qDebug() << "can't specify the item because of the null pointer"; + return; } - UBGraphicsScene *groupScene = scene(); - if (groupScene) { + UBCoreGraphicsScene *groupScene = corescene(); + if (groupScene) + { groupScene->addItemToDeletion(item); } @@ -170,9 +168,9 @@ void UBGraphicsGroupContainerItem::paint(QPainter *painter, const QStyleOptionGr // } } -UBGraphicsScene *UBGraphicsGroupContainerItem::scene() +UBCoreGraphicsScene *UBGraphicsGroupContainerItem::corescene() { - UBGraphicsScene *castScene = dynamic_cast(QGraphicsItem::scene()); + UBCoreGraphicsScene *castScene = dynamic_cast(QGraphicsItem::scene()); return castScene; } @@ -218,15 +216,14 @@ void UBGraphicsGroupContainerItem::setUuid(const QUuid &pUuid) void UBGraphicsGroupContainerItem::destroy() { - UBGraphicsScene *groupScene = scene(); + UBCoreGraphicsScene *groupScene = corescene(); - foreach (QGraphicsItem *item, childItems()) { - - if (groupScene) { - groupScene->addItemToDeletion(item); - } + if (groupScene) { + // groupScene->addItemToDeletion(this); + } + foreach (QGraphicsItem *item, childItems()) { pRemoveFromGroup(item); item->setFlag(QGraphicsItem::ItemIsSelectable, true); item->setFlag(QGraphicsItem::ItemIsFocusable, true); diff --git a/src/domain/UBGraphicsGroupContainerItem.h b/src/domain/UBGraphicsGroupContainerItem.h index 535627eb..00f643d3 100644 --- a/src/domain/UBGraphicsGroupContainerItem.h +++ b/src/domain/UBGraphicsGroupContainerItem.h @@ -4,6 +4,7 @@ #include #include "domain/UBItem.h" +#include "frameworks/UBCoreGraphicsScene.h" class UBGraphicsGroupContainerItem : public QGraphicsItem, public UBItem, public UBGraphicsItem { @@ -23,7 +24,7 @@ public: virtual UBGraphicsItemDelegate* Delegate() const { return mDelegate;} - virtual UBGraphicsScene* scene(); + virtual UBCoreGraphicsScene *corescene(); virtual UBGraphicsGroupContainerItem *deepCopy() const; virtual void copyItemParameters(UBItem *copy) const; diff --git a/src/domain/UBGraphicsItemDelegate.cpp b/src/domain/UBGraphicsItemDelegate.cpp index 4abeb1a2..6c0349e4 100644 --- a/src/domain/UBGraphicsItemDelegate.cpp +++ b/src/domain/UBGraphicsItemDelegate.cpp @@ -113,7 +113,6 @@ UBGraphicsItemDelegate::UBGraphicsItemDelegate(QGraphicsItem* pDelegated, QObjec , mFlippable(false) , mToolBarUsed(useToolBar) { - // NOOP connect(UBApplication::boardController, SIGNAL(zoomChanged(qreal)), this, SLOT(onZoomChanged())); } @@ -167,7 +166,7 @@ void UBGraphicsItemDelegate::init() UBGraphicsItemDelegate::~UBGraphicsItemDelegate() { - qDeleteAll(mButtons); + disconnect(UBApplication::boardController, SIGNAL(zoomChanged(qreal)), this, SLOT(onZoomChanged())); // do not release mMimeData. // the mMimeData is owned by QDrag since the setMimeData call as specified in the documentation } diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index b4cbfc4e..f6a12f95 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -1363,6 +1363,7 @@ UBGraphicsW3CWidgetItem* UBGraphicsScene::addOEmbed(const QUrl& pContentUrl, con UBGraphicsGroupContainerItem *UBGraphicsScene::createGroup(QList items) { UBGraphicsGroupContainerItem *groupItem = new UBGraphicsGroupContainerItem(); + addItem(groupItem); foreach (QGraphicsItem *item, items) { if (item->type() == UBGraphicsGroupContainerItem::Type) { @@ -1379,7 +1380,6 @@ UBGraphicsGroupContainerItem *UBGraphicsScene::createGroup(QListsetVisible(true); groupItem->setFocus(); diff --git a/src/frameworks/UBCoreGraphicsScene.cpp b/src/frameworks/UBCoreGraphicsScene.cpp index fd4948ec..1f306425 100644 --- a/src/frameworks/UBCoreGraphicsScene.cpp +++ b/src/frameworks/UBCoreGraphicsScene.cpp @@ -30,12 +30,23 @@ UBCoreGraphicsScene::UBCoreGraphicsScene(QObject * parent) UBCoreGraphicsScene::~UBCoreGraphicsScene() { //we must delete removed items that are no more in any scene - foreach (const QGraphicsItem* item, mItemsToDelete) + //at groups deleting some items can be added to mItemsToDelete, so we need to use iterators. + if (mItemsToDelete.count()) { - if (item->scene() == NULL || item->scene() == this) + QSet::iterator it = mItemsToDelete.begin(); + QGraphicsItem* item = *it; + do { - delete item; - } + item = *it; + if (item && (item->scene() == NULL || item->scene() == this)) + { + mItemsToDelete.remove(*it); + delete item; + } + + it = mItemsToDelete.begin(); + + }while(mItemsToDelete.count()); } } @@ -46,9 +57,7 @@ void UBCoreGraphicsScene::addItem(QGraphicsItem* item) removeItemFromDeletion(curItem); } } - - mItemsToDelete << item; - + if (item->scene() != this) QGraphicsScene::addItem(item); } @@ -59,9 +68,7 @@ void UBCoreGraphicsScene::removeItem(QGraphicsItem* item, bool forceDelete) QGraphicsScene::removeItem(item); if (forceDelete) { - mItemsToDelete.remove(item); - delete item; - item = 0; + deleteItem(item); } } From 0b108d466a0f32c242166fed4d2d9cfbda3087b8 Mon Sep 17 00:00:00 2001 From: Aleksei Kanash Date: Fri, 31 Aug 2012 10:11:43 +0300 Subject: [PATCH 2/5] Fixed adding items from section to group. --- src/adaptors/UBCFFSubsetAdaptor.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/adaptors/UBCFFSubsetAdaptor.cpp b/src/adaptors/UBCFFSubsetAdaptor.cpp index 097540a7..87c83cc4 100644 --- a/src/adaptors/UBCFFSubsetAdaptor.cpp +++ b/src/adaptors/UBCFFSubsetAdaptor.cpp @@ -174,9 +174,7 @@ bool UBCFFSubsetAdaptor::UBCFFSubsetReader::parseGSection(const QDomElement &ele QDomElement currentSvgElement = element.firstChildElement(); while (!currentSvgElement.isNull()) { - if (!parseSvgElement(currentSvgElement)) - return false; - + parseSvgElement(currentSvgElement); currentSvgElement = currentSvgElement.nextSiblingElement(); } @@ -187,8 +185,8 @@ bool UBCFFSubsetAdaptor::UBCFFSubsetReader::parseGSection(const QDomElement &ele else { delete mGSectionContainer; - mGSectionContainer = NULL; } + mGSectionContainer = NULL; return true; } From 4b7939b6c3b97e10a82f5e86f5c4c93d7f0388e0 Mon Sep 17 00:00:00 2001 From: Aleksei Kanash Date: Fri, 31 Aug 2012 11:47:14 +0300 Subject: [PATCH 3/5] Grouped items doesn't manages by mFastAccessItems now. --- src/board/UBBoardController.cpp | 5 +++-- src/domain/UBGraphicsGroupContainerItem.cpp | 6 ++++++ src/domain/UBGraphicsPolygonItem.cpp | 4 ++-- src/domain/UBGraphicsScene.cpp | 11 +++++++++++ 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/board/UBBoardController.cpp b/src/board/UBBoardController.cpp index d2397285..891591d7 100644 --- a/src/board/UBBoardController.cpp +++ b/src/board/UBBoardController.cpp @@ -1482,11 +1482,12 @@ void UBBoardController::ClearUndoStack() UBGraphicsItemUndoCommand *cmd = (UBGraphicsItemUndoCommand*)UBApplication::undoStack->command(i); // go through all added and removed objects, for create list of unique objects + // grouped items will be deleted by groups, so we don't need do delete that items. QSetIterator itAdded(cmd->GetAddedList()); while (itAdded.hasNext()) { QGraphicsItem* item = itAdded.next(); - if( !uniqueItems.contains(item) ) + if( !uniqueItems.contains(item) && !(item->parentItem() && UBGraphicsGroupContainerItem::Type == item->parentItem()->type())) uniqueItems.insert(item); } @@ -1494,7 +1495,7 @@ void UBBoardController::ClearUndoStack() while (itRemoved.hasNext()) { QGraphicsItem* item = itRemoved.next(); - if( !uniqueItems.contains(item) ) + if( !uniqueItems.contains(item) && (item->parentItem() && UBGraphicsGroupContainerItem::Type != item->parentItem()->type())) uniqueItems.insert(item); } } diff --git a/src/domain/UBGraphicsGroupContainerItem.cpp b/src/domain/UBGraphicsGroupContainerItem.cpp index ff4a8bc4..1c63d004 100644 --- a/src/domain/UBGraphicsGroupContainerItem.cpp +++ b/src/domain/UBGraphicsGroupContainerItem.cpp @@ -324,6 +324,12 @@ void UBGraphicsGroupContainerItem::pRemoveFromGroup(QGraphicsItem *item) item->setParentItem(newParent); item->setPos(oldPos); + UBGraphicsScene *Scene = dynamic_cast(item->scene()); + if (Scene) + { + Scene->addItem(item); + } + // removing position from translation component of the new transform if (!item->pos().isNull()) itemTransform *= QTransform::fromTranslate(-item->x(), -item->y()); diff --git a/src/domain/UBGraphicsPolygonItem.cpp b/src/domain/UBGraphicsPolygonItem.cpp index b21f2760..8e0e6f85 100644 --- a/src/domain/UBGraphicsPolygonItem.cpp +++ b/src/domain/UBGraphicsPolygonItem.cpp @@ -67,8 +67,8 @@ void UBGraphicsPolygonItem::clearStroke() if (mStroke!=NULL) { mStroke->remove(this); - if (mStroke->polygons().empty()) - delete mStroke; + //if (mStroke->polygons().empty()) + // delete mStroke; mStroke = NULL; } } diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index f6a12f95..f5463574 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -1374,9 +1374,11 @@ UBGraphicsGroupContainerItem *UBGraphicsScene::createGroup(QListaddToGroup(chItem); + mFastAccessItems.removeAll(item); } } else { groupItem->addToGroup(item); + mFastAccessItems.removeAll(item); } } @@ -1396,6 +1398,15 @@ UBGraphicsGroupContainerItem *UBGraphicsScene::createGroup(QListchildItems().count(); i++) + { + QGraphicsItem *it = qgraphicsitem_cast(groupItem->childItems().at(i)); + if (it) + { + mFastAccessItems.removeAll(it); + } + } + groupItem->setVisible(true); groupItem->setFocus(); From 8b19847c1121b9ca8f93259b0b6c7d2067a0d91e Mon Sep 17 00:00:00 2001 From: Aleksei Kanash Date: Fri, 31 Aug 2012 12:18:50 +0300 Subject: [PATCH 4/5] Fixed deletion removed groups. Code was cleaned. Corrections in removing strokes. --- src/board/UBBoardController.cpp | 2 +- src/domain/UBGraphicsGroupContainerItem.cpp | 4 ---- src/domain/UBGraphicsPolygonItem.cpp | 4 ++-- src/domain/UBGraphicsScene.cpp | 17 +++++++++++++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/board/UBBoardController.cpp b/src/board/UBBoardController.cpp index 891591d7..3c56d617 100644 --- a/src/board/UBBoardController.cpp +++ b/src/board/UBBoardController.cpp @@ -1495,7 +1495,7 @@ void UBBoardController::ClearUndoStack() while (itRemoved.hasNext()) { QGraphicsItem* item = itRemoved.next(); - if( !uniqueItems.contains(item) && (item->parentItem() && UBGraphicsGroupContainerItem::Type != item->parentItem()->type())) + if( !uniqueItems.contains(item) && !(item->parentItem() && UBGraphicsGroupContainerItem::Type == item->parentItem()->type())) uniqueItems.insert(item); } } diff --git a/src/domain/UBGraphicsGroupContainerItem.cpp b/src/domain/UBGraphicsGroupContainerItem.cpp index 1c63d004..b3e9dec8 100644 --- a/src/domain/UBGraphicsGroupContainerItem.cpp +++ b/src/domain/UBGraphicsGroupContainerItem.cpp @@ -219,10 +219,6 @@ void UBGraphicsGroupContainerItem::destroy() { UBCoreGraphicsScene *groupScene = corescene(); - if (groupScene) { - // groupScene->addItemToDeletion(this); - } - foreach (QGraphicsItem *item, childItems()) { pRemoveFromGroup(item); item->setFlag(QGraphicsItem::ItemIsSelectable, true); diff --git a/src/domain/UBGraphicsPolygonItem.cpp b/src/domain/UBGraphicsPolygonItem.cpp index 8e0e6f85..b21f2760 100644 --- a/src/domain/UBGraphicsPolygonItem.cpp +++ b/src/domain/UBGraphicsPolygonItem.cpp @@ -67,8 +67,8 @@ void UBGraphicsPolygonItem::clearStroke() if (mStroke!=NULL) { mStroke->remove(this); - //if (mStroke->polygons().empty()) - // delete mStroke; + if (mStroke->polygons().empty()) + delete mStroke; mStroke = NULL; } } diff --git a/src/domain/UBGraphicsScene.cpp b/src/domain/UBGraphicsScene.cpp index f5463574..a982a2d1 100644 --- a/src/domain/UBGraphicsScene.cpp +++ b/src/domain/UBGraphicsScene.cpp @@ -305,7 +305,10 @@ UBGraphicsScene::~UBGraphicsScene() { if (mCurrentStroke) if (mCurrentStroke->polygons().empty()) + { delete mCurrentStroke; + mCurrentStroke = NULL; + } if (mZLayerController) delete mZLayerController; @@ -433,6 +436,11 @@ bool UBGraphicsScene::inputDevicePress(const QPointF& scenePos, const qreal& pre } } + if (mCurrentStroke && mCurrentStroke->polygons().empty()){ + delete mCurrentStroke; + mCurrentStroke = NULL; + } + return accepted; } @@ -474,6 +482,10 @@ bool UBGraphicsScene::inputDeviceMove(const QPointF& scenePos, const qreal& pres UBCoreGraphicsScene::removeItemFromDeletion(mpLastPolygon); mAddedItems.remove(mpLastPolygon); mCurrentStroke->remove(mpLastPolygon); + if (mCurrentStroke->polygons().empty()){ + delete mCurrentStroke; + mCurrentStroke = NULL; + } removeItem(mpLastPolygon); mPreviousPolygonItems.removeAll(mpLastPolygon); } @@ -2285,6 +2297,11 @@ void UBGraphicsScene::setToolCursor(int tool) { deselectAllItems(); } + + if (mCurrentStroke && mCurrentStroke->polygons().empty()){ + delete mCurrentStroke; + } + mCurrentStroke = NULL; } void UBGraphicsScene::initStroke(){ From 29ef687d41c717e35644fc0f6a976c55c63d0740 Mon Sep 17 00:00:00 2001 From: Aleksei Kanash Date: Fri, 31 Aug 2012 13:00:30 +0300 Subject: [PATCH 5/5] Useless code removed. --- src/domain/UBGraphicsGroupContainerItem.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/domain/UBGraphicsGroupContainerItem.cpp b/src/domain/UBGraphicsGroupContainerItem.cpp index b3e9dec8..10be1c4d 100644 --- a/src/domain/UBGraphicsGroupContainerItem.cpp +++ b/src/domain/UBGraphicsGroupContainerItem.cpp @@ -216,9 +216,6 @@ void UBGraphicsGroupContainerItem::setUuid(const QUuid &pUuid) void UBGraphicsGroupContainerItem::destroy() { - UBCoreGraphicsScene *groupScene = corescene(); - - foreach (QGraphicsItem *item, childItems()) { pRemoveFromGroup(item); item->setFlag(QGraphicsItem::ItemIsSelectable, true);