cc: Add test for AA quads due to precision loss

cc: Fix precision loss causing AA quads

By changing 1.f to 1.0, this allows the inverse contents
scale to cancel out properly in the transformation matrix.

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

[enne (OOO), 2013-09-06 23:44:20]

This is epenner's two character patch from
https://codereview.chromium.org/23440005/ that fixes precision loss, but also
adds a test.

I verified this test fails on both Linux and Android and is fixed by the two
character change.  As a bonus, changing transformation matrices to float also
fixes this bug, but this test will prevent future regressions.

[epennerAtGoogle, 2013-09-06 23:52:26]

On 2013/09/06 23:44:20, enne wrote:
> This is epenner's two character patch from
> https://codereview.chromium.org/23440005/ that fixes precision loss, but also
> adds a test.
> 
> I verified this test fails on both Linux and Android and is fixed by the two
> character change.  As a bonus, changing transformation matrices to float also
> fixes this bug, but this test will prevent future regressions.

LGTM, and thanks!

[aelias_OOO_until_Jul13, 2013-09-06 23:59:20]

Drive-by: would a cleaner fix be to switch contents_scale_x/y to return a value
of type SkMScalar?  It would make it more explicit that we're matching the
precision of the matrices, and avoid forcing a promotion to double if we do
switch to float matrices.

[epennerAtGoogle, 2013-09-07 00:00:32]

https://codereview.chromium.org/23572033/diff/3001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/23572033/diff/3001/cc/output/gl_renderer.cc#n...
cc/output/gl_renderer.cc:1241: Settings().allow_antialiasing &&
!quad->force_anti_aliasing_off &&
Nice short circuit.

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

On 2013/09/06 23:59:20, aelias wrote:
> Drive-by: would a cleaner fix be to switch contents_scale_x/y to return a
value
> of type SkMScalar?  It would make it more explicit that we're matching the
> precision of the matrices, and avoid forcing a promotion to double if we do
> switch to float matrices.

Done.

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

On 2013/09/07 00:07:06, enne wrote:
> On 2013/09/06 23:59:20, aelias wrote:
> > Drive-by: would a cleaner fix be to switch contents_scale_x/y to return a
> value
> > of type SkMScalar?  It would make it more explicit that we're matching the
> > precision of the matrices, and avoid forcing a promotion to double if we do
> > switch to float matrices.
> 
> Done.

Er, or more specifically, I used an SkMScalar in LayerTreeHostCommon.  Changing
contents scale to be an SkMScalar everywhere seemed more invasive.

[aelias_OOO_until_Jul13, 2013-09-07 00:14:39]

lgtm.  I like how it makes it clear that some funny business is going on :)

[epennerAtGoogle, 2013-09-07 00:20:10]

On 2013/09/07 00:14:39, aelias wrote:
> lgtm.  I like how it makes it clear that some funny business is going on :)

Good call.

Btw, changing all scales to double might even cause problems for doubles in some
obscure cases (If you want perfect == comparisons after doing some math). So
this is best IMO.

[danakj, 2013-09-07 00:54:59]

LGTM

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

https://codereview.chromium.org/23572033/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:1412: SkMScalar one(1.0);
nit: use SK_MScalar1? or maybe not?

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

https://codereview.chromium.org/23572033/diff/6001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl_unittest.cc:6055: class FakeGLRenderer: public
GLRenderer {
nit: GLRendererWithSetupQuadForAntialiasing (this will collide with a globally
defined FakeGLRenderer one day I am certain).

[enne (OOO), 2013-09-07 01:25:18]

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

https://codereview.chromium.org/23572033/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:1412: SkMScalar one(1.0);
On 2013/09/07 00:54:59, danakj wrote:
> nit: use SK_MScalar1? or maybe not?

Sure, done.

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

https://codereview.chromium.org/23572033/diff/6001/cc/trees/layer_tree_host_i...
cc/trees/layer_tree_host_impl_unittest.cc:6055: class FakeGLRenderer: public
GLRenderer {
On 2013/09/07 00:54:59, danakj wrote:
> nit: GLRendererWithSetupQuadForAntialiasing (this will collide with a globally
> defined FakeGLRenderer one day I am certain).

Done.

[commit-bot: I haz the power, 2013-09-07 01:26:09]

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

[commit-bot: I haz the power, 2013-09-07 04:29:31]

Change committed as 221889