From bfa0311ef0d0d97995726995a29daf4dea2dbc18 Mon Sep 17 00:00:00 2001 From: Pasukhin Dmitry Date: Thu, 12 Feb 2026 20:28:20 +0000 Subject: [PATCH] Foundation Classes - Tree & collection performance optimizations, move semantics, unified map API (#1065) NCollection_UBTree/EBTree: - Add move constructor and move assignment operators - Replace recursive Select() and delNode() with iterative stack-based traversal to avoid stack overflow on deeply unbalanced trees - Optimize EBTree::Add() and Remove() to use single-lookup TryEmplaced() instead of double-lookup UnBind()+Bind() / Contains()+operator() - Remove unused DEFINE_HUBTREE / DEFINE_HEBTREE / IMPLEMENT_HUBTREE / IMPLEMENT_HEBTREE macros - Remove unused includes from EBTree (Standard_Type, Standard_Transient, NCollection_List, Standard_Integer, NCollection_Sequence) - Fix doxygen @param tags and comment style NCollection_LocalArray: - Add move constructor and move assignment operators with optimized three-way branching (stack-stack copy, heap-heap swap, stack-heap steal) - Add Reallocate() method supporting grow-with-copy for use as a dynamically growable stack - Add static_assert enforcing trivially copyable element type NCollection_CellFilter: - Replace const_cast destructive-copy hack in Cell with proper move semantics; delete copy constructor and copy assignment - Add Cell constructor from CellIndex for lightweight lookup keys - Refactor add()/iterateAdd() to accept CellIndex instead of Cell, use TryEmplaced() for single-lookup cell insertion - Refactor remove()/inspect() to use Contained() API with const_cast instead of C-style cast on Seek() - Change ListNode default constructor from runtime throw to = delete - Use size_t for dimension loops and add dimension size guard in IsEqual - Remove SUN WorkShop 5.3 workaround - Fix typo "usially" -> "usually" in class documentation NCollection map API unification (Contained, TryEmplace, TryBind): - Add Contained() to all map types returning std::optional with std::reference_wrapper; key-only maps return const key ref, data maps return std::pair of const key ref + value ref - Add TryEmplace()/TryEmplaced() to NCollection_FlatMap and NCollection_IndexedMap for parity with NCollection_Map - Add TryBind() to NCollection_IndexedDataMap for parity with NCollection_DataMap and NCollection_FlatDataMap - Remove Seek()/ChangeSeek() from NCollection_Map (replaced by Contained()) Dead compiler workaround removal: - NCollection_DefineAlloc: remove Borland/SUN #if branch, keep only the version with placement delete - NCollection_SparseArrayBase: remove SUN WorkShop 5.3 workaround GTests: - Add move constructor/assignment tests for LocalArray, UBTree, EBTree - Add Contained tests for NCollection_Map - Add CellFilter tests and UBTree deep-unbalanced-tree stress test --- .../TKernel/GTests/FILES.cmake | 2 + .../GTests/NCollection_CellFilter_Test.cxx | 262 +++++++++++ .../GTests/NCollection_FlatMap_Test.cxx | 50 +++ .../GTests/NCollection_LocalArray_Test.cxx | 151 +++++++ .../TKernel/GTests/NCollection_Map_Test.cxx | 30 ++ .../GTests/NCollection_UBTree_Test.cxx | 414 ++++++++++++++++++ .../NCollection/NCollection_CellFilter.hxx | 177 +++----- .../NCollection/NCollection_DataMap.hxx | 26 ++ .../NCollection/NCollection_DefineAlloc.hxx | 30 +- .../NCollection/NCollection_EBTree.hxx | 158 +++---- .../NCollection/NCollection_FlatDataMap.hxx | 29 ++ .../NCollection/NCollection_FlatMap.hxx | 57 +++ .../NCollection_IndexedDataMap.hxx | 53 +++ .../NCollection/NCollection_IndexedMap.hxx | 30 ++ .../NCollection/NCollection_LocalArray.hxx | 120 ++++- .../TKernel/NCollection/NCollection_Map.hxx | 30 ++ .../NCollection_SparseArrayBase.hxx | 5 - .../NCollection/NCollection_UBTree.hxx | 220 ++++------ .../BRepMesh/BRepMesh_CircleInspector.hxx | 14 +- .../BRepMesh/BRepMesh_VertexInspector.hxx | 14 +- .../BRepBuilderAPI_FastSewing.hxx | 14 +- .../BRepBuilderAPI_VertexInspector.hxx | 23 +- .../BRepExtrema_ProximityValueTool.hxx | 23 +- .../TKGeomBase/Extrema/Extrema_GGenExtCC.hxx | 18 +- 24 files changed, 1543 insertions(+), 407 deletions(-) create mode 100644 src/FoundationClasses/TKernel/GTests/NCollection_CellFilter_Test.cxx create mode 100644 src/FoundationClasses/TKernel/GTests/NCollection_UBTree_Test.cxx diff --git a/src/FoundationClasses/TKernel/GTests/FILES.cmake b/src/FoundationClasses/TKernel/GTests/FILES.cmake index 68b9577415..4cc93886ca 100644 --- a/src/FoundationClasses/TKernel/GTests/FILES.cmake +++ b/src/FoundationClasses/TKernel/GTests/FILES.cmake @@ -8,6 +8,7 @@ set(OCCT_TKernel_GTests_FILES NCollection_Array1_Test.cxx NCollection_Array2_Test.cxx NCollection_BaseAllocator_Test.cxx + NCollection_CellFilter_Test.cxx NCollection_DataMap_Test.cxx NCollection_DoubleMap_Test.cxx NCollection_FlatDataMap_Test.cxx @@ -21,6 +22,7 @@ set(OCCT_TKernel_GTests_FILES NCollection_PackedMap_Test.cxx NCollection_Sequence_Test.cxx NCollection_SparseArray_Test.cxx + NCollection_UBTree_Test.cxx NCollection_Vec4_Test.cxx NCollection_Vector_Test.cxx OSD_Path_Test.cxx diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_CellFilter_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_CellFilter_Test.cxx new file mode 100644 index 0000000000..f31b280865 --- /dev/null +++ b/src/FoundationClasses/TKernel/GTests/NCollection_CellFilter_Test.cxx @@ -0,0 +1,262 @@ +// Copyright (c) 2025 OPEN CASCADE SAS +// +// This file is part of Open CASCADE Technology software library. +// +// This library is free software; you can redistribute it and/or modify it under +// the terms of the GNU Lesser General Public License version 2.1 as published +// by the Free Software Foundation, with special exception defined in the file +// OCCT_LGPL_EXCEPTION.txt. Consult the file LICENSE_LGPL_21.txt included in OCCT +// distribution for complete text of the license and disclaimer of any warranty. +// +// Alternatively, this file may be used under the terms of Open CASCADE +// commercial license or contractual agreement. + +#include + +#include +#include + +namespace +{ + +//! Simple 2D point for testing +struct TestPoint2D +{ + double X; + double Y; + + TestPoint2D(double theX = 0.0, double theY = 0.0) + : X(theX), + Y(theY) + { + } +}; + +//! Simple 2D inspector for CellFilter tests +class TestInspector2D +{ +public: + typedef int Target; + typedef TestPoint2D Point; + + enum + { + Dimension = 2 + }; + + static double Coord(int i, const Point& thePnt) { return i == 0 ? thePnt.X : thePnt.Y; } + + static bool IsEqual(const Target& theT1, const Target& theT2) { return theT1 == theT2; } + + TestInspector2D() + : myPurgeValue(-1) + { + } + + void SetPurgeValue(int theVal) { myPurgeValue = theVal; } + + NCollection_CellFilter_Action Inspect(const Target& theTarget) + { + if (theTarget == myPurgeValue) + return CellFilter_Purge; + myFound.push_back(theTarget); + return CellFilter_Keep; + } + + const std::vector& Found() const { return myFound; } + + void Reset() { myFound.clear(); } + +private: + std::vector myFound; + int myPurgeValue; +}; + +//! Simple 3D point for testing +struct TestPoint3D +{ + double X; + double Y; + double Z; + + TestPoint3D(double theX = 0.0, double theY = 0.0, double theZ = 0.0) + : X(theX), + Y(theY), + Z(theZ) + { + } +}; + +//! Simple 3D inspector for CellFilter tests +class TestInspector3D +{ +public: + typedef int Target; + typedef TestPoint3D Point; + + enum + { + Dimension = 3 + }; + + static double Coord(int i, const Point& thePnt) + { + return i == 0 ? thePnt.X : (i == 1 ? thePnt.Y : thePnt.Z); + } + + static bool IsEqual(const Target& theT1, const Target& theT2) { return theT1 == theT2; } + + NCollection_CellFilter_Action Inspect(const Target& theTarget) + { + myFound.push_back(theTarget); + return CellFilter_Keep; + } + + const std::vector& Found() const { return myFound; } + + void Reset() { myFound.clear(); } + +private: + std::vector myFound; +}; + +} // namespace + +TEST(NCollection_CellFilterTest, AddInspect2D_SinglePoint) +{ + NCollection_CellFilter aFilter(1.0); + + // Add targets at different points + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + aFilter.Add(20, TestPoint2D(1.5, 1.5)); + aFilter.Add(30, TestPoint2D(0.3, 0.3)); + + // Inspect cell containing (0.5, 0.5) -- should find 10 and 30 (same cell) + TestInspector2D anInspector; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + + // Both 10 and 30 are in cell (0,0) + EXPECT_EQ(2u, anInspector.Found().size()); +} + +TEST(NCollection_CellFilterTest, AddInspect2D_Range) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(1, TestPoint2D(0.5, 0.5)); + aFilter.Add(2, TestPoint2D(1.5, 0.5)); + aFilter.Add(3, TestPoint2D(2.5, 0.5)); + + // Inspect range covering cells (0,0) and (1,0) + TestInspector2D anInspector; + aFilter.Inspect(TestPoint2D(0.0, 0.0), TestPoint2D(1.5, 0.5), anInspector); + + // Should find targets 1 and 2 + EXPECT_EQ(2u, anInspector.Found().size()); +} + +TEST(NCollection_CellFilterTest, Remove2D) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + aFilter.Add(20, TestPoint2D(0.5, 0.5)); + + // Remove target 10 + aFilter.Remove(10, TestPoint2D(0.5, 0.5)); + + // Inspect -- should only find 20 + TestInspector2D anInspector; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + + ASSERT_EQ(1u, anInspector.Found().size()); + EXPECT_EQ(20, anInspector.Found()[0]); +} + +TEST(NCollection_CellFilterTest, EmptyCellCleanupAfterRemove) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + + // Remove the only target -- cell should be cleaned up + aFilter.Remove(10, TestPoint2D(0.5, 0.5)); + + // Inspect -- should find nothing + TestInspector2D anInspector; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + EXPECT_TRUE(anInspector.Found().empty()); +} + +TEST(NCollection_CellFilterTest, EmptyCellCleanupAfterPurge) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + + // Inspect with purge action for target 10 + TestInspector2D anInspector; + anInspector.SetPurgeValue(10); + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + + // Found should be empty (purged, not kept) + EXPECT_TRUE(anInspector.Found().empty()); + + // Second inspect -- cell should have been cleaned up, nothing found + TestInspector2D anInspector2; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector2); + EXPECT_TRUE(anInspector2.Found().empty()); +} + +TEST(NCollection_CellFilterTest, AddInspect3D) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(1, TestPoint3D(0.5, 0.5, 0.5)); + aFilter.Add(2, TestPoint3D(1.5, 1.5, 1.5)); + aFilter.Add(3, TestPoint3D(0.3, 0.3, 0.3)); + + // Inspect cell containing (0.5, 0.5, 0.5) + TestInspector3D anInspector; + aFilter.Inspect(TestPoint3D(0.5, 0.5, 0.5), anInspector); + + // Both 1 and 3 are in cell (0,0,0) + EXPECT_EQ(2u, anInspector.Found().size()); +} + +TEST(NCollection_CellFilterTest, Reset) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + + // Reset with new cell size + aFilter.Reset(2.0); + + // After reset, all data should be cleared + TestInspector2D anInspector; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + EXPECT_TRUE(anInspector.Found().empty()); +} + +TEST(NCollection_CellFilterTest, InspectWithPurge) +{ + NCollection_CellFilter aFilter(1.0); + + aFilter.Add(10, TestPoint2D(0.5, 0.5)); + aFilter.Add(20, TestPoint2D(0.5, 0.5)); + aFilter.Add(30, TestPoint2D(0.5, 0.5)); + + // Inspect with purge for target 20 + TestInspector2D anInspector; + anInspector.SetPurgeValue(20); + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector); + + // Should find 10 and 30 (20 was purged) + EXPECT_EQ(2u, anInspector.Found().size()); + + // Re-inspect -- 20 should be gone + TestInspector2D anInspector2; + aFilter.Inspect(TestPoint2D(0.5, 0.5), anInspector2); + EXPECT_EQ(2u, anInspector2.Found().size()); +} diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_FlatMap_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_FlatMap_Test.cxx index a3548794cf..f2b0dcfb27 100644 --- a/src/FoundationClasses/TKernel/GTests/NCollection_FlatMap_Test.cxx +++ b/src/FoundationClasses/TKernel/GTests/NCollection_FlatMap_Test.cxx @@ -225,6 +225,56 @@ TEST_F(NCollection_FlatMapTest, EmplacedExistingKey) EXPECT_EQ(1, aMap.Size()); } +// Tests for Seek method +TEST_F(NCollection_FlatMapTest, SeekFound) +{ + NCollection_FlatMap aMap; + aMap.Add(10); + aMap.Add(20); + aMap.Add(30); + + const int* pKey = aMap.Seek(10); + ASSERT_NE(nullptr, pKey); + EXPECT_EQ(10, *pKey); + + pKey = aMap.Seek(30); + ASSERT_NE(nullptr, pKey); + EXPECT_EQ(30, *pKey); +} + +TEST_F(NCollection_FlatMapTest, SeekNotFound) +{ + NCollection_FlatMap aMap; + aMap.Add(10); + + const int* pKey = aMap.Seek(99); + EXPECT_EQ(nullptr, pKey); + + // Seek on empty map + NCollection_FlatMap anEmptyMap; + EXPECT_EQ(nullptr, anEmptyMap.Seek(10)); +} + +TEST_F(NCollection_FlatMapTest, ChangeSeekModify) +{ + NCollection_FlatMap aMap; + aMap.Add("Hello"); + aMap.Add("World"); + + TCollection_AsciiString* pKey = aMap.ChangeSeek("Hello"); + ASSERT_NE(nullptr, pKey); + EXPECT_TRUE(pKey->IsEqual("Hello")); +} + +TEST_F(NCollection_FlatMapTest, ChangeSeekNotFound) +{ + NCollection_FlatMap aMap; + aMap.Add(10); + + int* pKey = aMap.ChangeSeek(99); + EXPECT_EQ(nullptr, pKey); +} + // Tests for hasher constructor TEST_F(NCollection_FlatMapTest, HasherConstructorCopy) { diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx index fe3f8255a0..aed6efdb62 100644 --- a/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx +++ b/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx @@ -191,4 +191,155 @@ TEST(NCollection_LocalArrayTest, TransitionStackToHeap) { EXPECT_EQ(static_cast(i * 5), array[i]); } +} + +// Test Reallocate with copy (stack to stack) +TEST(NCollection_LocalArrayTest, ReallocateStackToStack_WithCopy) +{ + NCollection_LocalArray anArray(10); + for (size_t i = 0; i < 10; ++i) + anArray[i] = static_cast(i * 100); + + anArray.Reallocate(32, true); + EXPECT_EQ(32u, anArray.Size()); + + // Original elements should be preserved + for (size_t i = 0; i < 10; ++i) + EXPECT_EQ(static_cast(i * 100), anArray[i]); +} + +// Test Reallocate with copy (stack to heap) +TEST(NCollection_LocalArrayTest, ReallocateStackToHeap_WithCopy) +{ + NCollection_LocalArray anArray(8); + for (size_t i = 0; i < 8; ++i) + anArray[i] = static_cast(i + 1); + + // Force heap allocation + anArray.Reallocate(16, true); + EXPECT_EQ(16u, anArray.Size()); + + // Original elements should be preserved + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i + 1), anArray[i]); +} + +// Test Reallocate with copy (heap to larger heap) +TEST(NCollection_LocalArrayTest, ReallocateHeapToHeap_WithCopy) +{ + NCollection_LocalArray anArray(8); // starts on heap + for (size_t i = 0; i < 8; ++i) + anArray[i] = static_cast(i * 10); + + anArray.Reallocate(16, true); + EXPECT_EQ(16u, anArray.Size()); + + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i * 10), anArray[i]); +} + +// Test Reallocate without copy +TEST(NCollection_LocalArrayTest, ReallocateNoCopy) +{ + NCollection_LocalArray anArray(8); + for (size_t i = 0; i < 8; ++i) + anArray[i] = static_cast(i + 1); + + anArray.Reallocate(16, false); + EXPECT_EQ(16u, anArray.Size()); + // Content is undefined -- just verify it doesn't crash +} + +// Test Reallocate shrink just updates logical size +TEST(NCollection_LocalArrayTest, ReallocateShrink) +{ + NCollection_LocalArray anArray(16); // starts on heap + for (size_t i = 0; i < 16; ++i) + anArray[i] = static_cast(i * 5); + + // Shrink - should only update logical size, data preserved + anArray.Reallocate(4, true); + EXPECT_EQ(4u, anArray.Size()); + + for (size_t i = 0; i < 4; ++i) + EXPECT_EQ(static_cast(i * 5), anArray[i]); +} + +// Test move constructor from stack-allocated source +TEST(NCollection_LocalArrayTest, MoveConstructor_FromStack) +{ + NCollection_LocalArray aSrc(10); + for (size_t i = 0; i < 10; ++i) + aSrc[i] = static_cast(i * 7); + + NCollection_LocalArray aDst(std::move(aSrc)); + + // Destination has the data + EXPECT_EQ(10u, aDst.Size()); + for (size_t i = 0; i < 10; ++i) + EXPECT_EQ(static_cast(i * 7), aDst[i]); + + // Source is empty + EXPECT_EQ(0u, aSrc.Size()); +} + +// Test move constructor from heap-allocated source +TEST(NCollection_LocalArrayTest, MoveConstructor_FromHeap) +{ + NCollection_LocalArray aSrc(16); // exceeds stack buffer, goes to heap + for (size_t i = 0; i < 16; ++i) + aSrc[i] = static_cast(i * 3); + + NCollection_LocalArray aDst(std::move(aSrc)); + + // Destination has the data + EXPECT_EQ(16u, aDst.Size()); + for (size_t i = 0; i < 16; ++i) + EXPECT_EQ(static_cast(i * 3), aDst[i]); + + // Source is empty + EXPECT_EQ(0u, aSrc.Size()); +} + +// Test move assignment operator +TEST(NCollection_LocalArrayTest, MoveAssignment) +{ + NCollection_LocalArray aSrc(8); + for (size_t i = 0; i < 8; ++i) + aSrc[i] = static_cast(i + 100); + + NCollection_LocalArray aDst(4); + for (size_t i = 0; i < 4; ++i) + aDst[i] = -1; + + aDst = std::move(aSrc); + + EXPECT_EQ(8u, aDst.Size()); + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i + 100), aDst[i]); + + EXPECT_EQ(0u, aSrc.Size()); +} + +// Test Reallocate used as a growable stack +TEST(NCollection_LocalArrayTest, ReallocateAsGrowableStack) +{ + NCollection_LocalArray aStack(4); + int aTop = 0; + + // Push more than initial capacity + for (int i = 0; i < 20; ++i) + { + if (aTop >= static_cast(aStack.Size())) + aStack.Reallocate(aStack.Size() * 2, true); + aStack[aTop++] = i * 3; + } + + EXPECT_EQ(20, aTop); + + // Pop and verify + for (int i = 19; i >= 0; --i) + { + EXPECT_EQ(i * 3, aStack[--aTop]); + } } \ No newline at end of file diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_Map_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_Map_Test.cxx index c478316ae4..640859bb5a 100644 --- a/src/FoundationClasses/TKernel/GTests/NCollection_Map_Test.cxx +++ b/src/FoundationClasses/TKernel/GTests/NCollection_Map_Test.cxx @@ -454,6 +454,36 @@ TEST(NCollection_MapTest, RangeBasedForLoop) EXPECT_TRUE(aFoundKeys.count(300) > 0); } +// Tests for Contained method +TEST(NCollection_MapTest, ContainedFound) +{ + NCollection_Map aMap; + aMap.Add(10); + aMap.Add(20); + aMap.Add(30); + + auto aResult = aMap.Contained(10); + ASSERT_TRUE(aResult.has_value()); + EXPECT_EQ(10, aResult->get()); + + aResult = aMap.Contained(30); + ASSERT_TRUE(aResult.has_value()); + EXPECT_EQ(30, aResult->get()); +} + +TEST(NCollection_MapTest, ContainedNotFound) +{ + NCollection_Map aMap; + aMap.Add(10); + + auto aResult = aMap.Contained(99); + EXPECT_FALSE(aResult.has_value()); + + // Contained on empty map + NCollection_Map anEmptyMap; + EXPECT_FALSE(anEmptyMap.Contained(10).has_value()); +} + // Test iterator equality using NCollection_StlIterator TEST(NCollection_MapTest, IteratorEquality) { diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_UBTree_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_UBTree_Test.cxx new file mode 100644 index 0000000000..a0b33f72bc --- /dev/null +++ b/src/FoundationClasses/TKernel/GTests/NCollection_UBTree_Test.cxx @@ -0,0 +1,414 @@ +// Copyright (c) 2025 OPEN CASCADE SAS +// +// This file is part of Open CASCADE Technology software library. +// +// This library is free software; you can redistribute it and/or modify it under +// the terms of the GNU Lesser General Public License version 2.1 as published +// by the Free Software Foundation, with special exception defined in the file +// OCCT_LGPL_EXCEPTION.txt. Consult the file LICENSE_LGPL_21.txt included in OCCT +// distribution for complete text of the license and disclaimer of any warranty. +// +// Alternatively, this file may be used under the terms of Open CASCADE +// commercial license or contractual agreement. + +#include +#include + +#include +#include +#include + +namespace +{ + +//! Simple 1D interval bounding type for testing. +//! Satisfies the UBTree bounding type interface: Add, IsOut, SquareExtent. +struct TestBnd1D +{ + double Min; + double Max; + + TestBnd1D() + : Min(0.0), + Max(0.0) + { + } + + TestBnd1D(double theMin, double theMax) + : Min(theMin), + Max(theMax) + { + } + + void Add(const TestBnd1D& theOther) + { + if (theOther.Min < Min) + Min = theOther.Min; + if (theOther.Max > Max) + Max = theOther.Max; + } + + bool IsOut(const TestBnd1D& theOther) const { return theOther.Min > Max || theOther.Max < Min; } + + double SquareExtent() const + { + double d = Max - Min; + return d * d; + } +}; + +//! Selector that collects all objects overlapping a given query interval. +class TestSelector1D : public NCollection_UBTree::Selector +{ +public: + TestSelector1D(const TestBnd1D& theQuery) + : myQuery(theQuery) + { + } + + bool Reject(const TestBnd1D& theBnd) const override { return myQuery.IsOut(theBnd); } + + bool Accept(const int& theObj) override + { + myResults.push_back(theObj); + return true; + } + + const std::vector& Results() const { return myResults; } + +private: + TestBnd1D myQuery; + std::vector myResults; +}; + +//! Selector that stops after accepting a given number of objects. +class TestStopSelector1D : public NCollection_UBTree::Selector +{ +public: + TestStopSelector1D(const TestBnd1D& theQuery, int theMaxAccept) + : myQuery(theQuery), + myMaxAccept(theMaxAccept), + myAccepted(0) + { + } + + bool Reject(const TestBnd1D& theBnd) const override { return myQuery.IsOut(theBnd); } + + bool Accept(const int& theObj) override + { + myResults.push_back(theObj); + myAccepted++; + if (myAccepted >= myMaxAccept) + myStop = true; + return true; + } + + const std::vector& Results() const { return myResults; } + +private: + TestBnd1D myQuery; + int myMaxAccept; + int myAccepted; + std::vector myResults; +}; + +} // namespace + +// ======================= UBTree Tests ======================= + +TEST(NCollection_UBTreeTest, EmptyTree) +{ + NCollection_UBTree aTree; + + EXPECT_TRUE(aTree.IsEmpty()); + + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(0, nSel); +} + +TEST(NCollection_UBTreeTest, AddAndSelect_SingleObject) +{ + NCollection_UBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 5.0)); + EXPECT_FALSE(aTree.IsEmpty()); + + // Query overlapping + TestSelector1D aSelector(TestBnd1D(3.0, 7.0)); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(1, nSel); + ASSERT_EQ(1u, aSelector.Results().size()); + EXPECT_EQ(1, aSelector.Results()[0]); + + // Query not overlapping + TestSelector1D aSelector2(TestBnd1D(6.0, 10.0)); + int nSel2 = aTree.Select(aSelector2); + EXPECT_EQ(0, nSel2); +} + +TEST(NCollection_UBTreeTest, AddAndSelect_MultipleObjects) +{ + NCollection_UBTree aTree; + + // Add intervals: [0,2], [3,5], [4,6], [8,10] + aTree.Add(1, TestBnd1D(0.0, 2.0)); + aTree.Add(2, TestBnd1D(3.0, 5.0)); + aTree.Add(3, TestBnd1D(4.0, 6.0)); + aTree.Add(4, TestBnd1D(8.0, 10.0)); + + // Query [3.5, 5.5] should overlap with objects 2 and 3 + TestSelector1D aSelector(TestBnd1D(3.5, 5.5)); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(2, nSel); + + std::vector aResults = aSelector.Results(); + std::sort(aResults.begin(), aResults.end()); + ASSERT_EQ(2u, aResults.size()); + EXPECT_EQ(2, aResults[0]); + EXPECT_EQ(3, aResults[1]); + + // Query [0, 10] should overlap all + TestSelector1D aSelector2(TestBnd1D(0.0, 10.0)); + int nSel2 = aTree.Select(aSelector2); + EXPECT_EQ(4, nSel2); +} + +TEST(NCollection_UBTreeTest, StopSelector) +{ + NCollection_UBTree aTree; + + for (int i = 0; i < 10; ++i) + { + aTree.Add(i, TestBnd1D(double(i), double(i + 1))); + } + + // Query overlaps all, but stop after 3 + TestStopSelector1D aSelector(TestBnd1D(0.0, 11.0), 3); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(3, nSel); + EXPECT_EQ(3u, aSelector.Results().size()); +} + +TEST(NCollection_UBTreeTest, Clear) +{ + NCollection_UBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 5.0)); + aTree.Add(2, TestBnd1D(3.0, 7.0)); + + EXPECT_FALSE(aTree.IsEmpty()); + + aTree.Clear(); + EXPECT_TRUE(aTree.IsEmpty()); + + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + EXPECT_EQ(0, aTree.Select(aSelector)); +} + +TEST(NCollection_UBTreeTest, ManyObjects) +{ + NCollection_UBTree aTree; + + const int N = 100; + for (int i = 0; i < N; ++i) + { + aTree.Add(i, TestBnd1D(double(i), double(i + 1))); + } + + // Query that overlaps all + TestSelector1D aSelector(TestBnd1D(0.0, double(N + 1))); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(N, nSel); +} + +TEST(NCollection_UBTreeTest, DeepUnbalancedTree) +{ + // Insert sorted non-overlapping intervals to create a deeply unbalanced tree. + // Each new interval is "out" of all previous bounds, forcing the tree + // to grow on one side -- producing depth ~ N. + NCollection_UBTree aTree; + + const int N = 200; // well over the initial stack size of 64 + for (int i = 0; i < N; ++i) + { + double aLo = double(i) * 10.0; + aTree.Add(i, TestBnd1D(aLo, aLo + 1.0)); + } + + // Select all -- should find all N objects despite deep recursion + TestSelector1D aSelector(TestBnd1D(0.0, double(N) * 10.0)); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(N, nSel); +} + +TEST(NCollection_UBTreeTest, MoveConstructor) +{ + NCollection_UBTree aSrc; + aSrc.Add(1, TestBnd1D(0.0, 2.0)); + aSrc.Add(2, TestBnd1D(3.0, 5.0)); + aSrc.Add(3, TestBnd1D(4.0, 6.0)); + + NCollection_UBTree aDst(std::move(aSrc)); + + // Source is empty + EXPECT_TRUE(aSrc.IsEmpty()); + + // Destination has all objects + EXPECT_FALSE(aDst.IsEmpty()); + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + int nSel = aDst.Select(aSelector); + EXPECT_EQ(3, nSel); +} + +TEST(NCollection_UBTreeTest, MoveAssignment) +{ + NCollection_UBTree aSrc; + aSrc.Add(1, TestBnd1D(0.0, 2.0)); + aSrc.Add(2, TestBnd1D(3.0, 5.0)); + + NCollection_UBTree aDst; + aDst.Add(10, TestBnd1D(100.0, 200.0)); + + aDst = std::move(aSrc); + + EXPECT_TRUE(aSrc.IsEmpty()); + EXPECT_FALSE(aDst.IsEmpty()); + + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + int nSel = aDst.Select(aSelector); + EXPECT_EQ(2, nSel); +} + +// ======================= EBTree Tests ======================= + +TEST(NCollection_EBTreeTest, AddContainsRemove) +{ + NCollection_EBTree aTree; + + EXPECT_TRUE(aTree.IsEmpty()); + + aTree.Add(1, TestBnd1D(0.0, 2.0)); + aTree.Add(2, TestBnd1D(3.0, 5.0)); + aTree.Add(3, TestBnd1D(4.0, 6.0)); + + EXPECT_TRUE(aTree.Contains(1)); + EXPECT_TRUE(aTree.Contains(2)); + EXPECT_TRUE(aTree.Contains(3)); + EXPECT_FALSE(aTree.Contains(4)); + + // Duplicate add should return false + EXPECT_FALSE(aTree.Add(1, TestBnd1D(0.0, 2.0))); + + // Remove object 2 + EXPECT_TRUE(aTree.Remove(2)); + EXPECT_FALSE(aTree.Contains(2)); + EXPECT_TRUE(aTree.Contains(1)); + EXPECT_TRUE(aTree.Contains(3)); + + // Remove non-existent + EXPECT_FALSE(aTree.Remove(4)); +} + +TEST(NCollection_EBTreeTest, RemoveRoot) +{ + NCollection_EBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 5.0)); + EXPECT_TRUE(aTree.Contains(1)); + + // Removing the only element (root) should clear the tree + EXPECT_TRUE(aTree.Remove(1)); + EXPECT_TRUE(aTree.IsEmpty()); +} + +TEST(NCollection_EBTreeTest, SelectAfterRemove) +{ + NCollection_EBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 2.0)); + aTree.Add(2, TestBnd1D(3.0, 5.0)); + aTree.Add(3, TestBnd1D(4.0, 6.0)); + + // Remove object 2 + aTree.Remove(2); + + // Select overlapping [3.5, 5.5] -- only object 3 should remain + TestSelector1D aSelector(TestBnd1D(3.5, 5.5)); + int nSel = aTree.Select(aSelector); + EXPECT_EQ(1, nSel); + ASSERT_EQ(1u, aSelector.Results().size()); + EXPECT_EQ(3, aSelector.Results()[0]); +} + +TEST(NCollection_EBTreeTest, FindNode) +{ + NCollection_EBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 2.0)); + aTree.Add(2, TestBnd1D(3.0, 5.0)); + + const auto& aNode = aTree.FindNode(1); + EXPECT_TRUE(aNode.IsLeaf()); + EXPECT_EQ(1, aNode.Object()); +} + +TEST(NCollection_EBTreeTest, RemoveAll) +{ + NCollection_EBTree aTree; + + aTree.Add(1, TestBnd1D(0.0, 2.0)); + aTree.Add(2, TestBnd1D(3.0, 5.0)); + aTree.Add(3, TestBnd1D(4.0, 6.0)); + + EXPECT_TRUE(aTree.Remove(1)); + EXPECT_TRUE(aTree.Remove(2)); + EXPECT_TRUE(aTree.Remove(3)); + EXPECT_TRUE(aTree.IsEmpty()); +} + +TEST(NCollection_EBTreeTest, MoveConstructor) +{ + NCollection_EBTree aSrc; + aSrc.Add(1, TestBnd1D(0.0, 2.0)); + aSrc.Add(2, TestBnd1D(3.0, 5.0)); + aSrc.Add(3, TestBnd1D(4.0, 6.0)); + + NCollection_EBTree aDst(std::move(aSrc)); + + // Source is empty + EXPECT_TRUE(aSrc.IsEmpty()); + EXPECT_FALSE(aSrc.Contains(1)); + + // Destination has all objects + EXPECT_FALSE(aDst.IsEmpty()); + EXPECT_TRUE(aDst.Contains(1)); + EXPECT_TRUE(aDst.Contains(2)); + EXPECT_TRUE(aDst.Contains(3)); + + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + int nSel = aDst.Select(aSelector); + EXPECT_EQ(3, nSel); +} + +TEST(NCollection_EBTreeTest, MoveAssignment) +{ + NCollection_EBTree aSrc; + aSrc.Add(1, TestBnd1D(0.0, 2.0)); + aSrc.Add(2, TestBnd1D(3.0, 5.0)); + + NCollection_EBTree aDst; + aDst.Add(10, TestBnd1D(100.0, 200.0)); + + aDst = std::move(aSrc); + + EXPECT_TRUE(aSrc.IsEmpty()); + EXPECT_FALSE(aDst.IsEmpty()); + EXPECT_TRUE(aDst.Contains(1)); + EXPECT_TRUE(aDst.Contains(2)); + EXPECT_FALSE(aDst.Contains(10)); + + TestSelector1D aSelector(TestBnd1D(0.0, 10.0)); + int nSel = aDst.Select(aSelector); + EXPECT_EQ(2, nSel); +} diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_CellFilter.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_CellFilter.hxx index 90a98eb5a6..a16d4dbbc7 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_CellFilter.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_CellFilter.hxx @@ -84,7 +84,7 @@ enum NCollection_CellFilter_Action * * - typedef "Point" defining type of geometrical points used * - * - enum Dimension whose value must be dimension of the point + * - static constexpr int Dimension whose value must be dimension of the point * * - method Coord() returning value of the i-th coordinate of the point: * @@ -97,7 +97,7 @@ enum NCollection_CellFilter_Action * bool IsEqual (const Target& theT1, const Target& theT2); * * - method Inspect() performing necessary actions on the candidate target - * object (usially comparison with the currently checked bullet object): + * object (usually comparison with the currently checked bullet object): * * NCollection_CellFilter_Action Inspect (const Target& theObject); * @@ -165,7 +165,7 @@ public: void Add(const Target& theTarget, const Point& thePnt) { Cell aCell(thePnt, myCellSize); - add(aCell, theTarget); + add(aCell.index, theTarget); } //! Adds a target object for further search in the range of cells @@ -176,9 +176,9 @@ public: // get cells range by minimal and maximal coordinates Cell aCellMin(thePntMin, myCellSize); Cell aCellMax(thePntMax, myCellSize); - Cell aCell = aCellMin; + Cell aCell(aCellMin.index); // add object recursively into all cells in range - iterateAdd(myDim - 1, aCell, aCellMin, aCellMax, theTarget); + iterateAdd(myDim - 1, aCell.index, aCellMin, aCellMax, theTarget); } //! Find a target object at a point and remove it from the structures. @@ -199,7 +199,7 @@ public: // get cells range by minimal and maximal coordinates Cell aCellMin(thePntMin, myCellSize); Cell aCellMax(thePntMax, myCellSize); - Cell aCell = aCellMin; + Cell aCell(aCellMin.index); // remove object recursively from all cells in range iterateRemove(myDim - 1, aCell, aCellMin, aCellMax, theTarget); } @@ -219,34 +219,26 @@ public: // get cells range by minimal and maximal coordinates Cell aCellMin(thePntMin, myCellSize); Cell aCellMax(thePntMax, myCellSize); - Cell aCell = aCellMin; + Cell aCell(aCellMin.index); // inspect object recursively into all cells in range iterateInspect(myDim - 1, aCell, aCellMin, aCellMax, theInspector); } -#if defined(__SUNPRO_CC) && (__SUNPRO_CC <= 0x530) -public: // work-around against obsolete SUN WorkShop 5.3 compiler -#else protected: -#endif - /** * Auxiliary class for storing points belonging to the cell as the list */ struct ListNode { - ListNode() - { - // Empty constructor is forbidden. - throw Standard_NoSuchObject("NCollection_CellFilter::ListNode()"); - } + ListNode() = delete; Target Object; ListNode* Next; }; //! Cell index type. - typedef int Cell_IndexType; + typedef int Cell_IndexType; + typedef NCollection_LocalArray CellIndex; /** * Auxiliary structure representing a cell in the space. @@ -274,22 +266,28 @@ protected: } } - //! Copy constructor: ensure that list is not deleted twice - Cell(const Cell& theOther) - : index(theOther.index.Size()) + //! Constructor from cell index; creates a lookup-only cell (no object list). + Cell(const CellIndex& theIndex) + : index(theIndex.Size()), + Objects(nullptr) { - (*this) = theOther; + std::memcpy(index, theIndex, theIndex.Size() * sizeof(Cell_IndexType)); } - //! Assignment operator: ensure that list is not deleted twice - void operator=(const Cell& theOther) noexcept + //! Move constructor: transfers ownership of the object list + Cell(Cell&& theOther) noexcept + : index(std::move(theOther.index)), + Objects(theOther.Objects) { - int aDim = int(theOther.index.Size()); - for (int anIdx = 0; anIdx < aDim; anIdx++) - index[anIdx] = theOther.index[anIdx]; + theOther.Objects = nullptr; + } - Objects = theOther.Objects; - ((Cell&)theOther).Objects = nullptr; + Cell& operator=(Cell&& theOther) noexcept + { + index = std::move(theOther.index); + Objects = theOther.Objects; + theOther.Objects = nullptr; + return *this; } //! Destructor; calls destructors for targets contained in the list @@ -304,8 +302,10 @@ protected: //! Compare cell with other one bool IsEqual(const Cell& theOther) const noexcept { - int aDim = int(theOther.index.Size()); - for (int i = 0; i < aDim; i++) + const size_t aDim = index.Size(); + if (aDim != theOther.index.Size()) + return false; + for (size_t i = 0; i < aDim; i++) if (index[i] != theOther.index[i]) return false; return true; @@ -314,15 +314,14 @@ protected: bool operator==(const Cell& theOther) const noexcept { return IsEqual(theOther); } public: - NCollection_LocalArray index; - ListNode* Objects; + CellIndex index; + ListNode* Objects; }; struct CellHasher { size_t operator()(const Cell& theCell) const noexcept { - // number of bits per each dimension in the hash code const std::size_t aDim = theCell.index.Size(); return opencascade::hashBytes(&theCell.index[0], static_cast(aDim * sizeof(Cell_IndexType))); @@ -348,10 +347,10 @@ protected: } //! Add a new target object into the specified cell - void add(const Cell& theCell, const Target& theTarget) + void add(const CellIndex& theIndex, const Target& theTarget) { // add a new cell or get reference to existing one - Cell& aMapCell = (Cell&)myCells.Added(theCell); + Cell& aMapCell = const_cast(myCells.TryEmplaced(theIndex)); // create a new list node and add it to the beginning of the list ListNode* aNode = (ListNode*)myAllocator->Allocate(sizeof(ListNode)); @@ -363,23 +362,23 @@ protected: //! Internal addition function, performing iteration for adjacent cells //! by one dimension; called recursively to cover all dimensions void iterateAdd(int idim, - Cell& theCell, - const Cell& theCellMin, - const Cell& theCellMax, + CellIndex& theIndex, + const Cell& theMinIndex, + const Cell& theMaxIndex, const Target& theTarget) { - const Cell_IndexType aStart = theCellMin.index[idim]; - const Cell_IndexType anEnd = theCellMax.index[idim]; + const Cell_IndexType aStart = theMinIndex.index[idim]; + const Cell_IndexType anEnd = theMaxIndex.index[idim]; for (Cell_IndexType i = aStart; i <= anEnd; ++i) { - theCell.index[idim] = i; + theIndex[idim] = i; if (idim) // recurse { - iterateAdd(idim - 1, theCell, theCellMin, theCellMax, theTarget); + iterateAdd(idim - 1, theIndex, theMinIndex, theMaxIndex, theTarget); } else // add to this cell { - add(theCell, theTarget); + add(theIndex, theTarget); } } } @@ -387,14 +386,16 @@ protected: //! Remove the target object from the specified cell void remove(const Cell& theCell, const Target& theTarget) { - // check if any objects are recorded in that cell - if (!myCells.Contains(theCell)) + // Modifying the Objects field does not affect the hash, const_cast is safe + auto aMapCellOpt = myCells.Contained(theCell); + if (!aMapCellOpt) return; + Cell& aMapCell = const_cast(aMapCellOpt->get()); + // iterate by objects in the cell and check each - Cell& aMapCell = (Cell&)myCells.Added(theCell); - ListNode* aNode = aMapCell.Objects; - ListNode* aPrev = nullptr; + ListNode* aNode = aMapCell.Objects; + ListNode* aPrev = nullptr; while (aNode) { ListNode* aNext = aNode->Next; @@ -408,6 +409,10 @@ protected: aPrev = aNode; aNode = aNext; } + + // cleanup empty cell to prevent dead cell accumulation + if (!aMapCell.Objects) + myCells.Remove(theCell); } //! Internal removal function, performing iteration for adjacent cells @@ -437,14 +442,16 @@ protected: //! Inspect the target objects in the specified cell. void inspect(const Cell& theCell, Inspector& theInspector) { - // check if any objects are recorded in that cell - if (!myCells.Contains(theCell)) + // Modifying the Objects field does not affect the hash, const_cast is safe + auto aMapCellOpt = myCells.Contained(theCell); + if (!aMapCellOpt) return; + Cell& aMapCell = const_cast(aMapCellOpt->get()); + // iterate by objects in the cell and check each - Cell& aMapCell = (Cell&)myCells.Added(theCell); - ListNode* aNode = aMapCell.Objects; - ListNode* aPrev = nullptr; + ListNode* aNode = aMapCell.Objects; + ListNode* aPrev = nullptr; while (aNode) { ListNode* aNext = aNode->Next; @@ -460,6 +467,10 @@ protected: aPrev = aNode; aNode = aNext; } + + // cleanup empty cell to prevent dead cell accumulation + if (!aMapCell.Objects) + myCells.Remove(theCell); } //! Inspect the target objects in the specified range of the cells @@ -492,62 +503,4 @@ protected: NCollection_Array1 myCellSize; }; -/** - * Base class defining part of the Inspector interface - * for CellFilter algorithm, working with gp_XYZ points in 3d space - */ - -class gp_XYZ; - -struct NCollection_CellFilter_InspectorXYZ -{ - //! Points dimension - enum - { - Dimension = 3 - }; - - //! Points type - typedef gp_XYZ Point; - - //! Access to coordinate - static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } - - //! Auxiliary method to shift point by each coordinate on given value; - //! useful for preparing a points range for Inspect with tolerance - Point Shift(const Point& thePnt, double theTol) const - { - return Point(thePnt.X() + theTol, thePnt.Y() + theTol, thePnt.Z() + theTol); - } -}; - -/** - * Base class defining part of the Inspector interface - * for CellFilter algorithm, working with gp_XY points in 2d space - */ - -class gp_XY; - -struct NCollection_CellFilter_InspectorXY -{ - //! Points dimension - enum - { - Dimension = 2 - }; - - //! Points type - typedef gp_XY Point; - - //! Access to coordinate - static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } - - //! Auxiliary method to shift point by each coordinate on given value; - //! useful for preparing a points range for Inspect with tolerance - Point Shift(const Point& thePnt, double theTol) const - { - return Point(thePnt.X() + theTol, thePnt.Y() + theTol); - } -}; - #endif diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_DataMap.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_DataMap.hxx index c02cc19ade..31454a5827 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_DataMap.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_DataMap.hxx @@ -24,6 +24,8 @@ #include #include +#include +#include #include #include @@ -570,6 +572,30 @@ public: return lookup(theKey, p); } + //! Contained returns optional pair of const references to key and value. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey) const + { + DataMapNode* p = nullptr; + if (!lookup(theKey, p)) + return std::nullopt; + return std::make_pair(std::cref(p->Key()), std::cref(p->Value())); + } + + //! Contained returns optional pair of const key reference and mutable value reference. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey) + { + DataMapNode* p = nullptr; + if (!lookup(theKey, p)) + return std::nullopt; + return std::make_pair(std::cref(p->Key()), std::ref(p->ChangeValue())); + } + //! UnBind removes Item Key pair from map bool UnBind(const TheKeyType& theKey) { diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_DefineAlloc.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_DefineAlloc.hxx index 72250e0b3d..bbfb92d710 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_DefineAlloc.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_DefineAlloc.hxx @@ -19,25 +19,15 @@ #include // Macro to overload placement new and delete operators for NCollection allocators. -// For Borland C and old SUN compilers do not define placement delete -// as it is not supported. -#if defined(__BORLANDC__) || (defined(__SUNPRO_CC) && (__SUNPRO_CC <= 0x530)) - #define DEFINE_NCOLLECTION_ALLOC \ - void* operator new(size_t theSize, const occ::handle& theAllocator) \ - { \ - return theAllocator->Allocate(theSize); \ - } -#else - #define DEFINE_NCOLLECTION_ALLOC \ - void* operator new(size_t theSize, const occ::handle& theAllocator) \ - { \ - return theAllocator->Allocate(theSize); \ - } \ - void operator delete(void* theAddress, \ - const occ::handle& theAllocator) \ - { \ - theAllocator->Free(theAddress); \ - } -#endif +#define DEFINE_NCOLLECTION_ALLOC \ + void* operator new(size_t theSize, const occ::handle& theAllocator) \ + { \ + return theAllocator->Allocate(theSize); \ + } \ + void operator delete(void* theAddress, \ + const occ::handle& theAllocator) \ + { \ + theAllocator->Free(theAddress); \ + } #endif diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_EBTree.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_EBTree.hxx index ed5be887f4..90afb1cde3 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_EBTree.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_EBTree.hxx @@ -17,11 +17,6 @@ #define NCollection_EBTree_HeaderFile #include -#include -#include -#include -#include -#include #include /** @@ -51,6 +46,22 @@ public: { } + NCollection_EBTree(NCollection_EBTree&& theOther) noexcept + : UBTree(std::move(theOther)), + myObjNodeMap(std::move(theOther.myObjNodeMap)) + { + } + + NCollection_EBTree& operator=(NCollection_EBTree&& theOther) noexcept + { + if (this != &theOther) + { + UBTree::operator=(std::move(theOther)); + myObjNodeMap = std::move(theOther.myObjNodeMap); + } + return *this; + } + /** * Updates the tree with a new object and its bounding box. * Extends the functionality of the parent method by maintaining @@ -69,14 +80,16 @@ public: // Update the map TreeNode& aNewNode = this->ChangeLastNode(); myObjNodeMap.Bind(theObj, &aNewNode); - // If the new node is not the root (has a parent) check the neighbour node + // If the new node is not the root (has a parent) check the neighbour node. + // Gemmate moved old content to child(0), so the map entry points to stale address. if (!aNewNode.IsRoot()) { TreeNode& aNeiNode = aNewNode.ChangeParent().ChangeChild(0); if (aNeiNode.IsLeaf()) { - myObjNodeMap.UnBind(aNeiNode.Object()); - myObjNodeMap.Bind(aNeiNode.Object(), &aNeiNode); + TreeNode** aNeiPtr = myObjNodeMap.ChangeSeek(aNeiNode.Object()); + if (aNeiPtr) + *aNeiPtr = &aNeiNode; } } result = true; @@ -126,107 +139,46 @@ private: NCollection_DataMap myObjNodeMap; ///< map of object to node pointer }; -// ================== METHODS TEMPLATES ===================== - -//======================================================================= -// function : Remove -// purpose : Removes the given object and updates the tree. -// Returns false if the tree does not contain theObj. -//======================================================================= +//================================================================================================== template bool NCollection_EBTree::Remove(const TheObjType& theObj) { - bool result = false; - if (Contains(theObj)) + // Single lookup using ChangeSeek instead of Contains() + operator()() + TreeNode** pNodePtr = myObjNodeMap.ChangeSeek(theObj); + if (!pNodePtr) + return false; + + TreeNode* pNode = *pNodePtr; + if (pNode->IsRoot()) { - TreeNode* pNode = myObjNodeMap(theObj); - if (pNode->IsRoot()) - { - // it is the root, so clear all the tree - Clear(); - } - else - { - // it is a child of some parent, - // so kill the child that contains theObj - // and update bounding boxes of all ancestors - myObjNodeMap.UnBind(theObj); - TreeNode* pParent = &pNode->ChangeParent(); - pParent->Kill((pNode == &pParent->Child(0) ? 0 : 1), this->Allocator()); - if (pParent->IsLeaf()) - { - // the parent node became a leaf, so update the map - myObjNodeMap.UnBind(pParent->Object()); - myObjNodeMap.Bind(pParent->Object(), pParent); - } - while (!pParent->IsRoot()) - { - pParent = &pParent->ChangeParent(); - pParent->ChangeBnd() = pParent->Child(0).Bnd(); - pParent->ChangeBnd().Add(pParent->Child(1).Bnd()); - } - } - result = true; + // it is the root, so clear all the tree + Clear(); } - return result; + else + { + // it is a child of some parent, + // so kill the child that contains theObj + // and update bounding boxes of all ancestors + myObjNodeMap.UnBind(theObj); + TreeNode* pParent = &pNode->ChangeParent(); + pParent->Kill((pNode == &pParent->Child(0) ? 0 : 1), this->Allocator()); + if (pParent->IsLeaf()) + { + // The parent node became a leaf (absorbed surviving child), + // so update the map entry to point to the new address. + TreeNode** aParentPtr = myObjNodeMap.ChangeSeek(pParent->Object()); + if (aParentPtr) + *aParentPtr = pParent; + } + while (!pParent->IsRoot()) + { + pParent = &pParent->ChangeParent(); + pParent->ChangeBnd() = pParent->Child(0).Bnd(); + pParent->ChangeBnd().Add(pParent->Child(1).Bnd()); + } + } + return true; } -// ====================================================================== -// Declaration of handled version of NCollection_EBTree. -// In the macros below the arguments are: -// _HEBTREE - the desired name of handled class -// _OBJTYPE - the name of the object type -// _BNDTYPE - the name of the bounding box type -// _HUBTREE - the name of parent class -// (defined using macro DEFINE_HUBTREE) - -#define DEFINE_HEBTREE(_HEBTREE, _OBJTYPE, _BNDTYPE, _HUBTREE) \ - class _HEBTREE : public _HUBTREE \ - { \ - public: \ - typedef NCollection_UBTree<_OBJTYPE, _BNDTYPE> UBTree; \ - typedef NCollection_EBTree<_OBJTYPE, _BNDTYPE> EBTree; \ - \ - _HEBTREE() \ - : _HUBTREE(new EBTree) \ - { \ - } \ - /* Empty constructor */ \ - \ - /* Access to the methods of EBTree */ \ - \ - bool Remove(const _OBJTYPE& theObj) \ - { \ - return ChangeETree().Remove(theObj); \ - } \ - \ - bool Contains(const _OBJTYPE& theObj) const \ - { \ - return ETree().Contains(theObj); \ - } \ - \ - const UBTree::TreeNode& FindNode(const _OBJTYPE& theObj) const \ - { \ - return ETree().FindNode(theObj); \ - } \ - \ - /* Access to the extended tree algorithm */ \ - \ - const EBTree& ETree() const noexcept \ - { \ - return (const EBTree&)Tree(); \ - } \ - EBTree& ChangeETree() noexcept \ - { \ - return (EBTree&)ChangeTree(); \ - } \ - \ - DEFINE_STANDARD_RTTI_INLINE(_HEBTREE, _HUBTREE) \ - /* Type management */ \ - }; \ - DEFINE_STANDARD_HANDLE(_HEBTREE, _HUBTREE) - -#define IMPLEMENT_HEBTREE(_HEBTREE, _HUBTREE) - #endif diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_FlatDataMap.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_FlatDataMap.hxx index 6352648aa8..6e89362aac 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_FlatDataMap.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_FlatDataMap.hxx @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -371,6 +372,34 @@ public: return findSlot(theKey).has_value(); } + //! Contained returns optional pair of const references to key and value. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey) const + { + if (mySize == 0) + return std::nullopt; + const std::optional aIdx = findSlot(theKey); + if (!aIdx.has_value()) + return std::nullopt; + return std::make_pair(std::cref(mySlots[*aIdx].Key()), std::cref(mySlots[*aIdx].Item())); + } + + //! Contained returns optional pair of const key reference and mutable value reference. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey) + { + if (mySize == 0) + return std::nullopt; + const std::optional aIdx = findSlot(theKey); + if (!aIdx.has_value()) + return std::nullopt; + return std::make_pair(std::cref(mySlots[*aIdx].Key()), std::ref(mySlots[*aIdx].Item())); + } + //! Find value by key, returns nullptr if not found const TheItemType* Seek(const TheKeyType& theKey) const { diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_FlatMap.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_FlatMap.hxx index 8e235d2b0d..fdfb3d5319 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_FlatMap.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_FlatMap.hxx @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -338,6 +339,40 @@ public: return findSlot(theKey).has_value(); } + //! Contained returns optional const reference to the key in the map. + //! Returns std::nullopt if the key is not found. + std::optional> Contained(const TheKeyType& theKey) const + { + if (mySize == 0) + return std::nullopt; + const std::optional aIdx = findSlot(theKey); + if (!aIdx.has_value()) + return std::nullopt; + return std::cref(mySlots[*aIdx].Key()); + } + + //! Seek returns pointer to key in map. Returns NULL if not found. + const TheKeyType* Seek(const TheKeyType& theKey) const + { + if (mySize == 0) + return nullptr; + const std::optional aIdx = findSlot(theKey); + if (!aIdx.has_value()) + return nullptr; + return &mySlots[*aIdx].Key(); + } + + //! ChangeSeek returns modifiable pointer to key in map. Returns NULL if not found. + TheKeyType* ChangeSeek(const TheKeyType& theKey) + { + if (mySize == 0) + return nullptr; + const std::optional aIdx = findSlot(theKey); + if (!aIdx.has_value()) + return nullptr; + return &mySlots[*aIdx].Key(); + } + public: // **************** Modification methods **************** @@ -398,6 +433,28 @@ public: return emplaceImpl(std::move(aTempKey), std::false_type{}, std::true_type{}); } + //! TryEmplace constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return true if key was newly added, false if key already existed + template + bool TryEmplace(Args&&... theArgs) + { + ensureCapacity(); + TheKeyType aTempKey(std::forward(theArgs)...); + return emplaceImpl(std::move(aTempKey), std::true_type{}, std::false_type{}); + } + + //! TryEmplaced constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return const reference to the key (existing or newly added) + template + const TheKeyType& TryEmplaced(Args&&... theArgs) + { + ensureCapacity(); + TheKeyType aTempKey(std::forward(theArgs)...); + return emplaceImpl(std::move(aTempKey), std::true_type{}, std::true_type{}); + } + //! Remove key from set //! @return true if key was found and removed bool Remove(const TheKeyType& theKey) diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedDataMap.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedDataMap.hxx index 38c77744fb..5bf097631c 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedDataMap.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedDataMap.hxx @@ -25,6 +25,8 @@ #include #include +#include +#include #include #include @@ -515,6 +517,33 @@ public: return bindImpl(std::move(theKey1), std::move(theItem), std::true_type{}, std::true_type{}); } + //! TryBind binds Item to Key only if Key is not yet bound. + //! @param theKey1 key to add + //! @param theItem item to bind if Key is not yet bound + //! @return true if key was newly added, false if key already existed + bool TryBind(const TheKeyType& theKey1, const TheItemType& theItem) + { + return bindImpl(theKey1, theItem, std::true_type{}, std::false_type{}); + } + + //! TryBind binds Item to Key only if Key is not yet bound. + bool TryBind(TheKeyType&& theKey1, const TheItemType& theItem) + { + return bindImpl(std::move(theKey1), theItem, std::true_type{}, std::false_type{}); + } + + //! TryBind binds Item to Key only if Key is not yet bound. + bool TryBind(const TheKeyType& theKey1, TheItemType&& theItem) + { + return bindImpl(theKey1, std::move(theItem), std::true_type{}, std::false_type{}); + } + + //! TryBind binds Item to Key only if Key is not yet bound. + bool TryBind(TheKeyType&& theKey1, TheItemType&& theItem) + { + return bindImpl(std::move(theKey1), std::move(theItem), std::true_type{}, std::false_type{}); + } + //! Bind binds Item to Key in map; overwrites value if Key already exists. //! @param theKey1 key to add/update //! @param theItem new item; overrides value previously bound to the key @@ -628,6 +657,30 @@ public: return static_cast(lookup(theKey1, aNode)); } + //! Contained returns optional pair of const references to key and value. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey1) const + { + IndexedDataMapNode* aNode; + if (!lookup(theKey1, aNode)) + return std::nullopt; + return std::make_pair(std::cref(aNode->Key()), std::cref(aNode->Value())); + } + + //! Contained returns optional pair of const key reference and mutable value reference. + //! Returns std::nullopt if the key is not found. + std::optional< + std::pair, std::reference_wrapper>> + Contained(const TheKeyType& theKey1) + { + IndexedDataMapNode* aNode; + if (!lookup(theKey1, aNode)) + return std::nullopt; + return std::make_pair(std::cref(aNode->Key()), std::ref(aNode->ChangeValue())); + } + //! Substitute void Substitute(const int theIndex, const TheKeyType& theKey1, const TheItemType& theItem) { diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedMap.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedMap.hxx index efbfbef08e..b5533c8c74 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedMap.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_IndexedMap.hxx @@ -25,6 +25,8 @@ #include #include +#include +#include #include /** @@ -373,6 +375,24 @@ public: return emplaceImpl(std::false_type{}, std::true_type{}, std::forward(theArgs)...); } + //! TryEmplace constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return index of the key (new or existing) + template + int TryEmplace(Args&&... theArgs) + { + return emplaceImpl(std::true_type{}, std::false_type{}, std::forward(theArgs)...); + } + + //! TryEmplaced constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return const reference to the key (existing or newly added) + template + const TheKeyType& TryEmplaced(Args&&... theArgs) + { + return emplaceImpl(std::true_type{}, std::true_type{}, std::forward(theArgs)...); + } + //! Contains bool Contains(const TheKeyType& theKey1) const { @@ -380,6 +400,16 @@ public: return lookup(theKey1, p); } + //! Contained returns optional const reference to the key in the map. + //! Returns std::nullopt if the key is not found. + std::optional> Contained(const TheKeyType& theKey1) const + { + IndexedMapNode* p; + if (!lookup(theKey1, p)) + return std::nullopt; + return std::cref(p->Value()); + } + //! Substitute void Substitute(const int theIndex, const TheKeyType& theKey1) { diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx index 4a4aa6412b..41ba425d7a 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx @@ -18,14 +18,22 @@ #include #include +#include +#include +#include + //! Auxiliary class optimizing creation of array buffer //! (using stack allocation for small arrays). template class NCollection_LocalArray { + static_assert(std::is_trivially_copyable::value, + "NCollection_LocalArray uses memcpy/realloc and requires trivially copyable types"); + public: explicit NCollection_LocalArray(const size_t theSize) - : myPtr(myBuffer) + : myPtr(myBuffer), + mySize(0) { Allocate(theSize); } @@ -38,22 +46,114 @@ public: ~NCollection_LocalArray() { Deallocate(); } - void Allocate(const size_t theSize) - { - Deallocate(); - if (theSize > MAX_ARRAY_SIZE) - myPtr = (theItem*)Standard::Allocate(theSize * sizeof(theItem)); - else - myPtr = myBuffer; + void Allocate(const size_t theSize) { Reallocate(theSize, false); } - mySize = theSize; + //! Reallocate the array to a new size. + //! @param[in] theNewSize new number of elements + //! @param[in] theToCopy if true, existing elements are copied to the new buffer + void Reallocate(const size_t theNewSize, bool theToCopy = true) + { + if (theNewSize <= mySize) + { + // Shrinking - just update the logical size, keep existing allocation + mySize = theNewSize; + return; + } + + const bool isOnHeap = (myPtr != myBuffer); + const size_t aNewBytes = theNewSize * sizeof(theItem); + + if (theNewSize <= static_cast(MAX_ARRAY_SIZE)) + { + // New size fits in stack buffer + if (isOnHeap) + { + if (theToCopy && mySize > 0) + { + std::memcpy(myBuffer, myPtr, std::min(mySize, theNewSize) * sizeof(theItem)); + } + Standard::Free(myPtr); + myPtr = myBuffer; + } + mySize = theNewSize; + return; + } + + if (isOnHeap) + { + // Already on heap - use Standard::Reallocate (preserves content when growing) + if (theToCopy) + { + myPtr = (theItem*)Standard::Reallocate(myPtr, aNewBytes); + } + else + { + Standard::Free(myPtr); + myPtr = (theItem*)Standard::Allocate(aNewBytes); + } + } + else + { + // Stack to heap transition + myPtr = (theItem*)Standard::Allocate(aNewBytes); + if (theToCopy && mySize > 0) + { + std::memcpy(myPtr, myBuffer, std::min(mySize, theNewSize) * sizeof(theItem)); + } + } + mySize = theNewSize; } size_t Size() const noexcept { return mySize; } operator theItem*() const noexcept { return myPtr; } -private: + NCollection_LocalArray(NCollection_LocalArray&& theOther) noexcept + : myPtr(myBuffer), + mySize(theOther.mySize) + { + if (theOther.myPtr == theOther.myBuffer) + { + std::memcpy(myBuffer, theOther.myBuffer, mySize * sizeof(theItem)); + } + else + { + myPtr = theOther.myPtr; + theOther.myPtr = theOther.myBuffer; + } + theOther.mySize = 0; + } + + NCollection_LocalArray& operator=(NCollection_LocalArray&& theOther) noexcept + { + if (this != &theOther) + { + mySize = theOther.mySize; + if (theOther.myPtr == theOther.myBuffer) + { + // Source on stack: copy data to our buffer + Deallocate(); + myPtr = myBuffer; + std::memcpy(myBuffer, theOther.myBuffer, mySize * sizeof(theItem)); + } + else if (myPtr != myBuffer) + { + // Both on heap: swap pointers, theOther frees our old allocation on destruction + theItem* anOldPtr = myPtr; + myPtr = theOther.myPtr; + theOther.myPtr = anOldPtr; + } + else + { + // this on stack, theOther on heap: take the pointer + myPtr = theOther.myPtr; + theOther.myPtr = theOther.myBuffer; + } + theOther.mySize = 0; + } + return *this; + } + NCollection_LocalArray(const NCollection_LocalArray&) = delete; NCollection_LocalArray& operator=(const NCollection_LocalArray&) = delete; diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_Map.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_Map.hxx index 242b074c9e..1f5868de20 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_Map.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_Map.hxx @@ -23,6 +23,8 @@ #include #include +#include +#include #include #include @@ -316,6 +318,24 @@ public: return emplaceImpl(std::false_type{}, std::true_type{}, std::forward(theArgs)...); } + //! TryEmplace constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return true if key was newly added, false if key already existed + template + bool TryEmplace(Args&&... theArgs) + { + return emplaceImpl(std::true_type{}, std::false_type{}, std::forward(theArgs)...); + } + + //! TryEmplaced constructs key in-place only if not already present. + //! @param theArgs arguments forwarded to key constructor + //! @return const reference to the key (existing or newly added) + template + const TheKeyType& TryEmplaced(Args&&... theArgs) + { + return emplaceImpl(std::true_type{}, std::true_type{}, std::forward(theArgs)...); + } + //! Contains bool Contains(const TheKeyType& theKey) const { @@ -323,6 +343,16 @@ public: return lookup(theKey, p); } + //! Contained returns optional const reference to the key in the map. + //! Returns std::nullopt if the key is not found. + std::optional> Contained(const TheKeyType& theKey) const + { + MapNode* p; + if (!lookup(theKey, p)) + return std::nullopt; + return std::cref(p->Key()); + } + //! Remove bool Remove(const TheKeyType& K) { diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx index a77d7e6307..d44b0cdc43 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_SparseArrayBase.hxx @@ -47,12 +47,7 @@ public: //!@} -#if defined(__SUNPRO_CC) && (__SUNPRO_CC <= 0x530) -public: // work-around against obsolete SUN WorkShop 5.3 compiler -#else private: -#endif - /** * The block of data contains array of items, counter * and bit field, allocated as single piece of memory addressed diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_UBTree.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_UBTree.hxx index 316cade868..7acfef43e0 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_UBTree.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_UBTree.hxx @@ -18,6 +18,7 @@ #include #include +#include /** * The algorithm of unbalanced binary tree of overlapped bounding boxes. @@ -171,7 +172,7 @@ public: * added object. * @param theBnd * bounding box of theObj. - * @theAlloc + * @param theAlloc * allocator providing memory to the new child nodes, provided by the * calling Tree instance. */ @@ -180,7 +181,6 @@ public: const TheBndType& theBnd, const occ::handle& theAlloc) { - // TreeNode *children = new TreeNode [2]; TreeNode* children = (TreeNode*)theAlloc->Allocate(2 * sizeof(TreeNode)); new (&children[0]) TreeNode; new (&children[1]) TreeNode; @@ -215,33 +215,53 @@ public: myChildren[0].myParent = this; myChildren[1].myParent = this; } - // oldChildren[0].myChildren = oldChildren[1].myChildren = 0L; - // delete [] oldChildren; oldChildren[iopp].~TreeNode(); delNode(&oldChildren[i], theAlloc); // remove the whole branch theAlloc->Free(oldChildren); } } - // ~TreeNode () { if (myChildren) delete [] myChildren; } ~TreeNode() { myChildren = nullptr; } - /** - * Deleter of tree node. The whole hierarchy of its children also deleted. - * This method should be used instead of operator delete. - */ + //! Deleter of tree node. The whole hierarchy of its children is also deleted. + //! This method should be used instead of operator delete. + //! Uses iterative traversal to avoid stack overflow on deeply unbalanced trees. static void delNode(TreeNode* theNode, const occ::handle& theAlloc) { - if (theNode) + if (!theNode) + return; + + // Collect children arrays during pre-order traversal, free them after. + constexpr int THE_INIT_STACK_SIZE = 64; + NCollection_LocalArray aChildArrays(THE_INIT_STACK_SIZE); + NCollection_LocalArray aStack(THE_INIT_STACK_SIZE); + int aNumArrays = 0; + int aTop = 0; + + aStack[aTop++] = theNode; + + while (aTop > 0) { - if (theNode->myChildren) + TreeNode* aNode = aStack[--aTop]; + if (aNode->myChildren) { - delNode(&theNode->myChildren[0], theAlloc); - delNode(&theNode->myChildren[1], theAlloc); - theAlloc->Free(theNode->myChildren); + // Record children array for later freeing + if (aNumArrays >= static_cast(aChildArrays.Size())) + aChildArrays.Reallocate(aChildArrays.Size() * 2, true); + aChildArrays[aNumArrays++] = aNode->myChildren; + + // Push both children for traversal + if (aTop + 2 > static_cast(aStack.Size())) + aStack.Reallocate(aStack.Size() * 2, true); + aStack[aTop++] = &aNode->myChildren[1]; + aStack[aTop++] = &aNode->myChildren[0]; } - theNode->~TreeNode(); + aNode->~TreeNode(); } + + // Free all collected children arrays + for (int i = 0; i < aNumArrays; ++i) + theAlloc->Free(aChildArrays[i]); } private: @@ -280,6 +300,29 @@ public: { } + NCollection_UBTree(NCollection_UBTree&& theOther) noexcept + : myRoot(theOther.myRoot), + myLastNode(theOther.myLastNode), + myAlloc(std::move(theOther.myAlloc)) + { + theOther.myRoot = nullptr; + theOther.myLastNode = nullptr; + } + + NCollection_UBTree& operator=(NCollection_UBTree&& theOther) noexcept + { + if (this != &theOther) + { + Clear(); + myRoot = theOther.myRoot; + myLastNode = theOther.myLastNode; + myAlloc = std::move(theOther.myAlloc); + theOther.myRoot = nullptr; + theOther.myLastNode = nullptr; + } + return *this; + } + /** * Update the tree with a new object and its bounding box. * @param theObj @@ -293,7 +336,7 @@ public: /** * Searches in the tree all objects conforming to the given selector. - * return + * @return * Number of objects accepted */ virtual int Select(Selector& theSelector) const @@ -310,7 +353,6 @@ public: * kept. */ virtual void Clear(const occ::handle& aNewAlloc = nullptr) - // { if (myRoot) delete myRoot; myRoot = 0L; } { if (myRoot) { @@ -374,11 +416,7 @@ private: occ::handle myAlloc; ///< Allocator for TreeNode }; -// ================== METHODS TEMPLATES ===================== -//======================================================================= -// function : Add -// purpose : Updates the tree with a new object and its bounding box -//======================================================================= +//================================================================================================== template bool NCollection_UBTree::Add(const TheObjType& theObj, @@ -436,124 +474,46 @@ bool NCollection_UBTree::Add(const TheObjType& theObj, return true; } -//======================================================================= -// function : Select -// purpose : Recursively searches in the branch all objects conforming -// to the given selector. -// Returns the number of objects found. -//======================================================================= +//================================================================================================== template int NCollection_UBTree::Select(const TreeNode& theBranch, Selector& theSelector) const { - // Try to reject the branch by bounding box - if (theSelector.Reject(theBranch.Bnd())) - return 0; + // Explicit stack for iterative DFS. Covers balanced trees up to 2^64 nodes; + // Reallocate handles deeply unbalanced trees. + constexpr int THE_INIT_STACK_SIZE = 64; + NCollection_LocalArray aStack(THE_INIT_STACK_SIZE); + int aTop = 0; + int nSel = 0; - int nSel = 0; + aStack[aTop++] = &theBranch; - if (theBranch.IsLeaf()) + while (aTop > 0) { - // It is a leaf => try to accept the object - if (theSelector.Accept(theBranch.Object())) - nSel++; - } - else - { - // It is a branch => select from its children - nSel += Select(theBranch.Child(0), theSelector); - if (!theSelector.Stop()) - nSel += Select(theBranch.Child(1), theSelector); - } + const TreeNode* aNode = aStack[--aTop]; + if (theSelector.Reject(aNode->Bnd())) + continue; + + if (aNode->IsLeaf()) + { + if (theSelector.Accept(aNode->Object())) + nSel++; + if (theSelector.Stop()) + break; + } + else + { + // Ensure stack has space for 2 children + if (aTop + 2 > static_cast(aStack.Size())) + aStack.Reallocate(aStack.Size() * 2, true); + // Push child(1) first so child(0) is processed first (LIFO order) + aStack[aTop++] = &aNode->Child(1); + aStack[aTop++] = &aNode->Child(0); + } + } return nSel; } -// ====================================================================== -/** - * Declaration of handled version of NCollection_UBTree. - * In the macros below the arguments are: - * _HUBTREE - the desired name of handled class - * _OBJTYPE - the name of the object type - * _BNDTYPE - the name of the bounding box type - * _HPARENT - the name of parent class (usually Standard_Transient) - */ -#define DEFINE_HUBTREE(_HUBTREE, _OBJTYPE, _BNDTYPE, _HPARENT) \ - class _HUBTREE : public _HPARENT \ - { \ - public: \ - typedef NCollection_UBTree<_OBJTYPE, _BNDTYPE> UBTree; \ - \ - _HUBTREE() \ - : myTree(new UBTree) \ - { \ - } \ - /* Empty constructor */ \ - _HUBTREE(const occ::handle& theAlloc) \ - : myTree(new UBTree(theAlloc)) \ - { \ - } \ - /* Constructor */ \ - \ - /* Access to the methods of UBTree */ \ - \ - bool Add(const _OBJTYPE& theObj, const _BNDTYPE& theBnd) \ - { \ - return ChangeTree().Add(theObj, theBnd); \ - } \ - \ - int Select(UBTree::Selector& theSelector) const \ - { \ - return Tree().Select(theSelector); \ - } \ - \ - void Clear() \ - { \ - ChangeTree().Clear(); \ - } \ - \ - bool IsEmpty() const noexcept \ - { \ - return Tree().IsEmpty(); \ - } \ - \ - const UBTree::TreeNode& Root() const \ - { \ - return Tree().Root(); \ - } \ - \ - /* Access to the tree algorithm */ \ - \ - const UBTree& Tree() const noexcept \ - { \ - return *myTree; \ - } \ - UBTree& ChangeTree() noexcept \ - { \ - return *myTree; \ - } \ - \ - ~_HUBTREE() \ - { \ - delete myTree; \ - } \ - /* Destructor */ \ - \ - DEFINE_STANDARD_RTTI_INLINE(_HUBTREE, _HPARENT) \ - /* Type management */ \ - \ - private: \ - /* Copying and assignment are prohibited */ \ - _HUBTREE(UBTree*); \ - _HUBTREE(const _HUBTREE&); \ - void operator=(const _HUBTREE&); \ - \ - private: \ - UBTree* myTree; /* pointer to the tree algorithm */ \ - }; \ - DEFINE_STANDARD_HANDLE(_HUBTREE, _HPARENT) - -#define IMPLEMENT_HUBTREE(_HUBTREE, _HPARENT) - #endif diff --git a/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_CircleInspector.hxx b/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_CircleInspector.hxx index 2e486300da..d965952ccd 100644 --- a/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_CircleInspector.hxx +++ b/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_CircleInspector.hxx @@ -22,10 +22,20 @@ #include //! Auxiliary class to find circles shot by the given point. -class BRepMesh_CircleInspector : public NCollection_CellFilter_InspectorXY +class BRepMesh_CircleInspector { public: - typedef int Target; + static constexpr int Dimension = 2; + + typedef gp_XY Point; + typedef int Target; + + static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } + + static Point Shift(const Point& thePnt, double theTol) + { + return Point(thePnt.X() + theTol, thePnt.Y() + theTol); + } //! Constructor. //! @param theTolerance tolerance to be used for identification of shot circles. diff --git a/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_VertexInspector.hxx b/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_VertexInspector.hxx index 84053dd628..e5f7f5a720 100644 --- a/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_VertexInspector.hxx +++ b/src/ModelingAlgorithms/TKMesh/BRepMesh/BRepMesh_VertexInspector.hxx @@ -23,10 +23,20 @@ #include //! Class intended for fast searching of the coincidence points. -class BRepMesh_VertexInspector : public NCollection_CellFilter_InspectorXY +class BRepMesh_VertexInspector { public: - typedef int Target; + static constexpr int Dimension = 2; + + typedef gp_XY Point; + typedef int Target; + + static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } + + static Point Shift(const Point& thePnt, double theTol) + { + return Point(thePnt.X() + theTol, thePnt.Y() + theTol); + } //! Constructor. //! @param theAllocator memory allocator to be used by internal collections. diff --git a/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_FastSewing.hxx b/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_FastSewing.hxx index f6a859d8db..a66476f9e6 100644 --- a/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_FastSewing.hxx +++ b/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_FastSewing.hxx @@ -249,10 +249,20 @@ protected: //! This inspector will find a node nearest to the given point //! not far than on the given tolerance - class NodeInspector : public NCollection_CellFilter_InspectorXYZ + class NodeInspector { public: - typedef int Target; + static constexpr int Dimension = 3; + + typedef gp_XYZ Point; + typedef int Target; + + static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } + + static Point Shift(const Point& thePnt, double theTol) + { + return Point(thePnt.X() + theTol, thePnt.Y() + theTol, thePnt.Z() + theTol); + } NodeInspector(const NCollection_Vector& theVec, const gp_Pnt& thePnt, diff --git a/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_VertexInspector.hxx b/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_VertexInspector.hxx index 0010f75453..9db761e36b 100644 --- a/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_VertexInspector.hxx +++ b/src/ModelingAlgorithms/TKTopAlgo/BRepBuilderAPI/BRepBuilderAPI_VertexInspector.hxx @@ -24,18 +24,23 @@ typedef NCollection_Vector VectorOfPoint; -//======================================================================= -//! Class BRepBuilderAPI_VertexInspector -//! derived from NCollection_CellFilter_InspectorXYZ -//! This class define the Inspector interface for CellFilter algorithm, -//! working with gp_XYZ points in 3d space. -//! Used in search of coincidence points with a certain tolerance. -//======================================================================= +//! Inspector for CellFilter algorithm working with gp_XYZ points in 3d space. +//! Used in search of coincidence points with a certain tolerance. -class BRepBuilderAPI_VertexInspector : public NCollection_CellFilter_InspectorXYZ +class BRepBuilderAPI_VertexInspector { public: - typedef int Target; + static constexpr int Dimension = 3; + + typedef gp_XYZ Point; + typedef int Target; + + static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } + + static Point Shift(const Point& thePnt, double theTol) + { + return Point(thePnt.X() + theTol, thePnt.Y() + theTol, thePnt.Z() + theTol); + } //! Constructor; remembers the tolerance BRepBuilderAPI_VertexInspector(const double theTol) diff --git a/src/ModelingAlgorithms/TKTopAlgo/BRepExtrema/BRepExtrema_ProximityValueTool.hxx b/src/ModelingAlgorithms/TKTopAlgo/BRepExtrema/BRepExtrema_ProximityValueTool.hxx index 7b9f22fc8c..629bafe550 100644 --- a/src/ModelingAlgorithms/TKTopAlgo/BRepExtrema/BRepExtrema_ProximityValueTool.hxx +++ b/src/ModelingAlgorithms/TKTopAlgo/BRepExtrema/BRepExtrema_ProximityValueTool.hxx @@ -23,15 +23,22 @@ typedef NCollection_Vector VectorOfPoint; -//! Class BRepExtrema_VertexInspector -//! derived from NCollection_CellFilter_InspectorXYZ -//! This class define the Inspector interface for CellFilter algorithm, -//! working with gp_XYZ points in 3d space. -//! Used in search of coincidence points with a certain tolerance. -class BRepExtrema_VertexInspector : public NCollection_CellFilter_InspectorXYZ +//! Inspector for CellFilter algorithm working with gp_XYZ points in 3d space. +//! Used in search of coincidence points with a certain tolerance. +class BRepExtrema_VertexInspector { public: - typedef int Target; + static constexpr int Dimension = 3; + + typedef gp_XYZ Point; + typedef int Target; + + static double Coord(int i, const Point& thePnt) { return thePnt.Coord(i + 1); } + + static Point Shift(const Point& thePnt, double theTol) + { + return Point(thePnt.X() + theTol, thePnt.Y() + theTol, thePnt.Z() + theTol); + } //! Constructor; remembers the tolerance BRepExtrema_VertexInspector() @@ -53,7 +60,7 @@ public: myIsNeedAdd = true; } - bool IsNeedAdd() { return myIsNeedAdd; } + bool IsNeedAdd() const { return myIsNeedAdd; } //! Implementation of inspection method Standard_EXPORT NCollection_CellFilter_Action Inspect(const int theTarget); diff --git a/src/ModelingData/TKGeomBase/Extrema/Extrema_GGenExtCC.hxx b/src/ModelingData/TKGeomBase/Extrema/Extrema_GGenExtCC.hxx index 2c6721b206..c232e7f862 100644 --- a/src/ModelingData/TKGeomBase/Extrema/Extrema_GGenExtCC.hxx +++ b/src/ModelingData/TKGeomBase/Extrema/Extrema_GGenExtCC.hxx @@ -201,20 +201,30 @@ inline void Extrema_GGenExtCC_ChangeIntervals(occ::handle