Fix UI crashes if editing a constrained coordinate that results in a not-assemble-able model topology

[adamkewley]

Very specific bug:

• User opened model containing a mixture of coordinates, where some coordinates have is_free_to_satisfy_contraints set to true
• When dragging a coordinate in the model's slider in the Coordinates panel, the UI will crash

Tracing the crash, it's a nullptr segfault. Looks like this:

• coordinate.getSpeed(state); segfaults
• Because an earlier part of the UI draw draws the value column, which the user is editing
• The user edit is calling coordinate.setValue(state, value);. This is usually fine for most models.
• However, with a model that has free_to_satisfy_constraints enabled, setValue can throw an exception during a projectQ call in the implementation
• If it throws an exception, OSC will detect the exception and call model.rollback() to try and preserve the user's model
• This all occurs midway in the UI rendering loop, which has something like for (Coordinate coord : coordinates) {}
• The iterator for that loop is invalidated by the rollback, because rolling back also involves calling Model::buildSystem etc., which will re-synthesize the OpenSim::Coordinates

[adamkewley]

Reading into this more, the bug in OpenSim Creator is subtle, but needs to be sorted:

• OSC's iterating over OpenSim::Coordinates in a range for loop to render the coordinate editor UI (this is usually fine)
• Changing the value/speed of a coordinate that's constrained (via is_free_to_satisfy_constraints) can throw an exception from OpenSim if those constraints can't be satisfied
• OSC detects the exception and handles it by calling model.rollback() (the assumption being that the last not-yet-committed edit is probably the source of the exception, so roll it back to an earlier version)
• The rollback mechanism overwrites the edited scratch-space model with a probably-safe committed version of the model.
• The copy assignment overwrite operation nukes the CoordinateSet and Coordinates in the scratch-space model
• The range for loop in the UI now contains out-of-date data (it's looking at the pre-rollback memory)
• Segfault

The architectural issue here is that the UI is iterating over data that's being indirectly edited via some other mechanism (rollbacks), which wasn't being exercised before because setting a coordinate's value was usually safe (because most coordinates don't have this constraint-satisfaction thing going on).

The architectural solution is to change the core (undoable) model datastructure in OSC to not allow direct model mutation and, instead, provide an API like queueMutation(std::function<bool(OpenSim::Model&)>); which applies all mutations after the UI has finished rendering a frame, so that there's no chance that the UI becomes inconsistent with the thing it's iterating over.

(and now I'm beginning to see why retained-mode UIs separate mutation propagation from painting 😉)

[adamkewley]

Looking into this, it will require double-buffering the UndoableModelStatePair, such that the UI only reads a never-modified-during-a-frame version of the model and mutating methods play with a backbuffer that's swapped at a heavily-controlled time in the frame draw

[adamkewley]

The UI now throws exceptions much more aggressively when it encounters issues, which causes control to reach the top of the draw call, revert there (where it's safer, further up the stack, etc.) and then go back to rendering.