canvas: small optimizations to draw path

This does the following: . Removes duplicate lookup of PaintCanvas. Draw() already looks up the paintcanvas, so no need for other places to also look it up. . Renames GetPaintCanvasForDraw to WillDraw() as by the time this is called we know there is a PaintCanvas. . Changes some rect types to gfx types as most places already take gfx types and we were needlessly converting between the two. Bug: none Change-Id: I25bf6d246c1659b0a7c7822bf98a3426c52f7a9b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4235062 Reviewed-by: Robert Flack <flackr@chromium.org> Reviewed-by: Justin Novosad <junov@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/main@{#1103870}
chromium · Feb 10, 2023 · c5daebf · c5daebf
1 parent 538b63d
commit c5daebf
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 100 deletions.
diff --git a/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc b/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
@@ -1044,16 +1044,13 @@ void BaseRenderingContext2D::DrawPathInternal(
   if (paint_type == CanvasRenderingContext2DState::kStrokePaintType)
     InflateStrokeRect(bounds);
 
-  if (!GetOrCreatePaintCanvas())
-    return;
-
   Draw<OverdrawOp::kNone>(
       [sk_path, use_paint_cache](cc::PaintCanvas* c,
                                  const cc::PaintFlags* flags)  // draw lambda
       { c->drawPath(sk_path, *flags, use_paint_cache); },
       [](const SkIRect& rect)  // overdraw test lambda
       { return false; },
-      gfx::RectFToSkRect(bounds), paint_type,
+      bounds, paint_type,
       GetState().HasPattern(paint_type)
           ? CanvasRenderingContext2DState::kNonOpaqueImage
           : CanvasRenderingContext2DState::kNoImage,
@@ -1123,13 +1120,6 @@ void BaseRenderingContext2D::fillRect(double x,
                                                 width, height);
   }
 
-  // clamp to float to avoid float cast overflow when used as SkScalar
-  AdjustRectForCanvas(x, y, width, height);
-  float fx = ClampTo<float>(x);
-  float fy = ClampTo<float>(y);
-  float fwidth = ClampTo<float>(width);
-  float fheight = ClampTo<float>(height);
-
   // We are assuming that if the pattern is not accelerated and the current
   // canvas is accelerated, the texture of the pattern will not be able to be
   // moved to the texture of the canvas receiving the pattern (because if the
@@ -1146,15 +1136,15 @@ void BaseRenderingContext2D::fillRect(double x,
         GPUFallbackToCPUScenario::kLargePatternDrawnToGPU);
   }
 
-  SkRect rect = SkRect::MakeXYWH(fx, fy, fwidth, fheight);
+  // clamp to float to avoid float cast overflow when used as SkScalar
+  AdjustRectForCanvas(x, y, width, height);
+  gfx::RectF rect(ClampTo<float>(x), ClampTo<float>(y), ClampTo<float>(width),
+                  ClampTo<float>(height));
   Draw<OverdrawOp::kNone>(
       [rect](cc::PaintCanvas* c, const cc::PaintFlags* flags)  // draw lambda
-      { c->drawRect(rect, *flags); },
+      { c->drawRect(gfx::RectFToSkRect(rect), *flags); },
       [rect, this](const SkIRect& clip_bounds)  // overdraw test lambda
-      {
-        return RectContainsTransformedRect(gfx::SkRectToRectF(rect),
-                                           clip_bounds);
-      },
+      { return RectContainsTransformedRect(rect, clip_bounds); },
       rect, CanvasRenderingContext2DState::kFillPaintType,
       GetState().HasPattern(CanvasRenderingContext2DState::kFillPaintType)
           ? CanvasRenderingContext2DState::kNonOpaqueImage
@@ -1210,8 +1200,7 @@ void BaseRenderingContext2D::strokeRect(double x,
   Draw<OverdrawOp::kNone>(
       [rect](cc::PaintCanvas* c, const cc::PaintFlags* flags)  // draw lambda
       { StrokeRectOnCanvas(rect, c, flags); },
-      kNoOverdraw, gfx::RectFToSkRect(bounds),
-      CanvasRenderingContext2DState::kStrokePaintType,
+      kNoOverdraw, bounds, CanvasRenderingContext2DState::kStrokePaintType,
       GetState().HasPattern(CanvasRenderingContext2DState::kStrokePaintType)
           ? CanvasRenderingContext2DState::kNonOpaqueImage
           : CanvasRenderingContext2DState::kNoImage,
@@ -1368,23 +1357,19 @@ void BaseRenderingContext2D::clearRect(double x,
     if (for_reset) {
       // In the reset case, we can use kUntransformedUnclippedFill because we
       // know the state state was reset.
-      CheckOverdraw(gfx::RectFToSkRect(rect), &clear_flags,
-                    CanvasRenderingContext2DState::kNoImage,
+      CheckOverdraw(&clear_flags, CanvasRenderingContext2DState::kNoImage,
                     OverdrawOp::kContextReset);
     } else {
-      CheckOverdraw(gfx::RectFToSkRect(rect), &clear_flags,
-                    CanvasRenderingContext2DState::kNoImage,
+      CheckOverdraw(&clear_flags, CanvasRenderingContext2DState::kNoImage,
                     OverdrawOp::kClearRect);
     }
-    GetPaintCanvasForDraw(clip_bounds,
-                          CanvasPerformanceMonitor::DrawType::kOther)
-        ->drawRect(gfx::RectFToSkRect(rect), clear_flags);
+    WillDraw(clip_bounds, CanvasPerformanceMonitor::DrawType::kOther);
+    c->drawRect(gfx::RectFToSkRect(rect), clear_flags);
   } else {
     SkIRect dirty_rect;
     if (ComputeDirtyRect(rect, clip_bounds, &dirty_rect)) {
-      GetPaintCanvasForDraw(clip_bounds,
-                            CanvasPerformanceMonitor::DrawType::kOther)
-          ->drawRect(gfx::RectFToSkRect(rect), clear_flags);
+      WillDraw(clip_bounds, CanvasPerformanceMonitor::DrawType::kOther);
+      c->drawRect(gfx::RectFToSkRect(rect), clear_flags);
     }
   }
 }
@@ -1726,19 +1711,12 @@ void BaseRenderingContext2D::drawImage(CanvasImageSource* image_source,
       },
       [this, dst_rect](const SkIRect& clip_bounds)  // overdraw test lambda
       { return RectContainsTransformedRect(dst_rect, clip_bounds); },
-      gfx::RectFToSkRect(dst_rect),
-      CanvasRenderingContext2DState::kImagePaintType,
+      dst_rect, CanvasRenderingContext2DState::kImagePaintType,
       image_source->IsOpaque() ? CanvasRenderingContext2DState::kOpaqueImage
                                : CanvasRenderingContext2DState::kNonOpaqueImage,
       CanvasPerformanceMonitor::DrawType::kImage);
 }
 
-void BaseRenderingContext2D::ClearCanvasForSrcCompositeOp() {
-  cc::PaintCanvas* c = GetOrCreatePaintCanvas();
-  if (c)
-    c->clear(HasAlpha() ? SkColors::kTransparent : SkColors::kBlack);
-}
-
 bool BaseRenderingContext2D::RectContainsTransformedRect(
     const gfx::RectF& rect,
     const SkIRect& transformed_rect) const {
@@ -2207,15 +2185,19 @@ void BaseRenderingContext2D::putImageData(ImageData* data,
         NOTREACHED() << "Failed to convert ImageData with writePixels.";
 
       PutByteArray(converted_bitmap.pixmap(), source_rect, dest_offset);
-      GetPaintCanvasForDraw(gfx::RectToSkIRect(dest_rect),
-                            CanvasPerformanceMonitor::DrawType::kImageData);
+      if (GetPaintCanvas()) {
+        WillDraw(gfx::RectToSkIRect(dest_rect),
+                 CanvasPerformanceMonitor::DrawType::kImageData);
+      }
       return;
     }
   }
 
   PutByteArray(data_pixmap, source_rect, dest_offset);
-  GetPaintCanvasForDraw(gfx::RectToSkIRect(dest_rect),
-                        CanvasPerformanceMonitor::DrawType::kImageData);
+  if (GetPaintCanvas()) {
+    WillDraw(gfx::RectToSkIRect(dest_rect),
+             CanvasPerformanceMonitor::DrawType::kImageData);
+  }
 }
 
 void BaseRenderingContext2D::PutByteArray(const SkPixmap& source,

diff --git a/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.h b/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.h
@@ -269,9 +269,11 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
     return const_cast<BaseRenderingContext2D*>(this)->GetPaintCanvas();
   }
   virtual cc::PaintCanvas* GetPaintCanvas() = 0;
-  virtual cc::PaintCanvas* GetPaintCanvasForDraw(
-      const SkIRect& dirty_rect,
-      CanvasPerformanceMonitor::DrawType) = 0;
+
+  // Called when about to draw. When this is called GetPaintCanvas() has already
+  // been called and returned a non-null value.
+  virtual void WillDraw(const SkIRect& dirty_rect,
+                        CanvasPerformanceMonitor::DrawType) = 0;
 
   virtual sk_sp<PaintFilter> StateGetFilter() = 0;
   virtual void SnapshotStateForFilter() = 0;
@@ -428,7 +430,7 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
             typename DrawCoversClipBoundsFunc>
   void Draw(const DrawFunc&,
             const DrawCoversClipBoundsFunc&,
-            const SkRect& bounds,
+            const gfx::RectF& bounds,
             CanvasRenderingContext2DState::PaintType,
             CanvasRenderingContext2DState::ImageType,
             CanvasPerformanceMonitor::DrawType);
@@ -454,8 +456,7 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
     return nullptr;
   }
 
-  void CheckOverdraw(const SkRect&,
-                     const cc::PaintFlags*,
+  void CheckOverdraw(const cc::PaintFlags*,
                      CanvasRenderingContext2DState::ImageType,
                      BaseRenderingContext2D::OverdrawOp overdraw_op);
 
@@ -597,12 +598,14 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
     return false;
   }
 
+  // `paint_canvas` is null if this function is called asynchronously.
   template <OverdrawOp CurrentOverdrawOp,
             typename DrawFunc,
             typename DrawCoversClipBoundsFunc>
-  void DrawInternal(const DrawFunc&,
+  void DrawInternal(cc::PaintCanvas* paint_canvas,
+                    const DrawFunc&,
                     const DrawCoversClipBoundsFunc&,
-                    const SkRect& bounds,
+                    const gfx::RectF& bounds,
                     CanvasRenderingContext2DState::PaintType,
                     CanvasRenderingContext2DState::ImageType,
                     const SkIRect& clip_bounds,
@@ -643,7 +646,6 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
   template <typename T>
   void AdjustRectForCanvas(T& x, T& y, T& width, T& height);
 
-  void ClearCanvasForSrcCompositeOp();
   bool RectContainsTransformedRect(const gfx::RectF&, const SkIRect&) const;
   // Sets the origin to be tainted by the content of the canvas, such
   // as a cross-origin image. This is as opposed to some other reason
@@ -685,7 +687,6 @@ class MODULES_EXPORT BaseRenderingContext2D : public CanvasPath {
 };
 
 ALWAYS_INLINE void BaseRenderingContext2D::CheckOverdraw(
-    const SkRect& rect,
     const cc::PaintFlags* flags,
     CanvasRenderingContext2DState::ImageType image_type,
     BaseRenderingContext2D::OverdrawOp overdraw_op) {
@@ -730,41 +731,51 @@ template <BaseRenderingContext2D::OverdrawOp CurrentOverdrawOp,
           typename DrawFunc,
           typename DrawCoversClipBoundsFunc>
 void BaseRenderingContext2D::DrawInternal(
+    cc::PaintCanvas* paint_canvas,
     const DrawFunc& draw_func,
     const DrawCoversClipBoundsFunc& draw_covers_clip_bounds,
-    const SkRect& bounds,
+    const gfx::RectF& bounds,
     CanvasRenderingContext2DState::PaintType paint_type,
     CanvasRenderingContext2DState::ImageType image_type,
     const SkIRect& clip_bounds,
     CanvasPerformanceMonitor::DrawType draw_type) {
+  if (UNLIKELY(!paint_canvas)) {
+    // This is the async draw case.
+    paint_canvas = GetPaintCanvas();
+    if (!paint_canvas) {
+      return;
+    }
+  }
   const CanvasRenderingContext2DState& state = GetState();
   SkBlendMode global_composite = state.GlobalComposite();
   if (ShouldUseCompositedDraw(paint_type, image_type)) {
-    CompositedDraw(draw_func, GetPaintCanvasForDraw(clip_bounds, draw_type),
-                   paint_type, image_type);
+    WillDraw(clip_bounds, draw_type);
+    CompositedDraw(draw_func, paint_canvas, paint_type, image_type);
   } else if (global_composite == SkBlendMode::kSrc) {
-    ClearCanvasForSrcCompositeOp();  // Takes care of CheckOverdraw()
+    // Takes care of CheckOverdraw()
+    paint_canvas->clear(HasAlpha() ? SkColors::kTransparent : SkColors::kBlack);
     const cc::PaintFlags* flags =
         state.GetFlags(paint_type, kDrawForegroundOnly, image_type);
-    draw_func(GetPaintCanvasForDraw(clip_bounds, draw_type), flags);
+    WillDraw(clip_bounds, draw_type);
+    draw_func(paint_canvas, flags);
   } else {
     SkIRect dirty_rect;
-    if (ComputeDirtyRect(gfx::SkRectToRectF(bounds), clip_bounds,
-                         &dirty_rect)) {
+    if (ComputeDirtyRect(bounds, clip_bounds, &dirty_rect)) {
       const cc::PaintFlags* flags =
           state.GetFlags(paint_type, kDrawShadowAndForeground, image_type);
       if (paint_type != CanvasRenderingContext2DState::kStrokePaintType &&
           draw_covers_clip_bounds(clip_bounds)) {
         // Because CurrentOverdrawOp is a template argument the following branch
         // is optimized-out at compile time.
         if (CurrentOverdrawOp != OverdrawOp::kNone) {
-          CheckOverdraw(bounds, flags, image_type, CurrentOverdrawOp);
+          CheckOverdraw(flags, image_type, CurrentOverdrawOp);
         }
       }
-      draw_func(GetPaintCanvasForDraw(dirty_rect, draw_type), flags);
+      WillDraw(dirty_rect, draw_type);
+      draw_func(paint_canvas, flags);
     }
   }
-  if (UNLIKELY(GetPaintCanvas()->NeedsFlush())) {
+  if (UNLIKELY(paint_canvas->NeedsFlush())) {
     // This happens if draw_func called flush() on the PaintCanvas. The flush
     // cannot be performed inside the scope of draw_func because it would break
     // the logic of CompositedDraw.
@@ -778,7 +789,7 @@ template <BaseRenderingContext2D::OverdrawOp CurrentOverdrawOp,
 void BaseRenderingContext2D::Draw(
     const DrawFunc& draw_func,
     const DrawCoversClipBoundsFunc& draw_covers_clip_bounds,
-    const SkRect& bounds,
+    const gfx::RectF& bounds,
     CanvasRenderingContext2DState::PaintType paint_type,
     CanvasRenderingContext2DState::ImageType image_type,
     CanvasPerformanceMonitor::DrawType draw_type) {
@@ -795,12 +806,12 @@ void BaseRenderingContext2D::Draw(
     PostDeferrableAction(WTF::BindOnce(
         &BaseRenderingContext2D::DrawInternal<CurrentOverdrawOp, DrawFunc,
                                               DrawCoversClipBoundsFunc>,
-        WrapPersistent(this), draw_func, draw_covers_clip_bounds, bounds,
-        paint_type, image_type, clip_bounds, draw_type));
+        WrapPersistent(this), nullptr, draw_func, draw_covers_clip_bounds,
+        bounds, paint_type, image_type, clip_bounds, draw_type));
   } else {
     DrawInternal<CurrentOverdrawOp, DrawFunc, DrawCoversClipBoundsFunc>(
-        draw_func, draw_covers_clip_bounds, bounds, paint_type, image_type,
-        clip_bounds, draw_type);
+        paint_canvas, draw_func, draw_covers_clip_bounds, bounds, paint_type,
+        image_type, clip_bounds, draw_type);
   }
 }
 

diff --git a/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d_test.cc b/third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d_test.cc
@@ -78,11 +78,8 @@ class TestRenderingContext2D final
   cc::PaintCanvas* GetPaintCanvas() override {
     return recorder_.getRecordingCanvas();
   }
-  cc::PaintCanvas* GetPaintCanvasForDraw(
-      const SkIRect& dirty_rect,
-      CanvasPerformanceMonitor::DrawType) override {
-    return recorder_.getRecordingCanvas();
-  }
+  void WillDraw(const SkIRect& dirty_rect,
+                CanvasPerformanceMonitor::DrawType) override {}
 
   sk_sp<PaintFilter> StateGetFilter() override {
     return GetState().GetFilterForOffscreenCanvas({}, this);

diff --git a/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc b/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc
@@ -451,13 +451,9 @@ cc::PaintCanvas* CanvasRenderingContext2D::GetPaintCanvas() {
   return canvas()->GetCanvas2DLayerBridge()->GetPaintCanvas();
 }
 
-cc::PaintCanvas* CanvasRenderingContext2D::GetPaintCanvasForDraw(
+void CanvasRenderingContext2D::WillDraw(
     const SkIRect& dirty_rect,
     CanvasPerformanceMonitor::DrawType draw_type) {
-  if (UNLIKELY(isContextLost() || !canvas() ||
-               !canvas()->GetCanvas2DLayerBridge() ||
-               !canvas()->GetCanvas2DLayerBridge()->ResourceProvider()))
-    return nullptr;
   CanvasRenderingContext::DidDraw(dirty_rect, draw_type);
   // Always draw everything during printing.
   if (!layer_count_) {
@@ -467,7 +463,6 @@ cc::PaintCanvas* CanvasRenderingContext2D::GetPaintCanvasForDraw(
         ->ResourceProvider()
         ->FlushIfRecordingLimitExceeded();
   }
-  return canvas()->GetCanvas2DLayerBridge()->GetPaintCanvas();
 }
 
 void CanvasRenderingContext2D::FlushCanvas() {
@@ -980,7 +975,7 @@ void CanvasRenderingContext2D::drawFormattedText(
         [&recording](cc::PaintCanvas* c,
                      const cc::PaintFlags* flags)  // draw lambda
         { c->drawPicture(std::move(recording)); },
-        [](const SkIRect& rect) { return false; }, gfx::RectFToSkRect(bounds),
+        [](const SkIRect& rect) { return false; }, bounds,
         CanvasRenderingContext2DState::PaintType::kFillPaintType,
         CanvasRenderingContext2DState::kNoImage,
         CanvasPerformanceMonitor::DrawType::kText);
@@ -1095,8 +1090,7 @@ void CanvasRenderingContext2D::DrawTextInternal(
       },
       [](const SkIRect& rect)  // overdraw test lambda
       { return false; },
-      gfx::RectFToSkRect(bounds), paint_type,
-      CanvasRenderingContext2DState::kNoImage,
+      bounds, paint_type, CanvasRenderingContext2DState::kNoImage,
       CanvasPerformanceMonitor::DrawType::kText);
 }
 

diff --git a/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h b/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.h
@@ -179,9 +179,8 @@ class MODULES_EXPORT CanvasRenderingContext2D final
 
   cc::PaintCanvas* GetOrCreatePaintCanvas() final;
   cc::PaintCanvas* GetPaintCanvas() final;
-  cc::PaintCanvas* GetPaintCanvasForDraw(
-      const SkIRect& dirty_rect,
-      CanvasPerformanceMonitor::DrawType) final;
+  void WillDraw(const SkIRect& dirty_rect,
+                CanvasPerformanceMonitor::DrawType) final;
 
   SkColorInfo CanvasRenderingContextSkColorInfo() const override {
     return color_params_.GetSkColorInfo();

diff --git a/.../blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc b/.../blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
@@ -298,19 +298,19 @@ cc::PaintCanvas* OffscreenCanvasRenderingContext2D::GetPaintCanvas() {
   return GetCanvasResourceProvider()->Canvas();
 }
 
-cc::PaintCanvas* OffscreenCanvasRenderingContext2D::GetPaintCanvasForDraw(
+void OffscreenCanvasRenderingContext2D::WillDraw(
     const SkIRect& dirty_rect,
     CanvasPerformanceMonitor::DrawType draw_type) {
-  if (!is_valid_size_ || !GetCanvasResourceProvider())
-    return nullptr;
+  // Call sites should ensure GetPaintCanvas() returns non-null before calling
+  // this.
+  DCHECK(GetPaintCanvas());
   dirty_rect_for_commit_.join(dirty_rect);
   GetCanvasPerformanceMonitor().DidDraw(draw_type);
   Host()->DidDraw(dirty_rect_for_commit_);
   if (!layer_count_) {
     // TODO(crbug.com/1246486): Make auto-flushing layer friendly.
     GetCanvasResourceProvider()->FlushIfRecordingLimitExceeded();
   }
-  return GetCanvasResourceProvider()->Canvas();
 }
 
 sk_sp<PaintFilter> OffscreenCanvasRenderingContext2D::StateGetFilter() {
@@ -735,8 +735,7 @@ void OffscreenCanvasRenderingContext2D::DrawTextInternal(
       },
       [](const SkIRect& rect)  // overdraw test lambda
       { return false; },
-      gfx::RectFToSkRect(bounds), paint_type,
-      CanvasRenderingContext2DState::kNoImage,
+      bounds, paint_type, CanvasRenderingContext2DState::kNoImage,
       CanvasPerformanceMonitor::DrawType::kText);
 
   // |paint_canvas| maybe rese during Draw. If that happens,