Simplify GetNativeGpuMemoryBufferConfigurations()

1. Previously GetNativeGpuMemoryBufferConfigurations() was calling
IsNativeGpuMemoryBufferConfigurationSupported() twice for many
configurations. Also support for some formats wasn't tested.
Updated this function to check support for all BufferFormat values
and to avoid redundant work.

2. Moved kEnableNativeGpuMemoryBuffers flag to ClientNativePixmapDmaBuf
and removed kDisableNativeGpuMemoryBuffers completely. These flags didn't
have any effect on platforms other than Linux

3. Disabled RG_88 support in ClientNativePixmapDmaBuf. RG_88 pixmaps were
already disabled in GetNativeGpuMemoryBufferConfigurations() and they are
broken on chromeos.

Bug: 852011, 954233
Change-Id: I1651d97325bc5fdea02e9f03e0a83dcb3e2109c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1570760Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652757}

chrome/browser/chromeos/BUILD.gn

@@ -266,6 +266,7 @@ source_set("chromeos") {
     "//ui/events/devices",
     "//ui/events/platform",
     "//ui/file_manager",
+    "//ui/gfx:gfx_switches",
     "//ui/keyboard",
     "//ui/message_center",
     "//ui/message_center/public/cpp",

chrome/browser/chromeos/login/chrome_restart_request.cc

@@ -42,7 +42,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
-#include "gpu/ipc/host/gpu_switches.h"
 #include "media/base/media_switches.h"
 #include "media/media_buildflags.h"
 #include "services/service_manager/sandbox/switches.h"
@@ -50,6 +49,7 @@
 #include "ui/base/ui_base_switches.h"
 #include "ui/display/display_switches.h"
 #include "ui/events/event_switches.h"
+#include "ui/gfx/switches.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/ozone/public/ozone_switches.h"
 #include "ui/wm/core/wm_core_switches.h"

content/browser/gpu/compositor_util.cc

@@ -175,11 +175,6 @@ const GpuFeatureData GetGpuFeatureData(
       {"multiple_raster_threads", gpu::kGpuFeatureStatusEnabled,
        NumberOfRendererRasterThreads() == 1, "Raster is using a single thread.",
        false, true},
-      {"native_gpu_memory_buffers", gpu::kGpuFeatureStatusEnabled,
-       !gpu::AreNativeGpuMemoryBuffersEnabled(),
-       "Native GpuMemoryBuffers have been disabled, either via about:flags or "
-       "command line.",
-       true, true},
       {"surface_control",
        SafeGetFeatureStatus(gpu_feature_info,
                             gpu::GPU_FEATURE_TYPE_ANDROID_SURFACE_CONTROL),
@@ -436,10 +431,6 @@ bool IsGpuMemoryBufferCompositorResourcesEnabled() {
     return false;
   }
 
-  // Native GPU memory buffers are required.
-  if (!gpu::AreNativeGpuMemoryBuffersEnabled())
-    return false;
-
 #if defined(OS_MACOSX)
   return true;
 #else

content/renderer/render_thread_impl_browsertest.cc

@@ -48,7 +48,6 @@
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gpu_memory_buffer_manager.h"
 #include "gpu/config/gpu_switches.h"
-#include "gpu/ipc/host/gpu_switches.h"
 #include "ipc/ipc.mojom.h"
 #include "ipc/ipc_channel_mojo.h"
 #include "mojo/core/embedder/embedder.h"
@@ -62,6 +61,7 @@
 #include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/buffer_format_util.h"
+#include "ui/gfx/switches.h"
 
 // IPC messages for testing ----------------------------------------------------
 
@@ -448,13 +448,8 @@ class RenderThreadImplGpuMemoryBufferBrowserTest
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(switches::kSingleProcess);
     NativeBufferFlag native_buffer_flag = ::testing::get<0>(GetParam());
-    switch (native_buffer_flag) {
-      case kEnableNativeBuffers:
-        command_line->AppendSwitch(switches::kEnableNativeGpuMemoryBuffers);
-        break;
-      case kDisableNativeBuffers:
-        command_line->AppendSwitch(switches::kDisableNativeGpuMemoryBuffers);
-        break;
+    if (native_buffer_flag == kEnableNativeBuffers) {
+      command_line->AppendSwitch(switches::kEnableNativeGpuMemoryBuffers);
     }
   }
 

content/test/BUILD.gn

@@ -1065,6 +1065,7 @@ test("content_browsertests") {
     "//ui/events:test_support",
     "//ui/events/blink:blink",
     "//ui/gfx",
+    "//ui/gfx:gfx_switches",
     "//ui/gfx/geometry",
     "//ui/gl",
     "//ui/gl:test_support",

gpu/ipc/host/BUILD.gn

@@ -8,8 +8,6 @@ source_set("host") {
   sources = [
     "gpu_memory_buffer_support.cc",
     "gpu_memory_buffer_support.h",
-    "gpu_switches.cc",
-    "gpu_switches.h",
     "shader_disk_cache.cc",
     "shader_disk_cache.h",
   ]

gpu/ipc/host/gpu_memory_buffer_support.cc

@@ -4,83 +4,47 @@
 
 #include "gpu/ipc/host/gpu_memory_buffer_support.h"
 
-#include "base/command_line.h"
 #include "base/logging.h"
 #include "build/build_config.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
 #include "gpu/ipc/common/gpu_memory_buffer_support.h"
-#include "gpu/ipc/host/gpu_switches.h"
 #include "ui/gl/gl_bindings.h"
 
 namespace gpu {
 
-bool AreNativeGpuMemoryBuffersEnabled() {
-#if defined(OS_MACOSX) || defined(OS_FUCHSIA)
-  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kDisableNativeGpuMemoryBuffers);
-#else
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableNativeGpuMemoryBuffers);
-#endif
-}
-
 GpuMemoryBufferConfigurationSet GetNativeGpuMemoryBufferConfigurations(
     GpuMemoryBufferSupport* support) {
   GpuMemoryBufferConfigurationSet configurations;
 
 #if defined(USE_OZONE) || defined(OS_MACOSX) || defined(OS_WIN) || \
     defined(OS_ANDROID)
-  if (AreNativeGpuMemoryBuffersEnabled()) {
-    const gfx::BufferFormat kNativeFormats[] = {
-        gfx::BufferFormat::R_8,
-        gfx::BufferFormat::RG_88,
-        gfx::BufferFormat::R_16,
-        gfx::BufferFormat::BGR_565,
-        gfx::BufferFormat::RGBA_4444,
-        gfx::BufferFormat::RGBA_8888,
-        gfx::BufferFormat::BGRA_8888,
-        gfx::BufferFormat::BGRX_1010102,
-        gfx::BufferFormat::RGBX_1010102,
-        gfx::BufferFormat::RGBA_F16,
-        gfx::BufferFormat::UYVY_422,
-        gfx::BufferFormat::YVU_420,
-        gfx::BufferFormat::YUV_420_BIPLANAR};
-    const gfx::BufferUsage kNativeUsages[] = {
-        gfx::BufferUsage::GPU_READ,
-        gfx::BufferUsage::SCANOUT,
-        gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE,
-        gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
-        gfx::BufferUsage::SCANOUT_CPU_READ_WRITE,
-        gfx::BufferUsage::GPU_READ_CPU_READ_WRITE};
-    for (auto format : kNativeFormats) {
-      for (auto usage : kNativeUsages) {
-        if (support->IsNativeGpuMemoryBufferConfigurationSupported(format,
-                                                                   usage))
-          configurations.insert(std::make_pair(format, usage));
-      }
-    }
-  }
+  const gfx::BufferFormat kBufferFormats[] = {
+      gfx::BufferFormat::R_8,          gfx::BufferFormat::R_16,
+      gfx::BufferFormat::RG_88,        gfx::BufferFormat::BGR_565,
+      gfx::BufferFormat::RGBA_4444,    gfx::BufferFormat::RGBX_8888,
+      gfx::BufferFormat::RGBA_8888,    gfx::BufferFormat::BGRX_8888,
+      gfx::BufferFormat::BGRX_1010102, gfx::BufferFormat::RGBX_1010102,
+      gfx::BufferFormat::BGRA_8888,    gfx::BufferFormat::RGBA_F16,
+      gfx::BufferFormat::YVU_420,      gfx::BufferFormat::YUV_420_BIPLANAR,
+      gfx::BufferFormat::UYVY_422};
 
-  const gfx::BufferFormat kGPUReadWriteFormats[] = {
-      gfx::BufferFormat::BGR_565,   gfx::BufferFormat::RGBA_8888,
-      gfx::BufferFormat::RGBX_8888, gfx::BufferFormat::BGRA_8888,
-      gfx::BufferFormat::BGRX_8888, gfx::BufferFormat::UYVY_422,
-      gfx::BufferFormat::YVU_420,   gfx::BufferFormat::YUV_420_BIPLANAR,
-      gfx::BufferFormat::R_8};
-  const gfx::BufferUsage kGPUReadWriteUsages[] = {
+  const gfx::BufferUsage kUsages[] = {
       gfx::BufferUsage::GPU_READ,
       gfx::BufferUsage::SCANOUT,
       gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE,
       gfx::BufferUsage::CAMERA_AND_CPU_READ_WRITE,
       gfx::BufferUsage::SCANOUT_CPU_READ_WRITE,
-      gfx::BufferUsage::SCANOUT_VDA_WRITE};
-  for (auto format : kGPUReadWriteFormats) {
-    for (auto usage : kGPUReadWriteUsages) {
+      gfx::BufferUsage::SCANOUT_VDA_WRITE,
+      gfx::BufferUsage::GPU_READ_CPU_READ_WRITE};
+
+  for (auto format : kBufferFormats) {
+    for (auto usage : kUsages) {
       if (support->IsNativeGpuMemoryBufferConfigurationSupported(format, usage))
         configurations.insert(std::make_pair(format, usage));
     }
   }
-#endif  // defined(USE_OZONE) || defined(OS_MACOSX) || defined(OS_WIN)
+#endif  // defined(USE_OZONE) || defined(OS_MACOSX) || defined(OS_WIN) ||
+        // defined(OS_ANDROID)
 
   return configurations;
 }

gpu/ipc/host/gpu_memory_buffer_support.h

@@ -38,8 +38,6 @@ namespace gpu {
 
 class GpuMemoryBufferSupport;
 
-bool AreNativeGpuMemoryBuffersEnabled();
-
 // Returns the set of supported configurations.
 GpuMemoryBufferConfigurationSet GetNativeGpuMemoryBufferConfigurations(
     GpuMemoryBufferSupport* support);

gpu/ipc/host/gpu_switches.cc

-// Copyright 2016 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "gpu/ipc/host/gpu_switches.h"
-
-namespace switches {
-
-// Enable native GPU memory buffer support when available.
-const char kEnableNativeGpuMemoryBuffers[] = "enable-native-gpu-memory-buffers";
-
-// Disables native GPU memory buffer support.
-const char kDisableNativeGpuMemoryBuffers[] =
-    "disable-native-gpu-memory-buffers";
-
-}  // namespace switches

gpu/ipc/host/gpu_switches.h

-// Copyright 2016 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Defines all the command-line switches used by gpu/ipc/host
-
-#ifndef GPU_IPC_HOST_GPU_SWITCHES_H_
-#define GPU_IPC_HOST_GPU_SWITCHES_H_
-
-namespace switches {
-
-extern const char kEnableNativeGpuMemoryBuffers[];
-extern const char kDisableNativeGpuMemoryBuffers[];
-
-}  // namespace switches
-
-#endif  // GPU_IPC_HOST_GPU_SWITCHES_H_

ui/gfx/BUILD.gn

@@ -528,6 +528,7 @@ jumbo_source_set("memory_buffer_sources") {
   ]
 
   deps = [
+    ":gfx_switches",
     ":native_widget_types",
     "//base",
     "//ui/gfx/geometry",

ui/gfx/linux/client_native_pixmap_dmabuf.cc

@@ -13,6 +13,7 @@
 
 #include <utility>
 
+#include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/posix/eintr_wrapper.h"
@@ -21,6 +22,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
+#include "ui/gfx/switches.h"
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 11, 0)
 #include <linux/dma-buf.h>
@@ -61,6 +63,12 @@ void PrimeSyncEnd(int dmabuf_fd) {
   PLOG_IF(ERROR, rv) << "Failed DMA_BUF_SYNC_END";
 }
 
+bool AllowCpuMappableBuffers() {
+  static bool result = base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableNativeGpuMemoryBuffers);
+  return result;
+}
+
 }  // namespace
 
 ClientNativePixmapDmaBuf::PlaneInfo::PlaneInfo() {}
@@ -106,10 +114,16 @@ bool ClientNativePixmapDmaBuf::IsConfigurationSupported(
              format == gfx::BufferFormat::RGBA_8888 ||
              format == gfx::BufferFormat::BGRA_8888;
     case gfx::BufferUsage::SCANOUT_CPU_READ_WRITE:
+      // TODO(crbug.com/954233): RG_88 is enabled only with
+      // --enable-native-gpu-memory-buffers . Otherwise it breaks some telemetry
+      // tests. Fix that issue and enable it again.
+      if (format == gfx::BufferFormat::RG_88 && !AllowCpuMappableBuffers())
+        return false;
+
       return
 #if defined(ARCH_CPU_X86_FAMILY)
-          // Currently only Intel driver (i.e. minigbm and Mesa) supports R_8
-          // RG_88, NV12 and XB30/XR30. https://crbug.com/356871
+          // Currently only Intel driver (i.e. minigbm and Mesa) supports
+          // R_8 RG_88, NV12 and XB30/XR30.
           format == gfx::BufferFormat::R_8 ||
           format == gfx::BufferFormat::RG_88 ||
           format == gfx::BufferFormat::YUV_420_BIPLANAR ||
@@ -125,11 +139,12 @@ bool ClientNativePixmapDmaBuf::IsConfigurationSupported(
       return false;
 
     case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE:
+      if (!AllowCpuMappableBuffers())
+        return false;
       return
 #if defined(ARCH_CPU_X86_FAMILY)
-          // Currently only Intel driver (i.e. minigbm and
-          // Mesa) supports R_8 RG_88 and NV12.
-          // https://crbug.com/356871
+          // Currently only Intel driver (i.e. minigbm and Mesa) supports R_8,
+          // RG_88 and NV12.
           format == gfx::BufferFormat::R_8 ||
           format == gfx::BufferFormat::RG_88 ||
           format == gfx::BufferFormat::YUV_420_BIPLANAR ||

ui/gfx/switches.cc

@@ -15,6 +15,9 @@ const char kDisableFontSubpixelPositioning[] =
 // Run in headless mode, i.e., without a UI or display server dependencies.
 const char kHeadless[] = "headless";
 
+// Enable native CPU-mappable GPU memory buffer support on Linux.
+const char kEnableNativeGpuMemoryBuffers[] = "enable-native-gpu-memory-buffers";
+
 }  // namespace switches
 
 namespace features {

ui/gfx/switches.h

@@ -15,6 +15,8 @@ GFX_SWITCHES_EXPORT extern const char kDisableFontSubpixelPositioning[];
 
 GFX_SWITCHES_EXPORT extern const char kHeadless[];
 
+GFX_SWITCHES_EXPORT extern const char kEnableNativeGpuMemoryBuffers[];
+
 }  // namespace switches
 
 namespace features {