Remove implicit flooring Scale() method from Point and Size.

Remove implicit flooring Scale() method from Point and Size.

When scaling an integer point or size, return a floating point
result. Implicitly flooring hides design problems and bugs. Add
conversion functions to floor or ceil a SizeF or PointF
into an integer format again.

All existing behaviour has been preserved by replacing uses of
foo.Scale() with ToFlooredFoo(foo.Scale()).

R=sky
BUG=147395
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160970

[danakj, 2012-10-07 18:31:54]

No change in behaviour, just making the flooring operation explicit for integer
scaling.

[not at google - send to devlin, 2012-10-08 06:02:22]

extensions lgtm

[Nico, 2012-10-08 11:12:36]

This CL has six people on the Reviewers: line. Can you say what you want each of
us to look at?

[danakj, 2012-10-08 15:03:07]

Sure!

Sky please look at the ui/gfx changes.

Everyone else is an owner of some place with changes. Please take a look at
the changes in your files.

Thanks!

On Monday, October 8, 2012, wrote:

> This CL has six people on the Reviewers: line. Can you say what you want
> each of
> us to look at?
>
>
http://codereview.chromium.**org/11081007/<http://codereview.chromium.org/110...
>
> --
>
>
>

[sky, 2012-10-08 15:45:39]

ui/gfx LGTM

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/point_conversions.h
File ui/gfx/point_conversions.h (right):

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/point_conversions.h#...
ui/gfx/point_conversions.h:13: // Returns the smallest Rect that encloses the
given RectF.
Rect->Point (same on 16)

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/size_conversions.h
File ui/gfx/size_conversions.h (right):

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/size_conversions.h#n...
ui/gfx/size_conversions.h:13: // Returns the smallest Rect that encloses the
given RectF.
Rect->Size (same on 16)

[oshima, 2012-10-08 17:18:30]

chromeos LGTM

[sadrul, 2012-10-08 23:05:23]

LGTM (reviewed only touch/, renderer_host/, x11_util)

http://codereview.chromium.org/11081007/diff/1072/content/browser/renderer_ho...
File content/browser/renderer_host/dip_util.cc (left):

http://codereview.chromium.org/11081007/diff/1072/content/browser/renderer_ho...
content/browser/renderer_host/dip_util.cc:42:
rect_in_pixel.size().Scale(scale));
Curious why this doesn't just rect_in_pixel.Scale(scale)

http://codereview.chromium.org/11081007/diff/1072/content/browser/renderer_ho...
content/browser/renderer_host/dip_util.cc:59: rect_in_dip.size().Scale(scale));
ditto

[danakj, 2012-10-09 04:28:00]

http://codereview.chromium.org/11081007/diff/1072/content/browser/renderer_ho...
File content/browser/renderer_host/dip_util.cc (left):

http://codereview.chromium.org/11081007/diff/1072/content/browser/renderer_ho...
content/browser/renderer_host/dip_util.cc:42:
rect_in_pixel.size().Scale(scale));
On 2012/10/08 23:05:23, sadrul wrote:
> Curious why this doesn't just rect_in_pixel.Scale(scale)

Me too. It should probably be scaling and doing ToEnclosingIntRect (or
ToEnclosedIntRect) if it wants to get back to ints.

Personally I think device pixels should be in ints, but other coordinate spaces
should not be. This is going to be a bit of a mess to untangle for non-integer
device scales.

But that's all for another day/CL.

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/point_conversions.h
File ui/gfx/point_conversions.h (right):

http://codereview.chromium.org/11081007/diff/1072/ui/gfx/point_conversions.h#...
ui/gfx/point_conversions.h:13: // Returns the smallest Rect that encloses the
given RectF.
On 2012/10/08 15:45:40, sky wrote:
> Rect->Point (same on 16)

Oops, thanks!

[danakj, 2012-10-09 17:39:06]

+jamesr for webkit/

[danakj, 2012-10-09 17:39:54]

+piman for ui/compositor

[danakj, 2012-10-09 17:43:38]

+avi for chrome/browser/tab_contents
+sail for ui/base/resource

+jamesr also for content/renderer

[piman, 2012-10-09 17:44:25]

LGTM - I guess the plan is for someone to come back and fix issues now made
obvious by this?

[danakj, 2012-10-09 17:45:10]

On 2012/10/09 17:44:25, piman wrote:
> LGTM - I guess the plan is for someone to come back and fix issues now made
> obvious by this?

Ya I think various owners are the right people to coordinate that.

[piman, 2012-10-09 17:45:55]

LGTM for content/ too.

[sail, 2012-10-09 17:50:31]

ui/base/resource LGTM

[jamesr, 2012-10-09 18:17:37]

webkit/ lgtm

[Avi (use Gerrit), 2012-10-09 18:38:39]

LGTM even though ThumbnailGenerator isn't in tab_contents any more so you don't
need it.

[commit-bot: I haz the power, 2012-10-09 20:21:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11081007/17004

[danakj, 2012-10-09 20:21:55]

thanks everyone :)

[commit-bot: I haz the power, 2012-10-09 20:51:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11081007/18005

[commit-bot: I haz the power, 2012-10-09 23:17:37]

Change committed as 160970