Code review comment for lp:~vanvugt/mir/bypass

There's a lot going on here. But it looks like only Alan's raising new issues that "Needs Fixing" right now...

Alan,

I agree BasicDisplayBufferCompositor is a bit odd and could possibly be cleaned up. But that's a pre-existing issue.

I dislike BypassFilter/BypassMatch too. But that's all the Scene interface gives us right now. I would prefer that Scene simply provided an iterator (which points to surfaces) instead. But I thought in the interest of keeping diffs small and delivering on time, I should work with the existing iteration interface.

On first read I like your proposal of DisplayBuffer::effects_bypass. But then I realized that would either move the bypass decision logic into the interface class DisplayBuffer, or it would move the platform-independent decision logic into GBMDisplayBuffer. Hence rendering it inaccessible to other platforms/drivers. So I think the decision logic is best placed where it is -- in the compositing logic common to all platforms.

Re: lambdas... Again, I would prefer to see a simple Scene::iterator instead. But that's a pre-existing problem.

pointer used after mutex unlocked: I'm not sure there's significant risk yet. Will have to investigate and likely (regretfully) have to change the Scene interface. The big issue is that I cannot use OperatorForScene it is current form. Because I need to ensure only one surface gets bypassed per frame. That means doing it after for_each_if().