[TD]Prevent crash in SelectionSingleton

- in SelectionSingleton::setVisible, if the selection changes during
  loop, a crash may occur.
- in QGraphicsScene, hiding an item changes its selected status.
This commit is contained in:
wandererfan
2019-08-28 08:44:00 -04:00
committed by WandererFan
parent 812aea7262
commit f68e58e389
3 changed files with 34 additions and 11 deletions

View File

@@ -1233,6 +1233,7 @@ bool SelectionSingleton::updateSelection(bool show, const char* pDocName,
FC_LOG("Update Selection "<<Chng.DocName << '#' << Chng.ObjName << '.' <<Chng.SubName);
notify(std::move(Chng));
return true;
}
@@ -1337,6 +1338,12 @@ void SelectionSingleton::setVisible(int visible) {
else if(visible>0)
visible = 1;
for(auto &sel : _SelList) {
//Note: if selection is changed while processing this list, the contents of _SelList will be
//changed during loop execution. This may cause crash here when a "non-entry" is processed.
// if (_SelList.size() == 0) {
// Base::Console().Log("Gui::SS::setVisible - _SelList altered during loop - break!\n");
// break;
// }
if(sel.DocName.empty() || sel.FeatName.empty() || !sel.pObject)
continue;
// get parent object
@@ -1350,7 +1357,6 @@ void SelectionSingleton::setVisible(int visible) {
// prevent setting the same object visibility more than once
if(!filter.insert(std::make_pair(obj,parent)).second)
continue;
int vis = parent->isElementVisible(elementName.c_str());
if(vis>=0) {
if(vis>0) vis = 1;
@@ -1371,11 +1377,12 @@ void SelectionSingleton::setVisible(int visible) {
// Fall back to direct object visibility setting
}
if(!filter.insert(std::make_pair(obj,(App::DocumentObject*)0)).second)
if(!filter.insert(std::make_pair(obj,(App::DocumentObject*)0)).second){
continue;
}
auto vp = Application::Instance->getViewProvider(obj);
if(vp) {
int vis;
if(visible>=0)

View File

@@ -1033,6 +1033,8 @@ void MDIViewPage::clearSceneSelection()
//!Update QGIView's selection state based on Selection made outside Drawing Interface
void MDIViewPage::selectQGIView(App::DocumentObject *obj, const bool isSelected)
{
// Base::Console().Message("MDIVP::selectQGIV(%s) - %d\n", obj->getNameInDocument(), isSelected);
App::DocumentObject* objCopy = obj;
TechDraw::DrawHatch* hatchObj = dynamic_cast<TechDraw::DrawHatch*>(objCopy);
if (hatchObj) { //Hatch does not have a QGIV of it's own. mark parent as selected.
@@ -1055,6 +1057,7 @@ void MDIViewPage::selectQGIView(App::DocumentObject *obj, const bool isSelected)
//really "onTreeSelectionChanged"
void MDIViewPage::onSelectionChanged(const Gui::SelectionChanges& msg)
{
// Base::Console().Message("MDIVP::onSelectionChanged()\n");
std::vector<Gui::SelectionSingleton::SelObj> selObjs = Gui::Selection().getSelection(msg.pDocName);
if (msg.Type == Gui::SelectionChanges::ClrSelection) {
clearSceneSelection();
@@ -1082,6 +1085,7 @@ void MDIViewPage::onSelectionChanged(const Gui::SelectionChanges& msg)
//! maintain QGScene selected items in selection order
void MDIViewPage::sceneSelectionManager()
{
// Base::Console().Message("MDIVP::sceneSelectionManager()\n");
QList<QGraphicsItem*> sceneSel = m_view->scene()->selectedItems();
if (sceneSel.isEmpty()) {
@@ -1128,6 +1132,7 @@ void MDIViewPage::sceneSelectionManager()
//triggered by m_view->scene() signal
void MDIViewPage::sceneSelectionChanged()
{
// Base::Console().Message("MDIVP::sceneSelctionChanged()\n");
sceneSelectionManager();
// QList<QGraphicsItem*> dbsceneSel = m_view->scene()->selectedItems();
@@ -1152,6 +1157,7 @@ void MDIViewPage::sceneSelectionChanged()
//Note: Qt says: "no guarantee of selection order"!!!
void MDIViewPage::setTreeToSceneSelect(void)
{
// Base::Console().Message("MDIVP::setTreeToSceneSelect()\n");
bool saveBlock = blockConnection(true); // block selectionChanged signal from Tree/Observer
blockSelection(true);
Gui::Selection().clearSelection();
@@ -1319,6 +1325,7 @@ void MDIViewPage::setTreeToSceneSelect(void)
bool MDIViewPage::compareSelections(std::vector<Gui::SelectionObject> treeSel, QList<QGraphicsItem*> sceneSel)
{
// Base::Console().Message("MDIVP::compareSelections()\n");
bool result = true;
if (treeSel.empty() && sceneSel.empty()) {

View File

@@ -121,11 +121,11 @@ void ViewProviderDrawingView::onChanged(const App::Property *prop)
}
if (prop == &Visibility) {
if(Visibility.getValue()) {
show();
} else {
hide();
}
// if(Visibility.getValue()) {
// show();
// } else {
// hide();
// }
} else if (prop == &KeepLabel) {
QGIView* qgiv = getQView();
if (qgiv) {
@@ -161,11 +161,20 @@ void ViewProviderDrawingView::hide(void)
if (obj->getTypeId().isDerivedFrom(TechDraw::DrawView::getClassTypeId())) {
QGIView* qView = getQView();
if (qView) {
qView->draw();
qView->hide();
//note: hiding an item in the scene clears its selection status
// this confuses Gui::Selection.
// So we block selection changes while we are hiding the qgiv
// in FC Tree hiding does not change selection state.
// block/unblock selection protects against crash in Gui::SelectionSingleton::setVisible
MDIViewPage* mdi = getMDIViewPage();
if (mdi != nullptr) { //if there is no mdivp, there is nothing to hide!
mdi->blockSelection(true);
qView->hide();
ViewProviderDocumentObject::hide();
mdi->blockSelection(false);
}
}
}
ViewProviderDocumentObject::hide();
}
QGIView* ViewProviderDrawingView::getQView(void)