Async animations should not be disabled for large color layers

NEW
Unassigned

Status

()

Core
Layout
3 years ago
2 years ago

People

(Reporter: cwiiis, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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

3 years ago
Created attachment 8527778 [details] [diff] [review]
Don't disable layer pre-rendering for large color layers

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

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

3 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: 1067435

Updated

3 years ago
Flags: needinfo?(milan)
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

3 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)
[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?
Hey Jet,

Do you think this can be done as part of 2.5? Thanks
Flags: needinfo?(bugs)

Comment 12

2 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)
(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.
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)
Can we remove the nomination until we have a use case to solve?
[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
blocking-b2g: 2.5? → ---
tracking-b2g: --- → backlog
Flags: needinfo?(npark)
(Reporter)

Comment 17

2 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.
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)
You need to log in before you can comment on or make changes to this bug.