Improve ShapeTesselator

[tpaviot]

Summary by Sourcery

Optimize and extend ShapeTesselator by adding optional parallel face processing, leveraging precomputed normals when available, and reducing memory allocations and string formatting overhead across tessellation and export routines.

Enhancements:

• Introduce a parallel-processing path for face tessellation using OpenMP when enabled and requested, while preserving a sequential fallback.
• Reuse collected face lists instead of re-exploring the shape during tessellation to streamline face processing.
• Prefer precomputed normals from Poly_Triangulation with a UV-based fallback, improving normal computation performance and robustness.
• Preallocate exact container sizes and switch from push/insert patterns to indexed writes for triangles, edges, and flattened vertex/normal tuples to reduce reallocations and improve performance.
• Simplify X3D export by writing coordinates and normals directly from consolidated buffers with more efficient float formatting into streams.

[sourcery-ai]

Reviewer's Guide

Refactors ShapeTesselator to support optional OpenMP-parallel face processing, improves normal computation by using precomputed triangulation normals when available, and optimizes several data paths by preallocating exact buffer sizes and avoiding repeated insert/push_back operations and intermediate string formatting.

Updated class diagram for ShapeTesselator parallel processing and normals optimization

classDiagram

class ShapeTesselator {
  - bool computed
  - bool use_parallel
  - TopoDS_Shape myShape
  - Standard_Real myDeviation
  - std_vector_face_ptr face_list
  - std_vector_edge edge_list
  + ShapeTesselator(aShape)
  + Tessellate(compute_edges, mesh_quality, parallel)
  + GetVerticesPositionAsTuple() const
  + GetNormalsAsTuple() const
  + ExportShapeToX3DTriangleSet() const
  - ProcessFaces(faces)
  - ProcessSingleFace(face, triangulation, location, face_data)
  - ProcessNormals(face, triangulation, face_data)
  - ProcessTriangles(face, triangulation, face_data)
}

class Face {
  + std_vector_float vertex_coords
  + std_vector_float normal_coords
  + std_vector_int triangle_indices
  + Standard_Integer number_of_triangles
  + Standard_Integer number_of_normals
  + Standard_Integer number_of_invalid_normals
}

class Edge {
  + std_vector_float vertex_coords
}

ShapeTesselator "1" *-- "*" Face
ShapeTesselator "1" *-- "*" Edge

Loading

Flow diagram for ShapeTesselator Tessellate face processing and normals computation

flowchart TD

  A[Tessellate] --> B[Collect faces from myShape]
  B --> C[Set use_parallel flag]
  C --> D{OpenMP available and use_parallel and num_faces > 1}

  D -->|Yes| E[Parallel for over faces]
  D -->|No| F[Sequential loop over faces]

  E --> G[For each face:<br/>ProcessSingleFace]
  F --> G

  G --> H[ProcessNormals]

  H --> I{triangulation HasNormals}
  I -->|Yes| J[Copy precomputed normals<br/>from triangulation]
  I -->|No| K{triangulation HasUVNodes}

  K -->|Yes| L[Compute normals<br/>via BRepGProp_Face Normal]
  K -->|No| M[Increment<br/>number_of_invalid_normals]

  J --> N[Update face_data<br/>normal_coords]
  L --> N
  M --> N

  N --> O[ProcessTriangles<br/>and fill triangle_indices]
  O --> P[Append face_data<br/>into face_list]

Loading

File-Level Changes

Change	Details	Files
Add optional OpenMP-based parallelization for face tessellation while preserving a sequential fallback path.	Introduce a use_parallel member flag initialized in the constructor and set from the Tessellate parallel argument. Collect faces into a std::vector<TopoDS_Face> in Tessellate, reserve face_list based on its size, and pass it into ProcessFaces instead of iterating TopExp_Explorer inside ProcessFaces. Rewrite ProcessFaces to accept a face vector and, when OpenMP is available and use_parallel is true, process faces in a parallel for loop that builds per-face results in a temporary vector of unique_ptr and then sequentially moves non-null results into face_list; otherwise, process faces sequentially as before.	src/Tesselator/ShapeTesselator.cpp src/Tesselator/ShapeTesselator.h
Improve normal computation by using Poly_Triangulation precomputed normals when available and falling back to UV-based normals with better validity handling.	Change ProcessSingleFace to treat normals as available when triangulation has either normals or UV nodes, and delegate to ProcessNormals; otherwise, increment number_of_invalid_normals. Update ProcessNormals to first allocate the normal buffer, then, if triangulation->HasNormals(), copy and optionally reverse those normals directly into face_data and return early. Add a guarded fallback path that checks HasUVNodes(), increments number_of_invalid_normals and returns if UV data is missing, otherwise constructs BRepGProp_Face and computes normalized or zeroed normals from UV coordinates while respecting internal orientation.	src/Tesselator/ShapeTesselator.cpp
Optimize triangle and edge data extraction by pre-sizing containers and writing into them by index instead of repeated insert/push_back.	In ProcessTriangles, resize triangle_indices to nb_triangles * 3, set number_of_triangles upfront, and fill indices by direct indexing while still honoring face orientation reversal. In both branches of ProcessSingleEdge, replace reserve + insert on vertex_coords with resize to nb_nodes * 3 and direct writes at the correct positions after applying any location transform. In GetVerticesPositionAsTuple and GetNormalsAsTuple, preallocate the exact result size based on tot_triangle_count and write values sequentially using an index instead of repeated vector::insert calls.	src/Tesselator/ShapeTesselator.cpp
Streamline X3D TriangleSet export by writing floats directly to output streams and avoiding intermediate index buffers and helper allocations.	Replace formatFloatNumber with inline writeFloat that writes directly to an std::ostringstream, treating very small values as 0 without constructing temporary strings. Rework ExportShapeToX3DTriangleSet to iterate triangles by index, derive vertex indices from consolidated_triangle_indices, and write vertex and normal components directly to separate vertex and normal streams using writeFloat. Simplify final X3D output assembly by building the TriangleSet markup in a single std::ostringstream using the previously accumulated vertex and normal buffers.	src/Tesselator/ShapeTesselator.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• In ExportShapeToX3DTriangleSet, normals are now derived via consolidated_triangle_indices and consolidated_normals instead of using ObjGetTriangle/normals_idx; please double-check that vertex and normal indexing are still aligned for all code paths, as this changes the previous separation of vertex and normal indices and may alter output for cases with distinct normal indexing.
• The new #include <mutex> appears unused after these changes; consider removing it to avoid confusion about implied synchronization mechanisms around the OpenMP parallelization.
• The OpenMP branch in ProcessFaces writes into face_list after the parallel region, but face_list is only reserved to faces.size() and not cleared beforehand; if Tessellate can be called multiple times on the same object, consider explicitly clearing face_list before appending to avoid accumulating stale data.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `ExportShapeToX3DTriangleSet`, normals are now derived via `consolidated_triangle_indices` and `consolidated_normals` instead of using `ObjGetTriangle`/`normals_idx`; please double-check that vertex and normal indexing are still aligned for all code paths, as this changes the previous separation of vertex and normal indices and may alter output for cases with distinct normal indexing.
- The new `#include <mutex>` appears unused after these changes; consider removing it to avoid confusion about implied synchronization mechanisms around the OpenMP parallelization.
- The OpenMP branch in `ProcessFaces` writes into `face_list` after the parallel region, but `face_list` is only reserved to `faces.size()` and not cleared beforehand; if `Tessellate` can be called multiple times on the same object, consider explicitly clearing `face_list` before appending to avoid accumulating stale data.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}