From 672261764efa027463f64188f18d5582cb56d2fd Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 2 Oct 2026 09:37:17 -0400 Subject: [PATCH] [Fix] (SharedImage): allocate and import ARGB8888/XRGB8888 images so KWin's offscreen Quick views get buffers --- MobileGL/MG_Backend/BackendObject.h | 3 ++- .../DirectVulkan/Renderer/VkTextureManager.cpp | 2 +- MobileGL/MG_Gbm/MobileGLGbm.cpp | 9 +++++---- MobileGL/MG_Impl/SharedImageApi.cpp | 3 ++- MobileGL/MG_Impl/SharedImageApi.h | 5 +++++ .../MG_Remote/Server/SharedImageRegistry.cpp | 9 +++++++-- MobileGL/MG_Remote/Server/SharedImageRegistry.h | 8 ++++++++ MobileGL/MG_State/EGLState/Core.cpp | 9 +++++---- MobileGL/MG_State/EGLState/Core.h | 5 ++++- MobileGL/MG_Test/EGLState/EGLStateTest.cpp | 16 ++++++++++------ MobileGL/MG_Test/Wire/SharedImageTest.cpp | 6 ++++++ 11 files changed, 55 insertions(+), 20 deletions(-) diff --git a/MobileGL/MG_Backend/BackendObject.h b/MobileGL/MG_Backend/BackendObject.h index 4d2339e3..43dec21a 100644 --- a/MobileGL/MG_Backend/BackendObject.h +++ b/MobileGL/MG_Backend/BackendObject.h @@ -588,7 +588,8 @@ namespace MobileGL { // // The server-side view of one image, as a backend sees it: `NativeBuffer` is the image's // AHardwareBuffer (null where there is none), `Fourcc` a DRM fourcc - ABGR8888 (R, G, B, A in - // memory, GL RGBA8) or XBGR8888 (alpha ignored). + // memory, GL RGBA8) or XBGR8888 (alpha ignored); ARGB8888 / XRGB8888 images are stored the + // same way (SharedImageRegistry.h says why), so a backend binds every one as RGBA8. struct SharedImageView { Uint64 Id = 0; Uint32 Width = 0; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp index 696f95de..dcfaa021 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkTextureManager.cpp @@ -3001,7 +3001,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { replacement.usageFlags = imported.usage; replacement.sharedImageId = id; replacement.sharedImageOwner = image; - replacement.sharedImageAlphaOne = image->Fourcc == MG_Remote::Server::SharedImages::kFourccXbgr8888; + replacement.sharedImageAlphaOne = MG_Remote::Server::SharedImages::FourccIgnoresAlpha(image->Fourcc); ++m_textureImageEpoch; // a new attachment image invalidates cached render passes DeferResourceRelease(Move(resource)); std::destroy_at(&resource); diff --git a/MobileGL/MG_Gbm/MobileGLGbm.cpp b/MobileGL/MG_Gbm/MobileGLGbm.cpp index 0975ed4f..9580cbb0 100644 --- a/MobileGL/MG_Gbm/MobileGLGbm.cpp +++ b/MobileGL/MG_Gbm/MobileGLGbm.cpp @@ -117,11 +117,12 @@ namespace { return format; } - // What a shared image can be. ARGB8888/XRGB8888 (B, G, R, A in memory) are absent on purpose: - // the server's images hold R, G, B, A, and an image whose bytes disagree with its fourcc is - // worse than no image. + // What a shared image can be. The ARGB/XRGB orders are stored like the ABGR/XBGR ones: the + // layout is the server's (modifier INVALID, no map is offered), so no reader can tell, and + // every GL that touches the image addresses its channels logically. bool FormatSupported(uint32_t format) { - return format == MOBILEGL_SHARED_IMAGE_FOURCC_ABGR8888 || format == MOBILEGL_SHARED_IMAGE_FOURCC_XBGR8888; + return format == MOBILEGL_SHARED_IMAGE_FOURCC_ABGR8888 || format == MOBILEGL_SHARED_IMAGE_FOURCC_XBGR8888 || + format == MOBILEGL_SHARED_IMAGE_FOURCC_ARGB8888 || format == MOBILEGL_SHARED_IMAGE_FOURCC_XRGB8888; } bool ModifierAcceptable(uint64_t modifier) { return modifier == kModifierInvalid || modifier == kModifierLinear; } diff --git a/MobileGL/MG_Impl/SharedImageApi.cpp b/MobileGL/MG_Impl/SharedImageApi.cpp index 292ffc77..b36de6cd 100644 --- a/MobileGL/MG_Impl/SharedImageApi.cpp +++ b/MobileGL/MG_Impl/SharedImageApi.cpp @@ -26,7 +26,8 @@ namespace { using MobileGL::MG_Backend::BackendObject; - constexpr uint32_t kFormats[] = {MOBILEGL_SHARED_IMAGE_FOURCC_ABGR8888, MOBILEGL_SHARED_IMAGE_FOURCC_XBGR8888}; + constexpr uint32_t kFormats[] = {MOBILEGL_SHARED_IMAGE_FOURCC_ABGR8888, MOBILEGL_SHARED_IMAGE_FOURCC_XBGR8888, + MOBILEGL_SHARED_IMAGE_FOURCC_ARGB8888, MOBILEGL_SHARED_IMAGE_FOURCC_XRGB8888}; bool FourccSupported(uint32_t fourcc) { for (uint32_t format : kFormats) { diff --git a/MobileGL/MG_Impl/SharedImageApi.h b/MobileGL/MG_Impl/SharedImageApi.h index 6c57fdc5..b6b479e6 100644 --- a/MobileGL/MG_Impl/SharedImageApi.h +++ b/MobileGL/MG_Impl/SharedImageApi.h @@ -41,6 +41,11 @@ extern "C" { /* DRM fourccs (little-endian packed). ABGR8888 is R, G, B, A in memory - GL's RGBA8. */ #define MOBILEGL_SHARED_IMAGE_FOURCC_ABGR8888 0x34324241u /* 'AB24' */ #define MOBILEGL_SHARED_IMAGE_FOURCC_XBGR8888 0x34324258u /* 'XB24' */ +/* ARGB8888 / XRGB8888: allocatable too. The layout of every shared image is the server's own + * (modifier INVALID, never mapped), and channels are addressed logically by the GL that reads and + * writes them, so the name an image is allocated under never changes its colours. */ +#define MOBILEGL_SHARED_IMAGE_FOURCC_ARGB8888 0x34325241u /* 'AR24' */ +#define MOBILEGL_SHARED_IMAGE_FOURCC_XRGB8888 0x34325258u /* 'XR24' */ /* One allocated image. VERSIONED BY SIZE: the caller sets struct_size to the sizeof it was built * against and the library writes no byte past it, so fields are only ever appended. */ diff --git a/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp b/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp index c0efa832..fb458c48 100644 --- a/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp +++ b/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp @@ -110,7 +110,7 @@ namespace MobileGL::MG_Remote::Server::SharedImages { desc.width = image.Width; desc.height = image.Height; desc.layers = 1; - desc.format = image.Fourcc == kFourccXbgr8888 ? AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM + desc.format = FourccIgnoresAlpha(image.Fourcc) ? AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM : AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM; desc.usage = AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE | AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT; AHardwareBuffer* ahb = nullptr; @@ -186,7 +186,12 @@ namespace MobileGL::MG_Remote::Server::SharedImages { ReleaseNative(Native); } - Bool FourccSupported(Uint32 fourcc) { return fourcc == kFourccAbgr8888 || fourcc == kFourccXbgr8888; } + Bool FourccSupported(Uint32 fourcc) { + return fourcc == kFourccAbgr8888 || fourcc == kFourccXbgr8888 || fourcc == kFourccArgb8888 || + fourcc == kFourccXrgb8888; + } + + Bool FourccIgnoresAlpha(Uint32 fourcc) { return fourcc == kFourccXbgr8888 || fourcc == kFourccXrgb8888; } ImageRef Allocate(Uint32 width, Uint32 height, Uint32 fourcc, std::string& why) { if (width == 0 || height == 0 || width > 16384 || height > 16384) { diff --git a/MobileGL/MG_Remote/Server/SharedImageRegistry.h b/MobileGL/MG_Remote/Server/SharedImageRegistry.h index b69949ce..ac950ed0 100644 --- a/MobileGL/MG_Remote/Server/SharedImageRegistry.h +++ b/MobileGL/MG_Remote/Server/SharedImageRegistry.h @@ -38,12 +38,20 @@ namespace MobileGL::MG_Remote::Server::SharedImages { } inline constexpr Uint32 kFourccAbgr8888 = FourCC('A', 'B', '2', '4'); inline constexpr Uint32 kFourccXbgr8888 = FourCC('X', 'B', '2', '4'); + // ARGB8888 / XRGB8888 name B, G, R, A in memory. An image of either is still stored R, G, B, A: + // its layout is the allocator's (the modifier is INVALID and no CPU mapping is offered), every + // reader and writer is this server, and they all address channels LOGICALLY - so what a + // client renders as red samples as red, whichever of the four names it allocated under. + inline constexpr Uint32 kFourccArgb8888 = FourCC('A', 'R', '2', '4'); + inline constexpr Uint32 kFourccXrgb8888 = FourCC('X', 'R', '2', '4'); // "The layout is the allocator's": what an AHardwareBuffer is to anybody but its allocator. inline constexpr Uint64 kModifierInvalid = 0x00ffffffffffffffull; inline constexpr Uint64 kModifierLinear = 0; // Whether images of this fourcc can be allocated (and so imported) at all. Bool FourccSupported(Uint32 fourcc); + // The X formats: alpha reads as 1 whatever is stored. + Bool FourccIgnoresAlpha(Uint32 fourcc); struct Image { Uint64 Id = 0; diff --git a/MobileGL/MG_State/EGLState/Core.cpp b/MobileGL/MG_State/EGLState/Core.cpp index ead4557d..d445f0ea 100644 --- a/MobileGL/MG_State/EGLState/Core.cpp +++ b/MobileGL/MG_State/EGLState/Core.cpp @@ -1578,9 +1578,9 @@ namespace MobileGL { } Bool EGLContext::IsDmaBufFourccSupported(Uint32 fourcc) { - // Only the GL RGBA8 byte order: an ARGB/XRGB-ordered import would need the - // backend to swizzle the image it binds, which nothing records yet. - return fourcc == kDrmFourccAbgr8888 || fourcc == kDrmFourccXbgr8888; + // The four orders a shared image can be allocated under (SharedImageApi.h). + return fourcc == kDrmFourccAbgr8888 || fourcc == kDrmFourccXbgr8888 || fourcc == kDrmFourccArgb8888 || + fourcc == kDrmFourccXrgb8888; } Bool EGLContext::PrepareDmaBufImport(EGLDisplayHandle display, EGLContextHandle context, @@ -1753,7 +1753,8 @@ namespace MobileGL { SetError(EGL_BAD_PARAMETER); return false; } - static constexpr Uint32 kFormats[] = {kDrmFourccAbgr8888, kDrmFourccXbgr8888}; + static constexpr Uint32 kFormats[] = {kDrmFourccAbgr8888, kDrmFourccXbgr8888, kDrmFourccArgb8888, + kDrmFourccXrgb8888}; const EGLint total = available ? static_cast(std::size(kFormats)) : 0; if (maxFormats == 0) { *numFormats = total; diff --git a/MobileGL/MG_State/EGLState/Core.h b/MobileGL/MG_State/EGLState/Core.h index 34716777..5e37820d 100644 --- a/MobileGL/MG_State/EGLState/Core.h +++ b/MobileGL/MG_State/EGLState/Core.h @@ -149,7 +149,8 @@ namespace MobileGL { // DMA-BUF IMPORT (EGL_EXT_image_dma_buf_import[_modifiers]). A dma-buf this library // can name is one of the backend's shared images (MG_Backend::SharedImageExport): - // single plane, DRM ABGR8888 or XBGR8888 - the GL RGBA8 byte order. The state + // single plane, DRM ABGR8888 / XBGR8888 / ARGB8888 / XRGB8888 (all stored RGBA8 by the + // server, whose layout no client can observe). The state // validates and records; the backend identifies the descriptor in between. struct DmaBufImportAttribs { EGLint Width = 0; @@ -169,6 +170,8 @@ namespace MobileGL { }; static constexpr Uint32 kDrmFourccAbgr8888 = 0x34324241u; // 'AB24' static constexpr Uint32 kDrmFourccXbgr8888 = 0x34324258u; // 'XB24' + static constexpr Uint32 kDrmFourccArgb8888 = 0x34325241u; // 'AR24' + static constexpr Uint32 kDrmFourccXrgb8888 = 0x34325258u; // 'XR24' static Bool IsDmaBufFourccSupported(Uint32 fourcc); // The checks eglCreateImage owes an EGL_LINUX_DMA_BUF_EXT target before anything diff --git a/MobileGL/MG_Test/EGLState/EGLStateTest.cpp b/MobileGL/MG_Test/EGLState/EGLStateTest.cpp index b55ce920..7961c782 100644 --- a/MobileGL/MG_Test/EGLState/EGLStateTest.cpp +++ b/MobileGL/MG_Test/EGLState/EGLStateTest.cpp @@ -138,7 +138,9 @@ TEST(EGLStateProfile, CompatibilityProfileRequiresExplicitCompatBit) { namespace { constexpr EGLAttrib kAbgr8888 = StateContext::kDrmFourccAbgr8888; constexpr EGLAttrib kXbgr8888 = StateContext::kDrmFourccXbgr8888; - constexpr EGLAttrib kArgb8888 = 0x34325241; // 'AR24': not a shared-image byte order + constexpr EGLAttrib kArgb8888 = StateContext::kDrmFourccArgb8888; + constexpr EGLAttrib kXrgb8888 = StateContext::kDrmFourccXrgb8888; + constexpr EGLAttrib kRgb565 = 0x36314752; // 'RG16': not a shared-image format // A complete single-plane import, as a compositor builds it for a linux-dmabuf wl_buffer. std::vector DmaBufAttribs(EGLAttrib fourcc = kAbgr8888) { @@ -208,8 +210,8 @@ TEST(EGLStateDmaBufImport, RefusesWhatTheExtensionRefuses) { EXPECT_EQ(PrepareError(*fixture, DmaBufAttribs(), EGL_NO_CONTEXT, ¬ABuffer), EGL_BAD_PARAMETER); // An attribute no dma-buf import has. EXPECT_EQ(PrepareError(*fixture, WithAttrib(DmaBufAttribs(), EGL_CONFIG_ID, 1)), EGL_BAD_PARAMETER); - // Formats outside the shared images' byte order, and planes a single-plane format lacks. - EXPECT_EQ(PrepareError(*fixture, DmaBufAttribs(kArgb8888)), EGL_BAD_MATCH); + // A format no shared image has, and planes a single-plane format lacks. + EXPECT_EQ(PrepareError(*fixture, DmaBufAttribs(kRgb565)), EGL_BAD_MATCH); EXPECT_EQ(PrepareError(*fixture, WithAttrib(DmaBufAttribs(), EGL_DMA_BUF_PLANE1_FD_EXT, 8)), EGL_BAD_ATTRIBUTE); // Values no image can have. auto zeroWidth = WithoutAttrib(DmaBufAttribs(), EGL_WIDTH); @@ -265,12 +267,14 @@ TEST(EGLStateDmaBufImport, FormatAndModifierQueries) { auto fixture = CreateFixture(); EGLint count = -1; ASSERT_TRUE(fixture->State.QueryDmaBufFormats(fixture->Display, true, 0, nullptr, &count)); - EXPECT_EQ(count, 2); + EXPECT_EQ(count, 4); EGLint formats[4] = {}; ASSERT_TRUE(fixture->State.QueryDmaBufFormats(fixture->Display, true, 4, formats, &count)); - ASSERT_EQ(count, 2); + ASSERT_EQ(count, 4); EXPECT_EQ(formats[0], static_cast(kAbgr8888)); EXPECT_EQ(formats[1], static_cast(kXbgr8888)); + EXPECT_EQ(formats[2], static_cast(kArgb8888)); + EXPECT_EQ(formats[3], static_cast(kXrgb8888)); ASSERT_TRUE(fixture->State.QueryDmaBufFormats(fixture->Display, true, 1, formats, &count)); EXPECT_EQ(count, 1); @@ -287,7 +291,7 @@ TEST(EGLStateDmaBufImport, FormatAndModifierQueries) { ASSERT_TRUE(fixture->State.QueryDmaBufModifiers(fixture->Display, true, static_cast(kAbgr8888), 2, modifiers, externalOnly, &count)); EXPECT_EQ(count, 0); - EXPECT_FALSE(fixture->State.QueryDmaBufModifiers(fixture->Display, true, static_cast(kArgb8888), 0, + EXPECT_FALSE(fixture->State.QueryDmaBufModifiers(fixture->Display, true, static_cast(kRgb565), 0, nullptr, nullptr, &count)); EXPECT_EQ(fixture->State.ConsumeError(), EGL_BAD_PARAMETER); diff --git a/MobileGL/MG_Test/Wire/SharedImageTest.cpp b/MobileGL/MG_Test/Wire/SharedImageTest.cpp index 6e1bf041..ca18318b 100644 --- a/MobileGL/MG_Test/Wire/SharedImageTest.cpp +++ b/MobileGL/MG_Test/Wire/SharedImageTest.cpp @@ -91,6 +91,12 @@ TEST(SharedImage, AnImageDiesWithItsLastReferenceAndIsNoLongerIdentified) { TEST(SharedImage, UnsupportedFormatsAndSizesAreRefused) { std::string why; EXPECT_EQ(SI::Allocate(16, 16, SI::FourCC('N', 'V', '1', '2'), why), nullptr); + // The ARGB orders are allocatable: the layout is the server's, and X ignores alpha. + SI::ImageRef argb = SI::Allocate(16, 16, SI::kFourccArgb8888, why); + ASSERT_NE(argb, nullptr) << why; + EXPECT_EQ(argb->Fourcc, SI::kFourccArgb8888); + EXPECT_TRUE(SI::FourccIgnoresAlpha(SI::kFourccXrgb8888)); + EXPECT_FALSE(SI::FourccIgnoresAlpha(SI::kFourccArgb8888)); EXPECT_EQ(SI::Allocate(0, 16, SI::kFourccAbgr8888, why), nullptr); EXPECT_EQ(SI::Allocate(16, 1u << 20, SI::kFourccAbgr8888, why), nullptr); }