702739 - Pre-render overflow area of frames translated by CSS transforms, that are active and partially visible

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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]

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]

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]

* 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)]

Bug 524925 would probably help with your issues too.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: wip

Something like this might work.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: 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)]

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)]

Attachment: 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 (inactive)]

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)]

(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]

Thanks roc!  We'll test this in b2g asap.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Is this ready for checkin-needed?

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06e8ee7aa95d
https://hg.mozilla.org/integration/mozilla-inbound/rev/2674bffd5575

Comment 14

[Matt Brubeck (:mbrubeck)]

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)]

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]

(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)]

You can set a breakpoint in nsDisplayTransform::ComputeVisibility to see if the prerendering path is kicking in.

Comment 18

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/973938d037a9
https://hg.mozilla.org/mozilla-central/rev/27cc305a5a1a

Comment 19

[Mats Palmgren (inactive)]

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)]

Yes, I think so.