From 6ef1612dc9ea2d864657de0b86a4502e8c0fca25 Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 21 Sep 2026 15:45:16 +0200 Subject: [PATCH 1/8] Fix heap buffer overflow for high valence subdivision vertices The one-ring construction in CatmullClark1RingT::init() and GeneralCatmullClark1RingT::init() wrote into the fixed capacity ring buffers (MAX_RING_FACE_VALENCE / MAX_RING_EDGE_VALENCE) without any runtime bound, only asserts guarded these writes. A vertex shared by more than MAX_RING_FACE_VALENCE faces (or a ring with more than MAX_RING_EDGE_VALENCE edges) thus corrupted the heap in release builds. The ring construction now stops when the ring buffers are exhausted and clamps the evaluation start indices accordingly, so no out of bounds write can happen for any topology. In addition patches whose rings exceed the supported size limits can now get detected in constant time through a flag that is calculated at commit time for every topology. rtcInterpolate and rtcInterpolateN, which did not perform any validity check before, use this flag to reject such patches and return zeros. The ray tracing path already rejected these patches through SubdivMesh::valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- kernels/common/scene_subdiv_mesh.cpp | 30 ++++++++++++- kernels/subdiv/catmullclark_ring.h | 29 ++++++++++++ kernels/subdiv/half_edge.h | 66 +++++++++++++++++++++++++++- 3 files changed, 122 insertions(+), 3 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 4dc2080d36..804831e27c 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -561,8 +561,11 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - for (size_t i=0; ifaceVertices[f]; i++) + const char valid_sizes = edge->validPatchSizes() ? 1 : 0; + for (size_t i=0; ifaceVertices[f]; i++) { edge[i].patch_type = patch_type; + edge[i].valid_sizes = valid_sizes; + } } }); } @@ -878,6 +881,17 @@ namespace embree bool has_P = P; bool has_dP = dPdu; assert(!has_dP || dPdv); bool has_ddP = ddPdudu; assert(!has_ddP || (ddPdvdv && ddPdudu)); + + /* patches that exceed the supported valence limits cannot get evaluated */ + if (unlikely(!topo->getHalfEdge(primID)->hasValidSizes())) + { + for (unsigned int j=0; jgetHalfEdge(primID)->hasValidSizes())) + { + for (unsigned int j=0; j MAX_RING_EDGE_VALENCE)) + break; + vertex_level = max(vertex_level,p->edge_level); crease_weight[i/2] = p->edge_crease_weight; assert(p->hasOpposite() || p->edge_crease_weight == float(inf)); @@ -178,6 +182,9 @@ namespace embree if (index0 < min_vertex_index) { min_vertex_index = index0; min_vertex_index_face = i>>1; } /*! mark first border edge and store dummy vertex for face between the two border edges */ + if (unlikely(i+2 > MAX_RING_EDGE_VALENCE)) + break; + border_index = i; crease_weight[i/2] = inf; ring[i++] = Vertex_t::loadu(vertices+index0*stride); @@ -196,6 +203,10 @@ namespace embree eval_unique_identifier = min_vertex_index; eval_start_index = min_vertex_index_face; + /* the ring may have been truncated above, thus clamp the start index */ + if (unlikely(eval_start_index >= face_valence)) + eval_start_index = 0; + assert( hasValidPositions() ); } @@ -593,6 +604,10 @@ namespace embree vertex_level = 0.0f; do { + /* stop when the ring buffers are exhausted to avoid writing out of bounds */ + if (unlikely(f+1 > MAX_RING_FACE_VALENCE || e+1 > MAX_RING_EDGE_VALENCE)) + break; + HalfEdge* p_prev = p->prev(); HalfEdge* p_next = p->next(); const float crease_weight = p->edge_crease_weight; @@ -606,11 +621,16 @@ namespace embree /* store first N-2 vertices of face */ unsigned int vn = 0; for (p = p_next; p!=p_prev; p=p->next()) { + if (unlikely(e >= MAX_RING_EDGE_VALENCE)) break; ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); vn++; } faces[f++] = Face(vn,crease_weight); only_quads &= (vn == 2); + + /* stop in case the face got truncated above */ + if (unlikely(p != p_prev)) + break; /* continue with next face */ if (likely(p->hasOpposite())) @@ -624,6 +644,9 @@ namespace embree if (vertex_index < min_vertex_index) { min_vertex_index = vertex_index; min_vertex_index_face = f; min_vertex_index_vertex = e; } /*! mark first border edge and store dummy vertex for face between the two border edges */ + if (unlikely(f+1 > MAX_RING_FACE_VALENCE || e+2 > MAX_RING_EDGE_VALENCE)) + break; + border_face = f; faces[f++] = Face(2,inf); ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); @@ -643,6 +666,12 @@ namespace embree eval_start_face_index = min_vertex_index_face; eval_start_vertex_index = min_vertex_index_vertex; + /* the ring may have been truncated above, thus clamp the start indices */ + if (unlikely(eval_start_face_index >= face_valence)) + eval_start_face_index = 0; + if (unlikely(eval_start_vertex_index >= edge_valence)) + eval_start_vertex_index = 0; + assert( hasValidPositions() ); } diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index 8cb4f24845..b61ef1c013 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -48,7 +48,7 @@ namespace embree HalfEdge () : vtx_index(-1), next_half_edge_ofs(0), prev_half_edge_ofs(0), opposite_half_edge_ofs(0), edge_crease_weight(0), - vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX) + vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX), valid_sizes(0) { static_assert(sizeof(HalfEdge) == 32, "invalid half edge size"); } @@ -354,6 +354,67 @@ namespace embree return faceValence <= MAX_RING_FACE_VALENCE && edgeValence <= MAX_RING_EDGE_VALENCE; } + + /*! tests if the ring around the start vertex is within the supported size + * limits. In contrast to validRing this test only depends on the topology + * and not on the vertex positions. */ + __forceinline bool validRingSizes() const + { + size_t faceValence = 0; + size_t edgeValence = 0; + + const HalfEdge* p = this; + do + { + /* check size of current face */ + const size_t n = p->numEdges(); + if (n < 3 || n > MAX_PATCH_VALENCE) + return false; + edgeValence += n-2; + + faceValence++; + p = p->prev(); + + /* continue with next face */ + if (likely(p->hasOpposite())) + p = p->opposite(); + + /* if there is no opposite go the long way to the other side of the border */ + else { + faceValence++; + edgeValence++; + p = this; + while (p->hasOpposite()) + p = p->opposite()->next(); + } + + /* stop early for degenerated topology */ + if (faceValence > MAX_RING_FACE_VALENCE || edgeValence > MAX_RING_EDGE_VALENCE) + return false; + + } while (p != this); + + return true; + } + + public: + + /*! tests if this patch and all its rings are within the supported size + * limits. Patches that are not, cannot get evaluated. */ + __forceinline bool validPatchSizes() const + { + size_t N = 1; + if (!this->validRingSizes()) return false; + for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { + if (!p->validRingSizes()) return false; + } + return N >= 3 && N <= MAX_PATCH_VALENCE; + } + + /*! returns the cached result of validPatchSizes computed at commit time */ + __forceinline bool hasValidSizes() const { + return valid_sizes != 0; + } private: unsigned int vtx_index; //!< index of edge start vertex @@ -367,6 +428,7 @@ namespace embree float edge_level; //!< subdivision factor for edge PatchType patch_type; //!< stores type of subdiv patch VertexType vertex_type; //!< stores type of the start vertex - char align[2]; + char valid_sizes; //!< stores if the patch and all its rings are within the supported size limits + char align[1]; }; } From 126d1247b4f0f16cc0b356f2479c2e47c3b1156c Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Wed, 23 Sep 2026 10:11:55 +0200 Subject: [PATCH 2/8] Address subdivision validity review feedback Fold the high-valence subdivision patch check into the existing Topology::valid() path instead of carrying a separate hasValidSizes() query at interpolation sites. The cached half-edge flag is now a bool and interpolation uses the same validity model as the builders, while still checking the selected attribute topology when vertex attributes are bound to a non-default topology. With invalid patches filtered before PatchEval, remove the defensive ring truncation code from the Catmull-Clark ring constructors and keep assertions for the invariants that commit-time validation guarantees. This avoids constructing partially truncated rings and keeps invalid patch handling centralized in the topology validity path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- kernels/common/scene_subdiv_mesh.cpp | 20 +++++++++------- kernels/common/scene_subdiv_mesh.h | 2 ++ kernels/subdiv/catmullclark_ring.h | 35 +++++----------------------- kernels/subdiv/half_edge.h | 17 +++++--------- 4 files changed, 26 insertions(+), 48 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 804831e27c..2545b506b9 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -561,10 +561,10 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - const char valid_sizes = edge->validPatchSizes() ? 1 : 0; + const bool valid = edge->validPatchTopology(); for (size_t i=0; ifaceVertices[f]; i++) { edge[i].patch_type = patch_type; - edge[i].valid_sizes = valid_sizes; + edge[i].valid_patch = valid; } } }); @@ -882,8 +882,10 @@ namespace embree bool has_dP = dPdu; assert(!has_dP || dPdv); bool has_ddP = ddPdudu; assert(!has_ddP || (ddPdvdv && ddPdudu)); - /* patches that exceed the supported valence limits cannot get evaluated */ - if (unlikely(!topo->getHalfEdge(primID)->hasValidSizes())) + const HalfEdge* halfEdge = topo->getHalfEdge(primID); + + /* invalid patches cannot get evaluated */ + if (unlikely(!this->valid(primID) || !topo->valid(primID))) { for (unsigned int j=0; j(baseEntry->at(interpolationSlot(primID,i/4,stride)),commitCounter, - topo->getHalfEdge(primID),src+i*sizeof(float),stride,u,v, + halfEdge,src+i*sizeof(float),stride,u,v, has_P ? &Pt : nullptr, has_dP ? &dPdut : nullptr, has_dP ? &dPdvt : nullptr, @@ -980,8 +982,10 @@ namespace embree foreach_unique(valid1,primID,[&](const vbool4& valid1, const unsigned int primID) { - /* patches that exceed the supported valence limits cannot get evaluated */ - if (unlikely(!topo->getHalfEdge(primID)->hasValidSizes())) + const HalfEdge* halfEdge = topo->getHalfEdge(primID); + + /* invalid patches cannot get evaluated */ + if (unlikely(!this->valid(primID) || !topo->valid(primID))) { for (unsigned int j=0; j(baseEntry->at(interpolationSlot(primID,j/4,stride)),commitCounter, - topo->getHalfEdge(primID),src+j*sizeof(float),stride,valid1,uu,vv, + halfEdge,src+j*sizeof(float),stride,valid1,uu,vv, P ? P+j*N+i : nullptr, dPdu ? dPdu+j*N+i : nullptr, dPdv ? dPdv+j*N+i : nullptr, diff --git a/kernels/common/scene_subdiv_mesh.h b/kernels/common/scene_subdiv_mesh.h index 49bb8e769a..d99988f4da 100644 --- a/kernels/common/scene_subdiv_mesh.h +++ b/kernels/common/scene_subdiv_mesh.h @@ -169,6 +169,8 @@ namespace embree /*! check if the i'th primitive is valid in this topology */ __forceinline bool valid(size_t i) const { + if (unlikely(!getHalfEdge(i)->valid_patch)) + return false; if (unlikely(subdiv_mode == RTC_SUBDIVISION_MODE_NO_BOUNDARY)) { if (getHalfEdge(i)->faceHasBorder()) return false; } diff --git a/kernels/subdiv/catmullclark_ring.h b/kernels/subdiv/catmullclark_ring.h index ad11657188..1a046a0312 100644 --- a/kernels/subdiv/catmullclark_ring.h +++ b/kernels/subdiv/catmullclark_ring.h @@ -151,10 +151,7 @@ namespace embree do { - /* stop when the ring buffers are exhausted to avoid writing out of bounds */ - if (unlikely(i+2 > MAX_RING_EDGE_VALENCE)) - break; - + assert(i+2 <= MAX_RING_EDGE_VALENCE); vertex_level = max(vertex_level,p->edge_level); crease_weight[i/2] = p->edge_crease_weight; assert(p->hasOpposite() || p->edge_crease_weight == float(inf)); @@ -182,9 +179,7 @@ namespace embree if (index0 < min_vertex_index) { min_vertex_index = index0; min_vertex_index_face = i>>1; } /*! mark first border edge and store dummy vertex for face between the two border edges */ - if (unlikely(i+2 > MAX_RING_EDGE_VALENCE)) - break; - + assert(i+2 <= MAX_RING_EDGE_VALENCE); border_index = i; crease_weight[i/2] = inf; ring[i++] = Vertex_t::loadu(vertices+index0*stride); @@ -203,10 +198,6 @@ namespace embree eval_unique_identifier = min_vertex_index; eval_start_index = min_vertex_index_face; - /* the ring may have been truncated above, thus clamp the start index */ - if (unlikely(eval_start_index >= face_valence)) - eval_start_index = 0; - assert( hasValidPositions() ); } @@ -604,10 +595,6 @@ namespace embree vertex_level = 0.0f; do { - /* stop when the ring buffers are exhausted to avoid writing out of bounds */ - if (unlikely(f+1 > MAX_RING_FACE_VALENCE || e+1 > MAX_RING_EDGE_VALENCE)) - break; - HalfEdge* p_prev = p->prev(); HalfEdge* p_next = p->next(); const float crease_weight = p->edge_crease_weight; @@ -621,16 +608,13 @@ namespace embree /* store first N-2 vertices of face */ unsigned int vn = 0; for (p = p_next; p!=p_prev; p=p->next()) { - if (unlikely(e >= MAX_RING_EDGE_VALENCE)) break; + assert(e < MAX_RING_EDGE_VALENCE); ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); vn++; } + assert(f < MAX_RING_FACE_VALENCE); faces[f++] = Face(vn,crease_weight); only_quads &= (vn == 2); - - /* stop in case the face got truncated above */ - if (unlikely(p != p_prev)) - break; /* continue with next face */ if (likely(p->hasOpposite())) @@ -644,9 +628,8 @@ namespace embree if (vertex_index < min_vertex_index) { min_vertex_index = vertex_index; min_vertex_index_face = f; min_vertex_index_vertex = e; } /*! mark first border edge and store dummy vertex for face between the two border edges */ - if (unlikely(f+1 > MAX_RING_FACE_VALENCE || e+2 > MAX_RING_EDGE_VALENCE)) - break; - + assert(f < MAX_RING_FACE_VALENCE); + assert(e+2 <= MAX_RING_EDGE_VALENCE); border_face = f; faces[f++] = Face(2,inf); ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); @@ -666,12 +649,6 @@ namespace embree eval_start_face_index = min_vertex_index_face; eval_start_vertex_index = min_vertex_index_vertex; - /* the ring may have been truncated above, thus clamp the start indices */ - if (unlikely(eval_start_face_index >= face_valence)) - eval_start_face_index = 0; - if (unlikely(eval_start_vertex_index >= edge_valence)) - eval_start_vertex_index = 0; - assert( hasValidPositions() ); } diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index b61ef1c013..7fbd7892d2 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -48,7 +48,7 @@ namespace embree HalfEdge () : vtx_index(-1), next_half_edge_ofs(0), prev_half_edge_ofs(0), opposite_half_edge_ofs(0), edge_crease_weight(0), - vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX), valid_sizes(0) + vertex_crease_weight(0), edge_level(0), patch_type(COMPLEX_PATCH), vertex_type(REGULAR_VERTEX), valid_patch(false) { static_assert(sizeof(HalfEdge) == 32, "invalid half edge size"); } @@ -358,7 +358,7 @@ namespace embree /*! tests if the ring around the start vertex is within the supported size * limits. In contrast to validRing this test only depends on the topology * and not on the vertex positions. */ - __forceinline bool validRingSizes() const + __forceinline bool validRingTopology() const { size_t faceValence = 0; size_t edgeValence = 0; @@ -401,20 +401,15 @@ namespace embree /*! tests if this patch and all its rings are within the supported size * limits. Patches that are not, cannot get evaluated. */ - __forceinline bool validPatchSizes() const + __forceinline bool validPatchTopology() const { size_t N = 1; - if (!this->validRingSizes()) return false; + if (!this->validRingTopology()) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRingSizes()) return false; + if (!p->validRingTopology()) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } - - /*! returns the cached result of validPatchSizes computed at commit time */ - __forceinline bool hasValidSizes() const { - return valid_sizes != 0; - } private: unsigned int vtx_index; //!< index of edge start vertex @@ -428,7 +423,7 @@ namespace embree float edge_level; //!< subdivision factor for edge PatchType patch_type; //!< stores type of subdiv patch VertexType vertex_type; //!< stores type of the start vertex - char valid_sizes; //!< stores if the patch and all its rings are within the supported size limits + bool valid_patch; //!< stores if the patch can be evaluated char align[1]; }; } From 777e2e2e33d9eafe41d5695e6ac5fdd8c98a9be6 Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 12 Oct 2026 15:18:37 +0200 Subject: [PATCH 3/8] Tighten subdivision validity review fixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- kernels/common/scene_subdiv_mesh.cpp | 4 ++-- kernels/subdiv/catmullclark_ring.h | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 2545b506b9..e948d542d4 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -885,7 +885,7 @@ namespace embree const HalfEdge* halfEdge = topo->getHalfEdge(primID); /* invalid patches cannot get evaluated */ - if (unlikely(!this->valid(primID) || !topo->valid(primID))) + if (unlikely(!topo->valid(primID))) { for (unsigned int j=0; jgetHalfEdge(primID); /* invalid patches cannot get evaluated */ - if (unlikely(!this->valid(primID) || !topo->valid(primID))) + if (unlikely(!topo->valid(primID))) { for (unsigned int j=0; jedge_level); crease_weight[i/2] = p->edge_crease_weight; assert(p->hasOpposite() || p->edge_crease_weight == float(inf)); @@ -179,7 +178,6 @@ namespace embree if (index0 < min_vertex_index) { min_vertex_index = index0; min_vertex_index_face = i>>1; } /*! mark first border edge and store dummy vertex for face between the two border edges */ - assert(i+2 <= MAX_RING_EDGE_VALENCE); border_index = i; crease_weight[i/2] = inf; ring[i++] = Vertex_t::loadu(vertices+index0*stride); @@ -608,11 +606,9 @@ namespace embree /* store first N-2 vertices of face */ unsigned int vn = 0; for (p = p_next; p!=p_prev; p=p->next()) { - assert(e < MAX_RING_EDGE_VALENCE); ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); vn++; } - assert(f < MAX_RING_FACE_VALENCE); faces[f++] = Face(vn,crease_weight); only_quads &= (vn == 2); @@ -628,8 +624,6 @@ namespace embree if (vertex_index < min_vertex_index) { min_vertex_index = vertex_index; min_vertex_index_face = f; min_vertex_index_vertex = e; } /*! mark first border edge and store dummy vertex for face between the two border edges */ - assert(f < MAX_RING_FACE_VALENCE); - assert(e+2 <= MAX_RING_EDGE_VALENCE); border_face = f; faces[f++] = Face(2,inf); ring[e++] = Vertex_t::loadu(vertices+p->getStartVertexIndex()*stride); From 41db7ff3123ac4ef65bfc9f21c13386a7f9d4ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Je=CC=81re=CC=81mie=20Dumas?= Date: Wed, 30 Sep 2026 09:04:09 -0700 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=90=9B=20Restore=20explicit=20SIMD=20?= =?UTF-8?q?mask=20accessors=20for=20Emscripten?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep m128i() and m128d() available when compiling with Emscripten. SSE2 equality, shuffle, and masked integer loads call the explicit accessors regardless of platform. Continue excluding the implicit integer and double conversion operators for Emscripten, and leave the MSVC ARM64 path unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- common/simd/vboolf4_sse2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/simd/vboolf4_sse2.h b/common/simd/vboolf4_sse2.h index 9b85adb70a..8c16d730a3 100644 --- a/common/simd/vboolf4_sse2.h +++ b/common/simd/vboolf4_sse2.h @@ -58,10 +58,10 @@ namespace embree #if !defined(__EMSCRIPTEN__) __forceinline operator const __m128i() const { return _mm_castps_si128(v); } __forceinline operator const __m128d() const { return _mm_castps_pd(v); } + #endif /* kept for source compatibility with code that calls .m128i()/.m128d() explicitly */ __forceinline const __m128i m128i() const { return _mm_castps_si128(v); } __forceinline const __m128d m128d() const { return _mm_castps_pd(v); } - #endif #endif __forceinline vboolf(bool a) From 974887112a3de5375b119dcd82cbec1018fe407c Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Fri, 23 Oct 2026 15:54:26 +0200 Subject: [PATCH 5/8] deduplicated some code --- kernels/common/scene_subdiv_mesh.cpp | 6 ++- kernels/subdiv/half_edge.h | 80 +++++++--------------------- 2 files changed, 24 insertions(+), 62 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 1786b975f6..ddb44b8ff8 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -566,7 +566,11 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - const bool valid = edge->validPatchTopology(); + bool valid = edge->validPatchTopology(); + const HalfEdge* geometryEdge = &mesh->topology[0].halfEdges[mesh->faceStartEdge[f]]; + for (size_t t=0; tnumTimeSteps && valid; t++) { + valid = valid && geometryEdge->valid(mesh->vertices[t]); + } for (size_t i=0; ifaceVertices[f]; i++) { edge[i].patch_type = patch_type; edge[i].valid_patch = valid; diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index 7fbd7892d2..c668b40596 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -205,9 +205,9 @@ namespace embree __forceinline bool valid(const BufferView& vertices) const { size_t N = 1; - if (!this->validRing(vertices)) return false; + if (!this->validRing(&vertices)) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(vertices)) return false; + if (!p->validRing(&vertices)) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } @@ -306,23 +306,9 @@ namespace embree return bounds; } - /*! tests if this is a valid face */ - __forceinline bool validFace(const BufferView& vertices, size_t& N) const - { - const Vec3fa v = vertices[getStartVertexIndex()]; - if (!isvalid(v)) return false; - size_t n = 1; - for (const HalfEdge* p = next(); p!=this; p=p->next(), n++) { - const Vec3fa v = vertices[p->getStartVertexIndex()]; - if (!isvalid(v)) return false; - } - if (n < 2) return false; - N += n-2; - return n >= 3 && n <= MAX_PATCH_VALENCE; - } - - /*! tests if this is a valid ring */ - __forceinline bool validRing(const BufferView& vertices) const + private: + template + __forceinline bool validRing(const BufferView* vertices) const { size_t faceValence = 0; size_t edgeValence = 0; @@ -330,9 +316,19 @@ namespace embree const HalfEdge* p = this; do { - /* calculate bounds of current face */ - if (!p->validFace(vertices,edgeValence)) + size_t n = 0; + const HalfEdge* q = p; + do + { + if (checkVertices && vertices && !isvalid((*vertices)[q->getStartVertexIndex()])) + return false; + n++; + q = q->next(); + } while (q != p); + + if (n < 3 || n > MAX_PATCH_VALENCE) return false; + edgeValence += n-2; faceValence++; p = p->prev(); @@ -349,44 +345,6 @@ namespace embree while (p->hasOpposite()) p = p->opposite()->next(); } - - } while (p != this); - - return faceValence <= MAX_RING_FACE_VALENCE && edgeValence <= MAX_RING_EDGE_VALENCE; - } - - /*! tests if the ring around the start vertex is within the supported size - * limits. In contrast to validRing this test only depends on the topology - * and not on the vertex positions. */ - __forceinline bool validRingTopology() const - { - size_t faceValence = 0; - size_t edgeValence = 0; - - const HalfEdge* p = this; - do - { - /* check size of current face */ - const size_t n = p->numEdges(); - if (n < 3 || n > MAX_PATCH_VALENCE) - return false; - edgeValence += n-2; - - faceValence++; - p = p->prev(); - - /* continue with next face */ - if (likely(p->hasOpposite())) - p = p->opposite(); - - /* if there is no opposite go the long way to the other side of the border */ - else { - faceValence++; - edgeValence++; - p = this; - while (p->hasOpposite()) - p = p->opposite()->next(); - } /* stop early for degenerated topology */ if (faceValence > MAX_RING_FACE_VALENCE || edgeValence > MAX_RING_EDGE_VALENCE) @@ -404,9 +362,9 @@ namespace embree __forceinline bool validPatchTopology() const { size_t N = 1; - if (!this->validRingTopology()) return false; + if (!this->validRing(nullptr)) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRingTopology()) return false; + if (!p->validRing(nullptr)) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } From aebc82e0bd27f5f0be0b51a4b1e0c2778e70f02c Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 26 Oct 2026 13:13:03 +0100 Subject: [PATCH 6/8] Simplified checks for invalid subd geometry --- kernels/common/scene_subdiv_mesh.cpp | 2 +- kernels/subdiv/half_edge.h | 64 +++++++++++----------------- 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 13193deddb..873a453054 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -566,7 +566,7 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - bool valid = edge->validPatchTopology(); + bool valid = true; const HalfEdge* geometryEdge = &mesh->topology[0].halfEdges[mesh->faceStartEdge[f]]; for (size_t t=0; tnumTimeSteps && valid; t++) { valid = valid && geometryEdge->valid(mesh->vertices[t]); diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index c668b40596..5704c7b17e 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -205,9 +205,9 @@ namespace embree __forceinline bool valid(const BufferView& vertices) const { size_t N = 1; - if (!this->validRing(&vertices)) return false; + if (!this->validRing(vertices)) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(&vertices)) return false; + if (!p->validRing(vertices)) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } @@ -306,9 +306,23 @@ namespace embree return bounds; } - private: - template - __forceinline bool validRing(const BufferView* vertices) const + /*! tests if this is a valid face */ + __forceinline bool validFace(const BufferView& vertices, size_t& N) const + { + const Vec3fa v = vertices[getStartVertexIndex()]; + if (!isvalid(v)) return false; + size_t n = 1; + for (const HalfEdge* p = next(); p!=this; p=p->next(), n++) { + const Vec3fa v = vertices[p->getStartVertexIndex()]; + if (!isvalid(v)) return false; + } + if (n < 2) return false; + N += n-2; + return n >= 3 && n <= MAX_PATCH_VALENCE; + } + + /*! tests if this is a valid ring */ + __forceinline bool validRing(const BufferView& vertices) const { size_t faceValence = 0; size_t edgeValence = 0; @@ -316,19 +330,9 @@ namespace embree const HalfEdge* p = this; do { - size_t n = 0; - const HalfEdge* q = p; - do - { - if (checkVertices && vertices && !isvalid((*vertices)[q->getStartVertexIndex()])) - return false; - n++; - q = q->next(); - } while (q != p); - - if (n < 3 || n > MAX_PATCH_VALENCE) + /* calculate bounds of current face */ + if (!p->validFace(vertices,edgeValence)) return false; - edgeValence += n-2; faceValence++; p = p->prev(); @@ -345,28 +349,10 @@ namespace embree while (p->hasOpposite()) p = p->opposite()->next(); } - - /* stop early for degenerated topology */ - if (faceValence > MAX_RING_FACE_VALENCE || edgeValence > MAX_RING_EDGE_VALENCE) - return false; - - } while (p != this); - - return true; - } - - public: - - /*! tests if this patch and all its rings are within the supported size - * limits. Patches that are not, cannot get evaluated. */ - __forceinline bool validPatchTopology() const - { - size_t N = 1; - if (!this->validRing(nullptr)) return false; - for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(nullptr)) return false; - } - return N >= 3 && N <= MAX_PATCH_VALENCE; + + } while (p != this); + + return faceValence <= MAX_RING_FACE_VALENCE && edgeValence <= MAX_RING_EDGE_VALENCE; } private: From 217a7b94acf601374762bd0fd0fe3aa3afe85230 Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Mon, 26 Oct 2026 15:34:37 +0100 Subject: [PATCH 7/8] Revert "Simplified checks for invalid subd geometry" This reverts commit aebc82e0bd27f5f0be0b51a4b1e0c2778e70f02c. --- kernels/common/scene_subdiv_mesh.cpp | 2 +- kernels/subdiv/half_edge.h | 64 +++++++++++++++++----------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 873a453054..13193deddb 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -566,7 +566,7 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - bool valid = true; + bool valid = edge->validPatchTopology(); const HalfEdge* geometryEdge = &mesh->topology[0].halfEdges[mesh->faceStartEdge[f]]; for (size_t t=0; tnumTimeSteps && valid; t++) { valid = valid && geometryEdge->valid(mesh->vertices[t]); diff --git a/kernels/subdiv/half_edge.h b/kernels/subdiv/half_edge.h index 5704c7b17e..c668b40596 100644 --- a/kernels/subdiv/half_edge.h +++ b/kernels/subdiv/half_edge.h @@ -205,9 +205,9 @@ namespace embree __forceinline bool valid(const BufferView& vertices) const { size_t N = 1; - if (!this->validRing(vertices)) return false; + if (!this->validRing(&vertices)) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(vertices)) return false; + if (!p->validRing(&vertices)) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } @@ -306,23 +306,9 @@ namespace embree return bounds; } - /*! tests if this is a valid face */ - __forceinline bool validFace(const BufferView& vertices, size_t& N) const - { - const Vec3fa v = vertices[getStartVertexIndex()]; - if (!isvalid(v)) return false; - size_t n = 1; - for (const HalfEdge* p = next(); p!=this; p=p->next(), n++) { - const Vec3fa v = vertices[p->getStartVertexIndex()]; - if (!isvalid(v)) return false; - } - if (n < 2) return false; - N += n-2; - return n >= 3 && n <= MAX_PATCH_VALENCE; - } - - /*! tests if this is a valid ring */ - __forceinline bool validRing(const BufferView& vertices) const + private: + template + __forceinline bool validRing(const BufferView* vertices) const { size_t faceValence = 0; size_t edgeValence = 0; @@ -330,9 +316,19 @@ namespace embree const HalfEdge* p = this; do { - /* calculate bounds of current face */ - if (!p->validFace(vertices,edgeValence)) + size_t n = 0; + const HalfEdge* q = p; + do + { + if (checkVertices && vertices && !isvalid((*vertices)[q->getStartVertexIndex()])) + return false; + n++; + q = q->next(); + } while (q != p); + + if (n < 3 || n > MAX_PATCH_VALENCE) return false; + edgeValence += n-2; faceValence++; p = p->prev(); @@ -349,10 +345,28 @@ namespace embree while (p->hasOpposite()) p = p->opposite()->next(); } - - } while (p != this); - - return faceValence <= MAX_RING_FACE_VALENCE && edgeValence <= MAX_RING_EDGE_VALENCE; + + /* stop early for degenerated topology */ + if (faceValence > MAX_RING_FACE_VALENCE || edgeValence > MAX_RING_EDGE_VALENCE) + return false; + + } while (p != this); + + return true; + } + + public: + + /*! tests if this patch and all its rings are within the supported size + * limits. Patches that are not, cannot get evaluated. */ + __forceinline bool validPatchTopology() const + { + size_t N = 1; + if (!this->validRing(nullptr)) return false; + for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { + if (!p->validRing(nullptr)) return false; + } + return N >= 3 && N <= MAX_PATCH_VALENCE; } private: From 6dc9a318d0a96ce47e0bb17d1b528390cf8a70de Mon Sep 17 00:00:00 2001 From: "Werner, Stefan" Date: Thu, 29 Oct 2026 12:52:36 +0100 Subject: [PATCH 8/8] Make HalfEdge::valid() take an optional vertex buffer Pass NULL to skip the vertex check; this replaces validPatchTopology(). rtcInterpolate now rejects only patches flagged at commit as exceeding ring size limits. --- kernels/common/scene_subdiv_mesh.cpp | 12 ++++-------- kernels/subdiv/half_edge.h | 25 +++++-------------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/kernels/common/scene_subdiv_mesh.cpp b/kernels/common/scene_subdiv_mesh.cpp index 13193deddb..0620975252 100644 --- a/kernels/common/scene_subdiv_mesh.cpp +++ b/kernels/common/scene_subdiv_mesh.cpp @@ -543,7 +543,7 @@ namespace embree { /* calculate if face is valid */ for (size_t t=0; tnumTimeSteps; t++) - mesh->invalidFace(f,t) = !edge->valid(mesh->vertices[t]) || mesh->holeSet->holeSet.lookup(unsigned(f)); + mesh->invalidFace(f,t) = !edge->valid(&mesh->vertices[t]) || mesh->holeSet->holeSet.lookup(unsigned(f)); } /* pin some edges and vertices */ @@ -566,11 +566,7 @@ namespace embree /* we have to calculate patch_type last! */ HalfEdge::PatchType patch_type = edge->patchType(); - bool valid = edge->validPatchTopology(); - const HalfEdge* geometryEdge = &mesh->topology[0].halfEdges[mesh->faceStartEdge[f]]; - for (size_t t=0; tnumTimeSteps && valid; t++) { - valid = valid && geometryEdge->valid(mesh->vertices[t]); - } + bool valid = edge->valid(nullptr); for (size_t i=0; ifaceVertices[f]; i++) { edge[i].patch_type = patch_type; edge[i].valid_patch = valid; @@ -896,7 +892,7 @@ namespace embree const HalfEdge* halfEdge = topo->getHalfEdge(primID); /* invalid patches cannot get evaluated */ - if (unlikely(!topo->valid(primID))) + if (unlikely(!halfEdge->valid_patch)) { for (unsigned int j=0; jgetHalfEdge(primID); /* invalid patches cannot get evaluated */ - if (unlikely(!topo->valid(primID))) + if (unlikely(!halfEdge->valid_patch)) { for (unsigned int j=0; j& vertices) const + /*! tests if this is a valid patch, the vertex check is skipped when vertices is NULL */ + __forceinline bool valid(const BufferView* vertices) const { size_t N = 1; - if (!this->validRing(&vertices)) return false; + if (!this->validRing(vertices)) return false; for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(&vertices)) return false; + if (!p->validRing(vertices)) return false; } return N >= 3 && N <= MAX_PATCH_VALENCE; } @@ -307,7 +307,6 @@ namespace embree } private: - template __forceinline bool validRing(const BufferView* vertices) const { size_t faceValence = 0; @@ -320,7 +319,7 @@ namespace embree const HalfEdge* q = p; do { - if (checkVertices && vertices && !isvalid((*vertices)[q->getStartVertexIndex()])) + if (vertices && !isvalid((*vertices)[q->getStartVertexIndex()])) return false; n++; q = q->next(); @@ -355,20 +354,6 @@ namespace embree return true; } - public: - - /*! tests if this patch and all its rings are within the supported size - * limits. Patches that are not, cannot get evaluated. */ - __forceinline bool validPatchTopology() const - { - size_t N = 1; - if (!this->validRing(nullptr)) return false; - for (const HalfEdge* p=this->next(); p!=this; p=p->next(), N++) { - if (!p->validRing(nullptr)) return false; - } - return N >= 3 && N <= MAX_PATCH_VALENCE; - } - private: unsigned int vtx_index; //!< index of edge start vertex int next_half_edge_ofs; //!< relative offset to next half edge of face