Code review comment for lp:~vanvugt/mir/timerless-multimonitor-frame-sync

I like the approach, but some small concerns:

159 + if (global_frame_count.load() < local_frame_count ||
160 + local_frame_count <= 0) // Wrap around, unlikely, but handle it...
161 + {
162 + global_frame_count.store(local_frame_count);
163 + }
164 + else
165 + {
166 + local_frame_count = global_frame_count.load();
167 + }

As there is no synchronization between the first load and the store I don't think the logic is mathematically correct. Could we improve on this by using compare_exchange? (Not that I think there's much chance of a problem in practice.)

~~~~

134 +std::atomic_ulong mc::DefaultDisplayBufferCompositor::global_frame_count(0);

There's no reason for this to be "published" in a header: it could be a namespace scope variable (in anonymous namespace) rather than a static class member.

~~~~

I don't have a better name but "frameno" doesn't really indicate what the parameter is for.