[SofaTopologyMapping] Cleanups of some topological mappings + better initialization #1126
Conversation
| @@ -101,10 +106,8 @@ void Hexa2QuadTopologicalMapping::init() | |||
| return; | |||
| } | |||
|
|
|||
| QuadSetTopologyContainer *to_tstc; | |||
| toModel->getContext()->get(to_tstc); | |||
There was a problem hiding this comment.
could toModel be different than to_tstc ?
I think you are right, but I don't remember why this get() was here.
There was a problem hiding this comment.
Yes, toModel is a BaseMeshTopology, so it could be something else than a QuadSetTopologyContainer. I'm not sure if casting it to a QuadSetTopologyContainer is really necessary here... but at least this PR keeps the "toModel must be a QuadSetTopologyContainer" requirement.
| @@ -267,14 +272,14 @@ void Quad2TriangleTopologicalMapping::updateTopologicalMappingTopDown() | |||
| } | |||
| case core::topology::QUADSREMOVED: | |||
| { | |||
| const sofa::helper::vector<unsigned int> &tab = ( static_cast< const QuadsRemoved *>( *itBegin ) )->getArray(); | |||
| const auto & tab = ( static_cast< const QuadsRemoved *>( *itBegin ) )->getArray(); | |||
There was a problem hiding this comment.
I have nothing against using auto type, but in those cases maybe changing the name of the variable would be good.
Because auto tab gives no info of what we are manipulating. maybe tabIndices or what you think is best.
There was a problem hiding this comment.
I went to the file very quickly changing to auto where my IDE would suggest to do so (and a bit more), I agree that the name isn't very well chosen. I will try if I have some time to change the names... but here the idea of auto is more that it will automatically handle the case where the return value type changes in the future.
epernod
added
pr: clean
|
[ci-build][with-all-tests] |
guparan
added
pr: status wip
…e good typedef and rename some variables.
epernod
added
pr: status to review
|
Thanks a lot @epernod !! |
Various cleanup and better checks at the init stage to make sure the topological mappings have everything they need.
Only Hexa2Quad, Quad2Triangle and Tetra2Triangle for now.
This PR:
Reviewers will merge only if all these checks are true.