cc: Use SkMScalar instead of doubles everywhere in cc

cc: Use SkMScalar instead of doubles for transforms in cc

BUG=269819
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222683

[enne (OOO), 2013-09-07 00:01:45]

This change is just the change to make transforms use SkMScalar, but doesn't
change SkMScalar to double.

When SkMScalar flips to be a float, there's about ~30 layout tests that have
image-only failures (sparse pixel diffs on rotated/perspective edges) and one cc
pixeltest failure (for the same reason) that need rebaselining.  This change
fixes all the non pixel tests to pass with either float or double.

Here are some comments about this patch to help point out the more interesting
parts:

https://codereview.chromium.org/23043011/diff/6001/cc/base/float_quad_unittes...
File cc/base/float_quad_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/base/float_quad_unittes...
cc/base/float_quad_unittest.cc:36: EXPECT_TRUE(quad.IsRectilinear()) << "case "
<< i;
This is just a driveby test cleanup I did while trying to solve this case (fixed
by the rotation change mentioned elsewhere).

https://codereview.chromium.org/23043011/diff/6001/cc/layers/delegated_render...
File cc/layers/delegated_renderer_layer_impl_unittest.cc (left):

https://codereview.chromium.org/23043011/diff/6001/cc/layers/delegated_render...
cc/layers/delegated_renderer_layer_impl_unittest.cc:470:
delegated_renderer_layer->SetBounds(gfx::Size(30, 30));
30/50 here isn't perfectly representable with floating point numbers, so I
changed it to 75/100 and updated all the math.

Thanks for all the informative comments!

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:3581: #ifdef SK_MSCALAR_IS_DOUBLE
This is the sketchiest part of this patch.  I'm not happy with this, but it's
less clear how to fix this to work both ways.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:6657: SkMScalar initial_parent_scale
= 1.75;
The best part of this patch is that there's no style guide for SkMScalar, so I
feel like we can ignore the style guide rule for literals.  Is it a float? Is it
a double? Who knows!

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform.cc#newcod...
ui/gfx/transform.cc:126: double radians = (degrees / 180.0) * M_PI;
This needed to stay a double for precision when rotating 90/180/270 degrees.

[danakj, 2013-09-09 17:57:45]

Cool!

https://codereview.chromium.org/23043011/diff/6001/cc/animation/layer_animati...
File cc/animation/layer_animation_controller_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/layer_animati...
cc/animation/layer_animation_controller_unittest.cc:21: void
ExpectTranslateX(SkMScalar translate_x, const gfx::Transform& matrix) {
Nothing seems to use this? Delete it?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operation.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:66: SkMScalar error = std::abs(1.0 - (dot *
dot) / (length_2 * other_length_2));
s/1.0/SK_MScalar1/?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:82: return from * (1 - progress) + to *
progress;
this should be doing SkDoubleToMScalar() on its return value.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:278: ApplyScaleToBox(BlendSkMScalars(from_x,
to_x, min_progress),
These values should be SkMScalarToFloat()'d

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:283: ApplyScaleToBox(BlendSkMScalars(from_x,
to_x, max_progress),
As should these.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operations.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations.cc:39: SkMScalar progress) const {
progress should be a double?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations.cc:47: SkMScalar min_progress,
progress should remain as doubles, no? it did elsewhere.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations.cc:184: SkMScalar progress,
progress as double?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operations_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:274: SkMScalar progress = 0.25;
double?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:289: static void
CheckProgress(SkMScalar progress,
double?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:360: SkMScalar progress = 0.25;
double for all these?

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:605: SkMScalar progress = 0.5;
It's a bit off that these are 0.5 and not 0.5f or whatever, but Idk what to
really suggest, so this is fine.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:723: SkMScalar min_progress = 0.0;
more doubles?

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.cc#newco...
cc/base/math_util.cc:114: SkMScalarToFloat(transform.matrix().get(1, 3)));
We need to add getFloat() to the SkMatrix44. Care to do that now?

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.h
File cc/base/math_util.h (right):

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.h#newcode39
cc/base/math_util.h:39: bool ShouldBeClipped() const { return w() <= 0.0; }
We're going to be left in this weird limbo world with integer, float, and double
literals being used for matrix values. I really don't know what's best though,
other than maybe making a rule to use float everywhere? Or use ints and floats?
Or??

The animation code used mostly integer literals, this code uses doubles. Over
time I'm sure we'll see floats if we change SkMScalar to float.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:2793: SkMScalar
half_width_of_rotated_layer = (100.0 / sqrt(2.0)) * 0.5;
should these be float literals? Or SkDoubleToMScalar?

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:3566: int diagonal_radius =
ceil(sqrt(2.0) * 25.0);
does it help if you use floats here?

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:3567: gfx::Rect
unclipped_surface_content = gfx::Rect(50.0 - diagonal_radius,
woa wierd, using double literals for an integer rect.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:6657: SkMScalar initial_parent_scale
= 1.75;
`On 2013/09/07 00:01:45, enne wrote:
> The best part of this patch is that there's no style guide for SkMScalar, so I
> feel like we can ignore the style guide rule for literals.  Is it a float? Is
it
> a double? Who knows!

Ya but I'm worried we'll end up with using all of them and we'll be like "what's
going on!??!" at some point. I wonder if we should be using float everywhere for
them?

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_i...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl_unittest.cc:2209:
host_impl_->ScrollBegin(gfx::Point(1, 1),
Why is this needed? Can you add a comment explaining?

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform.h
File ui/gfx/transform.h (right):

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform.h#newcode219
ui/gfx/transform.h:219: bool Blend(const Transform& from, SkMScalar progress);
progress isn't a transform element, should stay as double?

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc
File ui/gfx/transform_util.cc (right):

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:60: product = std::min(std::max(product, -one), one);
SK_MScalar1?

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:76: if (std::abs(product - 1.0) < epsilon) {
should this be SK_MScalar1? or integer literal? or float literal? i see integer
literals down below here..

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:83: SkMScalar theta = std::acos(product);
Should |product| and |w| and all these variables above here be doubles to
preserve precision when doing acos() and sin?

[enne (OOO), 2013-09-10 22:32:31]

https://codereview.chromium.org/23043011/diff/6001/cc/animation/layer_animati...
File cc/animation/layer_animation_controller_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/layer_animati...
cc/animation/layer_animation_controller_unittest.cc:21: void
ExpectTranslateX(SkMScalar translate_x, const gfx::Transform& matrix) {
On 2013/09/09 17:57:45, danakj wrote:
> Nothing seems to use this? Delete it?

Done.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operation.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:66: SkMScalar error = std::abs(1.0 - (dot *
dot) / (length_2 * other_length_2));
On 2013/09/09 17:57:45, danakj wrote:
> s/1.0/SK_MScalar1/?

Done.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:82: return from * (1 - progress) + to *
progress;
On 2013/09/09 17:57:45, danakj wrote:
> this should be doing SkDoubleToMScalar() on its return value.

progress here should really just be an SkMScalar.  It's not like having it be a
double adds any precision.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:278: ApplyScaleToBox(BlendSkMScalars(from_x,
to_x, min_progress),
On 2013/09/09 17:57:45, danakj wrote:
> These values should be SkMScalarToFloat()'d

Done.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operation.cc:283: ApplyScaleToBox(BlendSkMScalars(from_x,
to_x, max_progress),
On 2013/09/09 17:57:45, danakj wrote:
> As should these.

Done.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operations.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations.cc:39: SkMScalar progress) const {
On 2013/09/09 17:57:45, danakj wrote:
> progress should be a double?

I think an SkMScalar.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations.cc:47: SkMScalar min_progress,
On 2013/09/09 17:57:45, danakj wrote:
> progress should remain as doubles, no? it did elsewhere.

SkMScalar, as elsewhere.

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
File cc/animation/transform_operations_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/animation/transform_ope...
cc/animation/transform_operations_unittest.cc:605: SkMScalar progress = 0.5;
On 2013/09/09 17:57:45, danakj wrote:
> It's a bit off that these are 0.5 and not 0.5f or whatever, but Idk what to
> really suggest, so this is fine.

Changed all progress to float literals to avoid narrowing conversions later.

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/base/math_util.cc#newco...
cc/base/math_util.cc:114: SkMScalarToFloat(transform.matrix().get(1, 3)));
On 2013/09/09 17:57:45, danakj wrote:
> We need to add getFloat() to the SkMatrix44. Care to do that now?

Not particularly, but https://codereview.chromium.org/23757031/.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:2793: SkMScalar
half_width_of_rotated_layer = (100.0 / sqrt(2.0)) * 0.5;
On 2013/09/09 17:57:45, danakj wrote:
> should these be float literals? Or SkDoubleToMScalar?

I'll do the latter.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:3566: int diagonal_radius =
ceil(sqrt(2.0) * 25.0);
On 2013/09/09 17:57:45, danakj wrote:
> does it help if you use floats here?

Sadly, sqrt(2) * 25 = 35.35, so it easily rounds up to 36 in both cases.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:3567: gfx::Rect
unclipped_surface_content = gfx::Rect(50.0 - diagonal_radius,
On 2013/09/09 17:57:45, danakj wrote:
> woa wierd, using double literals for an integer rect.

Done.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common_unittest.cc:6657: SkMScalar initial_parent_scale
= 1.75;
On 2013/09/09 17:57:45, danakj wrote:
> `On 2013/09/07 00:01:45, enne wrote:
> > The best part of this patch is that there's no style guide for SkMScalar, so
I
> > feel like we can ignore the style guide rule for literals.  Is it a float?
Is
> it
> > a double? Who knows!
> 
> Ya but I'm worried we'll end up with using all of them and we'll be like
"what's
> going on!??!" at some point. I wonder if we should be using float everywhere
for
> them?

I wasn't totally serious here.  I think the plan should be that we  use floats
for SkMScalar for new code, but don't worry too hard about old code assigning
doubles to SkMScalars.

When I switch transforms to floats, I think there will likely be some narrowing
conversion errors that I'll have to fix, but I don't think it's worth the
trouble to change every SkMScalar literal to be a float as well.

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_i...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/6001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl_unittest.cc:2209:
host_impl_->ScrollBegin(gfx::Point(1, 1),
On 2013/09/09 17:57:45, danakj wrote:
> Why is this needed? Can you add a comment explaining?

Yeah, when this is floating point the origin no longer hits the rotated
scrolling layer.  1, 1 is a little bit off the layer's corner and into the
scrolling layer, and so it hits it.

I'm not sure that a comment would be helpful here.  The test is already pretty
clear about which layer it's scrolling, and it's just picking a point that's
more guaranteed to hit that layer.  Did you have something in mind?

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc
File ui/gfx/transform_util.cc (right):

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:60: product = std::min(std::max(product, -one), one);
On 2013/09/09 17:57:45, danakj wrote:
> SK_MScalar1?

Done.

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:76: if (std::abs(product - 1.0) < epsilon) {
On 2013/09/09 17:57:45, danakj wrote:
> should this be SK_MScalar1? or integer literal? or float literal? i see
integer
> literals down below here..

Done.

https://codereview.chromium.org/23043011/diff/6001/ui/gfx/transform_util.cc#n...
ui/gfx/transform_util.cc:83: SkMScalar theta = std::acos(product);
On 2013/09/09 17:57:45, danakj wrote:
> Should |product| and |w| and all these variables above here be doubles to
> preserve precision when doing acos() and sin?

I think it's less important here.  The doubles for rotations were to help with
the 90/180/270 case.  This is slerping and will be accurate at the end points
because it's clamped to -1 or 1.

https://codereview.chromium.org/23043011/diff/22001/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/23043011/diff/22001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:127: SkMScalar cosTheta =
SkDoubleToScalar(std::cos(radians));
Things that are not the same: SkDoubleToScalar, SkDoubleToMScalar.  /o\

This ended up being the issue for that one test that required #ifdefs.  \o/

[danakj, 2013-09-11 18:53:46]

LGTM

https://codereview.chromium.org/23043011/diff/22001/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/23043011/diff/22001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:127: SkMScalar cosTheta =
SkDoubleToScalar(std::cos(radians));
On 2013/09/10 22:32:32, enne wrote:
> Things that are not the same: SkDoubleToScalar, SkDoubleToMScalar.  /o\
> 
> This ended up being the issue for that one test that required #ifdefs.  \o/

ohh, nice.

https://codereview.chromium.org/23043011/diff/24001/cc/animation/transform_op...
File cc/animation/transform_operation.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/animation/transform_op...
cc/animation/transform_operation.cc:67: std::abs(SK_MScalar1 - (dot * dot) /
(length_2 * other_length_2));
based on our discussions, i guess 1.f would also be acceptable instead of
MScalar1, but whatever you like.

https://codereview.chromium.org/23043011/diff/24001/cc/animation/transform_op...
cc/animation/transform_operation.cc:162: ? std::numeric_limits<SkMScalar>::max()
i thought clang-format lines up the ? with the = above it, or am i
mis-remembering.

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc#newc...
cc/base/math_util.cc:113:
gfx::Vector2dF(SkMScalarToFloat(transform.matrix().get(0, 3)),
can probably use getFloat() now? (thanks for doing that)

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc#newc...
cc/base/math_util.cc:468: static inline SkMScalar ScaleOnAxis(SkMScalar a,
SkMScalar b, SkMScalar c) {
here we're multiplying stuff just to take the sqrt, it'd be nice to not throw
away bits. it reminds me of
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/vector2d.cc...

Do you think it's alright to keep the arguments as doubles (or cast them to
double when we multiply them)? Or do you think it's really not worth it?

https://codereview.chromium.org/23043011/diff/24001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common_unittest.cc:5054: test_point = gfx::Point(51,
49);
This is my favorite part

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:366: const double translate_z = matrix_.get(2, 3);
SkMScalar

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:368: return gfx::Vector2dF(matrix_.get(0, 3), matrix_.get(1,
3));
SkMScalarToFloat()

[enne (OOO), 2013-09-11 19:58:30]

https://codereview.chromium.org/23043011/diff/24001/cc/animation/transform_op...
File cc/animation/transform_operation.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/animation/transform_op...
cc/animation/transform_operation.cc:162: ? std::numeric_limits<SkMScalar>::max()
On 2013/09/11 18:53:46, danakj wrote:
> i thought clang-format lines up the ? with the = above it, or am i
> mis-remembering.

I think you're mis-remembering.  It lines up ? and :, but not ? and =.

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc#newc...
cc/base/math_util.cc:113:
gfx::Vector2dF(SkMScalarToFloat(transform.matrix().get(0, 3)),
On 2013/09/11 18:53:46, danakj wrote:
> can probably use getFloat() now? (thanks for doing that)

For some future version of now? I'll land this as-is and fix up later when this
eventually lands and rolls.

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc#newc...
cc/base/math_util.cc:468: static inline SkMScalar ScaleOnAxis(SkMScalar a,
SkMScalar b, SkMScalar c) {
On 2013/09/11 18:53:46, danakj wrote:
> here we're multiplying stuff just to take the sqrt, it'd be nice to not throw
> away bits. it reminds me of
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/vector2d.cc...
> 
> Do you think it's alright to keep the arguments as doubles (or cast them to
> double when we multiply them)? Or do you think it's really not worth it?

Ok, sure.  I'll buy your argument that the sqrt should be done as a double.

https://codereview.chromium.org/23043011/diff/24001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common_unittest.cc:5054: test_point = gfx::Point(51,
49);
On 2013/09/11 18:53:46, danakj wrote:
> This is my favorite part

I have no idea how this worked before, but I love that "right and up" and "left
and down" used to use the same y value.

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:366: const double translate_z = matrix_.get(2, 3);
On 2013/09/11 18:53:46, danakj wrote:
> SkMScalar

Done.

https://codereview.chromium.org/23043011/diff/24001/ui/gfx/transform.cc#newco...
ui/gfx/transform.cc:368: return gfx::Vector2dF(matrix_.get(0, 3), matrix_.get(1,
3));
On 2013/09/11 18:53:46, danakj wrote:
> SkMScalarToFloat()

Done.

[danakj, 2013-09-11 19:59:40]

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc
File cc/base/math_util.cc (right):

https://codereview.chromium.org/23043011/diff/24001/cc/base/math_util.cc#newc...
cc/base/math_util.cc:113:
gfx::Vector2dF(SkMScalarToFloat(transform.matrix().get(0, 3)),
On 2013/09/11 19:58:30, enne wrote:
> On 2013/09/11 18:53:46, danakj wrote:
> > can probably use getFloat() now? (thanks for doing that)
> 
> For some future version of now? I'll land this as-is and fix up later when
this
> eventually lands and rolls.

Ya sorry I figured the skia folks would have committed it by now.

[commit-bot: I haz the power, 2013-09-11 20:02:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/23043011/33001

[tomhudson, 2013-09-11 21:13:55]

On 2013/09/11 19:59:40, danakj wrote:

> > For some future version of now? I'll land this as-is and fix up later when
> this
> > eventually lands and rolls.
> 
> Ya sorry I figured the skia folks would have committed it by now.

The Skia tree has been closed. :( If getFloat() lands in Skia tomorrow, it'll
likely roll into Chrome on Friday; always expect a 1-2 day delay.

[commit-bot: I haz the power, 2013-09-12 00:51:28]

Change committed as 222683