cc: Make heavy layer tree test more realistic

cc: Make heavy layer tree test more realistic

The heavy layer tree test is supposed to simulate a complex layer tree
with transform animation. Currently it does so by forcing repaint of all
layers, but we should only really animate layer transforms to better
match reality.

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

[chromium-reviews, 2014-05-29 12:25:23]

Its forcing redraw which damages the scene but does not cause any painting
right?
On May 29, 2014 1:10 PM, <skyostil@chromium.org> wrote:

> Reviewers: enne,
>
> Description:
> cc: Make heavy layer tree test more realistic
>
> The heavy layer tree test is supposed to simulate a complex layer tree
> with transform animation. Currently it does so by forcing repaint of all
> layers, but we should only really animate layer transforms to better
> match reality.
>
> BUG=348100
>
> Please review this at https://codereview.chromium.org/305903003/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+5, -1 lines):
>   M cc/trees/layer_tree_host_perftest.cc
>
>
> Index: cc/trees/layer_tree_host_perftest.cc
> diff --git a/cc/trees/layer_tree_host_perftest.cc
> b/cc/trees/layer_tree_host_perftest.cc
> index 70b2a645236b977abcdce3f542c90ebf4015d9ca..
> 5a938af9c3181182658467e60a8eb47332376406 100644
> --- a/cc/trees/layer_tree_host_perftest.cc
> +++ b/cc/trees/layer_tree_host_perftest.cc
> @@ -38,6 +38,7 @@ class LayerTreeHostPerfTest : public LayerTreeTest {
>                      base::TimeDelta::FromMilliseconds(kTimeLimitMillis),
>                      kTimeCheckInterval),
>          commit_timer_(0, base::TimeDelta(), 1),
> +        animation_frame_(0),
>          full_damage_each_frame_(false),
>          animation_driven_drawing_(false),
>          measure_commit_cost_(false) {
> @@ -56,7 +57,9 @@ class LayerTreeHostPerfTest : public LayerTreeTest {
>    virtual void Animate(base::TimeTicks monotonic_time) OVERRIDE {
>      if (animation_driven_drawing_ && !TestEnded()) {
>        layer_tree_host()->SetNeedsAnimate();
> -      layer_tree_host()->SetNextCommitForcesRedraw();
> +      // Simulate a transform animation on the root layer.
> +      Layer* layer = layer_tree_host()->root_layer();
> +      layer->SetPosition(gfx::PointF(0, animation_frame_++ % 2));
>      }
>    }
>
> @@ -104,6 +107,7 @@ class LayerTreeHostPerfTest : public LayerTreeTest {
>   protected:
>    LapTimer draw_timer_;
>    LapTimer commit_timer_;
> +  int animation_frame_;
>
>    std::string test_name_;
>    FakeContentLayerClient fake_content_layer_client_;
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Sami, 2014-05-29 16:09:42]

On 2014/05/29 12:25:23, chromium-reviews_chromium.org wrote:
> Its forcing redraw which damages the scene but does not cause any painting
> right?

Right, it's not actually painting anything in the test so this is somewhat
academic :) I think forcing repaints still causes some extra work for tile
management, so moving the layers around instead is a tad more realistic.

[enne (OOO), 2014-05-29 17:51:06]

This does seem more legit.  lgtm

[Sami, 2014-05-29 17:54:59]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2014-05-29 17:58:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/skyostil@chromium.org/305903003/1

[commit-bot: I haz the power, 2014-05-29 23:06:18]

Change committed as 273646

[danakj, 2014-05-30 01:02:13]

On 2014/05/29 16:09:42, Sami wrote:
> On 2014/05/29 12:25:23, http://chromium-reviews_chromium.org wrote:
> > Its forcing redraw which damages the scene but does not cause any painting
> > right?
> 
> Right, it's not actually painting anything in the test so this is somewhat
> academic :) I think forcing repaints still causes some extra work for tile
> management, so moving the layers around instead is a tad more realistic.

Sorry, I don't understand why you are seeing repaints before this change.
Setting damage on the frame just makes us redraw the full viewport from our
existing resources. Why would moving the root layer's position be dramatically
different than setting full frame damage?

I'm worried we're breaking this test more here.

[Sami, 2014-05-30 09:49:54]

On 2014/05/30 01:02:13, danakj (OOO_back_june_6) wrote:
> Sorry, I don't understand why you are seeing repaints before this change.
> Setting damage on the frame just makes us redraw the full viewport from our
> existing resources. Why would moving the root layer's position be dramatically
> different than setting full frame damage?
> 
> I'm worried we're breaking this test more here.

Hmm, I think you are right and I may have reached the wrong conclusion here.
LTH::SetNextCommitForcesRedraw makes LTH *act* as if it painted content (in
UpdateLayers), but it doesn't actually trigger any painting -- in tests or
otherwise. Like you said, the end result should be the same with both
SetNextCommitForcesRedraw and through wiggling the layers around, except there
is a bit of additional property pushing going on in the latter case.

The difference doesn't seem to be measurable so I'll revert to get us back to
the simpler times.

[Sami, 2014-05-30 09:50:19]

A revert of this CL has been created in
https://codereview.chromium.org/306023006/ by skyostil@chromium.org.

The reason for reverting is: Actually what we had was working fine..