Add localToAbsoluteTransform and localToAncestorTransform.

Add localToAbsoluteTransform and localToAncestorTransform.

These interfaces on LayoutObject will be used to more efficiently
represent object locations in the accessibility tree, rather than storing
everything using absolute coordinates.

BUG=551601
Committed: https://crrev.com/3dd9a3d87d5a495f040021cb7c0120d25b5c5743
Cr-Commit-Position: refs/heads/master@{#381616}

[dmazzoni, 2016-03-08 18:00:31]

dmazzoni@chromium.org changed reviewers:
+ chrishtr@chromium.org

[szager1, 2016-03-08 23:34:14]

szager@chromium.org changed reviewers:
+ szager@chromium.org

[szager1, 2016-03-08 23:34:15]

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:167: const float
kMaxError = 0.5;
Internally, TransformState uses LayoutUnit's.  I kinda think this should be
LayoutUnit::epsilon(), and use sqrt() from <math.h> to generate the expected
values.

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:169: // Sotate (100,
0) 90 degrees to (0, 100)
typo

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/geometry/TransformState.cpp (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/geometry/TransformState.cpp:122:
applyAccumulatedOffset();
I think this needs to be wrappend in a (accumulate == FlattenTransform) check;
chrishtr, can you comment?

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/geometry/TransformState.h (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/geometry/TransformState.h:140: bool
m_forceAccumulatingTransform;
Please reorder to keep all the bool's together.

[dmazzoni, 2016-03-09 18:15:00]

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:167: const float
kMaxError = 0.5;
On 2016/03/08 23:34:15, szager1 wrote:
> Internally, TransformState uses LayoutUnit's.  I kinda think this should be
> LayoutUnit::epsilon(), and use sqrt() from <math.h> to generate the expected
> values.

Done.

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:169: // Sotate (100,
0) 90 degrees to (0, 100)
On 2016/03/08 23:34:15, szager1 wrote:
> typo

Done.

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/geometry/TransformState.cpp (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/geometry/TransformState.cpp:122:
applyAccumulatedOffset();
On 2016/03/08 23:34:15, szager1 wrote:
> I think this needs to be wrappend in a (accumulate == FlattenTransform) check;
> chrishtr, can you comment?

I don't think it's needed because the accumulated offset should be zero when
m_forceAccumulatingTransform is true, but let me know if I'm missing something

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/geometry/TransformState.h (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/geometry/TransformState.h:140: bool
m_forceAccumulatingTransform;
On 2016/03/08 23:34:15, szager1 wrote:
> Please reorder to keep all the bool's together.

Done.

[szager1, 2016-03-09 18:44:14]

l-g-t-m, but please wait for chrishtr

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/geometry/TransformState.cpp (right):

https://codereview.chromium.org/1777613002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/geometry/TransformState.cpp:122:
applyAccumulatedOffset();
On 2016/03/09 18:14:59, dmazzoni wrote:
> On 2016/03/08 23:34:15, szager1 wrote:
> > I think this needs to be wrappend in a (accumulate == FlattenTransform)
check;
> > chrishtr, can you comment?
> 
> I don't think it's needed because the accumulated offset should be zero when
> m_forceAccumulatingTransform is true, but let me know if I'm missing something

Right you are, this is fine.

[chrishtr, 2016-03-10 16:53:39]

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:984: // Convert a local
quad into the coordinate system of container, taking transforms into account.
Copy the comment from ancestorToLocalQuad regarding what happens if
the |ancestor| argument is null.

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:125:
TEST_F(LayoutObjectTest, LocalToAbsoluteTransform)
Put this in MapCoordinatesTest instead. The logic for these methods
is spread across multiple subclasses of LayoutObject.

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/geometry/TransformState.cpp (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/geometry/TransformState.cpp:115: if
(m_forceAccumulatingTransform)
I don't think this works in the case of a flattening transform. The
correct transformation matrix should be the transform matrix of the
element, multiplied by a flattening transform matrix. I think this amounts to
applying the same logic as in gfx::Transform::FlattenTo2d().

[dmazzoni, 2016-03-14 20:10:37]

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:984: // Convert a local
quad into the coordinate system of container, taking transforms into account.
On 2016/03/10 16:53:38, chrishtr wrote:
> Copy the comment from ancestorToLocalQuad regarding what happens if
> the |ancestor| argument is null.

Done.

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObjectTest.cpp:125:
TEST_F(LayoutObjectTest, LocalToAbsoluteTransform)
On 2016/03/10 16:53:39, chrishtr wrote:
> Put this in MapCoordinatesTest instead. The logic for these methods
> is spread across multiple subclasses of LayoutObject.

Done.

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/geometry/TransformState.cpp (right):

https://codereview.chromium.org/1777613002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/geometry/TransformState.cpp:115: if
(m_forceAccumulatingTransform)
On 2016/03/10 16:53:39, chrishtr wrote:
> I don't think this works in the case of a flattening transform. The
> correct transformation matrix should be the transform matrix of the
> element, multiplied by a flattening transform matrix. I think this amounts to
> applying the same logic as in gfx::Transform::FlattenTo2d().

Good catch. Fixed this and added a new test.

[chrishtr, 2016-03-15 23:06:43]

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:999: // If the
LayoutBoxModelObject ancestor is non-null, the result will transform into the
space of the ancestor.
You can just refer to the comments for localToAncestorQuad for behavior w.r.t.
the
|ancestor| parameter, rather than copying the comment.

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/transforms/TransformationMatrix.h
(right):

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/transforms/TransformationMatrix.h:323: void
flattenTo2d();
Add a note that this is the same as gfx::TransformTo2d, and refer to the more
extensive comments there.

Also remove the note about lossy, or just rephrase to note that it is
a non-invertible transform.

[dmazzoni, 2016-03-16 05:42:01]

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:999: // If the
LayoutBoxModelObject ancestor is non-null, the result will transform into the
space of the ancestor.
On 2016/03/15 23:06:42, chrishtr wrote:
> You can just refer to the comments for localToAncestorQuad for behavior w.r.t.
> the
> |ancestor| parameter, rather than copying the comment.

Done.

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/transforms/TransformationMatrix.h
(right):

https://codereview.chromium.org/1777613002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/transforms/TransformationMatrix.h:323: void
flattenTo2d();
On 2016/03/15 23:06:42, chrishtr wrote:
> Add a note that this is the same as gfx::TransformTo2d, and refer to the more
> extensive comments there.
> 
> Also remove the note about lossy, or just rephrase to note that it is
> a non-invertible transform.

Done.

[chrishtr, 2016-03-16 22:03:35]

lgtm

[dmazzoni, 2016-03-16 22:09:32]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-03-16 22:10:07]

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

[commit-bot: I haz the power, 2016-03-17 00:56:24]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-17 00:57:26]

Description was changed from

==========
Add localToAbsoluteTransform and localToAncestorTransform.

These interfaces on LayoutObject will be used to more efficiently
represent object locations in the accessibility tree, rather than storing
everything using absolute coordinates.

BUG=551601
==========

to

==========
Add localToAbsoluteTransform and localToAncestorTransform.

These interfaces on LayoutObject will be used to more efficiently
represent object locations in the accessibility tree, rather than storing
everything using absolute coordinates.

BUG=551601

Committed: https://crrev.com/3dd9a3d87d5a495f040021cb7c0120d25b5c5743
Cr-Commit-Position: refs/heads/master@{#381616}
==========

[commit-bot: I haz the power, 2016-03-17 00:57:29]

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