From cbefd4e9099447fb6ddeee7ea854ad2a39bc1bca Mon Sep 17 00:00:00 2001
From: Alan Griffiths <alan@octopull.co.uk>
Date: Fri, 15 Mar 2024 09:44:56 +0000
Subject: [PATCH] platform/linux-dmabuf: Be more lenient with EGL
 implementations (#3279)

Some EGL implementations (*cough* NVIDIA) return formats from
`eglQueryDmaBufFormatsEXT` that generate
`EGL_BAD_PARAMETER` errors when querying for the associated modifiers
with `eglQueryDmaBufModifiersEXT`.

This doesn't need to be fatal, as long as there are *some* formats for
which we can successfully query
the modifiers, so make it non-fatal.

Fixes: #3278
---
 include/platform/mir/graphics/drm_formats.h |  1 +
 src/platform/graphics/drm_formats.cpp       |  7 ++----
 src/platform/graphics/linux_dmabuf.cpp      | 24 +++++++++++++++++++--
 src/platform/symbols.map                    |  1 +
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/include/platform/mir/graphics/drm_formats.h b/include/platform/mir/graphics/drm_formats.h
index 851c5271087..9344ec9c7d7 100644
--- a/include/platform/mir/graphics/drm_formats.h
+++ b/include/platform/mir/graphics/drm_formats.h
@@ -58,6 +58,7 @@ class DRMFormat
 };
 
 auto drm_modifier_to_string(uint64_t modifier) -> std::string;
+auto drm_format_to_string(uint32_t format) -> char const*;
 }
 
 #endif //MIR_PLATFORM_GRAPHICS_DRM_FORMATS_H_
diff --git a/src/platform/graphics/drm_formats.cpp b/src/platform/graphics/drm_formats.cpp
index d1923a2b383..b8d7553c2d4 100644
--- a/src/platform/graphics/drm_formats.cpp
+++ b/src/platform/graphics/drm_formats.cpp
@@ -30,9 +30,7 @@
 
 namespace mg = mir::graphics;
 
-namespace
-{
-constexpr auto drm_format_to_string(uint32_t format) -> char const*
+auto mg::drm_format_to_string(uint32_t format) -> char const*
 {
 #define STRINGIFY(val) \
     case val:          \
@@ -65,7 +63,6 @@ constexpr auto drm_format_to_string(uint32_t format) -> char const*
 #undef STRINGIFY_BIGENDIAN
 }
 
-}
 struct mg::DRMFormat::FormatInfo
 {
     uint32_t format;
@@ -456,7 +453,7 @@ constexpr auto info_for_format(uint32_t fourcc_format) -> mg::DRMFormat::FormatI
     }
     BOOST_THROW_EXCEPTION((
         std::runtime_error{
-            std::string{"Unsupported DRM format: "} + drm_format_to_string(fourcc_format)}));
+            std::string{"Unsupported DRM format: "} + mg::drm_format_to_string(fourcc_format)}));
 }
 
 }
diff --git a/src/platform/graphics/linux_dmabuf.cpp b/src/platform/graphics/linux_dmabuf.cpp
index f12ad91e6fb..7604053fb54 100644
--- a/src/platform/graphics/linux_dmabuf.cpp
+++ b/src/platform/graphics/linux_dmabuf.cpp
@@ -87,7 +87,7 @@ class mg::DmaBufFormatDescriptors
             resize(returned_formats);
         }
 
-        for (auto i = 0u; i < formats.size(); ++i)
+        for (auto i = 0u; i < formats.size();)
         {
             auto [format, modifiers, external_only] = (*this)[i];
 
@@ -101,7 +101,17 @@ class mg::DmaBufFormatDescriptors
                     nullptr,
                     &num_modifiers) != EGL_TRUE)
             {
-                BOOST_THROW_EXCEPTION((mg::egl_error("Failed to query number of modifiers")));
+                mir::log_warning("eglQueryDmaBufModifiers failed for format %s: %s",
+                    mg::drm_format_to_string(static_cast<uint32_t>(format)),
+                    mg::egl_category().message(eglGetError()).c_str());
+
+                // Remove that format and its modifiers from our list
+                formats.erase(formats.begin() + i);
+                modifiers_for_format.erase(modifiers_for_format.begin() + i);
+                external_only_for_format.erase(external_only_for_format.begin() + i);
+                // formats[i] is now the format *after* the one we've just removed,
+                // so we can can continue iterating through the formats from here.
+                continue;
             }
             modifiers.resize(num_modifiers);
             external_only.resize(num_modifiers);
@@ -138,6 +148,16 @@ class mg::DmaBufFormatDescriptors
                 modifiers.push_back(DRM_FORMAT_MOD_INVALID);
                 external_only.push_back(false);
             }
+
+            // We've processed this format; move on to the next one
+            ++i;
+        }
+        if (this->num_formats() == 0)
+        {
+            /* We can get here if the EGL implementation *claimed* to support some formats,
+             * but querying the supported modifiers failed for *all* formats.
+             */
+            BOOST_THROW_EXCEPTION((std::runtime_error{"EGL claimed support for DMA-Buf import modifiers, but all queries failed"}));
         }
     }
 
diff --git a/src/platform/symbols.map b/src/platform/symbols.map
index 19e4f41980b..76600551f15 100644
--- a/src/platform/symbols.map
+++ b/src/platform/symbols.map
@@ -70,6 +70,7 @@ MIR_PLATFORM_2.16 {
     mir::graphics::common::EGLContextExecutor::spawn*;
     mir::graphics::contains_alpha*;
     mir::graphics::drm_modifier_to_string*;
+    mir::graphics::drm_format_to_string*;
     mir::graphics::egl_category*;
     mir::graphics::gl::Program::?Program*;
     mir::graphics::gl::ProgramFactory::?ProgramFactory*;