Coding - Optimize container usage and add regression GTests by dpasukhi · Pull Request #1102 · Open-Cascade-SAS/OCCT

dpasukhi · 2026-02-20T21:02:20Z

Refactor several hot paths in TKBO/TKOffset/TKOpenGl to reduce redundant lookups/copies and align containers with OCCT collection types.

Changes:

TKBO:
- Switch BOPTools_Set storage from NCollection_List to NCollection_Vector.
- Replace unsafe C-style edge cast with TopoDS::Edge().
- Apply small lookup/copy optimizations in BOPAlgo_Builder and BOPAlgo_PaveFiller (Seek() usage, const refs, reduced temporaries).
TKOffset:
- Reduce repeated vertex/tolerance extraction in BRepOffset_Inter2d.
- Cache MinLoc transformation in ExtentEdge().
TKMesh:
- Simplify IMeshData_Face surface adaptor initialization.
- Add BRepMesh_FaceChecker_Test and register it in GTests/FILES.cmake.
TKOpenGl:
- Replace std::map/std::set with NCollection_DataMap/NCollection_Map in ray-tracing scene update state.
- Simplify texture transform and shader prefix string construction.
- Add OpenGl_View_Raytrace_TextureTransform_Test and register it in GTests/FILES.cmake.

Behavior is preserved; changes target robustness and update-path performance.

Refactor several hot paths in TKBO/TKOffset/TKOpenGl to reduce redundant lookups/copies and align containers with OCCT collection types. Changes: - TKBO: - Switch BOPTools_Set storage from NCollection_List to NCollection_Vector. - Replace unsafe C-style edge cast with TopoDS::Edge(). - Apply small lookup/copy optimizations in BOPAlgo_Builder and BOPAlgo_PaveFiller (Seek() usage, const refs, reduced temporaries). - TKOffset: - Reduce repeated vertex/tolerance extraction in BRepOffset_Inter2d. - Cache MinLoc transformation in ExtentEdge(). - TKMesh: - Simplify IMeshData_Face surface adaptor initialization. - Add BRepMesh_FaceChecker_Test and register it in GTests/FILES.cmake. - TKOpenGl: - Replace std::map/std::set with NCollection_DataMap/NCollection_Map in ray-tracing scene update state. - Simplify texture transform and shader prefix string construction. - Add OpenGl_View_Raytrace_TextureTransform_Test and register it in GTests/FILES.cmake. Behavior is preserved; changes target robustness and update-path performance.

Copilot

Pull request overview

This PR refactors several performance-sensitive paths across TKBO/TKOffset/TKMesh/TKOpenGl by reducing redundant lookups/copies and switching scene-update containers to OCCT collections, and adds GTest coverage intended to prevent regressions.

Changes:

TKOpenGl: replace std::map/std::set usage in ray-tracing update state with NCollection_DataMap/NCollection_Map, plus small string/transform construction simplifications.
TKBO/TKOffset/TKMesh: micro-optimizations (cached lookups/transformations, fewer temporaries) and robustness tweaks (safe TopoDS::Edge() cast, simplified adaptor init).
Add and register new regression GTests for ray-trace texture transform and face checker behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Visualization/TKOpenGl/OpenGl/OpenGl_View_Raytrace.cxx	Switches ray-tracing update tracking containers to OCCT maps; optimizes update logic and texture transform/shader prefix building.
src/Visualization/TKOpenGl/OpenGl/OpenGl_View.hxx	Updates ray-tracing state fields to `NCollection_DataMap` / `NCollection_Map`.
src/Visualization/TKOpenGl/GTests/OpenGl_View_Raytrace_TextureTransform_Test.cxx	Adds GTests for texture transform matrix composition/sign convention.
src/Visualization/TKOpenGl/GTests/FILES.cmake	Registers the new TKOpenGl GTest source.
src/ModelingAlgorithms/TKOffset/BRepOffset/BRepOffset_Inter2d.cxx	Reduces repeated vertex/tolerance extraction and caches `MinLoc` transformation.
src/ModelingAlgorithms/TKMesh/IMeshData/IMeshData_Face.hxx	Simplifies surface adaptor initialization to avoid an extra temporary.
src/ModelingAlgorithms/TKMesh/GTests/FILES.cmake	Registers the new TKMesh GTest source.
src/ModelingAlgorithms/TKMesh/GTests/BRepMesh_FaceChecker_Test.cxx	Adds GTests intended to cover segment intersection acceptance heuristics.
src/ModelingAlgorithms/TKBO/BOPTools/BOPTools_Set.hxx	Switches internal storage from `NCollection_List` to `NCollection_Vector`.
src/ModelingAlgorithms/TKBO/BOPTools/BOPTools_Set.cxx	Updates implementation for vector storage and replaces unsafe edge cast with `TopoDS::Edge()`.
src/ModelingAlgorithms/TKBO/BOPAlgo/BOPAlgo_PaveFiller_6.cxx	Replaces `Find()`/`IsBound()` pattern with `Seek()` to avoid redundant map lookups.
src/ModelingAlgorithms/TKBO/BOPAlgo/BOPAlgo_PaveFiller.hxx	Adds an rvalue overload for `SetArguments()` to allow move semantics.
src/ModelingAlgorithms/TKBO/BOPAlgo/BOPAlgo_Builder.cxx	Reduces unnecessary copies and improves face handling when reversing/orienting.

Copilot · 2026-02-20T21:08:49Z

+constexpr double THE_MAX_TANGENT_ANGLE = 5. * M_PI / 180.;
+
+bool acceptSegmentIntersection(const gp_Pnt2d&                     theRefPnt1,
+                               const gp_Pnt2d&                     theRefPnt2,
+                               const gp_Pnt2d&                     theCandidatePnt1,
+                               const gp_Pnt2d&                     theCandidatePnt2,
+                               const NCollection_Vector<gp_Pnt2d>& theLoopPoint2,
+                               const double                        theTolerance,
+                               const bool                          theUseSelfLoopCheck)
+{
+  gp_Pnt2d                         anIntPnt;
+  const BRepMesh_GeomTool::IntFlag anIntStatus = BRepMesh_GeomTool::IntSegSeg(theRefPnt1.XY(),
+                                                                              theRefPnt2.XY(),
+                                                                              theCandidatePnt1.XY(),
+                                                                              theCandidatePnt2.XY(),
+                                                                              false,
+                                                                              false,
+                                                                              anIntPnt);
+  if (anIntStatus != BRepMesh_GeomTool::Cross)
+  {
+    return false;
+  }
+
+  const gp_Vec2d aRefDir(theRefPnt1.XY(), theRefPnt2.XY());
+  const double   anAngle = aRefDir.Angle(gp_Vec2d(theCandidatePnt1.XY(), theCandidatePnt2.XY()));
+  if (std::abs(anAngle) < THE_MAX_TANGENT_ANGLE)
+  {
+    return false;
+  }
+
+  if (!theUseSelfLoopCheck)
+  {
+    return true;
+  }
+
+  const double aMaxLoopSize = M_PI * theTolerance * theTolerance;
+  gp_XY        aPrevVec;
+  double       aSumS   = 0.;
+  const gp_XY& aRefPnt = anIntPnt.Coord();
+  for (int i = 0; i < theLoopPoint2.Size(); ++i)
+  {
+    const gp_XY aCurVec = theLoopPoint2(i).XY() - aRefPnt;
+    if (aCurVec.SquareModulus() < gp::Resolution())
+    {
+      continue;
+    }
+    if (aPrevVec.SquareModulus() > gp::Resolution())
+    {
+      aSumS += aPrevVec ^ aCurVec;
+    }
+    aPrevVec = aCurVec;
+  }
+
+  return std::abs(aSumS / 2.) >= aMaxLoopSize;
+}
+} // namespace


This test currently checks a locally duplicated acceptSegmentIntersection() implementation rather than invoking BRepMesh_FaceChecker (or the selector logic) from production code. As a result, it may not catch regressions in the actual BRepMesh_FaceChecker implementation (e.g., if tolerances/loop logic are adjusted).

Consider constructing a minimal face/wire setup and running BRepMesh_FaceChecker on it (asserting the expected acceptance/rejection), or extracting the intersection-acceptance predicate into a shared internal helper that both production and test can call.

Copilot · 2026-02-20T21:08:49Z

+BVH_Mat4f buildTextureTransformForTest(const occ::handle<Graphic3d_TextureParams>& theParams)
+{
+  BVH_Mat4f aMatrix;
+  aMatrix.InitIdentity();
+  if (theParams.IsNull())
+  {
+    return aMatrix;
+  }
+
+  const float aScaleX       = theParams->Scale().x();
+  const float aScaleY       = theParams->Scale().y();
+  aMatrix.ChangeValue(0, 0) = aScaleX;
+  aMatrix.ChangeValue(1, 1) = aScaleY;
+
+  const NCollection_Vec2<float> aTrans = -theParams->Translation();
+  aMatrix.ChangeValue(0, 3)            = aScaleX * aTrans.x();
+  aMatrix.ChangeValue(1, 3)            = aScaleY * aTrans.y();
+
+  const float aAngle = -theParams->Rotation() * static_cast<float>(M_PI / 180.0);
+  const float aSin   = std::sin(aAngle);
+  const float aCos   = std::cos(aAngle);
+
+  BVH_Mat4f aRotationMat;
+  aRotationMat.SetValue(0, 0, aCos);
+  aRotationMat.SetValue(1, 1, aCos);
+  aRotationMat.SetValue(0, 1, -aSin);
+  aRotationMat.SetValue(1, 0, aSin);
+
+  return aMatrix * aRotationMat;
+}


This test validates a locally re-implemented buildTextureTransformForTest() instead of exercising the production buildTextureTransform() logic in OpenGl_View_Raytrace.cxx. This can yield false confidence if the production implementation changes but the copied test helper is not updated in sync.

Consider refactoring the production transform builder into a shared helper (e.g., a small utility function in a header/namespace) so the test calls the exact same code path, or test the transformation indirectly through a public API that uses buildTextureTransform() (e.g., material/texture conversion).

dpasukhi requested a review from Copilot February 20, 2026 21:02

dpasukhi self-assigned this Feb 20, 2026

dpasukhi added the 1. Coding Coding rules, trivial changes and misprints label Feb 20, 2026

github-project-automation bot added this to Maintenance Feb 20, 2026

github-project-automation bot moved this to Todo in Maintenance Feb 20, 2026

Copilot started reviewing on behalf of dpasukhi February 20, 2026 21:02 View session

// formatting

e9cc2be

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Cleanup - Remove obsolete test files for BRepMesh and OpenGl

41eb23c

dpasukhi merged commit 82303a9 into Open-Cascade-SAS:IR Feb 20, 2026
18 checks passed

dpasukhi deleted the perf_check branch February 20, 2026 22:36

github-project-automation bot moved this from Todo to Done in Maintenance Feb 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coding - Optimize container usage and add regression GTests#1102

Coding - Optimize container usage and add regression GTests#1102
dpasukhi merged 3 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:perf_check

dpasukhi commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dpasukhi commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants