hannobraun/fornjot

Validation is no longer reliable in the presence of the geometry layer

hannobraun opened this issue · 0 comments

Context

I've recently started moving geometric information out of the object graph, into a separate geometry layer (#2116). I've been working on this for a few weeks, and overall, it's going well. However, I've now discovered a problem: The presence of the geometry layer allows new patterns of modifying object geometry (which is kinda the point). In the presence of these new patterns, the validation infrastructure no longer works reliably.

The Problem

Here's how validation works, currently:

  • Objects are immutable, so the only way to make changes is to create a new ones.
  • You may change an object in multiple step, creating temporary versions, but you store the final result.
  • When an object is inserted into the stores, validation is run for it.

Previously, all topological and geometric information was contained within objects. That meant, the only way to update this information was to create and store new objects. And since no other information is relevant for validation, that made sure that validation is run whenever relevant information changed.

With the geometry layer, it is now possible (by design) to change the geometry of an object, without creating a new object. This means, information that is relevant to validation can change, without validation checks being re-run. You can create an invalid object without noticing.

Impact

The impact is limited, so far. While it's possible to update the geometry of an existing object, this capability isn't really used within Fornjot yet. For example, the transform operation, which previously had to create new objects to have any impact, has only been updated in a minimally invasive manner, and is still creating new objects despite no longer needing to (with a proper redesign slated for later).

The impact will grow, as the work of extracting geometric data into the geometry layer continues, and more code is updated to reflect the new reality.

Solution

The validation layer needs to be expanded. Specifically, it needs to receive events whenever geometry is updated, which is straight-forward to do.

Unfortunately, processing those events is not as easy as re-running validation for the changed object. Changing the geometry of one object can have effects on the validation results of another. For example, updating the position of a surface changes where half-edges on that surface are located in 3D space. This could then cause a validation failure, if non-sibling half-edges are now coincident.

I think it's practical to keep track of these relationships and re-run validation checks as necessary. This won't be as effortlessly reliable as the current approach though, as changes to the object graph that change these relationships would need to be reflected in the validation layer

Open Questions

  • Does it make sense to introduce a more general solution for tracking those relationships relevant for validation? Some way to query the object graph, i.e. "give me every object of type x that references surface y"?
    • That would introduce significant complexity, and I'm leaning towards not doing that, unless more use cases for this kind of infrastructure emerge.
  • Is it actually desirable to run validation checks on every geometry change?
    • I'm pretty sure the answer is "no", as validation checks can be expensive, and geometry changes can occur at a high rate. For example if an application lets the user drag an edge to change geometry, updating it every pixel (this kind of case is where the initial idea for the geometry layer came from).
    • I'm currently leaning towards ignoring this problem and opening a follow-up issue for it.
    • One approach to addressing that, is to just keep track of which checks need to be re-run, but only do so on command. That could be configurable, so simple use cases (like unit tests) don't have to remember to run validation.
  • Will performance be impacted significantly, as the object stores grow? Currently, the object stores are append-only, meaning that superseded objects stay around forever.
    • I always assumed this would need to be addressed eventually, but so far only RAM use is impacted. With this issue addressed, validation would become more expensive over time, as more and more checks for more and more (now irrelevant) objects would need to run.
    • Hopefully it's not too bad and can be ignored for now. Eventually, the object stores would need to be garbage-collected, and once that is a thing, the validation layer can receive events whenever an object is collected, so it knows to no longer validate it.

Next Steps

I'm currently investigating (as part of #2116), whether I can ignore this and move on, or if I need to fix it now. I'd prefer to take care of the remaining geometry layer work first, as that is currently in a half-done state. I might even prefer to address #2118 first, as that would allow simplifying how geometry is represented, which would in turn make this issue easier. If I address this issue first, then all of those changes would create churn in the new code.

I think it would also make sense to finish #2157 first, for similar reasons. The cleaned up validation infrastructure would make this issue easier, and reduce churn later on.