Modeling Data - Optimize BRep hot paths with enum discriminants and caching by dpasukhi · Pull Request #1061 · Open-Cascade-SAS/OCCT

dpasukhi · 2026-02-10T23:25:44Z

Replace virtual dispatch with enum-based type discrimination and add
targeted caching to eliminate overhead in the most frequently called
BRep functions.

Enum discriminants:

Add BRep_CurveRepKind (9 variants) and BRep_PointRepKind (3 variants)
as uint8_t enums set once at construction time.
Replace all virtual Is*() queries (IsCurve3D, IsCurveOnSurface,
IsRegularity, IsPolygon3D, etc.) with inline enum comparisons,
eliminating vtable lookups in tight iteration loops.
Closed variant constructors overwrite the kind in the body after
base class initialization.

Tag pre-filters:

Gate expensive virtual parameter-matching calls (e.g.
IsCurveOnSurface(S, L)) behind cheap enum checks in BRep_Builder
and BRep_Tool search loops.
Fix dead code in BRep_Builder::Transfert() by checking
IsCurveOnClosedSurface() before IsCurveOnSurface(), making the
closed surface branch reachable.
Change BRep_Tool::Continuity()/MaxContinuity() from ChangeCurves()
to Curves() to avoid needless cache invalidation on read-only access.

Cached Curve3D/Polygon3D on BRep_TEdge:

Add myCurve3D and myPolygon3D handle members for O(1) lookup in
BRep_Tool::Curve(), Polygon3D(), IsGeometric(), and Range().
ChangeCurves() nullifies both caches; BRep_Builder re-populates
after modification; fallback list scan when cache is null.
EmptyCopy() preserves the Curve3D cache for cloned edges.

Cached IsPlane flag on BRep_TFace:

Compute and cache myIsPlane when surface is set, avoiding repeated
RTTI downcasts in BRep_Tool::IsClosed(E, F).
EmptyCopy() copies the flag directly.

Tolerance clamping at write time:

Move std::max(T, Precision::Confusion()) from BRep_Tool::Tolerance()
read path to Tolerance() setters on BRep_TEdge, BRep_TVertex, and
BRep_TFace, removing a branch from the hot read path.
Default-construct tolerance to Precision::Confusion() instead of
RealEpsilon().

TopLoc_Location identity shortcuts:

Predivided()/Divided() return early when the other location is
Identity, skipping SList traversal and gp_Trsf composition.
BRep_Tool::Surface/Curve/Polygon3D skip location multiplication
when sub-location is Identity.

Minor hot-path optimizations:

TopLoc_SListOfItemLocation::Construct() uses move assignment to
avoid atomic refcount increment.
TopoDS_Iterator skips TopAbs::Compose() for FORWARD-oriented
parent shapes (the common case).
TopoDS_Shape::Location()/Move() defer Transformation() extraction
to the rare validation path.

Cache frequently accessed data in BRep_TEdge, BRep_TFace, and BRep_TVertex to eliminate repeated linear scans and redundant computations in BRep_Tool: - BRep_TEdge: Add cached myCurve3D and myPolygon3D handles for O(1) access to the most commonly queried representations, avoiding O(n) list traversal. - BRep_TFace: Add cached myIsPlane flag computed once when surface is set, eliminating repeated RTTI down_cast checks in IsClosed() and CurveOnPlane(). - BRep_TEdge/TFace/TVertex: Pre-clamp tolerance to Precision::Confusion() at set-time, removing the branch from every Tolerance() read (1,126 calls). - BRep_Builder: Maintain new caches when updating curves and polygons. - BRep_Tool: Use cached values in Curve(), Polygon3D(), IsGeometric(), Range(), IsClosed(), and all three Tolerance() overloads. https://claude.ai/code/session_016dWEFeDy4ZJRrcw2GgXDtm

…Rep curve representations Replace 13 virtual Is*() type-check methods in BRep_CurveRepresentation hierarchy with O(1) non-virtual inline enum comparisons. This eliminates virtual dispatch overhead (~10-15 cycles/call) across all BRep_Tool hot paths that iterate curve representation lists. Key changes: - Add BRep_CurveRepKind enum (uint8_t) with 9 concrete type tags - Make all simple Is*() methods non-virtual inline using enum comparison - Keep parameterized Is*(S,L) variants virtual (they access subclass data) - Forward kind tag through constructor chain to all 9 concrete subclasses - Add identity fast paths to TopLoc_Location::Predivided and Divided - Fix unsafe C-style casts in BRep_Tool::Continuity and MaxContinuity https://claude.ai/code/session_016dWEFeDy4ZJRrcw2GgXDtm

…Rep point representations Apply the same enum discriminant optimization to BRep_PointRepresentation hierarchy as was done for BRep_CurveRepresentation. Replaces 3 virtual Is*() type-check methods with O(1) non-virtual inline enum comparisons. - Add BRep_PointRepKind enum (uint8_t) with 3 tags: PointOnCurve, PointOnSurface, PointOnCurveOnSurface - Make IsPointOnCurve(), IsPointOnSurface(), IsPointOnCurveOnSurface() non-virtual inline using enum comparison - Keep parameterized Is*(args...) variants virtual - Forward kind through BRep_PointsOnSurface intermediate class https://claude.ai/code/session_016dWEFeDy4ZJRrcw2GgXDtm

…ep hot paths Replace IsKind() RTTI hierarchy walks with O(1) enum tag checks in BRep_TEdge::EmptyCopy, which is called during every shape copy operation. Replace down_cast<BRep_GCurve> with tag-based pre-filtering and static_cast in BRep_Builder loop patterns (UpdateCurves, Range, UpdateVertex), skipping polygon representations that can never be GCurve subclasses. Copy cached myIsPlane flag directly in BRep_TFace::EmptyCopy instead of recomputing via Surface() setter which performs 3 RTTI downcasts. Add early exit and const reference in BRep_Tool::SetUVPoints to avoid full list traversal and unnecessary handle reference counting. https://claude.ai/code/session_016dWEFeDy4ZJRrcw2GgXDtm

Add inline tag pre-filters before virtual Is*(S,L) calls across BRep_Tool.cxx (10 sites) and BRep_Builder.cxx (3 sites), so that elements of non-matching type skip the expensive virtual parameter comparison entirely. Also pre-filter IsPoint*(C,L) calls (6 sites in BRep_Tool, 3 in BRep_Builder). Reorder IsClosed checks to put the cheaper tag-based IsCurveOnClosedSurface/IsPolygonOnClosedTriangulation check before the virtual IsCurveOnSurface(S,L)/IsPolygonOnTriangulation(T,L) call. Guard Transformation() call in TopoDS_Shape::Location() and Move() setters behind the theRaiseExc flag, avoiding an unconditional function call on every shape location assignment (theRaiseExc is always false in practice). Skip TopAbs::Compose for FORWARD-oriented parent in TopoDS_Iterator::updateCurrentShape(), the identity case that needs no orientation composition. Use direct field assignment in BRep_TVertex::EmptyCopy() for consistency with BRep_TFace::EmptyCopy(). Use move assignment in TopLoc_SListOfItemLocation::Construct() to avoid the atomic refcount increment inherent in the copy-based Assign() path. https://claude.ai/code/session_016dWEFeDy4ZJRrcw2GgXDtm

…er and optimize caching mechanisms

… parameter directly and optimize caching logic

…n BRep representations

Copilot

Pull request overview

This PR optimizes critical BRep hot paths by replacing virtual dispatch with enum-based type discrimination and implementing targeted caching strategies. The changes aim to eliminate performance overhead in frequently called functions.

Changes:

Introduced BRep_CurveRepKind and BRep_PointRepKind uint8_t enums replacing virtual Is*() queries with inline enum comparisons
Added caching of Curve3D and Polygon3D on BRep_TEdge and IsPlane flag on BRep_TFace for O(1) lookups
Moved tolerance clamping from read paths to write paths in setters for BRep_TEdge, BRep_TVertex, and BRep_TFace
Added identity shortcuts in TopLoc_Location::Predivided()/Divided() and optimized location multiplication in BRep_Tool methods

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
TopoDS_Shape.hxx	Defers transformation extraction to validation path only
TopoDS_Iterator.cxx	Skips orientation composition for FORWARD-oriented shapes
BRep_Tool.cxx	Adds enum pre-filters, caching lookups, removes tolerance clamping, optimizes location checks
BRep_TVertex.lxx/cxx	Moves tolerance clamping to setters, updates default tolerance
BRep_TFace.hxx/cxx	Adds cached `IsPlane` flag and moves tolerance clamping to setter
BRep_TEdge.lxx/hxx/cxx	Adds `Curve3D`/`Polygon3D` cache members and invalidation in `ChangeCurves()`
BRep_PolygonOnTriangulation.hxx/cxx	Removes virtual `IsPolygonOnTriangulation()`, uses enum discriminant
BRep_PolygonOnSurface.hxx/cxx	Removes virtual `IsPolygonOnSurface()`, uses enum discriminant
BRep_PolygonOnClosedTriangulation.hxx/cxx	Removes virtual override, sets enum in constructor body
BRep_PolygonOnClosedSurface.hxx/cxx	Removes virtual override, sets enum in constructor body
BRep_Polygon3D.hxx/cxx	Removes virtual `IsPolygon3D()`, uses enum discriminant
BRep_PointsOnSurface.hxx/cxx	Adds `BRep_PointRepKind` parameter to constructor
BRep_PointRepresentation.hxx/cxx	Introduces `BRep_PointRepKind` enum and inline `Is*()` methods
BRep_PointOnSurface.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_PointOnCurveOnSurface.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_PointOnCurve.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_GCurve.hxx/cxx	Adds `BRep_CurveRepKind` parameter to constructor
BRep_CurveRepresentation.hxx/cxx	Introduces `BRep_CurveRepKind` enum and inline `Is*()` methods
BRep_CurveOnSurface.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_CurveOnClosedSurface.hxx/cxx	Removes virtual overrides, sets enum in constructor body
BRep_CurveOn2Surfaces.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_Curve3D.hxx/cxx	Removes virtual override, passes enum to base constructor
BRep_Builder.cxx	Updates calls with enum checks, cache population, fixes dead code in `Transfert()`
TopLoc_SListOfItemLocation.hxx	Uses move assignment instead of `Assign()`
TopLoc_Location.cxx	Adds identity shortcuts in `Divided()`/`Predivided()`

Copilot · 2026-02-10T23:26:42Z

src/ModelingData/TKBRep/BRep/BRep_TVertex.cxx

+  TV->myPnt       = myPnt;
+  TV->myTolerance = myTolerance;


Direct member assignment in EmptyCopy() bypasses the setter which performs tolerance clamping. This creates inconsistency as the setter ensures tolerance is at least Precision::Confusion(), but direct assignment in EmptyCopy() does not. If myTolerance is already clamped (guaranteed by the constructor and setter), document this assumption, or use the setter for consistency.

Suggested change

TV->myPnt = myPnt;

TV->myTolerance = myTolerance;

TV->myPnt = myPnt;

TV->SetTolerance(myTolerance);

Copilot · 2026-02-10T23:26:42Z

src/ModelingData/TKBRep/BRep/BRep_TFace.cxx

+  TF->mySurface = mySurface;
+  TF->myIsPlane = myIsPlane;


Direct assignment of mySurface bypasses the Surface() setter which computes myIsPlane. While myIsPlane is copied directly on line 87, if mySurface changes in the future without updating myIsPlane, this creates a maintenance risk. Consider documenting that myIsPlane must always be copied when mySurface is assigned directly.

Suggested change

TF->mySurface = mySurface;

TF->myIsPlane = myIsPlane;

TF->Surface(mySurface);

Copilot · 2026-02-10T23:26:42Z

src/ModelingData/TKBRep/BRep/BRep_Builder.cxx

@@ -1139,12 +1149,7 @@ void BRep_Builder::Transfert(const TopoDS_Edge& Ein, const TopoDS_Edge& Eout) co

    const occ::handle<BRep_CurveRepresentation>& CR = itcr.Value();



The reordering of the condition check from IsCurveOnSurface() before IsCurveOnClosedSurface() to the reverse fixes dead code, but the PR description states this makes the closed surface branch reachable. Add a comment explaining why the order matters (i.e., IsCurveOnClosedSurface() returns true for IsCurveOnSurface(), so checking closed first is necessary).

Suggested change

// Note: IsCurveOnClosedSurface() implies IsCurveOnSurface(), so the closed-surface

// branch must be checked first to keep it reachable and avoid dead code.

…uilder::Transfert

…ake file to include new tests

… signatures and logic clarity; include TopoDS_Shape in BRep_TFace_Test

claude and others added 8 commits February 10, 2026 21:49

Modeling Data - Refactor BRep representations to include kind paramet…

99baf48

…er and optimize caching mechanisms

Modeling Data - Refactor BRep representation constructors to set kind…

6d8091c

… parameter directly and optimize caching logic

Modeling Data - Remove redundant comments and optimize code clarity i…

7fac3a8

…n BRep representations

dpasukhi requested a review from Copilot February 10, 2026 23:25

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

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

Copilot AI reviewed Feb 10, 2026

View reviewed changes

dpasukhi added 3 commits February 10, 2026 23:34

Modeling Data - Add comment to clarify closed surface check in BRep_B…

2b3c794

…uilder::Transfert

Modeling Data - Add unit tests for BRep representations and update CM…

d3b560c

…ake file to include new tests

Modeling Data - Update BRep_TFace and BRep_Tool for improved function…

8159cfd

… signatures and logic clarity; include TopoDS_Shape in BRep_TFace_Test

dpasukhi marked this pull request as draft February 11, 2026 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Modeling Data - Optimize BRep hot paths with enum discriminants and caching#1061

Modeling Data - Optimize BRep hot paths with enum discriminants and caching#1061
dpasukhi wants to merge 11 commits intoOpen-Cascade-SAS:IRfrom
dpasukhi:optimize-brep-caching

dpasukhi commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 10, 2026

Uh oh!

Copilot AI Feb 10, 2026

Uh oh!

Copilot AI Feb 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	TF->mySurface = mySurface;
	TF->myIsPlane = myIsPlane;
	TF->Surface(mySurface);

		@@ -1139,12 +1149,7 @@ void BRep_Builder::Transfert(const TopoDS_Edge& Ein, const TopoDS_Edge& Eout) co

		const occ::handle<BRep_CurveRepresentation>& CR = itcr.Value();



	// Note: IsCurveOnClosedSurface() implies IsCurveOnSurface(), so the closed-surface
	// branch must be checked first to keep it reachable and avoid dead code.

Comments

Conversation

dpasukhi commented Feb 10, 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 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants