Support canvas size as default object size

Support canvas size as default object size

By adding a defaultObjectSize parameter to

 * CanvasImageSource::elementSize,
 * CanvasImageSource::defaultDestinationSize
 * CanvasImageSource::getSourceImageForCanvas

we can support the default sizing algorithm in HTMLImageElement for
images that lack an intrinsic size. This affects both drawImage() and
createPattern().

At time of writing there doesn't exist clear spec text for how
createPattern should behave. In
https://github.com/whatwg/html/issues/735 the behavior in this CL has
been suggested as a reasonble starting point.

BUG=581357, 475009
Committed: https://crrev.com/c1bb9d9096de137378cb1029021aed40fcbf76e4
Cr-Commit-Position: refs/heads/master@{#379818}

[davve, 2016-03-04 14:11:53]

davve@opera.com changed reviewers:
+ fs@opera.com, junov@chromium.org

[davve, 2016-03-04 14:11:54]

This is a new attempt at supporting the default object sizing algorithm in
canvas. PTAL.

[fs, 2016-03-04 14:31:26]

core/ changes LGTM (assuming Justin is ok with the interface changes.)

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1029: if
(imageSource->elementSize(defaultObjectSize).width() *
imageSource->elementSize(defaultObjectSize).height() > width() * height() *
ExpensiveCanvasHeuristicParameters::ExpensiveImageSizeRatio)
Nit: Maybe store the result of imageSource->elementSize(...) (explicitly) on the
stack on first use to avoid recomputing it three times? (And simplify this
expression somewhat too.)

[Justin Novosad, 2016-03-04 14:38:59]

You got rid of the statefulness that existed in the previous incarnation.  It is
more readable and less fragile like this.

mega lgtm!

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1029: if
(imageSource->elementSize(defaultObjectSize).width() *
imageSource->elementSize(defaultObjectSize).height() > width() * height() *
ExpensiveCanvasHeuristicParameters::ExpensiveImageSizeRatio)
On 2016/03/04 14:31:26, fs wrote:
> Nit: Maybe store the result of imageSource->elementSize(...) (explicitly) on
the
> stack on first use to avoid recomputing it three times? (And simplify this
> expression somewhat too.)

+1

[davve, 2016-03-04 15:10:36]

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
(right):

https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1029: if
(imageSource->elementSize(defaultObjectSize).width() *
imageSource->elementSize(defaultObjectSize).height() > width() * height() *
ExpensiveCanvasHeuristicParameters::ExpensiveImageSizeRatio)
On 2016/03/04 14:38:59, Justin Novosad wrote:
> On 2016/03/04 14:31:26, fs wrote:
> > Nit: Maybe store the result of imageSource->elementSize(...) (explicitly) on
> the
> > stack on first use to avoid recomputing it three times? (And simplify this
> > expression somewhat too.)
> 
> +1

Done.

[Justin Novosad, 2016-03-04 15:45:41]

On 2016/03/04 15:10:36, David Vest wrote:
>
https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp
> (right):
> 
>
https://codereview.chromium.org/1767633002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp:1029: if
> (imageSource->elementSize(defaultObjectSize).width() *
> imageSource->elementSize(defaultObjectSize).height() > width() * height() *
> ExpensiveCanvasHeuristicParameters::ExpensiveImageSizeRatio)
> On 2016/03/04 14:38:59, Justin Novosad wrote:
> > On 2016/03/04 14:31:26, fs wrote:
> > > Nit: Maybe store the result of imageSource->elementSize(...) (explicitly)
on
> > the
> > > stack on first use to avoid recomputing it three times? (And simplify this
> > > expression somewhat too.)
> > 
> > +1
> 
> Done.

lgtm stands

[davve, 2016-03-08 12:13:55]

The CQ bit was checked by davve@opera.com

[davve, 2016-03-08 12:13:55]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/1767633002/#ps40001
(title: "Save image size on stack")

[commit-bot: I haz the power, 2016-03-08 12:14:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1767633002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1767633002/40001

[commit-bot: I haz the power, 2016-03-08 13:23:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-08 13:24:11]

Description was changed from

==========
Support canvas size as default object size

By adding a defaultObjectSize parameter to

 * CanvasImageSource::elementSize,
 * CanvasImageSource::defaultDestinationSize
 * CanvasImageSource::getSourceImageForCanvas

we can support the default sizing algorithm in HTMLImageElement for
images that lack an intrinsic size. This affects both drawImage() and
createPattern().

At time of writing there doesn't exist clear spec text for how
createPattern should behave. In
https://github.com/whatwg/html/issues/735 the behavior in this CL has
been suggested as a reasonble starting point.

BUG=581357, 475009
==========

to

==========
Support canvas size as default object size

By adding a defaultObjectSize parameter to

 * CanvasImageSource::elementSize,
 * CanvasImageSource::defaultDestinationSize
 * CanvasImageSource::getSourceImageForCanvas

we can support the default sizing algorithm in HTMLImageElement for
images that lack an intrinsic size. This affects both drawImage() and
createPattern().

At time of writing there doesn't exist clear spec text for how
createPattern should behave. In
https://github.com/whatwg/html/issues/735 the behavior in this CL has
been suggested as a reasonble starting point.

BUG=581357, 475009

Committed: https://crrev.com/c1bb9d9096de137378cb1029021aed40fcbf76e4
Cr-Commit-Position: refs/heads/master@{#379818}
==========

[commit-bot: I haz the power, 2016-03-08 13:24:12]

Patchset 3 (id:??) landed as
https://crrev.com/c1bb9d9096de137378cb1029021aed40fcbf76e4
Cr-Commit-Position: refs/heads/master@{#379818}