Skip to content

Commit 2afd1a9

Browse files
lelandaisblelandaisb
committed
Fix issue#265: crash after two identical setGroup commands
# Conflicts: # test_link/test_groups.py
1 parent cc43f7d commit 2afd1a9

5 files changed

Lines changed: 113 additions & 25 deletions

File tree

‎src/Core/Topo/CommandSetGeomAssociation.cpp‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -147,16 +147,12 @@ void CommandSetGeomAssociation::validGeomEntity()
147147
/*----------------------------------------------------------------------------*/
148148
void CommandSetGeomAssociation::project(Block* bloc)
149149
{
150-
getInfoCommand().addTopoInfoEntity(bloc, Internal::InfoCommand::DISPMODIFIED);
151-
bloc->saveTopoProperty();
152-
bloc->setGeomAssociation(m_geom_entity);
150+
setGeomAssociation(bloc, [bloc](Internal::InfoCommand* icmd) { bloc->saveBlockTopoProperty(icmd); });
153151
}
154152
/*----------------------------------------------------------------------------*/
155153
void CommandSetGeomAssociation::project(CoFace* coface)
156154
{
157-
getInfoCommand().addTopoInfoEntity(coface, Internal::InfoCommand::DISPMODIFIED);
158-
coface->saveTopoProperty();
159-
coface->setGeomAssociation(m_geom_entity);
155+
setGeomAssociation(coface, [coface](Internal::InfoCommand* icmd) { coface->saveCoFaceTopoProperty(icmd); });
160156

161157
std::vector<CoEdge*> coedges = coface->getCoEdges();
162158

@@ -210,9 +206,7 @@ void CommandSetGeomAssociation::project(CoFace* coface)
210206
/*----------------------------------------------------------------------------*/
211207
void CommandSetGeomAssociation::project(CoEdge* coedge)
212208
{
213-
getInfoCommand().addTopoInfoEntity(coedge, Internal::InfoCommand::DISPMODIFIED);
214-
coedge->saveTopoProperty();
215-
coedge->setGeomAssociation(m_geom_entity);
209+
setGeomAssociation(coedge, [coedge](Internal::InfoCommand* icmd) { coedge->saveCoEdgeTopoProperty(icmd); });
216210

217211
const std::vector<Vertex*>& vertices = coedge->getVertices();
218212
for (auto iter = vertices.begin(); iter != vertices.end(); ++iter){
@@ -229,8 +223,7 @@ void CommandSetGeomAssociation::project(CoEdge* coedge)
229223
/*----------------------------------------------------------------------------*/
230224
void CommandSetGeomAssociation::project(Vertex* vtx)
231225
{
232-
vtx->saveTopoProperty();
233-
vtx->setGeomAssociation(m_geom_entity);
226+
setGeomAssociation(vtx, [vtx](Internal::InfoCommand* icmd) { vtx->saveVertexTopoProperty(icmd); });
234227

235228
// déplace le sommet topologique si la projection implique un déplacement
236229
if (m_geom_entity && m_move_vertices){
@@ -258,14 +251,44 @@ void CommandSetGeomAssociation::project(Vertex* vtx)
258251
}
259252
}
260253
}
261-
getInfoCommand().addTopoInfoEntity(vtx, Internal::InfoCommand::DISPMODIFIED);
262254
}
263255
/*----------------------------------------------------------------------------*/
264256
void CommandSetGeomAssociation::getPreviewRepresentation(Utils::DisplayRepresentation& dr)
265257
{
266258
return getPreviewRepresentationCoedgeDisplayModified(dr);
267259
}
268260
/*----------------------------------------------------------------------------*/
261+
void CommandSetGeomAssociation::setGeomAssociation(TopoEntity* e, std::function<void(Internal::InfoCommand* icmd)> saveGroups)
262+
{
263+
getInfoCommand().addTopoInfoEntity(e, Internal::InfoCommand::DISPMODIFIED);
264+
e->saveTopoProperty();
265+
e->setGeomAssociation(m_geom_entity);
266+
267+
// si m_geom_entity est dans le même groupe que e => e sort du groupe
268+
// 1. les groupes géométriques
269+
Group::GroupManager& gm = e->getContext().getGroupManager();
270+
std::vector<std::string> geom_groups = Utils::toNames(gm.getGroupsFor(m_geom_entity));
271+
std::sort(geom_groups.begin(), geom_groups.end());
272+
// 2. les groupes topologiques
273+
Group::GroupHelperForCommand helper(getInfoCommand(), gm);
274+
std::vector<std::string> topo_groups = Utils::toNames(helper.getGroupsFor(e));
275+
std::sort(topo_groups.begin(), topo_groups.end());
276+
// 3. l'intersection
277+
std::vector<std::string> common_groups;
278+
std::set_intersection(
279+
geom_groups.begin(), geom_groups.end(),
280+
topo_groups.begin(), topo_groups.end(),
281+
std::back_inserter(common_groups)
282+
);
283+
// 4. suppression des groupes
284+
if (common_groups.size() > 0) {
285+
// au moins un groupe à supprimer, il faut sauvegarder les listes de groupes
286+
saveGroups(&getInfoCommand());
287+
for (std::string gn : common_groups)
288+
helper.removeFromGroup(gn, e);
289+
}
290+
}
291+
/*----------------------------------------------------------------------------*/
269292
} // end namespace Topo
270293
/*----------------------------------------------------------------------------*/
271294
} // end namespace Mgx3D

‎src/Core/Topo/TopoEntity.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,14 @@ setGeomAssociation(Geom::GeomEntity* ge)
116116

117117
// on met à jour la relation réciproque (de Geom vers Topo)
118118
Topo::TopoManager& tm = getContext().getTopoManager();
119-
if (m_topo_property->getGeomAssociation())
120-
tm.removeRefTopo(m_topo_property->getGeomAssociation(), this);
119+
Geom::GeomEntity* oldGe = m_topo_property->getGeomAssociation();
120+
if (oldGe)
121+
tm.removeRefTopo(oldGe, this);
122+
121123
if (ge)
122124
tm.addRefTopo(ge, this);
123125

124126
// mise à jour de la couleur si changement de dimension pour la projection
125-
Geom::GeomEntity* oldGe = m_topo_property->getGeomAssociation();
126127
bool need_update_color = (0==ge || 0==oldGe || ge->getDim() != oldGe->getDim());
127128

128129
// la relation Topo vers Geom

‎src/Core/protected/Group/GroupHelperForCommand.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ class GroupHelperForCommand {
7979
group->remove(e);
8080
e->remove(group);
8181

82-
// il faut peut-être ajouter le groupe par défaut
83-
if (e->getNbGroups() == 0) {
82+
if (e->getNbGroups() == 0 && e->getGeomAssociation() == 0) {
8483
addToGroup("", e);
8584
}
8685

‎src/Core/protected/Topo/CommandSetGeomAssociation.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ class CommandSetGeomAssociation: public Topo::CommandEditTopo{
7979
virtual void getPreviewRepresentation(Utils::DisplayRepresentation& dr);
8080

8181
private:
82+
void setGeomAssociation(TopoEntity* e, std::function<void(Internal::InfoCommand* icmd)> saveGroups);
8283

8384
/*------------------------------------------------------------------------*/
8485
/** Met à jour l'association et c'est tout */

‎test_link/test_groups.py‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,43 +272,54 @@ def test_two_adds(capfd):
272272
def test_geom_assoc1():
273273
ctx = Mgx3D.getStdContext()
274274
ctx.clearSession() # Clean the session after the previous test
275+
gm = ctx.getGeomManager()
275276
tm = ctx.getTopoManager()
276277

277278
# Création de la boite Vol0000
278-
ctx.getGeomManager().newBox (Mgx3D.Point(0, 0, 0), Mgx3D.Point(1, 1, 1))
279-
# Création d'un sommet géométrique par coordonnées
280-
tm.newTopoVertex (Mgx3D.Point(0, 0, 0),"")
279+
gm.newBox (Mgx3D.Point(0, 0, 0), Mgx3D.Point(1, 1, 1))
280+
assert gm.getInfos("Pt0001", 0).groups() == ['Hors_Groupe_0D']
281281

282+
# Création d'un sommet géométrique par coordonnées
283+
tm.newTopoVertex (Mgx3D.Point(0, 0, 0), "")
282284
assert tm.getInfos("Som0000", 0).groups() == ['Hors_Groupe_0D']
283285

284286
# Affectation d'une projection vers Pt0001 pour les entités topologiques Som0000
285-
# L'association ne peut se faire car le groupe Hors_Groupe_0D possède Pt0001 ainsi que Som0000
286-
# Il est recommandé de supprimer la topologie du groupe
287+
# Som0000 va être sorti du groupe Hors_Groupe_0D car Pt0001 appartient à Hors_Groupe_0D
288+
# Som0000 va donc hériter de ce groupe par associativité
287289
tm.setGeomAssociation (["Som0000"], "Pt0001", True)
290+
assert gm.getInfos("Pt0001", 0).groups() == ['Hors_Groupe_0D']
291+
assert tm.getInfos("Som0000", 0).groups() == []
288292

293+
ctx.undo()
294+
assert gm.getInfos("Pt0001", 0).groups() == ['Hors_Groupe_0D']
289295
assert tm.getInfos("Som0000", 0).groups() == ['Hors_Groupe_0D']
290296

291297
def test_geom_assoc2():
292298
ctx = Mgx3D.getStdContext()
293299
ctx.clearSession() # Clean the session after the previous test
300+
gm = ctx.getGeomManager()
294301
tm = ctx.getTopoManager()
295302

296303
# Création de la boite Vol0000
297304
ctx.getGeomManager().newBox (Mgx3D.Point(0, 0, 0), Mgx3D.Point(1, 1, 1))
298305
# Modifie le groupe A
299306
ctx.getGeomManager().addToGroup (["Pt0001"], 0, "A")
307+
assert gm.getInfos("Pt0001", 0).groups() == ['A']
300308

301309
# Création d'un sommet géométrique par coordonnées
302310
tm.newTopoVertex (Mgx3D.Point(0, 0, 0),"A")
303-
304311
assert tm.getInfos("Som0000", 0).groups() == ['A']
305312

306313
# Affectation d'une projection vers Pt0001 pour les entités topologiques Som0000
307-
# L'association ne peut se faire car le groupe Hors_Groupe_0D possède Pt0001 ainsi que Som0000
308-
# Il est recommandé de supprimer la topologie du groupe
314+
# Som0000 va être sorti du groupe A car Pt0001 appartient à A
315+
# Som0000 va donc hériter de ce groupe par associativité
309316
tm.setGeomAssociation (["Som0000"], "Pt0001", True)
317+
assert tm.getInfos("Som0000", 0).groups() == []
318+
assert gm.getInfos("Pt0001", 0).groups() == ['A']
310319

320+
ctx.undo()
311321
assert tm.getInfos("Som0000", 0).groups() == ['A']
322+
assert gm.getInfos("Pt0001", 0).groups() == ['A']
312323

313324
# issue#245: undo on addToGroup raises an error
314325
def test_topo_surface():
@@ -352,3 +363,56 @@ def test_issue265():
352363
gm.setGroup(["Vol0001"], 3, "aaa")
353364
assert gm.getInfos("Vol0000", 3).groups() == ["aaa"]
354365
assert gm.getInfos("Vol0001", 3).groups() == ["aaa"]
366+
367+
# mesh becomes invisible during geom association
368+
def test_issue261_1():
369+
ctx = Mgx3D.getStdContext()
370+
ctx.clearSession() # Clean the session after the previous test
371+
gm = ctx.getGeomManager()
372+
tm = ctx.getTopoManager()
373+
374+
gn = "test"
375+
# Création de la boite Vol0000
376+
gm.newBox (Mgx3D.Point(0, 0, 0), Mgx3D.Point(1, 1, 1), gn)
377+
assert gm.getInfos("Vol0000", 3).groups() == [gn]
378+
379+
# Création d'un bloc unitaire mis dans le groupe test
380+
tm.newFreeTopoInGroup (gn, 3)
381+
assert tm.getInfos("Bl0000", 3).groups() == [gn]
382+
383+
# Affectation d'une projection vers Vol0000 pour les entités topologiques Bl0000
384+
tm.setGeomAssociation (["Bl0000"], "Vol0000", False)
385+
# lors de l'association, Bl0000 doit être retiré de gn car il en "hérite" par la géométrie
386+
assert tm.getInfos("Bl0000", 3).groups() == []
387+
assert gm.getInfos("Vol0000", 3).groups() == [gn]
388+
389+
# test du undo
390+
ctx.undo()
391+
assert tm.getInfos("Bl0000", 3).groups() == [gn]
392+
assert gm.getInfos("Vol0000", 3).groups() == [gn]
393+
394+
def test_issue261_2():
395+
ctx = Mgx3D.getStdContext()
396+
ctx.clearSession() # Clean the session after the previous test
397+
gm = ctx.getGeomManager()
398+
tm = ctx.getTopoManager()
399+
400+
gn = "Hors_Groupe_3D"
401+
# Création de la boite Vol0000
402+
gm.newBox (Mgx3D.Point(0, 0, 0), Mgx3D.Point(1, 1, 1))
403+
assert gm.getInfos("Vol0000", 3).groups() == [gn]
404+
405+
# Création d'un bloc unitaire mis dans le groupe test
406+
tm.newFreeTopoInGroup (gn, 3)
407+
assert tm.getInfos("Bl0000", 3).groups() == [gn]
408+
409+
# Affectation d'une projection vers Vol0000 pour les entités topologiques Bl0000
410+
tm.setGeomAssociation (["Bl0000"], "Vol0000", False)
411+
# lors de l'association, Bl0000 doit être retiré de gn car il en "hérite" par la géométrie
412+
assert tm.getInfos("Bl0000", 3).groups() == []
413+
assert gm.getInfos("Vol0000", 3).groups() == [gn]
414+
415+
# test du undo
416+
ctx.undo()
417+
assert tm.getInfos("Bl0000", 3).groups() == [gn]
418+
assert gm.getInfos("Vol0000", 3).groups() == [gn]

0 commit comments

Comments
 (0)