Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute surface position on demand instead of storing it #1634

Closed
hannobraun opened this issue Mar 1, 2023 · 0 comments · Fixed by #1640
Closed

Compute surface position on demand instead of storing it #1634

hannobraun opened this issue Mar 1, 2023 · 0 comments · Fixed by #1640
Assignees
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs

Comments

@hannobraun
Copy link
Owner

(This issue is part of a larger cleanup effort. See #1589.)

SurfaceVertex is the representation of a vertex in surface coordinates. It stores the surface position of the vertex, and references GlobalVertex (which stores the global position). SurfaceVertex is referenced by HalfEdge and these SurfaceVertex references are shared by the neighboring HalfEdges. Since neighboring HalfEdges can be defined by different curves (and floating point numbers are not infinitely accurate), computing the position of a SurfaceVertex from the HalfEdge can lead to slightly different results for the same SurfaceVertex.

Such inconsistencies must be avoided, which is why surface positions are computed once and then stored in SurfaceVertex. However, there is a simpler way to void the inconsistency: By declaring that each HalfEdge "owns" its start vertex but not its end vertex (which is owned by the next HalfEdge in the Cycle), we can give each HalfEdge responsibility for computing the surface position of its start vertex. Then the stored position in SurfaceVertex can be removed and SurfaceVertex becomes redundant (only a reference to GlobalVertex will be left, which can just be inlined wherever SurfaceVertex is referenced).

This issue has some overlap with #1525, which also would result in SurfaceVertex being removed. The issues are not in conflict, however, and in fact complement each other quite well. With #1525 addressed, this issue would become extremely trivial. Addressing this issue would in turn address most of the rest of #1525. Which order this will end up happening in is going to depend on practicalities, and I'll figure this out as I go.

@hannobraun hannobraun added type: development Work to ease development or maintenance, without direct effect on features or bugs topic: core Issues relating to core geometry, operations, algorithms labels Mar 1, 2023
@hannobraun hannobraun self-assigned this Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Issues relating to core geometry, operations, algorithms type: development Work to ease development or maintenance, without direct effect on features or bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant