Modeling Data, Algorithms - Devirtualize adaptor dispatch, mark leaf Geom classes final#1063
Conversation
…Geom classes final Eliminate virtual dispatch for elementary geometry evaluation in GeomAdaptor_Curve, GeomAdaptor_Surface, and Geom2dAdaptor_Curve by storing gp_* primitives directly in std::variant and calling ElCLib/ElSLib static methods instead of going through virtual myCurve->D0()/mySurface->D0() calls. Extend CurveDataVariant/SurfaceDataVariant with elementary types: - GeomAdaptor_Curve: gp_Lin, gp_Circ, gp_Elips, gp_Hypr, gp_Parab - GeomAdaptor_Surface: gp_Pln, gp_Cylinder, gp_Cone, gp_Sphere, gp_Torus - Geom2dAdaptor_Curve: gp_Lin2d, gp_Circ2d, gp_Elips2d, gp_Hypr2d, gp_Parab2d The load() method now extracts and stores the gp_* primitive at construction time. D0-DN methods dispatch via switch on the curve/surface type enum, calling ElCLib/ElSLib directly for elementary types. Accessor methods (Line(), Circle(), Plane(), etc.) return from the variant when available, avoiding repeated downcasts. Mark every override method as final on all 29 concrete (leaf) classes in Geom_* and Geom2d_* hierarchies. These classes have no subclasses, so final enables compiler devirtualization and clearly documents the design intent. Affected methods include D0-DN, Reverse, Transform, Copy, DumpJson, and all other overridden virtuals. Fix ShallowCopy in GeomAdaptor_Curve and GeomAdaptor_Surface where elementary gp_* types stored in the variant were not copied to the new object, which would cause std::bad_variant_access on first evaluation of the copy.
There was a problem hiding this comment.
Pull request overview
This PR optimizes elementary curve/surface evaluation by avoiding virtual dispatch in adaptors and enabling better compiler devirtualization via final on leaf Geom_* / Geom2d_* overrides.
Changes:
- Store elementary
gp_*primitives directly in adaptorstd::variantpayloads and dispatch evaluation viaElCLib/ElSLib. - Fix
ShallowCopy()to correctly copy elementarygp_*variant values. - Mark overridden virtual methods as
finalin many leaf geometry classes to enable devirtualization.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ModelingData/TKG3d/GeomAdaptor/GeomAdaptor_Surface.hxx | Extends surface evaluation variant with elementary gp_* types. |
| src/ModelingData/TKG3d/GeomAdaptor/GeomAdaptor_Surface.cxx | Loads/stores elementary primitives; dispatches D0–DN via ElSLib; fixes ShallowCopy() for variants; updates accessors to return from variant. |
| src/ModelingData/TKG3d/GeomAdaptor/GeomAdaptor_Curve.hxx | Extends curve evaluation variant with elementary gp_* types and documents intent. |
| src/ModelingData/TKG3d/GeomAdaptor/GeomAdaptor_Curve.cxx | Loads/stores elementary primitives; dispatches D0–DN via ElCLib; fixes ShallowCopy() for variants; updates accessors/resolution to use variant. |
| src/ModelingData/TKG2d/Geom2dAdaptor/Geom2dAdaptor_Curve.hxx | Extends 2D curve variant with elementary gp_*2d types; marks evaluation APIs final. |
| src/ModelingData/TKG2d/Geom2dAdaptor/Geom2dAdaptor_Curve.cxx | Loads/stores elementary primitives; dispatches D0–DN via ElCLib; fixes ShallowCopy() for variants; updates accessors/resolution to use variant. |
| src/ModelingData/TKG3d/Geom/*.hxx | Marks many overridden methods final on leaf Geom_* classes. |
| src/ModelingData/TKG2d/Geom2d/*.hxx | Marks many overridden methods final on leaf Geom2d_* classes. |
| Standard_EXPORT gp_Pnt2d Value(const double U) const final; | ||
|
|
||
| //! Computes the point of parameter U. | ||
| Standard_EXPORT void D0(const double U, gp_Pnt2d& P) const override; | ||
| Standard_EXPORT void D0(const double U, gp_Pnt2d& P) const final; |
There was a problem hiding this comment.
final can only be used on virtual functions. These declarations currently show final but (in this hunk) do not also show override; if the corresponding base methods are virtual but the derived declarations are not marked override elsewhere, this can either fail to compile or silently stop overriding (depending on the exact base signature). Prefer override final (or final override) consistently for these overridden adaptor methods.
| //! Raised if the continuity of the current interval | ||
| //! is not C1. | ||
| Standard_EXPORT void D1(const double U, gp_Pnt2d& P, gp_Vec2d& V) const override; | ||
| Standard_EXPORT void D1(const double U, gp_Pnt2d& P, gp_Vec2d& V) const final; |
There was a problem hiding this comment.
final can only be used on virtual functions. These declarations currently show final but (in this hunk) do not also show override; if the corresponding base methods are virtual but the derived declarations are not marked override elsewhere, this can either fail to compile or silently stop overriding (depending on the exact base signature). Prefer override final (or final override) consistently for these overridden adaptor methods.
| //! Raised if the continuity of the current interval | ||
| //! is not C2. | ||
| Standard_EXPORT void D2(const double U, gp_Pnt2d& P, gp_Vec2d& V1, gp_Vec2d& V2) const override; | ||
| Standard_EXPORT void D2(const double U, gp_Pnt2d& P, gp_Vec2d& V1, gp_Vec2d& V2) const final; |
There was a problem hiding this comment.
final can only be used on virtual functions. These declarations currently show final but (in this hunk) do not also show override; if the corresponding base methods are virtual but the derived declarations are not marked override elsewhere, this can either fail to compile or silently stop overriding (depending on the exact base signature). Prefer override final (or final override) consistently for these overridden adaptor methods.
| gp_Vec2d& V1, | ||
| gp_Vec2d& V2, | ||
| gp_Vec2d& V3) const override; | ||
| gp_Vec2d& V3) const final; |
There was a problem hiding this comment.
final can only be used on virtual functions. These declarations currently show final but (in this hunk) do not also show override; if the corresponding base methods are virtual but the derived declarations are not marked override elsewhere, this can either fail to compile or silently stop overriding (depending on the exact base signature). Prefer override final (or final override) consistently for these overridden adaptor methods.
| //! is not CN. | ||
| //! Raised if N < 1. | ||
| Standard_EXPORT gp_Vec2d DN(const double U, const int N) const override; | ||
| Standard_EXPORT gp_Vec2d DN(const double U, const int N) const final; |
There was a problem hiding this comment.
final can only be used on virtual functions. These declarations currently show final but (in this hunk) do not also show override; if the corresponding base methods are virtual but the derived declarations are not marked override elsewhere, this can either fail to compile or silently stop overriding (depending on the exact base signature). Prefer override final (or final override) consistently for these overridden adaptor methods.
Eliminate virtual dispatch for elementary geometry evaluation in GeomAdaptor_Curve, GeomAdaptor_Surface, and Geom2dAdaptor_Curve by storing gp_* primitives directly in std::variant and calling ElCLib/ElSLib static methods instead of going through virtual myCurve->D0()/mySurface->D0() calls.
Extend CurveDataVariant/SurfaceDataVariant with elementary types:
The load() method now extracts and stores the gp_* primitive at construction time. D0-DN methods dispatch via switch on the curve/surface type enum, calling ElCLib/ElSLib directly for elementary types. Accessor methods (Line(), Circle(), Plane(), etc.) return from the variant when available, avoiding repeated downcasts.
Mark every override method as final on all 29 concrete (leaf) classes in Geom_* and Geom2d_* hierarchies. These classes have no subclasses, so final enables compiler devirtualization and clearly documents the design intent. Affected methods include D0-DN, Reverse, Transform, Copy, DumpJson, and all other overridden virtuals.
Fix ShallowCopy in GeomAdaptor_Curve and GeomAdaptor_Surface where elementary gp_* types stored in the variant were not copied to the new object, which would cause std::bad_variant_access on first evaluation of the copy.