Code review comment for lp:~vanvugt/mir/reverse-scene-traversal

*Needs discussion*

I think we need a serious rethink of the interface Scene provides. It is evolving into a bad case of coupling.

lock()/unlock() are a problem because they imply locking the whole of the data model, not just the part relevant to the current operation.

Locking the whole data model is problematic if, for example, you want to composit different screens on different threads (which we attempt).

This MP adding forward and reverse traversals and expecting the results to make sense requires maintaining a lock across connected traversals. That will make it difficult to reduce the scope of locking to affected parts of the data.

I know at present there's no way to lock just a subset of the current model - but this interface makes that hard to change. The lock()/unlock() were originally a hack around the lack of a post-traversal call on OperatorForScene.

I think we want something like:

  auto surfaces = scene->select(filter);

  for (auto x : subscene) ...

and/or

  for (auto it = surfaces.rbegin(); it != surfaces.rend(); ++it)

With "~surfaces" releasing the lock.