Closed
Bug 1104081
Opened 10 years ago
Closed 4 years ago
Async animations should not be disabled for large color layers
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
INVALID
tracking-b2g | backlog |
People
(Reporter: cwiiis, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Pre-rendering gets disabled on layers that are larger than the viewport, which has the side-effect of also disabling async animations on layers larger than the viewport.
We don't save anything by disabling pre-rendering on colour-layers, so we should at least special-case this one thing to enable async animations in more cases.
Reporter | ||
Comment 1•10 years ago
|
||
Following on from bug 1100357, here's a fix that should only apply to color layers. This fixes the issue that happens in the gaia homescreen.
Attachment #8527778 -
Flags: review?(matt.woodrow)
Comment 2•10 years ago
|
||
Comment on attachment 8527778 [details] [diff] [review]
Don't disable layer pre-rendering for large color layers
Review of attachment 8527778 [details] [diff] [review]:
-----------------------------------------------------------------
Sorry for the slow response.
Unfortunately this still isn't correct. The remaining unchanged caller of ShouldPrerenderTransformedContent is the one that adjusts the region that we build display items for. It's possible that the unmodified region contains only colors, but the full area of the transform would contain other content.
nsDisplayClearBackground is also not what you want, that's for erasing an area and is using for OSX native effects.
You probably want IsUniform(), and only for a single item, since multiple items that don't exactly overlap won't get a ColorLayer.
We're sort of torn between decisions that we need to make during display list building, and information we don't really have until Layer builder.
Let's talk about this next week, hopefully we can come up with something.
Attachment #8527778 -
Flags: review?(matt.woodrow)
Reporter | ||
Comment 3•10 years ago
|
||
Unfortunately I wasn't at Portland, can we still come up with a solution for this sometime soon though? We need this for the 2.2 release.
2.2? as this blocks app-grouping, which is a 2.2 feature.
blocking-b2g: --- → 2.2?
Flags: needinfo?(matt.woodrow)
Comment 4•10 years ago
|
||
roc, do you have any ideas for this?
The problem is that we don't know if the transformed content will contain only a ColorLayer (or display items that will build a ColorLayer) until after we've built the display list.
We don't want to build all display items that are within transforms regardless of visibility either, since that could have really bad edge cases that massively slow down display list building.
One possibility would be to have much more lenient (but still bounded) pre-rendering heuristics for the display list building phase to catch some of these cases. Then layer building will only actually apply pre-rendering if we hit the original requirement, or we only needed a ColorLayer.
Flags: needinfo?(matt.woodrow) → needinfo?(roc)
(In reply to Matt Woodrow (:mattwoodrow) from comment #4)
> The problem is that we don't know if the transformed content will contain
> only a ColorLayer (or display items that will build a ColorLayer) until
> after we've built the display list.
>
> We don't want to build all display items that are within transforms
> regardless of visibility either, since that could have really bad edge cases
> that massively slow down display list building.
>
> One possibility would be to have much more lenient (but still bounded)
> pre-rendering heuristics for the display list building phase to catch some
> of these cases. Then layer building will only actually apply pre-rendering
> if we hit the original requirement, or we only needed a ColorLayer.
That sounds reasonable.
Another approach would be to do a trial display list build with the full transform overflow area and abort it as soon as we find more than one item.
Flags: needinfo?(roc)
I prefer your suggestion though.
Reporter | ||
Comment 7•10 years ago
|
||
Given that this is unlikely to be fixed in time, we've decided we can live without it if need be. It would still be great to have this fixed for 2.2.
No longer blocks: app-grouping
Updated•10 years ago
|
Flags: needinfo?(milan)
Comment 8•10 years ago
|
||
Jet, do you think this should be 2.2+ or not? Judging by comment 7, I'd say, no, but...
Flags: needinfo?(milan) → needinfo?(bugs)
Comment 9•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #8)
> Jet, do you think this should be 2.2+ or not? Judging by comment 7, I'd
> say, no, but...
I agree.
Flags: needinfo?(bugs)
Comment 10•10 years ago
|
||
[Blocking Requested - why for this release]:
Per comment 8 and comment 9, and since FL is near, noming for 3.0? instead since this is in the master.
blocking-b2g: 2.2? → 3.0?
Comment 11•9 years ago
|
||
Hey Jet,
Do you think this can be done as part of 2.5? Thanks
Flags: needinfo?(bugs)
Comment 12•9 years ago
|
||
(In reply to Mahendra Potharaju [:mahe] from comment #11)
> Hey Jet,
>
> Do you think this can be done as part of 2.5? Thanks
We can probably pick this up along with bug 1100357 later in Q3 or early Q4. Does that work for your schedule? Is this one blocking a 2.5 feature?
Flags: needinfo?(bugs) → needinfo?(mpotharaju)
Comment 13•9 years ago
|
||
(In reply to Mahendra Potharaju [:mahe] from comment #11)
> Hey Jet,
>
> Do you think this can be done as part of 2.5? Thanks
Do you have specific use cases that you are hoping to improve with this bug?
I'd really like to see them so we can decide if this particular bug is the best way to solve them.
Adding this optimization is going to add a fair bit of code complexity, as well as runtime overhead (including in cases where it doesn't provide benefit), so I'm not in a rush to add it without knowing what we hope to gain from it.
Comment 14•9 years ago
|
||
Thanks Matt, Jet.
This is nominated for 2.5, was checking if this can come in as part of 2.5. As far as I know, this is not blocking any bug. If this needs more evaluation and use case weigh-in, I can put it in backlog. Please try to get this for Q4 as its coming from 2.0.
Flags: needinfo?(mpotharaju)
Comment 15•9 years ago
|
||
Can we remove the nomination until we have a use case to solve?
Comment 16•9 years ago
|
||
[Tracking Requested - why for this release]: Removed 2.5? nomination to identify use cases and right solution.
No-Jun, can you please add any use cases you think are relevant for this issue?
Thanks
Reporter | ||
Comment 17•9 years ago
|
||
One of the use-cases we had for this was the homescreen (the background of app divisions is a solid colour and its position and size animate - it can also be very large), but the new homescreen operates in a fashion that it wouldn't take advantage of this anymore. I'm unaware of any other specific use-cases.
I think more useful than this immediately would be just adjusting the will-change memory allowance - it's really quite low at the moment and is frequently tripped by our current applications. That's a different issue though.
Comment 18•9 years ago
|
||
Chris would be the better person to know about its specific use cases - I am not sure in which case we would see the layers with animations that are larger than viewport.
Flags: needinfo?(npark)
Comment 19•4 years ago
|
||
Now this isn't necessary at all.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•