Fix heap-buffer-overflow in getCharts() for cross-chart vertices#17
Open
jdumas wants to merge 1 commit intomkazhdan:masterfrom
Open
Fix heap-buffer-overflow in getCharts() for cross-chart vertices#17jdumas wants to merge 1 commit intomkazhdan:masterfrom
jdumas wants to merge 1 commit intomkazhdan:masterfrom
Conversation
The `getCharts()` function in `AtlasCharts.inl` uses a flat `chartVertexID` vector to map atlas mesh vertex indices to chart-local vertex indices. When an atlas mesh vertex is shared between two UV charts (a "pinch point" — same UV position and same surface vertex at a seam), the first chart sets `chartVertexID[v]` to its local index. The second chart then skips adding the vertex and reuses the first chart's local index, which is invalid for the second chart's vertex array. This causes an out-of-bounds read in `AtlasChart::vertex()` when the second chart later accesses its (smaller) vertex vector with an index that belongs to the first chart. The fix adds a parallel `chartVertexChart` vector to track which chart each vertex was last assigned to. When a vertex is encountered in a different chart, it is re-added to the new chart with a fresh local index. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AtlasChart::vertex()when atlas mesh vertices are shared between different UV charts (pinch points)getCharts()function used a single flatchartVertexIDmap that incorrectly reused local vertex indices across chartschartVertexChartvector to track which chart each vertex was assigned toDetails
The bug occurs when
AtlasMesh::initialize()deduplicates texture vertices by (UV position, surface vertex index). Two texture vertices at a UV seam pinch point with the same position and same surface vertex get merged into one atlas mesh vertex, even though they belong to different charts (computed via edge connectivity intrianglesToComponents).In
getCharts(), the globalchartVertexID[v]is set by the first chart that encounters vertexv. When a second chart encounters the same vertex, it skips adding it and reuses the first chart's local index, which is meaningless for the second chart's vertex array, causing an out-of-bounds access.Found using AddressSanitizer on a mesh with UV pinch points.