Mir

Code review comment for lp:~mir-team/mir/touchspot-renderable

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Not a complete review yet, just some first notes:

162 +class BufferWriter

At first glance it would be more natural to introduce a Buffer::write() method instead of a class that acts on the buffer, unless we want to potentially support multiple implementations of it.

1577 +void mi::TouchspotController::visualize_touches(std::vector<Spot> const& touches)

Perhaps the logic in this method would be more clear if we split it out into different stages, e.g.:

1. grow touchpot renderable vector as needed (already there)
2. for each touch: set up and add renderable to scene
3. for unused renderables: remove from scene

1618 + scene->emit_scene_changed();

We should call this out of lock if possible.

« Back to merge proposal