Mir

Merge lp:~alan-griffiths/mir/fix-1535894-alt into lp:mir

fix-1535894-alt
Merge into development-branch

Proposed by Alan Griffiths on 2016-02-05

Status:	Rejected
Rejected by:	Alan Griffiths on 2016-02-11
Proposed branch:	lp:~alan-griffiths/mir/fix-1535894-alt
Merge into:	lp:mir
Diff against target:	984 lines (+181/-263) 26 files modified examples/render_surfaces.cpp (+3/-2) examples/server_example_adorning_compositor.cpp (+12/-1) examples/server_example_adorning_compositor.h (+1/-1) include/server/mir/compositor/display_buffer_compositor.h (+1/-1) include/server/mir/compositor/scene.h (+0/-10) include/server/mir/compositor/scene_element.h (+1/-0) include/test/mir/test/doubles/null_display_buffer_compositor_factory.h (+2/-1) playground/demo-shell/demo_compositor.cpp (+13/-1) playground/demo-shell/demo_compositor.h (+1/-1) src/server/compositor/default_display_buffer_compositor.cpp (+43/-46) src/server/compositor/default_display_buffer_compositor.h (+1/-1) src/server/compositor/multi_threaded_compositor.cpp (+3/-11) src/server/scene/rendering_tracker.cpp (+9/-0) src/server/scene/rendering_tracker.h (+1/-0) src/server/scene/surface_stack.cpp (+10/-24) src/server/scene/surface_stack.h (+0/-1) tests/acceptance-tests/test_buffer_stream_arrangement.cpp (+2/-2) tests/acceptance-tests/test_client_scaling.cpp (+2/-1) tests/acceptance-tests/test_render_override.cpp (+3/-1) tests/include/mir/test/doubles/stub_scene.h (+0/-4) tests/include/mir/test/doubles/stub_scene_element.h (+5/-0) tests/integration-tests/test_server_shutdown.cpp (+1/-1) tests/unit-tests/compositor/test_compositing_screencast.cpp (+4/-3) tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp (+44/-10) tests/unit-tests/compositor/test_multi_threaded_compositor.cpp (+19/-22) tests/unit-tests/scene/test_surface_stack.cpp (+0/-118)
To merge this branch:	bzr merge lp:~alan-griffiths/mir/fix-1535894-alt
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2016-02-11
Mir CI Bot	continuous-integration		Approve on 2016-02-11
Daniel van Vugt		2016-02-05	Needs Fixing on 2016-02-11
Review via email: mp+285223@code.launchpad.net

Commit message

compositor, scene: Use the DisplayBufferCompositor filtering to decide which surface have buffers that may still need compositing.

Description of the change

compositor, scene: Use the DisplayBufferCompositor filtering to decide which surface have buffers that may still need compositing.

This doesn't include logic that allows -c 3274 to be reverted, but if this approach is favoured, then I'll rework that to put the DisplayBufferCompositor(s) in control.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-08:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/253/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-08:

Errors look unrelated:

15:52:50 Setting up libuuid1:amd64 (2.27.1-1ubuntu4) ...
15:52:50 dpkg: error processing package libuuid1:amd64 (--configure):
15:52:50 subprocess installed post-installation script returned error exit status 1
15:52:50 Processing triggers for libc-bin (2.21-0ubuntu5) ...
15:52:51 Errors were encountered while processing:
15:52:51 libuuid1:amd64
15:52:51 W: No sandbox user '_apt' on the system, can not drop privileges
15:52:51 E: Sub-process /usr/bin/dpkg returned an error code (1)
15:52:51 apt-get failed.
15:52:51 Package installation failed

15:52:55 dpkg: error processing package libuuid1:i386 (--configure):
15:52:55 subprocess installed post-installation script returned error exit status 1
15:52:55 Processing triggers for libc-bin (2.21-0ubuntu5) ...
15:52:55 Errors were encountered while processing:
15:52:55 libuuid1:i386
15:52:55 W: No sandbox user '_apt' on the system, can not drop privileges
15:52:55 E: Sub-process /usr/bin/dpkg returned an error code (1)
15:52:55 apt-get failed.
15:52:55 Package installation failed

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-08:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/254/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-08:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/255/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-08:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6231/rebuild

review: Approve (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-10:

1. Needs info: I thought the goal here was to skip rendering on idle displays. That code doesn't seem to be present yet, so maybe it comes later? It would be: composite() returns "I had nothing new to render" so then post() never gets called. You either need to composite and post, or do neither. But never composite without post (wasted of GPU), and never post without composite (uninitialized pixels appear on screen).

Although that raises an important question: Is the bug safe to fix at all under the new display groups architecture? How do we safely tell Android to only post one of the displays in the group but not the other? I'm wondering if it's even possible to fix this under the display group approach... because you must either have one composite and one post, or zero of all.

2. Needs fixing: Please add the FIXME comment back in. It's an important one we shouldn't forget.

I'm also nervous that we're rewriting so much code that covers years of regression fixes. But that's not quite a blocking issue.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-10:

3. This looks like important code that will cause regressions if removed:
- int result = scene_changed ? 1 : 0;

That tells us to redraw the screen if a window has been dragged, but no windows contents changed. Obviously was missing a test.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-10:

OK, assuming Android has some magic workaround built-in to cope with posting a display group where only half of that group has been rendered, what we actually want instead of this branch is:

bool composited = false
for each compositor:
composited |= compositor->composite();
if (composited)
group.post();

That also means you can keep the frame counting logic where it is in the scene. No need to duplicate it into each compositor implementation.

But if Android can't handle half the monitors being rendered and then being told to post the whole group, then we actually can't safely do any of this at all...

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-10:

> 1. Needs info: I thought the goal here was to skip rendering on idle displays.
> That code doesn't seem to be present yet, so maybe it comes later? It would
> be: composite() returns "I had nothing new to render" so then post() never
> gets called. You either need to composite and post, or do neither. But never
> composite without post (wasted of GPU), and never post without composite
> (uninitialized pixels appear on screen).

Nope.

The point of *this* MP is that the DisplayBufferCompositor knows which surfaces it has rendered and interrogates them to see if they have further buffers pending.

With the logic in scene we were counting buffers belonging to surfaces that had not contributed to rendering. (And would not be consumed by the consequent render pass.)

In the scenario from the linked bug the nested server would repeatedly schedule a recomposite of whichever display on which nothing had been posted.

> 2. Needs fixing: Please add the FIXME comment back in. It's an important one
> we shouldn't forget.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-10:

> 3. This looks like important code that will cause regressions if removed:
> - int result = scene_changed ? 1 : 0;
>
> That tells us to redraw the screen if a window has been dragged, but no
> windows contents changed. Obviously was missing a test.

Obviously, this code duplicated the effect of CompositingFunctor::schedule_compositing(int num) and was redundant.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-11:

Oh cool. That all sounds like good news then. I'll need to re-evaluate later.

review: Abstain

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-11:

(1) Thanks for explaining. So I took some time today adding and analysing some debug logging to try and catch Mir in the act of repeatedly scheduling frames on some display but never consuming them. Unfortunately I can't yet reproduce such a busy wait but that could be just that I'm using desktop, or it could just be bad luck. So I'm assuming there's still a theoretical problem needing fixing.

But now I'm curious: You've clarified you're just fixing a busy wait. Or as I would say it "made Mir sleep properly". That's very good for power management, but only a small piece of the performance story. Even if this branch is working perfectly, you're still re-posting all displays on every frame (including the idle one) while any one of them is animating. So this is really a power management fix, but how is it an improvement to performance? You haven't changed the rendering load, and haven't reduced the number of posts either.

(2) Needs fixing: FIXME comment is still missing.

(4) I'm not yet confident this branch is working. As stated in (1) I can't yet put together a manual test case on desktop to reproduce any issue, and the diff below shows no new regression tests for anything getting fixed. So I think you need to try and write a regression test for the issue.

(5) In theory there's a much simpler solution you should consider. That is just make every call to BasicSurface::generate_renderables acquire its compositor buffer immediately, instead of the lazy acquisition it does in SurfaceSnapshot::buffer(). In theory that's a tiny change, it would solve the same problem, and is easier to test.

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-02-11:

(1) "You haven't changed the rendering load, and haven't reduced the number of posts either" [while any display is changing anyway]

(5) Here's possible alternative solution:
https://code.launchpad.net/~mir-team/mir/compositing-always-finishes/+merge/285684

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-11:

> but only a small piece of the performance story. Even if this branch is
> working perfectly, you're still re-posting all displays on every frame
> (including the idle one) while any one of them is animating.

Remember, this is in the nested server which has a CompositingFunctor for each output.

The earlier change (that you have valid concerns about) ensures that the CompositingFunctor doesn't run for changes that do not apply to its display buffers compositor.

> So this is really
> a power management fix, but how is it an improvement to performance? You
> haven't changed the rendering load, and haven't reduced the number of posts
> either.

The problem this addresses occurs whenever the "idle" CompositingFunctor runs. First it composites the surfaces visible to its DBC and then checks the scene for unconsumed frames. Updates on the other screen cause it to increment frames_scheduled and composite unnecessarily.

By only checking for unconsumed frames in the surfaces that have been rendered we avoid that extra pass.

> (2) Needs fixing: FIXME comment is still missing.
>
> (4) I'm not yet confident this branch is working. As stated in (1) I can't yet
> put together a manual test case on desktop to reproduce any issue, and the
> diff below shows no new regression tests for anything getting fixed. So I
> think you need to try and write a regression test for the issue.

Ack.

lp:~alan-griffiths/mir/fix-1535894-alt updated on 2016-02-11

3301. By Alan Griffiths on 2016-02-11: merge lp:mir
3302. By Alan Griffiths on 2016-02-11: Reinstate FIXME comment
3303. By Alan Griffiths on 2016-02-11: Remove some unused headers

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-02-11:

nm. -c 3301 seems to have addressed the problem I was seeing. (Not sure how, so I must have misinterpreted the system behaviour - we need better reporting out of the compositor.)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-11:

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/285/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/6271/rebuild

review: Approve (continuous-integration)

Unmerged revisions

3303. By Alan Griffiths on 2016-02-11: Remove some unused headers
3302. By Alan Griffiths on 2016-02-11: Reinstate FIXME comment
3301. By Alan Griffiths on 2016-02-11: merge lp:mir
3300. By Alan Griffiths on 2016-02-08: Cleaner code in examples
3299. By Alan Griffiths on 2016-02-08: Cleaner code (that also passes StaleFrames.* tests)
3298. By Alan Griffiths on 2016-02-05: Add corresponding DefaultDisplayBufferCompositor tests
3297. By Alan Griffiths on 2016-02-05: Remove unnecessary Scene::frames_pending()
3296. By Alan Griffiths on 2016-02-05: Fixup broken test
3295. By Alan Griffiths on 2016-02-05: First pass at pending buffer count

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Griffiths

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'examples/render_surfaces.cpp'
 --- examples/render_surfaces.cpp	2016-01-29 08:18:22 +0000
 +++ examples/render_surfaces.cpp	2016-02-11 14:50:57 +0000
@@ -241,7 +241,7 @@
+     {
+     }
--    void composite(mc::SceneElementSequence&& scene_sequence) override
++    int composite(mc::SceneElementSequence&& scene_sequence) override
+     {
          while (!created) std::this_thread::yield();
          stop_watch.stop();
@@ -252,12 +252,13 @@
              stop_watch.restart();
+         }
--        db_compositor->composite(std::move(scene_sequence));
++        auto const result = db_compositor->composite(std::move(scene_sequence));
          for (auto& m : moveables)
              m.step();
          frames++;
++        return result;
+     }
  private:
 === modified file 'examples/server_example_adorning_compositor.cpp'
 --- examples/server_example_adorning_compositor.cpp	2016-02-08 17:58:09 +0000
 +++ examples/server_example_adorning_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -174,7 +174,7 @@
+ }
+ }
--void me::AdorningDisplayBufferCompositor::composite(compositor::SceneElementSequence&& scene_sequence)
++int me::AdorningDisplayBufferCompositor::composite(compositor::SceneElementSequence&& scene_sequence)
+ {
      report->began_frame(this);
@@ -250,4 +250,15 @@
      render_target->swap_buffers();
      report->finished_frame(this);
++
++    auto max_pending_buffers = 0;
++    for (auto const& element : scene_sequence)
++    {
++        auto buffers_for_element = element->pending_buffers();
++
++        if (max_pending_buffers < buffers_for_element)
++            max_pending_buffers = buffers_for_element;
++    }
++
++    return max_pending_buffers;
+ }
 === modified file 'examples/server_example_adorning_compositor.h'
 --- examples/server_example_adorning_compositor.h	2016-01-29 08:18:22 +0000
 +++ examples/server_example_adorning_compositor.h	2016-02-11 14:50:57 +0000
@@ -40,7 +40,7 @@
          graphics::DisplayBuffer&, std::tuple<float, float, float> const& background_rgb,
          std::shared_ptr<compositor::CompositorReport> const& report);
--    void composite(compositor::SceneElementSequence&& scene_sequence) override;
++    int composite(compositor::SceneElementSequence&& scene_sequence) override;
  private:
      graphics::DisplayBuffer& db;
      renderer::gl::RenderTarget* const render_target;
 === modified file 'include/server/mir/compositor/display_buffer_compositor.h'
 --- include/server/mir/compositor/display_buffer_compositor.h	2015-02-22 07:46:25 +0000
 +++ include/server/mir/compositor/display_buffer_compositor.h	2016-02-11 14:50:57 +0000
@@ -32,7 +32,7 @@
  public:
      virtual ~DisplayBufferCompositor() = default;
--    virtual void composite(SceneElementSequence&& scene_sequence) = 0;
++    virtual int composite(SceneElementSequence&& scene_sequence) = 0;
  protected:
      DisplayBufferCompositor() = default;
 === modified file 'include/server/mir/compositor/scene.h'
 --- include/server/mir/compositor/scene.h	2015-02-22 07:46:25 +0000
 +++ include/server/mir/compositor/scene.h	2016-02-11 14:50:57 +0000
@@ -55,16 +55,6 @@
       */
      virtual SceneElementSequence scene_elements_for(CompositorID id) = 0;
--    /**
--     * Return the number of additional frames that you need to render to get
--     * fully up to date with the latest data in the scene. For a generic
--     * "scene change" this will be just 1. For surfaces that have multiple
--     * frames queued up however, it could be greater than 1. When the result
--     * reaches zero, you know you have consumed all the latest data from the
--     * scene.
--     */
--    virtual int frames_pending(CompositorID id) const = 0;
--
      virtual void register_compositor(CompositorID id) = 0;
      virtual void unregister_compositor(CompositorID id) = 0;
 === modified file 'include/server/mir/compositor/scene_element.h'
 --- include/server/mir/compositor/scene_element.h	2015-06-17 05:20:42 +0000
 +++ include/server/mir/compositor/scene_element.h	2016-02-11 14:50:57 +0000
@@ -38,6 +38,7 @@
      virtual std::shared_ptr<graphics::Renderable> renderable() const = 0;
      virtual void rendered() = 0;
      virtual void occluded() = 0;
++    virtual int pending_buffers() const = 0;
      //TODO: Decoration is opaque on purpose. It is only used by an internal example,
      //      and this function should be removed from the public API.
 === modified file 'include/test/mir/test/doubles/null_display_buffer_compositor_factory.h'
 --- include/test/mir/test/doubles/null_display_buffer_compositor_factory.h	2016-01-29 08:18:22 +0000
 +++ include/test/mir/test/doubles/null_display_buffer_compositor_factory.h	2016-02-11 14:50:57 +0000
@@ -39,11 +39,12 @@
+     {
          struct NullDisplayBufferCompositor : compositor::DisplayBufferCompositor
+         {
--            void composite(compositor::SceneElementSequence&&)
++            int composite(compositor::SceneElementSequence&&)
+             {
                  // yield() is needed to ensure reasonable runtime under
                  // valgrind for some tests
                  std::this_thread::yield();
++                return 0;
+             }
          };
 === modified file 'playground/demo-shell/demo_compositor.cpp'
 --- playground/demo-shell/demo_compositor.cpp	2016-01-29 08:18:22 +0000
 +++ playground/demo-shell/demo_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -58,7 +58,7 @@
          f(*i);
+ }
--void me::DemoCompositor::composite(mc::SceneElementSequence&& elements)
++int me::DemoCompositor::composite(mc::SceneElementSequence&& elements)
+ {
      report->began_frame(this);
      //a simple filtering out of renderables that shouldn't be drawn
@@ -106,6 +106,16 @@
          nonrenderlist_elements |= embellished;
+     }
++    auto max_pending_buffers = 0;
++    for (auto const& element : elements)
++    {
++        // We're about to render a buffer, so don't count that one
++        auto buffers_for_element = element->pending_buffers() - 1;
++
++        if (max_pending_buffers < buffers_for_element)
++            max_pending_buffers = buffers_for_element;
++    }
++
      /*
       * Note: Buffer lifetimes are ensured by the two objects holding
       *       references to them; elements and renderable_list.
@@ -141,6 +151,8 @@
+     }
      report->finished_frame(this);
++
++    return max_pending_buffers;
+ }
  void me::DemoCompositor::on_cursor_movement(
 === modified file 'playground/demo-shell/demo_compositor.h'
 --- playground/demo-shell/demo_compositor.h	2015-02-22 07:46:25 +0000
 +++ playground/demo-shell/demo_compositor.h	2016-02-11 14:50:57 +0000
@@ -50,7 +50,7 @@
          std::shared_ptr<compositor::CompositorReport> const& report);
      ~DemoCompositor();
--    void composite(compositor::SceneElementSequence&& elements) override;
++    int composite(compositor::SceneElementSequence&& elements) override;
      void zoom(float mag);
      void on_cursor_movement(geometry::Point const& p);
 === modified file 'src/server/compositor/default_display_buffer_compositor.cpp'
 --- src/server/compositor/default_display_buffer_compositor.cpp	2016-01-29 08:18:22 +0000
 +++ src/server/compositor/default_display_buffer_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright © 2012 Canonical Ltd.
++ * Copyright © 2012-2016 Canonical Ltd.
+  *
   * This program is free software: you can redistribute it and/or modify it
   * under the terms of the GNU General Public License version 3,
@@ -19,16 +19,13 @@
  #include "default_display_buffer_compositor.h"
--#include "mir/compositor/scene.h"
  #include "mir/compositor/scene_element.h"
  #include "mir/compositor/renderer.h"
  #include "mir/graphics/renderable.h"
  #include "mir/graphics/display_buffer.h"
  #include "mir/graphics/buffer.h"
--#include "mir/compositor/buffer_stream.h"
  #include "occlusion.h"
--#include <mutex>
--#include <cstdlib>
++
  #include <algorithm>
  namespace mc = mir::compositor;
@@ -44,52 +41,52 @@
+ {
+ }
--void mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& scene_elements)
++int mc::DefaultDisplayBufferCompositor::composite(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);
--
--    for (auto const& element : occlusions)
++    for (auto const& element : filter_occlusions_from(scene_elements, display_buffer.view_area()))
          element->occluded();
--    mg::RenderableList renderable_list;
--    renderable_list.reserve(scene_elements.size());
++    {
++        mg::RenderableList renderable_list;
++        renderable_list.reserve(scene_elements.size());
++
++        transform(begin(scene_elements), end(scene_elements), back_inserter(renderable_list), [] (auto& element)
++            {
++                element->rendered();
++                return element->renderable();
++            });
++
++        if (display_buffer.post_renderables_if_optimizable(renderable_list))
++        {
++            report->renderables_in_frame(this, renderable_list);
++            renderer->suspend();
++        }
++        else
++        {
++            renderer->set_rotation(display_buffer.orientation());
++            renderer->render(renderable_list);
++
++            report->renderables_in_frame(this, renderable_list);
++            report->rendered_frame(this);
++        }
++    }
++
++    report->finished_frame(this);
++
++    auto max_pending_buffers = 0;
      for (auto const& element : scene_elements)
+     {
--        element->rendered();
--        renderable_list.push_back(element->renderable());
--    }
--
--    /*
--     * Note: Buffer lifetimes are ensured by the two objects holding
--     *       references to them; scene_elements and renderable_list.
--     *       So no buffer is going to be released back to the client till
--     *       both of those containers get destroyed (end of the function).
--     *       Actually, there's a third reference held by the texture cache
--     *       in GLRenderer, but that gets released earlier in render().
--     */
--    scene_elements.clear();  // Those in use are still in renderable_list
--
--    if (display_buffer.post_renderables_if_optimizable(renderable_list))
--    {
--        report->renderables_in_frame(this, renderable_list);
--        renderer->suspend();
--    }
--    else
--    {
--        renderer->set_rotation(display_buffer.orientation());
--        renderer->render(renderable_list);
--
--        report->renderables_in_frame(this, renderable_list);
--        report->rendered_frame(this);
--
--        // Release the buffers we did use back to the clients, before starting
--        // on the potentially slow flip().
--        // FIXME: This clear() call is blocking a little because we drive IPC here (LP: #1395421)
--        renderable_list.clear();
--    }
--
--    report->finished_frame(this);
++        auto buffers_for_element = element->pending_buffers();
++
++        if (max_pending_buffers < buffers_for_element)
++            max_pending_buffers = buffers_for_element;
++    }
++
++    return max_pending_buffers;
++
++    // Returning destroys the local renderable_list and the temporary scene_elements which releases
++    // the buffers we did use back to the clients, before starting on the potentially slow flip().
++    // FIXME: releasing the buffers is blocking a little because we drive IPC here (LP: #1395421)
+ }
 === modified file 'src/server/compositor/default_display_buffer_compositor.h'
 --- src/server/compositor/default_display_buffer_compositor.h	2015-04-28 07:54:10 +0000
 +++ src/server/compositor/default_display_buffer_compositor.h	2016-02-11 14:50:57 +0000
@@ -43,7 +43,7 @@
          std::shared_ptr<Renderer> const& renderer,
          std::shared_ptr<CompositorReport> const& report);
--    void composite(SceneElementSequence&& scene_sequence) override;
++    int composite(SceneElementSequence&& scene_sequence) override;
  private:
      graphics::DisplayBuffer& display_buffer;
 === modified file 'src/server/compositor/multi_threaded_compositor.cpp'
 --- src/server/compositor/multi_threaded_compositor.cpp	2016-02-09 16:40:57 +0000
 +++ src/server/compositor/multi_threaded_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -131,14 +131,15 @@
                       * frames_scheduled indicates the number of frames that are scheduled
                       * to ensure all surfaces' queues are fully drained.
                       */
--                    frames_scheduled--;
++                    int pending = --frames_scheduled;
                      not_posted_yet = false;
                      lock.unlock();
                      for (auto& tuple : compositors)
+                     {
                          auto& compositor = std::get<1>(tuple);
--                        compositor->composite(scene->scene_elements_for(compositor.get()));
++                        auto max_pending_frames = compositor->composite(scene->scene_elements_for(compositor.get()));
++                        if (pending < max_pending_frames) pending = max_pending_frames;
+                     }
                      group.post();
@@ -161,15 +162,6 @@
                       * important to re-count number of frames pending, separately
                       * to the initial scene_elements_for()...
                       */
--                    int pending = 0;
--                    for (auto& compositor : compositors)
--                    {
--                        auto const comp_id = std::get<1>(compositor).get();
--                        int pend = scene->frames_pending(comp_id);
--                        if (pend > pending)
--                            pending = pend;
--                    }
--
                      if (pending > frames_scheduled)
                          frames_scheduled = pending;
+                 }
 === modified file 'src/server/scene/rendering_tracker.cpp'
 --- src/server/scene/rendering_tracker.cpp	2016-01-29 08:18:22 +0000
 +++ src/server/scene/rendering_tracker.cpp	2016-02-11 14:50:57 +0000
@@ -80,6 +80,15 @@
      return occlusions.find(cid) == occlusions.end();
+ }
++int ms::RenderingTracker::pending_buffers_for(compositor::CompositorID cid) const
++{
++    if (auto const surface = weak_surface.lock())
++        return surface->buffers_ready_for_compositor(cid);
++    else
++        return 0;
++}
++
++
  bool ms::RenderingTracker::occluded_in_all_active_compositors()
+ {
      return occlusions == active_compositors_;
 === modified file 'src/server/scene/rendering_tracker.h'
 --- src/server/scene/rendering_tracker.h	2016-01-29 08:18:22 +0000
 +++ src/server/scene/rendering_tracker.h	2016-02-11 14:50:57 +0000
@@ -43,6 +43,7 @@
      void occluded_in(compositor::CompositorID cid);
      void active_compositors(std::set<compositor::CompositorID> const& cids);
      bool is_exposed_in(compositor::CompositorID cid) const;
++    int pending_buffers_for(compositor::CompositorID cid) const;
  private:
      bool occluded_in_all_active_compositors();
 === modified file 'src/server/scene/surface_stack.cpp'
 --- src/server/scene/surface_stack.cpp	2016-01-29 08:18:22 +0000
 +++ src/server/scene/surface_stack.cpp	2016-02-11 14:50:57 +0000
@@ -73,6 +73,11 @@
          tracker->occluded_in(cid);
+     }
++    int pending_buffers() const override
++    {
++        return tracker->pending_buffers_for(cid);
++    }
++
      std::unique_ptr<mc::Decoration> decoration() const override
+     {
          return std::make_unique<mc::Decoration>(mc::Decoration::Type::surface, surface_name);
@@ -108,6 +113,11 @@
+     {
+     }
++    int pending_buffers() const override
++    {
++        return 0;
++    }
++
      std::unique_ptr<mc::Decoration> decoration() const override
+     {
          return std::make_unique<mc::Decoration>();
@@ -154,30 +164,6 @@
      return elements;
+ }
--int ms::SurfaceStack::frames_pending(mc::CompositorID id) const
--{
--    RecursiveReadLock lg(guard);
--
--    int result = scene_changed ? 1 : 0;
--    for (auto const& surface : surfaces)
--    {
--        if (surface->visible())
--        {
--            auto const tracker = rendering_trackers.find(surface.get());
--            if (tracker != rendering_trackers.end() && tracker->second->is_exposed_in(id))
--            {
--                // Note that we ask the surface and not a Renderable.
--                // This is because we don't want to waste time and resources
--                // on a snapshot till we're sure we need it...
--                int ready = surface->buffers_ready_for_compositor(id);
--                if (ready > result)
--                    result = ready;
--            }
--        }
--    }
--    return result;
--}
--
  void ms::SurfaceStack::register_compositor(mc::CompositorID cid)
+ {
      RecursiveWriteLock lg(guard);
 === modified file 'src/server/scene/surface_stack.h'
 --- src/server/scene/surface_stack.h	2016-01-29 08:18:22 +0000
 +++ src/server/scene/surface_stack.h	2016-02-11 14:50:57 +0000
@@ -74,7 +74,6 @@
      // From Scene
      compositor::SceneElementSequence scene_elements_for(compositor::CompositorID id) override;
--    int frames_pending(compositor::CompositorID) const override;
      void register_compositor(compositor::CompositorID id) override;
      void unregister_compositor(compositor::CompositorID id) override;
 === modified file 'tests/acceptance-tests/test_buffer_stream_arrangement.cpp'
 --- tests/acceptance-tests/test_buffer_stream_arrangement.cpp	2016-01-29 08:18:22 +0000
 +++ tests/acceptance-tests/test_buffer_stream_arrangement.cpp	2016-02-11 14:50:57 +0000
@@ -167,10 +167,10 @@
+     {
+     }
--    void composite(mc::SceneElementSequence&& scene_sequence) override
++    int composite(mc::SceneElementSequence&& scene_sequence) override
+     {
          ordering->note_scene_element_sequence(scene_sequence);
--        wrapped->composite(std::move(scene_sequence));
++        return wrapped->composite(std::move(scene_sequence));
+     }
      std::shared_ptr<mc::DisplayBufferCompositor> const wrapped;
 === modified file 'tests/acceptance-tests/test_client_scaling.cpp'
 --- tests/acceptance-tests/test_client_scaling.cpp	2016-01-29 08:18:22 +0000
 +++ tests/acceptance-tests/test_client_scaling.cpp	2016-02-11 14:50:57 +0000
@@ -120,10 +120,11 @@
+         {
+         }
--        void composite(mc::SceneElementSequence&& seq)
++        int composite(mc::SceneElementSequence&& seq)
+         {
              watch->note_renderable_sizes(this, seq);
              std::this_thread::yield();
++            return 0;
+         }
          std::shared_ptr<SizeWatcher> const watch;
 === modified file 'tests/acceptance-tests/test_render_override.cpp'
 --- tests/acceptance-tests/test_render_override.cpp	2015-06-17 05:20:42 +0000
 +++ tests/acceptance-tests/test_render_override.cpp	2016-02-11 14:50:57 +0000
@@ -74,7 +74,7 @@
+     {
+     }
--    void composite(mc::SceneElementSequence&& sequence) override
++    int composite(mc::SceneElementSequence&& sequence) override
+     {
          for(auto const& element : sequence)
+         {
@@ -82,6 +82,8 @@
              renderable->buffer(); //consume buffer to stop compositor from spinning
              tracker->surface_rendered_with_size(renderable->screen_position().size);
+         }
++
++        return 0;
+     }
  private:
      std::shared_ptr<CompositionTracker> const tracker;
 === modified file 'tests/include/mir/test/doubles/stub_scene.h'
 --- tests/include/mir/test/doubles/stub_scene.h	2015-02-22 07:46:25 +0000
 +++ tests/include/mir/test/doubles/stub_scene.h	2016-02-11 14:50:57 +0000
@@ -36,10 +36,6 @@
+     {
          return {};
+     }
--    int frames_pending(compositor::CompositorID) const override
--    {
--        return 0;
--    }
      void register_compositor(compositor::CompositorID) override
+     {
+     }
 === modified file 'tests/include/mir/test/doubles/stub_scene_element.h'
 --- tests/include/mir/test/doubles/stub_scene_element.h	2015-06-17 05:20:42 +0000
 +++ tests/include/mir/test/doubles/stub_scene_element.h	2016-02-11 14:50:57 +0000
@@ -56,6 +56,11 @@
+     {
+     }
++    int pending_buffers() const override
++    {
++        return 0;
++    }
++
      std::unique_ptr<compositor::Decoration> decoration() const override
+     {
          return nullptr;
 === modified file 'tests/integration-tests/test_server_shutdown.cpp'
 --- tests/integration-tests/test_server_shutdown.cpp	2016-01-29 08:18:22 +0000
 +++ tests/integration-tests/test_server_shutdown.cpp	2016-02-11 14:50:57 +0000
@@ -189,7 +189,7 @@
+     {
          struct ExceptionThrowingDisplayBufferCompositor : mc::DisplayBufferCompositor
+         {
--            void composite(mc::SceneElementSequence&&) override
++            int composite(mc::SceneElementSequence&&) override
+             {
                  throw std::runtime_error("ExceptionThrowingDisplayBufferCompositor");
+             }
 === modified file 'tests/unit-tests/compositor/test_compositing_screencast.cpp'
 --- tests/unit-tests/compositor/test_compositing_screencast.cpp	2016-02-01 22:53:06 +0000
 +++ tests/unit-tests/compositor/test_compositing_screencast.cpp	2016-02-11 14:50:57 +0000
@@ -118,9 +118,10 @@
  struct MockDisplayBufferCompositor : mc::DisplayBufferCompositor
+ {
--    void composite(mc::SceneElementSequence&& seq)
++    int composite(mc::SceneElementSequence&& seq)
+     {
          composite_(seq);
++        return 0;
+     }
      MOCK_METHOD1(composite_, void(mc::SceneElementSequence&));
@@ -134,9 +135,9 @@
+     {
+     }
--    void composite(mc::SceneElementSequence&& elements)
++    int composite(mc::SceneElementSequence&& elements)
+     {
--        comp.composite(std::move(elements));
++        return comp.composite(std::move(elements));
+     }
  private:
 === modified file 'tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp'
 --- tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp	2016-01-29 08:18:22 +0000
 +++ tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -48,6 +48,7 @@
  namespace mt = mir::test;
  namespace mtd = mir::test::doubles;
++using namespace testing;
  namespace
+ {
@@ -72,6 +73,11 @@
+     {
+     }
++    int pending_buffers() const override
++    {
++        return 0;
++    }
++
      std::unique_ptr<mc::Decoration> decoration() const override
+     {
          return nullptr;
@@ -97,7 +103,6 @@
         big(std::make_shared<mtd::FakeRenderable>(geom::Rectangle{{5, 10},{100, 200}})),
         fullscreen(std::make_shared<mtd::FakeRenderable>(screen))
+     {
--        using namespace testing;
          ON_CALL(display_buffer, orientation())
              .WillByDefault(Return(mir_orientation_normal));
          ON_CALL(display_buffer, view_area())
@@ -106,7 +111,7 @@
              .WillByDefault(Return(false));
+     }
--    testing::NiceMock<mtd::MockRenderer> mock_renderer;
++    NiceMock<mtd::MockRenderer> mock_renderer;
      geom::Rectangle screen{{0, 0}, {1366, 768}};
      testing::NiceMock<mtd::MockDisplayBuffer> display_buffer;
      std::shared_ptr<mtd::FakeRenderable> small;
@@ -132,7 +137,6 @@
  TEST_F(DefaultDisplayBufferCompositor, optimization_skips_composition)
+ {
--    using namespace testing;
      auto report = std::make_shared<mtd::MockCompositorReport>();
      Sequence seq;
@@ -190,7 +194,6 @@
  TEST_F(DefaultDisplayBufferCompositor, calls_renderer_in_sequence)
+ {
--    using namespace testing;
      Sequence render_seq;
      EXPECT_CALL(mock_renderer, suspend())
          .Times(0);
@@ -215,7 +218,6 @@
  TEST_F(DefaultDisplayBufferCompositor, optimization_toggles_seamlessly)
+ {
--    using namespace testing;
      ON_CALL(display_buffer, view_area())
          .WillByDefault(Return(screen));
      ON_CALL(display_buffer, orientation())
@@ -262,7 +264,6 @@
  TEST_F(DefaultDisplayBufferCompositor, occluded_surfaces_are_not_rendered)
+ {
--    using namespace testing;
      EXPECT_CALL(display_buffer, view_area())
          .WillRepeatedly(Return(screen));
      EXPECT_CALL(display_buffer, orientation())
@@ -305,13 +306,12 @@
      MOCK_CONST_METHOD0(decoration, std::unique_ptr<mc::Decoration>());
      MOCK_METHOD0(rendered, void());
      MOCK_METHOD0(occluded, void());
++    MOCK_CONST_METHOD0(pending_buffers, int());
  };
+ }
  TEST_F(DefaultDisplayBufferCompositor, marks_rendered_scene_elements)
+ {
--    using namespace testing;
--
      auto element0_rendered = std::make_shared<NiceMock<MockSceneElement>>(
          std::make_shared<mtd::FakeRenderable>(geom::Rectangle{{99,99},{2,2}}));
      auto element1_rendered = std::make_shared<NiceMock<MockSceneElement>>(
@@ -330,8 +330,6 @@
  TEST_F(DefaultDisplayBufferCompositor, marks_occluded_scene_elements)
+ {
--    using namespace testing;
--
      auto element0_occluded = std::make_shared<NiceMock<MockSceneElement>>(
          std::make_shared<mtd::FakeRenderable>(geom::Rectangle{{10,10},{20,20}}));
      auto element1_rendered = std::make_shared<NiceMock<MockSceneElement>>(
@@ -351,3 +349,39 @@
      compositor.composite({element0_occluded, element1_rendered, element2_occluded});
+ }
++TEST_F(DefaultDisplayBufferCompositor, reports_pending_frames_accurately)
++{
++    auto const element = std::make_shared<NiceMock<MockSceneElement>>(
++        std::make_shared<mtd::FakeRenderable>(geom::Rectangle{{10,10},{20,20}}));
++
++    mc::DefaultDisplayBufferCompositor compositor(
++        display_buffer,
++        mt::fake_shared(mock_renderer),
++        mr::null_compositor_report());
++
++    EXPECT_THAT(compositor.composite({element}), Eq(0));
++
++    auto const pending_frames_before = 3;
++    auto const pending_frames_after = pending_frames_before-1;
++
++    EXPECT_CALL(*element, pending_buffers()).WillOnce(Return(pending_frames_after));
++    EXPECT_THAT(compositor.composite({element}), Eq(pending_frames_after));
++}
++
++TEST_F(DefaultDisplayBufferCompositor, doesnt_count_pending_frames_from_occluded_surfaces)
++{
++    auto const element = std::make_shared<NiceMock<MockSceneElement>>(
++        std::make_shared<mtd::FakeRenderable>(geom::Rectangle{screen.bottom_right(),{20,20}}));
++
++    mc::DefaultDisplayBufferCompositor compositor(
++        display_buffer,
++        mt::fake_shared(mock_renderer),
++        mr::null_compositor_report());
++
++    EXPECT_THAT(compositor.composite({element}), Eq(0));
++
++    auto const pending_frames = 3;
++    ON_CALL(*element, pending_buffers()).WillByDefault(Return(pending_frames));
++
++    EXPECT_THAT(compositor.composite({element}), Eq(0));
++}
 === modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
 --- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2016-02-01 09:42:10 +0000
 +++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2016-02-11 14:50:57 +0000
@@ -96,8 +96,8 @@
  class StubScene : public mtd::StubScene
+ {
  public:
--    StubScene()
--        : pending{0}, throw_on_add_observer_{false}
++    StubScene() :
++        throw_on_add_observer_{false}
+     {
+     }
@@ -136,19 +136,7 @@
          throw_on_add_observer_ = flag;
+     }
--    int frames_pending(mc::CompositorID) const override
--    {
--        return pending;
--    }
--
--    void set_pending(int n)
--    {
--        pending = n;
--        observer->scene_changed();
--    }
--
  private:
--    std::atomic<int> pending;
      std::mutex observer_mutex;
      std::shared_ptr<ms::Observer> observer;
      bool throw_on_add_observer_;
@@ -157,20 +145,21 @@
  class RecordingDisplayBufferCompositor : public mc::DisplayBufferCompositor
+ {
  public:
--    RecordingDisplayBufferCompositor(std::function<void()> const& mark_render_buffer)
++    RecordingDisplayBufferCompositor(std::function<int()> const& mark_render_buffer)
          : mark_render_buffer{mark_render_buffer}
+     {
+     }
--    void composite(mc::SceneElementSequence&&)
++    int composite(mc::SceneElementSequence&&)
+     {
--        mark_render_buffer();
++        auto const result = mark_render_buffer();
          /* Reduce run-time under valgrind */
          std::this_thread::yield();
++        return result;
+     }
  private:
--    std::function<void()> const mark_render_buffer;
++    std::function<int()> const mark_render_buffer;
  };
@@ -182,12 +171,12 @@
          auto raw = new RecordingDisplayBufferCompositor{
              [&display_buffer,this]()
+             {
--                mark_render_buffer(display_buffer);
++                return mark_render_buffer(display_buffer);
              }};
          return std::unique_ptr<RecordingDisplayBufferCompositor>(raw);
+     }
--    void mark_render_buffer(mg::DisplayBuffer& display_buffer)
++    int mark_render_buffer(mg::DisplayBuffer& display_buffer)
+     {
          std::lock_guard<std::mutex> lk{m};
@@ -196,6 +185,8 @@
          ++records[&display_buffer].first;
          records[&display_buffer].second.insert(std::this_thread::get_id());
++
++        return pending;
+     }
      bool enough_records_gathered(unsigned int nbuffers, unsigned int min_record_count = 1000)
@@ -264,7 +255,10 @@
          return true;
+     }
++    void set_pending(int n) { pending = n; }
++
  private:
++    std::atomic<int> pending{0};
      std::mutex m;
      typedef std::pair<unsigned int, std::unordered_set<std::thread::id>> Record;
      std::unordered_map<mg::DisplayBuffer*,Record> records;
@@ -278,11 +272,12 @@
+     {
+     }
--    void composite(mc::SceneElementSequence&&) override
++    int composite(mc::SceneElementSequence&&) override
+     {
          fake_surface_update();
          /* Reduce run-time under valgrind */
          std::this_thread::yield();
++        return 0;
+     }
  private:
@@ -331,6 +326,7 @@
+             {
                  std::lock_guard<std::mutex> lock{thread_names_mutex};
                  thread_names.emplace_back(mt::current_thread_name());
++                return 0;
              }};
          return std::unique_ptr<RecordingDisplayBufferCompositor>(raw);
+     }
@@ -595,7 +591,8 @@
      ASSERT_LT(retry, max_retries);
      // Some surface queues up multiple frames
--    scene->set_pending(3);
++    factory->set_pending(3);
++    scene->emit_change_event();
      // Make sure they all get composited separately (2 + 3 more)
      retry = 0;
 === modified file 'tests/unit-tests/scene/test_surface_stack.cpp'
 --- tests/unit-tests/scene/test_surface_stack.cpp	2016-01-29 08:18:22 +0000
 +++ tests/unit-tests/scene/test_surface_stack.cpp	2016-02-11 14:50:57 +0000
@@ -335,124 +335,6 @@
      EXPECT_EQ("username@hostname: ~/Documents", decor->name);
+ }
--TEST_F(SurfaceStack, scene_counts_pending_accurately)
--{
--    using namespace testing;
--    ms::SurfaceStack stack{report};
--    stack.register_compositor(this);
--    mtd::StubBuffer stub_buffer;
--    int ready = 0;
--    auto mock_queue = std::make_shared<testing::NiceMock<mtd::MockBufferBundle>>();
--    ON_CALL(*mock_queue, buffers_ready_for_compositor(_))
--        .WillByDefault(InvokeWithoutArgs([&]{return ready;}));
--    ON_CALL(*mock_queue, client_release(_))
--        .WillByDefault(InvokeWithoutArgs([&]{ready++;}));
--    ON_CALL(*mock_queue, compositor_acquire(_))
--        .WillByDefault(InvokeWithoutArgs([&]{ready--; return mt::fake_shared(stub_buffer); }));
--
--    auto surface = std::make_shared<ms::BasicSurface>(
--        std::string("stub"),
--        geom::Rectangle{{},{}},
--        false,
--        std::make_shared<mc::BufferStreamSurfaces>(mock_queue),
--        std::shared_ptr<mir::input::InputChannel>(),
--        std::shared_ptr<mir::input::InputSender>(),
--        std::shared_ptr<mg::CursorImage>(),
--        report);
--    stack.add_surface(surface, default_params.input_mode);
--    surface->configure(mir_surface_attrib_visibility,
--                       mir_surface_visibility_exposed);
--
--    EXPECT_EQ(0, stack.frames_pending(this));
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--    EXPECT_EQ(3, stack.frames_pending(this));
--
--    for (int expect = 3; expect >= 0; --expect)
--    {
--        ASSERT_EQ(expect, stack.frames_pending(this));
--        auto snap = stack.scene_elements_for(compositor_id);
--        for (auto& element : snap)
--        {
--            auto consumed = element->renderable()->buffer();
--        }
--    }
--}
--
--TEST_F(SurfaceStack, scene_doesnt_count_pending_frames_from_occluded_surfaces)
--{  // Regression test for LP: #1418081
--    using namespace testing;
--
--    ms::SurfaceStack stack{report};
--    stack.register_compositor(this);
--    auto surface = std::make_shared<ms::BasicSurface>(
--        std::string("stub"),
--        geom::Rectangle{{},{}},
--        false,
--        std::make_shared<mtd::StubBufferStream>(),
--        std::shared_ptr<mir::input::InputChannel>(),
--        std::shared_ptr<mir::input::InputSender>(),
--        std::shared_ptr<mg::CursorImage>(),
--        report);
--
--    stack.add_surface(surface, default_params.input_mode);
--    auto elements = stack.scene_elements_for(this);
--    for (auto const& elem : elements)
--        elem->occluded();
--
--    EXPECT_EQ(0, stack.frames_pending(this));
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--    EXPECT_EQ(0, stack.frames_pending(this));
--}
--
--TEST_F(SurfaceStack, scene_doesnt_count_pending_frames_from_partially_exposed_surfaces)
--{  // Regression test for LP: #1499039
--    using namespace testing;
--
--    // Partially exposed means occluded in one compositor but not another
--    ms::SurfaceStack stack{report};
--    auto const comp1 = reinterpret_cast<mc::CompositorID>(0);
--    auto const comp2 = reinterpret_cast<mc::CompositorID>(1);
--
--    stack.register_compositor(comp1);
--    stack.register_compositor(comp2);
--    auto surface = std::make_shared<ms::BasicSurface>(
--        std::string("stub"),
--        geom::Rectangle{{},{}},
--        false,
--        std::make_shared<mtd::StubBufferStream>(),
--        std::shared_ptr<mir::input::InputChannel>(),
--        std::shared_ptr<mir::input::InputSender>(),
--        std::shared_ptr<mg::CursorImage>(),
--        report);
--
--    stack.add_surface(surface, default_params.input_mode);
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--    post_a_frame(*surface);
--
--    EXPECT_EQ(3, stack.frames_pending(comp1));
--    EXPECT_EQ(3, stack.frames_pending(comp2));
--
--    auto elements = stack.scene_elements_for(comp1);
--    for (auto const& elem : elements)
--    {
--        elem->rendered();
--    }
--
--    elements = stack.scene_elements_for(comp2);
--    for (auto const& elem : elements)
--    {
--        elem->occluded();
--    }
--
--    EXPECT_EQ(3, stack.frames_pending(comp1));
--    EXPECT_EQ(0, stack.frames_pending(comp2));
--}
--
  TEST_F(SurfaceStack, surfaces_are_emitted_by_layer)
+ {
      using namespace testing;