What is wrong with this method?

Hi, The method below is: 1. Quite slow 2. Sometimes gives an uncaught exception error. I have no additional debugging information unfortunately.

I'm not a C++ expert. Can you please tell me if it can be sped up and if it can be made more robust to avoid the uncaught exception? Thank you!

CellComplex::Ptr CellComplex::ByCells(const std::list<Cell::Ptr>& rkCells, const bool kCopyAttributes)
    {
        // ByOcctSolids does the actual construction. This method extracts the OCCT structures from the input
        // and wrap the output in a Topologic class.
        TopTools_ListOfShape occtShapes;
        for (const Cell::Ptr& kpCell : rkCells)
        {
            occtShapes.Append(kpCell->GetOcctShape());
        }

        TopoDS_CompSolid occtCompSolid = ByOcctSolids(occtShapes);
        CellComplex::Ptr pCellComplex = std::make_shared<CellComplex>(occtCompSolid);
        CellComplex::Ptr pCopyCellComplex = std::dynamic_pointer_cast<CellComplex>(pCellComplex->DeepCopy());
        if (kCopyAttributes)
        {
            std::list<Topology::Ptr> cellsAsTopologies;
            for (const Cell::Ptr& kpCell : rkCells)
            {
                cellsAsTopologies.push_back(kpCell);
                AttributeManager::GetInstance().DeepCopyAttributes(kpCell->GetOcctSolid(), pCopyCellComplex->GetOcctCompSolid());
            }
            CellComplex::Ptr pCopyCellComplex = TopologicalQuery::Downcast<CellComplex>(pCellComplex->DeepCopyAttributesFrom(cellsAsTopologies));
        }
        //GlobalCluster::GetInstance().AddTopology(pCopyCellComplex->GetOcctCompSolid());

        return pCopyCellComplex;
    }
Dmitrii Pasukhin's picture

Hello

  • `occtShapes.Append(kpCell->GetOcctShape());` copying is not optimal.
  • `CellComplex::Ptr pCellComplex = std::make_shared<CellComplex>(occtCompSolid);` TopoDS_CompSolid is an already a smart object wrapper. You double repeat wrapping of pointer.
  • `CellComplex::Ptr pCopyCellComplex = std::dynamic_pointer_cast<CellComplex>(pCellComplex->DeepCopy());` one of the compicated methods. Deep Copy is a too diffucult operation.
  • `cellsAsTopologies.push_back(kpCell);` again double allocation for the same data

I can't give you nothing more.

Best regards, Dmitrii.

Wassim Jabi's picture

Thank you, Dimitrii. We have to wrap the occt classes in our own pointers for clarity and to combine with other capabilities. We have our own class hierarchy. Point taken on the Deep Copy, but is it something that can be avoided from what you can see in this code snippet?
Lastly, do you see anything that can cause a memory leak in this method? Many thanks for your help!

Dmitrii Pasukhin's picture

Memory leaks - i don't think so. Just spending time on small allocations/deallocs

If you updates TopoDS_Shape and don't care about original object, deep copy is not needed or if you don't update TopoDS_SHape newer. In case if you need to keep not modifcated shapes - it is not possible to avoid deep copy(

 Best regards, Dmitrii.