Last Comment Bug 702739 - Pre-render overflow area of frames translated by CSS transforms, that are active and partially visible
: Pre-render overflow area of frames translated by CSS transforms, that are act...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on: 717521
Blocks: 706179
  Show dependency treegraph
 
Reported: 2011-11-15 13:11 PST by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-03-02 16:43 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix


Attachments
wip (5.78 KB, patch)
2011-12-14 02:15 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
no flags Details | Diff | Splinter Review
Don't dirty the transformed frame when we reflow it due to a transform change (2.78 KB, patch)
2011-12-14 17:22 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
mats: review+
Details | Diff | Splinter Review
prerendering fix (16.40 KB, patch)
2011-12-14 17:29 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
mats: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-11-15 13:11:22 PST
For the "gaia" user interface of b2g (demo http://andreasgal.github.com/gaia/), we have a lock screen that's simply a <div> moved around by a CSS transform (translation).  It's the "sunset" screen in the demo.  The CSS transform isn't animated by CSS animations, but rather the translation is just adjusted by script based on input events.

On device, the lock screen is ridiculously slow with GL enabled, probably somewhere around 3-5 fps.  With GL off, it's better but still slow.  Based on evidence from previous bugs, I'm guessing this is a combination of three factors
 (1) sizing the backing buffer to the <div>'s actual visible region.  This means that as it's moved into place, it slowly grows, forcing a realloc on each resize.
 (2) related to (1), having to continuously repaint because of the buffer realloc.
 (3) slowness when uploading to textures

(3) is orthogonal to layout/layers issues, but since the perf with GL disabled is noticeably better, I'm guessing that that's what's dominating this testcase.  However, fixing (1) and (2) is still worthwhile, in particular because it's a necessary condition for async CSS animations.

To fix (1) and (2) for this test, I think all we need to do is the following
 - when we're building a layer for a DisplayTransform (which implies that the underlying frame is visible)
   - if the transform is a translation
     - if the computed size of the underlying frame is within some bound (screen size would suffice for this testcase; probably want to make a make a bound on number of pixels instead of dimensions.  Maybe maxPixels = screenWidth * screenHeight * 2?)
       - create a layer for the transformed frame with a "fudged" visible region for its computed size
       - paint the layer into that fudged visible region

With this special case, as long as the transformed frame was visible and active, it would never be repainted.  This would have the side effect of "working around" the texture-uploading problem we're seeing with EGL.

When animated transforms are involved, there's much more cool stuff we can do, but that's work for another bug ...
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-11-15 19:46:05 PST
FTR: in a build off of latest m-c, which has some texture-upload improvements, my test is indeed noticeably faster.  But still not fast enough! ;)
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-11 08:54:46 PST
Focusing this bug for better action.

There are several simple animations in the b2g UI ("gaia") for which the framerate is too low.  All but one of them are simple animated translations.  I'm 99% sure this bug will significantly improve the perf there.  If that's the case for gaia, I suspect this might be true of general web content too.

Is there anyone on the layout or gfx teams who can up this bug?
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-11 09:42:03 PST
* who can *pick* up this bug

I should also add that the other simple animation is an animated scale+transform, which can be optimized similarly to translation-only but is slightly more complicated.
Comment 4 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-11 13:32:35 PST
Bug 524925 would probably help with your issues too.
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-14 02:15:58 PST
Created attachment 581567 [details] [diff] [review]
wip

Something like this might work.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-14 17:22:49 PST
Created attachment 581845 [details] [diff] [review]
Don't dirty the transformed frame when we reflow it due to a transform change
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-14 17:25:43 PST
The above patch is needed to make the tests in the next patch pass. It avoids having to reflow the children of the transformed frame when the transform changes, and avoids invalidations triggered by those reflows. It's simple and should be safe.

Bug 524925 is much better and obsoletes the above patch. But we should take the above patch anyway.
Comment 8 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-14 17:29:45 PST
Created attachment 581848 [details] [diff] [review]
prerendering fix

This is very simple actually. The heuristic in nsDisplayTransform::ShouldPrerenderTransformedContent might need to be tweaked later but it should be good enough for now.
Comment 9 Mats Palmgren (:mats) 2011-12-14 17:59:08 PST
Is this needed before Dec 20?  if so, and if bug 524925 fixes this issue completely
and better, how would you feel about taking that instead (without bug 665597)?
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-14 20:28:21 PST
(In reply to Mats Palmgren [:mats] from comment #9)
> Is this needed before Dec 20?

I don't know. The patch is small, I think we could take it before the branch.

> if so, and if bug 524925 fixes this issue
> completely and better, how would you feel about taking that instead (without bug
> 665597)?

Bug 524925 only obsoletes part 1, which is itself a very small and safe patch. I think we should just take part 1 before the branch and land 524925 after the branch.

Plus, bug 524925 does need bug 665597. See https://bugzilla.mozilla.org/show_bug.cgi?id=524925#c91
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-15 15:49:25 PST
Thanks roc!  We'll test this in b2g asap.
Comment 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-17 01:02:16 PST
Is this ready for checkin-needed?
Comment 14 Matt Brubeck (:mbrubeck) 2011-12-18 08:18:46 PST
Backed out on inbound (along with other patches in the same push) because of Mac and Win7 reftest failures:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff088809cd2a
Comment 15 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-18 13:59:04 PST
This didn't cause the failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/27cc305a5a1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/89a8e26f1df0
Comment 16 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-19 00:16:23 PST
(In reply to Chris Jones [:cjones] [:warhammer] from comment #11)
> Thanks roc!  We'll test this in b2g asap.

These don't seem to be kicking in where they should in b2g, but all those frames using transitions or animations, so there may be a bad interaction there.  Will file a followup.
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-12-19 00:24:42 PST
You can set a breakpoint in nsDisplayTransform::ComputeVisibility to see if the prerendering path is kicking in.
Comment 19 Mats Palmgren (:mats) 2012-03-02 07:04:18 PST
The backout was only for Fx11, so I think we can resolve this as Fixed again, but
with milestone:mozilla12 and status-firefox11:wontfix -- does that sound right?
Comment 20 Timothy Nikkel (:tnikkel) 2012-03-02 16:43:00 PST
Yes, I think so.

Note You need to log in before you can comment on or make changes to this bug.