Merge pull request #3134 from MirServer/remove-pixel-size

platform/DisplaySink: Remove pixel_size() accessor

include/platform/mir/graphics/display_sink.h

@@ -59,9 +59,6 @@ class DisplaySink
     /** The area the DisplaySink occupies in the virtual screen space. */
     virtual geometry::Rectangle view_area() const = 0;
 
-    /** The size in pixels of the underlying display */
-    virtual auto pixel_size() const -> geometry::Size = 0;
-
     /** This will render renderlist to the screen and post the result to the
      *  screen if there is a hardware optimization that can be done.
      *  \param [in] renderlist

include/platform/mir/graphics/platform.h

@@ -165,7 +165,6 @@ class GLRenderingProvider : public RenderingProvider
 
     virtual auto surface_for_sink(
         DisplaySink& sink,
-        geometry::Size size,
         GLConfig const& config) -> std::unique_ptr<gl::OutputSurface> = 0;
 };
 
@@ -308,13 +307,17 @@ class CPUAddressableDisplayAllocator : public DisplayAllocator
     public:
         MappableFB() = default;
         virtual ~MappableFB() override = default;
+
+        using renderer::software::WriteMappableBuffer::size;
     };
 
     virtual auto supported_formats() const
         -> std::vector<DRMFormat> = 0;
 
-    virtual auto alloc_fb(geometry::Size pixel_size, DRMFormat format)
+    virtual auto alloc_fb(DRMFormat format)
         -> std::unique_ptr<MappableFB> = 0;
+
+    virtual auto output_size() const -> geometry::Size = 0;
 };
 
 class GBMDisplayProvider : public DisplayProvider
@@ -382,7 +385,7 @@ class GBMDisplayAllocator : public DisplayAllocator
         virtual auto claim_framebuffer() -> std::unique_ptr<Framebuffer> = 0;
     };
 
-    virtual auto make_surface(geometry::Size size, DRMFormat format, std::span<uint64_t> modifiers) -> std::unique_ptr<GBMSurface> = 0;
+    virtual auto make_surface(DRMFormat format, std::span<uint64_t> modifiers) -> std::unique_ptr<GBMSurface> = 0;
 };
 
 class DmaBufBuffer;
@@ -420,6 +423,7 @@ class EGLStreamDisplayAllocator : public DisplayAllocator
     };
 
     virtual auto claim_stream() -> EGLStreamKHR = 0;
+    virtual auto output_size() const -> geometry::Size = 0;
 };
 
 class GenericEGLDisplayProvider : public DisplayProvider

src/platforms/common/server/cpu_copy_output_surface.cpp

@@ -127,8 +127,7 @@ class mgc::CPUCopyOutputSurface::Impl
     Impl(
         EGLDisplay dpy,
         EGLContext share_ctx,
-        mg::CPUAddressableDisplayAllocator& allocator,
-        geom::Size size);
+        mg::CPUAddressableDisplayAllocator& allocator);
 
     void bind();
 
@@ -144,7 +143,6 @@ class mgc::CPUCopyOutputSurface::Impl
     mg::CPUAddressableDisplayAllocator& allocator;
     EGLDisplay const dpy;
     EGLContext const ctx;
-    geometry::Size const size_;
     DRMFormat const format;
     RenderbufferHandle const colour_buffer;
     FramebufferHandle const fbo;
@@ -153,9 +151,8 @@ class mgc::CPUCopyOutputSurface::Impl
 mgc::CPUCopyOutputSurface::CPUCopyOutputSurface(
         EGLDisplay dpy,
         EGLContext share_ctx,
-        mg::CPUAddressableDisplayAllocator& allocator,
-        geom::Size size)
-        : impl{std::make_unique<Impl>(dpy, share_ctx, allocator, size)}
+        mg::CPUAddressableDisplayAllocator& allocator)
+        : impl{std::make_unique<Impl>(dpy, share_ctx, allocator)}
 {
 }
 
@@ -194,16 +191,14 @@ auto mgc::CPUCopyOutputSurface::layout() const -> Layout
 mgc::CPUCopyOutputSurface::Impl::Impl(
     EGLDisplay dpy,
     EGLContext share_ctx,
-    mg::CPUAddressableDisplayAllocator& allocator,
-    geom::Size size)
+    mg::CPUAddressableDisplayAllocator& allocator)
     : allocator{allocator},
       dpy{dpy},
       ctx{create_current_context(dpy, share_ctx)},
-      size_{size},
       format{select_format_from(allocator)}
 {
     glBindRenderbuffer(GL_RENDERBUFFER, colour_buffer);
-    glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8_OES, size_.width.as_int(), size_.height.as_int());
+    glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8_OES, size().width.as_int(), size().height.as_int());
 
     glBindFramebuffer(GL_FRAMEBUFFER, fbo);
     glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, colour_buffer);
@@ -264,7 +259,7 @@ void mgc::CPUCopyOutputSurface::Impl::release_current()
 
 auto mgc::CPUCopyOutputSurface::Impl::commit() -> std::unique_ptr<mg::Framebuffer>
 {
-    auto fb = allocator.alloc_fb(size_, format);
+    auto fb = allocator.alloc_fb(format);
     glBindFramebuffer(GL_FRAMEBUFFER, fbo);
     {
         /* TODO: We can usefully put this *into* DRMFormat */
@@ -288,15 +283,15 @@ auto mgc::CPUCopyOutputSurface::Impl::commit() -> std::unique_ptr<mg::Framebuffe
          */
         glReadPixels(
             0, 0,
-            size_.width.as_int(), size_.height.as_int(),
+            fb->size().width.as_uint32_t(), fb->size().height.as_uint32_t(),
             pixel_layout, GL_UNSIGNED_BYTE, mapping->data());
     }
     return fb;
 }
 
 auto mgc::CPUCopyOutputSurface::Impl::size() const -> geom::Size
 {
-    return size_;
+    return allocator.output_size();
 }
 
 auto mgc::CPUCopyOutputSurface::Impl::layout() const -> Layout

src/platforms/common/server/cpu_copy_output_surface.h

@@ -35,8 +35,7 @@ class CPUCopyOutputSurface : public gl::OutputSurface
     CPUCopyOutputSurface(
         EGLDisplay dpy,
         EGLContext share_ctx,
-        CPUAddressableDisplayAllocator& allocator,
-        geometry::Size size);
+        CPUAddressableDisplayAllocator& allocator);
 
     ~CPUCopyOutputSurface() override;
 
@@ -57,4 +56,4 @@ class CPUCopyOutputSurface : public gl::OutputSurface
     std::unique_ptr<Impl> const impl;
 };
 }
-}
+}

src/platforms/common/server/kms_cpu_addressable_display_provider.cpp

@@ -39,9 +39,10 @@ auto drm_get_cap_checked(mir::Fd const& drm_fd, uint64_t cap) -> uint64_t
 namespace mg = mir::graphics;
 namespace geom = mir::geometry;
 
-mg::kms::CPUAddressableDisplayAllocator::CPUAddressableDisplayAllocator(mir::Fd drm_fd)
+mg::kms::CPUAddressableDisplayAllocator::CPUAddressableDisplayAllocator(mir::Fd drm_fd, geom::Size size)
     : drm_fd{std::move(drm_fd)},
-      supports_modifiers{drm_get_cap_checked(this->drm_fd, DRM_CAP_ADDFB2_MODIFIERS) == 1}
+      supports_modifiers{drm_get_cap_checked(this->drm_fd, DRM_CAP_ADDFB2_MODIFIERS) == 1},
+      size{size}
 {
 }
 
@@ -52,18 +53,22 @@ auto mg::kms::CPUAddressableDisplayAllocator::supported_formats() const
     return {mg::DRMFormat{DRM_FORMAT_XRGB8888}, mg::DRMFormat{DRM_FORMAT_ARGB8888}};
 }
 
-auto mg::kms::CPUAddressableDisplayAllocator::alloc_fb(
-    geom::Size size, DRMFormat format) -> std::unique_ptr<MappableFB>
+auto mg::kms::CPUAddressableDisplayAllocator::alloc_fb(DRMFormat format) -> std::unique_ptr<MappableFB>
 {
     return std::make_unique<mg::CPUAddressableFB>(drm_fd, supports_modifiers, format, size);
 }
 
-auto mir::graphics::kms::CPUAddressableDisplayAllocator::create_if_supported(mir::Fd const& drm_fd)
+auto mg::kms::CPUAddressableDisplayAllocator::output_size() const -> geom::Size
+{
+    return size;
+}
+
+auto mir::graphics::kms::CPUAddressableDisplayAllocator::create_if_supported(mir::Fd const& drm_fd, geom::Size size)
 -> std::shared_ptr<CPUAddressableDisplayAllocator>
 {
     if  (drm_get_cap_checked(drm_fd, DRM_CAP_DUMB_BUFFER))
     {
-        return std::shared_ptr<CPUAddressableDisplayAllocator>(new CPUAddressableDisplayAllocator{drm_fd});
+        return std::shared_ptr<CPUAddressableDisplayAllocator>(new CPUAddressableDisplayAllocator{drm_fd, size});
     }
     else
     {

src/platforms/common/server/kms_cpu_addressable_display_provider.h

@@ -31,19 +31,22 @@ class CPUAddressableDisplayAllocator : public graphics::CPUAddressableDisplayAll
 public:
     /// Create an CPUAddressableDisplayAllocator if and only if supported by the device
     /// \return the provider, or an empty pointer
-    static auto create_if_supported(mir::Fd const& drm_fd) -> std::shared_ptr<CPUAddressableDisplayAllocator>;
+    static auto create_if_supported(mir::Fd const& drm_fd, geometry::Size size)
+        -> std::shared_ptr<CPUAddressableDisplayAllocator>;
 
     auto supported_formats() const
         -> std::vector<DRMFormat> override;
 
-    auto alloc_fb(geometry::Size pixel_size, DRMFormat format)
+    auto alloc_fb(DRMFormat format)
         -> std::unique_ptr<MappableFB> override;
 
+    auto output_size() const -> geometry::Size override;
 private:
-    explicit CPUAddressableDisplayAllocator(mir::Fd drm_fd);
+    explicit CPUAddressableDisplayAllocator(mir::Fd drm_fd, geometry::Size size);
 
     mir::Fd const drm_fd;
     bool const supports_modifiers;
+    geometry::Size const size;
 };
 }
 }

src/platforms/eglstream-kms/server/buffer_allocator.cpp

@@ -731,7 +731,6 @@ auto mir::graphics::eglstream::GLRenderingProvider::as_texture(std::shared_ptr<B
 
 auto mge::GLRenderingProvider::surface_for_sink(
     DisplaySink& sink,
-    geom::Size size,
     mg::GLConfig const& gl_config) -> std::unique_ptr<gl::OutputSurface>
 {
     if (auto stream_platform = sink.acquire_compatible_allocator<EGLStreamDisplayAllocator>())
@@ -743,7 +742,7 @@ auto mge::GLRenderingProvider::surface_for_sink(
                 pick_stream_surface_config(dpy, gl_config),
                 static_cast<EGLContext>(*ctx),
                 stream_platform->claim_stream(),
-                size);
+                stream_platform->output_size());
         }
         catch (std::exception const& err)
         {
@@ -759,8 +758,7 @@ auto mge::GLRenderingProvider::surface_for_sink(
         return std::make_unique<mgc::CPUCopyOutputSurface>(
             dpy,
             static_cast<EGLContext>(*ctx),
-            *cpu_provider,
-            size);
+            *cpu_provider);
     }
     BOOST_THROW_EXCEPTION((std::runtime_error{"DisplayInterfaceProvider does not support any viable output interface"}));
 }

src/platforms/eglstream-kms/server/buffer_allocator.h

@@ -108,7 +108,6 @@ class GLRenderingProvider : public graphics::GLRenderingProvider
 
     auto surface_for_sink(
         DisplaySink& sink,
-        geometry::Size size,
         GLConfig const& gl_config) -> std::unique_ptr<gl::OutputSurface> override;
 private:
     EGLDisplay dpy;

src/platforms/eglstream-kms/server/display.cpp

@@ -200,7 +200,7 @@ class DisplaySink
         return output->extents();
     }
 
-    auto pixel_size() const -> mir::geometry::Size override
+    auto output_size() const -> geom::Size override
     {
         return output->size();
     }
@@ -365,7 +365,7 @@ class DisplaySink
         }
         if (dynamic_cast<mg::CPUAddressableDisplayAllocator::Tag const*>(&type_tag))
         {
-            kms_allocator = mg::kms::CPUAddressableDisplayAllocator::create_if_supported(drm_node);
+            kms_allocator = mg::kms::CPUAddressableDisplayAllocator::create_if_supported(drm_node, output->size());
             return kms_allocator.get();
         }
         return nullptr;

src/platforms/gbm-kms/server/buffer_allocator.cpp

@@ -263,12 +263,10 @@ class GBMOutputSurface : public mg::gl::OutputSurface
         EGLContext share_context,
         mg::GLConfig const& config,
         mg::GBMDisplayAllocator& display,
-        mg::DRMFormat format,
-        mir::geometry::Size size)
+        mg::DRMFormat format)
         : GBMOutputSurface(
-              size,
               dpy,
-              create_renderable(dpy, share_context, format, config, display, size))
+              create_renderable(dpy, share_context, format, config, display))
     {
     }
 
@@ -313,7 +311,16 @@ class GBMOutputSurface : public mg::gl::OutputSurface
 
     auto size() const -> geom::Size override
     {
-        return size_;
+        EGLint width, height;
+        if (eglQuerySurface(dpy, egl_surf, EGL_WIDTH, &width) != EGL_TRUE)
+        {
+            BOOST_THROW_EXCEPTION((mg::egl_error("Failed to query surface width")));
+        }
+        if (eglQuerySurface(dpy, egl_surf, EGL_HEIGHT, &height) != EGL_TRUE)
+        {
+            BOOST_THROW_EXCEPTION((mg::egl_error("Failed to query surface height")));
+        }
+        return geom::Size{width, height};
     }
 
     auto layout() const -> Layout override
@@ -386,8 +393,7 @@ class GBMOutputSurface : public mg::gl::OutputSurface
         EGLContext share_context,
         mg::DRMFormat format,
         mg::GLConfig const& config,
-        mg::GBMDisplayAllocator& allocator,
-        mir::geometry::Size size)
+        mg::GBMDisplayAllocator& allocator)
          -> std::tuple<std::unique_ptr<mg::GBMDisplayAllocator::GBMSurface>, EGLContext, EGLSurface>
     {
         mg::EGLExtensions::PlatformBaseEXT egl_ext;
@@ -426,7 +432,7 @@ class GBMOutputSurface : public mg::gl::OutputSurface
 
         auto modifiers = allocator.modifiers_for_format(resolved_format);
 
-        auto surf = allocator.make_surface(size, resolved_format, modifiers);
+        auto surf = allocator.make_surface(resolved_format, modifiers);
 
         auto egl_surf = egl_ext.eglCreatePlatformWindowSurface(
             dpy,
@@ -455,18 +461,15 @@ class GBMOutputSurface : public mg::gl::OutputSurface
     }
 
     GBMOutputSurface(
-        geom::Size size,
         EGLDisplay dpy,
         std::tuple<std::unique_ptr<mg::GBMDisplayAllocator::GBMSurface>, EGLContext, EGLSurface> renderables)
-        : size_{size},
-          surface{std::move(std::get<0>(renderables))},
+        : surface{std::move(std::get<0>(renderables))},
           egl_surf{std::get<2>(renderables)},
           dpy{dpy},
           ctx{std::get<1>(renderables)}
     {
     }
 
-    geom::Size const size_;
     std::unique_ptr<mg::GBMDisplayAllocator::GBMSurface> const surface;
     EGLSurface const egl_surf;
     EGLDisplay const dpy;
@@ -511,7 +514,6 @@ auto mgg::GLRenderingProvider::suitability_for_display(
 
 auto mgg::GLRenderingProvider::surface_for_sink(
     DisplaySink& sink,
-    geometry::Size size,
     GLConfig const& config)
     -> std::unique_ptr<gl::OutputSurface>
 {
@@ -526,8 +528,7 @@ auto mgg::GLRenderingProvider::surface_for_sink(
                     ctx,
                     config,
                     *gbm_allocator,
-                    DRMFormat{DRM_FORMAT_XRGB8888},
-                    size);
+                    DRMFormat{DRM_FORMAT_XRGB8888});
             }
         }
     }
@@ -536,8 +537,7 @@ auto mgg::GLRenderingProvider::surface_for_sink(
     return std::make_unique<mgc::CPUCopyOutputSurface>(
         dpy,
         ctx,
-        *cpu_allocator,
-        size);
+        *cpu_allocator);
 }
 
 auto mgg::GLRenderingProvider::make_framebuffer_provider(DisplaySink& /*sink*/)

src/platforms/gbm-kms/server/buffer_allocator.h

@@ -106,7 +106,6 @@ class GLRenderingProvider : public graphics::GLRenderingProvider
 
     auto surface_for_sink(
         DisplaySink& sink,
-        geometry::Size size,
         GLConfig const& config) -> std::unique_ptr<gl::OutputSurface> override;
 
 private:

src/platforms/gbm-kms/server/gbm_display_allocator.cpp

@@ -25,9 +25,10 @@ namespace mg = mir::graphics;
 namespace mgg = mir::graphics::gbm;
 namespace geom = mir::geometry;
 
-mgg::GBMDisplayAllocator::GBMDisplayAllocator(mir::Fd drm_fd, std::shared_ptr<struct gbm_device> gbm)
+mgg::GBMDisplayAllocator::GBMDisplayAllocator(mir::Fd drm_fd, std::shared_ptr<struct gbm_device> gbm, geom::Size size)
     : fd{std::move(drm_fd)},
-      gbm{std::move(gbm)}
+      gbm{std::move(gbm)},
+      size{size}
 {
 }
 
@@ -205,9 +206,9 @@ class GBMSurfaceImpl : public mgg::GBMDisplayAllocator::GBMSurface
 };
 }
 
-auto mgg::GBMDisplayAllocator::make_surface(geom::Size size, DRMFormat format, std::span<uint64_t> modifiers)
+auto mgg::GBMDisplayAllocator::make_surface(DRMFormat format, std::span<uint64_t> modifiers)
     -> std::unique_ptr<GBMSurface>
 {
-    return std::make_unique<GBMSurfaceImpl>(fd, gbm.get(), std::move(size), format, modifiers);
+    return std::make_unique<GBMSurfaceImpl>(fd, gbm.get(), size, format, modifiers);
 }