bugfix: BasicScreenShooter now selects a GLRenderingProvider based of…

…f of its compatibility with the BufferAllocator (#3456)

fixes: #3431

src/server/compositor/basic_screen_shooter.cpp

@@ -217,26 +217,37 @@ auto mc::BasicScreenShooter::Self::renderer_for_buffer(std::shared_ptr<mrs::Writ
 }
 
 auto mc::BasicScreenShooter::select_provider(
-    std::span<std::shared_ptr<mg::GLRenderingProvider>> const& providers)
+    std::span<std::shared_ptr<mg::GLRenderingProvider>> const& providers,
+    std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator)
     -> std::shared_ptr<mg::GLRenderingProvider>
 {
     auto display_provider = std::make_shared<Self::OneShotBufferDisplayProvider>();
     OffscreenDisplaySink temp_db{display_provider, geom::Size{640, 480}};
 
-    for (auto const& render_provider : providers)
+    std::pair<mg::probe::Result, std::shared_ptr<mg::GLRenderingProvider>> best_provider = std::make_pair(
+        mg::probe::unsupported, nullptr);
+    for (auto const& provider: providers)
     {
         /* TODO: There might be more sensible ways to select a provider
          * (one in use by at least one DisplaySink, the only one in use, the lowest-powered one,...)
          * That will be a job for a policy object, later.
          *
          * For now, just use the first that claims to work.
          */
-        if (render_provider->suitability_for_display(temp_db) >= mg::probe::supported)
+        auto suitability = provider->suitability_for_display(temp_db);
+        // We also need to make sure that the GLRenderingProvider can access client buffers...
+        if (provider->suitability_for_allocator(buffer_allocator) > mg::probe::unsupported &&
+            suitability > best_provider.first)
         {
-            return render_provider;
+            best_provider = std::make_pair(suitability, provider);
         }
     }
-    BOOST_THROW_EXCEPTION((std::runtime_error{"No rendering provider claims to support a CPU addressable target"}));
+    if (best_provider.first == mg::probe::unsupported)
+    {
+        BOOST_THROW_EXCEPTION((std::runtime_error{"No rendering provider claims to support a CPU addressable target"}));
+    }
+
+    return best_provider.second;
 }
 
 mc::BasicScreenShooter::BasicScreenShooter(
@@ -245,8 +256,9 @@ mc::BasicScreenShooter::BasicScreenShooter(
     Executor& executor,
     std::span<std::shared_ptr<mg::GLRenderingProvider>> const& providers,
     std::shared_ptr<mr::RendererFactory> render_factory,
+    std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
     std::shared_ptr<mir::graphics::GLConfig> const& config)
-    : self{std::make_shared<Self>(scene, clock, select_provider(providers), std::move(render_factory), config)},
+    : self{std::make_shared<Self>(scene, clock, select_provider(providers, buffer_allocator), std::move(render_factory), config)},
       executor{executor}
 {
 }

src/server/compositor/basic_screen_shooter.h

@@ -46,6 +46,7 @@ class BasicScreenShooter: public ScreenShooter
         Executor& executor,
         std::span<std::shared_ptr<graphics::GLRenderingProvider>> const& providers,
         std::shared_ptr<renderer::RendererFactory> render_factory,
+        std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator,
         std::shared_ptr<mir::graphics::GLConfig> const& config);
 
     void capture(
@@ -92,7 +93,9 @@ class BasicScreenShooter: public ScreenShooter
     std::shared_ptr<Self> const self;
     Executor& executor;
 
-    static auto select_provider(std::span<std::shared_ptr<graphics::GLRenderingProvider>> const& providers)
+    static auto select_provider(
+        std::span<std::shared_ptr<graphics::GLRenderingProvider>> const& providers,
+        std::shared_ptr<graphics::GraphicBufferAllocator> const& buffer_allocator)
         -> std::shared_ptr<graphics::GLRenderingProvider>;
 };
 }

src/server/compositor/default_configuration.cpp

@@ -120,6 +120,7 @@ auto mir::DefaultServerConfiguration::the_screen_shooter() -> std::shared_ptr<co
                     thread_pool_executor,
                     providers,
                     the_renderer_factory(),
+                    the_buffer_allocator(),
                     the_gl_config());
             }
             catch (...)

tests/unit-tests/compositor/test_basic_screen_shooter.cpp

@@ -32,6 +32,7 @@
 #include "mir/test/doubles/stub_scene_element.h"
 #include "mir/test/doubles/stub_renderable.h"
 #include "mir/test/doubles/stub_gl_config.h"
+#include "mir/test/doubles/stub_buffer_allocator.h"
 
 #include <gtest/gtest.h>
 
@@ -98,6 +99,8 @@ struct BasicScreenShooter : Test
                     }
                     BOOST_THROW_EXCEPTION((std::runtime_error{"CPU output support not available?!"}));
                 });
+        ON_CALL(*gl_provider, suitability_for_allocator(_))
+            .WillByDefault(Return(mg::probe::supported));
         ON_CALL(*gl_provider, suitability_for_display(_))
             .WillByDefault(Return(mg::probe::supported));
 
@@ -107,6 +110,7 @@ struct BasicScreenShooter : Test
             executor,
             gl_providers,
             renderer_factory,
+            buffer_allocator,
             std::make_shared<mtd::StubGLConfig>());
     }
 
@@ -135,6 +139,7 @@ struct BasicScreenShooter : Test
     std::shared_ptr<mtd::MockGlRenderingProvider> gl_provider{std::make_shared<testing::NiceMock<mtd::MockGlRenderingProvider>>()};
     std::vector<std::shared_ptr<mg::GLRenderingProvider>> gl_providers{gl_provider};
     std::shared_ptr<MockRendererFactory> renderer_factory{std::make_shared<testing::NiceMock<MockRendererFactory>>()};
+    std::shared_ptr<mtd::StubBufferAllocator> buffer_allocator;
     std::shared_ptr<mtd::AdvanceableClock> clock{std::make_shared<mtd::AdvanceableClock>()};
     mtd::ExplicitExecutor executor;
     std::unique_ptr<mc::BasicScreenShooter> shooter;
@@ -249,6 +254,7 @@ TEST_F(BasicScreenShooter, ensures_renderer_is_current_on_only_one_thread)
         mir::thread_pool_executor,
         gl_providers,
         renderer_factory,
+        buffer_allocator,
         std::make_shared<mtd::StubGLConfig>());
 
     ON_CALL(*next_renderer, render(_))