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 :wink:)