New issue
Advanced search Search tips

Issue 836897 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Feature


Sign in to add a comment

Make animations work end-to-end with blink property trees

Project Member Reported by pdr@chromium.org, Apr 25 2018

Issue description

This is a tracking bug for making animations work end-to-end with blink property trees. The primary change will be to update composited animations in PaintArtifactCompositor using blink property trees. This has been partially implemented but the design needs to be fully validated and tested.

A component of this work is to remove layer_id as a key from the cc animations code, and instead use ElementIds.
 

Comment 1 by pdr@chromium.org, Apr 30 2018

I've just landed a patch that sets up the test infrastructure for this bug (https://crrev.com/3eb645cc).

A goal for this bug is to pass the following layout tests with --enable-blink-gen-property-trees:
animations/
transitions/
virtual/threaded/animations/
virtual/threaded/transitions/
irtual/threaded/fast/animationworklet/

These can be run with the following one-liner:
third_party/blink/tools/run_web_tests.py --debug --additional-driver-flag=--enable-blink-gen-property-trees animations transitions virtual/threaded/animations virtual/threaded/transitions virtual/threaded/fast/animationworklet

There are currently only 20 failures listed in .../LayoutTests/FlagExpectations/enable-blink-gen-property-trees
Owner: petermayo@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, May 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8795e724ca85409b52952de8573ab002694ff61

commit c8795e724ca85409b52952de8573ab002694ff61
Author: Peter Mayo <petermayo@chromium.org>
Date: Fri May 11 18:09:06 2018

Stabilize test expectations for enable-blink-gen-property-trees

repeat
  run layout tests with addition-driver-flag=enable-blink-gen-property-trees
  merge observations with expectations
until no unexpected results seen

6 rounds for that sync and build.

Bug: 836897
Change-Id: Ic5145522cc6de2ffce7b073fccba25d202c8790b
Reviewed-on: https://chromium-review.googlesource.com/1047187
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557939}
[modify] https://crrev.com/c8795e724ca85409b52952de8573ab002694ff61/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees

Comment 4 by pdr@chromium.org, May 14 2018

Blockedon: 842937
Blockedon: 845613
Project Member

Comment 6 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d4b496fb58d979f8b511519e93f8c6284f7bf25

commit 5d4b496fb58d979f8b511519e93f8c6284f7bf25
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed May 23 17:09:47 2018

BlinkGenPropertyTrees: Only call DocumentAnimations::UpdateAnimations once

Before this CL we were calling DocumentAnimations::UpdateAnimations
twice for BlinkGenPropertyTrees; once during compositing and once during
paint. Because BlinkGenPropertyTrees uses the same non-drawable layer
elimination as SPv2, it should use the latter callsite.

Bug: 836897,  845613 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia9a725285f14cc5e0b085d83ebb047d4d96d16bb
Reviewed-on: https://chromium-review.googlesource.com/1068626
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561129}
[modify] https://crrev.com/5d4b496fb58d979f8b511519e93f8c6284f7bf25/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[modify] https://crrev.com/5d4b496fb58d979f8b511519e93f8c6284f7bf25/third_party/blink/renderer/core/animation/animation.cc
[modify] https://crrev.com/5d4b496fb58d979f8b511519e93f8c6284f7bf25/third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d7d73f5164b90b5a1f840a4928744f811f64c54

commit 1d7d73f5164b90b5a1f840a4928744f811f64c54
Author: Robert Flack <flackr@chromium.org>
Date: Mon May 28 17:29:36 2018

Convert element to layer map to just track elements in list.

The element_layers_map_ is only used to determine when elements are in
the active or pending tree for the purposes of ticking animations. This
patch converts the element layers map to a set tracking which element
ids are present.

Bug: 836897
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:linux-blink-gen-property-trees
Change-Id: I858a00d4f80fc5ac36a21e5d4cf21e0e379abdec
Reviewed-on: https://chromium-review.googlesource.com/1071072
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562286}
[modify] https://crrev.com/1d7d73f5164b90b5a1f840a4928744f811f64c54/cc/layers/layer_impl.cc
[modify] https://crrev.com/1d7d73f5164b90b5a1f840a4928744f811f64c54/cc/layers/scrollbar_layer_unittest.cc
[modify] https://crrev.com/1d7d73f5164b90b5a1f840a4928744f811f64c54/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/1d7d73f5164b90b5a1f840a4928744f811f64c54/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/1d7d73f5164b90b5a1f840a4928744f811f64c54/cc/trees/layer_tree_impl.h

Comment 8 by bokan@chromium.org, Jun 15 2018

Note: I had to disable GraphicsLayerTest.updateLayerShouldFlattenTransformWithAnimations when run with blink-gen-property-trees in https://crrev.com/c/1083012 because we now run unit tests with layer lists enabled (if BGPT is on). This causes this test to fail because we can't set an element id on a layer when layer lists is enabled in CC. This looks like some unfinished animation work so we should remember to turn it back of as part of this.
Blockedon: 855691
Blockedon: 855702
Blockedon: 855688
Blockedon: 855718
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a65105c1eaaf765382a2f52c516197c6f5ae2fbd

commit a65105c1eaaf765382a2f52c516197c6f5ae2fbd
Author: Peter Mayo <petermayo@chromium.org>
Date: Mon Jun 25 20:14:06 2018

[BlinkGenPropertyTrees] Group 836897  failures

Add new bug numbers for types of failures seen in animations failures
when enabling blink-gen-property-trees.

TBR=flackr@chromium.org
BUG=836897

Change-Id: I3375acfaac7470834de65e905714e5244b1715c3
Reviewed-on: https://chromium-review.googlesource.com/1113904
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570149}
[modify] https://crrev.com/a65105c1eaaf765382a2f52c516197c6f5ae2fbd/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees

Blockedon: 872773
Blockedon: 873673
Blockedon: 874251
Blockedon: 874253
Blockedon: -873673
Status: Fixed (was: Started)
Every bug is now either unblocked or closed so I think we can close this metabug.
Blockedon: 888260
Blockedon: 889806
Cc: petermayo@chromium.org
Owner: flackr@chromium.org
Temporarily re-opening due to two bugs discovered when running with bgpt enabled.
Status: Started (was: Fixed)
Blockedon: 891716
Blockedon: 896549
Blockedon: 898176
Blockedon: 905861
Blockedon: -891716
Blockedon: -898176
Blockedon: -905861
Status: Fixed (was: Started)
Blockedon: 912337
Blockedon: 912380
Blockedon: 912574
Blockedon: 909055
Status: Assigned (was: Fixed)
Reopening the metabug that keeps on giving :-).
Blockedon: 909054
Blockedon: 913170
Blockedon: 921105
Blockedon: 922035

Sign in to add a comment