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

Simplify transform code by using cache instead of partial objects #1406

Merged
merged 8 commits into from
Nov 30, 2022

Conversation

hannobraun
Copy link
Owner

The core problem of transforming objects is that references in an object graph that were the same before transformation must be the same after transformation. For example, if the two vertices of a half-edge reference a single curve (which they must, otherwise the half-edge is invalid), then after transforming the half-edge, the vertices of the transformed half-edge must still reference a single (now transformed) curve.

Previously, this was achieved by converting full objects into partial objects before transformation, then transforming those, updating them as necessary (e.g. make sure the two vertices reference the same curve), then convert that back into a full object. In fact, this was the first use case for the partial object API.

This pull request changes that, making the transform logic much simpler. Instead of making the detour through partial objects, a simple cache is used, to make sure that each object in the object graph results in exactly one transformed object in the transformed object graph. The new transform code is extremely simple and hard to get wrong.

Beyond the benefits for the transform code itself, this pull request assists in advancing #1249, by taking some pressure off the partial object API. Although I haven't looked into it yet, I assume that some cleanup attempts that have previously stalled can now be made, as the partial object API has to support fewer use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant