Closed
Bug 702739
Opened 13 years ago
Closed 13 years ago
Pre-render overflow area of frames translated by CSS transforms, that are active and partially visible
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla12
Tracking | Status | |
---|---|---|
firefox11 | --- | wontfix |
People
(Reporter: cjones, Assigned: roc)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
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 ...
Reporter | ||
Comment 1•13 years ago
|
||
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! ;)
Reporter | ||
Comment 2•13 years ago
|
||
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?
Summary: Add some special cases for CSS transforms in layer builder → Pre-render overflow area of frames translated by CSS transforms, that are active and partially visible
Reporter | ||
Comment 3•13 years ago
|
||
* 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.
Assignee | ||
Comment 4•13 years ago
|
||
Bug 524925 would probably help with your issues too.
Assignee | ||
Comment 5•13 years ago
|
||
Something like this might work.
Assignee | ||
Comment 6•13 years ago
|
||
Assignee: nobody → roc
Attachment #581845 -
Flags: review?(matspal)
Assignee | ||
Comment 7•13 years ago
|
||
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.
Assignee | ||
Comment 8•13 years ago
|
||
This is very simple actually. The heuristic in nsDisplayTransform::ShouldPrerenderTransformedContent might need to be tweaked later but it should be good enough for now.
Attachment #581567 -
Attachment is obsolete: true
Attachment #581848 -
Flags: review?(matspal)
Comment 9•13 years ago
|
||
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)?
Assignee | ||
Comment 10•13 years ago
|
||
(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
Updated•13 years ago
|
Attachment #581848 -
Flags: review?(matspal) → review+
Updated•13 years ago
|
Attachment #581845 -
Flags: review?(matspal) → review+
Reporter | ||
Comment 11•13 years ago
|
||
Thanks roc! We'll test this in b2g asap.
Reporter | ||
Comment 12•13 years ago
|
||
Is this ready for checkin-needed?
Assignee | ||
Comment 13•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/06e8ee7aa95d
https://hg.mozilla.org/integration/mozilla-inbound/rev/2674bffd5575
Whiteboard: [inbound]
Comment 14•13 years ago
|
||
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
Whiteboard: [inbound]
Assignee | ||
Comment 15•13 years ago
|
||
This didn't cause the failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/27cc305a5a1a
https://hg.mozilla.org/integration/mozilla-inbound/rev/89a8e26f1df0
Whiteboard: [inbound]
Reporter | ||
Comment 16•13 years ago
|
||
(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.
Assignee | ||
Comment 17•13 years ago
|
||
You can set a breakpoint in nsDisplayTransform::ComputeVisibility to see if the prerendering path is kicking in.
Comment 18•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/973938d037a9
https://hg.mozilla.org/mozilla-central/rev/27cc305a5a1a
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla11
Updated•13 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•13 years ago
|
||
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•13 years ago
|
||
Yes, I think so.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
status-firefox11:
--- → wontfix
Resolution: --- → FIXED
Target Milestone: mozilla11 → mozilla12
You need to log in
before you can comment on or make changes to this bug.
Description
•