chore: cherry-pick 4deb522c22 and d88e959e01 from chromium (#33852)

* chore: cherry-pick d88e959e01 from chromium * chore: update patches * chore: update patches * chore: update patches * chore: update patches * chore: update patches * chore: update patches * chore: update patches * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Electron Bot <electron@github.com>
electron · Apr 29, 2022 · cac38cb · cac38cb
1 parent e8f89da
commit cac38cb
Show file tree

Hide file tree

Showing 3 changed files with 352 additions and 0 deletions.
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
@@ -169,5 +169,7 @@ follow_wayland_specification_regarding_xkb_keymap.patch
 use-after-free_of_id_and_idref_attributes.patch
 fix_--without-valid_build.patch
 usb_fix_oob_access_with_non-sequential_interfaces.patch
+skia_renderer_-_don_t_explicitly_clip_scissor_for_large_transforms.patch
+skia_renderer_use_rectf_intersect_in_applyscissor.patch
 cherry-pick-1a31e2110440.patch
 m100_change_ownership_of_blobbytesprovider.patch
diff --git a/patches/chromium/skia_renderer_-_don_t_explicitly_clip_scissor_for_large_transforms.patch b/patches/chromium/skia_renderer_-_don_t_explicitly_clip_scissor_for_large_transforms.patch
@@ -0,0 +1,271 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Michael Ludwig <michaelludwig@google.com>
+Date: Tue, 7 Dec 2021 20:49:07 +0000
+Subject: - Don't explicitly clip scissor for large transforms
+
+This adds a check to CanExplicitlyScissor that confirms that the device
+space scissor rect, transformed to the quad's local space, can be
+transformed back to device space and equal the same pixel bounds.
+
+Without this check, sufficiently large scales and translates could
+cause the local-space coordinates of the scissor rect to be in a float
+range that does not have single-pixel precision, meaning it could round
+significantly. Clipping the quad's coordinates to those rounded edges
+and then transforming to device space can result in coordinates that
+fall outside the original device-space scissor rect.
+
+If however, we ensure we can round-trip the scissor coordinates, then
+any clipping to the quad's coordinates will also be projected to within
+the scissor rect as well.
+
+(cherry picked from commit ab1b76f3e7cdad702c562f0b43bf3367caff4812)
+
+Bug: 1272250
+Change-Id: I7c37c54efd082723797ccf32b5d19ef285c520c1
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3306893
+Commit-Queue: Michael Ludwig <michaelludwig@google.com>
+Reviewed-by: Brian Salomon <bsalomon@google.com>
+Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
+Cr-Original-Commit-Position: refs/heads/main@{#946552}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3320870
+Auto-Submit: Michael Ludwig <michaelludwig@google.com>
+Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
+Cr-Commit-Position: refs/branch-heads/4692@{#786}
+Cr-Branched-From: 038cd96142d384c0d2238973f1cb277725a62eba-refs/heads/main@{#938553}
+
+diff --git a/components/viz/service/display/skia_renderer.cc b/components/viz/service/display/skia_renderer.cc
+index 67d8f6b28a4edee1764660ede8eb48581c2b897e..054b18016922520a5545de64fb1243888117dd9c 100644
+--- a/components/viz/service/display/skia_renderer.cc
++++ b/components/viz/service/display/skia_renderer.cc
+@@ -125,45 +125,6 @@ bool IsTextureResource(DisplayResourceProviderSkia* resource_provider,
+   return !resource_provider->IsResourceSoftwareBacked(resource_id);
+ }
+
+-void ApplyExplicitScissor(const DrawQuad* quad,
+-                          const gfx::Rect& scissor_rect,
+-                          const gfx::Transform& device_transform,
+-                          unsigned* aa_flags,
+-                          gfx::RectF* vis_rect) {
+-  // Inset rectangular edges and turn off the AA for clipped edges. Operates in
+-  // the quad's space, so apply inverse of transform to get new scissor
+-  gfx::RectF scissor(scissor_rect);
+-  device_transform.TransformRectReverse(&scissor);
+-
+-  float left_inset = scissor.x() - vis_rect->x();
+-  float top_inset = scissor.y() - vis_rect->y();
+-  float right_inset = vis_rect->right() - scissor.right();
+-  float bottom_inset = vis_rect->bottom() - scissor.bottom();
+-
+-  if (left_inset >= kAAEpsilon) {
+-    *aa_flags &= ~SkCanvas::kLeft_QuadAAFlag;
+-  } else {
+-    left_inset = 0;
+-  }
+-  if (top_inset >= kAAEpsilon) {
+-    *aa_flags &= ~SkCanvas::kTop_QuadAAFlag;
+-  } else {
+-    top_inset = 0;
+-  }
+-  if (right_inset >= kAAEpsilon) {
+-    *aa_flags &= ~SkCanvas::kRight_QuadAAFlag;
+-  } else {
+-    right_inset = 0;
+-  }
+-  if (bottom_inset >= kAAEpsilon) {
+-    *aa_flags &= ~SkCanvas::kBottom_QuadAAFlag;
+-  } else {
+-    bottom_inset = 0;
+-  }
+-
+-  vis_rect->Inset(left_inset, top_inset, right_inset, bottom_inset);
+-}
+-
+ unsigned GetCornerAAFlags(const DrawQuad* quad,
+                           const SkPoint& vertex,
+                           unsigned edge_mask) {
+@@ -556,6 +517,10 @@ struct SkiaRenderer::DrawQuadParams {
+     p.setAntiAlias(aa_flags != SkCanvas::kNone_QuadAAFlags);
+     return p;
+   }
++
++  void ApplyScissor(const SkiaRenderer* renderer,
++                    const DrawQuad* quad,
++                    const gfx::Rect* scissor_to_apply);
+ };
+
+ SkiaRenderer::DrawQuadParams::DrawQuadParams(const gfx::Transform& cdt,
+@@ -1324,18 +1289,7 @@ SkiaRenderer::DrawQuadParams SkiaRenderer::CalculateDrawQuadParams(
+     params.opacity = 1.f;
+   }
+
+-  // Applying the scissor explicitly means avoiding a clipRect() call and
+-  // allows more quads to be batched together in a DrawEdgeAAImageSet call
+-  if (scissor_rect) {
+-    if (CanExplicitlyScissor(quad, draw_region, params.content_device_transform,
+-                             *scissor_rect)) {
+-      ApplyExplicitScissor(quad, *scissor_rect, params.content_device_transform,
+-                           &params.aa_flags, &params.visible_rect);
+-      params.vis_tex_coords = params.visible_rect;
+-    } else {
+-      params.scissor_rect = *scissor_rect;
+-    }
+-  }
++  params.ApplyScissor(this, quad, scissor_rect);
+
+   // Determine final rounded rect clip geometry. We transform it from target
+   // space to window space to make batching and canvas preparation easier
+@@ -1365,28 +1319,40 @@ SkiaRenderer::DrawQuadParams SkiaRenderer::CalculateDrawQuadParams(
+   return params;
+ }
+
+-bool SkiaRenderer::CanExplicitlyScissor(
++void SkiaRenderer::DrawQuadParams::ApplyScissor(
++    const SkiaRenderer* renderer,
+     const DrawQuad* quad,
+-    const gfx::QuadF* draw_region,
+-    const gfx::Transform& contents_device_transform,
+-    const gfx::Rect& scissor_rect) const {
++    const gfx::Rect* scissor_to_apply) {
++  // No scissor should have been set before calling ApplyScissor
++  DCHECK(!scissor_rect.has_value());
++
++  if (!scissor_to_apply) {
++    // No scissor at all, which matches the DCHECK'ed state above
++    return;
++  }
++
++  // Assume at start that the scissor will be applied through the canvas clip,
++  // so that this can simply return when it detects the scissor cannot be
++  // applied explicitly to |visible_rect|.
++  scissor_rect = *scissor_to_apply;
++
+   // PICTURE_CONTENT is not like the others, since it is executing a list of
+   // draw calls into the canvas.
+   if (quad->material == DrawQuad::Material::kPictureContent)
+-    return false;
++    return;
+   // Intersection with scissor and a quadrilateral is not necessarily a quad,
+   // so don't complicate things
+-  if (draw_region)
+-    return false;
++  if (draw_region.has_value())
++    return;
+
+   // This is slightly different than
+   // gfx::Transform::IsPositiveScaleAndTranslation in that it also allows zero
+   // scales. This is because in the common orthographic case the z scale is 0.
+-  if (!contents_device_transform.IsScaleOrTranslation() ||
+-      contents_device_transform.matrix().get(0, 0) < 0.0f ||
+-      contents_device_transform.matrix().get(1, 1) < 0.0f ||
+-      contents_device_transform.matrix().get(2, 2) < 0.0f) {
+-    return false;
++  if (!content_device_transform.IsScaleOrTranslation() ||
++      content_device_transform.matrix().get(0, 0) < 0.0f ||
++      content_device_transform.matrix().get(1, 1) < 0.0f ||
++      content_device_transform.matrix().get(2, 2) < 0.0f) {
++    return;
+   }
+
+   // State check: should not have a CompositorRenderPassDrawQuad if we got here.
+@@ -1396,22 +1362,69 @@ bool SkiaRenderer::CanExplicitlyScissor(
+     // geometry beyond the quad's visible_rect, so it's not safe to pre-clip.
+     auto pass_id =
+         AggregatedRenderPassDrawQuad::MaterialCast(quad)->render_pass_id;
+-    if (FiltersForPass(pass_id) || BackdropFiltersForPass(pass_id))
+-      return false;
++    if (renderer->FiltersForPass(pass_id) ||
++        renderer->BackdropFiltersForPass(pass_id))
++      return;
+   }
+
+   // If the intersection of the scissor and the quad's visible_rect results in
+   // subpixel device-space geometry, do not drop the scissor. Otherwise Skia
+   // sees an unclipped anti-aliased hairline and uses different AA methods that
+   // would cause the rasterized result to extend beyond the scissor.
+-  gfx::RectF device_bounds(quad->visible_rect);
+-  contents_device_transform.TransformRect(&device_bounds);
+-  device_bounds.Intersect(gfx::RectF(scissor_rect));
++  gfx::RectF device_bounds(visible_rect);
++  content_device_transform.TransformRect(&device_bounds);
++  device_bounds.Intersect(gfx::RectF(*scissor_rect));
+   if (device_bounds.width() < 1.0f || device_bounds.height() < 1.0f) {
+-    return false;
++    return;
++  }
++
++  // The explicit scissor is applied in the quad's local space. If the transform
++  // does not leave sufficient precision to round-trip the scissor rect to-from
++  // device->local->device space, the explicitly "clipped" geometry does not
++  // necessarily respect the original scissor.
++  gfx::RectF local_scissor(*scissor_rect);
++  content_device_transform.TransformRectReverse(&local_scissor);
++  gfx::RectF remapped_scissor(local_scissor);
++  content_device_transform.TransformRect(&remapped_scissor);
++  if (gfx::ToRoundedRect(remapped_scissor) != *scissor_rect) {
++    return;
++  }
++
++  // At this point, we've determined that we can transform the scissor rect into
++  // the quad's local space and adjust |vis_rect|, such that when it's mapped to
++  // device space, it will be contained in in the original scissor.
++  // Applying the scissor explicitly means avoiding a clipRect() call and
++  // allows more quads to be batched together in a DrawEdgeAAImageSet call
++  float left_inset = local_scissor.x() - visible_rect.x();
++  float top_inset = local_scissor.y() - visible_rect.y();
++  float right_inset = visible_rect.right() - local_scissor.right();
++  float bottom_inset = visible_rect.bottom() - local_scissor.bottom();
++
++  // The scissor is a non-AA clip, so we unset the bit flag for clipped edges.
++  if (left_inset >= kAAEpsilon) {
++    aa_flags &= ~SkCanvas::kLeft_QuadAAFlag;
++  } else {
++    left_inset = 0;
++  }
++  if (top_inset >= kAAEpsilon) {
++    aa_flags &= ~SkCanvas::kTop_QuadAAFlag;
++  } else {
++    top_inset = 0;
++  }
++  if (right_inset >= kAAEpsilon) {
++    aa_flags &= ~SkCanvas::kRight_QuadAAFlag;
++  } else {
++    right_inset = 0;
++  }
++  if (bottom_inset >= kAAEpsilon) {
++    aa_flags &= ~SkCanvas::kBottom_QuadAAFlag;
++  } else {
++    bottom_inset = 0;
+   }
+
+-  return true;
++  visible_rect.Inset(left_inset, top_inset, right_inset, bottom_inset);
++  vis_tex_coords = visible_rect;
++  scissor_rect.reset();
+ }
+
+ const DrawQuad* SkiaRenderer::CanPassBeDrawnDirectly(
+diff --git a/components/viz/service/display/skia_renderer.h b/components/viz/service/display/skia_renderer.h
+index b94ffe0bae845928db4f3dc05d71fef72e4b3523..933c63fa4625797eb00ec256fcb7abc41a4c88ad 100644
+--- a/components/viz/service/display/skia_renderer.h
++++ b/components/viz/service/display/skia_renderer.h
+@@ -139,6 +139,7 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
+                                          const gfx::Rect* scissor_rect,
+                                          const DrawQuad* quad,
+                                          const gfx::QuadF* draw_region) const;
++
+   DrawRPDQParams CalculateRPDQParams(const AggregatedRenderPassDrawQuad* quad,
+                                      DrawQuadParams* params);
+   // Modifies |params| and |rpdq_params| to apply correctly when drawing the
+@@ -156,12 +157,6 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
+       const SkImage* image,
+       const gfx::RectF& valid_texel_bounds,
+       DrawQuadParams* params) const;
+-  // True or false if the DrawQuad can have the scissor rect applied by
+-  // modifying the quad's visible_rect instead of as a separate clip operation.
+-  bool CanExplicitlyScissor(const DrawQuad* quad,
+-                            const gfx::QuadF* draw_region,
+-                            const gfx::Transform& contents_device_transform,
+-                            const gfx::Rect& scissor_rect) const;
+
+   bool MustFlushBatchedQuads(const DrawQuad* new_quad,
+                              const DrawRPDQParams* rpdq_params,
diff --git a/patches/chromium/skia_renderer_use_rectf_intersect_in_applyscissor.patch b/patches/chromium/skia_renderer_use_rectf_intersect_in_applyscissor.patch
@@ -0,0 +1,79 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Michael Ludwig <michaelludwig@google.com>
+Date: Sat, 2 Apr 2022 01:05:15 +0000
+Subject: Use RectF::Intersect in ApplyScissor
+
+(cherry picked from commit 540e2ecde447b0757dd5bb079a59d8faef3183c1)
+
+Bug: 1299287, 1307317
+Change-Id: I026090466ebfb3dee0e9daf0609f04babcf42092
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3516507
+Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
+Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
+Commit-Queue: Michael Ludwig <michaelludwig@google.com>
+Cr-Original-Commit-Position: refs/heads/main@{#982400}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564640
+Cr-Commit-Position: refs/branch-heads/4896@{#1017}
+Cr-Branched-From: 1f63ff4bc27570761b35ffbc7f938f6586f7bee8-refs/heads/main@{#972766}
+
+diff --git a/components/viz/service/display/skia_renderer.cc b/components/viz/service/display/skia_renderer.cc
+index 054b18016922520a5545de64fb1243888117dd9c..07b45ac958ef609ad8a103240e3c06edb0eaefc6 100644
+--- a/components/viz/service/display/skia_renderer.cc
++++ b/components/viz/service/display/skia_renderer.cc
+@@ -1395,34 +1395,20 @@ void SkiaRenderer::DrawQuadParams::ApplyScissor(
+   // device space, it will be contained in in the original scissor.
+   // Applying the scissor explicitly means avoiding a clipRect() call and
+   // allows more quads to be batched together in a DrawEdgeAAImageSet call
+-  float left_inset = local_scissor.x() - visible_rect.x();
+-  float top_inset = local_scissor.y() - visible_rect.y();
+-  float right_inset = visible_rect.right() - local_scissor.right();
+-  float bottom_inset = visible_rect.bottom() - local_scissor.bottom();
++  float x_epsilon = kAAEpsilon / content_device_transform.matrix().get(0, 0);
++  float y_epsilon = kAAEpsilon / content_device_transform.matrix().get(1, 1);
+
+-  // The scissor is a non-AA clip, so we unset the bit flag for clipped edges.
+-  if (left_inset >= kAAEpsilon) {
++  // The scissor is a non-AA clip, so unset the bit flag for clipped edges.
++  if (local_scissor.x() - visible_rect.x() >= x_epsilon)
+     aa_flags &= ~SkCanvas::kLeft_QuadAAFlag;
+-  } else {
+-    left_inset = 0;
+-  }
+-  if (top_inset >= kAAEpsilon) {
++  if (local_scissor.y() - visible_rect.y() >= y_epsilon)
+     aa_flags &= ~SkCanvas::kTop_QuadAAFlag;
+-  } else {
+-    top_inset = 0;
+-  }
+-  if (right_inset >= kAAEpsilon) {
++  if (visible_rect.right() - local_scissor.right() >= x_epsilon)
+     aa_flags &= ~SkCanvas::kRight_QuadAAFlag;
+-  } else {
+-    right_inset = 0;
+-  }
+-  if (bottom_inset >= kAAEpsilon) {
++  if (visible_rect.bottom() - local_scissor.bottom() >= y_epsilon)
+     aa_flags &= ~SkCanvas::kBottom_QuadAAFlag;
+-  } else {
+-    bottom_inset = 0;
+-  }
+
+-  visible_rect.Inset(left_inset, top_inset, right_inset, bottom_inset);
++  visible_rect.Intersect(local_scissor);
+   vis_tex_coords = visible_rect;
+   scissor_rect.reset();
+ }
+diff --git a/content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt b/content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
+index 37559380e8a3a8f857100bea9e1a5f6e53ecdc08..5f3e220642451609fd587c98475d6a3b8f347e0c 100644
+--- a/content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
++++ b/content/test/gpu/gpu_tests/test_expectations/pixel_expectations.txt
+@@ -369,6 +369,9 @@ crbug.com/1086687 [ chromeos chromeos-board-kevin ] Pixel_PrecisionRoundedCorner
+ crbug.com/1218288 [ fuchsia-board-astro ] Pixel_PrecisionRoundedCorner [ Skip ]
+ crbug.com/1136875 [ fuchsia-board-qemu-x64 ] Pixel_PrecisionRoundedCorner [ RetryOnFailure ]
+
++# Fails on Nexus 5X.
++crbug.com/1307317 [ android android-chromium android-nexus-5x ] Pixel_PrecisionRoundedCorner [ Failure ]
++
+ # Still fails on Nexus 5 after all other Pixel_CanvasLowLatency* pass.
+ crbug.com/1097752 [ android android-nexus-5 ] Pixel_CanvasLowLatencyWebGLAlphaFalse [ Skip ]
+