chromecast: Use native display ids for touchscreen association

The touch device manager is currently using the public display::Display
interface to get the id and geometry of the display. This almost works,
except that when we associate touchscreens with displays the display id
is always set to "1" instead of the real native display id.

It's always "1" because the public display interface uses logical ids
that have a different meaning than the internal ids. However, touchscreen
association requires the internal ids to work.

To fix this, do the touchscreen work inside CastDisplayConfigurator where
it has access to native display internals. Because this has to also work
with stub display configuration (null NativeDisplayDelegate), this also
moves the stub display configuration path into CastDisplayConfigurator as
ConfigureDisplayFromCommandLine().

Bug: b/65594145
Test: rbyers.github.io/paint.html

Change-Id: Ic27529ec681e91a22e2395acb9bcf4ac04eab8ff
Reviewed-on: https://chromium-review.googlesource.com/933062
Commit-Queue: Michael Spang <spang@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Sergey Volk <servolk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538886}

chromecast/browser/cast_browser_main_parts.cc

@@ -89,7 +89,6 @@
 // header, but is exported to allow injecting the overlay-composited
 // callback.
 #include "chromecast/browser/cast_display_configurator.h"
-#include "chromecast/browser/cast_touch_device_manager.h"
 #include "chromecast/graphics/cast_screen.h"
 #include "ui/display/screen.h"
 #include "ui/ozone/platform/cast/overlay_manager_cast.h"  // nogncheck
@@ -455,8 +454,6 @@ int CastBrowserMainParts::PreCreateThreads() {
   display::Screen::SetScreenInstance(cast_browser_process_->cast_screen());
   display_configurator_ = std::make_unique<CastDisplayConfigurator>(
       cast_browser_process_->cast_screen());
-  touch_device_manager_ = std::make_unique<CastTouchDeviceManager>(
-      cast_browser_process_->cast_screen());
 #endif  // defined(USE_AURA)
 
   content::ChildProcessSecurityPolicy::GetInstance()->RegisterWebSafeScheme(
@@ -616,7 +613,6 @@ void CastBrowserMainParts::PostMainMessageLoopRun() {
   window_manager_.reset();
 
   display_configurator_.reset();
-  touch_device_manager_.reset();
 
   cast_browser_process_->cast_service()->Finalize();
   cast_browser_process_->metrics_service_client()->Finalize();

chromecast/browser/cast_browser_main_parts.h

@@ -46,7 +46,6 @@ class VideoPlaneController;
 namespace shell {
 class CastBrowserProcess;
 class CastDisplayConfigurator;
-class CastTouchDeviceManager;
 class URLRequestContextFactory;
 
 class CastBrowserMainParts : public content::BrowserMainParts {
@@ -93,7 +92,6 @@ class CastBrowserMainParts : public content::BrowserMainParts {
   std::unique_ptr<CastWindowManager> window_manager_;
 #if defined(USE_AURA)
   std::unique_ptr<CastDisplayConfigurator> display_configurator_;
-  std::unique_ptr<CastTouchDeviceManager> touch_device_manager_;
 #endif
 
 #if BUILDFLAG(IS_CAST_USING_CMA_BACKEND)

chromecast/browser/cast_display_configurator.cc

@@ -5,8 +5,14 @@
 #include "chromecast/browser/cast_display_configurator.h"
 
 #include "base/bind.h"
+#include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
+#include "chromecast/base/cast_features.h"
+#include "chromecast/browser/cast_touch_device_manager.h"
 #include "chromecast/graphics/cast_screen.h"
+#include "chromecast/public/graphics_properties_shlib.h"
 #include "ui/display/types/display_snapshot.h"
 #include "ui/display/types/native_display_delegate.h"
 #include "ui/gfx/geometry/rect.h"
@@ -15,17 +21,70 @@
 namespace chromecast {
 namespace shell {
 
+namespace {
+constexpr int64_t kStubDisplayId = 1;
+constexpr char kCastGraphicsHeight[] = "cast-graphics-height";
+constexpr char kCastGraphicsWidth[] = "cast-graphics-width";
+constexpr char kDisplayRotation[] = "display-rotation";
+
+// Helper to return the screen resolution (device pixels)
+// to use.
+gfx::Size GetScreenResolution() {
+  gfx::Size res(1280, 720);
+  const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
+  if (!base::FeatureList::IsEnabled(kTripleBuffer720) &&
+      GraphicsPropertiesShlib::IsSupported(GraphicsPropertiesShlib::k1080p,
+                                           cmd_line->argv())) {
+    res = gfx::Size(1920, 1080);
+  }
+
+  int cast_gfx_width = 0;
+  int cast_gfx_height = 0;
+  if (base::StringToInt(cmd_line->GetSwitchValueASCII(kCastGraphicsWidth),
+                        &cast_gfx_width) &&
+      base::StringToInt(cmd_line->GetSwitchValueASCII(kCastGraphicsHeight),
+                        &cast_gfx_height) &&
+      cast_gfx_width > 0 && cast_gfx_height > 0) {
+    res.set_width(cast_gfx_width);
+    res.set_height(cast_gfx_height);
+  }
+  return res;
+}
+
+display::Display::Rotation GetRotationFromCommandLine() {
+  std::string rotation =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          kDisplayRotation);
+  if (rotation == "90")
+    return display::Display::ROTATE_90;
+  else if (rotation == "180")
+    return display::Display::ROTATE_180;
+  else if (rotation == "270")
+    return display::Display::ROTATE_270;
+  else
+    return display::Display::ROTATE_0;
+}
+
+float GetDeviceScaleFactor(gfx::Size display_resolution) {
+  // Device scale factor computed relative to 720p display
+  return display_resolution.height() / 720.0f;
+}
+
+}  // namespace
+
 CastDisplayConfigurator::CastDisplayConfigurator(CastScreen* screen)
     : delegate_(
           ui::OzonePlatform::GetInstance()->CreateNativeDisplayDelegate()),
+      touch_device_manager_(std::make_unique<CastTouchDeviceManager>()),
       cast_screen_(screen),
       weak_factory_(this) {
-  if (!delegate_)
-    return;
-  DCHECK(cast_screen_);
-  delegate_->AddObserver(this);
-  delegate_->Initialize();
-  OnConfigurationChanged();
+  if (delegate_) {
+    delegate_->AddObserver(this);
+    delegate_->Initialize();
+    OnConfigurationChanged();
+  } else {
+    ConfigureDisplayFromCommandLine();
+  }
 }
 
 CastDisplayConfigurator::~CastDisplayConfigurator() {
@@ -41,6 +100,12 @@ void CastDisplayConfigurator::OnConfigurationChanged() {
                  weak_factory_.GetWeakPtr()));
 }
 
+void CastDisplayConfigurator::ConfigureDisplayFromCommandLine() {
+  const gfx::Size size = GetScreenResolution();
+  UpdateScreen(kStubDisplayId, gfx::Rect(size), GetDeviceScaleFactor(size),
+               GetRotationFromCommandLine());
+}
+
 void CastDisplayConfigurator::OnDisplaysAcquired(
     const std::vector<display::DisplaySnapshot*>& displays) {
   DCHECK(delegate_);
@@ -84,11 +149,23 @@ void CastDisplayConfigurator::OnDisplayConfigured(
     display->set_current_mode(mode);
     display->set_origin(origin);
 
-    cast_screen_->OnDisplayChanged(1.0f, bounds);
+    UpdateScreen(display->display_id(), bounds,
+                 GetDeviceScaleFactor(display->native_mode()->size()),
+                 GetRotationFromCommandLine());
   } else {
     LOG(FATAL) << "Failed to configure display";
   }
 }
 
+void CastDisplayConfigurator::UpdateScreen(
+    int64_t display_id,
+    const gfx::Rect& bounds,
+    float device_scale_factor,
+    display::Display::Rotation rotation) {
+  cast_screen_->OnDisplayChanged(display_id, device_scale_factor, rotation,
+                                 bounds);
+  touch_device_manager_->OnDisplayConfigured(display_id, rotation, bounds);
+}
+
 }  // namespace shell
 }  // namespace chromecast

chromecast/browser/cast_display_configurator.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "ui/display/display.h"
 #include "ui/display/types/native_display_observer.h"
 
 namespace display {
@@ -26,6 +27,7 @@ namespace chromecast {
 class CastScreen;
 
 namespace shell {
+class CastTouchDeviceManager;
 
 // The CastDisplayConfigurator class ensures native displays are initialized and
 // configured properly on platforms that need that (e.g. GBM/DRM graphics via
@@ -42,6 +44,8 @@ class CastDisplayConfigurator : public display::NativeDisplayObserver {
   void OnConfigurationChanged() override;
   void OnDisplaySnapshotsInvalidated() override {}
 
+  void ConfigureDisplayFromCommandLine();
+
  private:
   void OnDisplaysAcquired(
       const std::vector<display::DisplaySnapshot*>& displays);
@@ -49,8 +53,13 @@ class CastDisplayConfigurator : public display::NativeDisplayObserver {
                            const display::DisplayMode* mode,
                            const gfx::Point& origin,
                            bool success);
+  void UpdateScreen(int64_t display_id,
+                    const gfx::Rect& bounds,
+                    float device_scale_factor,
+                    display::Display::Rotation rotation);
 
   std::unique_ptr<display::NativeDisplayDelegate> delegate_;
+  std::unique_ptr<CastTouchDeviceManager> touch_device_manager_;
   CastScreen* const cast_screen_;
 
   base::WeakPtrFactory<CastDisplayConfigurator> weak_factory_;

chromecast/browser/cast_touch_device_manager.cc

@@ -16,45 +16,42 @@ namespace {
 
 ui::TouchDeviceTransform GetDeviceTransform(
     const ui::TouchscreenDevice& touchscreen,
-    display::Display display) {
-  gfx::RectF display_bounds = gfx::RectF(gfx::Rect(display.GetSizeInPixel()));
+    int64_t display_id,
+    display::Display::Rotation rotation,
+    const gfx::Rect& native_bounds_in_pixel) {
   gfx::SizeF touchscreen_size = gfx::SizeF(touchscreen.size);
 
   ui::TouchDeviceTransform touch_device_transform;
-  touch_device_transform.display_id = display.id();
+  touch_device_transform.display_id = display_id;
   touch_device_transform.device_id = touchscreen.id;
-  touch_device_transform.transform.Translate(display_bounds.x(),
-                                             display_bounds.y());
+  touch_device_transform.transform.Translate(native_bounds_in_pixel.x(),
+                                             native_bounds_in_pixel.y());
 
   float touchscreen_width = touchscreen_size.width();
   float touchscreen_height = touchscreen_size.height();
 
   // If the display orientation is rotated between portrait and landscape,
   // the width and height of the touchscreen must be swapped as well.
-  if (display.rotation() == display::Display::Rotation::ROTATE_90 ||
-      display.rotation() == display::Display::Rotation::ROTATE_270) {
+  if (rotation == display::Display::Rotation::ROTATE_90 ||
+      rotation == display::Display::Rotation::ROTATE_270) {
     touchscreen_width = touchscreen_size.height();
     touchscreen_height = touchscreen_size.width();
   }
 
   touch_device_transform.transform.Scale(
-      display_bounds.width() / touchscreen_width,
-      display_bounds.height() / touchscreen_height);
+      native_bounds_in_pixel.width() / touchscreen_width,
+      native_bounds_in_pixel.height() / touchscreen_height);
 
   return touch_device_transform;
 }
 
 }  // namespace
 
-CastTouchDeviceManager::CastTouchDeviceManager(CastScreen* cast_screen)
-    : cast_screen_(cast_screen) {
+CastTouchDeviceManager::CastTouchDeviceManager() {
   ui::DeviceDataManager::GetInstance()->AddObserver(this);
-  cast_screen_->AddObserver(this);
-  UpdateTouchscreenConfiguration();
 }
 
 CastTouchDeviceManager::~CastTouchDeviceManager() {
-  cast_screen_->RemoveObserver(this);
   ui::DeviceDataManager::GetInstance()->RemoveObserver(this);
 }
 
@@ -62,32 +59,28 @@ void CastTouchDeviceManager::OnTouchscreenDeviceConfigurationChanged() {
   UpdateTouchscreenConfiguration();
 }
 
-void CastTouchDeviceManager::OnDisplayAdded(
-    const display::Display& new_display) {
-  UpdateTouchscreenConfiguration();
-}
-
-void CastTouchDeviceManager::OnDisplayRemoved(
-    const display::Display& old_display) {
-  UpdateTouchscreenConfiguration();
-}
-
-void CastTouchDeviceManager::OnDisplayMetricsChanged(
-    const display::Display& display,
-    uint32_t changed_metrics) {
+void CastTouchDeviceManager::OnDisplayConfigured(
+    int64_t display_id,
+    display::Display::Rotation rotation,
+    const gfx::Rect& native_bounds_in_pixel) {
+  display_id_ = display_id;
+  display_rotation_ = rotation;
+  native_display_bounds_in_pixel_ = native_bounds_in_pixel;
   UpdateTouchscreenConfiguration();
 }
 
 void CastTouchDeviceManager::UpdateTouchscreenConfiguration() {
-  display::Display primary_display = cast_screen_->GetPrimaryDisplay();
   const std::vector<ui::TouchscreenDevice>& touchscreen_devices =
       ui::DeviceDataManager::GetInstance()->GetTouchscreenDevices();
+  if (native_display_bounds_in_pixel_ == gfx::Rect())
+    return;  // Waiting for display configuration.
   std::vector<ui::TouchDeviceTransform> touch_device_transforms;
 
   // All touchscreens are mapped onto primary display.
   for (const auto& touchscreen : touchscreen_devices) {
     touch_device_transforms.push_back(
-        GetDeviceTransform(touchscreen, primary_display));
+        GetDeviceTransform(touchscreen, display_id_, display_rotation_,
+                           native_display_bounds_in_pixel_));
   }
 
   ui::DeviceDataManager::GetInstance()->ConfigureTouchDevices(

chromecast/browser/cast_touch_device_manager.h

@@ -5,34 +5,35 @@
 #ifndef CHROMECAST_BROWSER_CAST_TOUCH_DEVICE_MANAGER_H_
 #define CHROMECAST_BROWSER_CAST_TOUCH_DEVICE_MANAGER_H_
 
+#include <inttypes.h>
+
 #include "base/macros.h"
-#include "chromecast/graphics/cast_screen.h"
-#include "ui/display/display_observer.h"
+#include "ui/display/display.h"
 #include "ui/events/devices/input_device_event_observer.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace chromecast {
 namespace shell {
 
 // Manages touchscreen->display mapping for cast browser.
-class CastTouchDeviceManager : public display::DisplayObserver,
-                               public ui::InputDeviceEventObserver {
+class CastTouchDeviceManager : public ui::InputDeviceEventObserver {
  public:
-  explicit CastTouchDeviceManager(CastScreen* screen);
+  explicit CastTouchDeviceManager();
   ~CastTouchDeviceManager() override;
 
   // ui::InputDeviceEventObserver:
   void OnTouchscreenDeviceConfigurationChanged() override;
 
-  // display::DisplayObserver:
-  void OnDisplayAdded(const display::Display& new_display) override;
-  void OnDisplayRemoved(const display::Display& old_display) override;
-  void OnDisplayMetricsChanged(const display::Display& display,
-                               uint32_t changed_metrics) override;
+  void OnDisplayConfigured(int64_t display_id,
+                           display::Display::Rotation rotation,
+                           const gfx::Rect& native_bounds_in_pixel);
 
  private:
   void UpdateTouchscreenConfiguration();
 
-  CastScreen* cast_screen_ = nullptr;
+  int64_t display_id_;
+  display::Display::Rotation display_rotation_;
+  gfx::Rect native_display_bounds_in_pixel_;
 
   DISALLOW_COPY_AND_ASSIGN(CastTouchDeviceManager);
 };

chromecast/graphics/cast_screen.cc

@@ -6,66 +6,11 @@
 
 #include <stdint.h>
 
-#include "base/command_line.h"
-#include "base/feature_list.h"
-#include "base/strings/string_number_conversions.h"
-#include "chromecast/base/cast_features.h"
-#include "chromecast/public/graphics_properties_shlib.h"
 #include "ui/aura/env.h"
-#include "ui/gfx/geometry/rect_conversions.h"
-#include "ui/gfx/geometry/size.h"
-#include "ui/gfx/geometry/size_conversions.h"
-#include "ui/gfx/native_widget_types.h"
+#include "ui/display/display.h"
 
 namespace chromecast {
 
-namespace {
-
-const int64_t kDisplayId = 1;
-constexpr char kCastGraphicsHeight[] = "cast-graphics-height";
-constexpr char kCastGraphicsWidth[] = "cast-graphics-width";
-constexpr char kDisplayRotation[] = "display-rotation";
-
-// Helper to return the screen resolution (device pixels)
-// to use.
-gfx::Size GetScreenResolution() {
-  gfx::Size res(1280, 720);
-  const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
-  if (!base::FeatureList::IsEnabled(kTripleBuffer720) &&
-      GraphicsPropertiesShlib::IsSupported(GraphicsPropertiesShlib::k1080p,
-                                           cmd_line->argv())) {
-    res = gfx::Size(1920, 1080);
-  }
-
-  int cast_gfx_width = 0;
-  int cast_gfx_height = 0;
-  if (base::StringToInt(cmd_line->GetSwitchValueASCII(kCastGraphicsWidth),
-                        &cast_gfx_width) &&
-      base::StringToInt(cmd_line->GetSwitchValueASCII(kCastGraphicsHeight),
-                        &cast_gfx_height) &&
-      cast_gfx_width > 0 && cast_gfx_height > 0) {
-    res.set_width(cast_gfx_width);
-    res.set_height(cast_gfx_height);
-  }
-  return res;
-}
-
-display::Display::Rotation GetRotationFromCommandLine() {
-  std::string rotation =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
-          kDisplayRotation);
-  if (rotation == "90")
-    return display::Display::ROTATE_90;
-  else if (rotation == "180")
-    return display::Display::ROTATE_180;
-  else if (rotation == "270")
-    return display::Display::ROTATE_270;
-  else
-    return display::Display::ROTATE_0;
-}
-
-}  // namespace
-
 CastScreen::~CastScreen() {
 }
 
@@ -88,18 +33,16 @@ display::Display CastScreen::GetDisplayNearestWindow(
 }
 
 CastScreen::CastScreen() {
-  // Device scale factor computed relative to 720p display
-  const gfx::Size size = GetScreenResolution();
-  const float device_scale_factor = size.height() / 720.0f;
-  OnDisplayChanged(device_scale_factor, gfx::Rect(size));
 }
 
-void CastScreen::OnDisplayChanged(float device_scale_factor, gfx::Rect bounds) {
-  VLOG(1) << __func__ << " device_scale_factor=" << device_scale_factor
-          << " bounds=" << bounds.ToString();
-  display::Display display(kDisplayId);
+void CastScreen::OnDisplayChanged(int64_t display_id,
+                                  float device_scale_factor,
+                                  display::Display::Rotation rotation,
+                                  const gfx::Rect& bounds) {
+  display::Display display(display_id);
   display.SetScaleAndBounds(device_scale_factor, bounds);
-  display.set_rotation(GetRotationFromCommandLine());
+  display.set_rotation(rotation);
+  VLOG(1) << __func__ << " " << display.ToString();
   ProcessDisplayChanged(display, true /* is_primary */);
 }
 

chromecast/graphics/cast_screen.h

@@ -31,7 +31,10 @@ class CastScreen : public display::ScreenBase {
   display::Display GetDisplayNearestWindow(
       gfx::NativeWindow window) const override;
 
-  void OnDisplayChanged(float scale_factor, gfx::Rect bounds);
+  void OnDisplayChanged(int64_t display_id,
+                        float scale_factor,
+                        display::Display::Rotation rotation,
+                        const gfx::Rect& bounds);
 
  private:
   CastScreen();