Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(64)

Issue 703783004: Use CanvasImageSource union type instead of overloading (Closed)

Created:
6 years, 1 month ago by Jens Widell
Modified:
6 years, 1 month ago
Reviewers:
Justin Novosad
CC:
blink-reviews, arv+blink, blink-reviews-html_chromium.org, dglazkov+blink, Rik, Inactive, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Use CanvasImageSource union type instead of overloading In CanvasRenderingContext2D.idl, use the already defined union type CanvasImageSource for the argument to drawImage() and createPattern(), instead of having one overload of each per union member type. This reduces the number of overloads for drawImage() from 12 to 3 and thus greatly reduces the amount of generated bindings code for it. BUG=430337 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185247

Patch Set 1 #

Patch Set 2 : adjust tests #

Total comments: 10

Patch Set 3 : test createPattern(ImageBitmap, ...) #

Patch Set 4 : don't append canvases to document.body #

Messages

Total messages: 10 (2 generated)
Jens Widell
PTAL https://codereview.chromium.org/703783004/diff/20001/Source/core/html/canvas/CanvasRenderingContext2D.idl File Source/core/html/canvas/CanvasRenderingContext2D.idl (left): https://codereview.chromium.org/703783004/diff/20001/Source/core/html/canvas/CanvasRenderingContext2D.idl#oldcode122 Source/core/html/canvas/CanvasRenderingContext2D.idl:122: [RuntimeEnabled=ExperimentalCanvasFeatures, RaisesException] void drawImage(ImageBitmap imageBitmap, unrestricted float x, ...
6 years, 1 month ago (2014-11-05 13:18:36 UTC) #2
Justin Novosad
This is great! The only thing missing is a test for createPattern(ImageBitmap) https://codereview.chromium.org/703783004/diff/20001/Source/core/html/canvas/CanvasRenderingContext2D.cpp File Source/core/html/canvas/CanvasRenderingContext2D.cpp ...
6 years, 1 month ago (2014-11-05 20:40:53 UTC) #3
Jens Widell
Test added for createPattern(ImageBitmap, ...). (Largely copied from the canvas-createImageBitmap-drawImage.html test.) https://codereview.chromium.org/703783004/diff/20001/Source/core/html/canvas/CanvasRenderingContext2D.cpp File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right): ...
6 years, 1 month ago (2014-11-06 11:44:40 UTC) #4
Jens Widell
junov@: ping? :-)
6 years, 1 month ago (2014-11-11 11:11:35 UTC) #5
Justin Novosad
On 2014/11/11 11:11:35, Jens Widell wrote: > junov@: ping? :-) Sorry, I was OOO. lgtm
6 years, 1 month ago (2014-11-12 20:39:11 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/703783004/60001
6 years, 1 month ago (2014-11-12 20:49:07 UTC) #8
commit-bot: I haz the power
Committed patchset #4 (id:60001) as 185247
6 years, 1 month ago (2014-11-12 21:55:46 UTC) #9
Nils Barth (inactive)
6 years, 1 month ago (2014-11-14 21:29:40 UTC) #10
Message was sent while issue was closed.
Woo-hoo!
This is great on so many levels:
- simplifies interface
- reduces size of bindings
- faster!
See:
https://codereview.chromium.org/611953003/
...for speed benefit.

Powered by Google App Engine
This is Rietveld 408576698