Mir

Code review comment for lp:~alan-griffiths/mir/fix-1535894

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Oh I forgot, we already have this feature... :)

void mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& scene_elements)
{
    report->began_frame(this);

    auto const& view_area = display_buffer.view_area();
    auto const& occlusions = mc::filter_occlusions_from(scene_elements, view_area);

Using this existing implementation would be preferable. It's just harder to modify now that we have display groups with the swap buffers call outside of the compositor. But not impossible. Plus doing the fix in the compositor would address my concerns for the future too.

All we need to do is return some result to the caller to tell them whether we have repainted (empty renderable_list), and so whether they should swap buffers.

Admittedly DefaultDisplayBufferCompositor is only used in USC and probably not in Unity8 any more. But we could do a similar fix in QtMir to address Unity8.

Fixing the problem in the compositor is ideal, because that's the only place where you might know about the additional rendering that's outside the expected confines of the surface rectangle.

It's not just other shells that don't exist yet which need this, but Unity8 needs it right now. Consider Unity8 on desktop with two monitors -- It does external rendering like minimize animations, shadows, and the whole launcher, that surface_stack.cpp doesn't know about and will get wrong using your proposed implementation.

review: Needs Fixing

« Back to merge proposal