From a1cc69c8e3de088e442384d1e1b22f187551d4b4 Mon Sep 17 00:00:00 2001 From: Wanderer Fan Date: Thu, 18 Aug 2022 16:45:40 -0400 Subject: [PATCH] [TD]code review changes for Gui --- src/Mod/TechDraw/Gui/DrawGuiUtil.cpp | 1 - src/Mod/TechDraw/Gui/QGSPage.cpp | 58 +++++------ .../TechDraw/Gui/ViewProviderDrawingView.cpp | 99 +++++++++++-------- .../TechDraw/Gui/ViewProviderDrawingView.h | 3 + src/Mod/TechDraw/Gui/ViewProviderPage.cpp | 3 +- 5 files changed, 87 insertions(+), 77 deletions(-) diff --git a/src/Mod/TechDraw/Gui/DrawGuiUtil.cpp b/src/Mod/TechDraw/Gui/DrawGuiUtil.cpp index 7378ca6f21..a7bda9d681 100644 --- a/src/Mod/TechDraw/Gui/DrawGuiUtil.cpp +++ b/src/Mod/TechDraw/Gui/DrawGuiUtil.cpp @@ -128,7 +128,6 @@ TechDraw::DrawPage* DrawGuiUtil::findPage(Gui::Command* cmd) Gui::MDIView* mv = w->activeWindow(); MDIViewPage* mvp = dynamic_cast(mv); if (mvp) { - QString windowTitle = mvp->windowTitle(); QGSPage* qp = mvp->getViewProviderPage()->getQGSPage(); page = qp->getDrawPage(); } diff --git a/src/Mod/TechDraw/Gui/QGSPage.cpp b/src/Mod/TechDraw/Gui/QGSPage.cpp index 75256167f2..1778433a12 100644 --- a/src/Mod/TechDraw/Gui/QGSPage.cpp +++ b/src/Mod/TechDraw/Gui/QGSPage.cpp @@ -215,15 +215,14 @@ void QGSPage::updateTemplate(bool forceUpdate) QPointF QGSPage::getTemplateCenter() { - QPointF result(0.0, 0.0); App::DocumentObject *obj = m_vpPage->getDrawPage()->Template.getValue(); auto pageTemplate( dynamic_cast(obj) ); if( pageTemplate != nullptr ) { double cx = Rez::guiX(pageTemplate->Width.getValue())/2.0; double cy = -Rez::guiX(pageTemplate->Height.getValue())/2.0; - result = QPointF(cx,cy); + return QPointF(cx,cy); } - return result; + return QPointF(0.0, 0.0); } void QGSPage::matchSceneRectToTemplate(void) @@ -708,14 +707,13 @@ QGIView * QGSPage::addWeldSymbol(TechDraw::DrawWeldSymbol* weld) void QGSPage::setDimensionGroups(void) { const std::vector &allItems = getViews(); - std::vector::const_iterator itInspect; int dimItemType = QGraphicsItem::UserType + 106; - for (itInspect = allItems.begin(); itInspect != allItems.end(); itInspect++) { - if (((*itInspect)->type() == dimItemType) && (!(*itInspect)->group())) { - QGIView* parent = findParent((*itInspect)); + for (auto& item : allItems) { + if (item->type() == dimItemType && !item->group()) { + QGIView* parent = findParent(item); if (parent) { - QGIViewDimension* dim = dynamic_cast((*itInspect)); + QGIViewDimension* dim = dynamic_cast(item); addDimToParent(dim,parent); } } @@ -725,14 +723,13 @@ void QGSPage::setDimensionGroups(void) void QGSPage::setBalloonGroups(void) { const std::vector &allItems = getViews(); - std::vector::const_iterator itInspect; int balloonItemType = QGraphicsItem::UserType + 140; - for (itInspect = allItems.begin(); itInspect != allItems.end(); itInspect++) { - if (((*itInspect)->type() == balloonItemType) && (!(*itInspect)->group())) { - QGIView* parent = findParent((*itInspect)); + for (auto& item : allItems) { + if (item->type() == balloonItemType && !item->group()) { + QGIView* parent = findParent(item); if (parent) { - QGIViewBalloon* balloon = dynamic_cast((*itInspect)); + QGIViewBalloon* balloon = dynamic_cast(item); addBalloonToParent(balloon,parent); } } @@ -743,16 +740,15 @@ void QGSPage::setLeaderGroups(void) { // Base::Console().Message("QGSP::setLeaderGroups()\n"); const std::vector &allItems = getViews(); - std::vector::const_iterator itInspect; int leadItemType = QGraphicsItem::UserType + 232; //make sure that qgileader belongs to correct parent. //quite possibly redundant - for (itInspect = allItems.begin(); itInspect != allItems.end(); itInspect++) { - if (((*itInspect)->type() == leadItemType) && (!(*itInspect)->group())) { - QGIView* parent = findParent((*itInspect)); + for (auto& item : allItems) { + if (item->type() == leadItemType && !item->group()) { + QGIView* parent = findParent(item); if (parent) { - QGILeaderLine* lead = dynamic_cast((*itInspect)); + QGILeaderLine* lead = dynamic_cast(item); addLeaderToParent(lead,parent); } } @@ -763,15 +759,14 @@ void QGSPage::setRichAnnoGroups(void) { // Base::Console().Message("QGSP::setRichAnnoGroups()\n"); const std::vector &allItems = getViews(); - std::vector::const_iterator itInspect; int annoItemType = QGraphicsItem::UserType + 233; //make sure that qgirichanno belongs to correct parent. - for (itInspect = allItems.begin(); itInspect != allItems.end(); itInspect++) { - if (((*itInspect)->type() == annoItemType) && (!(*itInspect)->group())) { - QGIView* parent = findParent((*itInspect)); + for (auto& item : allItems) { + if (item->type() == annoItemType && !item->group()) { + QGIView* parent = findParent(item); if (parent) { - QGIRichAnno* anno = dynamic_cast((*itInspect)); + QGIRichAnno* anno = dynamic_cast(item); addAnnoToParent(anno,parent); } } @@ -966,12 +961,6 @@ void QGSPage::findMissingViews(const std::vector &list, st void QGSPage::fixOrphans(bool force) { Q_UNUSED(force) -// Base::Console().Message("QGSP::fixOrphans()\n"); -// if(!force) { -// m_timer->start(100); -// return; -// } -// m_timer->stop(); // get all the DrawViews for this page, including the second level ones // if we ever have collections of collections, we'll need to revisit this @@ -1003,8 +992,9 @@ void QGSPage::fixOrphans(bool force) for (auto& qv: qvs) { if (!qv) continue; // already deleted? + App::DocumentObject* obj = doc->getObject(qv->getViewName()); - if (obj == nullptr) { + if (!obj) { //no DrawView anywhere in Document removeQView(qv); } else { @@ -1016,8 +1006,8 @@ void QGSPage::fixOrphans(bool force) removeQView(qv); } else if (numParentPages == 1) { //Does DrawView belong to this DrawPage? - TechDraw::DrawPage* pp = qv->getViewObject()->findParentPage(); - if (thisPage != pp) { + TechDraw::DrawPage* parentPage = qv->getViewObject()->findParentPage(); + if (thisPage != parentPage) { //DrawView does not belong to this DrawPage //remove QGItem from QGScene removeQView(qv); @@ -1026,9 +1016,9 @@ void QGSPage::fixOrphans(bool force) //DrawView belongs to multiple DrawPages //check if this MDIViewPage corresponds to any parent DrawPage //if not, delete the QGItem - std::vector pPages = qv->getViewObject()->findAllParentPages(); + std::vector potentialParentPages = qv->getViewObject()->findAllParentPages(); bool found = false; - for (auto p: pPages) { + for (auto p: potentialParentPages) { if (thisPage == p) { found = true; break; diff --git a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp index 67d75c51a8..8cdd6df927 100644 --- a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp +++ b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.cpp @@ -150,20 +150,26 @@ void ViewProviderDrawingView::hide() QGIView* ViewProviderDrawingView::getQView() { - QGIView *qView = nullptr; TechDraw::DrawView* dv = getViewObject(); - if (dv) { - Gui::Document* guiDoc = Gui::Application::Instance->getDocument(getViewObject()->getDocument()); - if (guiDoc) { - ViewProviderPage* vpp = getViewProviderPage(); - if (vpp) { - if (vpp->getQGSPage()) { - qView = dynamic_cast(vpp->getQGSPage()->findQViewForDocObj(getViewObject())); - } - } - } + if (!dv) { + return nullptr; } - return qView; + + Gui::Document* guiDoc = Gui::Application::Instance->getDocument(dv->getDocument()); + if (!guiDoc) { + return nullptr; + } + + ViewProviderPage* vpp = getViewProviderPage(); + if (!vpp) { + return nullptr; + } + + if (vpp->getQGSPage()) { + return dynamic_cast(vpp->getQGSPage()->findQViewForDocObj(getViewObject())); + } + + return nullptr; } bool ViewProviderDrawingView::isShow() const @@ -234,37 +240,50 @@ void ViewProviderDrawingView::onGuiRepaint(const TechDraw::DrawView* dv) std::vector pages = getViewObject()->findAllParentPages(); if (pages.size() > 1) { - for (auto& p : pages) { - std::vector views = p->Views.getValues(); - for (auto& v: views) { - if (v == getViewObject()) { - //view v belongs to this page p - ViewProviderPage* vpPage = getViewProviderPage(); - if (vpPage) { - if (vpPage->getQGSPage()) { - QGIView* qView = dynamic_cast(vpPage->getQGSPage()->findQViewForDocObj(v)); - if (qView != nullptr) { - qView->updateView(true); - } - } - } + multiParentPaint(pages); + } else if (dv == getViewObject()) { + singleParentPaint(dv); + } +} + +void ViewProviderDrawingView::multiParentPaint(std::vector& pages) +{ + for (auto& p : pages) { + std::vector views = p->Views.getValues(); + for (auto& v: views) { + if (v != getViewObject()) { //should this be dv from onGuiRepaint? + continue; + } + //view v belongs to this page p + ViewProviderPage* vpPage = getViewProviderPage(); + if (!vpPage) { + continue; + } + if (vpPage->getQGSPage()) { + QGIView* qView = dynamic_cast(vpPage->getQGSPage()->findQViewForDocObj(v)); + if (qView) { + qView->updateView(true); } } } - } else if (dv == getViewObject()) { - //original logic for 1 view on 1 page - if (!dv->isRemoving() && - !dv->isRestoring()) { - QGIView* qgiv = getQView(); - if (qgiv) { - qgiv->updateView(true); - } else { //we are not part of the Gui page yet. ask page to add us. - ViewProviderPage* vpPage = getViewProviderPage(); - if (vpPage) { - if (vpPage->getQGSPage()) { - vpPage->getQGSPage()->addView(dv); - } - } + } +} + +void ViewProviderDrawingView::singleParentPaint(const TechDraw::DrawView* dv) +{ + //original logic for 1 view on 1 page + if (dv->isRemoving() || + dv->isRestoring()) { + return; + } + QGIView* qgiv = getQView(); + if (qgiv) { + qgiv->updateView(true); + } else { //we are not part of the Gui page yet. ask page to add us. + ViewProviderPage* vpPage = getViewProviderPage(); + if (vpPage) { + if (vpPage->getQGSPage()) { + vpPage->getQGSPage()->addView(dv); } } } diff --git a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h index cb1d0910a2..2acdb899d3 100644 --- a/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h +++ b/src/Mod/TechDraw/Gui/ViewProviderDrawingView.h @@ -87,6 +87,9 @@ public: Connection connectProgressMessage; private: + void multiParentPaint(std::vector& pages); + void singleParentPaint(const TechDraw::DrawView* dv); + }; diff --git a/src/Mod/TechDraw/Gui/ViewProviderPage.cpp b/src/Mod/TechDraw/Gui/ViewProviderPage.cpp index 471588d71f..e576eeedec 100644 --- a/src/Mod/TechDraw/Gui/ViewProviderPage.cpp +++ b/src/Mod/TechDraw/Gui/ViewProviderPage.cpp @@ -334,8 +334,7 @@ void ViewProviderPage::removeMDIView(void) { if (!m_mdiView.isNull()) { //m_mdiView is a QPointer QList wList= Gui::getMainWindow()->windows(); - bool found = wList.contains(m_mdiView); - if (found) { + if (wList.contains(m_mdiView)) { Gui::getMainWindow()->removeWindow(m_mdiView); m_mdiView = nullptr; //m_mdiView will eventually be deleted and m_graphicsView = nullptr; //will take m_graphicsView with it