ozone/wayland: provide default buffer format for image through display

DisplayRenderer uses DisplayColorSpace to get suitable output
buffer format for Reshape(). However, Wayland didn't set that
and default value (RBGA_8888) was used instead. This didn't
work well as Wayland may have crashed because this specific
buffer format wasn't supported.

My observation showed that Chromium supports either BGRA_8888 or
RGBA_8888 as default buffer formats. Thus, figure out, which
of them is supported and use that when creating Display.

This fixes a crash that results in the following error -
wl_display@1.error(zwp_linux_buffer_params_v1@16, 7, "failed to import supplied dmabufs: Unsupported buffer format 875708993")

Bug: 1123382, 1121912
Change-Id: I7b3745726143f9c71db5bc8942be9933e67b7a96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401041
Commit-Queue: Maksim Sisov (GMT+3) <msisov@igalia.com>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806157}

ui/ozone/platform/wayland/host/wayland_screen.cc

@@ -12,9 +12,12 @@
 #include "ui/display/display_finder.h"
 #include "ui/display/display_list.h"
 #include "ui/display/display_observer.h"
+#include "ui/gfx/buffer_types.h"
+#include "ui/gfx/display_color_spaces.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/ozone/platform/wayland/host/wayland_buffer_manager_host.h"
 #include "ui/ozone/platform/wayland/host/wayland_connection.h"
 #include "ui/ozone/platform/wayland/host/wayland_cursor_position.h"
 #include "ui/ozone/platform/wayland/host/wayland_window.h"
@@ -24,6 +27,37 @@ namespace ui {
 WaylandScreen::WaylandScreen(WaylandConnection* connection)
     : connection_(connection), weak_factory_(this) {
   DCHECK(connection_);
+
+  // Chromium specifies either RGBA_8888 or BGRA_8888 as initial image format
+  // for alpha case and RGBX_8888 for no alpha case. Figure out
+  // which one is supported and use that. If RGBX_8888 is not supported, the
+  // format that |have_format_alpha| uses will be used by default (RGBA_8888 or
+  // BGRA_8888).
+  auto buffer_formats =
+      connection_->buffer_manager_host()->GetSupportedBufferFormats();
+  for (const auto& buffer_format : buffer_formats) {
+    auto format = buffer_format.first;
+    // RGBA_8888 is the preferred format.
+    if (format == gfx::BufferFormat::RGBA_8888)
+      image_format_alpha_ = gfx::BufferFormat::RGBA_8888;
+    if (!image_format_alpha_ && format == gfx::BufferFormat::BGRA_8888)
+      image_format_alpha_ = gfx::BufferFormat::BGRA_8888;
+
+    if (format == gfx::BufferFormat::RGBX_8888)
+      image_format_no_alpha_ = format;
+
+    if (image_format_alpha_ && image_format_no_alpha_)
+      break;
+  }
+
+  // If no buffer formats are found (neither wl_drm nor zwp_linux_dmabuf are
+  // supported or the system has very limited set of supported buffer formats),
+  // RGBA_8888 is used by default. On Wayland, that seems to be the most
+  // supported.
+  if (!image_format_alpha_)
+    image_format_alpha_ = gfx::BufferFormat::RGBA_8888;
+  if (!image_format_no_alpha_)
+    image_format_no_alpha_ = image_format_alpha_;
 }
 
 WaylandScreen::~WaylandScreen() = default;
@@ -61,12 +95,17 @@ void WaylandScreen::AddOrUpdateDisplay(uint32_t output_id,
   changed_display.set_bounds(new_bounds);
   changed_display.set_work_area(new_bounds);
 
+  gfx::DisplayColorSpaces color_spaces;
+  color_spaces.SetOutputBufferFormats(image_format_no_alpha_.value(),
+                                      image_format_alpha_.value());
+  changed_display.set_color_spaces(color_spaces);
+
   // There are 2 cases where |changed_display| must be set as primary:
   // 1. When it is the first one being added to the |display_list_|. Or
-  // 2. If it is nearest the origin than the previous primary or has the same
-  // origin as it. When an user, for example, swaps two side-by-side displays,
-  // at some point, as the notification come in, both will have the same
-  // origin.
+  // 2. If it is nearest the origin than the previous primary or has the
+  // same origin as it. When an user, for example, swaps two side-by-side
+  // displays, at some point, as the notification come in, both will have
+  // the same origin.
   auto type = display::DisplayList::Type::NOT_PRIMARY;
   if (display_list_.displays().empty()) {
     type = display::DisplayList::Type::PRIMARY;
@@ -109,14 +148,14 @@ display::Display WaylandScreen::GetDisplayForAcceleratedWidget(
   const auto entered_outputs_ids = window->entered_outputs_ids();
   // Although spec says a surface receives enter/leave surface events on
   // create/move/resize actions, this might be called right after a window is
-  // created, but it has not been configured by a Wayland compositor and it has
-  // not received enter surface events yet. Another case is when a user switches
-  // between displays in a single output mode - Wayland may not send enter
-  // events immediately, which can result in empty container of entered ids
-  // (check comments in WaylandWindow::RemoveEnteredOutputId). In this case,
-  // it's also safe to return the primary display.
-  // A child window will most probably enter the same display than its parent
-  // so we return the parent's display if there is a parent.
+  // created, but it has not been configured by a Wayland compositor and it
+  // has not received enter surface events yet. Another case is when a user
+  // switches between displays in a single output mode - Wayland may not send
+  // enter events immediately, which can result in empty container of entered
+  // ids (check comments in WaylandWindow::RemoveEnteredOutputId). In this
+  // case, it's also safe to return the primary display. A child window will
+  // most probably enter the same display than its parent so we return the
+  // parent's display if there is a parent.
   if (entered_outputs_ids.empty()) {
     if (parent_window)
       return GetDisplayForAcceleratedWidget(parent_window->GetWidget());
@@ -125,9 +164,9 @@ display::Display WaylandScreen::GetDisplayForAcceleratedWidget(
 
   DCHECK(!display_list_.displays().empty());
 
-  // A widget can be located on two or more displays. It would be better if the
-  // most in DIP occupied display was returned, but it's impossible to do so in
-  // Wayland. Thus, return the one that was used the earliest.
+  // A widget can be located on two or more displays. It would be better if
+  // the most in DIP occupied display was returned, but it's impossible to do
+  // so in Wayland. Thus, return the one that was used the earliest.
   for (const auto& display : display_list_.displays()) {
     if (display.id() == *entered_outputs_ids.begin())
       return display;
@@ -140,13 +179,13 @@ display::Display WaylandScreen::GetDisplayForAcceleratedWidget(
 gfx::Point WaylandScreen::GetCursorScreenPoint() const {
   // Wayland does not provide either location of surfaces in global space
   // coordinate system or location of a pointer. Instead, only locations of
-  // mouse/touch events are known. Given that Chromium assumes top-level windows
-  // are located at origin, always provide a cursor point in regards to
-  // surfaces' location.
+  // mouse/touch events are known. Given that Chromium assumes top-level
+  // windows are located at origin, always provide a cursor point in regards
+  // to surfaces' location.
   //
-  // If a pointer is located in any of the existing wayland windows, return the
-  // last known cursor position. Otherwise, return such a point, which is not
-  // contained by any of the windows.
+  // If a pointer is located in any of the existing wayland windows, return
+  // the last known cursor position. Otherwise, return such a point, which is
+  // not contained by any of the windows.
   auto* cursor_position = connection_->wayland_cursor_position();
   if (connection_->wayland_window_manager()->GetCurrentFocusedWindow() &&
       cursor_position)

ui/ozone/platform/wayland/host/wayland_screen.h

@@ -10,7 +10,9 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "base/optional.h"
 #include "ui/display/display_list.h"
+#include "ui/gfx/buffer_types.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/ozone/public/platform_screen.h"
 
@@ -66,6 +68,9 @@ class WaylandScreen : public PlatformScreen {
 
   base::ObserverList<display::DisplayObserver> observers_;
 
+  base::Optional<gfx::BufferFormat> image_format_alpha_;
+  base::Optional<gfx::BufferFormat> image_format_no_alpha_;
+
   base::WeakPtrFactory<WaylandScreen> weak_factory_;
 };