[SPv2] Begin to convert the Blink transform tree to cc.

[SPv2] Begin to convert the Blink transform tree to cc.

This is a first step toward building cc transform nodes from their Blink
counterparts. No squashing is done, and there is still data such as 3D rendering
contexts missing from this.

This maintains the same state of working that poster circle, or similar pages,
had beforehand, but the matrices are no longer multiplied by Blink, but instead
produce separate transform nodes visible to the compositor.

Since transform nodes still require an owning layer, fictitious sibling layers
are created for each transform node and attached to the root layer, but these
have no children and draw no content. They should be eliminated once cc no
longer expects them to exist.

Some more unit tests are migrated from the non-flat test cases, including a
check that distinct transform nodes are assigned.

To make it easier to inspect these trees now that they're non-trivial, an
additional trace event for property trees received (rather than built) by cc
was added.

BUG=563667
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/8afe68e6246bc2dd7080bf4fac3b6fea83d79297
Cr-Commit-Position: refs/heads/master@{#402466}

[jbroman, 2016-06-27 17:49:45]

Description was changed from

==========
[SPv2] Begin to convert the Blink transform tree to cc.

DO NOT SUBMIT: This CL needs a better description.

BUG=
==========

to

==========
[SPv2] Begin to convert the Blink transform tree to cc.

DO NOT SUBMIT: This CL needs a better description.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jbroman, 2016-06-27 20:31:01]

Description was changed from

==========
[SPv2] Begin to convert the Blink transform tree to cc.

DO NOT SUBMIT: This CL needs a better description.

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[SPv2] Begin to convert the Blink transform tree to cc.

This is a first step toward building cc transform nodes from their Blink
counterparts. No squashing is done, and there is still data such as 3D rendering
contexts missing from this.

This maintains the same state of working that poster circle, or similar pages,
had beforehand, but the matrices are no longer multiplied by Blink, but instead
produce separate transform nodes visible to the compositor.

Since transform nodes still require an owning layer, fictitious sibling layers
are created for each transform node and attached to the root layer, but these
have no children and draw no content. They should be eliminated once cc no
longer expects them to exist.

Some more unit tests are migrated from the non-flat test cases, including a
check that distinct transform nodes are assigned.

To make it easier to inspect these trees now that they're non-trivial, an
additional trace event for property trees received (rather than built) by cc
was added.

BUG=563667
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jbroman, 2016-06-27 20:31:35]

jbroman@chromium.org changed reviewers:
+ pdr@chromium.org, trchen@chromium.org

[jbroman, 2016-06-27 20:31:36]

This will have non-trivial merge conflicts with trchen's in-flight CLs,
unfortunately, but I thought I'd send this out anyhow.

[jbroman, 2016-06-27 20:34:29]

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
(right):

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h:57:
Vector<scoped_refptr<cc::Layer>> contentLayers;
I'm not thrilled with this, but I need some way to figure out which layers to
look at, and this seemed more reasonable to me than baking assumptions about
where exactly when the synthetic layers get attached into the unit tests.

[trchen, 2016-06-27 21:39:19]

lgtm except one variable naming. Sorry for blocking you!

I think our CL won't have serious conflict because transform and effects almost
have no interactionl (with the exception of blur filter). So far all people I
talked to agree that render surface should be drawn in device space (i.e.
transform node #0). Clip nodes are tough though.

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(right):

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:406:
cc::Layer* m_dummyTransformParent;
This made me thought it's some internal state to create a dummy tree. I think we
should just call it m_rootLayer. The dummy layer is only a temporary workaround,
and it is very unlikely we use any other layer to host the dummy layers.

[jbroman, 2016-06-27 21:53:57]

jbroman@chromium.org changed reviewers:
+ ajuma@chromium.org

[jbroman, 2016-06-27 21:53:58]

+ajuma for cc

On 2016/06/27 at 21:39:19, trchen wrote:
> lgtm except one variable naming. Sorry for blocking you!
> 
> I think our CL won't have serious conflict because transform and effects
almost have no interactionl (with the exception of blur filter).

Don't worry; I was just worried about leaping in front of your CL and causing
you a headache. :)

I think there might be some interaction because cc has expectations that every
render surface has an effect node, a clip node, _and_ a transform node (IIUC).
But we might not hit that problem right away.

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(right):

https://codereview.chromium.org/2101683002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:406:
cc::Layer* m_dummyTransformParent;
On 2016/06/27 at 21:39:19, trchen wrote:
> This made me thought it's some internal state to create a dummy tree. I think
we should just call it m_rootLayer. The dummy layer is only a temporary
workaround, and it is very unlikely we use any other layer to host the dummy
layers.

OK, done. It doesn't have to be the root (we could have some single child of the
root to which we attach all the synthetic layers, for instance), but since it
is, that does seem reasonable.

[ajuma, 2016-06-27 23:04:36]

cc lgtm

[pdr., 2016-06-27 23:09:23]

LGTM with or without a suggestion

https://codereview.chromium.org/2101683002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(right):

https://codereview.chromium.org/2101683002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:398:
int compositorIdForNode(const TransformPaintPropertyNode*);
WDYT about making the compositor ids an internal implementation detail of this
class?

So, instead of:
void PaintArtifactCompositor::updateInLayerListMode(...) {
    int transformId =
transformTreeManager.compositorIdForNode(paintChunk.properties.transform.get());
    ...
    layer->SetTransformTreeIndex(transformId);

You'd do something like:
void PaintArtifactCompositor::updateInLayerListMode(...) {
   
transformTreeManager.updateTransformLayerCompositorId(paintChunk.properties.transform.get(),
layer);

[jbroman, 2016-06-28 14:15:16]

On 2016/06/27 at 23:09:23, pdr wrote:
> LGTM with or without a suggestion
> 
>
https://codereview.chromium.org/2101683002/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
(right):
> 
>
https://codereview.chromium.org/2101683002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp:398:
int compositorIdForNode(const TransformPaintPropertyNode*);
> WDYT about making the compositor ids an internal implementation detail of this
class?
> 
> So, instead of:
> void PaintArtifactCompositor::updateInLayerListMode(...) {
>     int transformId =
transformTreeManager.compositorIdForNode(paintChunk.properties.transform.get());
>     ...
>     layer->SetTransformTreeIndex(transformId);
> 
> You'd do something like:
> void PaintArtifactCompositor::updateInLayerListMode(...) {
>    
transformTreeManager.updateTransformLayerCompositorId(paintChunk.properties.transform.get(),
layer);

I'd rather not, because I think it obscures the fact that we set all of the
property tree indices, which we currently can do all in one place. I wouldn't
object to such a refactor if, in the future, that changed.

[jbroman, 2016-06-28 14:15:24]

The CQ bit was checked by jbroman@chromium.org

[jbroman, 2016-06-28 14:15:24]

The patchset sent to the CQ was uploaded after l-g-t-m from trchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2101683002/#ps40001
(title: "dummyTransformParent -> rootLayer")

[commit-bot: I haz the power, 2016-06-28 14:15:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-28 14:49:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-28 14:51:31]

Description was changed from

==========
[SPv2] Begin to convert the Blink transform tree to cc.

This is a first step toward building cc transform nodes from their Blink
counterparts. No squashing is done, and there is still data such as 3D rendering
contexts missing from this.

This maintains the same state of working that poster circle, or similar pages,
had beforehand, but the matrices are no longer multiplied by Blink, but instead
produce separate transform nodes visible to the compositor.

Since transform nodes still require an owning layer, fictitious sibling layers
are created for each transform node and attached to the root layer, but these
have no children and draw no content. They should be eliminated once cc no
longer expects them to exist.

Some more unit tests are migrated from the non-flat test cases, including a
check that distinct transform nodes are assigned.

To make it easier to inspect these trees now that they're non-trivial, an
additional trace event for property trees received (rather than built) by cc
was added.

BUG=563667
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[SPv2] Begin to convert the Blink transform tree to cc.

This is a first step toward building cc transform nodes from their Blink
counterparts. No squashing is done, and there is still data such as 3D rendering
contexts missing from this.

This maintains the same state of working that poster circle, or similar pages,
had beforehand, but the matrices are no longer multiplied by Blink, but instead
produce separate transform nodes visible to the compositor.

Since transform nodes still require an owning layer, fictitious sibling layers
are created for each transform node and attached to the root layer, but these
have no children and draw no content. They should be eliminated once cc no
longer expects them to exist.

Some more unit tests are migrated from the non-flat test cases, including a
check that distinct transform nodes are assigned.

To make it easier to inspect these trees now that they're non-trivial, an
additional trace event for property trees received (rather than built) by cc
was added.

BUG=563667
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/8afe68e6246bc2dd7080bf4fac3b6fea83d79297
Cr-Commit-Position: refs/heads/master@{#402466}
==========

[commit-bot: I haz the power, 2016-06-28 14:51:33]

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