Mir

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

Revision history for this message
Kevin DuBois (kdub) wrote :

my thoughts:

pros:
* The occlusion detection made it difficult to unify the compositor's behavior around consuming buffers, which in turn made it unclear what the SwitchingBundle's responsibilities were. Consuming every buffer on a compositor pass makes it easier to get the switching logic right, and is at least a consistent requirement for a compositor writer.
* It does seem at odds to try to not consume a buffer, and never block a client.

cons:
* occlusion detection is valuable in reducing workload for the compositor (texture uploads)
* an obscured, visible client that can submit frames fast enough can keep the compositor recompositing the same pixels because of the 'uncomposited_buffers' logic.
* loosing the logic puts us in a worse position if we're ever told to send 'occluded' messages to the client. (although this is a weak point due to YAGNI)

I am interested in what never got fixed in platform-api that made this code unused though.
An alternative compromise between the pros and cons could be to consume the buffer from all surfaces (unifying the behavior of the compositors/BufferBundle), and not calling render() on the occluded surfaces. This would also mean fixing whatever is broken in platform-api?

I think that the benefits of doing this now outweigh the cost, especially as we refine the SurfaceStack/Compositor relationship. I guess I'll 'approve', but am still interested in other's thoughts, so lets not top-approve until a bit more discussion happens.

review: Approve

« Back to merge proposal