Closed Bug 780342 Opened 9 years ago Closed 9 years ago

Don't allow compositor-driven animation of frames that are not prerendered, provide diagnostics for when that happens

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: cjones, Assigned: dzbarsky)

Details

Attachments

(1 file)

Like we do for async animations.
Summary: Provide diagnostics for if transformed frames hit the prerender optimization, and if not why not → Don't allow compositor-driven animation of frames that are not prerendered, provide diagnostics for when that happens
Attached patch PatchSplinter Review
Some animations aren't prerendered before the first frame, so they display the message once.  That is ok, but when you have a bunch of messages with the same frame pointer, that is an actual performance problem.
Attachment #649051 - Flags: review?(jones.chris.g)
Comment on attachment 649051 [details] [diff] [review]
Patch

>diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js

>+pref("layers.offmainthreadcomposition.log-animations", true);

This needs to be off by default.  Let's file a Boot2Gecko bugzilla bug
and a gaia issue for exposing a setting to enable this.

>diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp

>+AddAnimationsAndTransitionsToLayer(Layer* aLayer, nsDisplayListBuilder* aBuilder,
>+                                   nsDisplayItem* aItem, nsCSSProperty aProperty)
> {
 
>+  // If the frame is not prerendered, bail out.  Layout will still perform the
>+  // animation.
>+  if (!nsDisplayTransform::ShouldPrerenderTransformedContent(aBuilder, frame)) {
>+    if (nsLayoutUtils::IsAnimationLoggingEnabled()) {
>+      printf_stderr("Performance warning: Async animation disabled because frame (%p) is not prerendered\n", frame);

Can we pull something more identifying off the nsIContent for the
frame?  Like an element name and/or id attribute?

Can do that in a followup.

r=me with those changes.
Attachment #649051 - Flags: review?(jones.chris.g) → review+
Changed to use the tag name and id if there is one.
https://hg.mozilla.org/mozilla-central/rev/0fbe36ab8964
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Why isn't this using PR_LOG? You can follow nsGlobalWindow and force logging on for this module in opt builds. This is easy to fix and I think should be fixed before it becomes entrenched.
It should be using the console service.  Now that it's off by default, that should be OK.
it == this logging, I mean
OK. Whichever we replace it with, the code currently here is an anti-pattern and I'd like to see it removed before someone copies it.
You need to log in before you can comment on or make changes to this bug.