ui/ozone/drm: don't use subpixel source crop

DRM supposedly supports subpixel source crop. However, according to
drm_plane_funcs.update_plane, devices which don't support that are free
to ignore the fractional part, and every device seems to do that as of
5.4. To deal with this, round source crops that are nearly integers and
reject overlay candidates that actually require subpixel source crop.

The rounding of nearly integer crops addresses an issue where floating
point imprecision in a quad's uv_crop could lead to a visible
difference depending on whether or not the quad was put in an overlay.
The visible difference arose because drivers truncate the fractional
part of the source crop, which could lead to a 1px discrepancy in the
source crop.

BUG=chromium:1099110

Change-Id: Ib20b16ef505574bd63502591db626a7e769f6e88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366397
Commit-Queue: David Stevens <stevensd@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801234}

ui/ozone/platform/drm/gpu/drm_overlay_manager.cc

@@ -149,6 +149,15 @@ bool DrmOverlayManager::CanHandleCandidate(
   if (!gfx::IsNearestRectWithinDistance(candidate.display_rect, 0.01f))
     return false;
 
+  // DRM supposedly supports subpixel source crop. However, according to
+  // drm_plane_funcs.update_plane, devices which don't support that are
+  // free to ignore the fractional part, and every device seems to do that as
+  // of 5.4. So reject candidates that require subpixel source crop.
+  gfx::RectF crop(candidate.crop_rect);
+  crop.Scale(candidate.buffer_size.width(), candidate.buffer_size.height());
+  if (!gfx::IsNearestRectWithinDistance(crop, 0.01f))
+    return false;
+
   if (candidate.is_clipped && !candidate.clip_rect.Contains(
                                   gfx::ToNearestRect(candidate.display_rect))) {
     return false;

ui/ozone/platform/drm/gpu/drm_overlay_validator_unittest.cc

@@ -30,9 +30,9 @@
 
 namespace {
 
-// Mode of size 6x4.
-const drmModeModeInfo kDefaultMode = {0, 6, 0, 0, 0, 0, 4,     0,
-                                      0, 0, 0, 0, 0, 0, {'\0'}};
+// Mode of size 12x8.
+const drmModeModeInfo kDefaultMode = {0, 12, 0, 0, 0, 0, 8,     0,
+                                      0, 0,  0, 0, 0, 0, {'\0'}};
 
 const gfx::AcceleratedWidget kDefaultWidgetHandle = 1;
 constexpr uint32_t kCrtcIdBase = 1;

ui/ozone/platform/drm/gpu/hardware_display_plane_manager.cc

@@ -12,17 +12,13 @@
 
 #include "base/logging.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/ozone/platform/drm/gpu/drm_device.h"
 #include "ui/ozone/platform/drm/gpu/drm_framebuffer.h"
 #include "ui/ozone/platform/drm/gpu/drm_gpu_util.h"
 #include "ui/ozone/platform/drm/gpu/hardware_display_plane.h"
 
 namespace ui {
-namespace {
-
-constexpr float kFixedPointScaleValue = 1 << 16;
-
-}  // namespace
 
 HardwareDisplayPlaneList::HardwareDisplayPlaneList() {
   atomic_property_set.reset(drmModeAtomicAlloc());
@@ -184,18 +180,14 @@ bool HardwareDisplayPlaneManager::AssignOverlayPlanes(
     gfx::Rect fixed_point_rect;
     if (hw_plane->type() != HardwareDisplayPlane::kDummy) {
       const gfx::Size& size = plane.buffer->size();
-      gfx::RectF crop_rect = plane.crop_rect;
-      crop_rect.Scale(size.width(), size.height());
-
-      // This returns a number in 16.16 fixed point, required by the DRM overlay
-      // APIs.
-      auto to_fixed_point = [](double v) -> uint32_t {
-        return v * kFixedPointScaleValue;
-      };
-      fixed_point_rect = gfx::Rect(to_fixed_point(crop_rect.x()),
-                                   to_fixed_point(crop_rect.y()),
-                                   to_fixed_point(crop_rect.width()),
-                                   to_fixed_point(crop_rect.height()));
+      gfx::RectF crop_rectf = plane.crop_rect;
+      crop_rectf.Scale(size.width(), size.height());
+      // DrmOverlayManager::CanHandleCandidate guarantees this is safe.
+      gfx::Rect crop_rect = gfx::ToNearestRect(crop_rectf);
+      // Convert to 16.16 fixed point required by the DRM overlay APIs.
+      fixed_point_rect =
+          gfx::Rect(crop_rect.x() << 16, crop_rect.y() << 16,
+                    crop_rect.width() << 16, crop_rect.height() << 16);
     }
 
     if (!SetPlaneData(plane_list, hw_plane, plane, crtc_id, fixed_point_rect)) {

ui/ozone/platform/drm/gpu/hardware_display_plane_manager_unittest.cc

@@ -1141,6 +1141,72 @@ TEST_F(HardwareDisplayPlaneManagerPlanesReadyTest,
   EXPECT_TRUE(callback_called);
 }
 
+TEST_P(HardwareDisplayPlaneManagerAtomicTest, OverlaySourceCrop) {
+  InitializeDrmState(/*crtc_count=*/1, /*planes_per_crtc=*/1);
+  fake_drm_->InitializeState(crtc_properties_, connector_properties_,
+                             plane_properties_, property_names_, use_atomic_);
+
+  {
+    ui::DrmOverlayPlaneList assigns;
+    assigns.push_back(ui::DrmOverlayPlane(fake_buffer_, 0));
+
+    fake_drm_->plane_manager()->BeginFrame(&state_);
+    EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+        &state_, assigns, crtc_properties_[0].id));
+
+    std::unique_ptr<gfx::GpuFence> out_fence;
+    scoped_refptr<ui::PageFlipRequest> page_flip_request =
+        base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+    EXPECT_TRUE(fake_drm_->plane_manager()->Commit(&state_, page_flip_request,
+                                                   &out_fence));
+
+    EXPECT_EQ(2u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_W"));
+    EXPECT_EQ(2u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_H"));
+  }
+
+  {
+    ui::DrmOverlayPlaneList assigns;
+    assigns.push_back(ui::DrmOverlayPlane(
+        fake_buffer_, 0, gfx::OverlayTransform::OVERLAY_TRANSFORM_NONE,
+        gfx::Rect(kDefaultBufferSize), gfx::RectF(0, 0, .5, 1), false,
+        nullptr));
+
+    fake_drm_->plane_manager()->BeginFrame(&state_);
+    EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+        &state_, assigns, crtc_properties_[0].id));
+
+    scoped_refptr<ui::PageFlipRequest> page_flip_request =
+        base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+    std::unique_ptr<gfx::GpuFence> out_fence;
+    EXPECT_TRUE(fake_drm_->plane_manager()->Commit(&state_, page_flip_request,
+                                                   &out_fence));
+
+    EXPECT_EQ(1u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_W"));
+    EXPECT_EQ(2u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_H"));
+  }
+
+  {
+    ui::DrmOverlayPlaneList assigns;
+    assigns.push_back(ui::DrmOverlayPlane(
+        fake_buffer_, 0, gfx::OverlayTransform::OVERLAY_TRANSFORM_NONE,
+        gfx::Rect(kDefaultBufferSize), gfx::RectF(0, 0, .999, .501), false,
+        nullptr));
+
+    fake_drm_->plane_manager()->BeginFrame(&state_);
+    EXPECT_TRUE(fake_drm_->plane_manager()->AssignOverlayPlanes(
+        &state_, assigns, crtc_properties_[0].id));
+
+    scoped_refptr<ui::PageFlipRequest> page_flip_request =
+        base::MakeRefCounted<ui::PageFlipRequest>(base::TimeDelta());
+    std::unique_ptr<gfx::GpuFence> out_fence;
+    EXPECT_TRUE(fake_drm_->plane_manager()->Commit(&state_, page_flip_request,
+                                                   &out_fence));
+
+    EXPECT_EQ(2u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_W"));
+    EXPECT_EQ(1u << 16, GetPlanePropertyValue(kPlaneOffset, "SRC_H"));
+  }
+}
+
 class HardwareDisplayPlaneAtomicMock : public ui::HardwareDisplayPlaneAtomic {
  public:
   HardwareDisplayPlaneAtomicMock() : ui::HardwareDisplayPlaneAtomic(1) {}