[Canvas] Simplify DidDraw methods (Reland)
This is a code cleanup related to DidDraw methods throughout
canvas-related code in blink. This CL removes unnecessary overloads,
unnecessary virtuals, unnecessary argument passing. In WebGL
rendering contexts, we remove an unnecessary recursion cycle. In
2D contexts, we remove an unnecessary and confusing virtual method
multiple inherritance name conflict (method renamed to DidDraw2D
in BaseRenderingContext2D)
Difference from original CL that was reverted: removed changes
to dirty_rect calulations in html_canvas_element.cc
Original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2947051/
Bug: 1217714
Change-Id: I98a2b87418389adfe368a2a7cb2c1175e3cc221e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2965277
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893061}
diff --git a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
index a49d4f5..8331d31 100644
--- a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
+++ b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
@@ -59,12 +59,7 @@
}
void CanvasRenderingContext::DidDraw(const SkIRect& dirty_rect) {
- Host()->DidDraw(SkRect::Make(dirty_rect));
- DidDrawCommon();
-}
-
-void CanvasRenderingContext::DidDraw() {
- Host()->DidDraw();
+ Host()->DidDraw(dirty_rect);
DidDrawCommon();
}
diff --git a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h
index 944d3e8..285b946f 100644
--- a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h
+++ b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h
@@ -154,8 +154,11 @@
return nullptr;
}
virtual bool IsPaintable() const = 0;
- virtual void DidDraw(const SkIRect& dirty_rect);
- virtual void DidDraw();
+ void DidDraw(const SkIRect& dirty_rect);
+ void DidDraw() {
+ return DidDraw(Host() ? SkIRect::MakeWH(Host()->width(), Host()->height())
+ : SkIRect::MakeEmpty());
+ }
// Return true if the content is updated.
virtual bool PaintRenderingResultsToCanvas(SourceDrawingBuffer) {
diff --git a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h
index 7310c32..1770f40 100644
--- a/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h
+++ b/third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h
@@ -46,8 +46,8 @@
virtual void DetachContext() = 0;
- virtual void DidDraw(const FloatRect& rect) = 0;
- virtual void DidDraw() = 0;
+ virtual void DidDraw(const SkIRect& rect) = 0;
+ void DidDraw() { DidDraw(SkIRect::MakeWH(width(), height())); }
virtual void PreFinalizeFrame() = 0;
virtual void PostFinalizeFrame() = 0;
diff --git a/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc b/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc
index 80fc682..e70ee88 100644
--- a/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc
+++ b/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc
@@ -452,8 +452,8 @@
return blocked;
}
-void HTMLCanvasElement::DidDraw(const FloatRect& rect) {
- if (rect.IsEmpty())
+void HTMLCanvasElement::DidDraw(const SkIRect& rect) {
+ if (rect.isEmpty())
return;
if (GetLayoutObject() && GetLayoutObject()->PreviousVisibilityVisible() &&
GetDocument().GetPage())
@@ -463,18 +463,14 @@
GetLayoutObject()->SetShouldCheckForPaintInvalidation();
if (IsRenderingContext2D() && context_->ShouldAntialias() && GetPage() &&
GetPage()->DeviceScaleFactorDeprecated() > 1.0f) {
- FloatRect inflated_rect = rect;
+ FloatRect inflated_rect = FloatRect(IntRect(rect));
inflated_rect.Inflate(1);
dirty_rect_.Unite(inflated_rect);
} else {
- dirty_rect_.Unite(rect);
+ dirty_rect_.Unite(FloatRect(IntRect(rect)));
}
if (IsRenderingContext2D() && canvas2d_bridge_)
- canvas2d_bridge_->DidDraw(rect);
-}
-
-void HTMLCanvasElement::DidDraw() {
- DidDraw(FloatRect(0, 0, Size().Width(), Size().Height()));
+ canvas2d_bridge_->DidDraw();
}
void HTMLCanvasElement::PreFinalizeFrame() {
@@ -585,8 +581,10 @@
if (dirty_rect_.IsEmpty())
return;
- if (canvas2d_bridge_)
- canvas2d_bridge_->DoPaintInvalidation(invalidation_rect);
+ if (canvas2d_bridge_) {
+ canvas2d_bridge_->DoPaintInvalidation(
+ EnclosingIntRect(invalidation_rect));
+ }
}
if (context_ && HasImageBitmapContext() && context_->CcLayer())
diff --git a/third_party/blink/renderer/core/html/canvas/html_canvas_element.h b/third_party/blink/renderer/core/html/canvas/html_canvas_element.h
index be19441..68cbedb 100644
--- a/third_party/blink/renderer/core/html/canvas/html_canvas_element.h
+++ b/third_party/blink/renderer/core/html/canvas/html_canvas_element.h
@@ -153,8 +153,8 @@
bool HasCanvasCapture() const final { return !listeners_.IsEmpty(); }
// Used for rendering
- void DidDraw(const FloatRect&) override;
- void DidDraw() override;
+ void DidDraw(const SkIRect&) override;
+ using CanvasRenderingContextHost::DidDraw;
void Paint(GraphicsContext&,
const PhysicalRect&,
diff --git a/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc b/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc
index ae472fe..4eabee7 100644
--- a/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc
+++ b/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.cc
@@ -467,12 +467,8 @@
return ResourceProvider();
}
-void OffscreenCanvas::DidDraw() {
- DidDraw(FloatRect(0, 0, Size().Width(), Size().Height()));
-}
-
-void OffscreenCanvas::DidDraw(const FloatRect& rect) {
- if (rect.IsEmpty())
+void OffscreenCanvas::DidDraw(const SkIRect& rect) {
+ if (rect.isEmpty())
return;
if (HasPlaceholderCanvas()) {
diff --git a/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h b/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h
index 7872b6e..9f54b3f8 100644
--- a/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h
+++ b/third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h
@@ -113,8 +113,8 @@
bool PushFrameIfNeeded();
bool PushFrame(scoped_refptr<CanvasResource> frame,
const SkIRect& damage_rect) override;
- void DidDraw(const FloatRect&) override;
- void DidDraw() override;
+ void DidDraw(const SkIRect&) override;
+ using CanvasRenderingContextHost::DidDraw;
void Commit(scoped_refptr<CanvasResource> bitmap_image,
const SkIRect& damage_rect) override;
bool ShouldAccelerate2dContext() const override;
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
index f03c212..492e4809 100644
--- 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
@@ -1266,12 +1266,12 @@
kClipFill);
c = GetPaintCanvas(); // Check overdraw may have swapped the PaintCanvas
c->drawRect(rect, clear_flags);
- DidDraw(clip_bounds);
+ DidDraw2D(clip_bounds);
} else {
SkIRect dirty_rect;
if (ComputeDirtyRect(rect, clip_bounds, &dirty_rect)) {
c->drawRect(rect, clear_flags);
- DidDraw(dirty_rect);
+ DidDraw2D(dirty_rect);
}
}
}
@@ -2138,7 +2138,7 @@
}
}
- DidDraw(dest_rect);
+ DidDraw2D(dest_rect);
}
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
index f4d2176b..8020fd8 100644
--- 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
@@ -287,7 +287,7 @@
virtual cc::PaintCanvas* GetOrCreatePaintCanvas() = 0;
virtual cc::PaintCanvas* GetPaintCanvas() const = 0;
- virtual void DidDraw(const SkIRect& dirty_rect) = 0;
+ virtual void DidDraw2D(const SkIRect& dirty_rect) = 0;
virtual bool StateHasFilter() = 0;
virtual sk_sp<PaintFilter> StateGetFilter() = 0;
@@ -576,13 +576,13 @@
(GetState().ShouldDrawShadows() &&
ShouldUseDropShadowPaintFilter(paint_type, image_type))) {
CompositedDraw(draw_func, GetPaintCanvas(), paint_type, image_type);
- DidDraw(clip_bounds);
+ DidDraw2D(clip_bounds);
} else if (GetState().GlobalComposite() == SkBlendMode::kSrc) {
ClearCanvas(); // takes care of checkOverdraw()
const PaintFlags* flags =
GetState().GetFlags(paint_type, kDrawForegroundOnly, image_type);
draw_func(GetPaintCanvas(), flags);
- DidDraw(clip_bounds);
+ DidDraw2D(clip_bounds);
} else {
SkIRect dirty_rect;
if (ComputeDirtyRect(bounds, clip_bounds, &dirty_rect)) {
@@ -592,7 +592,7 @@
draw_covers_clip_bounds(clip_bounds))
CheckOverdraw(bounds, flags, image_type, kClipFill);
draw_func(GetPaintCanvas(), flags);
- DidDraw(dirty_rect);
+ DidDraw2D(dirty_rect);
}
}
}
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
index 4ac842a..c111c39 100644
--- 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
@@ -433,10 +433,7 @@
}
}
-void CanvasRenderingContext2D::DidDraw(const SkIRect& dirty_rect) {
- if (dirty_rect.isEmpty())
- return;
-
+void CanvasRenderingContext2D::DidDraw2D(const SkIRect& dirty_rect) {
CanvasRenderingContext::DidDraw(dirty_rect);
}
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
index eb7ef37..c6ca01035 100644
--- 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
@@ -175,6 +175,7 @@
void ClearFilterReferences();
// BaseRenderingContext2D implementation
+ void DidDraw2D(const SkIRect& dirty_rect) final;
bool OriginClean() const final;
void SetOriginTainted() final;
bool WouldTaintOrigin(CanvasImageSource* source) final {
@@ -194,7 +195,6 @@
cc::PaintCanvas* GetOrCreatePaintCanvas() final;
cc::PaintCanvas* GetPaintCanvas() const final;
- void DidDraw(const SkIRect& dirty_rect) final;
scoped_refptr<StaticBitmapImage> GetImage() final;
bool StateHasFilter() final;
diff --git a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
index 515ac62..f4d106f 100644
--- a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
+++ b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc
@@ -278,16 +278,9 @@
return GetCanvasResourceProvider()->Canvas();
}
-void OffscreenCanvasRenderingContext2D::DidDraw() {
- dirty_rect_for_commit_.setWH(Width(), Height());
- Host()->DidDraw();
- if (GetCanvasResourceProvider() && GetCanvasResourceProvider()->needs_flush())
- FinalizeFrame();
-}
-
-void OffscreenCanvasRenderingContext2D::DidDraw(const SkIRect& dirty_rect) {
+void OffscreenCanvasRenderingContext2D::DidDraw2D(const SkIRect& dirty_rect) {
dirty_rect_for_commit_.join(dirty_rect);
- Host()->DidDraw(SkRect::Make(dirty_rect_for_commit_));
+ Host()->DidDraw(dirty_rect_for_commit_);
if (GetCanvasResourceProvider() && GetCanvasResourceProvider()->needs_flush())
FinalizeFrame();
}
diff --git a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h
index ec80a60df..3c5add39 100644
--- a/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h
+++ b/third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h
@@ -120,8 +120,7 @@
cc::PaintCanvas* GetOrCreatePaintCanvas() final;
cc::PaintCanvas* GetPaintCanvas() const final;
- void DidDraw() final;
- void DidDraw(const SkIRect& dirty_rect) final;
+ void DidDraw2D(const SkIRect& dirty_rect) final;
bool StateHasFilter() final;
sk_sp<PaintFilter> StateGetFilter() final;
diff --git a/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.cc b/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.cc
index c138063..2c21901e 100644
--- a/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.cc
+++ b/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.cc
@@ -41,7 +41,7 @@
did_record_draw_commands_in_paint_recorder_ = false;
}
-void PaintRenderingContext2D::DidDraw(const SkIRect&) {
+void PaintRenderingContext2D::DidDraw2D(const SkIRect&) {
did_record_draw_commands_in_paint_recorder_ = true;
}
diff --git a/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.h b/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.h
index cd568e8..3f4c202 100644
--- a/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.h
+++ b/third_party/blink/renderer/modules/csspaint/paint_rendering_context_2d.h
@@ -56,7 +56,7 @@
cc::PaintCanvas* GetOrCreatePaintCanvas() final { return GetPaintCanvas(); }
cc::PaintCanvas* GetPaintCanvas() const final;
- void DidDraw(const SkIRect&) final;
+ void DidDraw2D(const SkIRect&) final;
double shadowOffsetX() const final;
void setShadowOffsetX(double) final;
diff --git a/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc b/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
index 5e32a13c..3fcad84 100644
--- a/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
+++ b/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc
@@ -1385,26 +1385,14 @@
if (layout_box && settings->GetAcceleratedCompositingEnabled())
layout_box->ContentChanged(change_type);
}
- IntSize canvas_size = ClampedCanvasSize();
- DidDraw(SkIRect::MakeXYWH(0, 0, canvas_size.Width(), canvas_size.Height()));
+ DidDraw();
}
}
-
scoped_refptr<base::SingleThreadTaskRunner>
WebGLRenderingContextBase::GetContextTaskRunner() {
return task_runner_;
}
-void WebGLRenderingContextBase::DidDraw(const SkIRect& dirty_rect) {
- MarkContextChanged(kCanvasChanged);
- CanvasRenderingContext::DidDraw(dirty_rect);
-}
-
-void WebGLRenderingContextBase::DidDraw() {
- MarkContextChanged(kCanvasChanged);
- CanvasRenderingContext::DidDraw();
-}
-
bool WebGLRenderingContextBase::PushFrame() {
TRACE_EVENT0("blink", "WebGLRenderingContextBase::PushFrame");
DCHECK(Host());
diff --git a/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h b/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h
index 58a700a..08918a42 100644
--- a/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h
+++ b/third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.h
@@ -708,8 +708,6 @@
SourceDrawingBuffer) override;
cc::Layer* CcLayer() const override;
void Stop() override;
- void DidDraw(const SkIRect&) override;
- void DidDraw() override;
void FinalizeFrame() override;
bool PushFrame() override;
diff --git a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc
index 9e1702e0..bd0f7d82 100644
--- a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc
+++ b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc
@@ -258,7 +258,7 @@
// Calling to DidDraw because GetOrCreateResourceProvider created a new
// provider and cleared it
// TODO crbug/1090081: Check possibility to move DidDraw inside Clear.
- DidDraw(FloatRect(0.f, 0.f, size_.Width(), size_.Height()));
+ DidDraw();
if (IsAccelerated() && !layer_) {
layer_ = cc::TextureLayer::CreateForMailbox(this);
@@ -637,7 +637,7 @@
return layer_.get();
}
-void Canvas2DLayerBridge::DidDraw(const FloatRect& /* rect */) {
+void Canvas2DLayerBridge::DidDraw() {
if (ResourceProvider() && ResourceProvider()->needs_flush())
FinalizeFrame();
have_recorded_draw_commands_ = true;
@@ -669,9 +669,9 @@
rate_limiter_->Tick();
}
-void Canvas2DLayerBridge::DoPaintInvalidation(const FloatRect& dirty_rect) {
+void Canvas2DLayerBridge::DoPaintInvalidation(const IntRect& dirty_rect) {
if (layer_ && raster_mode_ == RasterMode::kGPU)
- layer_->SetNeedsDisplayRect(EnclosingIntRect(dirty_rect));
+ layer_->SetNeedsDisplayRect(dirty_rect);
}
scoped_refptr<StaticBitmapImage> Canvas2DLayerBridge::NewImageSnapshot() {
diff --git a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.h b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.h
index ecebb93..506f73e 100644
--- a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.h
+++ b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.h
@@ -90,8 +90,8 @@
void SetIsInHiddenPage(bool);
void SetIsBeingDisplayed(bool);
void SetFilterQuality(SkFilterQuality filter_quality);
- void DidDraw(const FloatRect&);
- void DoPaintInvalidation(const FloatRect& dirty_rect);
+ void DidDraw();
+ void DoPaintInvalidation(const IntRect& dirty_rect);
cc::Layer* Layer();
bool Restore();
diff --git a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc
index aef5382..5c74435 100644
--- a/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc
+++ b/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge_test.cc
@@ -341,7 +341,7 @@
}
void DrawSomething(Canvas2DLayerBridge* bridge) {
- bridge->DidDraw(FloatRect(0, 0, 1, 1));
+ bridge->DidDraw();
bridge->FinalizeFrame();
// Grabbing an image forces a flush
bridge->NewImageSnapshot();
@@ -979,7 +979,7 @@
5);
// Drawline so WritePixels has something to flush
bridge->GetPaintCanvas()->drawLine(0, 0, 2, 2, flags);
- bridge->DidDraw(FloatRect(0, 0, 1, 1));
+ bridge->DidDraw();
// WritePixels flushes recording. Post flush, a new drawing canvas is created
// that should have the matrix restored onto it.
